Bug 35106 - ILL - Add patron autocomplete to 'Edit request' Patron ID input
Summary: ILL - Add patron autocomplete to 'Edit request' Patron ID input
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: ILL (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Pedro Amorim
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on: 35094
Blocks: 35589
  Show dependency treegraph
 
Reported: 2023-10-19 11:05 UTC by Pedro Amorim
Modified: 2024-04-30 08:41 UTC (History)
11 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:
24.05.00


Attachments
Bug 35106: Add patron search autocomplete to Patron ID input (2.25 KB, patch)
2023-10-19 11:15 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 35106: Validate entered borrowernumber and biblio_id (5.08 KB, patch)
2023-10-19 11:15 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 35106: Add patron search autocomplete to Patron ID input (2.38 KB, patch)
2023-10-20 08:39 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 35106: Validate entered borrowernumber and biblio_id (5.08 KB, patch)
2023-10-20 08:39 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 35106: Add patron search autocomplete to Patron ID input (2.42 KB, patch)
2023-12-17 23:46 UTC, David Nind
Details | Diff | Splinter Review
Bug 35106: Validate entered borrowernumber and biblio_id (5.12 KB, patch)
2023-12-17 23:46 UTC, David Nind
Details | Diff | Splinter Review
Bug 35106: Add patron search autocomplete to Patron ID input (2.50 KB, patch)
2024-04-02 11:31 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 35106: Validate entered borrowernumber and biblio_id (5.23 KB, patch)
2024-04-02 11:31 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 35106: CSRF fix (1.20 KB, patch)
2024-04-02 11:31 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 35106: Add patron search autocomplete to Patron ID input (2.55 KB, patch)
2024-04-25 13:59 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 35106: Validate entered borrowernumber and biblio_id (5.28 KB, patch)
2024-04-25 13:59 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 35106: CSRF fix (1.25 KB, patch)
2024-04-25 13:59 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Pedro Amorim 2023-10-19 11:05:35 UTC
The Patron ID input in the 'Edit request' screen could provide search patron suggestions to help staff members pick the correct patron.
Comment 1 Pedro Amorim 2023-10-19 11:15:20 UTC Comment hidden (obsolete)
Comment 2 Pedro Amorim 2023-10-19 11:15:22 UTC Comment hidden (obsolete)
Comment 3 Owen Leonard 2023-10-19 16:49:33 UTC
I get an error when the page loads:

Uncaught TypeError: can't access property "_renderItem", node.autocomplete(...).data(...) is undefined
    patron_autocomplete http://127.0.0.1:8081/intranet-tmpl/prog/js/staff-global_23.0600038.js:490
    <anonymous> http://127.0.0.1:8081/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=FreeForm:1826
staff-global_23.0600038.js:490:10
Comment 4 Pedro Amorim 2023-10-19 17:23:43 UTC Comment hidden (obsolete)
Comment 5 Pedro Amorim 2023-10-20 08:39:04 UTC
Created attachment 157498 [details] [review]
Bug 35106: Add patron search autocomplete to Patron ID input

Test plan, k-t-d:
1) Install FreeForm and enable ILLmodule, run:
bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh)
2) Visit /cgi-bin/koha/ill/ill-requests.pl
3) Create 'New ILL request'
4) Select the request type, input cardnumber '42' and select a library. Hit 'Create'
5) Manage the request created just now: click the request id or 'manage request' button from the table
6) Click "Edit request" from the top actions toolbar
7) Input text in Patron ID e.g. 'koha'. Notice you get patron suggestions.
8) Click one of the suggestions and save the request, notice the patron is saved correctly.
Comment 6 Pedro Amorim 2023-10-20 08:39:06 UTC
Created attachment 157499 [details] [review]
Bug 35106: Validate entered borrowernumber and biblio_id

Following up from the test plan of the previous patch:
1) Edit the request again, input gibberish in the Patron ID e.g. 'asdasd'
2) Hit 'Submit'
3) Notice you get a 'The Patron ID you entered is invalid.' message.
4) Edit again, try to empty the input on the Patron ID, hit 'Submit'.
5) Notice it saves the patron as null, as expected

