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 !
Not to mention the random stuff interfering here too :)
Upgrading the severity. I have completely missed that on QAing bug 14757, I focused on the notices code and did not see this part!
Created attachment 53657 [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
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.
*** Bug 16985 has been marked as a duplicate of this bug. ***
Upping severity - appears to have bad side effects.
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
(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 forgot one call: circ/circulation.pl modifications => Koha::Patron::Modifications->GetModifications({ borrowernumber => $borrowernumber }), And two typos :) requrest applys
Created attachment 54027 [details] [review] Bug 16960: Update 1 missing occurrence of GetModifications
(In reply to Kyle M Hall from comment #7) > Created attachment 53920 [details] [review] [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.
Created attachment 54079 [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 Signed-off-by: Bob Birchall <bob@calyx.net.au>
Created attachment 54080 [details] [review] Bug 16960: Update 1 missing occurrence of GetModifications Signed-off-by: Bob Birchall <bob@calyx.net.au>
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.
Created attachment 54157 [details] [review] Bug 16960: Fix error on validating the registration
Created attachment 54461 [details] [review] 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>
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
Created attachment 54628 [details] [review] 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
Created attachment 54665 [details] [review] [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>
Created attachment 54666 [details] [review] [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>
Created attachment 54667 [details] [review] [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>
Created attachment 54668 [details] [review] [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>
Pushed to master for 16.11, thanks for the followups Jonathan!