Bug 38776

Summary: Propose a list of value for restriction comment
Product: Koha Reporter: Arthur Suzuki <arthur.suzuki>
Component: PatronsAssignee: Arthur Suzuki <arthur.suzuki>
Status: Failed QA --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: e.betemps, gmcharlt, jonathan.druart, kyle.m.hall, lucas, m.de.rooy, victor
Version: Main   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: Small patch Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Bug 38776: new "R_COMMENTS" authorized value category for manual restrictions comments
Bug 38776: (QA follow-up) UI improvement + category renamed
Bug 38776: (QA follow-up) hide the other_reason text field on page load
Bug 38776: new "R_COMMENTS" authorized value category for manual restrictions comments
Bug 38776: (QA follow-up) UI improvement + category renamed
Bug 38776: (QA follow-up) hide the other_reason text field on page load
Bug 38776: new "R_COMMENTS" authorized value category for manual restrictions comments
Bug 38776: (QA follow-up) UI improvement + category renamed
Bug 38776: (QA follow-up) hide the other_reason text field on page load

Description Arthur Suzuki 2024-12-23 15:23:34 UTC
It would be nice to let librarians use one of several pre-filled values in the restriction comment for a manual restrictions.
The values could be provided by authorised value with category "R_COMMENTS".
Values could be selected although another comment could be hand-typed.
Comment 1 Arthur Suzuki 2024-12-23 15:30:11 UTC
Created attachment 175912 [details] [review]
Bug 38776: new "R_COMMENTS" authorized value category for manual restrictions comments

Test plan :
- on a patron page, add a manual restriction and fill in a comment, that needs to be typed-in
- apply patch
- create a new authorized value category named "R_COMMENTS" and create a couple of values with a description
- return to patron page and when adding a manual restriction, on click on the input field you would be proposed the authorized values descriptions.
- also you can inspect the code and see that there is a datalist element containing the list of options.
Comment 2 Katrin Fischer 2024-12-23 15:36:18 UTC
Arthur, I like the idea, but there are some issues:

1) Category name

I feel "R_COMMENTS" is too general. I think we don't need to save on characters here looking at some of the other categories. 
What about RESTRICTION_COMMENTS?

2) Replacing free text with authorised value list

This removes the option to add free text entries which might be a needed feature for libraries. I see 2 possible solutions here:

a) Only display if there are existing values for the AV category.
b) Allow both: selecting from the list and free text (like the reasons pull down in suggestions)
Comment 3 Katrin Fischer 2024-12-23 15:38:02 UTC
Realize this is still NEW, so you are probably still working on it, but just in case:

3) Category needs to be added to sample data (just the category, even if no sample values)

4) Database update to add the category.
Comment 4 Arthur Suzuki 2025-01-06 10:08:31 UTC
Hi Katrin,
Indeed this is still a work in progress :)
I'll take all your remarks and make a followup.
Have a nice year 2025 (and following!)
Arthur
Comment 5 Arthur Suzuki 2025-01-06 15:33:06 UTC
Created attachment 176160 [details] [review]
Bug 38776: (QA follow-up) UI improvement + category renamed

- The authorised value category was renamed from "R_COMMENTS" to a
more explicit value : "RESTRICTION_COMMENTS"
- It is now easier in the patron detail page to either select one of
the defined value or to fill in a custom value (select "other" then
type your comment).
Comment 6 Arthur Suzuki 2025-01-06 15:37:09 UTC
Oups, I forgot to say it in the commit message but I've also made an atomicbug to create the authorised_value_category.
I didn't fill in any default value though, because this could change from a library to another + it is not mandatory to have authorised_value for Koha to work (previous behavior preserved).
This is for admin to save some librarian time by filling in some pre-defined restriction_comments frequently occurring values.
Comment 7 Emmanuel Bétemps 2025-01-13 14:49:06 UTC
Hi,

I just tested this patch (on a sandbox)

I created 2 authorised values and the drop-menu displays them
it also displays an 'Others' reason
That works !

But, once the manual restriction added, there is only 'Indefinite' displayed on the patron's account; the reason (authorised value) and its comment (free text) aren't displayed (neither on top of this screen neither in the Restriction tab)

