Description
Magnus Enger
2019-12-13 13:47:24 UTC
Created attachment 96246 [details] [review] Bug 24239 - Let the ILL module set ad hoc hard due dates The Swedish Libris ILL backend lets librarians store a specific due date when an ILL loan is received. This is stored as a ILL request attribute. This patch adds a syspref that can take the name of the illrequestattribute used to store the specific date. If the syspref is set, and if an item is connected to an ILL request, the due date will be taken from the illrequestattribute, instead of being calculated in the regular way, based on patroncategory and itemtype. To test: - Apply the patch and make sure the atomic database update is run - Use the FreeForm backend to add one ILL request. Take note of the illrequest_id of the request you created. We refer to this as "x" below. - Connect a biblio (with biblionumber y), that has an item with a barcode, to the ILL request directly in the database: UPADTE illrequests SET biblio_id = y WHERE illrequest_id = x; - Add an attribute to the ILL request directly in the database: INSERT INTO illrequestattributes SET illrequest_id = x, type = 'illduedate', value = '2023-01-01'; - Enter 'illduedate' as a value for the ILLUseExactDueDate syspref. - Go to circulation and issue the barcode of the item to the patron associated with the FreeForm ILL request. Verify that the loan gets a due date of 2023-01-01. - Ideally: return the item and issue it again through SIP2 and SCO, and verify that the due date is still 2023-01-01. (I don't think the REST API supports issuing yet?) - Verify that there are no regressions, so that regular calculation of due dates still work, if ILLUseExactDueDate is empty or not. Created attachment 106409 [details] [review] Bug 24239: Let the ILL module set ad hoc hard due dates The Swedish Libris ILL backend lets librarians store a specific due date when an ILL loan is received. This is stored as a ILL request attribute. This patch adds a syspref that can take the name of the illrequestattribute used to store the specific date. If the syspref is set, and if an item is connected to an ILL request, the due date will be taken from the illrequestattribute, instead of being calculated in the regular way, based on patroncategory and itemtype. To test: - Apply the patch and make sure the atomic database update is run - Use the FreeForm backend to add one ILL request. Take note of the illrequest_id of the request you created. We refer to this as "x" below. - Connect a biblio (with biblionumber y), that has an item with a barcode, to the ILL request directly in the database: UPADTE illrequests SET biblio_id = y WHERE illrequest_id = x; - Add an attribute to the ILL request directly in the database: INSERT INTO illrequestattributes SET illrequest_id = x, type = 'illduedate', value = '2023-01-01'; - Enter 'illduedate' as a value for the ILLUseExactDueDate syspref. - Go to circulation and issue the barcode of the item to the patron associated with the FreeForm ILL request. Verify that the loan gets a due date of 2023-01-01. - Ideally: return the item and issue it again through SIP2 and SCO, and verify that the due date is still 2023-01-01. (I don't think the REST API supports issuing yet?) - Verify that there are no regressions, so that regular calculation of due dates still work, if ILLUseExactDueDate is empty or not. I've rebased the patches and done a first code review. I like the concept of this as it's small and flexible. I wonder, if you are still around for this Magnus? I can spend some time on ILL developments right now as we are looking into a backend for our ILL workflow and this would help a great deal. Looking at the code it seems like providing no due date in the ill request would be problematic. I'd suggest to check if the date is set and then have a fallback to normal due_date calculation if it isn't. There is also a leftover warn. I can also sign-off, but I think for QA we'd need tests... (In reply to Katrin Fischer from comment #3) > I wonder, if you are still around for this Magnus? I am on holiday now, so I will probably not be able to look at it before August. (In reply to Magnus Enger from comment #4) > (In reply to Katrin Fischer from comment #3) > > I wonder, if you are still around for this Magnus? > > I am on holiday now, so I will probably not be able to look at it before > August. That's a little late for us :( Would you mind me tinkering with that a bit? I've been wondering about taking a different approach here and adding the itemnumber to the illrequest table as linking by biblionumber and borrowernumber seems not ideal. Would this work for you? (In reply to Katrin Fischer from comment #5) > Would you mind me tinkering with that a bit? Not at all, tinker away! :-) Didn't get to tinker yet... Magnus, would you agree that having the itemnumber in the illrequests table as an optional column would be helpful for the workflows using items? I think it would make it much easier/less error prone to find the illrequest for features like the one proposed here. What if... we added a column to illrequests, such as 'ill_defined_due_date' and just tell backend authors to put the data in there? It would be much simpler. (In reply to Tomás Cohen Arazi from comment #8) > What if... > we added a column to illrequests, such as 'ill_defined_due_date' and just > tell backend authors to put the data in there? It would be much simpler. I really like that, but I'd probably just name it due_date or date_due. Created attachment 130056 [details] [review] Bug 24239: Let the ILL module set ad hoc hard due dates The Swedish Libris ILL backend lets librarians store a specific due date when an ILL loan is received. This is stored as a ILL request attribute. This patch adds a syspref that can take the name of the illrequestattribute used to store the specific date. If the syspref is set, and if an item is connected to an ILL request, the due date will be taken from the illrequestattribute, instead of being calculated in the regular way, based on patroncategory and itemtype. To test: - Apply the patch and make sure the atomic database update is run - Use the FreeForm backend to add one ILL request. Take note of the illrequest_id of the request you created. We refer to this as "x" below. - Connect a biblio (with biblionumber y), that has an item with a barcode, to the ILL request directly in the database: UPADTE illrequests SET biblio_id = y WHERE illrequest_id = x; - Add an attribute to the ILL request directly in the database: INSERT INTO illrequestattributes SET illrequest_id = x, type = 'illduedate', value = '2023-01-01'; - Enter 'illduedate' as a value for the ILLUseExactDueDate syspref. - Go to circulation and issue the barcode of the item to the patron associated with the FreeForm ILL request. Verify that the loan gets a due date of 2023-01-01. - Ideally: return the item and issue it again through SIP2 and SCO, and verify that the due date is still 2023-01-01. (I don't think the REST API supports issuing yet?) - Verify that there are no regressions, so that regular calculation of due dates still work, if ILLUseExactDueDate is empty or not. Created attachment 130057 [details] [review] Bug 24239: Replace syspref with illrequests column date_due This patch replaces system preference introduced in previous patch with a column "date_due" for table illrequests. To test: 1. prove t/db_dependent/Circulation.t Sponsored-by: imCode Partner AB As Tomas proposed, I've added a new column illrequests.date_due and it now replaces the attribute/syspref introduced in Magnus's original patch. Populated by backend, and if defined, AddIssue will use it as the due date, but if left null (default behavior), default due date calculation will be used. Created attachment 130414 [details] [review] Bug 24239: Add column illrequests.date_due Created attachment 130415 [details] [review] Bug 24239: Unit tests Created attachment 130416 [details] [review] Bug 24239: Let the ILL module set ad hoc hard due dates The Swedish Libris ILL backend lets librarians store a specific due date when an ILL loan is received. This is stored as a ILL request attribute. This patch adds a syspref that can take the name of the illrequestattribute used to store the specific date. If the syspref is set, and if an item is connected to an ILL request, the due date will be taken from the illrequestattribute, instead of being calculated in the regular way, based on patroncategory and itemtype. To test: - Apply the patch and make sure the atomic database update is run - Use the FreeForm backend to add one ILL request. Take note of the illrequest_id of the request you created. We refer to this as "x" below. - Connect a biblio (with biblionumber y), that has an item with a barcode, to the ILL request directly in the database: UPADTE illrequests SET biblio_id = y WHERE illrequest_id = x; - Add an attribute to the ILL request directly in the database: INSERT INTO illrequestattributes SET illrequest_id = x, type = 'illduedate', value = '2023-01-01'; - Enter 'illduedate' as a value for the ILLUseExactDueDate syspref. - Go to circulation and issue the barcode of the item to the patron associated with the FreeForm ILL request. Verify that the loan gets a due date of 2023-01-01. - Ideally: return the item and issue it again through SIP2 and SCO, and verify that the due date is still 2023-01-01. (I don't think the REST API supports issuing yet?) - Verify that there are no regressions, so that regular calculation of due dates still work, if ILLUseExactDueDate is empty or not. - prove t/db_dependent/Circulation.t (Patch description, test plan and partial code credits to Magnus Enger) Please rebase! Willing to test it! Created attachment 133475 [details] [review] Bug 24239: Add column illrequests.date_due Created attachment 133476 [details] [review] Bug 24239: Unit tests Created attachment 133477 [details] [review] Bug 24239: Let the ILL module set ad hoc hard due dates The Swedish Libris ILL backend lets librarians store a specific due date when an ILL loan is received. This is stored as a ILL request attribute. This patch adds a syspref that can take the name of the illrequestattribute used to store the specific date. If the syspref is set, and if an item is connected to an ILL request, the due date will be taken from the illrequestattribute, instead of being calculated in the regular way, based on patroncategory and itemtype. To test: - Apply the patch and make sure the atomic database update is run - Use the FreeForm backend to add one ILL request. Take note of the illrequest_id of the request you created. We refer to this as "x" below. - Connect a biblio (with biblionumber y), that has an item with a barcode, to the ILL request directly in the database: UPADTE illrequests SET biblio_id = y WHERE illrequest_id = x; - Add an attribute to the ILL request directly in the database: INSERT INTO illrequestattributes SET illrequest_id = x, type = 'illduedate', value = '2023-01-01'; - Enter 'illduedate' as a value for the ILLUseExactDueDate syspref. - Go to circulation and issue the barcode of the item to the patron associated with the FreeForm ILL request. Verify that the loan gets a due date of 2023-01-01. - Ideally: return the item and issue it again through SIP2 and SCO, and verify that the due date is still 2023-01-01. (I don't think the REST API supports issuing yet?) - Verify that there are no regressions, so that regular calculation of due dates still work, if ILLUseExactDueDate is empty or not. - prove t/db_dependent/Circulation.t (Patch description, test plan and partial code credits to Magnus Enger) As far as I can see new patches were supplied after the status was set to "Patch doesn't apply", so I am setting this to "Needs signoff". I believe the test plan requires updating! * As we now have a illrequests.date_due, we should probably update that in step 4. * There is no ILLUseExactDueDate system preference anymore. I am rewriting it accordingly. Created attachment 135244 [details] [review] Bug 24239: Add column illrequests.date_due Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 135245 [details] [review] Bug 24239: Unit tests Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 135246 [details] [review] Bug 24239: Let the ILL module set ad hoc hard due dates The Swedish Libris ILL backend lets librarians store a specific due date when an ILL loan is received. This patch set adds a new date_due column to the illrequets table that can be used by the different backends to store a due date. If an illrequest has the date due set, it will be used when the item is checked out instead of the calculation using the circulation conditions. To test: - Apply the patch and make sure the atomic database update is run - Use the FreeForm backend to add one ILL request. Take note of the illrequest_id of the request you created. We refer to this as "x" below. - Connect a biblio (with biblionumber y), that has an item with a barcode, to the ILL request directly in the database: UPDATE illrequests SET biblio_id = y WHERE illrequest_id = x; - Next we set the due date, this would normally be done by or from the backend. UPDATE illrequests SET date_due = "2023-01-01" WHERE illrequest_id = x; - Go to circulation and issue the barcode of the item to the patron associated with the FreeForm ILL request. Verify that the loan gets a due date of 2023-01-01. - Ideally: return the item and issue it again through SIP2 and SCO, and verify that the due date is still 2023-01-01. - Verify that there are no regressions, so that regular calculation of due dates still work. - prove t/db_dependent/Circulation.t (Patch description, test plan and partial code credits to Magnus Enger) Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> (Patch description and test plan rewritten to reflect changes in development) Created attachment 135943 [details] [review] Bug 24239: Add column illrequests.date_due Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 135944 [details] [review] Bug 24239: Unit tests Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 135945 [details] [review] Bug 24239: Let the ILL module set ad hoc hard due dates The Swedish Libris ILL backend lets librarians store a specific due date when an ILL loan is received. This patch set adds a new date_due column to the illrequets table that can be used by the different backends to store a due date. If an illrequest has the date due set, it will be used when the item is checked out instead of the calculation using the circulation conditions. To test: - Apply the patch and make sure the atomic database update is run - Use the FreeForm backend to add one ILL request. Take note of the illrequest_id of the request you created. We refer to this as "x" below. - Connect a biblio (with biblionumber y), that has an item with a barcode, to the ILL request directly in the database: UPDATE illrequests SET biblio_id = y WHERE illrequest_id = x; - Next we set the due date, this would normally be done by or from the backend. UPDATE illrequests SET date_due = "2023-01-01" WHERE illrequest_id = x; - Go to circulation and issue the barcode of the item to the patron associated with the FreeForm ILL request. Verify that the loan gets a due date of 2023-01-01. - Ideally: return the item and issue it again through SIP2 and SCO, and verify that the due date is still 2023-01-01. - Verify that there are no regressions, so that regular calculation of due dates still work. - prove t/db_dependent/Circulation.t (Patch description, test plan and partial code credits to Magnus Enger) Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> (Patch description and test plan rewritten to reflect changes in development) Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 136458 [details] [review] Bug 24239: Add column illrequests.date_due Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 136459 [details] [review] Bug 24239: Unit tests Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 136460 [details] [review] Bug 24239: Let the ILL module set ad hoc hard due dates The Swedish Libris ILL backend lets librarians store a specific due date when an ILL loan is received. This patch set adds a new date_due column to the illrequets table that can be used by the different backends to store a due date. If an illrequest has the date due set, it will be used when the item is checked out instead of the calculation using the circulation conditions. To test: - Apply the patch and make sure the atomic database update is run - Use the FreeForm backend to add one ILL request. Take note of the illrequest_id of the request you created. We refer to this as "x" below. - Connect a biblio (with biblionumber y), that has an item with a barcode, to the ILL request directly in the database: UPDATE illrequests SET biblio_id = y WHERE illrequest_id = x; - Next we set the due date, this would normally be done by or from the backend. UPDATE illrequests SET date_due = "2023-01-01" WHERE illrequest_id = x; - Go to circulation and issue the barcode of the item to the patron associated with the FreeForm ILL request. Verify that the loan gets a due date of 2023-01-01. - Ideally: return the item and issue it again through SIP2 and SCO, and verify that the due date is still 2023-01-01. - Verify that there are no regressions, so that regular calculation of due dates still work. - prove t/db_dependent/Circulation.t (Patch description, test plan and partial code credits to Magnus Enger) Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> (Patch description and test plan rewritten to reflect changes in development) Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 136461 [details] [review] Bug 24239: (QA follow-up) Rename date_due => due_date Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 136462 [details] [review] Bug 24239: DBIC update Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 136463 [details] [review] Bug 24239: (QA follow-up) Make dbrev idempotent Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Pushed to master for 22.11. Nice work everyone, thanks! Enhancement will not be backported to 22.05.x series unless requested It would be of great value to many Swedish libraries if this could be backported to 22.05.x (if possible also 21.11.x). Thanks everyone for the great work! |