[Bug 41145] New: Logging patron attributes logs even if there's no changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41145 Bug ID: 41145 Summary: Logging patron attributes logs even if there's no changes Initiative type: --- Sponsorship --- status: Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Patrons Assignee: koha-bugs@lists.koha-community.org Reporter: lisette@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle@bywatersolutions.com Bug 26744 implemented logging for patron extended attributes. If a patron is in a batch import and the attributes are the same, it logs a change even if nothing else changed, thus making the patron logs large and unhelpful. This can be a problem for academic libraries who have automated loading processes to sync patron with their academic systems. Maybe could use the diff, if diff o=n don't log? -- 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=41145 --- Comment #1 from Gretchen Maxeiner <maxeinergl@gcc.edu> --- We are in this situation. Aside from a huge number of unnecessary log entries for the deleted/readded attributes, the entry is misleading to a staff member trouble-shooting on a patron who doesn't understand the quirk, and it blinds us on actual changes to a patron's attributes. Logging attribute changes is great but we only want to see actual changes. -- 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=41145 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |26744 CC| |aleisha@catalyst.net.nz Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26744 [Bug 26744] Log changes to extended patron attributes -- 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=41145 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |major CC| |jonathan.druart@gmail.com -- 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=41145 Alex Buckley <alexbuckley@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |alexbuckley@catalyst.net.nz --- Comment #2 from Alex Buckley <alexbuckley@catalyst.net.nz> --- Hi Lisette and Gretchen, We notice that the 'before' value is always empty in the modification logs, for example: { "attribute.INSTID" : { "after" : "value", "before" : "" } } Is that something that you are also seeing? Thanks, Alex -- 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=41145 --- Comment #3 from Alex Buckley <alexbuckley@catalyst.net.nz> --- Created attachment 189503 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=189503&action=edit Bug 41145: Do not delete the existing extended_attribute values when importing patrons Since bug 26744 was upstreamed when you use the Import patron tool or the import_patrons.pl script from the commandline and pass a preserve-extended-attributes parameter the borrowerlogs inaccurately have an entry added where the attribute 'before' value is empty. Test plan: 1. Create a patron attribute type named 'INSTID': Koha administration > Patrons and circulation > Patron attribute types > New patron attribute type. 2. Download the patron started CSV from: Tools > Import patrons > click the 'Download a Starter CSV file with all the columns.' link 3. Save values in the: cardnumber, surname, firstname, branchcode, categorycode, auth_method, lang, and patron_attributes columns 4. Make sure your value in the patron_attributes column is INSTID:1234 5. Import this csv file: cd misc ./import_patrons.pl --file <filename>.csv --matchpoint cardnumber --default branchcode=CPL --overwrite --preserve-extended-attributes --verbose --verbose --confirm 6. Navigate to the patron account in your browser 7. Click 'Modification log' and observe how many entries there are 8. Repeat step 5 9. Notice there is a new entry in your Modification log with an info column value of: { "attribute.INSTID" : { "after" : "1234", "before" : "" } } 10. Repeat step 5 and notice another new entry in the Modification log with an empty "before" value 11. Apply patch 12. Repeat step 5 and notice no new entry in the Modification log 13. Edit your csv and change the patron_attributes value to: INSTID:5678 14. Repeat step 5 and notice the new modification log entry with an info column value of: { "attribute.INSTID" : { "after" : "5678", "before" : "1234" } } 15. Add another new patron attribute type to your Koha named 'home' 16. edit your csv and change the patron_attributes value to: INSTID:5678,home:town 17. Repeat step 5 18. Notice a new Modification log entry with the value of: { "attribute.home" : { "after" : "town", "before" : "" } } Sponsored-by: Auckland University of Technology -- 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=41145 Alex Buckley <alexbuckley@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |alexbuckley@catalyst.net.nz |ity.org | Status|NEW |ASSIGNED -- 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=41145 --- Comment #4 from Alex Buckley <alexbuckley@catalyst.net.nz> --- Created attachment 189504 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=189504&action=edit Bug 41145: Add unit test coverage for stopping duplicated borrowerslog logging due to extended patron attributes being deleted on import This adds to the unit tests in t/db_dependent/Koha/Patrons/Import.t. It confirms that repeating the same patron import (with extended patron attributes) will not generate duplicated borrowersLog entries. Test plan: 1. prove t/db_dependent/Koha/Patrons/Import.t Sponsored-by: Auckland University of Technology -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41145 --- Comment #5 from Alex Buckley <alexbuckley@catalyst.net.nz> --- We have a partner library also seeing patron attribute logging even when there is no change made. This only happens for patron imports (either via the 'Import patron' tool in the staff client or running the import_patrons.pl script). Editing a patron account in the staff client and saving the attributes (without making any changes) does not generate extraneous log entries. Reviewing the logs from the patron imports, we see that the 'before' value is always blank, like so: { "attribute.INSTID" : { "after" : "value", "before" : "" } } This is the case even if the patron attribute actually did have an existing value. Debugging the code I think this blanking of the existing patron attribute value results in Koha logging a patron attribute change, because it regards the patron import as setting a value for a blank attribute. Even though that is no actually the case. The existing patron attribute is being blanked by https://git.koha-community.org/Koha-community/Koha/src/commit/74d5af9b39ab1f... I don't think that line should be there, otherwise we cannot pass a 'before' value through to Koha/Patron->extended_attributes() in the next line for logging. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41145 Alex Buckley <alexbuckley@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41145 Alex Buckley <alexbuckley@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Sponsorship status|--- |Sponsored -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41145 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41145 Gretchen Maxeiner <maxeinergl@gcc.edu> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |maxeinergl@gcc.edu --- Comment #6 from Gretchen Maxeiner <maxeinergl@gcc.edu> --- (In reply to Alex Buckley from comment #2)
Hi Lisette and Gretchen,
We notice that the 'before' value is always empty in the modification logs, for example:
{ "attribute.INSTID" : { "after" : "value", "before" : "" } }
Is that something that you are also seeing?
Thanks, Alex
A late reply to confirm this is also what we see -- the attibute "before" is listed as having been blank, but this was not true before the import. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41145 --- Comment #7 from Alex Buckley <alexbuckley@catalyst.net.nz> --- (In reply to Gretchen Maxeiner from comment #6)
(In reply to Alex Buckley from comment #2)
Hi Lisette and Gretchen,
We notice that the 'before' value is always empty in the modification logs, for example:
{ "attribute.INSTID" : { "after" : "value", "before" : "" } }
Is that something that you are also seeing?
Thanks, Alex
A late reply to confirm this is also what we see -- the attibute "before" is listed as having been blank, but this was not true before the import.
Many thanks for confirming Gretchen! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41145 Alex Buckley <alexbuckley@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This bug fix prevents release notes| |misleading patron attribute | |modification logs, when a | |library batch imports | |patrons with the | |BorrowersLog system | |preference set to 'Log'. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41145 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=41145 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #189503|0 |1 is obsolete| | --- Comment #8 from David Nind <david@davidnind.com> --- Created attachment 190139 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190139&action=edit Bug 41145: Do not delete the existing extended_attribute values when importing patrons Since bug 26744 was upstreamed when you use the Import patron tool or the import_patrons.pl script from the commandline and pass a preserve-extended-attributes parameter the borrowerlogs inaccurately have an entry added where the attribute 'before' value is empty. Test plan: 1. Create a patron attribute type named 'INSTID': Koha administration > Patrons and circulation > Patron attribute types > New patron attribute type. 2. Download the patron started CSV from: Tools > Import patrons > click the 'Download a Starter CSV file with all the columns.' link 3. Save values in the: cardnumber, surname, firstname, branchcode, categorycode, auth_method, lang, and patron_attributes columns 4. Make sure your value in the patron_attributes column is INSTID:1234 5. Import this csv file: cd misc ./import_patrons.pl --file <filename>.csv --matchpoint cardnumber --default branchcode=CPL --overwrite --preserve-extended-attributes --verbose --verbose --confirm 6. Navigate to the patron account in your browser 7. Click 'Modification log' and observe how many entries there are 8. Repeat step 5 9. Notice there is a new entry in your Modification log with an info column value of: { "attribute.INSTID" : { "after" : "1234", "before" : "" } } 10. Repeat step 5 and notice another new entry in the Modification log with an empty "before" value 11. Apply patch 12. Repeat step 5 and notice no new entry in the Modification log 13. Edit your csv and change the patron_attributes value to: INSTID:5678 14. Repeat step 5 and notice the new modification log entry with an info column value of: { "attribute.INSTID" : { "after" : "5678", "before" : "1234" } } 15. Add another new patron attribute type to your Koha named 'home' 16. edit your csv and change the patron_attributes value to: INSTID:5678,home:town 17. Repeat step 5 18. Notice a new Modification log entry with the value of: { "attribute.home" : { "after" : "town", "before" : "" } } Sponsored-by: Auckland University of Technology 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=41145 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #189504|0 |1 is obsolete| | --- Comment #9 from David Nind <david@davidnind.com> --- Created attachment 190140 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190140&action=edit Bug 41145: Add unit test coverage for stopping duplicated borrowerslog logging due to extended patron attributes being deleted on import This adds to the unit tests in t/db_dependent/Koha/Patrons/Import.t. It confirms that repeating the same patron import (with extended patron attributes) will not generate duplicated borrowersLog entries. Test plan: 1. prove t/db_dependent/Koha/Patrons/Import.t Sponsored-by: Auckland University of Technology 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=41145 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the|This bug fix prevents |This prevents misleading release notes|misleading patron attribute |patron attribute |modification logs, when a |modification logs, when a |library batch imports |library batch imports |patrons with the |patrons with the |BorrowersLog system |BorrowersLog system |preference set to 'Log'. |preference set to 'Log'. It | |now correctly only shows a | |log entry when a patron | |attribe value is changed. | | | |Example: | |- Before the change: for an | |existing patron with a | |patron attribute of | |INSTID:1234, with a | |re-import the log shows { | |"attribute.INSTID" : { | |"after" : "1234", "before" | |: "" } }, even though there | |is no change to the patron | |attribute. | |- After the change: | | . No log entry is shown | |if is no change to the | |patron attribute. | | . If there is a change to | |the patron attribute (for | |example, changed to 5678 on | |a re-import), it is now | |correctly shown - { | |"attribute.INSTID" : { | |"after" : "5678", "before" | |: "1234" } } CC| |david@davidnind.com --- Comment #10 from David Nind <david@davidnind.com> --- Testing notes (using KTD): 1. Values for the csv file: . cardnumber = 987654321 . surname = Surname . firstname = Firstname . branchcode = CPL . categorycode = PT . auth_method = password . lang = fr . patron_attributes columns = INSTID:1234 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41145 --- Comment #11 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 190150 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190150&action=edit Bug 41145: Do not delete the existing extended_attribute values when importing patrons Since bug 26744 was upstreamed when you use the Import patron tool or the import_patrons.pl script from the commandline and pass a preserve-extended-attributes parameter the borrowerlogs inaccurately have an entry added where the attribute 'before' value is empty. Test plan: 1. Create a patron attribute type named 'INSTID': Koha administration > Patrons and circulation > Patron attribute types > New patron attribute type. 2. Download the patron started CSV from: Tools > Import patrons > click the 'Download a Starter CSV file with all the columns.' link 3. Save values in the: cardnumber, surname, firstname, branchcode, categorycode, auth_method, lang, and patron_attributes columns 4. Make sure your value in the patron_attributes column is INSTID:1234 5. Import this csv file: cd misc ./import_patrons.pl --file <filename>.csv --matchpoint cardnumber --default branchcode=CPL --overwrite --preserve-extended-attributes --verbose --verbose --confirm 6. Navigate to the patron account in your browser 7. Click 'Modification log' and observe how many entries there are 8. Repeat step 5 9. Notice there is a new entry in your Modification log with an info column value of: { "attribute.INSTID" : { "after" : "1234", "before" : "" } } 10. Repeat step 5 and notice another new entry in the Modification log with an empty "before" value 11. Apply patch 12. Repeat step 5 and notice no new entry in the Modification log 13. Edit your csv and change the patron_attributes value to: INSTID:5678 14. Repeat step 5 and notice the new modification log entry with an info column value of: { "attribute.INSTID" : { "after" : "5678", "before" : "1234" } } 15. Add another new patron attribute type to your Koha named 'home' 16. edit your csv and change the patron_attributes value to: INSTID:5678,home:town 17. Repeat step 5 18. Notice a new Modification log entry with the value of: { "attribute.home" : { "after" : "town", "before" : "" } } Sponsored-by: Auckland University of Technology Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41145 --- Comment #12 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 190151 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190151&action=edit Bug 41145: Add unit test coverage for stopping duplicated borrowerslog logging due to extended patron attributes being deleted on import This adds to the unit tests in t/db_dependent/Koha/Patrons/Import.t. It confirms that repeating the same patron import (with extended patron attributes) will not generate duplicated borrowersLog entries. Test plan: 1. prove t/db_dependent/Koha/Patrons/Import.t Sponsored-by: Auckland University of Technology Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41145 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190139|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=41145 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190140|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=41145 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@openfifth.c |y.org |o.uk Status|Signed Off |Passed QA CC| |martin.renvoize@openfifth.c | |o.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41145 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_26_05_candidate CC| |lucas@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41145 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |26.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41145 --- Comment #13 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 26.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41145 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the|This prevents misleading |This prevents misleading release notes|patron attribute |patron attribute |modification logs, when a |modification logs, when a |library batch imports |library batch imports |patrons with the |patrons with the |BorrowersLog system |BorrowersLog system |preference set to 'Log'. It |preference set to 'Log'. It |now correctly only shows a |now correctly only shows a |log entry when a patron |log entry when a patron |attribe value is changed. |attribute value is changed. | | |Example: |Example: |- Before the change: for an |- Before the change: for an |existing patron with a |existing patron with a |patron attribute of |patron attribute of |INSTID:1234, with a |INSTID:1234, with a |re-import the log shows { |re-import the log shows { |"attribute.INSTID" : { |"attribute.INSTID" : { |"after" : "1234", "before" |"after" : "1234", "before" |: "" } }, even though there |: "" } }, even though there |is no change to the patron |is no change to the patron |attribute. |attribute. |- After the change: |- After the change: | . No log entry is shown | . No log entry is shown |if is no change to the |if there is no change to |patron attribute. |the patron attribute. | . If there is a change to | . If there is a change to |the patron attribute (for |the patron attribute (for |example, changed to 5678 on |example, changed to 5678 on |a re-import), it is now |a re-import), it is now |correctly shown - { |correctly shown - { |"attribute.INSTID" : { |"attribute.INSTID" : { |"after" : "5678", "before" |"after" : "5678", "before" |: "1234" } } |: "1234" } } -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41145 Jacob O'Mara <jacob.omara@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jacob.omara@openfifth.co.uk --- Comment #14 from Jacob O'Mara <jacob.omara@openfifth.co.uk> --- Thanks all, pushed to 25.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41145 Jacob O'Mara <jacob.omara@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Pushed to stable Version(s)|26.05.00 |26.05.00,25.11.01 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41145 --- Comment #15 from Jacob O'Mara <jacob.omara@openfifth.co.uk> --- Thanks all, pushed to 25.11.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org