(indefinite is linked to the expiration date (and I didn't fill that field)
Comment 8 Arthur Suzuki 2025-01-13 16:15:12 UTC
I have to make a followup anyway but just demonstrating how it works.
https://youtu.be/4PlaQASVeEs
Comment 9 Arthur Suzuki 2025-01-14 08:39:24 UTC
Created attachment 176493 [details] [review]
Bug 38776: (QA follow-up) hide the other_reason text field on page load
Comment 10 Arthur Suzuki 2025-01-14 08:56:01 UTC
Hello Emmanuel,
I cannot reproduce your issue :/
Can you retry after a page refresh?
It is possible that the javascript is improperly loaded from your cache instead of latest version from source.
Happy bug squashing with Victor + others :)
Comment 11 ByWater Sandboxes 2025-01-14 10:43:44 UTC
Created attachment 176496 [details] [review]
Bug 38776: new "R_COMMENTS" authorized value category for manual restrictions comments

Test plan :
- on a patron page, add a manual restriction and fill in a comment, that needs to be typed-in
- apply patch
- create a new authorized value category named "R_COMMENTS" and create a couple of values with a description
- return to patron page and when adding a manual restriction, on click on the input field you would be proposed the authorized values descriptions.
- also you can inspect the code and see that there is a datalist element containing the list of options.

Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com>
Comment 12 ByWater Sandboxes 2025-01-14 10:43:46 UTC
Created attachment 176497 [details] [review]
Bug 38776: (QA follow-up) UI improvement + category renamed

- The authorised value category was renamed from "R_COMMENTS" to a
more explicit value : "RESTRICTION_COMMENTS"
- It is now easier in the patron detail page to either select one of
the defined value or to fill in a custom value (select "other" then
type your comment).

Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com>
Comment 13 ByWater Sandboxes 2025-01-14 10:43:47 UTC
Created attachment 176498 [details] [review]
Bug 38776: (QA follow-up) hide the other_reason text field on page load

Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com>
Comment 14 Victor Grousset/tuxayo 2025-01-14 13:01:06 UTC
The same sandbox and same URL, the form looks correct on my PC. And on Emmanuel's PC we installed Chromium and tried there and it was also broken. Nothing on the OS (also a not too old Linux) should change how Firefox and Chromium on latest version how display basic HTML forms. It's strait up paranormal! 👽

Anyway, it's not something about the patch. Thanks for retesting.
Comment 15 Arthur Suzuki 2025-01-16 10:53:38 UTC
thanks!
Comment 16 Jonathan Druart 2025-02-12 13:18:27 UTC
Created attachment 177857 [details] [review]
Bug 38776: new "R_COMMENTS" authorized value category for manual restrictions comments

Test plan :
- on a patron page, add a manual restriction and fill in a comment, that needs to be typed-in
- apply patch
- create a new authorized value category named "R_COMMENTS" and create a couple of values with a description
- return to patron page and when adding a manual restriction, on click on the input field you would be proposed the authorized values descriptions.
- also you can inspect the code and see that there is a datalist element containing the list of options.

Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com>
Comment 17 Jonathan Druart 2025-02-12 13:18:30 UTC
Created attachment 177858 [details] [review]
Bug 38776: (QA follow-up) UI improvement + category renamed

- The authorised value category was renamed from "R_COMMENTS" to a
more explicit value : "RESTRICTION_COMMENTS"
- It is now easier in the patron detail page to either select one of
the defined value or to fill in a custom value (select "other" then
type your comment).

Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com>
Comment 18 Jonathan Druart 2025-02-12 13:18:33 UTC
Created attachment 177859 [details] [review]
Bug 38776: (QA follow-up) hide the other_reason text field on page load

Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com>
Comment 19 Jonathan Druart 2025-02-12 13:18:42 UTC
Patches rebased using the auto-rebase script.
Comment 20 Marcel de Rooy 2025-03-07 09:49:40 UTC
 FAIL   installer/data/mysql/atomicupdate/bug_38776.pl
   FAIL   file permissions
                File must have the exec flag

+        $dbh->do(q{INSERT INTO authorised_value_categories VALUES ('RESTRICTION_COMMENTS',0,0);});
This is the dbrev for an upgrade.
But I dont see a patch adding this for new installs in installer/data/mysql/somewhere..