Bug 35106

Summary: ILL - Add patron autocomplete to 'Edit request' Patron ID input
Product: Koha Reporter: Pedro Amorim <pedro.amorim>
Component: ILLAssignee: Pedro Amorim <pedro.amorim>
Status: Pushed to main --- QA Contact: Tomás Cohen Arazi <tomascohen>
Severity: enhancement    
Priority: P5 - low CC: david, jonathan.druart, katrin.fischer, lucas, magnus, martin.renvoize, nugged, oleonard, pedro.amorim, tomascohen, victor
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35105
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00
Bug Depends on: 35094    
Bug Blocks: 35589    
Attachments: Bug 35106: Add patron search autocomplete to Patron ID input
Bug 35106: Validate entered borrowernumber and biblio_id
Bug 35106: Add patron search autocomplete to Patron ID input
Bug 35106: Validate entered borrowernumber and biblio_id
Bug 35106: Add patron search autocomplete to Patron ID input
Bug 35106: Validate entered borrowernumber and biblio_id
Bug 35106: Add patron search autocomplete to Patron ID input
Bug 35106: Validate entered borrowernumber and biblio_id
Bug 35106: CSRF fix
Bug 35106: Add patron search autocomplete to Patron ID input
Bug 35106: Validate entered borrowernumber and biblio_id
Bug 35106: CSRF fix

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.