Bug 38441

Summary: Allow for an ILL history check workflow stage
Product: Koha Reporter: Pedro Amorim <pedro.amorim>
Component: ILLAssignee: Pedro Amorim <pedro.amorim>
Status: Pushed to main --- QA Contact: Tomás Cohen Arazi (tcohen) <tomascohen>
Severity: new feature    
Priority: P5 - low CC: andrew.auld, edith.speller, flyingendpaper, jeremy.evans, katrin.fischer, martin.renvoize, pedro.amorim, tomascohen
Version: Main   
Hardware: All   
OS: All   
URL: https://github.com/PTFS-Europe/koha/commits/bug_38441/
GIT URL: Change sponsored?: ---
Patch complexity: Medium patch Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
25.05.00
Circulation function:
Bug Depends on: 36197, 37448, 38685    
Bug Blocks: 39777, 39875    
Attachments: Bug 38441: New 'ILLHistoryCheck' system preference
Bug 38441: atomicupdate
Bug 38441: HistoryCheck Koha class
Bug 38441: ILLHistoryCheck: Staff UI
Bug 38441: ILLHistoryCheck: OPAC
Bug 38441: Allow / render HTML in ILL staff notes
Bug 38441: Style updates for staffnotes
Bug 38441: Add tests
Bug 38441: New 'ILLHistoryCheck' system preference
Bug 38441: atomicupdate
Bug 38441: HistoryCheck Koha class
Bug 38441: ILLHistoryCheck: Staff UI
Bug 38441: ILLHistoryCheck: OPAC
Bug 38441: Allow / render HTML in ILL staff notes
Bug 38441: Style updates for staffnotes
Bug 38441: Add tests
Bug 38441: New 'ILLHistoryCheck' system preference
Bug 38441: atomicupdate
Bug 38441: HistoryCheck Koha class
Bug 38441: ILLHistoryCheck: Staff UI
Bug 38441: ILLHistoryCheck: OPAC
Bug 38441: Allow / render HTML in ILL staff notes
Bug 38441: Style updates for staffnotes
Bug 38441: Add tests
Bug 38441: (QA follow-up): Handle undef userenv
Bug 38441: (QA follow-up): Fix find-misplaced-executables.t
Bug 38441: (QA follow-up): Fix find-missing-filters.t
Bug 38441: (QA follow-up): Fix query
Bug 38441: New 'ILLHistoryCheck' system preference
Bug 38441: atomicupdate
Bug 38441: HistoryCheck Koha class
Bug 38441: ILLHistoryCheck: Staff UI
Bug 38441: ILLHistoryCheck: OPAC
Bug 38441: Allow / render HTML in ILL staff notes
Bug 38441: Style updates for staffnotes
Bug 38441: Add tests
Bug 38441: (QA follow-up): Handle undef userenv
Bug 38441: (QA follow-up): Fix find-misplaced-executables.t
Bug 38441: (QA follow-up): Fix find-missing-filters.t
Bug 38441: (QA follow-up): HistoryCheck now only considers same patron
Bug 38441: New 'ILLHistoryCheck' system preference
Bug 38441: atomicupdate
Bug 38441: HistoryCheck Koha class
Bug 38441: ILLHistoryCheck: Staff UI
Bug 38441: ILLHistoryCheck: OPAC
Bug 38441: Allow / render HTML in ILL staff notes
Bug 38441: Style updates for staffnotes
Bug 38441: Add tests
Bug 38441: (QA follow-up): Handle undef userenv
Bug 38441: (QA follow-up): Fix find-misplaced-executables.t
Bug 38441: (QA follow-up): Fix find-missing-filters.t
Bug 38441: (QA follow-up): HistoryCheck now only considers same patron
Bug 38441: New 'ILLHistoryCheck' system preference
Bug 38441: atomicupdate
Bug 38441: HistoryCheck Koha class
Bug 38441: ILLHistoryCheck: Staff UI
Bug 38441: ILLHistoryCheck: OPAC
Bug 38441: Allow / render HTML in ILL staff notes
Bug 38441: Style updates for staffnotes
Bug 38441: Add tests
Bug 38441: (QA follow-up): Handle undef userenv
Bug 38441: (QA follow-up): Fix find-misplaced-executables.t
Bug 38441: (QA follow-up): Fix find-missing-filters.t
Bug 38441: (QA follow-up): HistoryCheck now only considers same patron
Bug 38441: (QA follow-up): Hackfest feedback
Bug 38441: (QA follow-up): Only look for matchin requests if patron
Bug 38441: (QA follow-up): Only look for matchin requests if patron
Bug 38441: New 'ILLHistoryCheck' system preference
Bug 38441: atomicupdate
Bug 38441: HistoryCheck Koha class
Bug 38441: ILLHistoryCheck: Staff UI
Bug 38441: ILLHistoryCheck: OPAC
Bug 38441: Allow / render HTML in ILL staff notes
Bug 38441: Style updates for staffnotes
Bug 38441: Add tests
Bug 38441: (QA follow-up): Handle undef userenv
Bug 38441: (QA follow-up): Fix find-misplaced-executables.t
Bug 38441: (QA follow-up): Fix find-missing-filters.t
Bug 38441: (QA follow-up): HistoryCheck now only considers same patron
Bug 38441: (QA follow-up): Hackfest feedback
Bug 38441: (QA follow-up): Only look for matchin requests if patron
Bug 38441: (QA follow-up) Minor change
Bug 38441: New 'ILLHistoryCheck' system preference
Bug 38441: atomicupdate
Bug 38441: HistoryCheck Koha class
Bug 38441: ILLHistoryCheck: Staff UI
Bug 38441: ILLHistoryCheck: OPAC
Bug 38441: Allow / render HTML in ILL staff notes
Bug 38441: Style updates for staffnotes
Bug 38441: Add tests
Bug 38441: (QA follow-up): Handle undef userenv
Bug 38441: (QA follow-up): Fix find-misplaced-executables.t
Bug 38441: (QA follow-up): Fix find-missing-filters.t
Bug 38441: (QA follow-up): HistoryCheck now only considers same patron
Bug 38441: (QA follow-up): Hackfest feedback
Bug 38441: (QA follow-up): Only look for matchin requests if patron
Bug 38441: (QA follow-up) Minor change

