[Bug 26059] New: Create guarantor/guarantee links on patron import
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26059 Bug ID: 26059 Summary: Create guarantor/guarantee links on patron import Change sponsored?: --- Product: Koha Version: 19.11 Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Patrons Assignee: koha-bugs@lists.koha-community.org Reporter: andrew@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com The change to allow multiple guarantors was meant to maintain the ability to import patrons with a Koha patron guarantor, but is not working as intended. -- 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=26059 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |kyle@bywatersolutions.com |ity.org | Depends on| |14570 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14570 [Bug 14570] Make it possible to add multiple guarantors to a record -- 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=26059 --- Comment #1 from Kyle M Hall <kyle@bywatersolutions.com> --- I found a couple problems: A) Adding the guarantor simply doesn't work B) We have two columns named 'relationship', one for the borrowers table, and one for the guarantor relationships table. This clearly doesn't work and will cause confusion. The one for the guarantor relationships table should be renamed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26059 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=26059 --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 107360 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=107360&action=edit Bug 26059: Create guarantor/guarantee links on patron import The change to allow multiple guarantors was meant to maintain the ability to import patrons with a Koha patron guarantor, but is not working as intended. A) Adding the guarantor simply doesn't work B) We have two columns named 'relationship', one for the borrowers table, and one for the guarantor relationships table. This clearly doesn't work and will cause confusion. The one for the guarantor relationships table should be renamed. C) guarantor_firstname and guarantor_surname in the CSV file do nothing and should be removed. This patch also fixes a minor issue that causes warnings like: CGI::param called in list context from /kohadevbox/koha/tools/import_borrowers.pl line 124 Test Plan: 1) Create a CSV with contents like: cardnumber,surname,firstname,branchcode,categorycode,guarantor_relationship,guarantor_id bloop,gloop,froop,MPL,J,father,48 brim,flim,zim,MPL,J,father,48 2) Attempt to upload this file, ensure you have a borrowernumber 48 that can have guarantors 3) Note the accounts are not linked 4) Apply this patch 5) Restart all the things! 6) Upload the file again 7) The patrons should now be linked! 8) Download the starter CSV file 9) Note the second relationship column is now guarantor_relationship 10) Note the columns guarantor_firstname and guarantor_surname are no longer present -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26059 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #107360|0 |1 is obsolete| | --- Comment #3 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 107364 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=107364&action=edit Bug 26059: Create guarantor/guarantee links on patron import The change to allow multiple guarantors was meant to maintain the ability to import patrons with a Koha patron guarantor, but is not working as intended. A) Adding the guarantor simply doesn't work B) We have two columns named 'relationship', one for the borrowers table, and one for the guarantor relationships table. This clearly doesn't work and will cause confusion. The one for the guarantor relationships table should be renamed. C) guarantor_firstname and guarantor_surname in the CSV file do nothing and should be removed. This patch also fixes a minor issue that causes warnings like: CGI::param called in list context from /kohadevbox/koha/tools/import_borrowers.pl line 124 Test Plan: 1) Create a CSV with contents like: cardnumber,surname,firstname,branchcode,categorycode,guarantor_relationship,guarantor_id bloop,gloop,froop,MPL,J,father,48 brim,flim,zim,MPL,J,father,48 2) Attempt to upload this file, ensure you have a borrowernumber 48 that can have guarantors 3) Note the accounts are not linked 4) Apply this patch 5) Restart all the things! 6) Upload the file again 7) The patrons should now be linked! 8) Download the starter CSV file 9) Note the second relationship column is now guarantor_relationship 10) Note the columns guarantor_firstname and guarantor_surname are no longer present -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26059 Amit Gupta <amitddng135@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |amit.gupta@informaticsgloba | |l.com, | |amitddng135@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26059 Amit Gupta <amitddng135@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #107364|0 |1 is obsolete| | --- Comment #4 from Amit Gupta <amitddng135@gmail.com> --- Created attachment 107505 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=107505&action=edit Bug 26059: Create guarantor/guarantee links on patron import Work perfectly. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26059 Amit Gupta <amitddng135@gmail.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=26059 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #107505|0 |1 is obsolete| | --- Comment #5 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 107703 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=107703&action=edit Bug 26059: Create guarantor/guarantee links on patron import The change to allow multiple guarantors was meant to maintain the ability to import patrons with a Koha patron guarantor, but is not working as intended. A) Adding the guarantor simply doesn't work B) We have two columns named 'relationship', one for the borrowers table, and one for the guarantor relationships table. This clearly doesn't work and will cause confusion. The one for the guarantor relationships table should be renamed. C) guarantor_firstname and guarantor_surname in the CSV file do nothing and should be removed. This patch also fixes a minor issue that causes warnings like: CGI::param called in list context from /kohadevbox/koha/tools/import_borrowers.pl line 124 Test Plan: 1) Create a CSV with contents like: cardnumber,surname,firstname,branchcode,categorycode,guarantor_relationship,guarantor_id bloop,gloop,froop,MPL,J,father,48 brim,flim,zim,MPL,J,father,48 2) Attempt to upload this file, ensure you have a borrowernumber 48 that can have guarantors 3) Note the accounts are not linked 4) Apply this patch 5) Restart all the things! 6) Upload the file again 7) The patrons should now be linked! 8) Download the starter CSV file 9) Note the second relationship column is now guarantor_relationship 10) Note the columns guarantor_firstname and guarantor_surname are no longer present Signed-off-by: Amit Gupta <amit.gupta@informaticsglobal.com> Signed-off-by: Marti Fuerst <mfuerst@hmcpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26059 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|19.11 |master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26059 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26059 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #107703|0 |1 is obsolete| | --- Comment #6 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 108319 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=108319&action=edit Bug 26059: Create guarantor/guarantee links on patron import The change to allow multiple guarantors was meant to maintain the ability to import patrons with a Koha patron guarantor, but is not working as intended. A) Adding the guarantor simply doesn't work B) We have two columns named 'relationship', one for the borrowers table, and one for the guarantor relationships table. This clearly doesn't work and will cause confusion. The one for the guarantor relationships table should be renamed. C) guarantor_firstname and guarantor_surname in the CSV file do nothing and should be removed. This patch also fixes a minor issue that causes warnings like: CGI::param called in list context from /kohadevbox/koha/tools/import_borrowers.pl line 124 Test Plan: 1) Create a CSV with contents like: cardnumber,surname,firstname,branchcode,categorycode,guarantor_relationship,guarantor_id bloop,gloop,froop,MPL,J,father,48 brim,flim,zim,MPL,J,father,48 2) Attempt to upload this file, ensure you have a borrowernumber 48 that can have guarantors 3) Note the accounts are not linked 4) Apply this patch 5) Restart all the things! 6) Upload the file again 7) The patrons should now be linked! 8) Download the starter CSV file 9) Note the second relationship column is now guarantor_relationship 10) Note the columns guarantor_firstname and guarantor_surname are no longer present Signed-off-by: Amit Gupta <amit.gupta@informaticsglobal.com> Signed-off-by: Marti Fuerst <mfuerst@hmcpl.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26059 --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Comment on attachment 108319 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=108319 Bug 26059: Create guarantor/guarantee links on patron import Review of attachment 108319: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=26059&attachment=108319) ----------------------------------------------------------------- ::: Koha/Patrons/Import.pm @@ +354,4 @@
Koha::Patron->new(\%borrower)->store; }; unless ( $@ ) { + $borrowernumber = $patron->id;
Is this line really needed? If so please explain. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26059 --- Comment #8 from Kyle M Hall <kyle@bywatersolutions.com> --- $patron is scoped to a different sibling block and doesn't survive to reach the code where the relationship is set. $borrowernumber is scoped to the parent block. Since we only need the borrowernumber, it was simpler to set $borrowernumber than to re-engineer $patron to be in the parent block. (In reply to Jonathan Druart from comment #7)
Comment on attachment 108319 [details] [review] Bug 26059: Create guarantor/guarantee links on patron import
Review of attachment 108319 [details] [review]: -----------------------------------------------------------------
::: Koha/Patrons/Import.pm @@ +354,4 @@
Koha::Patron->new(\%borrower)->store; }; unless ( $@ ) { + $borrowernumber = $patron->id;
Is this line really needed? If so please explain.
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26059 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- ok, I did not notice we were in the "unless $borrowernumber" block. Can you provide tests for this change please? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26059 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26059 --- Comment #10 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 108634 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=108634&action=edit Bug 26059: Add unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26059 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #108634|0 |1 is obsolete| | --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 108635 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=108635&action=edit Bug 26059: Add unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26059 Martha Fuerst <mfuerst@hmcpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mfuerst@hmcpl.org --- Comment #12 from Martha Fuerst <mfuerst@hmcpl.org> --- Making squeaky noises - we're still interested in this. Thanks! -Marti Fuerst -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26059 Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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=26059 Jonathan Druart <jonathan.druart@bugs.koha-community.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=26059 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=26059 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |21.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=26059 --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 21.05, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26059 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|21.05.00 |21.05.00,20.11.03 released in| | CC| |fridolin.somers@biblibre.co | |m Status|Pushed to master |Pushed to stable --- Comment #14 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 20.11.x for 20.11.03 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26059 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable Version(s)|21.05.00,20.11.03 |21.05.00,20.11.03,20.05.09 released in| | --- Comment #15 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Pushed to 20.05.x for 20.05.09 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26059 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |27755 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27755 [Bug 27755] Do not explode when importing patrons without valid guarantor_relationships -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26059 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |RESOLVED Resolution|--- |FIXED CC| |victor@tuxayo.net --- Comment #16 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org