Bug 24239 - Let the ILL module set ad hoc hard due dates
Summary: Let the ILL module set ad hoc hard due dates
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: ILL (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Magnus Enger
QA Contact: Kyle M Hall (khall)
URL:
Keywords:
Depends on:
Blocks: 31592
  Show dependency treegraph
 
Reported: 2019-12-13 13:47 UTC by Magnus Enger
Modified: 2022-11-28 12:26 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00
Circulation function:


Attachments
Bug 24239 - Let the ILL module set ad hoc hard due dates (6.80 KB, patch)
2019-12-13 14:13 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 24239: Let the ILL module set ad hoc hard due dates (6.60 KB, patch)
2020-06-29 16:11 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 24239: Let the ILL module set ad hoc hard due dates (6.55 KB, patch)
2022-02-01 23:59 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 24239: Replace syspref with illrequests column date_due (9.72 KB, patch)
2022-02-01 23:59 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 24239: Add column illrequests.date_due (3.13 KB, patch)
2022-02-09 23:12 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 24239: Unit tests (2.40 KB, patch)
2022-02-09 23:12 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 24239: Let the ILL module set ad hoc hard due dates (3.21 KB, patch)
2022-02-09 23:12 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 24239: Add column illrequests.date_due (3.13 KB, patch)
2022-04-20 14:29 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 24239: Unit tests (2.35 KB, patch)
2022-04-20 14:30 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 24239: Let the ILL module set ad hoc hard due dates (3.21 KB, patch)
2022-04-20 14:30 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 24239: Add column illrequests.date_due (3.19 KB, patch)
2022-05-20 12:49 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 24239: Unit tests (2.40 KB, patch)
2022-05-20 12:49 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 24239: Let the ILL module set ad hoc hard due dates (3.08 KB, patch)
2022-05-20 12:49 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 24239: Add column illrequests.date_due (3.24 KB, patch)
2022-06-10 15:34 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 24239: Unit tests (2.46 KB, patch)
2022-06-10 15:35 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 24239: Let the ILL module set ad hoc hard due dates (3.14 KB, patch)
2022-06-10 15:35 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 24239: Add column illrequests.date_due (2.33 KB, patch)
2022-06-23 18:45 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 24239: Unit tests (2.51 KB, patch)
2022-06-23 18:45 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 24239: Let the ILL module set ad hoc hard due dates (3.19 KB, patch)
2022-06-23 18:45 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 24239: (QA follow-up) Rename date_due => due_date (4.70 KB, patch)
2022-06-23 18:45 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 24239: DBIC update (1.70 KB, patch)
2022-06-23 18:46 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 24239: (QA follow-up) Make dbrev idempotent (1.53 KB, patch)
2022-06-23 18:54 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Magnus Enger 2019-12-13 13:47:24 UTC
Scenario: A member of staff has gotten an ILL loan with a date due 1 year into the future. The person handling the ILL request knows this, but needs a way to tell Koha to use the given hard due date when the loan is issued at the front desk, where noone knows about the special due date for this loan. 

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.
Comment 1 Magnus Enger 2019-12-13 14:13:57 UTC Comment hidden (obsolete)
Comment 2 Katrin Fischer 2020-06-29 16:11:47 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2020-06-29 16:17:37 UTC
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...
Comment 4 Magnus Enger 2020-06-30 09:23:10 UTC
(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.
Comment 5 Katrin Fischer 2020-06-30 10:56:40 UTC
(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?
Comment 6 Magnus Enger 2020-07-01 09:16:04 UTC
(In reply to Katrin Fischer from comment #5)
> Would you mind me tinkering with that a bit?

Not at all, tinker away! :-)
Comment 7 Katrin Fischer 2020-09-14 09:55:31 UTC
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.
Comment 8 Tomás Cohen Arazi (tcohen) 2021-10-29 10:59:32 UTC
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.
Comment 9 Katrin Fischer 2021-10-29 22:42:35 UTC
(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.
Comment 10 Lari Taskula 2022-02-01 23:59:44 UTC Comment hidden (obsolete)
Comment 11 Lari Taskula 2022-02-01 23:59:48 UTC Comment hidden (obsolete)
Comment 12 Lari Taskula 2022-02-02 00:08:45 UTC
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.
Comment 13 Lari Taskula 2022-02-09 23:12:39 UTC Comment hidden (obsolete)
Comment 14 Lari Taskula 2022-02-09 23:12:43 UTC Comment hidden (obsolete)
Comment 15 Lari Taskula 2022-02-09 23:12:47 UTC Comment hidden (obsolete)
Comment 16 Tomás Cohen Arazi (tcohen) 2022-04-20 12:47:08 UTC
Please rebase! Willing to test it!
Comment 17 Lari Taskula 2022-04-20 14:29:57 UTC Comment hidden (obsolete)
Comment 18 Lari Taskula 2022-04-20 14:30:02 UTC Comment hidden (obsolete)
Comment 19 Lari Taskula 2022-04-20 14:30:07 UTC Comment hidden (obsolete)
Comment 20 Magnus Enger 2022-05-03 13:18:55 UTC
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".
Comment 21 Katrin Fischer 2022-05-20 11:27:00 UTC
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.
Comment 22 Katrin Fischer 2022-05-20 12:49:31 UTC Comment hidden (obsolete)
Comment 23 Katrin Fischer 2022-05-20 12:49:35 UTC Comment hidden (obsolete)
Comment 24 Katrin Fischer 2022-05-20 12:49:38 UTC Comment hidden (obsolete)
Comment 25 Kyle M Hall (khall) 2022-06-10 15:34:58 UTC Comment hidden (obsolete)
Comment 26 Kyle M Hall (khall) 2022-06-10 15:35:08 UTC Comment hidden (obsolete)
Comment 27 Kyle M Hall (khall) 2022-06-10 15:35:11 UTC Comment hidden (obsolete)
Comment 28 Tomás Cohen Arazi (tcohen) 2022-06-23 18:45:38 UTC
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>
Comment 29 Tomás Cohen Arazi (tcohen) 2022-06-23 18:45:43 UTC
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>
Comment 30 Tomás Cohen Arazi (tcohen) 2022-06-23 18:45:48 UTC
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>
Comment 31 Tomás Cohen Arazi (tcohen) 2022-06-23 18:45:54 UTC
Created attachment 136461 [details] [review]
Bug 24239: (QA follow-up) Rename date_due => due_date

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 32 Tomás Cohen Arazi (tcohen) 2022-06-23 18:46:02 UTC
Created attachment 136462 [details] [review]
Bug 24239: DBIC update

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 33 Tomás Cohen Arazi (tcohen) 2022-06-23 18:54:23 UTC
Created attachment 136463 [details] [review]
Bug 24239: (QA follow-up) Make dbrev idempotent

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 34 Tomás Cohen Arazi (tcohen) 2022-06-23 19:00:25 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 35 Lucas Gass (lukeg) 2022-07-29 15:37:32 UTC
Enhancement will not be backported to 22.05.x series unless requested
Comment 36 Hans Pålsson 2022-11-28 12:26:09 UTC
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!