[Bug 17307] New: Edit button in patron search page doesn' t work without a category code
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17307 Bug ID: 17307 Summary: Edit button in patron search page doesn't work without a category code Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Patrons Assignee: koha-bugs@lists.koha-community.org Reporter: arouss1980@gmail.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com Description of the problem: The 'Edit' button in the patron search page doesn't work without a category code in the URL. [I've searched extensively and couldn't find this bug listed, so I'm submitting it as new. Please note that it's different from: Bug 4508 - can't edit patron attributes using little edit link, and Bug 4427 - Can't edit patron additional attributes and identifiers.] Steps to reproduce (in the current master 0254fd7): Go to Home > Patrons, search for a patron, click on the 'Edit' button (last column). You cannot edit the patron, and you get the following Software error in the browser: Can't call method "get_expiry_date" on an undefined value at /home/aroussos/kohaclone/members/memberentry.pl line 375. Notice how the URL is of the form: cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=52 If you follow a different route and click on the 'Edit' button above the patron name in the patron Details page it works fine because the URL is like this (notice the added '&categorycode=J' at the end): cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=52&categorycode=J -- 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=17307 Andreas Roussos <arouss1980@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|koha-bugs@lists.koha-commun |arouss1980@gmail.com |ity.org | URL| |/cgi-bin/koha/members/membe | |r.pl --- Comment #1 from Andreas Roussos <arouss1980@gmail.com> --- I'm no Perl expert but here's what I've found so far in my attempt to debug this. members/memberentry.pl in the current master (0254fd7): 372 if ( ($op eq 'modify' || $op eq 'insert' || $op eq 'save'|| $op eq 'duplicate') and ($step == 0 or $step == 3 )){ 373 unless ($newdata{'dateexpiry'}){ 374 my $patron_category = Koha::Patron::Categories->find( $newdata{categorycode} ); 375 $newdata{'dateexpiry'} = $patron_category->get_expiry_date( $newdata{dateenrolled} ); 376 } 377 } The problem in line 375 is that the value of the $patron_category variable is undefined, because at line 374 $newdata{categorycode} comes from $input->param(), as per the comment at line 168. Since no 'categorycode' value is passed in the URL, the call to Koha::Patron::Categories->find() fails. So, it seems that the easy fix is to add the categorycode in the URL. It's trivial to fix so I'll provide a patch soon. -- 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=17307 Andreas Roussos <arouss1980@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Edit button in patron |Some edit buttons/links for |search page doesn't work |patrons do not work without |without a category code |a categorycode in the URL -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17307 --- Comment #2 from Andreas Roussos <arouss1980@gmail.com> --- Edited the bug Summary to reflect that this needs fixing in more than one place. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17307 --- Comment #3 from Andreas Roussos <arouss1980@gmail.com> --- Created attachment 55623 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=55623&action=edit Bug 17307 - Some edit buttons/links for patrons do not work without a categorycode in the URL In the Staff client, the 'Edit' button in the patron search page doesn't work. This is also true for one of the 'Edit' links in the patron Detail page, and the 'Edit' link in the Norwegian national patron database search page. This reason behind this is a missing categorycode parameter in the URL, and this patch fixes that. Test plan: 0) [PREREQUISITES FOR STEP 3] Enable the Norwegian national patron database search by setting the following sysprefs: NorwegianPatronDBEnable -> Enable NorwegianPatronDBEndpoint -> test NorwegianPatronDBSearchNLAfterLocalHit -> Don't NorwegianPatronDBUsername -> username NorwegianPatronDBPassword -> password 1) Go to Home > Patrons, search for a patron, click on the 'Edit' button (last column). You cannot edit the patron, and you get the following Software error in your browser: Can't call method "get_expiry_date" on an undefined value at [...] 2) Go to the patron's Detail page and click on the 'Edit' link under the 'Library use' heading on the right. You'll get the same error. 3) Manually navigate to <STAFF_URL>/cgi-bin/koha/members/nl-search.pl, then search for the card number of one of your patrons. Click 'Edit' under the 'Existing patrons' heading. You'll get the same error. 4) Apply the patch. 5) Repeat steps 1), 2), and 3). This time the 'Edit' button/links work. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17307 Andreas Roussos <arouss1980@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch 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=17307 --- Comment #4 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 55698 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=55698&action=edit Bug 17307: Smaller counter-patch Given that the issue is trying to call a method on something that isn't an object, because the category code doesn't exist, then why is it even trying to change the expiry date? A simple postfix if fixes it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17307 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtompset@hotmail.com --- Comment #5 from M. Tompsett <mtompset@hotmail.com> --- (In reply to Andreas Roussos from comment #1)
373 unless ($newdata{'dateexpiry'}){ 374 my $patron_category = Koha::Patron::Categories->find( $newdata{categorycode} ); 375 $newdata{'dateexpiry'} = $patron_category->get_expiry_date( $newdata{dateenrolled} ); 376 }
Let's look at this block of code.
The problem in line 375 is that the value of the $patron_category variable is undefined, because at line 374 $newdata{categorycode} comes from $input->param(), as per the comment at line 168. Since no 'categorycode' value is passed in the URL, the call to Koha::Patron::Categories->find() fails.
So, it seems that the easy fix is to add the categorycode in the URL. It's trivial to fix so I'll provide a patch soon.
While this is all true, that's not what the block of code is trying to do. Patron's should have an expiry date for their account. The problem is not the lack of category code, the problem is the lack of expiry date. This code is trying to correct the expiry date based on the category code. However, given that we don't know what the category code is, the program shouldn't even be trying to set the dateexpiry at all. So, in the case that there is no category found, which is also possible by passing a BAD one, we should not set the dateexpiry. In short, set the dateexpiry to the function call *IF* there is a patron_category. Hence, my counter patch. Feel free to sign off on it and obsolete yours, Andreas, if you think it is better. I don't think fixing all the templates to pass category code is the correct way to handle the problem. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17307 Andreas Roussos <arouss1980@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #55623|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=17307 --- Comment #6 from Andreas Roussos <arouss1980@gmail.com> --- (In reply to M. Tompsett from comment #5) [snip]
While this is all true, that's not what the block of code is trying to do. Patron's should have an expiry date for their account. The problem is not the lack of category code, the problem is the lack of expiry date. This code is trying to correct the expiry date based on the category code. However, given that we don't know what the category code is, the program shouldn't even be trying to set the dateexpiry at all. So, in the case that there is no category found, which is also possible by passing a BAD one, we should not set the dateexpiry. In short, set the dateexpiry to the function call *IF* there is a patron_category. Hence, my counter patch. Feel free to sign off on it and obsolete yours, Andreas, if you think it is better.
I don't think fixing all the templates to pass category code is the correct way to handle the problem. I agree with your analysis above; the code shouldn't be trying to set an expiry date if no patron_category can be found, so it makes more sense to fix this in members/memberentry.pl rather than in the templates.
I've obsoleted my patch and will sign off on yours. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17307 Andreas Roussos <arouss1980@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #55698|0 |1 is obsolete| | --- Comment #7 from Andreas Roussos <arouss1980@gmail.com> --- Created attachment 55732 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=55732&action=edit Bug 17307: Smaller counter-patch Given that the issue is trying to call a method on something that isn't an object, because the category code doesn't exist, then why is it even trying to change the expiry date? A simple postfix if fixes it. Signed-off-by: Andreas Roussos <arouss1980@gmail.com> Applied counter-patch by M. Tompsett. All three patron 'Edit' links mentioned in previous patch's test plan work fine. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17307 Andreas Roussos <arouss1980@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=17307 Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hector.hecaxmmx@gmail.com --- Comment #8 from Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> --- Good Catch, Andreas -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17307 Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #55732|0 |1 is obsolete| | --- Comment #9 from Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> --- Created attachment 55760 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=55760&action=edit Bug 17307: Smaller counter-patch Given that the issue is trying to call a method on something that isn't an object, because the category code doesn't exist, then why is it even trying to change the expiry date? A simple postfix if fixes it. Applied counter-patch by M. Tompsett. All three patron 'Edit' links mentioned in previous patch's test plan work fine. Signed-off-by: Andreas Roussos <arouss1980@gmail.com> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17307 Katrin Fischer <katrin.fischer@bsz-bw.de> 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=17307 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #55760|0 |1 is obsolete| | --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 55805 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=55805&action=edit [PASSED QA] Bug 17307: Smaller counter-patch Given that the issue is trying to call a method on something that isn't an object, because the category code doesn't exist, then why is it even trying to change the expiry date? A simple postfix if fixes it. Applied counter-patch by M. Tompsett. All three patron 'Edit' links mentioned in previous patch's test plan work fine. Signed-off-by: Andreas Roussos <arouss1980@gmail.com> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17307 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Some edit buttons/links for |Some edit buttons/links for |patrons do not work without |patrons do not work |a categorycode in the URL | CC| |katrin.fischer@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17307 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |16911 CC| |jonathan.druart@bugs.koha-c | |ommunity.org Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16911 [Bug 16911] Koha::Patrons - Move ExtendMemberSubscriptionTo to ->renew_account -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17307 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |kyle@bywatersolutions.com --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for 16.11, thanks Mark! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org