Bug 23420

Summary: Add "SuggestionsUnwantedFields" to hide fields from the suggestions form
Product: Koha Reporter: Lisette Scheer <lisettepalouse+koha>
Component: AcquisitionsAssignee: Lucas Gass <lucas>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: bugzilla, david, jonathan.druart, lisettepalouse+koha, lucas, martin.renvoize
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21329
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This enhancement allows a library to configure the visibility of the input fields on the OPAC suggestion form. **New system preference**: `OPACSuggestionUnwantedFields` **Removed system preference**: `AllowPurchaseSuggestionBranchChoice`
Version(s) released in:
20.11.00
Bug Depends on:    
Bug Blocks: 26296, 29321    
Attachments: Bug 23420: Add OPACSuggestionUnwantedFields option
Bug 23420: Add OPACSuggestionUnwantedFields option
Bug 23420: Add OPACSuggestionUnwantedFields option
Bug 23420: Add OPACSuggestionUnwantedFields option
Bug 23420: (QA follow-up) fix DB update and add syspref.sql
Bug 23420: (follow-up) remove title
Bug 23420: Add OPACSuggestionUnwantedFields system pref
Bug 23420: Add OPACSuggestionUnwantedFields system pref
Bug 23420: Add OPACSuggestionUnwantedFields system pref
Bug 23420: (QA follow-up) Remove AllowPurchaseSuggestionBranchChoice syspref
Bug 23420: Add OPACSuggestionUnwantedFields system pref
Bug 23420: (QA follow-up) Remove AllowPurchaseSuggestionBranchChoice syspref
Bug 23420: (QA follow-up) Remove more AllowPurchaseSuggestionBranchChoice code

Description Lisette Scheer 2019-08-02 15:35:03 UTC
We should have a "SuggestionsUnwantedFields"  similar to the "BorrowersUnwantedFields" to allow libraries to hide fields from the purchase suggestion screens they aren't using. 
Lisette
Comment 1 Lucas Gass 2020-08-01 03:13:06 UTC Comment hidden (obsolete)
Comment 2 ByWater Sandboxes 2020-08-04 15:24:31 UTC Comment hidden (obsolete)
Comment 3 Lisette Scheer 2020-08-04 15:24:51 UTC
Worked great!
Comment 4 Katrin Fischer 2020-08-16 09:30:38 UTC
Really like this, but 2 problems:
- patch set is missing the change to sysprefs.sql
- after updatedatabase is run, the title is hidden by default. The statement needs to be corrected.
Comment 5 Lucas Gass 2020-08-17 22:13:22 UTC Comment hidden (obsolete)
Comment 6 Lucas Gass 2020-08-17 22:16:14 UTC
Thanks Cait,

I added a new patch, kept Lisette's sign-off, but reset the status to 'Needs Signoff'. Not sure what's appropriate here.
Comment 7 Katrin Fischer 2020-08-17 22:22:20 UTC
Usually the best way is adding a second patch was (QA follow-up) - then you can also switch back to signed off, because it makes it easy to see the additional changes. If there are too many, QA could send it back to Needs signoff, but in this case it would be fine. It also means the sign-off remains 'untouched' as the signed patch hasn't changed. (hope that makes sense, it's late!)
Comment 8 Lucas Gass 2020-08-18 01:36:04 UTC
Created attachment 108429 [details] [review]
Bug 23420: Add OPACSuggestionUnwantedFields option

To test:
-Apply patch
-Check OPACSuggestionUnwantedFields
-Check some fields as unwanted
-Go to the OPAC suggestion page and make sure these correct fields are hidden
-Go through each option and make sure it hides properly
Comment 9 Lucas Gass 2020-08-18 01:36:08 UTC
Created attachment 108430 [details] [review]
Bug 23420: (QA follow-up) fix DB update and add syspref.sql
Comment 10 Lucas Gass 2020-08-18 01:40:37 UTC
Also added a note with the sys pref to remind not to makeOPACSuggestionMandatoryFields unwanted
Comment 11 Lucas Gass 2020-08-18 01:42:13 UTC
Probably this is a separate bug but OPACSuggestionMandatoryFields doesn't seem to have a spot in syspref.sql
Comment 12 Owen Leonard 2020-08-25 12:10:48 UTC
Title is a mandatory field even if it isn't selected by OPACSuggestionMandatoryFields. I think that means it should be excluded from this preference.
Comment 13 Lucas Gass 2020-08-30 16:14:31 UTC
Created attachment 109299 [details] [review]
Bug 23420: (follow-up) remove title
Comment 14 Lucas Gass 2020-09-18 21:18:56 UTC
this no longer clean applies, will rebase
Comment 15 Lucas Gass 2020-09-18 22:20:04 UTC
Created attachment 110414 [details] [review]
Bug 23420: Add OPACSuggestionUnwantedFields system pref

To test:
-Apply patch
-Check OPACSuggestionUnwantedFields
-Check some fields as unwanted
-Go to the OPAC suggestion page and make sure these correct fields are hidden
-Go through each option and make sure it hides properly
Comment 16 David Nind 2020-09-19 02:04:24 UTC
The 'Library or branch' field doesn't seem to display at all.

If no unwanted fields are selected, it doesn't display.

When just it is unselected, it doesn't display either.

