In OPAC's self registration, if PatronSelfRegistrationVerifyByEmail is set to Require, OPAC dies with the following error DBD::mysql::st execute failed: Field 'changed_fields' doesn't have a default value [for Statement "INSERT INTO `borrower_modifications` ( `branchcode`, `categorycode`, `date_renewed`, `dateenrolled`, `dateexpiry`, `dateofbirth`, `debarred`, `email`, `firstname`, `gdpr_proc_consent`, `password`, `surname`, `timestamp`, `verification_token`) VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, current_timestamp, ? )" with ParamValues: 0="CPL", 1="S", 2=undef, 3=undef, 4=undef, 5=undef, 6=undef, 7="prueba2@algo.com", 8="algo", 9=undef, 10='E<rcyc6h<p', 11="prueba2", 12='aa165f462c22f414a2cd1081bace0921'] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836. Seems it's because changed_fields doesn't have a default value, and cannot be null.
Created attachment 102600 [details] [review] Bug 25086: set changed_fields column if not defined This patch sets changed_fields column to '' if not defined To test: 1. Do not apply this patch 2. Set PatronSelfRegistrationVerifyByEmail preference to require 3. Fill self registration form in opac and submit CHECK => OPAC dies with a nasty message, that says Field 'changed_fields' doesn't have a default value 4. Apply this patch and restart_all 5. Repeat step 3 SUCCESS => Registry process continues as expected 6. Sign off Sponsored-by: Orex Digital
Agustin, at first glance I'd say that the correct fix is to make the column nullable at DB level.
Created attachment 102893 [details] [review] Bug 25086: set changed_fields column if not defined This patch sets changed_fields column to '' if not defined To test: 1. Do not apply this patch 2. Set PatronSelfRegistrationVerifyByEmail preference to require 3. Fill self registration form in opac and submit CHECK => OPAC dies with a nasty message, that says Field 'changed_fields' doesn't have a default value 4. Apply this patch and restart_all 5. Repeat step 3 SUCCESS => Registry process continues as expected 6. Sign off Sponsored-by: Orex Digital Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Patch signed off.
However, the message of Joubu seems to mean that it doesn't pass QA.
Created attachment 103024 [details] [review] Bug 25086: set changed_fields column if not defined This patch sets changed_fields column to '' if not defined To test: 1. Do not apply this patch 2. Set PatronSelfRegistrationVerifyByEmail preference to require 3. Fill self registration form in opac and submit CHECK => OPAC dies with a nasty message, that says Field 'changed_fields' doesn't have a default value 4. Apply this patch and restart_all 5. Repeat step 3 SUCCESS => Registry process continues as expected 6. Sign off Sponsored-by: Orex Digital Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 103025 [details] [review] Bug 25086: (follow-up) Set changed_fields column as nullable This patch reverts previous patch (left there because it was signed off), and sets changed_fields column as nullable. Attend only this test plan and not previous patch. To test: 1. Do not apply any patch 2. Set PatronSelfRegistrationVerifyByEmail preference to require 3. Fill self registration form in opac and submit CHECK => OPAC dies with a nasty message, that says Field 'changed_fields' doesn't have a default value 4. Apply this patch and updatedatabase 5. Repeat step 3 SUCCESS => Registry process continues as expected 6. Sign off Sponsored-by: Orex Digital
Created attachment 103038 [details] [review] Bug 25086: (follow-up) Set changed_fields column as nullable This patch reverts previous patch (left there because it was signed off), and sets changed_fields column as nullable. Attend only this test plan and not previous patch. To test: 1. Do not apply any patch 2. Set PatronSelfRegistrationVerifyByEmail preference to require 3. Fill self registration form in opac and submit CHECK => OPAC dies with a nasty message, that says Field 'changed_fields' doesn't have a default value 4. Apply this patch and updatedatabase 5. Repeat step 3 SUCCESS => Registry process continues as expected 6. Sign off Sponsored-by: Orex Digital Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
It works! [U+1F44C]
Something is wrong here error: sha1 information is lacking or useless (installer/data/mysql/atomicupdate/bug_25086.perl). error: could not build fake ancestor Patch failed at 0001 Bug 25086: (follow-up) Set changed_fields column as nullable
Created attachment 103093 [details] [review] Bug 25086: set changed_fields column if not defined This patch sets changed_fields column to '' if not defined To test: 1. Do not apply this patch 2. Set PatronSelfRegistrationVerifyByEmail preference to require 3. Fill self registration form in opac and submit CHECK => OPAC dies with a nasty message, that says Field 'changed_fields' doesn't have a default value 4. Apply this patch and restart_all 5. Repeat step 3 SUCCESS => Registry process continues as expected 6. Sign off Sponsored-by: Orex Digital Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 103094 [details] [review] Bug 25086: (follow-up) Set changed_fields column as nullable This patch reverts previous patch (left there because it was signed off), and sets changed_fields column as nullable. Attend only this test plan and not previous patch. To test: 1. Do not apply any patch 2. Set PatronSelfRegistrationVerifyByEmail preference to require 3. Fill self registration form in opac and submit CHECK => OPAC dies with a nasty message, that says Field 'changed_fields' doesn't have a default value 4. Apply this patch and updatedatabase 5. Repeat step 3 SUCCESS => Registry process continues as expected 6. Sign off Sponsored-by: Orex Digital Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
/me forgot to attach the second patch. Fixed, sorry for hassle. SelfRegistration retested again just in case. Nothing suspicious found.
Created attachment 103095 [details] [review] Bug 25086: set changed_fields column if not defined This patch sets changed_fields column to '' if not defined To test: 1. Do not apply this patch 2. Set PatronSelfRegistrationVerifyByEmail preference to require 3. Fill self registration form in opac and submit CHECK => OPAC dies with a nasty message, that says Field 'changed_fields' doesn't have a default value 4. Apply this patch and restart_all 5. Repeat step 3 SUCCESS => Registry process continues as expected 6. Sign off Sponsored-by: Orex Digital Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 103096 [details] [review] Bug 25086: (follow-up) Set changed_fields column as nullable This patch reverts previous patch (left there because it was signed off), and sets changed_fields column as nullable. Attend only this test plan and not previous patch. To test: 1. Do not apply any patch 2. Set PatronSelfRegistrationVerifyByEmail preference to require 3. Fill self registration form in opac and submit CHECK => OPAC dies with a nasty message, that says Field 'changed_fields' doesn't have a default value 4. Apply this patch and updatedatabase 5. Repeat step 3 SUCCESS => Registry process continues as expected 6. Sign off Sponsored-by: Orex Digital
(In reply to Jonathan Druart from comment #10) > Something is wrong here > > error: sha1 information is lacking or useless > (installer/data/mysql/atomicupdate/bug_25086.perl). > error: could not build fake ancestor > Patch failed at 0001 Bug 25086: (follow-up) Set changed_fields column as > nullable You were right.. somehow I messed up my rebase and replaced original signed off commit with another. I just put things in order. Please try again. Thanks
Wait I'm sure I was the one who messed up. Because my command history shows that I only attached the last patch when we must reattach all patches each time. (IIUC) And because to do the signoff I was able to apply the 2 patches without issue. It was only after my signoff that Joubu wasn't able to apply the 2 patches. I'm rechecking what you just submitted Agustín.
Created attachment 103097 [details] [review] Bug 25086: set changed_fields column if not defined This patch sets changed_fields column to '' if not defined To test: 1. Do not apply this patch 2. Set PatronSelfRegistrationVerifyByEmail preference to require 3. Fill self registration form in opac and submit CHECK => OPAC dies with a nasty message, that says Field 'changed_fields' doesn't have a default value 4. Apply this patch and restart_all 5. Repeat step 3 SUCCESS => Registry process continues as expected 6. Sign off Sponsored-by: Orex Digital Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 103098 [details] [review] Bug 25086: (follow-up) Set changed_fields column as nullable This patch reverts previous patch (left there because it was signed off), and sets changed_fields column as nullable. Attend only this test plan and not previous patch. To test: 1. Do not apply any patch 2. Set PatronSelfRegistrationVerifyByEmail preference to require 3. Fill self registration form in opac and submit CHECK => OPAC dies with a nasty message, that says Field 'changed_fields' doesn't have a default value 4. Apply this patch and updatedatabase 5. Repeat step 3 SUCCESS => Registry process continues as expected 6. Sign off Sponsored-by: Orex Digital Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Working as expected. Agustín: I don't think that two patches are needed. From what I understand, if you change your strategy, you don't need a commit revert the V1. Nothing is released. So just discard or amend your commit and reattach it to the ticket. In the current state, I think you can just squash the two commits. And keep the commit message of the 2nd commit, without the part saying that it reverts the 1st one. That's my current understanding.
Created attachment 103115 [details] [review] Bug 25086: Set changed_fields column as nullable This patch changed_fields column as nullable. To test: 1. Do not apply any patch 2. Set PatronSelfRegistrationVerifyByEmail preference to require 3. Fill self registration form in opac and submit CHECK => OPAC dies with a nasty message, that says Field 'changed_fields' doesn't have a default value 4. Apply this patch and updatedatabase 5. Repeat step 3 SUCCESS => Registry process continues as expected 6. Sign off Sponsored-by: Orex Digital Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to Victor Grousset/tuxayo from comment #20) > Working as expected. > > Agustín: I don't think that two patches are needed. From what I understand, > if you change your strategy, you don't need a commit revert the V1. Nothing > is released. So just discard or amend your commit and reattach it to the > ticket. > > In the current state, I think you can just squash the two commits. And keep > the commit message of the 2nd commit, without the part saying that it > reverts the 1st one. > > That's my current understanding. Agreed
Nice work everyone! Pushed to master for 20.05
backported to 19.11.x for 19.11.06