Bug 31562

Summary: Patron 'flags' don't respect unwanted fields
Product: Koha Reporter: Nick Clemens <nick>
Component: PatronsAssignee: Nick Clemens <nick>
Status: CLOSED FIXED QA Contact: Martin Renvoize <martin.renvoize>
Severity: normal    
Priority: P5 - low CC: arthur.suzuki, gmcharlt, kyle.m.hall, lucas, martin.renvoize, victor
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18556
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00, 22.05.07, 21.11.14
Attachments: Bug 31562: Treat flags as other borrower fields
Bug 31562: Remove gonenoaddress and lost from OPAC borrower field preferences
Bug 31562: Respect mandatory fields pref
Bug 31562: Treat flags as other borrower fields
Bug 31562: Remove gonenoaddress and lost from OPAC borrower field preferences
Bug 31562: Respect mandatory fields pref
Bug 31562: Treat flags as other borrower fields
Bug 31562: Remove gonenoaddress and lost from OPAC borrower field preferences
Bug 31562: Respect mandatory fields pref
Bug 31562: Treat flags as other borrower fields
Bug 31562: Remove gonenoaddress and lost from OPAC borrower field preferences
Bug 31562: Respect mandatory fields pref
Bug 31562: [22.05.x] Treat flags as other borrower fields

Description Nick Clemens 2022-09-14 11:26:53 UTC
gonenoaddress and lost are shown in preference BorrowerUnwantedField but do not respect the settings.

This seems to be because they are handled in their own 'flags' code
Comment 1 Nick Clemens 2022-09-14 11:30:25 UTC
Created attachment 140621 [details] [review]
Bug 31562: Treat flags as other borrower fields

Rather than generate a custom hash for these fields, we should treat them as other borrower data fields

To test:
 1 - Edit a patron, note the 'Lost card' and 'Gone no address' fields
 2 - Edit syspref BorrowerunwantedField
 3 - Set gonenoaddress and lost as unwanted
 4 - Edit patron, the fields remain
 5 - Apply patch
 6 - Edit a patron, fields are hidden
 7 - Unhide one of the fields
 8 - Edit a patron and confirm it shows and saves correctly
 9 - Unhide the other field
10 - Confirm it can be edited and saved
Comment 2 Nick Clemens 2022-09-14 11:47:39 UTC
Created attachment 140622 [details] [review]
Bug 31562: Remove gonenoaddress and lost from OPAC borrower field preferences

These are not shown, so cannot be unwanted or mandatory
Comment 3 Nick Clemens 2022-09-14 11:47:44 UTC
Created attachment 140623 [details] [review]
Bug 31562: Respect mandatory fields pref

To test:
1 - Set gonenoaddress and lost in BorrowerMandatoryField
2 - Edit a patron, confirm fields are required
Comment 4 Nick Clemens 2022-09-14 12:12:16 UTC
Created attachment 140624 [details] [review]
Bug 31562: Treat flags as other borrower fields

Rather than generate a custom hash for these fields, we should treat them as other borrower data fields

To test:
 1 - Edit a patron, note the 'Lost card' and 'Gone no address' fields
 2 - Edit syspref BorrowerunwantedField
 3 - Set gonenoaddress and lost as unwanted
 4 - Edit patron, the fields remain
 5 - Apply patch
 6 - Edit a patron, fields are hidden
 7 - Unhide one of the fields
 8 - Edit a patron and confirm it shows and saves correctly
 9 - Unhide the other field
10 - Confirm it can be edited and saved
Comment 5 Nick Clemens 2022-09-14 12:12:20 UTC
Created attachment 140625 [details] [review]
Bug 31562: Remove gonenoaddress and lost from OPAC borrower field preferences

These are not shown, so cannot be unwanted or mandatory
Comment 6 Nick Clemens 2022-09-14 12:12:23 UTC
Created attachment 140626 [details] [review]
Bug 31562: Respect mandatory fields pref

To test:
1 - Set gonenoaddress and lost in BorrowerMandatoryField
2 - Edit a patron, confirm fields are required
Comment 7 David Nind 2022-09-14 18:02:38 UTC
Created attachment 140638 [details] [review]
Bug 31562: Treat flags as other borrower fields

