Created attachment 128113 [details] [review] Bug 29605: Add missing constraints from bug 15067 This patch fixes a missed update to kohastructure.sql. To test: 1. Checkout prior to bug 15067 (ab270c026be06f08745a14d1aea992877d20f812) would work. 2. Run: $ reset_all 3. Checkout v21.11.00 4. Run: $ updatedatabase $ koha-mysql kohadev > SHOW CREATE TABLE language_subtag_registry; => SUCCESS: There's UNIQUE KEY uniq_lang (subtag, type) 5. Run: > SHOW CREATE TABLE language_descriptions; => SUCCESS: There's UNIQUE KEY uniq_desc (subtag, type, lang) 6. Run: > SHOW CREATE TABLE language_rfc4646_to_iso639; => SUCCESS: There's UNIQUE KEY uniq_code (rfc4646_subtag, iso639_2_code) 7. Replicate a fresh install. Run: $ reset_all 8. Repeat 4, 5 and 6 => FAIL: The UNIQUE constraints are not there! 9. Apply this patch 10. Run: $ reset_all 11. Repeat 8 => SUCCESS: The UNIQUE constraints are there! 12. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 128114 [details] [review] Bug 29605: (follow-up) Add language_script_mapping primary key Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
How did we miss that?! It also means that there is a RM step that is missing when releasing
Oops, there are way more things that diverge!
Created attachment 128152 [details] [review] Bug 29605: Missing FK on tmp_holdsqueue.borrowernumber (tmp_holdsqueue_ibfk_3) Bad copy paste on the foreign_key_exists condition
Created attachment 128153 [details] [review] Bug 29605: account_offsets.type is not null
Created attachment 128154 [details] [review] Bug 29605: additional_contents.code Added 100 on 210600016, but 20 in kohastructure
Created attachment 128155 [details] [review] Bug 29605: additional_contents.lang Up to 50 on bug 23797 but bug 24387 switched it back to 25 (bad resolution conflict)
Created attachment 128156 [details] [review] Bug 29605: search_marc_map.marc_type NOT NULL was missing when MODIFY
Created attachment 128157 [details] [review] Bug 29605: language_script_mapping.language_subtag cannot be null It's a PK now
Created attachment 128158 [details] [review] Bug 29605: branchtransfers.cancellation_reason reason and cancellation_reason was inversed, and the enum values were lower cases
Test plan: % git checkout -b bug_29605 origin/master % yes|git bz apply 29605 % git checkout v21.05.00 % reset_all % git checkout bug_29605 % updatedatabase # We are going to update the 21.05.00 DB with all the 21.11 dbrevs # Generate the DBIC schema file using the current DB % perl misc/devel/update_dbix_class_files.pl --koha-conf $KOHA_CONF % git commit -a -m"test diff" # commit the changes to help understand % dbic # generate the schema file using kohastructure.sql % git diff => NO relevant changes should be seen in this diff, only comments and date/md5sum must differ.
(In reply to Jonathan Druart from comment #3) > How did we miss that?! > It also means that there is a RM step that is missing when releasing https://tree.taiga.io/project/koha-ils/task/117
(In reply to Jonathan Druart from comment #13) > (In reply to Jonathan Druart from comment #3) > > How did we miss that?! > > It also means that there is a RM step that is missing when releasing > > https://tree.taiga.io/project/koha-ils/task/117 Not sure where is the best place to comment, but I added a suggestion on taiga.
Created attachment 128239 [details] Bug 29605 - Error messages after reset_all I had a go at testing this, but I get "BEGIN failed--compilation aborted at...." error messages after the reset_all command. See attachment for the command line log.
(In reply to David Nind from comment #15) > Created attachment 128239 [details] > Bug 29605 - Error messages after reset_all > > I had a go at testing this, but I get "BEGIN failed--compilation aborted > at...." error messages after the reset_all command. > > See attachment for the command line log. Ah 21.05.x does not have same perl deps as 21.11. Try installing package libemail-valid-perl.
(In reply to Fridolin Somers from comment #16) > Ah 21.05.x does not have same perl deps as 21.11. > Try installing package libemail-valid-perl. Thanks Fridolin - that did the trick!
Created attachment 128313 [details] Bug 29605 - Result from git diff I followed Jonathan's test plan in comment 12 - I've attached the diff, so I hope that is what is expected.
Created attachment 128314 [details] Bug 29605 - Error when accessing staff interface and OPAC I haven't signed off because when I try to go into the staff interface or the OPAC I get errors (after flush_memcached and restart_all) - see the attached error trace.
(In reply to David Nind from comment #19) > Created attachment 128314 [details] > Bug 29605 - Error when accessing staff interface and OPAC > > I haven't signed off because when I try to go into the staff interface or > the OPAC I get errors (after flush_memcached and restart_all) - see the > attached error trace. apt install libcgi-session-serialize-yaml-perl
(In reply to Nick Clemens from comment #20) > apt install libcgi-session-serialize-yaml-perl Thanks Nick. I have installed that but get exactly the same error trace, so I'm not sure what might be going wrong with my KTD setup. if this is not an issue (it wasn't part of the test plan) for solving the actual bug as opposed to sorting out my KTD setup I would be happy to sign-off. On the other-hand, I don't want there to be additional problems....
I was slow to the game here.. adding to the ndiscussion on tiaga, I've used a few systems that take the approach Julian suggests and never had any real performance concerns. I quite like the idea of not having to maintain the structure twice.
(In reply to Martin Renvoize from comment #22) > I was slow to the game here.. adding to the ndiscussion on tiaga, I've used > a few systems that take the approach Julian suggests and never had any real > performance concerns. I quite like the idea of not having to maintain the > structure twice. Wrong bug?
After the upgrade: - language_script_mapping.language_subtag is_nullable 0 vs 1 - __PACKAGE__->add_unique_constraint("uniq_code", ["rfc4646_subtag", "iso639_2_code"]); - AdditionalContent.pm => field sizes differences for 'category' and 'content'
(In reply to David Nind from comment #19) > Created attachment 128314 [details] > Bug 29605 - Error when accessing staff interface and OPAC > > I haven't signed off because when I try to go into the staff interface or > the OPAC I get errors (after flush_memcached and restart_all) - see the > attached error trace. This is because reset_all (on the 21.05 branch) called cp_debian_files.pl which copied 'old' koha-* scripts and tools which don't acknowledge the location of CGI/Session/Serialize/yamlxs.pm To solve it, you should run: $ cd /kohadevbox/misc4dev $ perl cp_debian_files.pl --koha_dir /kohadevbox/koha/ --gitify_dir /kohadevbox/gitify/ --instance kohadev Best regards
Ok. This is ready to go IMHO. The only missing bit is the updated DBIC files, which make sense and should be done by the RM on pushing, so we are good. New revisited test plan: % git checkout -b bug_29605 origin/master % yes|git bz apply 29605 % git checkout v21.05.00 % reset_all % git checkout bug_29605 % updatedatabase # We are going to update the 21.05.00 DB with all the 21.11 dbrevs % perl cp_debian_files.pl --koha_dir /kohadevbox/koha/ --gitify_dir /kohadevbox/gitify/ --instance kohadev # Generate the DBIC schema file using the current DB % perl misc/devel/update_dbix_class_files.pl --koha-conf $KOHA_CONF % git commit -a -m"test diff" # commit the changes to help understand % dbic # generate the schema file using kohastructure.sql % git diff => NO relevant changes should be seen in this diff, only comments and date/md5sum must differ.
Created attachment 128437 [details] [review] Bug 29605: Add missing constraints from bug 15067 This patch fixes a missed update to kohastructure.sql. To test: 1. Checkout prior to bug 15067 (ab270c026be06f08745a14d1aea992877d20f812) would work. 2. Run: $ reset_all 3. Checkout v21.11.00 4. Run: $ updatedatabase $ koha-mysql kohadev > SHOW CREATE TABLE language_subtag_registry; => SUCCESS: There's UNIQUE KEY uniq_lang (subtag, type) 5. Run: > SHOW CREATE TABLE language_descriptions; => SUCCESS: There's UNIQUE KEY uniq_desc (subtag, type, lang) 6. Run: > SHOW CREATE TABLE language_rfc4646_to_iso639; => SUCCESS: There's UNIQUE KEY uniq_code (rfc4646_subtag, iso639_2_code) 7. Replicate a fresh install. Run: $ reset_all 8. Repeat 4, 5 and 6 => FAIL: The UNIQUE constraints are not there! 9. Apply this patch 10. Run: $ reset_all 11. Repeat 8 => SUCCESS: The UNIQUE constraints are there! 12. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 128438 [details] [review] Bug 29605: (follow-up) Add language_script_mapping primary key Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 128439 [details] [review] Bug 29605: Missing FK on tmp_holdsqueue.borrowernumber (tmp_holdsqueue_ibfk_3) Bad copy paste on the foreign_key_exists condition Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 128440 [details] [review] Bug 29605: account_offsets.type is not null Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 128441 [details] [review] Bug 29605: additional_contents.code Added 100 on 210600016, but 20 in kohastructure Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 128442 [details] [review] Bug 29605: additional_contents.lang Up to 50 on bug 23797 but bug 24387 switched it back to 25 (bad resolution conflict) Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 128443 [details] [review] Bug 29605: search_marc_map.marc_type NOT NULL was missing when MODIFY Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 128444 [details] [review] Bug 29605: language_script_mapping.language_subtag cannot be null It's a PK now Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 128445 [details] [review] Bug 29605: branchtransfers.cancellation_reason reason and cancellation_reason was inversed, and the enum values were lower cases Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 128483 [details] Bug 29605 - Error when accessing staff interface and OPAC - 2021-12-14 Still getting an error trace when accessing staff interface and OPAC after flush_memcached and restart_all (see attached). Testing notes (koha-testing-docker): 1. apt-get install libcgi-session-serialize-yaml-perl (comment 16) 2. apt-get install libemail-valid-perl (comment 20) 3. Use updated test plan in comment 26: . for perl cp_debian_files.pl... change to: perl /kohadevbox/misc4dev/cp_debian_files.pl --koha_dir /kohadevbox/koha/ --gitify_dir /kohadevbox/gitify/ --instance kohadev 5. As per comment 27 check SQL for steps 4, 5 and 6.
(In reply to David Nind from comment #36) > Created attachment 128483 [details] > Bug 29605 - Error when accessing staff interface and OPAC - 2021-12-14 > > Still getting an error trace when accessing staff interface and OPAC after > flush_memcached and restart_all (see attached). > > Testing notes (koha-testing-docker): > > 1. apt-get install libcgi-session-serialize-yaml-perl (comment 16) > > 2. apt-get install libemail-valid-perl (comment 20) > > 3. Use updated test plan in comment 26: > . for perl cp_debian_files.pl... change to: > perl /kohadevbox/misc4dev/cp_debian_files.pl --koha_dir > /kohadevbox/koha/ --gitify_dir /kohadevbox/gitify/ --instance kohadev And a gentle koha-plack --restart kohadev?
Can anyone QA this one?
Created attachment 129153 [details] [review] Bug 29605: Add missing constraints from bug 15067 This patch fixes a missed update to kohastructure.sql. To test: 1. Checkout prior to bug 15067 (ab270c026be06f08745a14d1aea992877d20f812) would work. 2. Run: $ reset_all 3. Checkout v21.11.00 4. Run: $ updatedatabase $ koha-mysql kohadev > SHOW CREATE TABLE language_subtag_registry; => SUCCESS: There's UNIQUE KEY uniq_lang (subtag, type) 5. Run: > SHOW CREATE TABLE language_descriptions; => SUCCESS: There's UNIQUE KEY uniq_desc (subtag, type, lang) 6. Run: > SHOW CREATE TABLE language_rfc4646_to_iso639; => SUCCESS: There's UNIQUE KEY uniq_code (rfc4646_subtag, iso639_2_code) 7. Replicate a fresh install. Run: $ reset_all 8. Repeat 4, 5 and 6 => FAIL: The UNIQUE constraints are not there! 9. Apply this patch 10. Run: $ reset_all 11. Repeat 8 => SUCCESS: The UNIQUE constraints are there! 12. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 129154 [details] [review] Bug 29605: (follow-up) Add language_script_mapping primary key Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 129155 [details] [review] Bug 29605: Missing FK on tmp_holdsqueue.borrowernumber (tmp_holdsqueue_ibfk_3) Bad copy paste on the foreign_key_exists condition Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 129156 [details] [review] Bug 29605: account_offsets.type is not null Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 129157 [details] [review] Bug 29605: additional_contents.code Added 100 on 210600016, but 20 in kohastructure Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 129158 [details] [review] Bug 29605: additional_contents.lang Up to 50 on bug 23797 but bug 24387 switched it back to 25 (bad resolution conflict) Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 129159 [details] [review] Bug 29605: search_marc_map.marc_type NOT NULL was missing when MODIFY Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 129160 [details] [review] Bug 29605: language_script_mapping.language_subtag cannot be null It's a PK now Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 129161 [details] [review] Bug 29605: branchtransfers.cancellation_reason reason and cancellation_reason was inversed, and the enum values were lower cases Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Bug 26947 - that might help for the future
Pushed to master for 22.05, thanks to everybody involved [U+1F984]
This needs to be backported.
(In reply to Jonathan Druart from comment #50) > This needs to be backported. Ah sure, sorry I misunderstood the meaning of this keyword.
Pushed to 21.11.x for 21.11.03
Missing dependency, not backported to 21.05
We're out of sync again already We need to refine the process here.. didn't we decide we should be only submitting atomicupdates and using koha-dump to generate the kohastructure file from it?
(In reply to Martin Renvoize from comment #54) > We're out of sync again already > > We need to refine the process here.. didn't we decide we should be only > submitting atomicupdates and using koha-dump to generate the kohastructure > file from it? We discussed that already. The problem is that most of the time the error is coming from the atomic update, the kohastructure is correct...
(In reply to Jonathan Druart from comment #55) > We discussed that already. > The problem is that most of the time the error is coming from the atomic > update, the kohastructure is correct... Yeah it's easy to test a new kohastructure.sql or an atomic update against the last kohastructure.sql, but systems in the wild don't always play nice with atomic updates. Bug 30951 also shows a problem with /usr/share/koha/intranet/cgi-bin/installer/data/mysql/db_revs/211102003.pl where it doesn't take into account existing duplicate keys before adding a primary key. I suppose Bug 30951 might need to update that database migration so that it does some kind of deduping... or to throw a fatal error that includes a line number to make it easier to troubleshoot...
For the adding a new constraint or key case we should probably be catching that at QA stage.. we're only human, and I'm likely a culprit for missing exactly that sort of issue. [U+1F62C]