https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42161 --- Comment #1 from Aude Charillon <aude.charillon@openfifth.co.uk> --- I also tested: A. Updating the mandatory patron attribute: this works. B. Updating both the mandatory and non-mandatory attribute: only the mandatory is taken into account in steps 10-12. C. Updating the non-mandatory patron attribute in the staff interface: the value correctly appears in the OPAC. Repeating steps 8 to 12: this time, the updated value does correctly display at step 10 in members-update.pl and at step 12 in the patron record. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42161 --- Comment #2 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 199331 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199331&action=edit Bug 42161: Fix identify_updated_extended_attributes for patrons with no prior attributes Two bugs in identify_updated_extended_attributes caused OPAC patron attribute updates to be silently lost: 1. $patron_attribute_types and $passed_attribute_types were declared as undef rather than empty hashrefs. When a patron had zero existing opac_editable attributes, `keys %{undef}` raised a fatal error, swallowing the modification request entirely. 2. The `any` check when comparing values did not filter by attribute code. If any other submitted attribute happened to share the same value as the existing value being tested (e.g. two empty-string attributes), the change was incorrectly treated as a no-op. Adds two new test cases covering both failure modes. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42161 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42161 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #199331|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42161 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42161 --- Comment #4 from David Nind <david@davidnind.com> --- Created attachment 202137 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202137&action=edit Bug 42161: Fix identify_updated_extended_attributes for patrons with no prior attributes Two bugs in identify_updated_extended_attributes caused OPAC patron attribute updates to be silently lost: 1. $patron_attribute_types and $passed_attribute_types were declared as undef rather than empty hashrefs. When a patron had zero existing opac_editable attributes, `keys %{undef}` raised a fatal error, swallowing the modification request entirely. 2. The `any` check when comparing values did not filter by attribute code. If any other submitted attribute happened to share the same value as the existing value being tested (e.g. two empty-string attributes), the change was incorrectly treated as a no-op. Adds two new test cases covering both failure modes. Sponsored-by: OpenFifth <https://openfifth.co.uk/> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42161 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |lisette@bywatersolutions.co |y.org |m CC| |lisette@bywatersolutions.co | |m -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42161 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #8 from Lisette Scheer <lisette@bywatersolutions.com> --- Failing QA since it works before patches -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42161 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #10 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Thanks everyone for testing, and Lisette's Failed QA was fair: the issue does not reproduce with the steps in comment 0. I have gone back through it properly and the original patch overstated things: - The claimed fatal error (keys %{undef}) for a patron with no existing attributes is wrong. The hashes autovivify, so that case already works on main. I have dropped that change and its test. - The second claim, that the value comparison does not filter by attribute code, is a real bug. The steps in comment 0 do not trigger it, because only one non-mandatory attribute is left blank there. It is triggered when the stored value of an attribute equals the submitted value of a different attribute. The typical case is a patron who self-registered leaving two non-mandatory attributes blank: both are stored with an empty value, and setting one of them in the OPAC later is ignored because the other is still submitted as empty. If nothing else changed, no modification request is created at all. I reproduced this on current main at the Koha::Patron level in KTD: stored ONE='' and TWO='', submitted ONE='' and TWO='wheelchair' returns no changes on main, and returns the change to TWO with the patch. I have not clicked through the OPAC and staff interface, so please shout if the new test plan does not behave as described. New patch attached (rebased on current main): - the commit message now describes only the real bug - the two new tests both fail without the fix and pass with it, and they check which attribute is returned, not just the count - the test plan uses two blank attributes The earlier signed-off patch is obsoleted as the code and tests changed, so this needs a fresh sign-off. Sorry for the noise. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42161 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #209586|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42161 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42161 --- Comment #12 from David Nind <david@davidnind.com> --- Created attachment 209629 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209629&action=edit Bug 42161: Fix OPAC attribute changes lost in identify_updated_extended_attributes When a patron edits their attributes in the OPAC, identify_updated_extended_attributes compares each stored attribute with the submitted ones. For an attribute type that has the same number of values stored and submitted, it checks that the stored value is among the submitted values, but it looked at the submitted values of all attribute types instead of only those of the type being checked. So if the old value of an attribute equalled the submitted value of any other attribute, the change was treated as "nothing changed" and was silently lost. The typical case is a patron who self-registered leaving two non-mandatory attributes blank: both are stored with an empty value, and setting one of them in the OPAC later is ignored because the other is still submitted as empty. If nothing else changed, no modification request is created at all. Only compare against the submitted values of the same attribute type. Test plan: 1. Create two patron attribute types, e.g. codes ONE and TWO, with "Display in OPAC" and "Editable in OPAC" ticked, and none of the mandatory options. 2. Set PatronSelfRegistration to 'Allow', make sure PatronSelfRegistrationDefaultCategory is set, and set OPACPatronDetails to 'Allow'. 3. In the OPAC, go to 'Create an account' and fill in the required details. Under 'Additional information' leave both ONE and TWO blank. Submit. 4. Log in to the OPAC as the new patron, using the password shown on the confirmation page. 5. In the 'Personal details' tab, enter a value for TWO only and submit the update request. 6. In the staff interface, go to 'Patrons requesting modifications' (members-update.pl) => Without the patch there is no request for the patron: the change to TWO was lost. => With the patch the request is listed, with TWO under 'Additional attributes and identifiers'. Approve it and check that the patron has the value for TWO. 7. Run $ prove t/db_dependent/Koha/Patron.t => The new tests in the identify_updated_extended_attributes subtest fail without the patch and pass with it. Sponsored-by: OpenFifth <https://openfifth.co.uk/> Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42161 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #14 from Lisette Scheer <lisette@bywatersolutions.com> --- Martin, thanks that works great and I was able to recreate on main. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42161 --- Comment #15 from Lisette Scheer <lisette@bywatersolutions.com> --- Created attachment 209636 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209636&action=edit Bug 42161: Fix OPAC attribute changes lost in identify_updated_extended_attributes When a patron edits their attributes in the OPAC, identify_updated_extended_attributes compares each stored attribute with the submitted ones. For an attribute type that has the same number of values stored and submitted, it checks that the stored value is among the submitted values, but it looked at the submitted values of all attribute types instead of only those of the type being checked. So if the old value of an attribute equalled the submitted value of any other attribute, the change was treated as "nothing changed" and was silently lost. The typical case is a patron who self-registered leaving two non-mandatory attributes blank: both are stored with an empty value, and setting one of them in the OPAC later is ignored because the other is still submitted as empty. If nothing else changed, no modification request is created at all. Only compare against the submitted values of the same attribute type. Test plan: 1. Create two patron attribute types, e.g. codes ONE and TWO, with "Display in OPAC" and "Editable in OPAC" ticked, and none of the mandatory options. 2. Set PatronSelfRegistration to 'Allow', make sure PatronSelfRegistrationDefaultCategory is set, and set OPACPatronDetails to 'Allow'. 3. In the OPAC, go to 'Create an account' and fill in the required details. Under 'Additional information' leave both ONE and TWO blank. Submit. 4. Log in to the OPAC as the new patron, using the password shown on the confirmation page. 5. In the 'Personal details' tab, enter a value for TWO only and submit the update request. 6. In the staff interface, go to 'Patrons requesting modifications' (members-update.pl) => Without the patch there is no request for the patron: the change to TWO was lost. => With the patch the request is listed, with TWO under 'Additional attributes and identifiers'. Approve it and check that the patron has the value for TWO. 7. Run $ prove t/db_dependent/Koha/Patron.t => The new tests in the identify_updated_extended_attributes subtest fail without the patch and pass with it. Sponsored-by: OpenFifth <https://openfifth.co.uk/> Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.