Bug 6906 introduced the new CheckPrevCheckout syspref. The description says: "New feature: provide granular means to configure warnings about items that have been issued to a particular borrower before, according to their checkout history." I assume "items" here is shorthand for "items connected to the same bibliographic record"? Just checking on the item level would not be all that useful, if there are more than one copy of e.g. a book. On the other hand, this does not work so well for periodicals/journals where individual issues are entered as separate items. If a patron borrows the last three issues of a journal, the second one will give a warning that the patron has previously borrowed this "title". Could it be possible to have a setting on the itemtypes to choose one of these two? - Check if a patron has previously borrowed individual items (suitable for periodicals/journals) - Check if a patron has previously borrowed any items connected to a bib record (suitable for books, DVDs etc)
Is this still valid after bug 16284? The need is specifically for serials, and that is covered in bug 16284. Would a library want to control CheckPrevCheckout per itemtype for other types of materials?
This granularity of CheckPrevCheckout by itemtype would still be useful functionality beyond serials - many libraries have unique things that they circulate that they would like to put a limitation on checkout within a timeframe in the interest of fairness of access - like laptops, hotspots, or local passes. Libraries may not want to turn on CheckPrevCheckout for a whole collection when they really only need it for certain subsets of items.
Created attachment 156974 [details] [review] Bug 20644: Add checkprevcheckout on item types I have added the "Check for previous checkouts" option to item types. This option is visible only when the system preference "CheckPrevCheckout" is set to either: - "Unless overridden by patron category or by item type, do" or - "Unless overridden by patron category or by item type, do not." This option takes precedence over patron and patron category preferences. TEST PLAN 1) Apply the patch. 2) Perform an atomic update (installer/data/mysql/updatedatabase.pl) 3) Set the "CheckPrevCheckout" system preference to "Unless overridden by patron category or by item type, do". 4) Go to "Koha administration > Item types > + New item type". 5) Fill in the necessary fields; the "Check for previous checkouts" field should be set to "Inherit from system preferences". 6) Click the "Save changes" button. 7) Create an item with the type you have created. 8) Check out this item for a patron, then check it in. 9) Check out the same item for the same patron again. 10) The message "Patron has previously checked out this title: <Title>. Check out anyway?" should appear. 11) Go to "Koha administration > Item types". 12) Edit the created item type. 13) Change the value of the "Check for previous checkouts" field to "No and try to override system preferences". 14) Click the "Save changes" button. 15) Check out the same item for the same patron. 16) The message should not appear. 17) Run the tests "Borrower_PrevCheckout.t" (prove t/db_dependent/Patron/Borrower_PrevCheckout.t) and "ItemTypes.t" (prove t/db_dependent/Koha/ItemTypes.t). 18) Verify that all tests pass successfully.
Created attachment 156977 [details] [review] Bug 20644: Add checkprevcheckout on item types I have added the "Check for previous checkouts" option to item types. This option is visible only when the system preference "CheckPrevCheckout" is set to either : - "Unless overridden by patron category or by item type, do" or - "Unless overridden by patron category or by item type, do not." This option takes precedence over patron and patron category preferences. TEST PLAN 1) Apply the patch. 2) Perform an atomic update (installer/data/mysql/updatedatabase.pl) 3) Set the "CheckPrevCheckout" system preference to "Unless overridden by patron category or by item type, do". 4) Go to "Koha administration > Item types > + New item type". 5) Fill in the necessary fields; the "Check for previous checkouts" field should be set to "Inherit from system preferences". 6) Click the "Save changes" button. 7) Create an item with the type you have created. 8) Check out this item for a patron, then check it in. 9) Check out the same item for the same patron again. 10) The message "Patron has previously checked out this title: <Title>. Check out anyway?" should appear. 11) Go to "Koha administration > Item types". 12) Edit the created item type. 13) Change the value of the "Check for previous checkouts" field to "No and try to override system preferences". 14) Click the "Save changes" button. 15) Check out the same item for the same patron. 16) The message should not appear. 17) Run the tests "Borrower_PrevCheckout.t" (prove t/db_dependent/Patron/Borrower_PrevCheckout.t) and "ItemTypes.t" (prove t/db_dependent/Koha/ItemTypes.t). 18) Verify that all tests pass successfully.
Created attachment 163627 [details] [review] Bug 20644: Add checkprevcheckout on item types I have added the "Check for previous checkouts" option to item types. This option is visible only when the system preference "CheckPrevCheckout" is set to either : - "Unless overridden by patron category or by item type, do" or - "Unless overridden by patron category or by item type, do not." This option takes precedence over patron and patron category preferences. TEST PLAN 1) Apply the patch. 2) Perform an atomic update (installer/data/mysql/updatedatabase.pl) 3) Set the "CheckPrevCheckout" system preference to "Unless overridden by patron category or by item type, do". 4) Go to "Koha administration > Item types > + New item type". 5) Fill in the necessary fields; the "Check for previous checkouts" field should be set to "Inherit from system preferences". 6) Click the "Save changes" button. 7) Create an item with the type you have created. 8) Check out this item for a patron, then check it in. 9) Check out the same item for the same patron again. 10) The message "Patron has previously checked out this title: <Title>. Check out anyway?" should appear. 11) Go to "Koha administration > Item types". 12) Edit the created item type. 13) Change the value of the "Check for previous checkouts" field to "No and try to override system preferences". 14) Click the "Save changes" button. 15) Check out the same item for the same patron. 16) The message should not appear. 17) Run the tests "Borrower_PrevCheckout.t" (prove t/db_dependent/Patron/Borrower_PrevCheckout.t) and "ItemTypes.t" (prove t/db_dependent/Koha/ItemTypes.t). 18) Verify that all tests pass successfully. Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Must fix: In wants_check_for_previous_checkout we should pass the item _object rather than unblessed. This would let us use the effective_itemtype routine to get the itemtype (in the case where Record Level itemtypes might be in use) The schema changes should be in their own patch to allow the RM to regenerate In the tests it says new types are generated as NULL but it should say 'inherit' QA tool complains about tidiness Should fix: Why does the item type choice say :try and override"? Looks like the item type setting trumps all except hard syspref, and it won't display if hard syspref set? Could fix: You can use testbuilder to generate the types for testing, there is no need to specify item type code, just check against what is generated. Would fix: Test could be a bit more verbose to be readable, the map is not as clear as a straightforward loop would be, but I like the laid out matrix explaining the results The update could test for existence of column and not run when exists, but the construction you use does work.
Created attachment 165585 [details] [review] Bug 20644: [DO NOT PUSH] DBIC Update Current status: Failed QA
Created attachment 165586 [details] [review] Bug 20644: Add checkprevcheckout on item types
Created attachment 165661 [details] [review] Bug 20644: [DO NOT PUSH] DBIC Update I have added the "Check for previous checkouts" option to item types. This option is visible only when the system preference "CheckPrevCheckout" is set to either : - "Unless overridden by patron category or by item type, do" or - "Unless overridden by patron category or by item type, do not." This option takes precedence over patron and patron category preferences. TEST PLAN 1) Apply the patch. 2) Perform an atomic update (installer/data/mysql/updatedatabase.pl) 3) Set the "CheckPrevCheckout" system preference to "Unless overridden by patron category or by item type, do". 4) Go to "Koha administration > Item types > + New item type". 5) Fill in the necessary fields; the "Check for previous checkouts" field should be set to "Inherit from system preferences". 6) Click the "Save changes" button. 7) Create an item with the type you have created. 8) Check out this item for a patron, then check it in. 9) Check out the same item for the same patron again. 10) The message "Patron has previously checked out this title: <Title>. Check out anyway?" should appear. 11) Go to "Koha administration > Item types". 12) Edit the created item type. 13) Change the value of the "Check for previous checkouts" field to "No and try to override system preferences". 14) Click the "Save changes" button. 15) Check out the same item for the same patron. 16) The message should not appear. 17) Run the tests "Borrower_PrevCheckout.t" (prove t/db_dependent/Patron/Borrower_PrevCheckout.t) and "ItemTypes.t" (prove t/db_dependent/Koha/ItemTypes.t). 18) Verify that all tests pass successfully.
Created attachment 165662 [details] [review] Bug 20644: Add checkprevcheckout on item types
I changed "item_unblessed" to "item_object". Put the shema changes in a different patch. Adjusted the tidiness. But for the point where in the tests it's marked NULL and not "inherit" I don't know what you're talking about, because for me it's already marked "inherit" as requested. For the "should fix" point, could you be more precise? I don't know which part you're talking about. The same goes for the "would fix" part. Could you be more specific about how to improve test comprehension? For the "could fix" part, I've tried to change using the builder, but with the builder you can't create an "ItemType" with the value "checkprevcheckout" at "undef" because this field can't be empty, so the builder throws an error. I tried this: my $itemType1 = $builder->build_object({ class => 'Koha::ItemTypes', value => { checkprevcheckout => undef, } }); and I get this error: "Null value for checkprevcheckout in Itemtype not allowed at /inlibro/git/koha-master-dev-matthias/t/lib/TestBuilder.pm line 430." Tell me if there is another solution with the builder or if we leave it as it is now? Finally, for the last point about the script that adds the column, there's already something that tests whether the column exists, isn't there? with : "ALTER TABLE itemtypes ADD IF NOT EXISTS checkprevcheckout" Does "ADD IF NOT EXISTS" do this?
(In reply to Matthias Le Gac from comment #12) > > But for the point where in the tests it's marked NULL and not "inherit" I > don't know what you're talking about, because for me it's already marked > "inherit" as requested. t/db_dependent/Koha/ItemTypes.t: 180 'Two item types with \'checkprevcheckout\' set to \'undef\' have been added' It's just the comment there > > For the "should fix" point, could you be more precise? I don't know which > part you're talking about. When editing an itemtype, it the system preference is 'soft' the options are: Yes and try to override system preference No and try to override system preference Inherit from system preference From what I understand it does not 'try' to override, it always does I think we should also display the system preference when set to 'Hard' otherwise this can be a somewhat hidden feature > The same goes for the "would fix" part. Could you be more specific about how > to improve test comprehension? For me, 4 nested 'map' calls is a bit obfuscated. Clearer would be something like: foreach my $syspref_setting ('hardno','hardyes','softno','softyes'){ foreach my $itemtype_setting( 'inherit','no','yes'){ ... The output from your tests is clear, the matrix in the comments is clear, if large, it's a matter of preference but not a blocker > > For the "could fix" part, I've tried to change using the builder, but with > the builder you can't create an "ItemType" with the value > "checkprevcheckout" at "undef" because this field can't be empty, so the > builder throws an error. > I tried this: > my $itemType1 = $builder->build_object({ > class => 'Koha::ItemTypes', > value => { > checkprevcheckout => undef, > } > }); > and I get this error: > "Null value for checkprevcheckout in Itemtype not allowed at > /inlibro/git/koha-master-dev-matthias/t/lib/TestBuilder.pm line 430." > Tell me if there is another solution with the builder or if we leave it as > it is now? t/db_dependent/Patron/Borrower_PrevCheckout.t 49 itemtype => 'YESIT', 59 itemtype => 'NOIT', 69 itemtype => 'INHERITIT' I just meant you don't need to specify the codes here, you use them later, but could save the generated codes as variables in the hash and reference them - this just ensures the tests won't have a conflict with existing data - rare, but slightly possible if tests die and rollback fails, etc. Again, not a blocker > Finally, for the last point about the script that adds the column, there's > already something that tests whether the column exists, isn't there? with : > "ALTER TABLE itemtypes ADD IF NOT EXISTS checkprevcheckout" > Does "ADD IF NOT EXISTS" do this? It does, but it is inconsistent with how we handle this in Koha: if( !column_exists( 'borrowers', 'middle_name' ) ) { I am not sure if your construction was blocked for being a MySQL-ism, but I on second glance it seems we should stick with the established pattern git grep column_exists git grep "ADD IF NOT EXISTS"
Created attachment 170619 [details] [review] Bug 20644: [DO NOT PUSH] DBIC Update
Created attachment 170620 [details] [review] Bug 20644: Add checkprevcheckout on item types
Created attachment 170621 [details] [review] Bug 20644: [DO NOT PUSH] DBIC Update
Created attachment 170622 [details] [review] Bug 20644: Add checkprevcheckout on item types I have added the "Check for previous checkouts" option to item types. This option is visible only when the system preference "CheckPrevCheckout" is set to either : - "Unless overridden by patron category or by item type, do" or - "Unless overridden by patron category or by item type, do not." This option takes precedence over patron and patron category preferences. TEST PLAN 1) Apply the patch. 2) Perform an atomic update (installer/data/mysql/updatedatabase.pl) 3) Set the "CheckPrevCheckout" system preference to "Unless overridden by patron category or by item type, do". 4) Go to "Koha administration > Item types > + New item type". 5) Fill in the necessary fields; the "Check for previous checkouts" field should be set to "Inherit from system preferences". 6) Click the "Save changes" button. 7) Create an item with the type you have created. 8) Check out this item for a patron, then check it in. 9) Check out the same item for the same patron again. 10) The message "Patron has previously checked out this title: <Title>. Check out anyway?" should appear. 11) Go to "Koha administration > Item types". 12) Edit the created item type. 13) Change the value of the "Check for previous checkouts" field to "No and try to override system preferences". 14) Click the "Save changes" button. 15) Check out the same item for the same patron. 16) The message should not appear. 17) Run the tests "Borrower_PrevCheckout.t" (prove t/db_dependent/Patron/Borrower_PrevCheckout.t) and "ItemTypes.t" (prove t/db_dependent/Koha/ItemTypes.t). 18) Verify that all tests pass successfully.
(In reply to Nick Clemens (kidclamp) from comment #13) > > t/db_dependent/Koha/ItemTypes.t: > 180 'Two item types with \'checkprevcheckout\' set to \'undef\' have been > added' > It's just the comment there ==> Fixed > > When editing an itemtype, it the system preference is 'soft' the options are: > Yes and try to override system preference > No and try to override system preference > Inherit from system preference > > From what I understand it does not 'try' to override, it always does > I think we should also display the system preference when set to 'Hard' > otherwise this can be a somewhat hidden feature ==> I removed the try in the options I don't think we should display the system preference when set to 'Hard' because when it's 'Hard' we can't get only two values "1" when it's hardyes and "0" when hardno, regardless of the value of "Check for previous checkouts:" > > For me, 4 nested 'map' calls is a bit obfuscated. Clearer would be something > like: > foreach my $syspref_setting ('hardno','hardyes','softno','softyes'){ > foreach my $itemtype_setting( 'inherit','no','yes'){ > ... > ==> Fixed > > t/db_dependent/Patron/Borrower_PrevCheckout.t > 49 itemtype => 'YESIT', > 59 itemtype => 'NOIT', > 69 itemtype => 'INHERITIT' > I just meant you don't need to specify the codes here, you use them later, > but could save the generated codes as variables in the hash and reference ==> Fixed we don't specify itemtype no more when creating a itemtype > It does, but it is inconsistent with how we handle this in Koha: > if( !column_exists( 'borrowers', 'middle_name' ) ) { ==> I updated the script by adding the condition to not add the column if it exists
Patch doesn't apply: Patch failed at 0001 Bug 20644: [DO NOT PUSH] DBIC Update [33mhint: Use 'git am --show-current-patch=diff' to see the failed patch[m When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-20644-DO-NOT-PUSH-DBIC-Update-khukogbt.patch
It looks like a pretty simple rebase, just need to deal with bookings. Can the database updates be done in a separate commit when this is rebased?
Created attachment 176538 [details] [review] Bug 20644: [DO NOT PUSH] DBIC Update
Created attachment 176539 [details] [review] Bug 20644: Add checkprevcheckout on item types I have added the "Check for previous checkouts" option to item types. This option is visible only when the system preference "CheckPrevCheckout" is set to either : - "Unless overridden by patron category or by item type, do" or - "Unless overridden by patron category or by item type, do not." This option takes precedence over patron and patron category preferences. TEST PLAN 1) Apply the patch. 2) Perform an atomic update (installer/data/mysql/updatedatabase.pl) 3) Set the "CheckPrevCheckout" system preference to "Unless overridden by patron category or by item type, do". 4) Go to "Koha administration > Item types > + New item type". 5) Fill in the necessary fields; the "Check for previous checkouts" field should be set to "Inherit from system preferences". 6) Click the "Save changes" button. 7) Create an item with the type you have created. 8) Check out this item for a patron, then check it in. 9) Check out the same item for the same patron again. 10) The message "Patron has previously checked out this title: <Title>. Check out anyway?" should appear. 11) Go to "Koha administration > Item types". 12) Edit the created item type. 13) Change the value of the "Check for previous checkouts" field to "No and try to override system preferences". 14) Click the "Save changes" button. 15) Check out the same item for the same patron. 16) The message should not appear. 17) Run the tests "Borrower_PrevCheckout.t" (prove t/db_dependent/Patron/Borrower_PrevCheckout.t) and "ItemTypes.t" (prove t/db_dependent/Koha/ItemTypes.t). 18) Verify that all tests pass successfully.
Rebased
Created attachment 178093 [details] [review] Bug 20644: [DO NOT PUSH] DBIC Update
Created attachment 178094 [details] [review] Bug 20644: [DO NOT PUSH] DBIC Update
Created attachment 179812 [details] [review] Bug 20644: [DO NOT PUSH] DBIC Update
Created attachment 179813 [details] [review] Bug 20644: Add checkprevcheckout on item types I have added the "Check for previous checkouts" option to item types. This option is visible only when the system preference "CheckPrevCheckout" is set to either : - "Unless overridden by patron category or by item type, do" or - "Unless overridden by patron category or by item type, do not." This option takes precedence over patron and patron category preferences. TEST PLAN 1) Apply the patch. 2) Perform an atomic update (installer/data/mysql/updatedatabase.pl) 3) Set the "CheckPrevCheckout" system preference to "Unless overridden by patron category or by item type, do". 4) Go to "Koha administration > Item types > + New item type". 5) Fill in the necessary fields; the "Check for previous checkouts" field should be set to "Inherit from system preferences". 6) Click the "Save changes" button. 7) Create an item with the type you have created. 8) Check out this item for a patron, then check it in. 9) Check out the same item for the same patron again. 10) The message "Patron has previously checked out this title: <Title>. Check out anyway?" should appear. 11) Go to "Koha administration > Item types". 12) Edit the created item type. 13) Change the value of the "Check for previous checkouts" field to "No and try to override system preferences". 14) Click the "Save changes" button. 15) Check out the same item for the same patron. 16) The message should not appear. 17) Run the tests "Borrower_PrevCheckout.t" (prove t/db_dependent/Patron/Borrower_PrevCheckout.t) and "ItemTypes.t" (prove t/db_dependent/Koha/ItemTypes.t). 18) Verify that all tests pass successfully.
Created attachment 179814 [details] [review] Bug 20644: [DO NOT PUSH] DBIC Update
Created attachment 179815 [details] [review] Bug 20644: Add checkprevcheckout on item types I have added the "Check for previous checkouts" option to item types. This option is visible only when the system preference "CheckPrevCheckout" is set to either : - "Unless overridden by patron category or by item type, do" or - "Unless overridden by patron category or by item type, do not." This option takes precedence over patron and patron category preferences. TEST PLAN 1) Apply the patch. 2) Perform an atomic update (installer/data/mysql/updatedatabase.pl) 3) Set the "CheckPrevCheckout" system preference to "Unless overridden by patron category or by item type, do". 4) Go to "Koha administration > Item types > + New item type". 5) Fill in the necessary fields; the "Check for previous checkouts" field should be set to "Inherit from system preferences". 6) Click the "Save changes" button. 7) Create an item with the type you have created. 8) Check out this item for a patron, then check it in. 9) Check out the same item for the same patron again. 10) The message "Patron has previously checked out this title: <Title>. Check out anyway?" should appear. 11) Go to "Koha administration > Item types". 12) Edit the created item type. 13) Change the value of the "Check for previous checkouts" field to "No and try to override system preferences". 14) Click the "Save changes" button. 15) Check out the same item for the same patron. 16) The message should not appear. 17) Run the tests "Borrower_PrevCheckout.t" (prove t/db_dependent/Patron/Borrower_PrevCheckout.t) and "ItemTypes.t" (prove t/db_dependent/Koha/ItemTypes.t). 18) Verify that all tests pass successfully.
rebased
Created attachment 180375 [details] [review] Bug 20644: Add checkprevcheckout on item types I have added the "Check for previous checkouts" option to item types. This option is visible only when the system preference "CheckPrevCheckout" is set to either : - "Unless overridden by patron category or by item type, do" or - "Unless overridden by patron category or by item type, do not." This option takes precedence over patron and patron category preferences. TEST PLAN 1) Apply the patch. 2) Perform an atomic update (installer/data/mysql/updatedatabase.pl) 3) Set the "CheckPrevCheckout" system preference to "Unless overridden by patron category or by item type, do". 4) Go to "Koha administration > Item types > + New item type". 5) Fill in the necessary fields; the "Check for previous checkouts" field should be set to "Inherit from system preferences". 6) Click the "Save changes" button. 7) Create an item with the type you have created. 8) Check out this item for a patron, then check it in. 9) Check out the same item for the same patron again. 10) The message "Patron has previously checked out this title: <Title>. Check out anyway?" should appear. 11) Go to "Koha administration > Item types". 12) Edit the created item type. 13) Change the value of the "Check for previous checkouts" field to "No and try to override system preferences". 14) Click the "Save changes" button. 15) Check out the same item for the same patron. 16) The message should not appear. 17) Run the tests "Borrower_PrevCheckout.t" (prove t/db_dependent/Patron/Borrower_PrevCheckout.t) and "ItemTypes.t" (prove t/db_dependent/Koha/ItemTypes.t). 18) Verify that all tests pass successfully. Signed-off-by: Laurence <laurence.rault@biblibre.com>
Functionality is working as expected Test plan 18) Verify that all tests pass successfully : could not be tested on sandbox
Tested this and in staff interface everything works as intended. However tests to not pass, they throw following errors: t/db_dependent/Patron/Borrower_PrevCheckout.t .. 152/170 # Failed test 'PostIssue: Same item, same patron [1]' # at t/db_dependent/Patron/Borrower_PrevCheckout.t line 385. # got: '0' # expected: '1' # Failed test 'PostIssue: Diff item, same bib, same patron [1]' # at t/db_dependent/Patron/Borrower_PrevCheckout.t line 385. # got: '0' # expected: '1' # Failed test 'patron, wants_check_for_previous_checkout, do_check_for_previous_checkout' # at t/db_dependent/Patron/Borrower_PrevCheckout.t line 555. # got: undef # expected: '1' # Failed test 'Check only one item if bibliographic record is serial' # at t/db_dependent/Patron/Borrower_PrevCheckout.t line 582. # got: '0' # expected: '1' # Looks like you failed 1 test of 2. # Failed test 'Check previous checkouts for serial' # at t/db_dependent/Patron/Borrower_PrevCheckout.t line 592. # Failed test 'no warnings' # at /usr/share/perl/5.34/Test/Builder.pm line 193. # There were 73 warning(s) ..... And: t/db_dependent/Koha/ItemTypes.t .. 1/19 # Failed test 'item types should be sorted by translated description' # at t/db_dependent/Koha/ItemTypes.t line 115. # got: '14 vrk av' # expected: 'a translated itemtype desc' # Failed test 'no warnings' # at /usr/share/perl/5.34/Test/Builder.pm line 193. # There were 9 warning(s)
(In reply to Emmi Takkinen from comment #33) > Tested this and in staff interface everything works as intended. However > tests to not pass, they throw following errors: > > t/db_dependent/Patron/Borrower_PrevCheckout.t .. 152/170 > # Failed test 'PostIssue: Same item, same patron [1]' > # at t/db_dependent/Patron/Borrower_PrevCheckout.t line 385. > # got: '0' > # expected: '1' > > # Failed test 'PostIssue: Diff item, same bib, same patron [1]' > # at t/db_dependent/Patron/Borrower_PrevCheckout.t line 385. > # got: '0' > # expected: '1' > > # Failed test 'patron, wants_check_for_previous_checkout, > do_check_for_previous_checkout' > # at t/db_dependent/Patron/Borrower_PrevCheckout.t line 555. > # got: undef > # expected: '1' > > # Failed test 'Check only one item if bibliographic record is serial' > # at t/db_dependent/Patron/Borrower_PrevCheckout.t line 582. > # got: '0' > # expected: '1' > # Looks like you failed 1 test of 2. > > # Failed test 'Check previous checkouts for serial' > # at t/db_dependent/Patron/Borrower_PrevCheckout.t line 592. > > # Failed test 'no warnings' > # at /usr/share/perl/5.34/Test/Builder.pm line 193. > # There were 73 warning(s) > ..... > > And: > > t/db_dependent/Koha/ItemTypes.t .. 1/19 > # Failed test 'item types should be sorted by translated description' > # at t/db_dependent/Koha/ItemTypes.t line 115. > # got: '14 vrk av' > # expected: 'a translated itemtype desc' > > # Failed test 'no warnings' > # at /usr/share/perl/5.34/Test/Builder.pm line 193. > # There were 9 warning(s) Strange. I just ran these 2 tests and they pass with no warnings. Can we get a third input on this? Putting back to Needs Signoff.
(In reply to Shi Yao Wang from comment #34) > (In reply to Emmi Takkinen from comment #33) > > Tested this and in staff interface everything works as intended. However > > tests to not pass, they throw following errors: > > > > t/db_dependent/Patron/Borrower_PrevCheckout.t .. 152/170 > > # Failed test 'PostIssue: Same item, same patron [1]' > > # at t/db_dependent/Patron/Borrower_PrevCheckout.t line 385. > > # got: '0' > > # expected: '1' > > > > # Failed test 'PostIssue: Diff item, same bib, same patron [1]' > > # at t/db_dependent/Patron/Borrower_PrevCheckout.t line 385. > > # got: '0' > > # expected: '1' > > > > # Failed test 'patron, wants_check_for_previous_checkout, > > do_check_for_previous_checkout' > > # at t/db_dependent/Patron/Borrower_PrevCheckout.t line 555. > > # got: undef > > # expected: '1' > > > > # Failed test 'Check only one item if bibliographic record is serial' > > # at t/db_dependent/Patron/Borrower_PrevCheckout.t line 582. > > # got: '0' > > # expected: '1' > > # Looks like you failed 1 test of 2. > > > > # Failed test 'Check previous checkouts for serial' > > # at t/db_dependent/Patron/Borrower_PrevCheckout.t line 592. > > > > # Failed test 'no warnings' > > # at /usr/share/perl/5.34/Test/Builder.pm line 193. > > # There were 73 warning(s) > > ..... > > > > And: > > > > t/db_dependent/Koha/ItemTypes.t .. 1/19 > > # Failed test 'item types should be sorted by translated description' > > # at t/db_dependent/Koha/ItemTypes.t line 115. > > # got: '14 vrk av' > > # expected: 'a translated itemtype desc' > > > > # Failed test 'no warnings' > > # at /usr/share/perl/5.34/Test/Builder.pm line 193. > > # There were 9 warning(s) > > Strange. I just ran these 2 tests and they pass with no warnings. > Can we get a third input on this? > > Putting back to Needs Signoff. Tested again, tests pass on my clean community database but not in my (much messier) production database. And this happens even when this patch isn't applied, so comes from somewhere else in the tests.
Created attachment 181680 [details] [review] Bug 20644: [DO NOT PUSH] DBIC Update
Created attachment 181681 [details] [review] Bug 20644: Add checkprevcheckout on item types I have added the "Check for previous checkouts" option to item types. This option is visible only when the system preference "CheckPrevCheckout" is set to either : - "Unless overridden by patron category or by item type, do" or - "Unless overridden by patron category or by item type, do not." This option takes precedence over patron and patron category preferences. TEST PLAN 1) Apply the patch. 2) Perform an atomic update (installer/data/mysql/updatedatabase.pl) 3) Set the "CheckPrevCheckout" system preference to "Unless overridden by patron category or by item type, do". 4) Go to "Koha administration > Item types > + New item type". 5) Fill in the necessary fields; the "Check for previous checkouts" field should be set to "Inherit from system preferences". 6) Click the "Save changes" button. 7) Create an item with the type you have created. 8) Check out this item for a patron, then check it in. 9) Check out the same item for the same patron again. 10) The message "Patron has previously checked out this title: <Title>. Check out anyway?" should appear. 11) Go to "Koha administration > Item types". 12) Edit the created item type. 13) Change the value of the "Check for previous checkouts" field to "No and try to override system preferences". 14) Click the "Save changes" button. 15) Check out the same item for the same patron. 16) The message should not appear. 17) Run the tests "Borrower_PrevCheckout.t" (prove t/db_dependent/Patron/Borrower_PrevCheckout.t) and "ItemTypes.t" (prove t/db_dependent/Koha/ItemTypes.t). 18) Verify that all tests pass successfully. Signed-off-by: Laurence <laurence.rault@biblibre.com>
Created attachment 182469 [details] [review] Bug 20644: [DO NOT PUSH] DBIC Update
Created attachment 182470 [details] [review] Bug 20644: Add checkprevcheckout on item types I have added the "Check for previous checkouts" option to item types. This option is visible only when the system preference "CheckPrevCheckout" is set to either : - "Unless overridden by patron category or by item type, do" or - "Unless overridden by patron category or by item type, do not." This option takes precedence over patron and patron category preferences. TEST PLAN 1) Apply the patch. 2) Perform an atomic update (installer/data/mysql/updatedatabase.pl) 3) Set the "CheckPrevCheckout" system preference to "Unless overridden by patron category or by item type, do". 4) Go to "Koha administration > Item types > + New item type". 5) Fill in the necessary fields; the "Check for previous checkouts" field should be set to "Inherit from system preferences". 6) Click the "Save changes" button. 7) Create an item with the type you have created. 8) Check out this item for a patron, then check it in. 9) Check out the same item for the same patron again. 10) The message "Patron has previously checked out this title: <Title>. Check out anyway?" should appear. 11) Go to "Koha administration > Item types". 12) Edit the created item type. 13) Change the value of the "Check for previous checkouts" field to "No and try to override system preferences". 14) Click the "Save changes" button. 15) Check out the same item for the same patron. 16) The message should not appear. 17) Run the tests "Borrower_PrevCheckout.t" (prove t/db_dependent/Patron/Borrower_PrevCheckout.t) and "ItemTypes.t" (prove t/db_dependent/Koha/ItemTypes.t). 18) Verify that all tests pass successfully. Signed-off-by: Laurence <laurence.rault@biblibre.com>
Created attachment 182476 [details] [review] Bug 20644: [DO NOT PUSH] DBIC Update Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 182477 [details] [review] Bug 20644: Add checkprevcheckout on item types I have added the "Check for previous checkouts" option to item types. This option is visible only when the system preference "CheckPrevCheckout" is set to either : - "Unless overridden by patron category or by item type, do" or - "Unless overridden by patron category or by item type, do not." This option takes precedence over patron and patron category preferences. TEST PLAN 1) Apply the patch. 2) Perform an atomic update (installer/data/mysql/updatedatabase.pl) 3) Set the "CheckPrevCheckout" system preference to "Unless overridden by patron category or by item type, do". 4) Go to "Koha administration > Item types > + New item type". 5) Fill in the necessary fields; the "Check for previous checkouts" field should be set to "Inherit from system preferences". 6) Click the "Save changes" button. 7) Create an item with the type you have created. 8) Check out this item for a patron, then check it in. 9) Check out the same item for the same patron again. 10) The message "Patron has previously checked out this title: <Title>. Check out anyway?" should appear. 11) Go to "Koha administration > Item types". 12) Edit the created item type. 13) Change the value of the "Check for previous checkouts" field to "No and try to override system preferences". 14) Click the "Save changes" button. 15) Check out the same item for the same patron. 16) The message should not appear. 17) Run the tests "Borrower_PrevCheckout.t" (prove t/db_dependent/Patron/Borrower_PrevCheckout.t) and "ItemTypes.t" (prove t/db_dependent/Koha/ItemTypes.t). 18) Verify that all tests pass successfully. Signed-off-by: Laurence <laurence.rault@biblibre.com> Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 182787 [details] [review] Bug 20644: [DO NOT PUSH] DBIC Update Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 182788 [details] [review] Bug 20644: Add checkprevcheckout on item types I have added the "Check for previous checkouts" option to item types. This option is visible only when the system preference "CheckPrevCheckout" is set to either : - "Unless overridden by patron category or by item type, do" or - "Unless overridden by patron category or by item type, do not." This option takes precedence over patron and patron category preferences. TEST PLAN 1) Apply the patch. 2) Perform an atomic update (installer/data/mysql/updatedatabase.pl) 3) Set the "CheckPrevCheckout" system preference to "Unless overridden by patron category or by item type, do". 4) Go to "Koha administration > Item types > + New item type". 5) Fill in the necessary fields; the "Check for previous checkouts" field should be set to "Inherit from system preferences". 6) Click the "Save changes" button. 7) Create an item with the type you have created. 8) Check out this item for a patron, then check it in. 9) Check out the same item for the same patron again. 10) The message "Patron has previously checked out this title: <Title>. Check out anyway?" should appear. 11) Go to "Koha administration > Item types". 12) Edit the created item type. 13) Change the value of the "Check for previous checkouts" field to "No and try to override system preferences". 14) Click the "Save changes" button. 15) Check out the same item for the same patron. 16) The message should not appear. 17) Run the tests "Borrower_PrevCheckout.t" (prove t/db_dependent/Patron/Borrower_PrevCheckout.t) and "ItemTypes.t" (prove t/db_dependent/Koha/ItemTypes.t). 18) Verify that all tests pass successfully. Signed-off-by: Laurence <laurence.rault@biblibre.com> Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
rebase
Created attachment 183403 [details] [review] Bug 20644: [DO NOT PUSH] DBIC Update Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi> Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Created attachment 183404 [details] [review] Bug 20644: Add checkprevcheckout on item types I have added the "Check for previous checkouts" option to item types. This option is visible only when the system preference "CheckPrevCheckout" is set to either : - "Unless overridden by patron category or by item type, do" or - "Unless overridden by patron category or by item type, do not." This option takes precedence over patron and patron category preferences. TEST PLAN 1) Apply the patch. 2) Perform an atomic update (installer/data/mysql/updatedatabase.pl) 3) Set the "CheckPrevCheckout" system preference to "Unless overridden by patron category or by item type, do". 4) Go to "Koha administration > Item types > + New item type". 5) Fill in the necessary fields; the "Check for previous checkouts" field should be set to "Inherit from system preferences". 6) Click the "Save changes" button. 7) Create an item with the type you have created. 8) Check out this item for a patron, then check it in. 9) Check out the same item for the same patron again. 10) The message "Patron has previously checked out this title: <Title>. Check out anyway?" should appear. 11) Go to "Koha administration > Item types". 12) Edit the created item type. 13) Change the value of the "Check for previous checkouts" field to "No and try to override system preferences". 14) Click the "Save changes" button. 15) Check out the same item for the same patron. 16) The message should not appear. 17) Run the tests "Borrower_PrevCheckout.t" (prove t/db_dependent/Patron/Borrower_PrevCheckout.t) and "ItemTypes.t" (prove t/db_dependent/Koha/ItemTypes.t). 18) Verify that all tests pass successfully. Signed-off-by: Laurence <laurence.rault@biblibre.com> Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi> Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Created attachment 183405 [details] [review] Bug 20644: [DO NOT PUSH] DBIC Update Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi> Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Created attachment 183406 [details] [review] Bug 20644: (QA follow-up) Remove duplicate variable I removed a duplicate variable and tidied.
Created attachment 183407 [details] [review] Bug 20644: (QA follow-up) Remove duplicate variable I removed a duplicate variable and tidied.
Created attachment 183408 [details] [review] Bug 20644: (QA follow-up) [DO NOT PUSH] DBIC Update Tidied the DBIC update
A couple problems: -The database update is not tidy -Tests are now failing, can you have a look?
atomic update file seems tidy and tests seems to pass for me. could someone else check?
I'm not seeing un-tidyness in the current db update. Tests failed for me as well: Test Summary Report ------------------- /kohadevbox/koha/t/db_dependent/Koha/ItemTypes.t (Wstat: 2816 (exited 11) Tests: 16 Failed: 0) Non-zero exit status: 11 Parse errors: Bad plan. You planned 19 tests but ran 16. Files=1, Tests=16, 1 wallclock secs ( 0.02 usr 0.00 sys + 1.08 cusr 0.12 csys = 1.22 CPU) Result: FAIL * Proving /kohadevbox/koha/t/db_dependent/Patron/Borrower_PrevCheckout.t FAIL Error: value hash contains unrecognized columns: checkprevcheckout at /kohadevbox/koha/t/lib/TestBuilder.pm line 311. # Looks like your test exited with 11 just after 2. /kohadevbox/koha/t/db_dependent/Patron/Borrower_PrevCheckout.t .. Dubious, test returned 11 (wstat 2816, 0xb00) Failed 170/172 subtests Test Summary Report ------------------- /kohadevbox/koha/t/db_dependent/Patron/Borrower_PrevCheckout.t (Wstat: 2816 (exited 11) Tests: 2 Failed: 0) Non-zero exit status: 11 Parse errors: Bad plan. You planned 172 tests but ran 2. Files=1, Tests=2, 1 wallclock secs ( 0.01 usr 0.00 sys + 0.91 cusr 0.08 csys = 1.00 CPU) Result: FAIL
(In reply to Lisette Scheer from comment #54) > I'm not seeing un-tidyness in the current db update. > > Tests failed for me as well: > Test Summary Report > ------------------- > /kohadevbox/koha/t/db_dependent/Koha/ItemTypes.t (Wstat: 2816 (exited 11) > Tests: 16 Failed: 0) > Non-zero exit status: 11 > Parse errors: Bad plan. You planned 19 tests but ran 16. > Files=1, Tests=16, 1 wallclock secs ( 0.02 usr 0.00 sys + 1.08 cusr > 0.12 csys = 1.22 CPU) > Result: FAIL > > * Proving > /kohadevbox/koha/t/db_dependent/Patron/Borrower_PrevCheckout.t FAIL > Error: value hash contains unrecognized columns: checkprevcheckout at > /kohadevbox/koha/t/lib/TestBuilder.pm line 311. > # Looks like your test exited with 11 just after 2. > /kohadevbox/koha/t/db_dependent/Patron/Borrower_PrevCheckout.t .. > Dubious, test returned 11 (wstat 2816, 0xb00) > Failed 170/172 subtests > > Test Summary Report > ------------------- > /kohadevbox/koha/t/db_dependent/Patron/Borrower_PrevCheckout.t (Wstat: 2816 > (exited 11) Tests: 2 Failed: 0) > Non-zero exit status: 11 > Parse errors: Bad plan. You planned 172 tests but ran 2. > Files=1, Tests=2, 1 wallclock secs ( 0.01 usr 0.00 sys + 0.91 cusr 0.08 > csys = 1.00 CPU) > Result: FAIL Here is what I get: >Test Summary Report >------------------- >t/00-check-atomic-updates.t (Wstat: 256 (exited 1) Tests: 2 >Failed: 1) > Failed test: 1 > Non-zero exit status: 1 >t/00-deprecated.t (Wstat: 256 (exited 1) Tests: 2 >Failed: 1) > Failed test: 2 > Non-zero exit status: 1 >t/Creators.t (Wstat: 512 (exited 2) Tests: 42 >Failed: 2) > Failed tests: 12, 14 > Non-zero exit status: 2 >Files=76, Tests=2220, 37 wallclock secs ( 0.27 usr 0.08 sys + 29.19 cusr 3.41 >csys = 32.95 CPU) >Result: FAIL the tests that fail except "t/00-check-atomic-updates.t" were already failing before the patches I'm not using kohadevbox so maybe that's why?
> the tests that fail except "t/00-check-atomic-updates.t" were already > failing before the patches > I'm not using kohadevbox so maybe that's why? All the tests are successful for me on main: kohadev-koha@kohadevbox:koha(main)$ prove t/00-check-atomic-updates.t t/00-check-atomic-updates.t .. ok All tests successful. Files=1, Tests=2, 0 wallclock secs ( 0.01 usr 0.01 sys + 0.03 cusr 0.01 csys = 0.06 CPU) Result: PASS kohadev-koha@kohadevbox:koha(main)$ prove t/00-deprecated.t t/00-deprecated.t .. ok All tests successful. Files=1, Tests=2, 0 wallclock secs ( 0.02 usr 0.00 sys + 0.15 cusr 0.16 csys = 0.33 CPU) Result: PASS kohadev-koha@kohadevbox:koha(main)$ prove t/Creators.t t/Creators.t .. ok All tests successful. Files=1, Tests=42, 1 wallclock secs ( 0.01 usr 0.01 sys + 0.57 cusr 0.06 csys = 0.65 CPU) Result: PASS kohadev-koha@kohadevbox:koha(main)$ prove t/db_dependent/Patron/Borrower_PrevCheckout.t t/db_dependent/Patron/Borrower_PrevCheckout.t .. ok All tests successful. Files=1, Tests=64, 5 wallclock secs ( 0.01 usr 0.01 sys + 3.59 cusr 0.70 csys = 4.31 CPU) Result: PASS kohadev-koha@kohadevbox:koha(main)$ prove t/db_dependent/Koha/ItemTypes.t t/db_dependent/Koha/ItemTypes.t .. ok All tests successful. Files=1, Tests=16, 2 wallclock secs ( 0.01 usr 0.01 sys + 1.17 cusr 0.12 csys = 1.31 CPU) Result: PASS
Please see Bug 40337 - checkprevcheckout must be defined as ENUM at DB level Also, your changes in Koha/Patrons.pm need tests to cover those changes.
Created attachment 185448 [details] [review] Bug 20644: Add checkprevcheckout on item types I have added the "Check for previous checkouts" option to item types. This option is visible only when the system preference "CheckPrevCheckout" is set to either : - "Unless overridden by patron category or by item type, do" or - "Unless overridden by patron category or by item type, do not." This option takes precedence over patron and patron category preferences. TEST PLAN 1) Apply the patch. 2) Perform an atomic update (installer/data/mysql/updatedatabase.pl) 3) Set the "CheckPrevCheckout" system preference to "Unless overridden by patron category or by item type, do". 4) Go to "Koha administration > Item types > + New item type". 5) Fill in the necessary fields; the "Check for previous checkouts" field should be set to "Inherit from system preferences". 6) Click the "Save changes" button. 7) Create an item with the type you have created. 8) Check out this item for a patron, then check it in. 9) Check out the same item for the same patron again. 10) The message "Patron has previously checked out this title: <Title>. Check out anyway?" should appear. 11) Go to "Koha administration > Item types". 12) Edit the created item type. 13) Change the value of the "Check for previous checkouts" field to "No and try to override system preferences". 14) Click the "Save changes" button. 15) Check out the same item for the same patron. 16) The message should not appear. 17) Run the tests "Borrower_PrevCheckout.t" (prove t/db_dependent/Patron/Borrower_PrevCheckout.t) and "ItemTypes.t" (prove t/db_dependent/Koha/ItemTypes.t). 18) Verify that all tests pass successfully. Signed-off-by: Laurence <laurence.rault@biblibre.com> Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi> Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Created attachment 185449 [details] [review] Bug 20644: (QA follow-up) Remove duplicate variable I removed a duplicate variable and tidied.
Created attachment 185450 [details] [review] Bug 20644: [DO NOT PUSH] DBIC Update Tidied
Created attachment 185451 [details] [review] Bug 20644: Change checkprevcheckout as ENUM at DB level
(In reply to Lucas Gass (lukeg) from comment #57) > Please see Bug 40337 - checkprevcheckout must be defined as ENUM at DB level > > Also, your changes in Koha/Patrons.pm need tests to cover those changes. checkprevcheckout is now an ENUM at DB level There are no changes in Koha/Patrons.pm. Maybe you meant Koha/Patron.pm? Those changes are tested in t/db_dependent/Patron/Borrower_PrevCheckout.t
I see the aforementioned tests are failing. Looking into it
Created attachment 185478 [details] [review] Bug 20644: Add checkprevcheckout on item types I have added the "Check for previous checkouts" option to item types. This option is visible only when the system preference "CheckPrevCheckout" is set to either : - "Unless overridden by patron category or by item type, do" or - "Unless overridden by patron category or by item type, do not." This option takes precedence over patron and patron category preferences. TEST PLAN 1) Apply the patch. 2) Perform an atomic update (installer/data/mysql/updatedatabase.pl) 3) Set the "CheckPrevCheckout" system preference to "Unless overridden by patron category or by item type, do". 4) Go to "Koha administration > Item types > + New item type". 5) Fill in the necessary fields; the "Check for previous checkouts" field should be set to "Inherit from system preferences". 6) Click the "Save changes" button. 7) Create an item with the type you have created. 8) Check out this item for a patron, then check it in. 9) Check out the same item for the same patron again. 10) The message "Patron has previously checked out this title: <Title>. Check out anyway?" should appear. 11) Go to "Koha administration > Item types". 12) Edit the created item type. 13) Change the value of the "Check for previous checkouts" field to "No and try to override system preferences". 14) Click the "Save changes" button. 15) Check out the same item for the same patron. 16) The message should not appear. 17) Run the tests "Borrower_PrevCheckout.t" (prove t/db_dependent/Patron/Borrower_PrevCheckout.t) and "ItemTypes.t" (prove t/db_dependent/Koha/ItemTypes.t). 18) Verify that all tests pass successfully. Signed-off-by: Laurence <laurence.rault@biblibre.com> Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi> Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Created attachment 185479 [details] [review] Bug 20644: (QA follow-up) Remove duplicate variable I removed a duplicate variable and tidied.
Created attachment 185480 [details] [review] Bug 20644: Change checkprevcheckout as ENUM at DB level
Created attachment 185481 [details] [review] Bug 20644: [DO NOT PUSH] DBIC Update
For some reason the previous `Bug 20644: [DO NOT PUSH] DBIC Update` was just a tidy on the atomic update file? so I squashed it into the previous commit `Bug 20644: (QA follow-up) Remove duplicate variable` to make way for an actual DBIC Update that was the missing piece to fixing the tests.
Created attachment 185557 [details] [review] Bug 20644: Add checkprevcheckout on item types I have added the "Check for previous checkouts" option to item types. This option is visible only when the system preference "CheckPrevCheckout" is set to either : - "Unless overridden by patron category or by item type, do" or - "Unless overridden by patron category or by item type, do not." This option takes precedence over patron and patron category preferences. TEST PLAN 1) Apply the patch. 2) Perform an atomic update (installer/data/mysql/updatedatabase.pl) 3) Set the "CheckPrevCheckout" system preference to "Unless overridden by patron category or by item type, do". 4) Go to "Koha administration > Item types > + New item type". 5) Fill in the necessary fields; the "Check for previous checkouts" field should be set to "Inherit from system preferences". 6) Click the "Save changes" button. 7) Create an item with the type you have created. 8) Check out this item for a patron, then check it in. 9) Check out the same item for the same patron again. 10) The message "Patron has previously checked out this title: <Title>. Check out anyway?" should appear. 11) Go to "Koha administration > Item types". 12) Edit the created item type. 13) Change the value of the "Check for previous checkouts" field to "No and try to override system preferences". 14) Click the "Save changes" button. 15) Check out the same item for the same patron. 16) The message should not appear. 17) Run the tests "Borrower_PrevCheckout.t" (prove t/db_dependent/Patron/Borrower_PrevCheckout.t) and "ItemTypes.t" (prove t/db_dependent/Koha/ItemTypes.t). 18) Verify that all tests pass successfully. Signed-off-by: Laurence <laurence.rault@biblibre.com> Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi> Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Created attachment 185558 [details] [review] Bug 20644: (QA follow-up) Remove duplicate variable I removed a duplicate variable and tidied. Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Created attachment 185559 [details] [review] Bug 20644: Change checkprevcheckout as ENUM at DB level Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Created attachment 185560 [details] [review] Bug 20644: [DO NOT PUSH] DBIC Update Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Nice work everyone! Pushed to main for 25.11
This is now causing t/db_dependent/api/v1/item_types.t to fail. Please follow-up.
I think this causes a problem when editing an item type. I was trying to make an existing itemtype "bookable" and got the error message "An error occurred when updating this item type. Perhaps the value already exists.". When I looked into plack-intranet-error.log, it said "[2025/08/21 15:48:59] [WARN] DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Column 'checkprevcheckout' cannot be null at /kohadevbox/koha/Koha/O bject.pm line 174" If CheckPrevCheckout is set to one of the 'Unless overridden by...' option, I can edit and save my itemtype. If CheckPrevCheckout is set to 'Do' or 'Do not', I can't edit an itemtype.
(In reply to Caroline Cyr La Rose from comment #75) > I think this causes a problem when editing an item type. I was trying to > make an existing itemtype "bookable" and got the error message "An error > occurred when updating this item type. Perhaps the value already exists.". > When I looked into plack-intranet-error.log, it said "[2025/08/21 15:48:59] > [WARN] DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: > DBD::mysql::st execute failed: Column 'checkprevcheckout' cannot be null at > /kohadevbox/koha/Koha/O bject.pm line 174" > > If CheckPrevCheckout is set to one of the 'Unless overridden by...' option, > I can edit and save my itemtype. If CheckPrevCheckout is set to 'Do' or 'Do > not', I can't edit an itemtype. I'll need a fix for this, and the tests I mention, before Monday. Or I will need to revert this, sorry.
(In reply to Caroline Cyr La Rose from comment #75) > I think this causes a problem when editing an item type. I was trying to > make an existing itemtype "bookable" and got the error message "An error > occurred when updating this item type. Perhaps the value already exists.". > When I looked into plack-intranet-error.log, it said "[2025/08/21 15:48:59] > [WARN] DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: > DBD::mysql::st execute failed: Column 'checkprevcheckout' cannot be null at > /kohadevbox/koha/Koha/O bject.pm line 174" > > If CheckPrevCheckout is set to one of the 'Unless overridden by...' option, > I can edit and save my itemtype. If CheckPrevCheckout is set to 'Do' or 'Do > not', I can't edit an itemtype. This is because when CheckPrevCheckout is softyes of softno the dropdown appears. When there is no dropdown it tries to save a NULL value which isn't allowed.
I got the code fix for Caroline's issue. I am trying to fix the tests as well to push both solutions in the same commit
Created attachment 185665 [details] [review] Bug 20644: (follow-up) Dont allow NULL value when the dropdown doesnt exist
We also need to fix the atomicupdate, ADD IF NOT EXISTS isn't right.
Created attachment 185666 [details] [review] Bug 20644: (follow-up) Fix atomicupdate
Created attachment 185669 [details] [review] Bug 20644: Update swagger definitions for checkprevcheck
I think these 3 follow-ups fix all the problems here. Can I get a sanity check please?
(In reply to Lucas Gass (lukeg) from comment #79) > Created attachment 185665 [details] [review] [review] > Bug 20644: (follow-up) Dont allow NULL value when the dropdown doesnt exist Would it be better if we assigned "inherit" by default at line 102? (In reply to Lucas Gass (lukeg) from comment #82) > Created attachment 185669 [details] [review] [review] > Bug 20644: Update swagger definitions for checkprevcheck t/db_dependent/api/v1/item_types.t tests are still failing. and it would be more accurate to say: > description: Produce a warning for this item type if this item has previously > been checked out to this patron if ''yes'', not if ''no'', defer to patron > setting if ''inherit''.' instead of > description: Produce a warning for this item type if this item has previously > been checked out to this patron if ''yes'', not if ''no'', defer to syspref > setting if ''inherit''.'
Created attachment 185670 [details] [review] Bug 20644: (follow-up) Dont allow NULL value when the dropdown doesnt exist Signed-off-by: David Nind <david@davidnind.com>
Created attachment 185671 [details] [review] Bug 20644: (follow-up) Fix atomicupdate Signed-off-by: David Nind <david@davidnind.com>
Created attachment 185672 [details] [review] Bug 20644: Update swagger definitions for checkprevcheck Signed-off-by: David Nind <david@davidnind.com>
I added my signoffs to the three follow-ups (using KTD): 1. Applied follow-up patches (using interactive mode) 2. Updated the database (no messages output): updatedatabase 3. Updated things: yarn build 4. Restarted everything: restart_all 5. Tested this things: 5.1 Making an item type bookable 5.2 Repeat 5.1 with all the options for the CheckPrevCheckout system preference
These tests all pass for me: prove t/db_dependent/api/v1/item_types.t prove t/db_dependent/Koha/ItemTypes.t prove t/db_dependent/Patron/Borrower_PrevCheckout.t
Nevermind about the tests not passing for me. I forgot to run `yarn api:bundle`. Now it works I will add a follow-up patch to correct the description in swagger definition and assign "inherit" by default
Created attachment 185673 [details] [review] Bug 20644: (follow-up) Fix swagger description + put "inherit" by default
Is the IF NOT EXISTS really necessary in the file installer/data/mysql/db_revs/250600010.pl? It's already being checked before applying the SQL with !column_exists( 'itemtypes', 'checkprevcheckout' ) and it returns an error with versions of MySQL such as Ver 15.1 Distrib 10.1.48-MariaDB and Ver 8.0.31 Error message: ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'IF NOT EXISTS checkprevcheckout enum('yes', 'no', 'inherit') NOT NULL DEFAULT' at line 1
Follow-ups here are pushed to main.
We still need release notes here, please add them.