All other fields can be displayed or not displayed.
Comment 17 Lucas Gass 2020-09-19 14:32:14 UTC
(In reply to David Nind from comment #16)
> The 'Library or branch' field doesn't seem to display at all.
> 
> If no unwanted fields are selected, it doesn't display.
> 
> When just it is unselected, it doesn't display either.
> 
> All other fields can be displayed or not displayed.

Hi David,

Thanks for testing. The branch/library selection field will not show on the purchase suggestions form unless the system preference 'AllowPurchaseSuggestionBranchChoice' is set to allow. 

Also worth noting, the 'Patron Reason' field will only display when there are authorized values in the OPAC_SUG category.
Comment 18 Lucas Gass 2020-09-19 15:16:37 UTC
Created attachment 110418 [details] [review]
Bug 23420: Add OPACSuggestionUnwantedFields system pref

To test:
-Apply patch
-Check OPACSuggestionUnwantedFields
-Check some fields as unwanted
-Go to the OPAC suggestion page and make sure these correct fields are hidden
-Turn on the system preference AllowPurchaseSuggestionBranchChoice to see the branch choice on the form
-Make sure you can correctly hide this one and still submit the purchase suggestion for without error.
-Go through each option and make sure it hides properly.

Note: You will only see the patron reason field when there are authorised values in the OPAC_SUG category
Comment 19 Lucas Gass 2020-09-19 15:17:50 UTC
I submitted a new patch with an updated test plan so I am switching this back to Needs Sign-off
Comment 20 David Nind 2020-09-19 19:16:55 UTC
Created attachment 110419 [details] [review]
Bug 23420: Add OPACSuggestionUnwantedFields system pref

To test:
-Apply patch
-Check OPACSuggestionUnwantedFields
-Check some fields as unwanted
-Go to the OPAC suggestion page and make sure these correct fields are hidden
-Turn on the system preference AllowPurchaseSuggestionBranchChoice to see the branch choice on the form
-Make sure you can correctly hide this one and still submit the purchase suggestion for without error.
-Go through each option and make sure it hides properly.

Note: You will only see the patron reason field when there are authorised values in the OPAC_SUG category

Signed-off-by: David Nind <david@davidnind.com>
Comment 21 David Nind 2020-09-19 19:20:37 UTC
Thanks Lucas! I have retested and signed off. Everything works as expected now with AllowPurchaseSuggestionBranchChoice enabled. David
Comment 22 Katrin Fischer 2020-09-20 19:51:58 UTC
Hi Lucas, I feel like having 2 switches for the same thing (visibility of library ) can be confusing. Is there a reason we should not take the chance here to remove AllowPurchaseSuggestionBranchChoice in favor of the new pref?

We could add a line to the databaseupdate to make the branch invisible if AllowPurchaseSuggestionBranchChoice is set to don't allow now.
Comment 23 Lucas Gass 2020-09-24 15:48:37 UTC
Created attachment 110671 [details] [review]
Bug 23420: (QA follow-up) Remove AllowPurchaseSuggestionBranchChoice syspref
Comment 24 Katrin Fischer 2020-09-24 18:35:40 UTC
Created attachment 110688 [details] [review]
Bug 23420: Add OPACSuggestionUnwantedFields system pref

To test:
-Apply patch
-Check OPACSuggestionUnwantedFields
-Check some fields as unwanted
-Go to the OPAC suggestion page and make sure these correct fields are hidden
-Turn on the system preference AllowPurchaseSuggestionBranchChoice to see the branch choice on the form
-Make sure you can correctly hide this one and still submit the purchase suggestion for without error.
-Go through each option and make sure it hides properly.

Note: You will only see the patron reason field when there are authorised values in the OPAC_SUG category

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 25 Katrin Fischer 2020-09-24 18:35:51 UTC
Created attachment 110689 [details] [review]
Bug 23420: (QA follow-up) Remove AllowPurchaseSuggestionBranchChoice syspref

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 26 Katrin Fischer 2020-09-24 18:37:12 UTC
Thx for the follow-up, Lucas!
Comment 27 Jonathan Druart 2020-10-01 12:55:44 UTC
The pref is still used

% git grep -i AllowPurchaseSuggestionBranchChoice **/*.pm **/*.pl **/*.t
C4/UsageStats.pm:        AllowPurchaseSuggestionBranchChoice
installer/data/mysql/updatedatabase.pl:    $dbh->do("INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES ('AllowPurchaseSuggestionBranchChoice', 0, 'Allow user to choose branch when making a purchase suggestion','1','YesNo')");
opac/opac-suggestions.pl:if ( C4::Context->preference("AllowPurchaseSuggestionBranchChoice") ) {
t/db_dependent/UsageStats.t:        AllowPurchaseSuggestionBranchChoice
Comment 28 Lucas Gass 2020-10-05 17:44:56 UTC
Created attachment 111254 [details] [review]
Bug 23420: (QA follow-up) Remove more AllowPurchaseSuggestionBranchChoice code

Test plan:
1. git grep -i AllowPurchaseSuggestionBranchChoice **/*.pm **/*.pl **/*.t
2. There should no longer be any code related to the old sys pref AllowPurchaseSuggestionBranchChoice
Comment 29 Jonathan Druart 2020-10-09 11:02:19 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 30 Lucas Gass 2020-10-20 18:21:16 UTC
enhancement will not be backported to 20.05.x