Repeat the test plan, but now for the Bibliographic record ID, notice
the message 'The Bibliographic record ID you entered is invalid.' is
shown if a no biblio was found.
Comment 7 Pedro Amorim 2023-10-20 08:40:38 UTC
(In reply to Owen Leonard from comment #3)
> I get an error when the page loads:
> 
> Uncaught TypeError: can't access property "_renderItem",
> node.autocomplete(...).data(...) is undefined
>     patron_autocomplete
> http://127.0.0.1:8081/intranet-tmpl/prog/js/staff-global_23.0600038.js:490
>     <anonymous>
> http://127.0.0.1:8081/cgi-bin/koha/ill/ill-requests.
> pl?method=create&backend=FreeForm:1826
> staff-global_23.0600038.js:490:10

I was able to reproduce this, thanks!
I've squashed the fix to the original "Add patron search autocomplete to Patron ID input" patch, can you please try again when convenient? Thanks!
Comment 8 David Nind 2023-12-17 23:46:20 UTC
Created attachment 159934 [details] [review]
Bug 35106: Add patron search autocomplete to Patron ID input

Test plan, k-t-d:
1) Install FreeForm and enable ILLmodule, run:
bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh)
2) Visit /cgi-bin/koha/ill/ill-requests.pl
3) Create 'New ILL request'
4) Select the request type, input cardnumber '42' and select a library. Hit 'Create'
5) Manage the request created just now: click the request id or 'manage request' button from the table
6) Click "Edit request" from the top actions toolbar
7) Input text in Patron ID e.g. 'koha'. Notice you get patron suggestions.
8) Click one of the suggestions and save the request, notice the patron is saved correctly.

Signed-off-by: David Nind <david@davidnind.com>
Comment 9 David Nind 2023-12-17 23:46:22 UTC
Created attachment 159935 [details] [review]
Bug 35106: Validate entered borrowernumber and biblio_id

Following up from the test plan of the previous patch:
1) Edit the request again, input gibberish in the Patron ID e.g. 'asdasd'
2) Hit 'Submit'
3) Notice you get a 'The Patron ID you entered is invalid.' message.
4) Edit again, try to empty the input on the Patron ID, hit 'Submit'.
5) Notice it saves the patron as null, as expected

Repeat the test plan, but now for the Bibliographic record ID, notice
the message 'The Bibliographic record ID you entered is invalid.' is
shown if a no biblio was found.

Signed-off-by: David Nind <david@davidnind.com>
Comment 10 David Nind 2023-12-17 23:49:26 UTC
I've signed off as it does what it says in the test plan, but just have one comment.

1. Error messages: these are plain text above the "Request details" section, but are different from how "normal" error messages are displayed. Normally in a coloured box (yellow I think?) in the center.
Comment 11 Pedro Amorim 2023-12-18 09:40:49 UTC
(In reply to David Nind from comment #10)
> I've signed off as it does what it says in the test plan, but just have one
> comment.
> 
> 1. Error messages: these are plain text above the "Request details" section,
> but are different from how "normal" error messages are displayed. Normally
> in a coloured box (yellow I think?) in the center.

Thank you David, you're right. I've opened bug 35589.
Comment 12 Tomás Cohen Arazi 2024-04-01 18:35:30 UTC
Pedro, please rebase. The second patch seems harder to rebase so I leave it to you.
Comment 13 Pedro Amorim 2024-04-02 11:31:46 UTC
Created attachment 164258 [details] [review]
Bug 35106: Add patron search autocomplete to Patron ID input

Test plan, k-t-d:
1) Install FreeForm and enable ILLmodule, run:
bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh)
2) Visit /cgi-bin/koha/ill/ill-requests.pl
3) Create 'New ILL request'
4) Select the request type, input cardnumber '42' and select a library. Hit 'Create'
5) Manage the request created just now: click the request id or 'manage request' button from the table
6) Click "Edit request" from the top actions toolbar
7) Input text in Patron ID e.g. 'koha'. Notice you get patron suggestions.
8) Click one of the suggestions and save the request, notice the patron is saved correctly.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 14 Pedro Amorim 2024-04-02 11:31:49 UTC
Created attachment 164259 [details] [review]
Bug 35106: Validate entered borrowernumber and biblio_id

