[Bug 36159] New: Patron imports record a change for non-text columns that are not in the import file
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 Bug ID: 36159 Summary: Patron imports record a change for non-text columns that are not in the import file Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Tools Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org When importing patrons we assume a default of '' for borrower columns not supplied in the file. When saving we compare the new object we built to the one form the database - for columns are that are not text type we get undef from the db and '' in the object we make. This means we see a difference and log into the BorrowersLog: "date_renewed" : { "after" : "", "before" : null }, "dateofbirth" : { "after" : "", "before" : null }, "debarred" : { "after" : "", "before" : null }, "flags" : { "after" : "", "before" : null }, "gonenoaddress" : { "after" : "", "before" : null }, "lost" : { "after" : "", "before" : null }, "password_expiration_date" : { "after" : "", "before" : null }, "sms_provider_id" : { "after" : "", "before" : null } } This can mean a lot of useless logging in sites that do automated imports -- 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=36159 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 162392 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162392&action=edit Test import file Enable 'BorrowersLog' system preference Import this file, matchig on cardnuber, and overwriting Check the logs, see the modification of columns that have no date Import again We log it again -- 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=36159 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com, | |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |3820 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=3820 [Bug 3820] More detailed patron record changes log -- 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=36159 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |kyle@bywatersolutions.com |ity.org | -- 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=36159 Kyle M Hall <kyle@bywatersolutions.com> 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=36159 --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 162669 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162669&action=edit Bug 36159: Patron imports record a change for non-text columns that are not in the import file When importing patrons we assume a default of '' for borrower columns not supplied in the file. When saving we compare the new object we built to the one form the database - for columns are that are not text type we get undef from the db and '' in the object we make. This means we see a difference and log into the BorrowersLog: "date_renewed" : { "after" : "", "before" : null }, "dateofbirth" : { "after" : "", "before" : null }, "debarred" : { "after" : "", "before" : null }, "flags" : { "after" : "", "before" : null }, "gonenoaddress" : { "after" : "", "before" : null }, "lost" : { "after" : "", "before" : null }, "password_expiration_date" : { "after" : "", "before" : null }, "sms_provider_id" : { "after" : "", "before" : null } } This can mean a lot of useless logging in sites that do automated imports Test Plan: 1) Enable 'BorrowersLog' system preference 2) Import the borrowers file attach do this bug report file, matchig on cardnuber, and overwriting Contents of the borrowers file are : surname,firstname,branchcode,categorycode,cardnumber,dateenrolled,patron_attributes,lastseen Acosta,Ednb,CPL,PT,23529001000463,02/01/2013,, 3) Check the logs, note the modification of columns that have no date 4) Import the file again with the same settings 5) Note the new action log 6) Apply this patch 7) Restart all the things! 8) Import the file again with the same settings 9) Note no new action log was created! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #162669|0 |1 is obsolete| | --- Comment #3 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 162672 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162672&action=edit Bug 36159: Patron imports record a change for non-text columns that are not in the import file When importing patrons we assume a default of '' for borrower columns not supplied in the file. When saving we compare the new object we built to the one form the database - for columns are that are not text type we get undef from the db and '' in the object we make. This means we see a difference and log into the BorrowersLog: "date_renewed" : { "after" : "", "before" : null }, "dateofbirth" : { "after" : "", "before" : null }, "debarred" : { "after" : "", "before" : null }, "flags" : { "after" : "", "before" : null }, "gonenoaddress" : { "after" : "", "before" : null }, "lost" : { "after" : "", "before" : null }, "password_expiration_date" : { "after" : "", "before" : null }, "sms_provider_id" : { "after" : "", "before" : null } } This can mean a lot of useless logging in sites that do automated imports Test Plan: 1) Enable 'BorrowersLog' system preference 2) Import the borrowers file attach do this bug report file, matchig on cardnuber, and overwriting Contents of the borrowers file are : surname,firstname,branchcode,categorycode,cardnumber,dateenrolled,patron_attributes,lastseen Acosta,Ednb,CPL,PT,23529001000463,02/01/2013,, 3) Check the logs, note the modification of columns that have no date 4) Import the file again with the same settings 5) Note the new action log 6) Apply this patch 7) Restart all the things! 8) Import the file again with the same settings 9) Note no new action log was created! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 Brendan Lawlor <blawlor@clamsnet.org> 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=36159 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #162672|0 |1 is obsolete| | --- Comment #4 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 162689 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162689&action=edit Bug 36159: Patron imports record a change for non-text columns that are not in the import file When importing patrons we assume a default of '' for borrower columns not supplied in the file. When saving we compare the new object we built to the one form the database - for columns are that are not text type we get undef from the db and '' in the object we make. This means we see a difference and log into the BorrowersLog: "date_renewed" : { "after" : "", "before" : null }, "dateofbirth" : { "after" : "", "before" : null }, "debarred" : { "after" : "", "before" : null }, "flags" : { "after" : "", "before" : null }, "gonenoaddress" : { "after" : "", "before" : null }, "lost" : { "after" : "", "before" : null }, "password_expiration_date" : { "after" : "", "before" : null }, "sms_provider_id" : { "after" : "", "before" : null } } This can mean a lot of useless logging in sites that do automated imports Test Plan: 1) Enable 'BorrowersLog' system preference 2) Import the borrowers file attach do this bug report file, matchig on cardnuber, and overwriting Contents of the borrowers file are : surname,firstname,branchcode,categorycode,cardnumber,dateenrolled,patron_attributes,lastseen Acosta,Ednb,CPL,PT,23529001000463,02/01/2013,, 3) Check the logs, note the modification of columns that have no date 4) Import the file again with the same settings 5) Note the new action log 6) Apply this patch 7) Restart all the things! 8) Import the file again with the same settings 9) Note no new action log was created! Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I think the defined check is there so we don't trigger warnings in the comparison... shouldn't we attempt to fix this at the import script level? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 --- Comment #6 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Martin Renvoize from comment #5)
I think the defined check is there so we don't trigger warnings in the comparison... shouldn't we attempt to fix this at the import script level?
I think they will both always have all of the columns: - the db object has them because of schema - the incoming object has them all because we fill everything found in set_column_keys Testing I don't see errors from this new code I do get an unrelated warn before and after the patch: [2024/03/05 18:40:18] [WARN] Use of uninitialized value within %csvkeycol in array element at /kohadevbox/koha/Koha/Patrons/Import.pm line 288, <$fh> line 2. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl Status|Signed Off |Failed QA --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- + if ( ( $from_storage->{$key} || $from_object->{$key} ) + && ( $from_storage->{$key} ne $from_object->{$key} ) ) Not sure why this should be changed. But this will trigger uninit warnings. This fix needs a test. Please add. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 --- Comment #8 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Marcel de Rooy from comment #7)
+ if ( ( $from_storage->{$key} || $from_object->{$key} ) + && ( $from_storage->{$key} ne $from_object->{$key} ) )
Not sure why this should be changed.
This is the actual fix for the bug. The other part of the patch is for handling datexpiry specifically. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #162689|0 |1 is obsolete| | --- Comment #9 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 164038 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164038&action=edit Bug 36159: Patron imports record a change for non-text columns that are not in the import file When importing patrons we assume a default of '' for borrower columns not supplied in the file. When saving we compare the new object we built to the one form the database - for columns are that are not text type we get undef from the db and '' in the object we make. This means we see a difference and log into the BorrowersLog: "date_renewed" : { "after" : "", "before" : null }, "dateofbirth" : { "after" : "", "before" : null }, "debarred" : { "after" : "", "before" : null }, "flags" : { "after" : "", "before" : null }, "gonenoaddress" : { "after" : "", "before" : null }, "lost" : { "after" : "", "before" : null }, "password_expiration_date" : { "after" : "", "before" : null }, "sms_provider_id" : { "after" : "", "before" : null } } This can mean a lot of useless logging in sites that do automated imports Test Plan: 1) Enable 'BorrowersLog' system preference 2) Import the borrowers file attach do this bug report file, matchig on cardnuber, and overwriting Contents of the borrowers file are : surname,firstname,branchcode,categorycode,cardnumber,dateenrolled,patron_attributes,lastseen Acosta,Ednb,CPL,PT,23529001000463,02/01/2013,, 3) Check the logs, note the modification of columns that have no date 4) Import the file again with the same settings 5) Note the new action log 6) Apply this patch 7) Restart all the things! 8) Import the file again with the same settings 9) Note no new action log was created! Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 --- Comment #10 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 164039 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164039&action=edit Bug 36159: (QA follow-up): Simplify code to supress warnings -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 164040 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164040&action=edit Bug 36159: Add unit test -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164038|0 |1 is obsolete| | --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 164113 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164113&action=edit Bug 36159: Patron imports record a change for non-text columns that are not in the import file When importing patrons we assume a default of '' for borrower columns not supplied in the file. When saving we compare the new object we built to the one form the database - for columns are that are not text type we get undef from the db and '' in the object we make. This means we see a difference and log into the BorrowersLog: "date_renewed" : { "after" : "", "before" : null }, "dateofbirth" : { "after" : "", "before" : null }, "debarred" : { "after" : "", "before" : null }, "flags" : { "after" : "", "before" : null }, "gonenoaddress" : { "after" : "", "before" : null }, "lost" : { "after" : "", "before" : null }, "password_expiration_date" : { "after" : "", "before" : null }, "sms_provider_id" : { "after" : "", "before" : null } } This can mean a lot of useless logging in sites that do automated imports Test Plan: 1) Enable 'BorrowersLog' system preference 2) Import the borrowers file attach do this bug report file, matchig on cardnuber, and overwriting Contents of the borrowers file are : surname,firstname,branchcode,categorycode,cardnumber,dateenrolled,patron_attributes,lastseen Acosta,Ednb,CPL,PT,23529001000463,02/01/2013,, 3) Check the logs, note the modification of columns that have no date 4) Import the file again with the same settings 5) Note the new action log 6) Apply this patch 7) Restart all the things! 8) Import the file again with the same settings 9) Note no new action log was created! Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164039|0 |1 is obsolete| | --- Comment #13 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 164114 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164114&action=edit Bug 36159: (QA follow-up): Simplify code to supress warnings Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164040|0 |1 is obsolete| | --- Comment #14 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 164115 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164115&action=edit Bug 36159: Add unit test Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 --- Comment #15 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Kyle M Hall from comment #8)
This is the actual fix for the bug. The other part of the patch is for handling datexpiry specifically.
Had a closer look. Fine with me. Closing my eyes for just a few untidy warns. Probably the datexpiry thing comes from an earlier: $self->dateexpiry( $self->category->get_expiry_date ); -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 --- Comment #16 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 164124 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164124&action=edit Bug 36159: (QA follow-up) Tidy code -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 --- Comment #17 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Thanks for the tidy and also for the unit tests :) Always makes one feel better about changes! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |24.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=36159 --- Comment #18 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Status|Pushed to master |Pushed to stable Version(s)|24.05.00 |24.05.00,23.11.05 released in| | --- Comment #19 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.11.x for 23.11.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=35929 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com --- Comment #20 from Lucas Gass <lucas@bywatersolutions.com> --- Doesn't apply to 23.05.x, no backport. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|martin.renvoize@ptfs-europe | |.com | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36159 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org