Description
Pedro Amorim
2023-12-15 10:20:14 UTC
Created attachment 159863 [details] [review] Bug 35581: Illbackend.pm -> ILL/Backend.pm Created attachment 159864 [details] [review] Bug 35581: Illbatch.pm -> ILL/Batch.pm Illbatches.pm -> ILL/Batches.pm Created attachment 159865 [details] [review] Bug 35581: Illbatchstatus.pm -> ILL/Batch/status.pm Illbatchstatuses.pm -> ILL/Batch/Statuses.pm Created attachment 159866 [details] [review] Bug 35581: Illcomment.pm -> ILL/Comment.pm Illcomments.pm -> ILL/Comments.pm Created attachment 159867 [details] [review] Bug 35581: Illrequestattribute.pm -> ILL/Request/Attribute.pm Illrequestattributes.pm -> ILL/Request/Attributes.pm Created attachment 159868 [details] [review] Bug 35581: Koha/Illrequest/Workflow.pm -> Koha/ILL/Request/Workflow.pm Koha/Illrequest/Workflow/Availability.pm -> Koha/ILL/Request/Workflow/Availability.pm Koha/Illrequest/Workflow/TypeDisclaimer.pm -> Koha/ILL/Request/Workflow/TypeDisclaimer.pm Created attachment 159869 [details] [review] Bug 35581: Koha/Illrequest/Config.pm -> Koha/ILL/Request/Config.pm Created attachment 159870 [details] [review] Bug 35581: Koha/Illrequest/Logger.pm -> Koha/ILL/Request/Logger.pm Created attachment 159871 [details] [review] Bug 35581: Koha/Illrequest/SupplierUpdate.pm -> Koha/ILL/Request/SupplierUpdate.pm Koha/Illrequest/SupplierUpdateProcessor.pm -> Koha/ILL/Request/SupplierUpdateProcessor.pm Created attachment 159872 [details] [review] Bug 35581: Koha/Illrequest.pm -> Koha/ILL/Request.pm Koha/Illrequests.pm -> Koha/ILL/Requests.pm Merged: t/db_dependent/Koha/Illrequests.t t/db_dependent/Illrequests.t Into: t/db_dependent/Koha/ILL/Requests.t ILL classes file structure is, for the most part, around 7 years old and doesn't follow a strict logic. It's so confusing that some test files exist redundantly. This housekeeping should help future work in regards to ISO18626 to add Koha as a supplying agency instead of just requesting agency, as is now. It should also help future housekeeping of moving backend related logic out of the Illrequest.pm into Illbackend.pm (now ILL/Request.pm and ILL/Backend.pm as of this patchset). It should also help in structuring the addition of a master generic form (see bug 35570) This patchset will require existing backends to be updated to match the new class names and structure, if they invoke them. Test plan, k-t-d, run tests: prove t/db_dependent/api/v1/ill_* prove t/db_dependent/Koha/ILL/* Test plan, k-t-d, manual: 1) Install FreeForm, enable ILL module, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 2) You'll have to switch the FreeForm repo to the one compatible with this work, like: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout reorganize_ILL 3) Do some generic ILL testing: 3.1) Create a request 3.2) Add a comment to a request 3.3) Edit a request 3.4) Edit a request's item metadata 3.5) Confirm a request 3.6) List requests 3.7) Filter requests list using left side filters 4) Install a metadata enrichment plugin: https://github.com/PTFS-Europe/koha-plugin-api-pubmed 4.1) Create an ILL batch and insert a pubmedid like 123 4.2) Add the request and finish batch 5) Verify all of the above works as expected Created attachment 159873 [details] [review] Bug 35581: QA Follow-up: Fix tidy warnings Fix forbidden pattern: Incorrect license statement (using postal address) Created attachment 160422 [details] [review] Bug 35581: Illbackend.pm -> ILL/Backend.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 160423 [details] [review] Bug 35581: Illbatch.pm -> ILL/Batch.pm Illbatches.pm -> ILL/Batches.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 160424 [details] [review] Bug 35581: Illbatchstatus.pm -> ILL/Batch/status.pm Illbatchstatuses.pm -> ILL/Batch/Statuses.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 160425 [details] [review] Bug 35581: Illcomment.pm -> ILL/Comment.pm Illcomments.pm -> ILL/Comments.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 160426 [details] [review] Bug 35581: Illrequestattribute.pm -> ILL/Request/Attribute.pm Illrequestattributes.pm -> ILL/Request/Attributes.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 160427 [details] [review] Bug 35581: Koha/Illrequest/Workflow.pm -> Koha/ILL/Request/Workflow.pm Koha/Illrequest/Workflow/Availability.pm -> Koha/ILL/Request/Workflow/Availability.pm Koha/Illrequest/Workflow/TypeDisclaimer.pm -> Koha/ILL/Request/Workflow/TypeDisclaimer.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 160428 [details] [review] Bug 35581: Koha/Illrequest/Config.pm -> Koha/ILL/Request/Config.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 160429 [details] [review] Bug 35581: Koha/Illrequest/Logger.pm -> Koha/ILL/Request/Logger.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 160430 [details] [review] Bug 35581: Koha/Illrequest/SupplierUpdate.pm -> Koha/ILL/Request/SupplierUpdate.pm Koha/Illrequest/SupplierUpdateProcessor.pm -> Koha/ILL/Request/SupplierUpdateProcessor.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 160431 [details] [review] Bug 35581: Koha/Illrequest.pm -> Koha/ILL/Request.pm Koha/Illrequests.pm -> Koha/ILL/Requests.pm Merged: t/db_dependent/Koha/Illrequests.t t/db_dependent/Illrequests.t Into: t/db_dependent/Koha/ILL/Requests.t ILL classes file structure is, for the most part, around 7 years old and doesn't follow a strict logic. It's so confusing that some test files exist redundantly. This housekeeping should help future work in regards to ISO18626 to add Koha as a supplying agency instead of just requesting agency, as is now. It should also help future housekeeping of moving backend related logic out of the Illrequest.pm into Illbackend.pm (now ILL/Request.pm and ILL/Backend.pm as of this patchset). It should also help in structuring the addition of a master generic form (see bug 35570) This patchset will require existing backends to be updated to match the new class names and structure, if they invoke them. Test plan, k-t-d, run tests: prove t/db_dependent/api/v1/ill_* prove t/db_dependent/Koha/ILL/* Test plan, k-t-d, manual: 1) Install FreeForm, enable ILL module, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 2) You'll have to switch the FreeForm repo to the one compatible with this work, like: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout reorganize_ILL 3) Do some generic ILL testing: 3.1) Create a request 3.2) Add a comment to a request 3.3) Edit a request 3.4) Edit a request's item metadata 3.5) Confirm a request 3.6) List requests 3.7) Filter requests list using left side filters 4) Install a metadata enrichment plugin: https://github.com/PTFS-Europe/koha-plugin-api-pubmed 4.1) Create an ILL batch and insert a pubmedid like 123 4.2) Add the request and finish batch 5) Verify all of the above works as expected Signed-off-by: David Nind <david@davidnind.com> Created attachment 160432 [details] [review] Bug 35581: QA Follow-up: Fix tidy warnings Fix forbidden pattern: Incorrect license statement (using postal address) Signed-off-by: David Nind <david@davidnind.com> Testing notes (using KTD): 1. Tests pass. 2. Also tested manually as per the test plan. For the PUBMED plugin, I created a request using these details: - Type: Journal article - Title: Regeneration of the heart - PMID: 22095736 (used when creating the batch) I'm not so familiar with the workflow for ILL requests, so it may be useful to have someone that is to add their sign off as well. Created attachment 160620 [details] [review] Bug 35581: (QA follow-up): Missed Illrequests.pm REST API class Koha/REST/V1/Illrequests.pm -> Koha/REST/V1/ILL/Requests.pm Test plan: 1 Apply patch 2 Make sure tests pass, run: prove t/db_dependent/api/v1/ill_* Created attachment 160622 [details] [review] Bug 35581: (QA follow-up): Update changes coming from bug 35331 Test plan: prove t/db_dependent/Koha/Plugins/Ill_hooks.t Created attachment 162020 [details] [review] Bug 35581: Illbackend.pm -> ILL/Backend.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162021 [details] [review] Bug 35581: Illbatch.pm -> ILL/Batch.pm Illbatches.pm -> ILL/Batches.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162022 [details] [review] Bug 35581: Illbatchstatus.pm -> ILL/Batch/status.pm Illbatchstatuses.pm -> ILL/Batch/Statuses.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162023 [details] [review] Bug 35581: Illcomment.pm -> ILL/Comment.pm Illcomments.pm -> ILL/Comments.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162024 [details] [review] Bug 35581: Illrequestattribute.pm -> ILL/Request/Attribute.pm Illrequestattributes.pm -> ILL/Request/Attributes.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162025 [details] [review] Bug 35581: Koha/Illrequest/Workflow.pm -> Koha/ILL/Request/Workflow.pm Koha/Illrequest/Workflow/Availability.pm -> Koha/ILL/Request/Workflow/Availability.pm Koha/Illrequest/Workflow/TypeDisclaimer.pm -> Koha/ILL/Request/Workflow/TypeDisclaimer.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162026 [details] [review] Bug 35581: Koha/Illrequest/Config.pm -> Koha/ILL/Request/Config.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162027 [details] [review] Bug 35581: Koha/Illrequest/Logger.pm -> Koha/ILL/Request/Logger.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162028 [details] [review] Bug 35581: Koha/Illrequest/SupplierUpdate.pm -> Koha/ILL/Request/SupplierUpdate.pm Koha/Illrequest/SupplierUpdateProcessor.pm -> Koha/ILL/Request/SupplierUpdateProcessor.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162029 [details] [review] Bug 35581: Koha/Illrequest.pm -> Koha/ILL/Request.pm Koha/Illrequests.pm -> Koha/ILL/Requests.pm Merged: t/db_dependent/Koha/Illrequests.t t/db_dependent/Illrequests.t Into: t/db_dependent/Koha/ILL/Requests.t ILL classes file structure is, for the most part, around 7 years old and doesn't follow a strict logic. It's so confusing that some test files exist redundantly. This housekeeping should help future work in regards to ISO18626 to add Koha as a supplying agency instead of just requesting agency, as is now. It should also help future housekeeping of moving backend related logic out of the Illrequest.pm into Illbackend.pm (now ILL/Request.pm and ILL/Backend.pm as of this patchset). It should also help in structuring the addition of a master generic form (see bug 35570) This patchset will require existing backends to be updated to match the new class names and structure, if they invoke them. Test plan, k-t-d, run tests: prove t/db_dependent/api/v1/ill_* prove t/db_dependent/Koha/ILL/* Test plan, k-t-d, manual: 1) Install FreeForm, enable ILL module, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 2) You'll have to switch the FreeForm repo to the one compatible with this work, like: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout reorganize_ILL 3) Do some generic ILL testing: 3.1) Create a request 3.2) Add a comment to a request 3.3) Edit a request 3.4) Edit a request's item metadata 3.5) Confirm a request 3.6) List requests 3.7) Filter requests list using left side filters 4) Install a metadata enrichment plugin: https://github.com/PTFS-Europe/koha-plugin-api-pubmed 4.1) Create an ILL batch and insert a pubmedid like 123 4.2) Add the request and finish batch 5) Verify all of the above works as expected Signed-off-by: David Nind <david@davidnind.com> Created attachment 162030 [details] [review] Bug 35581: QA Follow-up: Fix tidy warnings Fix forbidden pattern: Incorrect license statement (using postal address) Signed-off-by: David Nind <david@davidnind.com> Created attachment 162031 [details] [review] Bug 35581: (QA follow-up): Missed Illrequests.pm REST API class Koha/REST/V1/Illrequests.pm -> Koha/REST/V1/ILL/Requests.pm Test plan: 1 Apply patch 2 Make sure tests pass, run: prove t/db_dependent/api/v1/ill_* Created attachment 162032 [details] [review] Bug 35581: (QA follow-up): Update changes coming from bug 35331 Test plan: prove t/db_dependent/Koha/Plugins/Ill_hooks.t Rebased on bug 34282 Created attachment 162817 [details] [review] Bug 35581: Illbackend.pm -> ILL/Backend.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162818 [details] [review] Bug 35581: Illbatch.pm -> ILL/Batch.pm Illbatches.pm -> ILL/Batches.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162819 [details] [review] Bug 35581: Illbatchstatus.pm -> ILL/Batch/status.pm Illbatchstatuses.pm -> ILL/Batch/Statuses.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162820 [details] [review] Bug 35581: Illcomment.pm -> ILL/Comment.pm Illcomments.pm -> ILL/Comments.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162821 [details] [review] Bug 35581: Illrequestattribute.pm -> ILL/Request/Attribute.pm Illrequestattributes.pm -> ILL/Request/Attributes.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162822 [details] [review] Bug 35581: Koha/Illrequest/Workflow.pm -> Koha/ILL/Request/Workflow.pm Koha/Illrequest/Workflow/Availability.pm -> Koha/ILL/Request/Workflow/Availability.pm Koha/Illrequest/Workflow/TypeDisclaimer.pm -> Koha/ILL/Request/Workflow/TypeDisclaimer.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162823 [details] [review] Bug 35581: Koha/Illrequest/Config.pm -> Koha/ILL/Request/Config.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162824 [details] [review] Bug 35581: Koha/Illrequest/Logger.pm -> Koha/ILL/Request/Logger.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162825 [details] [review] Bug 35581: Koha/Illrequest/SupplierUpdate.pm -> Koha/ILL/Request/SupplierUpdate.pm Koha/Illrequest/SupplierUpdateProcessor.pm -> Koha/ILL/Request/SupplierUpdateProcessor.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162826 [details] [review] Bug 35581: Koha/Illrequest.pm -> Koha/ILL/Request.pm Koha/Illrequests.pm -> Koha/ILL/Requests.pm Merged: t/db_dependent/Koha/Illrequests.t t/db_dependent/Illrequests.t Into: t/db_dependent/Koha/ILL/Requests.t ILL classes file structure is, for the most part, around 7 years old and doesn't follow a strict logic. It's so confusing that some test files exist redundantly. This housekeeping should help future work in regards to ISO18626 to add Koha as a supplying agency instead of just requesting agency, as is now. It should also help future housekeeping of moving backend related logic out of the Illrequest.pm into Illbackend.pm (now ILL/Request.pm and ILL/Backend.pm as of this patchset). It should also help in structuring the addition of a master generic form (see bug 35570) This patchset will require existing backends to be updated to match the new class names and structure, if they invoke them. Test plan, k-t-d, run tests: prove t/db_dependent/api/v1/ill_* prove t/db_dependent/Koha/ILL/* Test plan, k-t-d, manual: 1) Install FreeForm, enable ILL module, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 2) You'll have to switch the FreeForm repo to the one compatible with this work, like: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout reorganize_ILL 3) Do some generic ILL testing: 3.1) Create a request 3.2) Add a comment to a request 3.3) Edit a request 3.4) Edit a request's item metadata 3.5) Confirm a request 3.6) List requests 3.7) Filter requests list using left side filters 4) Install a metadata enrichment plugin: https://github.com/PTFS-Europe/koha-plugin-api-pubmed 4.1) Create an ILL batch and insert a pubmedid like 123 4.2) Add the request and finish batch 5) Verify all of the above works as expected Signed-off-by: David Nind <david@davidnind.com> Created attachment 162827 [details] [review] Bug 35581: QA Follow-up: Fix tidy warnings Fix forbidden pattern: Incorrect license statement (using postal address) Signed-off-by: David Nind <david@davidnind.com> Created attachment 162828 [details] [review] Bug 35581: (QA follow-up): Missed Illrequests.pm REST API class Koha/REST/V1/Illrequests.pm -> Koha/REST/V1/ILL/Requests.pm Test plan: 1 Apply patch 2 Make sure tests pass, run: prove t/db_dependent/api/v1/ill_* Created attachment 162829 [details] [review] Bug 35581: (QA follow-up): Update changes coming from bug 35331 Test plan: prove t/db_dependent/Koha/Plugins/Ill_hooks.t Rebased on bug 34478 Created attachment 162880 [details] [review] Bug 35581: Illbackend.pm -> ILL/Backend.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162881 [details] [review] Bug 35581: Illbatch.pm -> ILL/Batch.pm Illbatches.pm -> ILL/Batches.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162882 [details] [review] Bug 35581: Illbatchstatus.pm -> ILL/Batch/status.pm Illbatchstatuses.pm -> ILL/Batch/Statuses.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162883 [details] [review] Bug 35581: Illcomment.pm -> ILL/Comment.pm Illcomments.pm -> ILL/Comments.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162884 [details] [review] Bug 35581: Illrequestattribute.pm -> ILL/Request/Attribute.pm Illrequestattributes.pm -> ILL/Request/Attributes.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162885 [details] [review] Bug 35581: Koha/Illrequest/Workflow.pm -> Koha/ILL/Request/Workflow.pm Koha/Illrequest/Workflow/Availability.pm -> Koha/ILL/Request/Workflow/Availability.pm Koha/Illrequest/Workflow/TypeDisclaimer.pm -> Koha/ILL/Request/Workflow/TypeDisclaimer.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162886 [details] [review] Bug 35581: Koha/Illrequest/Config.pm -> Koha/ILL/Request/Config.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162887 [details] [review] Bug 35581: Koha/Illrequest/Logger.pm -> Koha/ILL/Request/Logger.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162888 [details] [review] Bug 35581: Koha/Illrequest/SupplierUpdate.pm -> Koha/ILL/Request/SupplierUpdate.pm Koha/Illrequest/SupplierUpdateProcessor.pm -> Koha/ILL/Request/SupplierUpdateProcessor.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 162889 [details] [review] Bug 35581: Koha/Illrequest.pm -> Koha/ILL/Request.pm Koha/Illrequests.pm -> Koha/ILL/Requests.pm Merged: t/db_dependent/Koha/Illrequests.t t/db_dependent/Illrequests.t Into: t/db_dependent/Koha/ILL/Requests.t ILL classes file structure is, for the most part, around 7 years old and doesn't follow a strict logic. It's so confusing that some test files exist redundantly. This housekeeping should help future work in regards to ISO18626 to add Koha as a supplying agency instead of just requesting agency, as is now. It should also help future housekeeping of moving backend related logic out of the Illrequest.pm into Illbackend.pm (now ILL/Request.pm and ILL/Backend.pm as of this patchset). It should also help in structuring the addition of a master generic form (see bug 35570) This patchset will require existing backends to be updated to match the new class names and structure, if they invoke them. Test plan, k-t-d, run tests: prove t/db_dependent/api/v1/ill_* prove t/db_dependent/Koha/ILL/* Test plan, k-t-d, manual: 1) Install FreeForm, enable ILL module, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 2) You'll have to switch the FreeForm repo to the one compatible with this work, like: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout reorganize_ILL 3) Do some generic ILL testing: 3.1) Create a request 3.2) Add a comment to a request 3.3) Edit a request 3.4) Edit a request's item metadata 3.5) Confirm a request 3.6) List requests 3.7) Filter requests list using left side filters 4) Install a metadata enrichment plugin: https://github.com/PTFS-Europe/koha-plugin-api-pubmed 4.1) Create an ILL batch and insert a pubmedid like 123 4.2) Add the request and finish batch 5) Verify all of the above works as expected Signed-off-by: David Nind <david@davidnind.com> Created attachment 162890 [details] [review] Bug 35581: QA Follow-up: Fix tidy warnings Fix forbidden pattern: Incorrect license statement (using postal address) Signed-off-by: David Nind <david@davidnind.com> Created attachment 162891 [details] [review] Bug 35581: (QA follow-up): Missed Illrequests.pm REST API class Koha/REST/V1/Illrequests.pm -> Koha/REST/V1/ILL/Requests.pm Test plan: 1 Apply patch 2 Make sure tests pass, run: prove t/db_dependent/api/v1/ill_* Created attachment 162892 [details] [review] Bug 35581: (QA follow-up): Update changes coming from bug 35331 Test plan: prove t/db_dependent/Koha/Plugins/Ill_hooks.t Rebased on bug 36245 Created attachment 163803 [details] [review] Bug 35581: Koha::Illbackend -> Koha::ILL::Backend Koha/Illbackend.pm -> Koha/ILL/Backend.pm Koha/REST/V1/Illbackends.pm -> Koha/REST/V1/ILL/Backends.pm Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 163804 [details] [review] Bug 35581: Koha::*Illbatch* -> Koha::*ILL::*Batch* Koha/Illbatch.pm -> Koha/ILL/Batch.pm Koha/Illbatches.pm -> Koha/ILL/Batches.pm Koha/REST/V1/Illbatches.pm -> Koha/REST/V1/ILL/Batches.pm t/db_dependent/Koha/Illbatch.t -> t/db_dependent/Koha/ILL/Batch.t t/db_dependent/Illbatches.t -> t/db_dependent/Koha/ILL/Batches.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 163805 [details] [review] Bug 35581: Illbatchstatus* -> ILL::Batch::Status* Koha/IllbatchStatus.pm -> Koha/ILL/Batch/Status.pm Koha/IllbatchStatuses.pm -> Koha/ILL/Batch/Statuses.pm Koha/REST/V1/IllbatchStatuses.pm -> Koha/REST/V1/ILL/Batch/Statuses.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 163806 [details] [review] Bug 35581: Koha::Illcomment* -> Koha::ILL::Comment* Koha/Illcomment.pm -> Koha/ILL/Comment.pm Koha/Illcomments.pm -> Koha/ILL/Comments.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 163807 [details] [review] Bug 35581: Koha::Illrequestattribute* -> Koha::ILL::Request::Attribute* Koha/Illrequestattribute.pm -> Koha/ILL/Request/Attribute.pm Koha/Illrequestattributes.pm -> Koha/ILL/Request/Attributes.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 163808 [details] [review] Bug 35581: Koha::Illrequest::Workflow* -> Koha::ILL::Request::Workflow* Koha/Illrequest/Workflow.pm -> Koha/ILL/Request/Workflow.pm Koha/Illrequest/Workflow/Availability.pm -> Koha/ILL/Request/Workflow/Availability.pm Koha/Illrequest/Workflow/TypeDisclaimer.pm -> Koha/ILL/Request/Workflow/TypeDisclaimer.pm Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 163809 [details] [review] Bug 35581: Koha::Illrequest::Config -> Koha::ILL::Request::Config Signed-off-by: David Nind <david@davidnind.com> Created attachment 163810 [details] [review] Bug 35581: Koha::Illrequest::Logger -> Koha::ILL::Request::Logger Signed-off-by: David Nind <david@davidnind.com> Created attachment 163811 [details] [review] Bug 35581: Koha::Illrequest::SupplierUpdate* -> Koha::ILL::Request::SupplierUpdate* Koha/Illrequest/SupplierUpdate.pm -> Koha/ILL/Request/SupplierUpdate.pm Koha/Illrequest/SupplierUpdateProcessor.pm -> Koha/ILL/Request/SupplierUpdateProcessor.pm Signed-off-by: David Nind <david@davidnind.com> Created attachment 163812 [details] [review] Bug 35581: Koha::Illrequest -> Koha::ILL::Request Koha/Illrequests.pm -> Koha/ILL/Requests.pm Merged: t/db_dependent/Koha/Illrequests.t t/db_dependent/Illrequests.t Into: t/db_dependent/Koha/ILL/Requests.t ILL classes file structure is, for the most part, around 7 years old and doesn't follow a strict logic. It's so confusing that some test files exist redundantly. This housekeeping should help future work in regards to ISO18626 to add Koha as a supplying agency instead of just requesting agency, as is now. It should also help future housekeeping of moving backend related logic out of the Illrequest.pm into Illbackend.pm (now ILL/Request.pm and ILL/Backend.pm as of this patchset). It should also help in structuring the addition of a master generic form (see bug 35570) This patchset will require existing backends to be updated to match the new class names and structure, if they invoke them. Test plan, k-t-d, run tests: prove t/db_dependent/api/v1/ill_* prove t/db_dependent/Koha/ILL/* Test plan, k-t-d, manual: 1) Install FreeForm, enable ILL module, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 2) You'll have to switch the FreeForm repo to the one compatible with this work, like: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout reorganize_ILL 3) Do some generic ILL testing: 3.1) Create a request 3.2) Add a comment to a request 3.3) Edit a request 3.4) Edit a request's item metadata 3.5) Confirm a request 3.6) List requests 3.7) Filter requests list using left side filters 4) Install a metadata enrichment plugin: https://github.com/PTFS-Europe/koha-plugin-api-pubmed 4.1) Create an ILL batch and insert a pubmedid like 123 4.2) Add the request and finish batch 5) Verify all of the above works as expected Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 163813 [details] [review] Bug 35581: Koha::REST::V1::Illrequests -> Koha::REST::V1::ILL::Requests Test plan: 1. Apply patch 2. Make sure tests pass, run: prove t/db_dependent/api/v1/ill_* Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 163814 [details] [review] Bug 35581: Cleanup Comments.t Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 163815 [details] [review] Bug 35581: (follow-up) Remove internal use of ->illrequestattributes By now, we shouldn't be using the deprecated method internally. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 163816 [details] [review] Bug 35581: POD fixes Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 164069 [details] [review] Bug 35581: Koha::Illbackend -> Koha::ILL::Backend Koha/Illbackend.pm -> Koha/ILL/Backend.pm Koha/REST/V1/Illbackends.pm -> Koha/REST/V1/ILL/Backends.pm Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 164070 [details] [review] Bug 35581: Koha::*Illbatch* -> Koha::*ILL::*Batch* Koha/Illbatch.pm -> Koha/ILL/Batch.pm Koha/Illbatches.pm -> Koha/ILL/Batches.pm Koha/REST/V1/Illbatches.pm -> Koha/REST/V1/ILL/Batches.pm t/db_dependent/Koha/Illbatch.t -> t/db_dependent/Koha/ILL/Batch.t t/db_dependent/Illbatches.t -> t/db_dependent/Koha/ILL/Batches.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 164071 [details] [review] Bug 35581: Illbatchstatus* -> ILL::Batch::Status* Koha/IllbatchStatus.pm -> Koha/ILL/Batch/Status.pm Koha/IllbatchStatuses.pm -> Koha/ILL/Batch/Statuses.pm Koha/REST/V1/IllbatchStatuses.pm -> Koha/REST/V1/ILL/Batch/Statuses.pm Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 164072 [details] [review] Bug 35581: Koha::Illcomment* -> Koha::ILL::Comment* Koha/Illcomment.pm -> Koha/ILL/Comment.pm Koha/Illcomments.pm -> Koha/ILL/Comments.pm Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 164073 [details] [review] Bug 35581: Koha::Illrequestattribute* -> Koha::ILL::Request::Attribute* Koha/Illrequestattribute.pm -> Koha/ILL/Request/Attribute.pm Koha/Illrequestattributes.pm -> Koha/ILL/Request/Attributes.pm Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 164074 [details] [review] Bug 35581: Koha::Illrequest::Workflow* -> Koha::ILL::Request::Workflow* Koha/Illrequest/Workflow.pm -> Koha/ILL/Request/Workflow.pm Koha/Illrequest/Workflow/Availability.pm -> Koha/ILL/Request/Workflow/Availability.pm Koha/Illrequest/Workflow/TypeDisclaimer.pm -> Koha/ILL/Request/Workflow/TypeDisclaimer.pm Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 164075 [details] [review] Bug 35581: Koha::Illrequest::Config -> Koha::ILL::Request::Config Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 164076 [details] [review] Bug 35581: Koha::Illrequest::Logger -> Koha::ILL::Request::Logger Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 164077 [details] [review] Bug 35581: Koha::Illrequest::SupplierUpdate* -> Koha::ILL::Request::SupplierUpdate* Koha/Illrequest/SupplierUpdate.pm -> Koha/ILL/Request/SupplierUpdate.pm Koha/Illrequest/SupplierUpdateProcessor.pm -> Koha/ILL/Request/SupplierUpdateProcessor.pm Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 164078 [details] [review] Bug 35581: Koha::Illrequest -> Koha::ILL::Request Koha/Illrequests.pm -> Koha/ILL/Requests.pm Merged: t/db_dependent/Koha/Illrequests.t t/db_dependent/Illrequests.t Into: t/db_dependent/Koha/ILL/Requests.t ILL classes file structure is, for the most part, around 7 years old and doesn't follow a strict logic. It's so confusing that some test files exist redundantly. This housekeeping should help future work in regards to ISO18626 to add Koha as a supplying agency instead of just requesting agency, as is now. It should also help future housekeeping of moving backend related logic out of the Illrequest.pm into Illbackend.pm (now ILL/Request.pm and ILL/Backend.pm as of this patchset). It should also help in structuring the addition of a master generic form (see bug 35570) This patchset will require existing backends to be updated to match the new class names and structure, if they invoke them. Test plan, k-t-d, run tests: prove t/db_dependent/api/v1/ill_* prove t/db_dependent/Koha/ILL/* Test plan, k-t-d, manual: 1) Install FreeForm, enable ILL module, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 2) You'll have to switch the FreeForm repo to the one compatible with this work, like: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout reorganize_ILL 3) Do some generic ILL testing: 3.1) Create a request 3.2) Add a comment to a request 3.3) Edit a request 3.4) Edit a request's item metadata 3.5) Confirm a request 3.6) List requests 3.7) Filter requests list using left side filters 4) Install a metadata enrichment plugin: https://github.com/PTFS-Europe/koha-plugin-api-pubmed 4.1) Create an ILL batch and insert a pubmedid like 123 4.2) Add the request and finish batch 5) Verify all of the above works as expected Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 164079 [details] [review] Bug 35581: Koha::REST::V1::Illrequests -> Koha::REST::V1::ILL::Requests Test plan: 1. Apply patch 2. Make sure tests pass, run: prove t/db_dependent/api/v1/ill_* Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 164080 [details] [review] Bug 35581: Cleanup Comments.t Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 164081 [details] [review] Bug 35581: (follow-up) Remove internal use of ->illrequestattributes By now, we shouldn't be using the deprecated method internally. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 164082 [details] [review] Bug 35581: POD fixes Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> \o/ ./t/db_dependent/Illrequest directory still exists (it shouldn't after this work). I don't want to add anything else here before this is pushed because it's long as it is. @Tomas, @Katrin add it here or handle this later? It's not critically important anyway. (In reply to Pedro Amorim from comment #97) > ./t/db_dependent/Illrequest directory still exists (it shouldn't after this > work). > I don't want to add anything else here before this is pushed because it's > long as it is. > @Tomas, @Katrin add it here or handle this later? It's not critically > important anyway. I haven't had time to look here closely yet, leaving it to Tomas. (In reply to Katrin Fischer from comment #98) > (In reply to Pedro Amorim from comment #97) > > ./t/db_dependent/Illrequest directory still exists (it shouldn't after this > > work). > > I don't want to add anything else here before this is pushed because it's > > long as it is. > > @Tomas, @Katrin add it here or handle this later? It's not critically > > important anyway. > > I haven't had time to look here closely yet, leaving it to Tomas. Please handle any necessary follow-ups on a separate bug report. I'm adding 'additional_work'needed' so we don't forget to follow-up on this. I've added a follow-up to fix a failing test: t/db_dependent/Koha/ILL/Request.t Pushed for 24.05! Well done everyone, thank you! (In reply to Katrin Fischer from comment #99) > Please handle any necessary follow-ups on a separate bug report. I'm adding > 'additional_work'needed' so we don't forget to follow-up on this. > > I've added a follow-up to fix a failing test: > t/db_dependent/Koha/ILL/Request.t Thanks Katrin, I've opened bug 36662 detailing what was missed here and intend to follow-up on it soon. I've removed 'additional-work-needed' keyword from this one. I wonder what release notes we should add here, if any? This is just housekeeping work that precedes future features work, there is no new behavior or functionality added here. Please advise. Thanks! (In reply to Pedro Amorim from comment #102) > I wonder what release notes we should add here, if any? > This is just housekeeping work that precedes future features work, there is > no new behavior or functionality added here. Please advise. Thanks! I was thinking of some notes for backend authors - I expect they will all have to adapt their backends to the new structure. So something like a warning maybe that stands out a bit? (In reply to Katrin Fischer from comment #103) > (In reply to Pedro Amorim from comment #102) > > I wonder what release notes we should add here, if any? > > This is just housekeeping work that precedes future features work, there is > > no new behavior or functionality added here. Please advise. Thanks! > > I was thinking of some notes for backend authors - I expect they will all > have to adapt their backends to the new structure. So something like a > warning maybe that stands out a bit? The backend only needs to be updated if it invokes core class methods. In our case (PTFS-E) some of our backends do, others don't. I've added text to go in the release notes and tried to have it be as clear and simple as possible. Please take a look and adapt/update as needed. Thanks! We have some test failures where this bug is a likely candidate: t_db_dependent_TestBuilder_t not ok 553 - Module Koha::ILL::Comments should have koha_object[s]_class method if needed not ok 554 - Module Koha::Illcomment should be defined not ok 555 - Module Koha::Illcomments should be defined Created attachment 165448 [details] [review] Bug 35581: Fix Illcomment schema file missing koha_object_class methods I've added the requisite pieces to the schema class now.. will push shortly.. just removing the additiona_work_needed tag to prevent duplicate work. Not pushed to 23.11.x No changes to the manual required. |