Description Pedro Amorim 2024-11-14 10:49:36 UTC
A workflow stage that performs a check on whether an ILL request being placed has already been placed in the past or not.
Comment 1 Pedro Amorim 2024-11-15 16:20:28 UTC
Created attachment 174604 [details] [review]
Bug 38441: New 'ILLHistoryCheck' system preference
Comment 2 Pedro Amorim 2024-11-15 16:20:30 UTC
Created attachment 174605 [details] [review]
Bug 38441: atomicupdate
Comment 3 Pedro Amorim 2024-11-15 16:20:32 UTC
Created attachment 174606 [details] [review]
Bug 38441: HistoryCheck Koha class
Comment 4 Pedro Amorim 2024-11-15 16:20:36 UTC
Created attachment 174607 [details] [review]
Bug 38441: ILLHistoryCheck: Staff UI
Comment 5 Pedro Amorim 2024-11-15 16:20:38 UTC
Created attachment 174608 [details] [review]
Bug 38441: ILLHistoryCheck: OPAC
Comment 6 Pedro Amorim 2024-11-15 16:20:41 UTC
Created attachment 174609 [details] [review]
Bug 38441: Allow / render HTML in ILL staff notes
Comment 7 Pedro Amorim 2024-11-15 16:20:43 UTC
Created attachment 174610 [details] [review]
Bug 38441: Style updates for staffnotes

- Use pre for staffnotes in staff illview
- Ensure <pre> has same font-family as rest of the page
- Render html for staff notes in list. Truncate to 100 chars max
Comment 8 Pedro Amorim 2024-11-15 16:20:45 UTC
Created attachment 174611 [details] [review]
Bug 38441: Add tests

Test plan:
1) Apply patches
2) Run (ignore if using a sandbox):
  $ reset_all
3) Visit ILL system preferences:
  <staff_url>/cgi-bin/koha/admin/preferences.pl?tab=interlibrary_loans
4) Enable ILLModule and ILLHistoryCheck system preferences

-- STAFF testing --

5) Create a new ILL request, visit:
  <staff_url>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=Standard
6) Pick type = 'Journal article', enter '123' for ISSN, '42' for cardnumber and any library. Click create.
7) Repeat 5) and 6). Notice you're now on the 'Request history check' screen. Verify your previous ILL request is listed. Click 'Submit anyway'.
8) Repeat 5) and 6). But pick a different patron cardnumber this time e.g. '23529000035676'.
9) Verify you're shown the 'Request history check' screen listing the 2 previously created requests. Click 'Submit anyway'.
10) Repeat 5 and 6). Use cardnumber '23529000035676' again.
11) Verify you're shown the 'Request history check' screen. The listing now shows the requests found for this patron first, then lists the remaining found requests for other patrons.

-- OPAC testing --

12) Place a new request through the OPAC, visit:
  <opac_url>/cgi-bin/koha/opac-illrequests.pl?op=add_form&backend=Standard
13) Pick type = 'Journal article', enter '123' for ISSN. Click 'Create'. Notice it shows the 'history check' screen listing the existing 2 requests. Click 'Submit anyway'.
14) Return to the Staff UI:
  <staff_url>/cgi-bin/koha/ill/ill-requests.pl
15) Click on the latest ILL request. Verify 'Staff notes' contain information relating to already existing ILL requests.

-- Bonus testing --
16) On the Staff UI, create a new request for cardnumber 23529000035676 with ISSN = 321
17) On the OPAC, logged in as koha:koha, place a request with ISSN = 321. Verify you don't get shown the history check but the staff notes is still added
18) Play around with ILLModuleDisclaimerByType and
    ILLOpacUnauthenticatedRequest (easy)
19) Play around with ILLCheckAvailability and AutoILLBackendPriority
    (reach out for help using these preferences)

Run tests:
$ prove t/db_dependent/Koha/ILL/Request/Workflow/HistoryCheck.t
Comment 9 Pedro Amorim 2024-12-09 12:09:39 UTC
Created attachment 175300 [details] [review]
Bug 38441: New 'ILLHistoryCheck' system preference
Comment 10 Pedro Amorim 2024-12-09 12:09:42 UTC
Created attachment 175301 [details] [review]
Bug 38441: atomicupdate
Comment 11 Pedro Amorim 2024-12-09 12:09:45 UTC
Created attachment 175302 [details] [review]
Bug 38441: HistoryCheck Koha class
Comment 12 Pedro Amorim 2024-12-09 12:09:47 UTC
Created attachment 175303 [details] [review]
Bug 38441: ILLHistoryCheck: Staff UI
Comment 13 Pedro Amorim 2024-12-09 12:09:50 UTC
Created attachment 175304 [details] [review]
Bug 38441: ILLHistoryCheck: OPAC
Comment 14 Pedro Amorim 2024-12-09 12:09:53 UTC
Created attachment 175305 [details] [review]
Bug 38441: Allow / render HTML in ILL staff notes
Comment 15 Pedro Amorim 2024-12-09 12:09:56 UTC
Created attachment 175306 [details] [review]
Bug 38441: Style updates for staffnotes

- Use pre for staffnotes in staff illview
- Ensure <pre> has same font-family as rest of the page
- Render html for staff notes in list. Truncate to 100 chars max
Comment 16 Pedro Amorim 2024-12-09 12:09:59 UTC
Created attachment 175307 [details] [review]
Bug 38441: Add tests

Test plan:
1) Apply patches
2) Run (ignore if using a sandbox):
  $ reset_all
3) Visit ILL system preferences:
  <staff_url>/cgi-bin/koha/admin/preferences.pl?tab=interlibrary_loans
4) Enable ILLModule and ILLHistoryCheck system preferences

-- STAFF testing --

5) Create a new ILL request, visit:
  <staff_url>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=Standard
6) Pick type = 'Journal article', enter '123' for ISSN, '42' for cardnumber and any library. Click create.
7) Repeat 5) and 6). Notice you're now on the 'Request history check' screen. Verify your previous ILL request is listed. Click 'Submit anyway'.
8) Repeat 5) and 6). But pick a different patron cardnumber this time e.g. '23529000035676'.
9) Verify you're shown the 'Request history check' screen listing the 2 previously created requests. Click 'Submit anyway'.
10) Repeat 5 and 6). Use cardnumber '23529000035676' again.
11) Verify you're shown the 'Request history check' screen. The listing now shows the requests found for this patron first, then lists the remaining found requests for other patrons.

-- OPAC testing --

12) Place a new request through the OPAC, visit:
  <opac_url>/cgi-bin/koha/opac-illrequests.pl?op=add_form&backend=Standard
13) Pick type = 'Journal article', enter '123' for ISSN. Click 'Create'. Notice it shows the 'history check' screen listing the existing 2 requests. Click 'Submit anyway'.
14) Return to the Staff UI:
  <staff_url>/cgi-bin/koha/ill/ill-requests.pl
15) Click on the latest ILL request. Verify 'Staff notes' contain information relating to already existing ILL requests.