Rather than generate a custom hash for these fields, we should treat them as other borrower data fields

To test:
 1 - Edit a patron, note the 'Lost card' and 'Gone no address' fields
 2 - Edit syspref BorrowerunwantedField
 3 - Set gonenoaddress and lost as unwanted
 4 - Edit patron, the fields remain
 5 - Apply patch
 6 - Edit a patron, fields are hidden
 7 - Unhide one of the fields
 8 - Edit a patron and confirm it shows and saves correctly
 9 - Unhide the other field
10 - Confirm it can be edited and saved

Signed-off-by: David Nind <david@davidnind.com>
Comment 8 David Nind 2022-09-14 18:02:42 UTC
Created attachment 140639 [details] [review]
Bug 31562: Remove gonenoaddress and lost from OPAC borrower field preferences

These are not shown, so cannot be unwanted or mandatory

Signed-off-by: David Nind <david@davidnind.com>
Comment 9 David Nind 2022-09-14 18:02:47 UTC
Created attachment 140640 [details] [review]
Bug 31562: Respect mandatory fields pref

To test:
1 - Set gonenoaddress and lost in BorrowerMandatoryField
2 - Edit a patron, confirm fields are required

Signed-off-by: David Nind <david@davidnind.com>
Comment 10 Martin Renvoize 2022-09-15 11:34:38 UTC
Created attachment 140655 [details] [review]
Bug 31562: Treat flags as other borrower fields

Rather than generate a custom hash for these fields, we should treat them as other borrower data fields

To test:
 1 - Edit a patron, note the 'Lost card' and 'Gone no address' fields
 2 - Edit syspref BorrowerunwantedField
 3 - Set gonenoaddress and lost as unwanted
 4 - Edit patron, the fields remain
 5 - Apply patch
 6 - Edit a patron, fields are hidden
 7 - Unhide one of the fields
 8 - Edit a patron and confirm it shows and saves correctly
 9 - Unhide the other field
10 - Confirm it can be edited and saved

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2022-09-15 11:34:43 UTC
Created attachment 140656 [details] [review]
Bug 31562: Remove gonenoaddress and lost from OPAC borrower field preferences

These are not shown, so cannot be unwanted or mandatory

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize 2022-09-15 11:34:47 UTC
Created attachment 140657 [details] [review]
Bug 31562: Respect mandatory fields pref

To test:
1 - Set gonenoaddress and lost in BorrowerMandatoryField
2 - Edit a patron, confirm fields are required

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize 2022-09-15 11:36:19 UTC
This all appears to check out to me.. code is a littler clearer too.

No QA script failures.

Passing QA
Comment 14 Tomás Cohen Arazi 2022-09-22 12:43:28 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 15 Lucas Gass 2022-10-31 23:00:57 UTC
Patchset won't apply cleanly for 22.05.x. If needed please rebase
Comment 16 Nick Clemens 2022-11-01 11:21:08 UTC
Created attachment 142858 [details] [review]
Bug 31562: [22.05.x] Treat flags as other borrower fields

Rather than generate a custom hash for these fields, we should treat them as other borrower data fields

To test:
 1 - Edit a patron, note the 'Lost card' and 'Gone no address' fields
 2 - Edit syspref BorrowerunwantedField
 3 - Set gonenoaddress and lost as unwanted
 4 - Edit patron, the fields remain
 5 - Apply patch
 6 - Edit a patron, fields are hidden
 7 - Unhide one of the fields
 8 - Edit a patron and confirm it shows and saves correctly
 9 - Unhide the other field
10 - Confirm it can be edited and saved

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 31562: Remove gonenoaddress and lost from OPAC borrower field preferences

These are not shown, so cannot be unwanted or mandatory

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 31562: Respect mandatory fields pref

To test:
1 - Set gonenoaddress and lost in BorrowerMandatoryField
2 - Edit a patron, confirm fields are required

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 17 Lucas Gass 2022-11-01 14:11:27 UTC
Backported to 22.05.x for the upcoming 22.05.07 release
Comment 18 Arthur Suzuki 2022-11-14 17:02:44 UTC
applied to 21.11 for 21.11.14
Comment 19 Victor Grousset/tuxayo 2022-11-26 03:57:46 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.

Nothing to document it seems, marking resolved.