[Bug 16960] New: Patron::Modifications should be fixed
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16960 Bug ID: 16960 Summary: Patron::Modifications should be fixed 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: m.de.rooy@rijksmuseum.nl QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com The changes from opac-memberentry do not reach the table, since the Patron::Modifications object does not work well. Look at this from opac-memberentry: Koha::Patron::Modifications->new( borrowernumber => $borrowernumber ); Passing borrowernumber here is useless, since it is an instance of Koha::Objects. On the contrary, the following call $m->DelModifications; should have the borrowernumber passed ! So what could we add a sub new to Patron::Modifications like: +sub new { + my ( $class, $params ) = @_; + my $self = $class->SUPER::new(); + $self->{borrowernumber} = $params->{borrowernumber}; + return $self; +} And in opac-memberentry: + my $patronmod = Koha::Patron::Modifications->new({ borrowernumber => $borrowernumber }); + $patronmod->DelModifications({ borrowernumber => $borrowernumber }); + $patronmod->AddModifications(\%borrower_changes); But this is a horrible hack. We should not use a Koha::Objects like that.. And what about its unit test: t/db_dependent/Koha_borrower_modifications.t Currently fails ! -- 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=16960 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=16878 -- 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=16960 --- Comment #1 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Not to mention the random stuff interfering here too :) -- 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=16960 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=16929 -- 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=16960 --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Upgrading the severity. I have completely missed that on QAing bug 14757, I focused on the notices code and did not see this part! -- 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=16960 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |14757 CC| |jonathan.druart@bugs.koha-c | |ommunity.org Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14757 [Bug 14757] Allow the use of Template Toolkit syntax for slips and notices -- 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=16960 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |veron@veron.ch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16960 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. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16960 --- Comment #3 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 53657 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53657&action=edit Bug 16960 - Patron::Modifications should be fixed The changes from opac-memberentry do not reach the table, since the Patron::Modifications object does not work well. Test Plan: 1) Apply this patch 2) Create some patron modification requests 3) Ensure you can approve and deny modifications 4) Ensure patron self registration works -- 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=16960 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |kyle@bywatersolutions.com |ity.org | -- 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=16960 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #4 from Marc Véron <veron@veron.ch> --- Hi Kyle, Sorry, after applying the patch, I get an "Internal server error" on Home > Patrons (members-home.pl) plack-error.log says: Can't locate object method "GetPendingModificationsCount" via package "Koha::Patron::Modifications" at /home/marc/koha/members/members-home.pl line 91. git grep GetPendingModifications members/members-home.pl: Koha::Patron::Modifications->GetPendingModificationsCount( $branch ); ^ Leads to error. If I fix this line, members-home.pl displays as expected t/db_dependent/Koha_borrower_modifications.t:'Add a new pending modification and test GetPendingModificationsCount() again' t/db_dependent/Koha_borrower_modifications.t:## Check GetPendingModifications marc@debian8vm:~/koha$ ^ Inside comments, but I think it would be fine to change them for readability Otherwise, sending an update request from OPAC and applying it from Staff client works OK. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16960 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- *** Bug 16985 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16960 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |critical --- Comment #6 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Upping severity - appears to have bad side effects. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16960 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16960 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #53657|0 |1 is obsolete| | --- Comment #7 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 53920 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53920&action=edit Bug 16960 - Patron::Modifications should be fixed The changes from opac-memberentry do not reach the table, since the Patron::Modifications object does not work well. Test Plan: 1) Apply this patch 2) Create some patron modification requests 3) Ensure you can approve and deny modifications 4) Ensure patron self registration works -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16960 --- Comment #8 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Marc Véron from comment #4)
Hi Kyle,
Sorry, after applying the patch, I get an "Internal server error" on Home > Patrons (members-home.pl)
plack-error.log says: Can't locate object method "GetPendingModificationsCount" via package "Koha::Patron::Modifications" at /home/marc/koha/members/members-home.pl line 91.
git grep GetPendingModifications
members/members-home.pl: Koha::Patron::Modifications->GetPendingModificationsCount( $branch ); ^ Leads to error. If I fix this line, members-home.pl displays as expected
t/db_dependent/Koha_borrower_modifications.t:'Add a new pending modification and test GetPendingModificationsCount() again' t/db_dependent/Koha_borrower_modifications.t:## Check GetPendingModifications marc@debian8vm:~/koha$ ^ Inside comments, but I think it would be fine to change them for readability
Otherwise, sending an update request from OPAC and applying it from Staff client works OK.
This should now be fixed! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16960 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- You forgot one call: circ/circulation.pl modifications => Koha::Patron::Modifications->GetModifications({ borrowernumber => $borrowernumber }), And two typos :) requrest applys -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16960 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16960 --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 54027 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54027&action=edit Bug 16960: Update 1 missing occurrence of GetModifications -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16960 Bob Birchall <bob@calyx.net.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bob@calyx.net.au --- Comment #11 from Bob Birchall <bob@calyx.net.au> --- (In reply to Kyle M Hall from comment #7)
Created attachment 53920 [details] [review] Bug 16960 - Patron::Modifications should be fixed
The changes from opac-memberentry do not reach the table, since the Patron::Modifications object does not work well.
Test Plan: 1) Apply this patch 2) Create some patron modification requests 3) Ensure you can approve and deny modifications 4) Ensure patron self registration works
I wasn't completely sure what the test plan was asking for. This is what I did: - logged in to OPAC and changed some personal details; - logged in to staff client and accepted the changes; on searching for the patron the changes are present; - enabled self registration and registered in OPAC as a new patron; logged in as that patron OK; checked in staff - all details present; - logged in to OPAC again, made more changes to Personal Details; returned to Staff client and Denied those changes - on searching none of the data is amended. OK. I'll sign off on this. I'm having an issue with git so but will get some help. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16960 Bob Birchall <bob@calyx.net.au> 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=16960 Bob Birchall <bob@calyx.net.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #53920|0 |1 is obsolete| | --- Comment #12 from Bob Birchall <bob@calyx.net.au> --- Created attachment 54079 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54079&action=edit Bug 16960 - Patron::Modifications should be fixed The changes from opac-memberentry do not reach the table, since the Patron::Modifications object does not work well. Test Plan: 1) Apply this patch 2) Create some patron modification requests 3) Ensure you can approve and deny modifications 4) Ensure patron self registration works Signed-off-by: Bob Birchall <bob@calyx.net.au> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16960 Bob Birchall <bob@calyx.net.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54027|0 |1 is obsolete| | --- Comment #13 from Bob Birchall <bob@calyx.net.au> --- Created attachment 54080 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54080&action=edit Bug 16960: Update 1 missing occurrence of GetModifications Signed-off-by: Bob Birchall <bob@calyx.net.au> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16960 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I think this improves the situation, but there is still a problem with patron self registration: 1) Ensure email verification is mandatory 2) Register a new patron from the OPAC 3) Copy the URL (token now is back!) from the message_queue entry 4) Open the page, receive error: DBIx::Class::Storage::DBI::_dbh_execute(): Column 'address' cannot be null at /home/katrin/kohaclone/C4/Members.pm line 696 I had filled out surname, firstname and email - the address fields were left empty, but were also not set to be mandatory. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16960 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16960 --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 54157 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54157&action=edit Bug 16960: Fix error on validating the registration -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16960 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |17080 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17080 [Bug 17080] Koha::Object->new should handle default values for NOT NULL columns -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16960 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54157|0 |1 is obsolete| | --- Comment #16 from Marc Véron <veron@veron.ch> --- Created attachment 54461 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54461&action=edit Bug 16960: Fix error on validating the registration Followed steps in comment #14, works as expected. Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16960 Marc Véron <veron@veron.ch> 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=16960 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |16990 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16990 [Bug 16990] Show branch name instead of branch code when managing patron modification requests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16960 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #17 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- DBIx::Class::Storage::DBI::_dbh_execute(): Duplicate entry '7874' for key 'PRIMARY' at /home/koha/src/Koha/Object.pm line 109 at /home/koha/src/opac/opac-memberentry.pl line 428 To recreate: At the OPAC, change some of your patron's details. And change the same ones or others => boom -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16960 Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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=16960 --- Comment #18 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 54628 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54628&action=edit Bug 16960: Delete previous patron modifications If a patron edit his/her details a second time, we need to delete the first ones to avoid a "duplicate entry for key PRIMARY" error. Test plan: Log in at the OPAC Edit your details Edit them again => Without this patch, Koha will crash => With the patch applied, everything should work as expected -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16960 --- Comment #19 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 54665 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54665&action=edit [PASSED QA] Bug 16960 - Patron::Modifications should be fixed The changes from opac-memberentry do not reach the table, since the Patron::Modifications object does not work well. Test Plan: 1) Apply this patch 2) Create some patron modification requests 3) Ensure you can approve and deny modifications 4) Ensure patron self registration works Signed-off-by: Bob Birchall <bob@calyx.net.au> 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=16960 --- Comment #20 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 54666 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54666&action=edit [PASSED QA] Bug 16960: Update 1 missing occurrence of GetModifications Signed-off-by: Bob Birchall <bob@calyx.net.au> 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=16960 --- Comment #21 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 54667 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54667&action=edit [PASSED QA] Bug 16960: Fix error on validating the registration Followed steps in comment #14, works as expected. Signed-off-by: Marc Véron <veron@veron.ch> 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=16960 --- Comment #22 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 54668 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54668&action=edit [PASSED QA] Bug 16960: Delete previous patron modifications If a patron edit his/her details a second time, we need to delete the first ones to avoid a "duplicate entry for key PRIMARY" error. Test plan: Log in at the OPAC Edit your details Edit them again => Without this patch, Koha will crash => With the patch applied, everything should work as expected 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=16960 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=16960 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54080|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=16960 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54628|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=16960 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54461|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=16960 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54079|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=16960 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #23 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for 16.11, thanks for the followups Jonathan! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org