-- Bonus testing --
16) On the Staff UI, create a new request for cardnumber 23529000035676 with ISSN = 321
17) On the OPAC, logged in as koha:koha, place a request with ISSN = 321. Verify you don't get shown the history check but the staff notes is still added
18) Play around with ILLModuleDisclaimerByType and
    ILLOpacUnauthenticatedRequest (easy)
19) Play around with ILLCheckAvailability and AutoILLBackendPriority
    (reach out for help using these preferences)

Run tests:
$ prove t/db_dependent/Koha/ILL/Request/Workflow/HistoryCheck.t
Comment 17 Pedro Amorim 2025-01-14 16:43:05 UTC
Created attachment 176507 [details] [review]
Bug 38441: New 'ILLHistoryCheck' system preference
Comment 18 Pedro Amorim 2025-01-14 16:43:09 UTC
Created attachment 176508 [details] [review]
Bug 38441: atomicupdate
Comment 19 Pedro Amorim 2025-01-14 16:43:12 UTC
Created attachment 176509 [details] [review]
Bug 38441: HistoryCheck Koha class
Comment 20 Pedro Amorim 2025-01-14 16:43:14 UTC
Created attachment 176510 [details] [review]
Bug 38441: ILLHistoryCheck: Staff UI
Comment 21 Pedro Amorim 2025-01-14 16:43:17 UTC
Created attachment 176511 [details] [review]
Bug 38441: ILLHistoryCheck: OPAC
Comment 22 Pedro Amorim 2025-01-14 16:43:20 UTC
Created attachment 176512 [details] [review]
Bug 38441: Allow / render HTML in ILL staff notes
Comment 23 Pedro Amorim 2025-01-14 16:43:24 UTC
Created attachment 176513 [details] [review]
Bug 38441: Style updates for staffnotes

- Use pre for staffnotes in staff illview
- Ensure <pre> has same font-family as rest of the page
- Render html for staff notes in list. Truncate to 100 chars max
Comment 24 Pedro Amorim 2025-01-14 16:43:27 UTC
Created attachment 176514 [details] [review]
Bug 38441: Add tests

Test plan:
1) Apply patches
2) Run (ignore if using a sandbox):
  $ reset_all
3) Visit ILL system preferences:
  <staff_url>/cgi-bin/koha/admin/preferences.pl?tab=interlibrary_loans
4) Enable ILLModule and ILLHistoryCheck system preferences

-- STAFF testing --

5) Create a new ILL request, visit:
  <staff_url>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=Standard
6) Pick type = 'Journal article', enter '123' for ISSN, '42' for cardnumber and any library. Click create.
7) Repeat 5) and 6). Notice you're now on the 'Request history check' screen. Verify your previous ILL request is listed. Click 'Submit anyway'.
8) Repeat 5) and 6). But pick a different patron cardnumber this time e.g. '23529000035676'.
9) Verify you're shown the 'Request history check' screen listing the 2 previously created requests. Click 'Submit anyway'.
10) Repeat 5 and 6). Use cardnumber '23529000035676' again.
11) Verify you're shown the 'Request history check' screen. The listing now shows the requests found for this patron first, then lists the remaining found requests for other patrons.

-- OPAC testing --

12) Place a new request through the OPAC, visit:
  <opac_url>/cgi-bin/koha/opac-illrequests.pl?op=add_form&backend=Standard
13) Pick type = 'Journal article', enter '123' for ISSN. Click 'Create'. Notice it shows the 'history check' screen listing the existing 2 requests. Click 'Submit anyway'.
14) Return to the Staff UI:
  <staff_url>/cgi-bin/koha/ill/ill-requests.pl
15) Click on the latest ILL request. Verify 'Staff notes' contain information relating to already existing ILL requests.

-- Bonus testing --
16) On the Staff UI, create a new request for cardnumber 23529000035676 with ISSN = 321
17) On the OPAC, logged in as koha:koha, place a request with ISSN = 321. Verify you don't get shown the history check but the staff notes is still added
18) Play around with ILLModuleDisclaimerByType and
    ILLOpacUnauthenticatedRequest (easy)
19) Play around with ILLCheckAvailability and AutoILLBackendPriority
    (reach out for help using these preferences)

Run tests:
$ prove t/db_dependent/Koha/ILL/Request/Workflow/HistoryCheck.t
Comment 25 Pedro Amorim 2025-01-14 16:44:07 UTC
Rebased to make use of bug 38685
Comment 26 Pedro Amorim 2025-01-15 11:34:00 UTC
Created attachment 176561 [details] [review]
Bug 38441: (QA follow-up): Handle undef userenv

C4::Context->userenv may be undef (after a reset_all, trying to place an unauthenticated request without being logged).
This change prevents a nasty 500 error trying use undefined as hash
Comment 27 Pedro Amorim 2025-01-15 13:16:23 UTC
Created attachment 176566 [details] [review]
Bug 38441: (QA follow-up): Fix find-misplaced-executables.t

It appears the atomic update had wrong permissions:
prove xt/find-misplaced-executables.t
Comment 28 Pedro Amorim 2025-01-15 13:58:04 UTC
Created attachment 176573 [details] [review]
Bug 38441: (QA follow-up): Fix find-missing-filters.t

prove xt/find-missing-filters.t
Comment 29 Pedro Amorim 2025-01-20 16:08:08 UTC
Created attachment 176829 [details] [review]
Bug 38441: (QA follow-up): Fix query

Only add query fields if they're defined.
Comment 30 Pedro Amorim 2025-03-27 17:06:48 UTC
Created attachment 179800 [details] [review]
Bug 38441: New 'ILLHistoryCheck' system preference
Comment 31 Pedro Amorim 2025-03-27 17:06:51 UTC
Created attachment 179801 [details] [review]
Bug 38441: atomicupdate
Comment 32 Pedro Amorim 2025-03-27 17:06:54 UTC
Created attachment 179802 [details] [review]
Bug 38441: HistoryCheck Koha class
Comment 33 Pedro Amorim 2025-03-27 17:06:57 UTC
Created attachment 179803 [details] [review]
Bug 38441: ILLHistoryCheck: Staff UI
Comment 34 Pedro Amorim 2025-03-27 17:07:00 UTC
Created attachment 179804 [details] [review]
Bug 38441: ILLHistoryCheck: OPAC
Comment 35 Pedro Amorim 2025-03-27 17:07:04 UTC
Created attachment 179805 [details] [review]
Bug 38441: Allow / render HTML in ILL staff notes
Comment 36 Pedro Amorim 2025-03-27 17:07:07 UTC
Created attachment 179806 [details] [review]
Bug 38441: Style updates for staffnotes

