They are VARCHAR(7) but they must be enum('yes', 'no', 'inherit')
Created attachment 183918 [details] [review] Bug 40337: Define checkprevcheckout as ENUM On bug 40178 and bug 40179 we defined checkprevcheckout attributes as ENUM at the REST API level, so the specs prevent us from inserting values that are no yes, no, inherit in DB. However the DB schema didn't have that constraint. This has been caught by some tests that were failing: eg. t/db_dependent/selenium/patrons_search.t with "Not in enum list: yes, no, inherit." Test plan: Apply this patch, run `updatedatabase`, `dbic` and `restart_all` `prove t/db_dependent/selenium/patrons_search.t` should now pass.
Created attachment 183919 [details] [review] Bug 40337: DBIC schema changes
Created attachment 183946 [details] [review] Bug 40337: DBIC schema changes Signed-off-by: Eric Garcia <cubingguy714@gmail.com>
Created attachment 183947 [details] [review] Bug 40337: Define checkprevcheckout as ENUM On bug 40178 and bug 40179 we defined checkprevcheckout attributes as ENUM at the REST API level, so the specs prevent us from inserting values that are no yes, no, inherit in DB. However the DB schema didn't have that constraint. This has been caught by some tests that were failing: eg. t/db_dependent/selenium/patrons_search.t with "Not in enum list: yes, no, inherit." Test plan: Apply this patch, run `updatedatabase`, `dbic` and `restart_all` `prove t/db_dependent/selenium/patrons_search.t` should now pass. Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 183948 [details] [review] Bug 40337: DBIC schema changes Signed-off-by: Eric Garcia <cubingguy714@gmail.com> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Nice work everyone! Pushed to main for 25.11
Created attachment 183969 [details] [review] Bug 40337: Handle enum in Koha::Patron->anonymize We need to reset to the default value. We might have a problem (later) if it's NOT NULL and no default is set, which is not handled by this patch. But the situation does not seem to exist yet anyway.
Created attachment 183970 [details] [review] Bug 40337: Do not set checkprevcheckout to an empty string when importing patrons We must remove it so that it will be set as the default value defined at the DBMS level
Nice work everyone! Pushed to 25.05.x
follow-ups pushed to main
+-------------------+ | checkprevcheckout | +-------------------+ | inherit | | bpe8 | | MIbcjV | | ngYcotJ | | z85xcQ_ | | I523 | | zqw1P | | koirKi | | ErwNnJ | +-------------------+ Stuff from tests? Causing: ERROR: {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Data truncated for column 'checkprevcheckout' at row 54 at /usr/share/koha/C4/Installer.pm line 825
(In reply to Marcel de Rooy from comment #11) > +-------------------+ > | checkprevcheckout | > +-------------------+ > | inherit | > | bpe8 | > | MIbcjV | > | ngYcotJ | > | z85xcQ_ | > | I523 | > | zqw1P | > | koirKi | > | ErwNnJ | > +-------------------+ > > Stuff from tests? Causing: > ERROR: {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Data > truncated for column 'checkprevcheckout' at row 54 at > /usr/share/koha/C4/Installer.pm line 825 This happens if you have run some tests before the DB changes. Then the REST API will explode because it will send strings that do not match the ENUM values.
Created attachment 184396 [details] [review] Bug 40337: (QA follow-up) Make database update idempotent Running this update twice fails before this patch with: ERROR: {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Data truncated for column 'checkprevcheckout' at row 3 at /kohadevbox/koha/C4/Installer.pm line 825
Hold off.. still failing for me
Created attachment 184397 [details] [review] Bug 40337: (QA follow-up) Make database update idempotent Running this update twice fails before this patch with: ERROR: {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Data truncated for column 'checkprevcheckout' at row 3 at /kohadevbox/koha/C4/Installer.pm line 825 Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
(In reply to Martin Renvoize (ashimema) from comment #15) > Created attachment 184397 [details] [review] [review] > Bug 40337: (QA follow-up) Make database update idempotent > > Running this update twice fails before this patch with: > > ERROR: {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Data truncated > for column 'checkprevcheckout' at row 3 at /kohadevbox/koha/C4/Installer.pm > line 825 > > Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> I cannot test right now but this patch does not feel correct. Why couldn't we rum the alter table statements several times?
Created attachment 184425 [details] [review] Bug 40337: Remove test of datatype Keep the UPDATE to adjust bad data however.
Created attachment 184856 [details] [review] Bug 40337: Fix DB comparison 00:40:25 koha-1 | < `checkprevcheckout` enum('yes', 'no', 'inherit') NOT NULL DEFAULT 'inherit' COMMENT 'produce a warning for this patron if this item has previously been checked out to this patron if ''yes'', not if ''no'', defer to category setting if ''inherit''.', 00:40:25 koha-1 | --- 00:40:25 koha-1 | > `checkprevcheckout` enum('yes','no','inherit') NOT NULL DEFAULT 'inherit' COMMENT 'produce a warning for this patron if this item has previously been checked out to this patron if ''yes'', not if ''no'', defer to category setting if ''inherit''.', 00:40:25 koha-1 | ERROR - DB structures are not identical
(In reply to Jonathan Druart from comment #18) > Created attachment 184856 [details] [review] [review] > Bug 40337: Fix DB comparison > > 00:40:25 koha-1 | < `checkprevcheckout` enum('yes', 'no', 'inherit') > NOT NULL DEFAULT 'inherit' COMMENT 'produce a warning for this patron if > this item has previously been checked out to this patron if ''yes'', not if > ''no'', defer to category setting if ''inherit''.', > 00:40:25 koha-1 | --- > 00:40:25 koha-1 | > `checkprevcheckout` enum('yes','no','inherit') > NOT NULL DEFAULT 'inherit' COMMENT 'produce a warning for this patron if > this item has previously been checked out to this patron if ''yes'', not if > ''no'', defer to category setting if ''inherit''.', > 00:40:25 koha-1 | ERROR - DB structures are not identical Moved to bug 40548.
I'm not familiar with this bug, but it looks like bug 40528 and bug 40558 are opened because of it?
Ok I think I'm starting to see... I'm looking at installer/data/mysql/db_revs/250501001.pl on v25.05.02-1 and installer/data/mysql/db_revs/250600003.pl on main, and they don't include the Martin/Jonathan changes to installer/data/mysql/db_revs/250600003.pl It looks like they're missing the following SQL: UPDATE categories SET checkprevcheckout = 'inherit' WHERE checkprevcheckout NOT IN ('yes','no','inherit'); See the patches: Bug 40337: (QA follow-up) Make database update idempotent Bug 40337: Remove test of datatype
Oh I see... Martin and Jonathan posted about it after the original patches were pushed... I don't understand how this was supposed to be managed...
Was missing these commits in main: Bug 40337: (QA follow-up) Make database update idempotent Bug 40337: Remove test of datatype I have pushed them. The following isn't needed because it was pushed in Bug 40548: Bug 40337: Fix DB comparison
Thanks for that, Lucas. Although after getting more info from users, it looks like that might not be the problem causing bug 40528. They claim that checkprevcheckout is "inherit" but still causing errors. I suppose we might see what happens as more people upgrade.