Bug 31562 - Patron 'flags' don't respect unwanted fields
Summary: Patron 'flags' don't respect unwanted fields
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-09-14 11:26 UTC by Nick Clemens
Modified: 2023-12-28 20:42 UTC (History)
6 users (show)

See Also:
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 (9.14 KB, patch)
2022-09-14 11:30 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31562: Remove gonenoaddress and lost from OPAC borrower field preferences (2.68 KB, patch)
2022-09-14 11:47 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31562: Respect mandatory fields pref (4.02 KB, patch)
2022-09-14 11:47 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31562: Treat flags as other borrower fields (9.43 KB, patch)
2022-09-14 12:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31562: Remove gonenoaddress and lost from OPAC borrower field preferences (2.68 KB, patch)
2022-09-14 12:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31562: Respect mandatory fields pref (4.02 KB, patch)
2022-09-14 12:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31562: Treat flags as other borrower fields (9.48 KB, patch)
2022-09-14 18:02 UTC, David Nind
Details | Diff | Splinter Review
Bug 31562: Remove gonenoaddress and lost from OPAC borrower field preferences (2.73 KB, patch)
2022-09-14 18:02 UTC, David Nind
Details | Diff | Splinter Review
Bug 31562: Respect mandatory fields pref (4.07 KB, patch)
2022-09-14 18:02 UTC, David Nind
Details | Diff | Splinter Review
Bug 31562: Treat flags as other borrower fields (9.54 KB, patch)
2022-09-15 11:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31562: Remove gonenoaddress and lost from OPAC borrower field preferences (2.79 KB, patch)
2022-09-15 11:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31562: Respect mandatory fields pref (4.13 KB, patch)
2022-09-15 11:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31562: [22.05.x] Treat flags as other borrower fields (13.45 KB, patch)
2022-11-01 11:21 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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.