- Use pre for staffnotes in staff illview
- Ensure <pre> has same font-family as rest of the page
- Render html for staff notes in list. Truncate to 100 chars max
Comment 37 Pedro Amorim 2025-03-27 17:07:10 UTC
Created attachment 179807 [details] [review]
Bug 38441: Add tests

Test plan:
1) Apply patches
2) Run (ignore if using a sandbox):
  $ reset_all
3) Visit ILL system preferences:
  <staff_url>/cgi-bin/koha/admin/preferences.pl?tab=interlibrary_loans
4) Enable ILLModule and ILLHistoryCheck system preferences

-- STAFF testing --

5) Create a new ILL request, visit:
  <staff_url>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=Standard
6) Pick type = 'Journal article', enter '123' for ISSN, '42' for cardnumber and any library. Click create.
7) Repeat 5) and 6). Notice you're now on the 'Request history check' screen. Verify your previous ILL request is listed. Click 'Submit anyway'.
8) Repeat 5) and 6). But pick a different patron cardnumber this time e.g. '23529000035676'.
9) Verify you're shown the 'Request history check' screen listing the 2 previously created requests. Click 'Submit anyway'.
10) Repeat 5 and 6). Use cardnumber '23529000035676' again.
11) Verify you're shown the 'Request history check' screen. The listing now shows the requests found for this patron first, then lists the remaining found requests for other patrons.

-- OPAC testing --

12) Place a new request through the OPAC, visit:
  <opac_url>/cgi-bin/koha/opac-illrequests.pl?op=add_form&backend=Standard
13) Pick type = 'Journal article', enter '123' for ISSN. Click 'Create'. Notice it shows the 'history check' screen listing the existing 2 requests. Click 'Submit anyway'.
14) Return to the Staff UI:
  <staff_url>/cgi-bin/koha/ill/ill-requests.pl
15) Click on the latest ILL request. Verify 'Staff notes' contain information relating to already existing ILL requests.

-- Bonus testing --
16) On the Staff UI, create a new request for cardnumber 23529000035676 with ISSN = 321
17) On the OPAC, logged in as koha:koha, place a request with ISSN = 321. Verify you don't get shown the history check but the staff notes is still added
18) Play around with ILLModuleDisclaimerByType and
    ILLOpacUnauthenticatedRequest (easy)
19) Play around with ILLCheckAvailability and AutoILLBackendPriority
    (reach out for help using these preferences)

Run tests:
$ prove t/db_dependent/Koha/ILL/Request/Workflow/HistoryCheck.t
Comment 38 Pedro Amorim 2025-03-27 17:07:13 UTC
Created attachment 179808 [details] [review]
Bug 38441: (QA follow-up): Handle undef userenv

C4::Context->userenv may be undef (after a reset_all, trying to place an unauthenticated request without being logged).
This change prevents a nasty 500 error trying use undefined as hash
Comment 39 Pedro Amorim 2025-03-27 17:07:16 UTC
Created attachment 179809 [details] [review]
Bug 38441: (QA follow-up): Fix find-misplaced-executables.t

It appears the atomic update had wrong permissions:
prove xt/find-misplaced-executables.t
Comment 40 Pedro Amorim 2025-03-27 17:07:20 UTC
Created attachment 179810 [details] [review]
Bug 38441: (QA follow-up): Fix find-missing-filters.t

prove xt/find-missing-filters.t
Comment 41 Pedro Amorim 2025-03-27 17:07:24 UTC
Created attachment 179811 [details] [review]
Bug 38441: (QA follow-up): HistoryCheck now only considers same patron

Initial implementation checked existing ILLs for same patron and other patrons.
User testing reported ILLs for other patrons are not helpful, and makes this stage
screen show up much more frequently than it should.

This patch simplifies the original implementation to only consider the
patron of the request being placed for an ILL history check.

It also changes the ID matching to only 'DOI' or only 'Pubmed ID' if any of
those is set. This is to minimize scenarios where different articles from
the same journal come up as matching results e.g. same ISSN but different DOI.

It is also updating tests accordingly.

Other QA improvements

Improve HistoryCheck request list:
- Make link external and show icon (to prevent staff member from clicking away from the historyCheck screen unintentionally)
- Show requests' status
- Show requests' placed date
- Add id_prefix to request ID if possible
Comment 42 Pedro Amorim 2025-03-27 17:07:52 UTC
Rebased and tidied on top of tidied bug 36197 and bug 38685
Comment 43 Lisette Scheer 2025-03-31 13:46:42 UTC
Created attachment 180034 [details] [review]
Bug 38441: New 'ILLHistoryCheck' system preference

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 44 Lisette Scheer 2025-03-31 13:46:45 UTC
Created attachment 180035 [details] [review]
Bug 38441: atomicupdate

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 45 Lisette Scheer 2025-03-31 13:46:48 UTC
Created attachment 180036 [details] [review]
Bug 38441: HistoryCheck Koha class

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 46 Lisette Scheer 2025-03-31 13:46:51 UTC
Created attachment 180037 [details] [review]
Bug 38441: ILLHistoryCheck: Staff UI

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 47 Lisette Scheer 2025-03-31 13:46:55 UTC
Created attachment 180038 [details] [review]
Bug 38441: ILLHistoryCheck: OPAC

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 48 Lisette Scheer 2025-03-31 13:46:58 UTC
Created attachment 180039 [details] [review]
Bug 38441: Allow / render HTML in ILL staff notes

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 49 Lisette Scheer 2025-03-31 13:47:02 UTC
Created attachment 180040 [details] [review]
Bug 38441: Style updates for staffnotes

- Use pre for staffnotes in staff illview
- Ensure <pre> has same font-family as rest of the page
- Render html for staff notes in list. Truncate to 100 chars max

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 50 Lisette Scheer 2025-03-31 13:47:05 UTC
Created attachment 180041 [details] [review]
Bug 38441: Add tests

Test plan:
1) Apply patches
2) Run (ignore if using a sandbox):
  $ reset_all
3) Visit ILL system preferences:
  <staff_url>/cgi-bin/koha/admin/preferences.pl?tab=interlibrary_loans
4) Enable ILLModule and ILLHistoryCheck system preferences

-- STAFF testing --

5) Create a new ILL request, visit:
  <staff_url>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=Standard
