Bug 39871

Summary: Cannot clear item statuses with batch item modification tool
Product: Koha Reporter: Emily Lamancusa (emlam) <emily.lamancusa>
Component: CatalogingAssignee: Emily Lamancusa (emlam) <emily.lamancusa>
Status: Pushed to main --- QA Contact: Marcel de Rooy <m.de.rooy>
Severity: normal    
Priority: P5 - low CC: flaterdavid, lucas, m.de.rooy
Version: MainKeywords: release-notes-needed
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40259
GIT URL: Change sponsored?: ---
Patch complexity: Small patch Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
25.11.00
Circulation function:
Attachments: Bug 39871: Don't pass undef for non-nullable fields from batch mod tool
Bug 39871: Don't pass undef for non-nullable fields from batch mod tool
Bug 39871: Don't pass undef for non-nullable fields from batch mod tool
Bug 39871: Don't pass undef for non-nullable fields from batch mod tool
Bug 39871: (QA follow-up) Do not hardcode nullable check
Bug 39871: (QA follow-up) Do not hardcode nullable check

Description Emily Lamancusa (emlam) 2025-05-09 18:21:33 UTC
To replicate:
1. Set not-for-loan, withdrawn, and damaged statuses on some items
2. Try to edit only those items with the Batch item modification tool
3. Check the checkboxes to clear the withdrawn, not-for-loan, and damaged statuses
4. Click Save

If you view the background job report, it will say that the items could not be modified, and if you view the items you will see that the statuses were not cleared.

The background job logs show database warnings saying that those columns cannot be set to null.

More testing may be needed to see if this applies to other fields as well. I suspect it will apply to the lost field as well, but the lost status is currently hidden on the batch item modification screen on main for me - not sure if that is another bug or a configuration I wasn't aware of.
Comment 1 Emily Lamancusa (emlam) 2025-06-25 16:57:39 UTC Comment hidden (obsolete)
Comment 2 David Flater 2025-06-26 14:38:12 UTC Comment hidden (obsolete)
Comment 3 Owen Leonard 2025-06-26 15:19:28 UTC
Created attachment 183549 [details] [review]
Bug 39871: Don't pass undef for non-nullable fields from batch mod tool

To test:
1. Set not-for-loan, withdrawn, and damaged statuses on some items
2. Try to edit only those items with the Batch item modification tool
3. Check the checkboxes to clear the withdrawn, not-for-loan, and
   damaged statuses
4. Click Save
5. View the background job report
   --> Note that it says the items could not be modified
6. Look at the items
   --> The statuses have not been cleared
7. Apply patch and restart_all
8. Repeat steps 2-5
   --> The background job report says the modifications were successful
9. View the items
   --> The statuses have been cleared

Signed-off-by: David Flater <flaterdavid@gmail.com>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 4 Marcel de Rooy 2025-06-27 08:17:32 UTC
Looking here
Comment 5 Marcel de Rooy 2025-06-27 09:23:26 UTC
Doing something wrong helps you find bugs :)

batchMod works with a nested try/catch construction:

First the worker calls Koha/BackgroundJob/BatchUpdateItem.pm doing
    try {
        my ($results) = Koha::Items->search( { itemnumber => \@record_ids } )->batch_update(
etc
    } catch {
        warn $_;
        die "Something terrible has happened!"
            if ( $_ =~ /Rollback failed/ );    # Rollback failed

And batch_update does this
        try {
            $schema->txn_do(
                sub {
[etc]
$item->set($new_values)->store( { skip_record_index => 1 } );
[etc]
        } catch {
            push @errors, {
                error => eval { $_->{error} } || "$_",
            };
            warn $_
Note here that the catch does not die or raise an exception! I found the error in worker-output.log:
DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Column 'enumchron' cannot be null at /usr/share/koha/Koha/Object.pm line 174
This error was raised by doing something wrong but that does not matter here.
The point is that the inner try block only warns in the catch. So the outer try block does not catch anything! The error is just silently ignored.
I just got back: No items modified.

Note that this error should have its own new report. Will open one.

What did I do wrong btw? I tested my follow-up with withdrawn and enumchron. First I tested if enumchron was blanked with NULL. That was ok. Then I just altered the table items with enumchron NOT NULL and hoped to see that it would be empty string. Forgetting that I am asking DBIx if it is nullable, so DBIx had not changed and said Go ahead. Etc etc.
Comment 6 Marcel de Rooy 2025-06-27 09:30:46 UTC
Created attachment 183591 [details] [review]
Bug 39871: Don't pass undef for non-nullable fields from batch mod tool

To test:
1. Set not-for-loan, withdrawn, and damaged statuses on some items
2. Try to edit only those items with the Batch item modification tool
3. Check the checkboxes to clear the withdrawn, not-for-loan, and
   damaged statuses
4. Click Save
5. View the background job report
   --> Note that it says the items could not be modified
6. Look at the items
   --> The statuses have not been cleared
7. Apply patch and restart_all
8. Repeat steps 2-5
   --> The background job report says the modifications were successful
9. View the items
   --> The statuses have been cleared

Signed-off-by: David Flater <flaterdavid@gmail.com>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Marcel de Rooy 2025-06-27 09:30:48 UTC
Created attachment 183592 [details] [review]
Bug 39871: (QA follow-up) Do not hardcode nullable check

Lets check DBIx's columns_info.

Test plan:
Try to blank an integer column and a string column. Verify results.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 8 Marcel de Rooy 2025-06-27 09:32:19 UTC
(In reply to Emily Lamancusa (emlam) from comment #0)
> The background job logs show database warnings saying that those columns
> cannot be set to null.

Actually an issue on itself as confirmed above.
Comment 9 Marcel de Rooy 2025-06-27 09:39:35 UTC
Wait. This follow-up does not handle not-nullable dates correctly (if we have them in items)
Comment 10 Marcel de Rooy 2025-06-27 09:48:42 UTC
Created attachment 183596 [details] [review]
Bug 39871: (QA follow-up) Do not hardcode nullable check

Lets check DBIx's columns_info.

Test plan:
Try to blank an integer column and a string column. Verify results.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 11 Lucas Gass (lukeg) 2025-06-30 14:32:20 UTC
Nice work everyone!

Pushed to main for 25.11