Following up from the test plan of the previous patch:
1) Edit the request again, input gibberish in the Patron ID e.g. 'asdasd'
2) Hit 'Submit'
3) Notice you get a 'The Patron ID you entered is invalid.' message.
4) Edit again, try to empty the input on the Patron ID, hit 'Submit'.
5) Notice it saves the patron as null, as expected

Repeat the test plan, but now for the Bibliographic record ID, notice
the message 'The Bibliographic record ID you entered is invalid.' is
shown if a no biblio was found.

Signed-off-by: David Nind <david@davidnind.com>

squash this

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 15 Pedro Amorim 2024-04-02 11:31:52 UTC
Created attachment 164260 [details] [review]
Bug 35106: CSRF fix

The removal of 'cud-' from op needs to happen sooner
because ill-requests.tt expects 'edit_action' op,
not 'cud-edit_action'.

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 16 Pedro Amorim 2024-04-02 11:33:12 UTC
(In reply to Tomás Cohen Arazi from comment #12)
> Pedro, please rebase. The second patch seems harder to rebase so I leave it
> to you.

Had to submit a follow-up fix related to CSRF when trying to follow test plan for patch #2.
Comment 17 Tomás Cohen Arazi 2024-04-25 13:59:09 UTC
Created attachment 165543 [details] [review]
Bug 35106: Add patron search autocomplete to Patron ID input

Test plan, k-t-d:
1) Install FreeForm and enable ILLmodule, run:
bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh)
2) Visit /cgi-bin/koha/ill/ill-requests.pl
3) Create 'New ILL request'
4) Select the request type, input cardnumber '42' and select a library. Hit 'Create'
5) Manage the request created just now: click the request id or 'manage request' button from the table
6) Click "Edit request" from the top actions toolbar
7) Input text in Patron ID e.g. 'koha'. Notice you get patron suggestions.
8) Click one of the suggestions and save the request, notice the patron is saved correctly.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 18 Tomás Cohen Arazi 2024-04-25 13:59:12 UTC
Created attachment 165544 [details] [review]
Bug 35106: Validate entered borrowernumber and biblio_id

Following up from the test plan of the previous patch:
1) Edit the request again, input gibberish in the Patron ID e.g. 'asdasd'
2) Hit 'Submit'
3) Notice you get a 'The Patron ID you entered is invalid.' message.
4) Edit again, try to empty the input on the Patron ID, hit 'Submit'.
5) Notice it saves the patron as null, as expected

Repeat the test plan, but now for the Bibliographic record ID, notice
the message 'The Bibliographic record ID you entered is invalid.' is
shown if a no biblio was found.

Signed-off-by: David Nind <david@davidnind.com>

squash this

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 19 Tomás Cohen Arazi 2024-04-25 13:59:15 UTC
Created attachment 165545 [details] [review]
Bug 35106: CSRF fix

The removal of 'cud-' from op needs to happen sooner
because ill-requests.tt expects 'edit_action' op,
not 'cud-edit_action'.

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 20 Katrin Fischer 2024-04-29 16:31:05 UTC
Capitalization:
+                        <pre>The Patron ID you entered is invalid.</pre>
+                    [%   CASE 'invalid_biblio' %]
+                        <pre>The Bibliographic record ID you entered is invalid.</pre>


Bilbiographic and Patron should be lower case. Fixing the capitalization inline.

Also wondering: We don't use Patron ID anywhere in the GUI - should this be cardnumber or username?
Comment 21 Katrin Fischer 2024-04-29 16:55:23 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 22 Pedro Amorim 2024-04-30 08:21:48 UTC
It appears ILL was submitted with "Borrower number" in the UI but was QAd to have "Patron ID" instead:
https://github.com/Koha-Community/Koha/commit/a85ac6b

I don't have an opinion on it and would not be against changing it or keeping it.
Comment 23 Katrin Fischer 2024-04-30 08:41:18 UTC
(In reply to Pedro Amorim from comment #22)
> It appears ILL was submitted with "Borrower number" in the UI but was QAd to
> have "Patron ID" instead:
> https://github.com/Koha-Community/Koha/commit/a85ac6b
> 
> I don't have an opinion on it and would not be against changing it or
> keeping it.

Let's see if we get a stronger vote one way or the other. Since we can now properly, I think Patron ID is much better than Borrower number already.