6) Pick type = 'Journal article', enter '123' for ISSN, '42' for cardnumber and any library. Click create.
7) Repeat 5) and 6). Notice you're now on the 'Request history check' screen. Verify your previous ILL request is listed. Click 'Submit anyway'.
8) Repeat 5) and 6). But pick a different patron cardnumber this time e.g. '23529000035676'.
9) Verify you're shown the 'Request history check' screen listing the 2 previously created requests. Click 'Submit anyway'.
10) Repeat 5 and 6). Use cardnumber '23529000035676' again.
11) Verify you're shown the 'Request history check' screen. The listing now shows the requests found for this patron first, then lists the remaining found requests for other patrons.

-- OPAC testing --

12) Place a new request through the OPAC, visit:
  <opac_url>/cgi-bin/koha/opac-illrequests.pl?op=add_form&backend=Standard
13) Pick type = 'Journal article', enter '123' for ISSN. Click 'Create'. Notice it shows the 'history check' screen listing the existing 2 requests. Click 'Submit anyway'.
14) Return to the Staff UI:
  <staff_url>/cgi-bin/koha/ill/ill-requests.pl
15) Click on the latest ILL request. Verify 'Staff notes' contain information relating to already existing ILL requests.

-- Bonus testing --
16) On the Staff UI, create a new request for cardnumber 23529000035676 with ISSN = 321
17) On the OPAC, logged in as koha:koha, place a request with ISSN = 321. Verify you don't get shown the history check but the staff notes is still added
18) Play around with ILLModuleDisclaimerByType and
    ILLOpacUnauthenticatedRequest (easy)
19) Play around with ILLCheckAvailability and AutoILLBackendPriority
    (reach out for help using these preferences)

Run tests:
$ prove t/db_dependent/Koha/ILL/Request/Workflow/HistoryCheck.t

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 51 Lisette Scheer 2025-03-31 13:47:09 UTC
Created attachment 180042 [details] [review]
Bug 38441: (QA follow-up): Handle undef userenv

C4::Context->userenv may be undef (after a reset_all, trying to place an unauthenticated request without being logged).
This change prevents a nasty 500 error trying use undefined as hash

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 52 Lisette Scheer 2025-03-31 13:47:12 UTC
Created attachment 180043 [details] [review]
Bug 38441: (QA follow-up): Fix find-misplaced-executables.t

It appears the atomic update had wrong permissions:
prove xt/find-misplaced-executables.t

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 53 Lisette Scheer 2025-03-31 13:47:16 UTC
Created attachment 180044 [details] [review]
Bug 38441: (QA follow-up): Fix find-missing-filters.t

prove xt/find-missing-filters.t

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 54 Lisette Scheer 2025-03-31 13:47:21 UTC
Created attachment 180045 [details] [review]
Bug 38441: (QA follow-up): HistoryCheck now only considers same patron

Initial implementation checked existing ILLs for same patron and other patrons.
User testing reported ILLs for other patrons are not helpful, and makes this stage
screen show up much more frequently than it should.

This patch simplifies the original implementation to only consider the
patron of the request being placed for an ILL history check.

It also changes the ID matching to only 'DOI' or only 'Pubmed ID' if any of
those is set. This is to minimize scenarios where different articles from
the same journal come up as matching results e.g. same ISSN but different DOI.

It is also updating tests accordingly.

Other QA improvements

Improve HistoryCheck request list:
- Make link external and show icon (to prevent staff member from clicking away from the historyCheck screen unintentionally)
- Show requests' status
- Show requests' placed date
- Add id_prefix to request ID if possible

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 55 Pedro Amorim 2025-04-02 13:58:47 UTC
TODOs from hackfest QA session:
1) HistoryCheck matched requests: Properly format placed date with Koha date tt filter for history check
2) HistoryCheck matched requests: Details such as branchcode, cardnumber backend need to be capitalized and properly named
3) HistoryCheck matched requests: OPAC and Staff UI should be identical (same data presented in the same way)
4) HistoryCheck matched requests: Show the request status description, not the status code
5) Remove ISSN check from history check matching logic
6) Change UI message "been placed by this patron koha (42) :" to "been placed by koha (42) :"
7) Change UI message "if an ILL request has already been placed during the request process." to "if an ILL request has already been placed by this patron."
8) Change UI message "previous requests" information from staff notes into 'previous_requests'  pipe-separated list of ids
Comment 56 Pedro Amorim 2025-04-14 14:20:12 UTC
Created attachment 180907 [details] [review]
Bug 38441: New 'ILLHistoryCheck' system preference

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 57 Pedro Amorim 2025-04-14 14:20:17 UTC
Created attachment 180908 [details] [review]
Bug 38441: atomicupdate

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 58 Pedro Amorim 2025-04-14 14:20:20 UTC
Created attachment 180909 [details] [review]
Bug 38441: HistoryCheck Koha class

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 59 Pedro Amorim 2025-04-14 14:20:24 UTC
Created attachment 180910 [details] [review]
Bug 38441: ILLHistoryCheck: Staff UI

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 60 Pedro Amorim 2025-04-14 14:20:28 UTC
Created attachment 180911 [details] [review]
Bug 38441: ILLHistoryCheck: OPAC

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 61 Pedro Amorim 2025-04-14 14:20:33 UTC
Created attachment 180912 [details] [review]
Bug 38441: Allow / render HTML in ILL staff notes

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 62 Pedro Amorim 2025-04-14 14:20:38 UTC
Created attachment 180913 [details] [review]
Bug 38441: Style updates for staffnotes

- Use pre for staffnotes in staff illview
- Ensure <pre> has same font-family as rest of the page
- Render html for staff notes in list. Truncate to 100 chars max

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 63 Pedro Amorim 2025-04-14 14:20:42 UTC
Created attachment 180914 [details] [review]
Bug 38441: Add tests

Test plan:
1) Apply patches
2) Run (ignore if using a sandbox):
  $ reset_all
3) Visit ILL system preferences:
  <staff_url>/cgi-bin/koha/admin/preferences.pl?tab=interlibrary_loans
4) Enable ILLModule and ILLHistoryCheck system preferences

-- STAFF testing --

5) Create a new ILL request, visit:
  <staff_url>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=Standard
6) Pick type = 'Journal article', enter '123' for DOI, '42' for cardnumber and any library. Click create.
7) Repeat 5) and 6). Notice you're now on the 'Request history check' screen. Verify your previous ILL request is listed. Click 'Submit anyway'.
8) Repeat 5) and 6). But pick a different patron cardnumber this time e.g. '23529000035676'.
9) Verify you're not shown the 'Request history check' screen as its a different patron.

-- OPAC testing --

10) Place a new request through the OPAC, visit:
  <opac_url>/cgi-bin/koha/opac-illrequests.pl?op=add_form&backend=Standard
