Description
Andrew Isherwood
2022-05-09 11:04:36 UTC
Created attachment 144281 [details] [review] Bug 30719: Batch requests minor misc improvements - Fix wrong link in message in case there are no requests batch - Detail of batch update - View status of any request within a batch - Fix typo to correctly display status code on batch statuses list Created attachment 144285 [details] [review] Bug 30719: DB, API & tests This commit adds the following: - A database update that adds an "illbatches" table, a "illrequest.batch_id" foreign key and an "illbatches.borrowernumber" foreign key - Illbatch and Illbatches Koha objects - Adds 'batch' accessor to Illrequest object - CRUD API endpoints for working with ILL batches - Unit tests of the Koha objects and API endpoints - Adds 'batch_id' filter to /illrequests endpoint Created attachment 144286 [details] [review] Bug 30719: Batch ILL requests - Add API endpoint for ILL request creation - UI for creation of ILL batches, including: - Ability to paste in list of identifier - Auto metadata enrichment for each identifier, using installed third party service API plugins - Auto retrieval of identifier availability, using installed third party service API plugins - Auto creation of local requests within batch - UI for management of existing ILL batches, including adding requests to, and removing requests from, batches - Additional UI to allow users to navigate from requests to their enclosing batches and vice-versa Created attachment 144287 [details] [review] Bug 30719: Logging of batch operations This commit adds logging of the following batch operations: - Batch creation - Batch update (Updating of the batch itself, not its contents) - Batch delete Created attachment 144288 [details] [review] Bug 30719: Batch statuses This commits adds the following provision for batch statuses: - Database updates - Object definitions - CRUD REST API - Unit tests - UI adding support for batch statuses in batch UI - Admin UI for managing batch statuses Created attachment 144289 [details] [review] Bug 30719: DB, API & tests This commit adds the following: - A database update that adds an "illbatches" table, a "illrequest.batch_id" foreign key and an "illbatches.borrowernumber" foreign key - Illbatch and Illbatches Koha objects - Adds 'batch' accessor to Illrequest object - CRUD API endpoints for working with ILL batches - Unit tests of the Koha objects and API endpoints - Adds 'batch_id' filter to /illrequests endpoint Apply all current 5 patches, on ktd, preparation: - Run DBIC - Run reset_all - Enable ILLModule sys pref - Default ILLLIBS patron category needs to exist, create that - At least one ILL batch status needs to exist, admin -> ILL batch statuses, create for example name:NEW status:NEW - At least one ILL Backend with "provide_batch_requests" capability needs to exist, create ReprintsDesk backend from https://github.com/PTFS-Europe/koha-ill-reprintsdesk - Install ReprintsDesk backend dependencies: cpan XML::Compile XML::Smart XML::Compile:WSDL11 XML::Compile::SOAP12 - Need to restart plack here - Update backend_directory in koha-conf.xml to <backend_directory>/kohadevbox/koha/Koha/Illbackends</backend_directory> - Need to restart plack here - Install metadata enrichment plugin pubmedid from https://github.com/PTFS-Europe/koha-plugin-api-pubmed/releases/tag/1.1.0 through "Manage plugins" in admin - Need to restart plack here - Go to ILL Requests admin area, click "New ILL batch request" and select ReprintsDesk - Create new batch (insert name, cardnumber and library) and press "continue" - Add PubMed ID, one per line (example: 23668524\n29710265) and press "Process identifiers" - After processing is complete, click "add items to batch" and then "finish and view batch" Verify that: - The items requested in the batch are now illrequests in Koha, and are part of the batch that was just created - Clicking on "Batch requests" lists all batch requests created - Clicking on a batch name lists only the requests from that batch - If a batch is deleted, whatever requests previously on that batch are not deleted (they have been detached, their batch_id is now NULL) - When editing a request that is part of a batch, it's possible to change which batch the request is part of, or none. Test logs: - Before doing the above, enable illlog sys pref - Do some CRUD actions on batch requests - Check log viewer -> module "Interlibrary loans" - Verify that the actions performed have been logged Run tests: perl t/db_dependent/Illbatches.t perl t/db_dependent/Illbatchstatuses.t #currently failing, will check why soon Created attachment 144290 [details] [review] Bug 30719: Batch requests minor misc improvements - Fix wrong link in message in case there are no requests batch - Detail of batch update - View status of any request within a batch - Fix typo to correctly display status code on batch statuses list Created attachment 144291 [details] [review] Bug 30719: DB, API & tests This commit adds the following: - A database update that adds an "illbatches" table, a "illrequest.batch_id" foreign key and an "illbatches.borrowernumber" foreign key - Illbatch and Illbatches Koha objects - Adds 'batch' accessor to Illrequest object - CRUD API endpoints for working with ILL batches - Unit tests of the Koha objects and API endpoints - Adds 'batch_id' filter to /illrequests endpoint Created attachment 144292 [details] [review] Bug 30719: Batch ILL requests - Add API endpoint for ILL request creation - UI for creation of ILL batches, including: - Ability to paste in list of identifier - Auto metadata enrichment for each identifier, using installed third party service API plugins - Auto retrieval of identifier availability, using installed third party service API plugins - Auto creation of local requests within batch - UI for management of existing ILL batches, including adding requests to, and removing requests from, batches - Additional UI to allow users to navigate from requests to their enclosing batches and vice-versa Created attachment 144293 [details] [review] Bug 30719: Logging of batch operations This commit adds logging of the following batch operations: - Batch creation - Batch update (Updating of the batch itself, not its contents) - Batch delete Created attachment 144294 [details] [review] Bug 30719: Batch statuses This commits adds the following provision for batch statuses: - Database updates - Object definitions - CRUD REST API - Unit tests - UI adding support for batch statuses in batch UI - Admin UI for managing batch statuses Created attachment 144295 [details] [review] Bug 30719: Batch requests minor misc improvements - Fix wrong link in message in case there are no requests batch - Detail of batch update - View status of any request within a batch - Fix typo to correctly display status code on batch statuses list Apply all current 5 patches, on ktd, preparation: - Run DBIC - Run reset_all - Enable ILLModule sys pref - Default ILLLIBS patron category needs to exist, create that - At least one ILL batch status needs to exist, admin -> ILL batch statuses, create for example name:NEW status:NEW - At least one ILL Backend with "provide_batch_requests" capability needs to exist, create ReprintsDesk backend from https://github.com/PTFS-Europe/koha-ill-reprintsdesk - Install ReprintsDesk backend dependencies: cpan XML::Compile XML::Smart XML::Compile:WSDL11 XML::Compile::SOAP12 - Need to restart plack here - Update backend_directory in koha-conf.xml to <backend_directory>/kohadevbox/koha/Koha/Illbackends</backend_directory> - Need to restart plack here - Install metadata enrichment plugin pubmedid from https://github.com/PTFS-Europe/koha-plugin-api-pubmed/releases/tag/1.1.0 through "Manage plugins" in admin - Need to restart plack here - Go to ILL Requests admin area, click "New ILL batch request" and select ReprintsDesk - Create new batch (insert name, cardnumber and library) and press "continue" - Add PubMed ID, one per line (example: 23668524\n29710265) and press "Process identifiers" - After processing is complete, click "add items to batch" and then "finish and view batch" Verify that: - The items requested in the batch are now illrequests in Koha, and are part of the batch that was just created - Clicking on "Batch requests" lists all batch requests created - Clicking on a batch name lists only the requests from that batch - If a batch is deleted, whatever requests previously on that batch are not deleted (they have been detached, their batch_id is now NULL) - When editing a request that is part of a batch, it's possible to change which batch the request is part of, or none. Test logs: - Before doing the above, enable illlog sys pref - Do some CRUD actions on batch requests - Check log viewer -> module "Interlibrary loans" - Verify that the actions performed have been logged Run tests: perl t/db_dependent/Illbatches.t perl t/db_dependent/Illbatchstatuses.t #currently failing, will check why soon Created attachment 144296 [details] [review] Koha 22.11.00 Rosalie is here! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 144297 [details] [review] Koha 22.11.00 Rosalie is here! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 144298 [details] [review] Bug 30719: DB, API & tests This commit adds the following: - A database update that adds an "illbatches" table, a "illrequest.batch_id" foreign key and an "illbatches.borrowernumber" foreign key - Illbatch and Illbatches Koha objects - Adds 'batch' accessor to Illrequest object - CRUD API endpoints for working with ILL batches - Unit tests of the Koha objects and API endpoints - Adds 'batch_id' filter to /illrequests endpoint Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 144299 [details] [review] Bug 30719: Batch ILL requests - Add API endpoint for ILL request creation - UI for creation of ILL batches, including: - Ability to paste in list of identifier - Auto metadata enrichment for each identifier, using installed third party service API plugins - Auto retrieval of identifier availability, using installed third party service API plugins - Auto creation of local requests within batch - UI for management of existing ILL batches, including adding requests to, and removing requests from, batches - Additional UI to allow users to navigate from requests to their enclosing batches and vice-versa Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 144300 [details] [review] Bug 30719: Logging of batch operations This commit adds logging of the following batch operations: - Batch creation - Batch update (Updating of the batch itself, not its contents) - Batch delete Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 144301 [details] [review] Bug 30719: Batch statuses This commits adds the following provision for batch statuses: - Database updates - Object definitions - CRUD REST API - Unit tests - UI adding support for batch statuses in batch UI - Admin UI for managing batch statuses Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 144302 [details] [review] Bug 30719: Batch requests minor misc improvements - Fix wrong link in message in case there are no requests batch - Detail of batch update - View status of any request within a batch - Fix typo to correctly display status code on batch statuses list Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Looking at bug history, I am asking for another signoff please. I'm asking the customer whose sponsored this work to SO. We got a bit ahead of ourselves here. Created attachment 145760 [details] [review] Bug 30719: (QA Follow-up): - Fixed an issue with illrequest api definition that prevented the listing of any ILL requests if if any had a status_alias - Updated "Add items to batch" css class to match new 22.11 style - Fixed conflict issue where it prevented the creation of a request in a backend that has JS field validation utilizing the #cardnumber selector - Added a check for metadata enrichment plugins and bail batches JS code if none are found, this prevents a console error - Remove ysearch from patron auto complete and use new standard one. FIXME: The dialog modal .modal has a z-index of 1050 but the autocomplete patron suggestions .ui-front has a z-index of 100, it's working but shows behind the modal and is unclickable. Created attachment 145869 [details] [review] Bug 30719: (QA Follow-up): - Fix wrong link in message in case there are no requests batch - Detail of batch update - View status of any request within a batch - Fix typo to correctly display status code on batch statuses list - Fixed an issue with illrequest api definition that prevented the listing of any ILL requests if if any had a status_alias - Updated "Add items to batch" css class to match new 22.11 style - Fixed conflict issue where it prevented the creation of a request in a backend that has JS field validation utilizing the #cardnumber selector - Added a check for metadata enrichment plugins and bail batches JS code if none are found, this prevents a console error - Remove ysearch from patron auto complete and use new standard one and input cardnumber instead of patronid. Created attachment 145870 [details] [review] Bug 30719: (QA Follow-up): - Fix wrong link in message in case there are no requests batch - Detail of batch update - View status of any request within a batch - Fix typo to correctly display status code on batch statuses list - Fixed an issue with illrequest api definition that prevented the listing of any ILL requests if if any had a status_alias - Updated "Add items to batch" css class to match new 22.11 style - Fixed conflict issue where it prevented the creation of a request in a backend that has JS field validation utilizing the #cardnumber selector - Added a check for metadata enrichment plugins and bail batches JS code if none are found, this prevents a console error - Remove ysearch from patron auto complete and use new standard one and input cardnumber instead of patronid. Sponsored-by: UKHSA Created attachment 145876 [details] [review] Bug 30719: (QA Follow-up): - Fix wrong link 'Create a new batch status message' in case there are no existing batch statuses - Detail of batch update - View status of any request within a batch - Fix typo to correctly display status code on batch statuses list page - Fixed an issue with illrequest api definition that prevented the listing of any ILL requests if any request had a status_alias - Updated "Add items to batch" css class to match new 22.11 style - Fixed conflict issue where it prevented the creation of a single request in a backend that is using JS field validation utilizing the #cardnumber selector - Added a check for metadata enrichment plugins and bail batches JS code if none are found, this prevents a console error - Remove ysearch from patron auto complete in batch creation and use patron_autocomplete instead - 'New batch' and 'list batches' buttons no longer show if no metadata plugin is installed Sponsored-by: UKHSA Created attachment 145934 [details] [review] Bug 30719: (QA Follow-up): - Fix wrong link 'Create a new batch status message' in case there are no existing batch statuses - Detail of batch update - View status of any request within a batch - Fix typo to correctly display status code on batch statuses list page - Fixed an issue with illrequest api definition that prevented the listing of any ILL requests if any request had a status_alias - Updated "Add items to batch" css class to match new 22.11 style - Fixed conflict issue where it prevented the creation of a single request in a backend that is using JS field validation utilizing the #cardnumber selector - Added a check for metadata enrichment plugins and bail batches JS code if none are found, this prevents a console error - Remove ysearch from patron auto complete in batch creation and use patron_autocomplete instead - 'New batch' and 'list batches' buttons no longer show if no metadata plugin is installed - Fixed tests, all passing now - Improved error messaging for non-existent cardnumber input in patron in ill batch creation - Improved error messaging for duplicate batch name creation error Sponsored-by: UKHSA Apply patches, on ktd, preparation: - Run dbic - Run reset_all - Enable ILLModule sys pref - At least one ILL batch status needs to exist, admin -> (bottom right) Interlibrary Loan batch statuses, add that (name:NEW status:NEW) - At least one ILL Backend with "provide_batch_requests" capability needs to exist, add ReprintsDesk backend from https://github.com/PTFS-Europe/koha-ill-reprintsdesk to kohadevbox/koha/Koha/Illbackends with name 'ReprintsDesk' - Update backend_directory in koha-conf.xml to <backend_directory>/kohadevbox/koha/Koha/Illbackends</backend_directory> - At least one metadata enrichment plugin needs to exist, admin -> Manage plugins add that (https://github.com/PTFS-Europe/koha-plugin-api-pubmed/releases/tag/1.1.0) - restart_all - Run yarn build to update CSS Test plan: - Go to ILL Requests admin area, click "New ILL batch request" and select ReprintsDesk - Create new batch (insert name, cardnumber and library) and press "continue" - Add PubMed ID, one per line (example: 23668524\n29710265) and press "Process identifiers" - After processing is complete, click "add items to batch" and then "finish and view batch" Verify that: - The items requested in the batch are now illrequests in Koha, and are part of the batch that was just created - Clicking on "Batch requests" lists all existing batches - From the previous step, clicking on a batch name lists only the requests from that batch - If a batch is deleted, whatever requests that were previously on that batch are not deleted (their batch_id is now NULL) - When editing a request that is part of a batch, it's possible to change which batch the request is part of, or none. (Optional) Test logging: - Before doing the above, enable illlog sys pref - Do some CRUD actions on batch requests - Check log viewer -> module "Interlibrary loans" - Verify that the actions performed have been logged (Optional) Test error messaging: - Try creating a batch with an already existing batch name, verify it doesn't create and error message shows appropriately - Try creating a batch with a non-existent borrower cardnumber, verifiy it doesn't create the batch and error message shows appropriately Run tests and ensure all are passing: prove -v t/db_dependent/Illbatches.t prove -v t/db_dependent/api/v1/illbatches.t prove -v t/db_dependent/IllbatchStatuses.t prove -v t/db_dependent/api/v1/illbatchstatuses.t Created attachment 146152 [details] [review] Bug 30719: (QA Follow-up): - Fix wrong link 'Create a new batch status message' in case there are no existing batch statuses - Detail of batch update - View status of any request within a batch - Fix typo to correctly display status code on batch statuses list page - Fixed an issue with illrequest api definition that prevented the listing of any ILL requests if any request had a status_alias - Updated "Add items to batch" css class to match new 22.11 style - Fixed conflict issue where it prevented the creation of a single request in a backend that is using JS field validation utilizing the #cardnumber selector - Added a check for metadata enrichment plugins and bail batches JS code if none are found, this prevents a console error - Remove ysearch from patron auto complete in batch creation and use patron_autocomplete instead - 'New batch' and 'list batches' buttons no longer show if no metadata plugin is installed - Fixed tests, all passing now - Improved error messaging for non-existent cardnumber input in patron in ill batch creation - Improved error messaging for duplicate batch name creation error - Fix sidemenu filters and search not working after new batch_id column Sponsored-by: UKHSA Created attachment 147658 [details] [review] Bug 30719: (QA Follow-up): - Fix wrong link 'Create a new batch status message' in case there are no existing batch statuses - Detail of batch update - View status of any request within a batch - Fix typo to correctly display status code on batch statuses list page - Fixed an issue with illrequest api definition that prevented the listing of any ILL requests if any request had a status_alias - Updated "Add items to batch" css class to match new 22.11 style - Fixed conflict issue where it prevented the creation of a single request in a backend that is using JS field validation utilizing the #cardnumber selector - Added a check for metadata enrichment plugins and bail batches JS code if none are found, this prevents a console error - Remove ysearch from patron auto complete in batch creation and use patron_autocomplete instead - 'New batch' and 'list batches' buttons no longer show if no metadata plugin is installed - Fixed tests, all passing now - Improved error messaging for non-existent cardnumber input in patron in ill batch creation - Improved error messaging for duplicate batch name creation error - Fix sidemenu filters and search not working after new batch_id column - Move 'add items to batch' button to bottom right and make button bigger - Add a11y aria tags - Update cancel/close button label depending on step of batch creation - Keep 'finish and see batch' button disabled until 'add items to batch' has been clicked Sponsored-by: UKHSA Created attachment 147929 [details] [review] Bug 30719: Batch ILL requests - Add API endpoint for ILL request creation - UI for creation of ILL batches, including: - Ability to paste in list of identifier - Auto metadata enrichment for each identifier, using installed third party service API plugins - Auto retrieval of identifier availability, using installed third party service API plugins - Auto creation of local requests within batch - UI for management of existing ILL batches, including adding requests to, and removing requests from, batches - Additional UI to allow users to navigate from requests to their enclosing batches and vice-versa Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 147930 [details] [review] Bug 30719: DB, API & tests This commit adds the following: - A database update that adds an "illbatches" table, a "illrequest.batch_id" foreign key and an "illbatches.borrowernumber" foreign key - Illbatch and Illbatches Koha objects - Adds 'batch' accessor to Illrequest object - CRUD API endpoints for working with ILL batches - Unit tests of the Koha objects and API endpoints - Adds 'batch_id' filter to /illrequests endpoint Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 147931 [details] [review] Bug 30719: Batch ILL requests - Add API endpoint for ILL request creation - UI for creation of ILL batches, including: - Ability to paste in list of identifier - Auto metadata enrichment for each identifier, using installed third party service API plugins - Auto retrieval of identifier availability, using installed third party service API plugins - Auto creation of local requests within batch - UI for management of existing ILL batches, including adding requests to, and removing requests from, batches - Additional UI to allow users to navigate from requests to their enclosing batches and vice-versa Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 147932 [details] [review] Bug 30719: Logging of batch operations This commit adds logging of the following batch operations: - Batch creation - Batch update (Updating of the batch itself, not its contents) - Batch delete Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 147933 [details] [review] Bug 30719: Batch statuses This commits adds the following provision for batch statuses: - Database updates - Object definitions - CRUD REST API - Unit tests - UI adding support for batch statuses in batch UI - Admin UI for managing batch statuses Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 147934 [details] [review] Bug 30719: (QA Follow-up): - Fix wrong link 'Create a new batch status message' in case there are no existing batch statuses - Detail of batch update - View status of any request within a batch - Fix typo to correctly display status code on batch statuses list page - Fixed an issue with illrequest api definition that prevented the listing of any ILL requests if any request had a status_alias - Updated "Add items to batch" css class to match new 22.11 style - Fixed conflict issue where it prevented the creation of a single request in a backend that is using JS field validation utilizing the #cardnumber selector - Added a check for metadata enrichment plugins and bail batches JS code if none are found, this prevents a console error - Remove ysearch from patron auto complete in batch creation and use patron_autocomplete instead - 'New batch' and 'list batches' buttons no longer show if no metadata plugin is installed - Fixed tests, all passing now - Improved error messaging for non-existent cardnumber input in patron in ill batch creation - Improved error messaging for duplicate batch name creation error - Fix sidemenu filters and search not working after new batch_id column - Move 'add items to batch' button to bottom right and make button bigger - Add a11y aria tags - Update cancel/close button label depending on step of batch creation - Keep 'finish and see batch' button disabled until 'add items to batch' has been clicked Sponsored-by: UKHSA Created attachment 147952 [details] [review] Bug 30719: DB, API & tests This commit adds the following: - A database update that adds an "illbatches" table, a "illrequest.batch_id" foreign key and an "illbatches.borrowernumber" foreign key - Illbatch and Illbatches Koha objects - Adds 'batch' accessor to Illrequest object - CRUD API endpoints for working with ILL batches - Unit tests of the Koha objects and API endpoints - Adds 'batch_id' filter to /illrequests endpoint Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 147953 [details] [review] Bug 30719: Batch ILL requests - Add API endpoint for ILL request creation - UI for creation of ILL batches, including: - Ability to paste in list of identifier - Auto metadata enrichment for each identifier, using installed third party service API plugins - Auto retrieval of identifier availability, using installed third party service API plugins - Auto creation of local requests within batch - UI for management of existing ILL batches, including adding requests to, and removing requests from, batches - Additional UI to allow users to navigate from requests to their enclosing batches and vice-versa Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 147954 [details] [review] Bug 30719: Logging of batch operations This commit adds logging of the following batch operations: - Batch creation - Batch update (Updating of the batch itself, not its contents) - Batch delete Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 147955 [details] [review] Bug 30719: Batch statuses This commits adds the following provision for batch statuses: - Database updates - Object definitions - CRUD REST API - Unit tests - UI adding support for batch statuses in batch UI - Admin UI for managing batch statuses Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 147956 [details] [review] Bug 30719: (QA Follow-up): - Fix wrong link 'Create a new batch status message' in case there are no existing batch statuses - Detail of batch update - View status of any request within a batch - Fix typo to correctly display status code on batch statuses list page - Fixed an issue with illrequest api definition that prevented the listing of any ILL requests if any request had a status_alias - Updated "Add items to batch" css class to match new 22.11 style - Fixed conflict issue where it prevented the creation of a single request in a backend that is using JS field validation utilizing the #cardnumber selector - Added a check for metadata enrichment plugins and bail batches JS code if none are found, this prevents a console error - Remove ysearch from patron auto complete in batch creation and use patron_autocomplete instead - 'New batch' and 'list batches' buttons no longer show if no metadata plugin is installed - Fixed tests, all passing now - Improved error messaging for non-existent cardnumber input in patron in ill batch creation - Improved error messaging for duplicate batch name creation error - Fix sidemenu filters and search not working after new batch_id column - Move 'add items to batch' button to bottom right and make button bigger - Add a11y aria tags - Update cancel/close button label depending on step of batch creation - Keep 'finish and see batch' button disabled until 'add items to batch' has been clicked Sponsored-by: UKHSA Didn't apply. Rebased. New rebase on top of bug 22440 coming soon Created attachment 151152 [details] [review] Bug 30719: DB and API - Adds 'batch' accessor to Illrequest object - New illbatches and illbatch_statuses tables - New foreign key 'batch_id' in illrequests table - Atomic update file - Default illbatch_statuses - Add 'add' ill_requests api method - Add POST method in ill_requests path - Add 'batch_id property to ill_request api definition - Updated swagger.yml with new batches and batchstatuses endpoints Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 151153 [details] [review] Bug 30719: ILL Batches - Add batch column to requests table - Establish if there are any availability or metadata enrichment plugins and pass that to the template - Verify if we have any backend that can support batches, if not, don't show the option - Updates to the ILL toolbar - New ILL batch modal - New Koha classes - API specs Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 151154 [details] [review] Bug 30719: ILL Batch Statuses - UI adding support for batch statuses in batch UI - Admin UI for managing batch statuses - API specs Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 151155 [details] [review] Bug 30719: Tests Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 151156 [details] [review] Bug 30719: dbic specific, DONT PUSH This has now been rebased on top of 22440 and 33716 and is ready for testing. Created attachment 151284 [details] [review] Bug 30719: DB and API - Adds 'batch' accessor to Illrequest object - New illbatches and illbatch_statuses tables - New foreign key 'batch_id' in illrequests table - Atomic update file - Default illbatch_statuses - Add 'add' ill_requests api method - Add POST method in ill_requests path - Add 'batch_id property to ill_request api definition - Updated swagger.yml with new batches and batchstatuses endpoints Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 151285 [details] [review] Bug 30719: ILL Batches - Add batch column to requests table - Establish if there are any availability or metadata enrichment plugins and pass that to the template - Verify if we have any backend that can support batches, if not, don't show the option - Updates to the ILL toolbar - New ILL batch modal - New Koha classes - API specs Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 151286 [details] [review] Bug 30719: ILL Batch Statuses - UI adding support for batch statuses in batch UI - Admin UI for managing batch statuses - API specs Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 151287 [details] [review] Bug 30719: Tests Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 151288 [details] [review] Bug 30719: dbic specific, DONT PUSH Created attachment 151303 [details] [review] Bug 30719: DB and API - Adds 'batch' accessor to Illrequest object - New illbatches and illbatch_statuses tables - New foreign key 'batch_id' in illrequests table - Atomic update file - Default illbatch_statuses - Add 'add' ill_requests api method - Add POST method in ill_requests path - Add 'batch_id property to ill_request api definition - Updated swagger.yml with new batches and batchstatuses endpoints Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 151304 [details] [review] Bug 30719: ILL Batches - Add batch column to requests table - Establish if there are any availability or metadata enrichment plugins and pass that to the template - Verify if we have any backend that can support batches, if not, don't show the option - Updates to the ILL toolbar - New ILL batch modal - New Koha classes - API specs Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 151305 [details] [review] Bug 30719: ILL Batch Statuses - UI adding support for batch statuses in batch UI - Admin UI for managing batch statuses - API specs Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 151306 [details] [review] Bug 30719: Tests Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 151307 [details] [review] Bug 30719: dbic specific, DONT PUSH Created attachment 151308 [details] [review] Bug 30719: DB and API - Adds 'batch' accessor to Illrequest object - New illbatches and illbatch_statuses tables - New foreign key 'batch_id' in illrequests table - Atomic update file - Default illbatch_statuses - Add 'add' ill_requests api method - Add POST method in ill_requests path - Add 'batch_id property to ill_request api definition - Updated swagger.yml with new batches and batchstatuses endpoints Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 151309 [details] [review] Bug 30719: ILL Batches - Add batch column to requests table - Establish if there are any availability or metadata enrichment plugins and pass that to the template - Verify if we have any backend that can support batches, if not, don't show the option - Updates to the ILL toolbar - New ILL batch modal - New Koha classes - API specs Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 151310 [details] [review] Bug 30719: ILL Batch Statuses - UI adding support for batch statuses in batch UI - Admin UI for managing batch statuses - API specs Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 151311 [details] [review] Bug 30719: Tests Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 151312 [details] [review] Bug 30719: dbic specific, DONT PUSH Created attachment 151513 [details] [review] Bug 30719: DB and API - Adds 'batch' accessor to Illrequest object - New illbatches and illbatch_statuses tables - New foreign key 'batch_id' in illrequests table - Atomic update file - Default illbatch_statuses - Add 'add' ill_requests api method - Add POST method in ill_requests path - Add 'batch_id property to ill_request api definition - Updated swagger.yml with new batches and batchstatuses endpoints Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 151514 [details] [review] Bug 30719: ILL Batches - Add batch column to requests table - Establish if there are any availability or metadata enrichment plugins and pass that to the template - Verify if we have any backend that can support batches, if not, don't show the option - Updates to the ILL toolbar - New ILL batch modal - New Koha classes - API specs Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 151515 [details] [review] Bug 30719: ILL Batch Statuses - UI adding support for batch statuses in batch UI - Admin UI for managing batch statuses - API specs Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 151516 [details] [review] Bug 30719: Tests Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 151517 [details] [review] Bug 30719: dbic specific, DONT PUSH Created attachment 151519 [details] [review] Bug 30719: DB and API - Adds 'batch' accessor to Illrequest object - New illbatches and illbatch_statuses tables - New foreign key 'batch_id' in illrequests table - Atomic update file - Default illbatch_statuses - Add 'add' ill_requests api method - Add POST method in ill_requests path - Add 'batch_id property to ill_request api definition - Updated swagger.yml with new batches and batchstatuses endpoints Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 151520 [details] [review] Bug 30719: ILL Batches - Add batch column to requests table - Establish if there are any availability or metadata enrichment plugins and pass that to the template - Verify if we have any backend that can support batches, if not, don't show the option - Updates to the ILL toolbar - New ILL batch modal - New Koha classes - API specs Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 151521 [details] [review] Bug 30719: ILL Batch Statuses - UI adding support for batch statuses in batch UI - Admin UI for managing batch statuses - API specs Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 151522 [details] [review] Bug 30719: Tests Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 151523 [details] [review] Bug 30719: dbic specific, DONT PUSH Created attachment 151571 [details] [review] Bug 30719: DB and API - Adds 'batch' accessor to Illrequest object - New illbatches and illbatch_statuses tables - New foreign key 'batch_id' in illrequests table - Atomic update file - Default illbatch_statuses - Add 'add' ill_requests api method - Add POST method in ill_requests path - Add 'batch_id property to ill_request api definition - Updated swagger.yml with new batches and batchstatuses endpoints Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Created attachment 151572 [details] [review] Bug 30719: ILL Batches - Add batch column to requests table - Establish if there are any availability or metadata enrichment plugins and pass that to the template - Verify if we have any backend that can support batches, if not, don't show the option - Updates to the ILL toolbar - New ILL batch modal - New Koha classes - API specs Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Created attachment 151573 [details] [review] Bug 30719: ILL Batch Statuses - UI adding support for batch statuses in batch UI - Admin UI for managing batch statuses - API specs Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Created attachment 151574 [details] [review] Bug 30719: Tests Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Created attachment 151575 [details] [review] Bug 30719: dbic specific, DONT PUSH Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Created attachment 151611 [details] [review] Bug 30719: DB and API - Adds 'batch' accessor to Illrequest object - New illbatches and illbatch_statuses tables - New foreign key 'batch_id' in illrequests table - Atomic update file - Default illbatch_statuses - Add 'add' ill_requests api method - Add POST method in ill_requests path - Add 'batch_id property to ill_request api definition - Updated swagger.yml with new batches and batchstatuses endpoints Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Created attachment 151612 [details] [review] Bug 30719: ILL Batches - Add batch column to requests table - Establish if there are any availability or metadata enrichment plugins and pass that to the template - Verify if we have any backend that can support batches, if not, don't show the option - Updates to the ILL toolbar - New ILL batch modal - New Koha classes - API specs Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Created attachment 151613 [details] [review] Bug 30719: ILL Batch Statuses - UI adding support for batch statuses in batch UI - Admin UI for managing batch statuses - API specs Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Created attachment 151614 [details] [review] Bug 30719: Tests Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Created attachment 151615 [details] [review] Bug 30719: dbic specific, DONT PUSH Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> rebased after bug 33762 Created attachment 151804 [details] [review] Bug 33786: ILL requests table id Make sure requests table is unique when visiting patron ILL history so the table state is not shared unintentionally Reproduce: 1) Have a borrower with >20 ILL requests in their history 2) Visit cgi-bin/koha/members/ill-requests.pl?borrowernumber=<borrowernumber> 3) On the table, click page 2 4) Visit a different borrower with <20 ILL requests 5) Verify that no requests are shown, this is because the table is using page 2 from step 3) 6) Go back to original borrower, click table page 1 7) Now go back to 2nd borrower, verify is now showing page 1 correctly Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 151805 [details] [review] Bug 30719: DB and API - Adds 'batch' accessor to Illrequest object - New illbatches and illbatch_statuses tables - New foreign key 'batch_id' in illrequests table - Atomic update file - Default illbatch_statuses - Add 'add' ill_requests api method - Add POST method in ill_requests path - Add 'batch_id property to ill_request api definition - Updated swagger.yml with new batches and batchstatuses endpoints Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Created attachment 151806 [details] [review] Bug 30719: ILL Batches - Add batch column to requests table - Establish if there are any availability or metadata enrichment plugins and pass that to the template - Verify if we have any backend that can support batches, if not, don't show the option - Updates to the ILL toolbar - New ILL batch modal - New Koha classes - API specs Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Created attachment 151807 [details] [review] Bug 30719: ILL Batch Statuses - UI adding support for batch statuses in batch UI - Admin UI for managing batch statuses - API specs Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Created attachment 151808 [details] [review] Bug 30719: Tests Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Created attachment 151809 [details] [review] Bug 30719: dbic specific, DONT PUSH Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> rebased for bug 33716 rebase for bug 33786 The dependency to bug 33716 is a bit of an issue here, we need to see how to resolve that. As I signed it off I can't QA, but it will send this one to BLOCKED even if passing. I skipped the last patch (DBIC) and ran dbic manually for testing as there was a conflict. 1) QA script FAIL koha-tmpl/intranet-tmpl/prog/en/includes/ill-batch-modal.inc FAIL valid_template options_for_libraries: not found FAIL installer/data/mysql/kohastructure.sql FAIL boolean_vs_tinyint WARNING - The new column (is_system) for table illbatch_statuses is using INT(1) as type, must be TINYINT(1) if it has a boolean purpose, see the SQL12 coding guideline FAIL koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt FAIL filters wrong_html_filter at line 669 ( <a href="/cgi-bin/koha/ill/ill-requests.pl?batch_id=[% request.batch.id | html %]">) 2a) Database update is not idempotent Easy fix with our existing check methods: DEV atomic update /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_30719_add_ill_batches.pl [11:39:04]: Bug 30719 - Add ILL batches ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Duplicate column name 'batch_id' at /kohadevbox/koha/C4/Installer.pm line 741 2b) Database update and kohastructure are not using COMMENT We standardized on using COMMENT as they display in schema.koha-community.org etc. Could you please move the comments to COMMENT syntax? 3) Unit tests are failing (after running db Update and dbic) Test Summary Report ------------------- t/db_dependent/api/v1/ill_requests.t (Wstat: 512 Tests: 2 Failed: 2) Failed tests: 1-2 Non-zero exit status: 2 Files=5, Tests=33, 21 wallclock secs ( 0.03 usr 0.02 sys + 18.99 cusr 1.70 csys = 20.74 CPU) Result: FAIL 4) Perltidy It might be nice (not blocker) to run the new perltidy over the code when working on it, thinking especially of the new files. There are some small inconsistencies like spaces and such: +use JSON qw( to_json ); +use base qw(Koha::Object); 5) FIXMEs Koha/REST/V1/Illbatches.pm + #FIXME: This should be $c->objects-search + my @batches = Koha::Illbatches->search()->as_list; + + #FIXME: Below should be coming from $c->objects accessors + # Get all patrons associated with all our batches +# FIXME: This should be moved to Koha::Illbackend +sub can_batch { + // TODO: need to also reset progress bar and already processed identifiers + // FIXME: This should be a kohaTable not KohaTable Are these something we should change now or at least have a follow-up bug for filed? 6) API If I understand correctly the API is not using the common/standardized terminology: + id: + type: string + description: Internal ILL batch identifier Should be batch_id or maybe ill_batch_id? (compared with other API routes: hold_id, patron_id, etc.) + borrowernumber: + type: string + description: Borrower number of the patron of the ILL batch Should be: patron_id + branchcode: + type: string + description: Branch code of the branch of the ILL batch Should be library_id + branch: + type: + - object + - "null" + description: The branch associated with the batch Should possibly be library + statuscode: + type: string + description: Code of the status of the ILL batch Should probably be status_code Similar for illbatchstatus.yaml: id: ill_batch_status_id ? 7a) Illbatch_statuses are not translatable They should probably be moved to en, like we just did with the debit types and credit types too. For reporting it's nice when the description can be translated. +++ b/installer/data/mysql/mandatory/illbatch_statuses.sql @@ -0,0 +1,5 @@ +INSERT INTO illbatch_statuses ( name, code, is_system ) VALUES +('New', 'NEW', 1), +('In progress', 'IN_PROGRESS', 1), +('Completed', 'COMPLETED', 1), +('Unknown', 'UNKNOWN', 1); 7b) Strings can be transalted inside the .js files We don't really need: ill-batch-modal-strings.inc ill-batch-table-strings.inc This file is probably no longer needed as we can now translate strings in .js files. Not blocker, could still be moved later. It might simplify the code in koha-tmpl/intranet-tmpl/prog/js/ill-batch-modal.js quite a bit. Also: + var ill_batch_item_remove = _("Are you sure you want to remove this item from the batch"); Missing question mark at the end? 7c) Avoid "building" sentences from multiple parts + <a href="#" aria-current="page"> + [% IF status.id %] + Modify + [% ELSE %] + New + [% END %] batch status + </a> This will end up to be something like: %sModify%New%batch status. This is hard to read, but also harder to translate. Imaging having a language where you need to change the sequence, to be "Batch status edit". Better: + <a href="#" aria-current="page"> + [% IF status.id %] + <span>Modify batch status</span> + [% ELSE %] + <span>New batch status</span> + [% END %] + </a> The span would be to make both separate strings. The parsing looks for HTML tags. repeating the <a> inside the IF-ELSE would also work. Also here: + <h1> + View ILL requests + [% IF batch %] + for batch "[% batch.name | html %]" + [% END %] + </h1> 7d) Not sure if this is translatable I have doubts about this one here: <strong>[% status.is_system ? "Yes" : "No" | html %]</strong> I think we need to verify or just move to an IF-ELSE construct. 8) Capitalization + <li><a href="/cgi-bin/koha/admin/ill_batch_statuses.pl">Interlibrary Loan batch statuses</a></li> Interlibrary loan... + <th scope="col">Request Status</th> Request status <dt><a href="/cgi-bin/koha/admin/ill_batch_statuses.pl">Interlibrary Loan batch statuses</a></dt> + Interlibrary Loan batch statuses › Administration › Koha + <a href="/cgi-bin/koha/admin/ill_batch_statuses.pl">Interlibrary Loan batch statuses</a> Maybe to a search replace for Interlibrary Loan? There are many more... 9) Terminology + <th scope="col">Branch</th> Should be Library. 10) Page titles and breadcrumbs Need updating to follow Owen's latest work using Template WRAPPER etc. koha-tmpl/intranet-tmpl/prog/en/modules/admin/ill_batch_statuses.tt 11) Using page-section and btn-primary This predates the staff interface redesign work and needs some touch ups for yellow main buttons and white backgrounds. 12) Null? This looks like it might display "null" when either one is empty (unconfirmed): + var displayText = [data.firstname, data.surname].join(' ') + ' ( ' + data.cardnumber + ' )'; Katrin, thank you for your amazing QA. I have 18 patches that address your observations here: https://github.com/PTFS-Europe/koha/commits/new_30719 Please have another look and then let me know how to proceed, if there's more fixing required, should I submit all these patches here or provide a squashed version? I did not address 11) directly because I'm not sure where exactly to apply it. Thank you! Sorry, but the database update doesn't work: DEV atomic update /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_30719_add_ill_batches.pl [12:29:56]: Bug 30719 - Add ILL batches ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Can't create table `koha_kohadev`.`illbatches` (errno: 150 "Foreign key constraint is incorrectly formed") at /kohadevbox/koha/C4/Installer.pm line 741 I used the remote branch and did a rebase on master. After a chat with Katrin, these are the QA points we're still missing: - add mandatory batch statuses on the atomicupdate - double-check description field for batch statuses - add page section on the batch statuses list - style "Save" button on batch status edit page - "New ILL batch request" should be "New ill requests batch" - Are you sure you want to remove this item from the batch should be "Are you sure you want to remove this request from the batch?" - add requests modal - check for noExport class for the "remove" column - batches list table - check for noExport class for the actions column - add page-section to the ill batches list table - perltidy I hope to be able to finish this in the near future and comeback when that's done. Thx for the summary Pedro, I believe we are close! Created attachment 154046 [details] [review] Bug 30719: (QA follow-up squash): This is a squash of 25 QA patches located at: https://github.com/PTFS-Europe/koha/commits/new_30719 Bug 30719: (QA follow-up) Batch column should be hidden by default Bug 30719: (QA follow-up) Fix wrong tt filter type Bug 30719: (QA follow-up) Make atomicupdate idempotent Bug 30719: (QA follow-up) Use COMMENT syntax in database files Bug 30719: (QA follow-up) Fix tiny boolean is_system Bug 30719: (QA follow-up) Add missing CONSTRAINT entries from kohastructure.sql to the atomicupdate file Bug 30719: (QA follow-up) Add missing koha_object_class and koha_objects_class methods Bug 30719: (QA follow-up) Swap search to find Bug 30719: (QA follow-up) Fix tests Bug 30719: (QA follow-up) API terminology - id -> batch_id Bug 30719: (QA follow-up) API terminology - borrowernumber -> patron_id Bug 30719: (QA follow-up) API terminology - branchcode -> library_id Bug 30719: (QA follow-up) Make mandatory illbatch_statuses translatable Bug 30719: (QA follow-up) Improve translatability Bug 30719: (QA follow-up) Fix capitalization of Interlibrary Loan Bug 30719: (QA follow-up) Change Branch to Library in ILL batches table Bug 30719: (QA follow-up) Add template WRAPPER to batch statuses breadrcrumbs Bug 30719: (QA follow-up) Utilize patron_to_html function to display patron info in batches table Bug 30719: (QA follow-up) Add mandatory batch statuses to the atomicupdate Bug 30719: (QA follow-up) Add page-section to the batch statuses list page Bug 30719: (QA follow-up) Style Save button on batch status edit page Bug 30719: (QA follow-up) Add question mark to label string, rephrase new ILL batch button Bug 30719: (QA follow-up) Add noExport class to action columns in batch list table and batch modal table Bug 30719: (QA follow-up) Add page-section and headers to ILL batch table Bug 30719: (QA follow-up) Perltidy Created attachment 154047 [details] [review] Bug 30719: DB and API - Adds 'batch' accessor to Illrequest object - New illbatches and illbatch_statuses tables - New foreign key 'batch_id' in illrequests table - Atomic update file - Default illbatch_statuses - Add 'add' ill_requests api method - Add POST method in ill_requests path - Add 'batch_id property to ill_request api definition - Updated swagger.yml with new batches and batchstatuses endpoints Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Created attachment 154048 [details] [review] Bug 30719: ILL Batches - Add batch column to requests table - Establish if there are any availability or metadata enrichment plugins and pass that to the template - Verify if we have any backend that can support batches, if not, don't show the option - Updates to the ILL toolbar - New ILL batch modal - New Koha classes - API specs Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Created attachment 154049 [details] [review] Bug 30719: ILL Batch Statuses - UI adding support for batch statuses in batch UI - Admin UI for managing batch statuses - API specs Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Created attachment 154050 [details] [review] Bug 30719: Tests Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Created attachment 154051 [details] [review] Bug 30719: dbic specific, DONT PUSH Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Created attachment 154052 [details] [review] Bug 30719: (QA follow-up squash): This is a squash of 25 QA patches located at: https://github.com/PTFS-Europe/koha/commits/new_30719 Bug 30719: (QA follow-up) Batch column should be hidden by default Bug 30719: (QA follow-up) Fix wrong tt filter type Bug 30719: (QA follow-up) Make atomicupdate idempotent Bug 30719: (QA follow-up) Use COMMENT syntax in database files Bug 30719: (QA follow-up) Fix tiny boolean is_system Bug 30719: (QA follow-up) Add missing CONSTRAINT entries from kohastructure.sql to the atomicupdate file Bug 30719: (QA follow-up) Add missing koha_object_class and koha_objects_class methods Bug 30719: (QA follow-up) Swap search to find Bug 30719: (QA follow-up) Fix tests Bug 30719: (QA follow-up) API terminology - id -> batch_id Bug 30719: (QA follow-up) API terminology - borrowernumber -> patron_id Bug 30719: (QA follow-up) API terminology - branchcode -> library_id Bug 30719: (QA follow-up) Make mandatory illbatch_statuses translatable Bug 30719: (QA follow-up) Improve translatability Bug 30719: (QA follow-up) Fix capitalization of Interlibrary Loan Bug 30719: (QA follow-up) Change Branch to Library in ILL batches table Bug 30719: (QA follow-up) Add template WRAPPER to batch statuses breadrcrumbs Bug 30719: (QA follow-up) Utilize patron_to_html function to display patron info in batches table Bug 30719: (QA follow-up) Add mandatory batch statuses to the atomicupdate Bug 30719: (QA follow-up) Add page-section to the batch statuses list page Bug 30719: (QA follow-up) Style Save button on batch status edit page Bug 30719: (QA follow-up) Add question mark to label string, rephrase new ILL batch button Bug 30719: (QA follow-up) Add noExport class to action columns in batch list table and batch modal table Bug 30719: (QA follow-up) Add page-section and headers to ILL batch table Bug 30719: (QA follow-up) Perltidy Rebased on top of master and fixed the DBIC conflict. Katrin, all yours when you're available! Thank you very much! Created attachment 155421 [details] [review] Bug 30719: DB and API - Adds 'batch' accessor to Illrequest object - New illbatches and illbatch_statuses tables - New foreign key 'batch_id' in illrequests table - Atomic update file - Default illbatch_statuses - Add 'add' ill_requests api method - Add POST method in ill_requests path - Add 'batch_id property to ill_request api definition - Updated swagger.yml with new batches and batchstatuses endpoints Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Created attachment 155422 [details] [review] Bug 30719: ILL Batches - Add batch column to requests table - Establish if there are any availability or metadata enrichment plugins and pass that to the template - Verify if we have any backend that can support batches, if not, don't show the option - Updates to the ILL toolbar - New ILL batch modal - New Koha classes - API specs Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Created attachment 155423 [details] [review] Bug 30719: ILL Batch Statuses - UI adding support for batch statuses in batch UI - Admin UI for managing batch statuses - API specs Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Created attachment 155424 [details] [review] Bug 30719: Tests Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Created attachment 155425 [details] [review] Bug 30719: dbic specific, DONT PUSH Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Created attachment 155426 [details] [review] Bug 30719: (QA follow-up) Squash: This is a squash of 25 QA patches located at: https://github.com/PTFS-Europe/koha/commits/new_30719 Bug 30719: (QA follow-up) Batch column should be hidden by default Bug 30719: (QA follow-up) Fix wrong tt filter type Bug 30719: (QA follow-up) Make atomicupdate idempotent Bug 30719: (QA follow-up) Use COMMENT syntax in database files Bug 30719: (QA follow-up) Fix tiny boolean is_system Bug 30719: (QA follow-up) Add missing CONSTRAINT entries from kohastructure.sql to the atomicupdate file Bug 30719: (QA follow-up) Add missing koha_object_class and koha_objects_class methods Bug 30719: (QA follow-up) Swap search to find Bug 30719: (QA follow-up) Fix tests Bug 30719: (QA follow-up) API terminology - id -> batch_id Bug 30719: (QA follow-up) API terminology - borrowernumber -> patron_id Bug 30719: (QA follow-up) API terminology - branchcode -> library_id Bug 30719: (QA follow-up) Make mandatory illbatch_statuses translatable Bug 30719: (QA follow-up) Improve translatability Bug 30719: (QA follow-up) Fix capitalization of Interlibrary Loan Bug 30719: (QA follow-up) Change Branch to Library in ILL batches table Bug 30719: (QA follow-up) Add template WRAPPER to batch statuses breadrcrumbs Bug 30719: (QA follow-up) Utilize patron_to_html function to display patron info in batches table Bug 30719: (QA follow-up) Add mandatory batch statuses to the atomicupdate Bug 30719: (QA follow-up) Add page-section to the batch statuses list page Bug 30719: (QA follow-up) Style Save button on batch status edit page Bug 30719: (QA follow-up) Add question mark to label string, rephrase new ILL batch button Bug 30719: (QA follow-up) Add noExport class to action columns in batch list table and batch modal table Bug 30719: (QA follow-up) Add page-section and headers to ILL batch table Bug 30719: (QA follow-up) Perltidy Created attachment 155427 [details] [review] Bug 30719: (QA follow-up) c->validation fixes Created attachment 155428 [details] [review] Bug 30719: DB and API - Adds 'batch' accessor to Illrequest object - New illbatches and illbatch_statuses tables - New foreign key 'batch_id' in illrequests table - Atomic update file - Default illbatch_statuses - Add 'add' ill_requests api method - Add POST method in ill_requests path - Add 'batch_id property to ill_request api definition - Updated swagger.yml with new batches and batchstatuses endpoints Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 155429 [details] [review] Bug 30719: ILL Batches - Add batch column to requests table - Establish if there are any availability or metadata enrichment plugins and pass that to the template - Verify if we have any backend that can support batches, if not, don't show the option - Updates to the ILL toolbar - New ILL batch modal - New Koha classes - API specs Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 155430 [details] [review] Bug 30719: ILL Batch Statuses - UI adding support for batch statuses in batch UI - Admin UI for managing batch statuses - API specs Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 155431 [details] [review] Bug 30719: Tests Co-authored-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 155432 [details] [review] Bug 30719: dbic specific, DONT PUSH Signed-off-by: Edith Speller <Edith.Speller@ukhsa.gov.uk> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 155433 [details] [review] Bug 30719: (QA follow-up) Squash: This is a squash of 25 QA patches located at: https://github.com/PTFS-Europe/koha/commits/new_30719 Bug 30719: (QA follow-up) Batch column should be hidden by default Bug 30719: (QA follow-up) Fix wrong tt filter type Bug 30719: (QA follow-up) Make atomicupdate idempotent Bug 30719: (QA follow-up) Use COMMENT syntax in database files Bug 30719: (QA follow-up) Fix tiny boolean is_system Bug 30719: (QA follow-up) Add missing CONSTRAINT entries from kohastructure.sql to the atomicupdate file Bug 30719: (QA follow-up) Add missing koha_object_class and koha_objects_class methods Bug 30719: (QA follow-up) Swap search to find Bug 30719: (QA follow-up) Fix tests Bug 30719: (QA follow-up) API terminology - id -> batch_id Bug 30719: (QA follow-up) API terminology - borrowernumber -> patron_id Bug 30719: (QA follow-up) API terminology - branchcode -> library_id Bug 30719: (QA follow-up) Make mandatory illbatch_statuses translatable Bug 30719: (QA follow-up) Improve translatability Bug 30719: (QA follow-up) Fix capitalization of Interlibrary Loan Bug 30719: (QA follow-up) Change Branch to Library in ILL batches table Bug 30719: (QA follow-up) Add template WRAPPER to batch statuses breadrcrumbs Bug 30719: (QA follow-up) Utilize patron_to_html function to display patron info in batches table Bug 30719: (QA follow-up) Add mandatory batch statuses to the atomicupdate Bug 30719: (QA follow-up) Add page-section to the batch statuses list page Bug 30719: (QA follow-up) Style Save button on batch status edit page Bug 30719: (QA follow-up) Add question mark to label string, rephrase new ILL batch button Bug 30719: (QA follow-up) Add noExport class to action columns in batch list table and batch modal table Bug 30719: (QA follow-up) Add page-section and headers to ILL batch table Bug 30719: (QA follow-up) Perltidy Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 155434 [details] [review] Bug 30719: (QA follow-up) c->validation fixes Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 155435 [details] [review] Bug 30719: (QA follow-up) Fix "Request Status" capitalization Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Tortured Pedro long enough on this one ;) It's hard to take over someone else's code and I think we made a lot of improvements here trying to bring it closer to current standard. QA tools fail on tidiness (and a false positive for options_for_libraries), but are otherwise ok now. Given the size of this patch set and the age, I am willing to let that slip here. All new fiels have been perltidied. Created attachment 156556 [details] [review] Bug 30719: Atomic update fixes Hi all. I know the history of this bug and am willing to overlook some things, but as-is I believe this requires some more work. High * The GET /illbatches endpoint is not /ill/batchess? * Illbatches.pm is too old school. Entirely relying on fetching the whole resultset: #FIXME: This should be $c->objects-search my @batches = Koha::Illbatches->search()->as_list; * And crafting the response manually, even the 'patron' attribute is done manually instead of an embed * The request body needs to be manually translated into the DB model: my $batch = Koha::Illbatch->new($body); we have Koha::Illbatches->new_from_api to avoid that. But there's obviously a reason it wasn't done like that. Lower * Table names, and column names: this is using old school naming for things. We can solve it on the API front, but even that is not entirely done. Please let me know if there's anything I can do to help you with this. Created attachment 156748 [details] [review] Bug 30719: QA follow-up: Rename illbatches endpoint to ill/batches Created attachment 156749 [details] [review] Bug 30719: QA follow-up: Rewrite Illbatches list endpoint Update accessors Add +strings embed Add x-koha-embed to batches list andpoint Add embed to API call from the front-end Update table to get data from _strings Add x-koha-embed to tests Add strings_map to Illbatch Add to_api_mapping to Illbatch (In reply to Tomás Cohen Arazi from comment #126) > Hi all. I know the history of this bug and am willing to overlook some > things, but as-is I believe this requires some more work. > > High > > * The GET /illbatches endpoint is not /ill/batchess? > * Illbatches.pm is too old school. Entirely relying on fetching the whole > resultset: > #FIXME: This should be $c->objects-search > my @batches = Koha::Illbatches->search()->as_list; > * And crafting the response manually, even the 'patron' attribute is done > manually instead of an embed > * The request body needs to be manually translated into the DB model: > my $batch = Koha::Illbatch->new($body); > we have Koha::Illbatches->new_from_api to avoid that. But there's obviously > a > reason it wasn't done like that. > > Lower > > * Table names, and column names: this is using old school naming for things. > We can solve it on the API front, but even that is not entirely done. > > Please let me know if there's anything I can do to help you with this. Hi Tomas, thanks for looking. Can you please check my latest 2 patches and confirm that this is what's required? More work will follow for the other endpoints and for illbatch statuses if so. Created attachment 156752 [details] [review] Bug 30719: QA follow-up: Rewrite ill batches get endpoint Created attachment 156783 [details] [review] Bug 30719: QA follow-up: Rewrite remaining ill batches endpoints get, add and update rewrite Created attachment 156785 [details] [review] Bug 30719: QA follow-up: Rename illbatchstatuses illbatchstatuses endpoint is now ill/batchstatuses Created attachment 156811 [details] [review] Bug 30719: (QA follow-up) Pick better column names and cleanup This patch takes on normalizing the attribute names, embeds, and also makes the whole API more kosher, in terms of using accessors for related objects, using the standard structure for strings_map, etc. Created attachment 156812 [details] [review] Bug 30719: QA follow-up: Update statuscode -> status_code on the js files Update remaining batch_id -> ill_batch_id Update batch object in Illrequest.pm strings_map Created attachment 156827 [details] [review] Bug 30719: QA follow-up: Updating remaining naming Update statuscode -> status_code on the js files Update remaining batch_id -> ill_batch_id Update batch object in Illrequest.pm strings_map Created attachment 156828 [details] [review] Bug 30719: QA follow-up: DB updates updates to atomicupdate and kohastructure.sql Pushed to master for 23.11. Nice work everyone, thanks! Enhancement not pushed to 23.05.x |