11) Pick type = 'Journal article', enter '123' for DOI. Click 'Create'. Notice it shows the 'history check' screen listing the existing requests. Click 'Submit anyway'.
12) Return to the Staff UI:
  <staff_url>/cgi-bin/koha/ill/ill-requests.pl
13) Click on the latest ILL request. Verify 'History check requests' is displayed and contains information relating to the already existing ILL requests.

-- Bonus testing --
16) Play around with ILLModuleDisclaimerByType and
    ILLOpacUnauthenticatedRequest enabled at the same time as
    HistoryCheck (easy)
17) Play around with ILLCheckAvailability and AutoILLBackendPriority
    enabled at the same time as HistoryCheck
    (reach out for help using these preferences)

Run tests:
$ prove t/db_dependent/Koha/ILL/Request/Workflow/HistoryCheck.t

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 64 Pedro Amorim 2025-04-14 14:20:46 UTC
Created attachment 180915 [details] [review]
Bug 38441: (QA follow-up): Handle undef userenv

C4::Context->userenv may be undef (after a reset_all, trying to place an unauthenticated request without being logged).
This change prevents a nasty 500 error trying use undefined as hash

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 65 Pedro Amorim 2025-04-14 14:20:50 UTC
Created attachment 180916 [details] [review]
Bug 38441: (QA follow-up): Fix find-misplaced-executables.t

It appears the atomic update had wrong permissions:
prove xt/find-misplaced-executables.t

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 66 Pedro Amorim 2025-04-14 14:20:54 UTC
Created attachment 180917 [details] [review]
Bug 38441: (QA follow-up): Fix find-missing-filters.t

prove xt/find-missing-filters.t

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 67 Pedro Amorim 2025-04-14 14:20:58 UTC
Created attachment 180918 [details] [review]
Bug 38441: (QA follow-up): HistoryCheck now only considers same patron

Initial implementation checked existing ILLs for same patron and other patrons.
User testing reported ILLs for other patrons are not helpful, and makes this stage
screen show up much more frequently than it should.

This patch simplifies the original implementation to only consider the
patron of the request being placed for an ILL history check.

It also changes the ID matching to only 'DOI' or only 'Pubmed ID' if any of
those is set. This is to minimize scenarios where different articles from
the same journal come up as matching results e.g. same ISSN but different DOI.

It is also updating tests accordingly.

Other QA improvements

Improve HistoryCheck request list:
- Make link external and show icon (to prevent staff member from clicking away from the historyCheck screen unintentionally)
- Show requests' status
- Show requests' placed date
- Add id_prefix to request ID if possible

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 68 Pedro Amorim 2025-04-14 14:21:02 UTC
Created attachment 180919 [details] [review]
Bug 38441: (QA follow-up): Hackfest feedback

This commit addresses all Hackfest QA related feedback:
1) Update of all strings that required update, including UI messages and
   sys pref description
2) Moved the history check information from staffnotes to its own
   pipe-separated illrequestattribute 'historycheck_requests'
3) Fix display of ILL request information on the history check screen
   (Staff), including proper label naming (e.g. branchcode -> library),
show status description instead of code, uppercase labels.
4) Updated OPAC display to match Staff UI's display of matching history
   requests
5) Removed ISSN criteria for matching requests check
6) Update tests accordingly
Comment 69 Pedro Amorim 2025-04-14 14:23:39 UTC
Hi guys thank you for your patience here.
I've now rebased the patch set on top of the updated bug 36197 and bug 38685 and followed up with all the QA feedback provided at Hackfest.
Thank you in advance for the time spent here.
Comment 70 Pedro Amorim 2025-04-17 13:14:03 UTC
Created attachment 181072 [details] [review]
Bug 38441: (QA follow-up): Only look for matchin requests if patron

Previous commit 'HistoryCheck now only considers same patron' added the following regression:
HistoryCheck is performed and shown in the OPAC even if no user is authenticated (using ILLOpacUnauthenticatedRequest).
This fixes that. History check is skipped unless a user exists.
Comment 71 Tomás Cohen Arazi (tcohen) 2025-04-17 15:34:51 UTC
Hi, it all looks great. Can you check this failing test:

```shell
kohadev-koha@kohadevbox:koha(qa_38441)$ prove t/db_dependent/Koha/ILL/Request/Workflow/HistoryCheck.t
t/db_dependent/Koha/ILL/Request/Workflow/HistoryCheck.t .. 1/2     # Looks like you planned 2 tests but ran 1.

#   Failed test 'after_request_created'
#   at t/db_dependent/Koha/ILL/Request/Workflow/HistoryCheck.t line 188.
Can't call method "value" on an undefined value at t/db_dependent/Koha/ILL/Request/Workflow/HistoryCheck.t line 182.
# Looks like your test exited with 11 just after 2.
t/db_dependent/Koha/ILL/Request/Workflow/HistoryCheck.t .. Dubious, test returned 11 (wstat 2816, 0xb00)
Failed 1/2 subtests

Test Summary Report
-------------------
t/db_dependent/Koha/ILL/Request/Workflow/HistoryCheck.t (Wstat: 2816 (exited 11) Tests: 2 Failed: 1)
  Failed test:  2
  Non-zero exit status: 11
Files=1, Tests=2,  2 wallclock secs ( 0.01 usr  0.02 sys +  1.19 cusr  0.62 csys =  1.84 CPU)
Result: FAIL

```
Comment 72 Pedro Amorim 2025-04-17 16:15:36 UTC
Created attachment 181111 [details] [review]
Bug 38441: (QA follow-up): Only look for matchin requests if patron

Previous commit 'HistoryCheck now only considers same patron' added the following regression:
HistoryCheck is performed and shown in the OPAC even if no user is authenticated (using ILLOpacUnauthenticatedRequest).
This fixes that. History check is skipped unless a user exists.
Comment 73 Pedro Amorim 2025-04-17 16:17:05 UTC
(In reply to Tomás Cohen Arazi (tcohen) from comment #71)
> Hi, it all looks great. Can you check this failing test:

Thank you Tomas, my last patch fixed a bug but made it evident that some of the tests were not enforcing the presence of a cardnumber (which the bugfix now checks).

I've squashed the update to the tests to the same patch "Only look for matchin requests if patron" and resubmitted.
Comment 74 Tomás Cohen Arazi (tcohen) 2025-04-17 18:15:51 UTC
Created attachment 181112 [details] [review]
Bug 38441: New 'ILLHistoryCheck' system preference

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 75 Tomás Cohen Arazi (tcohen) 2025-04-17 18:15:54 UTC
Created attachment 181113 [details] [review]
Bug 38441: atomicupdate

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 76 Tomás Cohen Arazi (tcohen) 2025-04-17 18:15:57 UTC
Created attachment 181114 [details] [review]
Bug 38441: HistoryCheck Koha class

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 77 Tomás Cohen Arazi (tcohen) 2025-04-17 18:16:00 UTC
Created attachment 181115 [details] [review]
Bug 38441: ILLHistoryCheck: Staff UI

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 78 Tomás Cohen Arazi (tcohen) 2025-04-17 18:16:03 UTC
Created attachment 181116 [details] [review]
Bug 38441: ILLHistoryCheck: OPAC

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 79 Tomás Cohen Arazi (tcohen) 2025-04-17 18:16:06 UTC
Created attachment 181117 [details] [review]
Bug 38441: Allow / render HTML in ILL staff notes

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 80 Tomás Cohen Arazi (tcohen) 2025-04-17 18:16:09 UTC
Created attachment 181118 [details] [review]
Bug 38441: Style updates for staffnotes

- Use pre for staffnotes in staff illview
- Ensure <pre> has same font-family as rest of the page
- Render html for staff notes in list. Truncate to 100 chars max

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 81 Tomás Cohen Arazi (tcohen) 2025-04-17 18:16:13 UTC
Created attachment 181119 [details] [review]
Bug 38441: Add tests

Test plan:
1) Apply patches
2) Run (ignore if using a sandbox):
  $ reset_all
3) Visit ILL system preferences:
  <staff_url>/cgi-bin/koha/admin/preferences.pl?tab=interlibrary_loans
4) Enable ILLModule and ILLHistoryCheck system preferences

-- STAFF testing --

5) Create a new ILL request, visit:
  <staff_url>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=Standard
6) Pick type = 'Journal article', enter '123' for DOI, '42' for cardnumber and any library. Click create.
7) Repeat 5) and 6). Notice you're now on the 'Request history check' screen. Verify your previous ILL request is listed. Click 'Submit anyway'.
8) Repeat 5) and 6). But pick a different patron cardnumber this time e.g. '23529000035676'.
9) Verify you're not shown the 'Request history check' screen as its a different patron.

-- OPAC testing --

10) Place a new request through the OPAC, visit:
  <opac_url>/cgi-bin/koha/opac-illrequests.pl?op=add_form&backend=Standard
11) Pick type = 'Journal article', enter '123' for DOI. Click 'Create'. Notice it shows the 'history check' screen listing the existing requests. Click 'Submit anyway'.
12) Return to the Staff UI:
  <staff_url>/cgi-bin/koha/ill/ill-requests.pl
13) Click on the latest ILL request. Verify 'History check requests' is displayed and contains information relating to the already existing ILL requests.

-- Bonus testing --
16) Play around with ILLModuleDisclaimerByType and
    ILLOpacUnauthenticatedRequest enabled at the same time as
    HistoryCheck (easy)
17) Play around with ILLCheckAvailability and AutoILLBackendPriority
    enabled at the same time as HistoryCheck
    (reach out for help using these preferences)

Run tests:
$ prove t/db_dependent/Koha/ILL/Request/Workflow/HistoryCheck.t

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 82 Tomás Cohen Arazi (tcohen) 2025-04-17 18:16:16 UTC
Created attachment 181120 [details] [review]
Bug 38441: (QA follow-up): Handle undef userenv

C4::Context->userenv may be undef (after a reset_all, trying to place an unauthenticated request without being logged).
This change prevents a nasty 500 error trying use undefined as hash

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 83 Tomás Cohen Arazi (tcohen) 2025-04-17 18:16:19 UTC
Created attachment 181121 [details] [review]
Bug 38441: (QA follow-up): Fix find-misplaced-executables.t

It appears the atomic update had wrong permissions:
prove xt/find-misplaced-executables.t

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 84 Tomás Cohen Arazi (tcohen) 2025-04-17 18:16:22 UTC
Created attachment 181122 [details] [review]
Bug 38441: (QA follow-up): Fix find-missing-filters.t

prove xt/find-missing-filters.t

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 85 Tomás Cohen Arazi (tcohen) 2025-04-17 18:16:25 UTC
Created attachment 181123 [details] [review]
Bug 38441: (QA follow-up): HistoryCheck now only considers same patron

Initial implementation checked existing ILLs for same patron and other patrons.
User testing reported ILLs for other patrons are not helpful, and makes this stage
screen show up much more frequently than it should.

This patch simplifies the original implementation to only consider the
patron of the request being placed for an ILL history check.

It also changes the ID matching to only 'DOI' or only 'Pubmed ID' if any of
those is set. This is to minimize scenarios where different articles from
the same journal come up as matching results e.g. same ISSN but different DOI.

It is also updating tests accordingly.

Other QA improvements

Improve HistoryCheck request list:
- Make link external and show icon (to prevent staff member from clicking away from the historyCheck screen unintentionally)
- Show requests' status
- Show requests' placed date
- Add id_prefix to request ID if possible

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 86 Tomás Cohen Arazi (tcohen) 2025-04-17 18:16:28 UTC
Created attachment 181124 [details] [review]
Bug 38441: (QA follow-up): Hackfest feedback

This commit addresses all Hackfest QA related feedback:
1) Update of all strings that required update, including UI messages and
   sys pref description
2) Moved the history check information from staffnotes to its own
   pipe-separated illrequestattribute 'historycheck_requests'
3) Fix display of ILL request information on the history check screen
   (Staff), including proper label naming (e.g. branchcode -> library),
show status description instead of code, uppercase labels.
4) Updated OPAC display to match Staff UI's display of matching history
   requests
5) Removed ISSN criteria for matching requests check
6) Update tests accordingly

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 87 Tomás Cohen Arazi (tcohen) 2025-04-17 18:16:31 UTC
Created attachment 181125 [details] [review]
Bug 38441: (QA follow-up): Only look for matchin requests if patron

Previous commit 'HistoryCheck now only considers same patron' added the following regression:
HistoryCheck is performed and shown in the OPAC even if no user is authenticated (using ILLOpacUnauthenticatedRequest).
This fixes that. History check is skipped unless a user exists.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 88 Tomás Cohen Arazi (tcohen) 2025-04-17 18:16:34 UTC
Created attachment 181126 [details] [review]
Bug 38441: (QA follow-up) Minor change

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 89 Katrin Fischer 2025-04-25 06:42:19 UTC
Tried to apply this on top of 36197, but got:

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 38441: New 'ILLHistoryCheck' system preference
Applying: Bug 38441: atomicupdate
Applying: Bug 38441: HistoryCheck Koha class
Applying: Bug 38441: ILLHistoryCheck: Staff UI
error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt).
error: could not build fake ancestor
Patch failed at 0001 Bug 38441: ILLHistoryCheck: Staff UI
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-38441-ILLHistoryCheck-Staff-UI-3hshu7iy.patch

Can you please rebase ASAP?
Comment 90 Pedro Amorim 2025-04-28 12:16:21 UTC
Created attachment 181593 [details] [review]
Bug 38441: New 'ILLHistoryCheck' system preference

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 91 Pedro Amorim 2025-04-28 12:16:24 UTC
Created attachment 181594 [details] [review]
Bug 38441: atomicupdate

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 92 Pedro Amorim 2025-04-28 12:16:28 UTC
Created attachment 181595 [details] [review]
Bug 38441: HistoryCheck Koha class

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 93 Pedro Amorim 2025-04-28 12:16:31 UTC
Created attachment 181596 [details] [review]
Bug 38441: ILLHistoryCheck: Staff UI

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 94 Pedro Amorim 2025-04-28 12:16:35 UTC
Created attachment 181597 [details] [review]
Bug 38441: ILLHistoryCheck: OPAC

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 95 Pedro Amorim 2025-04-28 12:16:39 UTC
Created attachment 181598 [details] [review]
Bug 38441: Allow / render HTML in ILL staff notes

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 96 Pedro Amorim 2025-04-28 12:16:42 UTC
Created attachment 181599 [details] [review]
Bug 38441: Style updates for staffnotes

- Use pre for staffnotes in staff illview
- Ensure <pre> has same font-family as rest of the page
- Render html for staff notes in list. Truncate to 100 chars max

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 97 Pedro Amorim 2025-04-28 12:16:46 UTC
Created attachment 181600 [details] [review]
Bug 38441: Add tests

Test plan:
1) Apply patches
2) Run (ignore if using a sandbox):
  $ reset_all
3) Visit ILL system preferences:
  <staff_url>/cgi-bin/koha/admin/preferences.pl?tab=interlibrary_loans
4) Enable ILLModule and ILLHistoryCheck system preferences

-- STAFF testing --

5) Create a new ILL request, visit:
  <staff_url>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=Standard
6) Pick type = 'Journal article', enter '123' for DOI, '42' for cardnumber and any library. Click create.
7) Repeat 5) and 6). Notice you're now on the 'Request history check' screen. Verify your previous ILL request is listed. Click 'Submit anyway'.
8) Repeat 5) and 6). But pick a different patron cardnumber this time e.g. '23529000035676'.
9) Verify you're not shown the 'Request history check' screen as its a different patron.

-- OPAC testing --

10) Place a new request through the OPAC, visit:
  <opac_url>/cgi-bin/koha/opac-illrequests.pl?op=add_form&backend=Standard
11) Pick type = 'Journal article', enter '123' for DOI. Click 'Create'. Notice it shows the 'history check' screen listing the existing requests. Click 'Submit anyway'.
12) Return to the Staff UI:
  <staff_url>/cgi-bin/koha/ill/ill-requests.pl
13) Click on the latest ILL request. Verify 'History check requests' is displayed and contains information relating to the already existing ILL requests.

-- Bonus testing --
16) Play around with ILLModuleDisclaimerByType and
    ILLOpacUnauthenticatedRequest enabled at the same time as
    HistoryCheck (easy)
17) Play around with ILLCheckAvailability and AutoILLBackendPriority
    enabled at the same time as HistoryCheck
    (reach out for help using these preferences)

Run tests:
$ prove t/db_dependent/Koha/ILL/Request/Workflow/HistoryCheck.t

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 98 Pedro Amorim 2025-04-28 12:16:51 UTC
Created attachment 181601 [details] [review]
Bug 38441: (QA follow-up): Handle undef userenv

C4::Context->userenv may be undef (after a reset_all, trying to place an unauthenticated request without being logged).
This change prevents a nasty 500 error trying use undefined as hash

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 99 Pedro Amorim 2025-04-28 12:16:54 UTC
Created attachment 181602 [details] [review]
Bug 38441: (QA follow-up): Fix find-misplaced-executables.t

It appears the atomic update had wrong permissions:
prove xt/find-misplaced-executables.t

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 100 Pedro Amorim 2025-04-28 12:16:58 UTC
Created attachment 181603 [details] [review]
Bug 38441: (QA follow-up): Fix find-missing-filters.t

prove xt/find-missing-filters.t

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 101 Pedro Amorim 2025-04-28 12:17:02 UTC
Created attachment 181604 [details] [review]
Bug 38441: (QA follow-up): HistoryCheck now only considers same patron

Initial implementation checked existing ILLs for same patron and other patrons.
User testing reported ILLs for other patrons are not helpful, and makes this stage
screen show up much more frequently than it should.

This patch simplifies the original implementation to only consider the
patron of the request being placed for an ILL history check.

It also changes the ID matching to only 'DOI' or only 'Pubmed ID' if any of
those is set. This is to minimize scenarios where different articles from
the same journal come up as matching results e.g. same ISSN but different DOI.

It is also updating tests accordingly.

Other QA improvements

Improve HistoryCheck request list:
- Make link external and show icon (to prevent staff member from clicking away from the historyCheck screen unintentionally)
- Show requests' status
- Show requests' placed date
- Add id_prefix to request ID if possible

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 102 Pedro Amorim 2025-04-28 12:17:06 UTC
Created attachment 181605 [details] [review]
Bug 38441: (QA follow-up): Hackfest feedback

This commit addresses all Hackfest QA related feedback:
1) Update of all strings that required update, including UI messages and
   sys pref description
2) Moved the history check information from staffnotes to its own
   pipe-separated illrequestattribute 'historycheck_requests'
3) Fix display of ILL request information on the history check screen
   (Staff), including proper label naming (e.g. branchcode -> library),
show status description instead of code, uppercase labels.
4) Updated OPAC display to match Staff UI's display of matching history
   requests
5) Removed ISSN criteria for matching requests check
6) Update tests accordingly

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 103 Pedro Amorim 2025-04-28 12:17:09 UTC
Created attachment 181606 [details] [review]
Bug 38441: (QA follow-up): Only look for matchin requests if patron

Previous commit 'HistoryCheck now only considers same patron' added the following regression:
HistoryCheck is performed and shown in the OPAC even if no user is authenticated (using ILLOpacUnauthenticatedRequest).
This fixes that. History check is skipped unless a user exists.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 104 Pedro Amorim 2025-04-28 12:17:13 UTC
Created attachment 181607 [details] [review]
Bug 38441: (QA follow-up) Minor change

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 105 Katrin Fischer 2025-04-28 16:54:42 UTC
Pushed for 25.05!

Well done everyone, thank you!