Bug 40777

Summary: 500 Error: Something went wrong when loading the table Should Exit Cleanly
Product: Koha Reporter: John Yorio <jyorio>
Component: CatalogingAssignee: Jonathan Druart <jonathan.druart>
Status: Passed QA --- QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: david, fridolin.somers, jonathan.druart, lisette, lucas, m.de.rooy, marion.durand, nick, phil, victor
Version: unspecified   
Hardware: All   
OS: All   
GIT URL: Initiative type: ---
Sponsorship status: --- Comma delimited list of Sponsors:
Crowdfunding goal: 0 Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
This adds an "Audit" option on the toolbar for records in the staff interface. This implements the check for missing home and current library data (952$a and 952$b fields). If a record has data inconsistencies, then using the audit option is shown on the error message that is shown when accessing the record details page: Something went wrong when loading the table. 500: Internal server error Have a look at the "Audit" button in the toolbar
Version(s) released in:
Circulation function:
Attachments: Bug 40777: Introduce Koha::Database::DataInconsistency
Bug 40777: Add 'Audit' button in the toolbar
Bug 40777: Add info in the alert
Bug 40777: Move all biblio tests to the new package
Bug 40777: Add tests for invalid_item_library and no_item_type
Bug 40777: Add tests for invalid_item_type
Bug 40777: Add tests for errors_in_marc
Bug 40777: Add tests for nonexistent_AV
Bug 40777: Add tests for empty_title
Bug 40777: Flatten arrays
Bug 40777: Introduce Koha::Database::DataInconsistency
Bug 40777: Add 'Audit' button in the toolbar
Bug 40777: Add info in the alert
Bug 40777: Move all biblio tests to the new package
Bug 40777: Add tests for invalid_item_library and no_item_type
Bug 40777: Add tests for invalid_item_type
Bug 40777: Add tests for errors_in_marc
Bug 40777: Add tests for nonexistent_AV
Bug 40777: Add tests for empty_title
Bug 40777: Flatten arrays
Bug 40777: Add tests for for_biblio
Bug 40777: Add POD
Bug 40777: Introduce Koha::Database::DataInconsistency
Bug 40777: Add 'Audit' button in the toolbar
Bug 40777: Add info in the alert
Bug 40777: Move all biblio tests to the new package
Bug 40777: Add tests for invalid_item_library and no_item_type
Bug 40777: Add tests for invalid_item_type
Bug 40777: Add tests for errors_in_marc
Bug 40777: Add tests for nonexistent_AV
Bug 40777: Add tests for empty_title
Bug 40777: Flatten arrays
Bug 40777: Add tests for for_biblio
Bug 40777: Add POD
Bug 40777: (QA follow-up): remove trailing space from translatable string

Description John Yorio 2025-09-09 15:53:59 UTC
It's possible this is related to/a duplicate of https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40762 but I am not certain.

In Koha 24.05 and 24.11, in the staff interface, if a bib with an item without a 952$a and 952$b is imported, when the staff member then goes to the bib record they receive a modal pop-up stating "Something went wrong when loading the table. 500: error". 

If they click OK on that message, a processing box sits on the screen (you can see the bib loaded behind it) and spins. 

The console log shows:
datatables_24.1107000.js: 408
DataTables warning: table id=holdings_table - Ajax error. For more information about this error, please see https://datatables.net/tn/7

The "processing" box is not modal and it is possible to click on Edit, Manage items, and then edit the problem item to add the missing Home and Current libraries and resolve the cause of the error.

This behavior does not occur in 23.11. 

While, of course, the item needs the Home and Current library, the error alert is a change in expected behavior from staff member perspective. A cleaner exit with a more informative message than a 500 error seems preferable.
Comment 1 Jonathan Druart 2025-10-07 14:18:18 UTC
Created attachment 187526 [details] [review]
Bug 40777: Introduce Koha::Database::DataInconsistency
Comment 2 Jonathan Druart 2025-10-07 14:18:20 UTC
Created attachment 187527 [details] [review]
Bug 40777: Add 'Audit' button in the toolbar
Comment 3 Jonathan Druart 2025-10-07 14:18:22 UTC
Created attachment 187528 [details] [review]
Bug 40777: Add info in the alert
Comment 4 Jonathan Druart 2025-10-07 14:21:05 UTC
Patches for discussion and feedback.

I am suggesting to move the code from the search_for_inconsistencies.pl maintenance script to a module to make it reusable from the Koha UI.

Those patch moves only the first check (no valid home or holding library) to the module, and add an "Audit" icon to the bibliographic toolbar.

https://snipboard.io/1owIgH.jpg

If people are willing to test and provide feedback, I will provide more work.
Comment 5 Marcel de Rooy 2025-10-07 14:55:48 UTC
From kohastructure
  `homebranch` varchar(10) DEFAULT NULL COMMENT 'foreign key from the branches table for the library that owns this item (MARC21 952$a)',
  `holdingbranch` varchar(10) DEFAULT NULL COMMENT 'foreign key from the branches table for the library that is currently in possession item (MARC21 952$b)',

While both columns may still be NULL, we will have such issues.
What about NOT NULL to ensure integrity?
If we create an item somewhere or import it, we should probably know where it belongs. Just suspecting that invalid branchcodes during import may be at stake.

And note that the REST API who is called to fetch the items, may need some extra check while we dont address it in the table design?
Comment 6 Jonathan Druart 2025-10-09 10:40:19 UTC
(In reply to Marcel de Rooy from comment #5)
> From kohastructure
>   `homebranch` varchar(10) DEFAULT NULL COMMENT 'foreign key from the
> branches table for the library that owns this item (MARC21 952$a)',
>   `holdingbranch` varchar(10) DEFAULT NULL COMMENT 'foreign key from the
> branches table for the library that is currently in possession item (MARC21
> 952$b)',
> 
> While both columns may still be NULL, we will have such issues.
> What about NOT NULL to ensure integrity?
> If we create an item somewhere or import it, we should probably know where
> it belongs. Just suspecting that invalid branchcodes during import may be at
> stake.
> 
> And note that the REST API who is called to fetch the items, may need some
> extra check while we dont address it in the table design?

This is outside the scope.

I have implemented the first test of misc/maintenance/search_for_data_inconsistencies.pl, but there are more:

* incorrect item type (yes you could argue that a FK is missing as well for this one)
* invalid marcxml
* item fields in MARC
* wrong authorised values
* record without title

The goal here is to offer the output of the script from the UI.
Comment 7 Jonathan Druart 2025-10-21 13:47:42 UTC
Created attachment 188222 [details] [review]
Bug 40777: Move all biblio tests to the new package
Comment 8 Jonathan Druart 2025-10-21 13:47:45 UTC
Created attachment 188223 [details] [review]
Bug 40777: Add tests for invalid_item_library and no_item_type
Comment 9 Jonathan Druart 2025-10-21 13:47:47 UTC
Created attachment 188224 [details] [review]
Bug 40777: Add tests for invalid_item_type
Comment 10 Jonathan Druart 2025-10-21 13:47:50 UTC
Created attachment 188225 [details] [review]
Bug 40777: Add tests for errors_in_marc
Comment 11 Jonathan Druart 2025-10-21 13:47:52 UTC
Created attachment 188226 [details] [review]
Bug 40777: Add tests for nonexistent_AV
Comment 12 Jonathan Druart 2025-10-21 13:47:54 UTC
Created attachment 188227 [details] [review]
Bug 40777: Add tests for empty_title
Comment 13 Jonathan Druart 2025-10-21 13:49:26 UTC
Ready for testing.

The tests directly related to a bibliographic record have been moved to the new packages (which are now covered by unit tests).

To test this you need to confirm that the UI shows (using the audit tool) the same errors/warnings we have with the maintenance script.

I can provide a more detailed test plan if needed ;)
Comment 14 Phil Ringnalda 2025-11-06 23:00:02 UTC
The UI shows literally


Errors found
ARRAY(0xaaab121c8750)
Comment 15 Jonathan Druart 2025-11-14 12:53:58 UTC
Created attachment 189598 [details] [review]
Bug 40777: Flatten arrays

Patch from commit e253e76
Comment 16 Jonathan Druart 2025-11-14 12:54:06 UTC
(In reply to Phil Ringnalda from comment #14)
> The UI shows literally
> 
> 
> Errors found
> ARRAY(0xaaab121c8750)

Oops sorry, it seems that I have not retested the UI after the follow-ups. Should be fixed now.
Comment 17 David Nind 2025-12-06 13:01:54 UTC
Created attachment 190261 [details] [review]
Bug 40777: Introduce Koha::Database::DataInconsistency

Signed-off-by: David Nind <david@davidnind.com>
Comment 18 David Nind 2025-12-06 13:01:58 UTC
Created attachment 190262 [details] [review]
Bug 40777: Add 'Audit' button in the toolbar

Signed-off-by: David Nind <david@davidnind.com>
Comment 19 David Nind 2025-12-06 13:02:02 UTC
Created attachment 190263 [details] [review]
Bug 40777: Add info in the alert

Signed-off-by: David Nind <david@davidnind.com>
Comment 20 David Nind 2025-12-06 13:02:06 UTC
Created attachment 190264 [details] [review]
Bug 40777: Move all biblio tests to the new package

Signed-off-by: David Nind <david@davidnind.com>
Comment 21 David Nind 2025-12-06 13:02:11 UTC
Created attachment 190265 [details] [review]
Bug 40777: Add tests for invalid_item_library and no_item_type

Signed-off-by: David Nind <david@davidnind.com>
Comment 22 David Nind 2025-12-06 13:02:15 UTC
Created attachment 190266 [details] [review]
Bug 40777: Add tests for invalid_item_type

Signed-off-by: David Nind <david@davidnind.com>
Comment 23 David Nind 2025-12-06 13:02:19 UTC
Created attachment 190267 [details] [review]
Bug 40777: Add tests for errors_in_marc

Signed-off-by: David Nind <david@davidnind.com>
Comment 24 David Nind 2025-12-06 13:02:23 UTC
Created attachment 190268 [details] [review]
Bug 40777: Add tests for nonexistent_AV

Signed-off-by: David Nind <david@davidnind.com>
Comment 25 David Nind 2025-12-06 13:02:27 UTC
Created attachment 190269 [details] [review]
Bug 40777: Add tests for empty_title

Signed-off-by: David Nind <david@davidnind.com>
Comment 26 David Nind 2025-12-06 13:02:31 UTC
Created attachment 190270 [details] [review]
Bug 40777: Flatten arrays

Signed-off-by: David Nind <david@davidnind.com>
Comment 27 David Nind 2025-12-06 13:12:53 UTC
Testing notes (using KTD):

1. Export a record with an item from the staff interface as MARCXML (Save > MARCXML).
2. Edit the file and remove the 952$a (home library) and 952$b (current library).
3. Delete the item and record in the staff interface.
4. Import the exported record and item from the MARCXML file:
   - Cataloging > Import > Stage for import
     . Settings > Format: MARCXML
   - Stage for import
   - View batch link on the job details page (assuming successfully staged)
     . New record framework: change to Books, Booklets, Workbooks
5. Turn on the browser developer tools and view the console.
6. Access the details page for the new record.
7. Note that you get:
   - A "Processing" dialog box on screen
   - A browser pop-up window/dialog box with:
       Something went wrong when loading the table.
       500: Internal server error
   - Click OK
   - The console shows:
      . Failed to load resource: the server responded with a status of 500 (Internal Server Error)
      . Got 500 (Internal Server Error)
      . DataTables warning: table id=holdings_table - Ajax error. For more information about this error, please see https://datatables.net/tn/7
8. Run misc/maintenance/search_for_data_inconsistencies.pl, you get these result (the itemnumber may be different):
    == Not defined items.homebranch and/or items.holdingbranch ==
    	* Item with itemnumber=973 does not have home and holding library defined
    => Edit these items and set valid homebranch and/or holdingbranch
9. Apply the patch.
10. Restart everything.
11. Refresh the details screen for the imported record.
12. You now get the same as step 7, but with added "Have a look at the "Audit" button in the toolbar" text in the browser pop-up dialog box:
    - A "Processing" dialog box on screen
    - A browser pop-up window/dialog box with:
        Something went wrong when loading the table.
        500: Internal server error
        Have a look at the "Audit" button in the toolbar
    - Click OK
    - The console shows:
      . GET http[URL for the record....] 500 (Internal Server Error)
      . Got 500 (Internal Server Error)
      . DataTables warning: table id=holdings_table - Ajax error. For more information about this error, please see https://datatables.net/tn/7
13. Click on the Audit button.
14. You get a yellow box at the top of the record with:
      Errors found
      Item with itemnumber=973 does not have home and holding library defined
    (Note that there is still the "Processing" dialog box, you can edit the item by select the "Items" option in the sidebar menu and then edit the item details - the home and current library files will be auto-populated with the current library, Centerville with KTD)
15. Repeat step 8, you get the same results.
16. The tests pass: prove t/db_dependent/Koha/Database/DataInconsistency.t 
17. Check the details page for another record that doesn't have any data issues - note the 'Audit' toolbar item, and if clicked should show a blue box at the top of the record with "No data inconsistency found."
Comment 28 Victor Grousset/tuxayo 2026-01-03 23:05:18 UTC
> move the code from the search_for_inconsistencies.pl maintenance script to a module to make it reusable from the Koha UI.

This is great :D


----


Turns out the tests fail if one happens to have started KTD with UNIMARC data.

And if I go to the UI and click audit. Like http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=1271 (unimarc test data)

I get:

The Framework ** is using the authorised value's category *TYPEDOC*, but the following biblioitems.itemtype do not have a value defined ({itemnumber => value }): {2275 => LIVR}
The Framework ** is using the authorised value's category *LOC*, but the following items.location do not have a value defined ({itemnumber => value }): {2275 => Documentaires adulte} 

So it doesn't seems like it's just the tests that have hard coded marc21 data.

------

No POD for subs and the new module.
Though the QA script skips it. Does that mean it's not actually mandated there? Or is that just because it's too hard given the legacy code everywhere to make this blocking in the script?
[SKIP] Koha/Database/DataInconsistency.pm
   SKIP   pod_coverage
                No POD exists


----

for_biblio() isn't covered by tests (I mean actual code coverage, not the weird error message we get in some cases)

In errors_in_marc() the cases "biblio_tag < 10" and "biblioitem_tag < 10" aren't covered but if it's not trivial I don't think it's a big deal.
Comment 29 Jonathan Druart 2026-01-05 10:24:29 UTC
Created attachment 190884 [details] [review]
Bug 40777: Add tests for for_biblio

Patch from commit 6fbb2b1
Comment 30 Jonathan Druart 2026-01-05 10:24:54 UTC
Created attachment 190885 [details] [review]
Bug 40777: Add POD

Patch from commit 8b8f7de
Comment 31 Jonathan Druart 2026-01-05 10:25:09 UTC
(In reply to Victor Grousset/tuxayo from comment #28)

> Turns out the tests fail if one happens to have started KTD with UNIMARC
> data.
> 
> And if I go to the UI and click audit. Like
> http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=1271
> (unimarc test data)
> 
> I get:
> 
> The Framework ** is using the authorised value's category *TYPEDOC*, but the
> following biblioitems.itemtype do not have a value defined ({itemnumber =>
> value }): {2275 => LIVR}
> The Framework ** is using the authorised value's category *LOC*, but the
> following items.location do not have a value defined ({itemnumber => value
> }): {2275 => Documentaires adulte} 
> 
> So it doesn't seems like it's just the tests that have hard coded marc21
> data.

Looks like incorrect data, it's a misc4dev's problem.

> No POD for subs and the new module.
> Though the QA script skips it. Does that mean it's not actually mandated
> there? Or is that just because it's too hard given the legacy code
> everywhere to make this blocking in the script?
> [SKIP] Koha/Database/DataInconsistency.pm
>    SKIP   pod_coverage
>                 No POD exists

Fixed in the last patch.

> for_biblio() isn't covered by tests (I mean actual code coverage, not the
> weird error message we get in some cases)

Added a test.

> In errors_in_marc() the cases "biblio_tag < 10" and "biblioitem_tag < 10"
> aren't covered but if it's not trivial I don't think it's a big deal.

Yeah, not a big deal :D
And feeling lazy for that right now.
Comment 32 Victor Grousset/tuxayo 2026-01-12 10:58:05 UTC
(In reply to Jonathan Druart from comment #31)
> > No POD for subs and the new module.
> Fixed in the last patch.

Thanks ^^


> > for_biblio() isn't covered by tests (I mean actual code coverage, not the
> > weird error message we get in some cases)
> 
> Added a test.

Coverage good! :)


>> Turns out the tests fail if one happens to have started KTD with UNIMARC
>> data.
>> 
>> And if I go to the UI and click audit. [...]
> Looks like incorrect data, it's a misc4dev's problem.

Indeed, I can just create the missing values in the UI.

But looking at the full test failures, and even when I create the missing values in TYPEDOC and LOC, there is also this:
        #   Failed test at t/db_dependent/Koha/Database/DataInconsistency.t line 309.
        #     Structures begin differing at:
        #          $got->[0] = 'Biblionumber 21655 has '@' in 001$@'
        #     $expected->[0] = 'Biblionumber 21655 has '-42' in 001$@'

        #   Failed test at t/db_dependent/Koha/Database/DataInconsistency.t line 324.
        #     Structures begin differing at:
        #          $got->[0] = 'Biblionumber 21655 has '@' in 001$@'
        #     $expected->[0] = 'Biblionumber 21655 has '-42' in 001$@'
        # Looks like you failed 2 tests of 2.

    #   Failed test 'ids_not_in_marc'

and

#   Failed test 'no warnings'
#   at /usr/share/perl/5.36/Test/Builder.pm line 193.
# There were 3 warning(s)
#     Previous test 0 ''
#     Argument "@" isn't numeric in numeric ne (!=) at /kohadevbox/koha/Koha/Database/DataInconsistency.pm line 235.
#  at /kohadevbox/koha/Koha/Database/DataInconsistency.pm line 235.



--------


There is something else weird. Still on UNIMARC, 099$t is by default TYPEDOC, which has no value. So the audit complains it can't find LIVR for example (many record have this value).
- Now if I change the framework to map to itemtype. 
- misc/batchRebuildBiblioTables.pl
- restart services
- the audit will stop complaining about LIVR not existing
- but it still doesn't existing in itemtype
- (same result if I apply bug 38336 and start KTD)

Do the steps make sense? If so, that looks like the audit let the issue slip through.
Comment 33 Jonathan Druart 2026-01-12 11:58:12 UTC
I don't know if they make sense, but the tests are not supposed to pass with UNIMARC and I am not going to investigate why they fail.

Re TYPEDOC and LIVR the behaviour with this patch should be the same as before. Do you see a change in the behaviour or was it the same on main?
Comment 34 Victor Grousset/tuxayo 2026-01-18 14:44:04 UTC
Created attachment 191603 [details] [review]
Bug 40777: Introduce Koha::Database::DataInconsistency

test plan:
1. Export a record with an item from the staff interface as MARCXML (Save > MARCXML).
2. Edit the file and remove the 952$a (home library) and 952$b (current library).
3. Delete the item and record in the staff interface.
4. Import the exported record and item from the MARCXML file:
   - Cataloging > Import > Stage for import
     . Settings > Format: MARCXML
   - Stage for import
   - View batch link on the job details page (assuming successfully staged)
     . New record framework: change to Books, Booklets, Workbooks
5. Turn on the browser developer tools and view the console.
6. Access the details page for the new record.
7. Note that you get:
   - A "Processing" dialog box on screen
   - A browser pop-up window/dialog box with:
       Something went wrong when loading the table.
       500: Internal server error
   - Click OK
   - The console shows:
      . Failed to load resource: the server responded with a status of 500 (Internal Server Error)
      . Got 500 (Internal Server Error)
      . DataTables warning: table id=holdings_table - Ajax error. For more information about this error, please see https://datatables.net/tn/7
8. Run misc/maintenance/search_for_data_inconsistencies.pl, you get these result (the itemnumber may be different):
    == Not defined items.homebranch and/or items.holdingbranch ==
    	* Item with itemnumber=973 does not have home and holding library defined
    => Edit these items and set valid homebranch and/or holdingbranch
    (↑↑ don't do it, it's part of the message content)
9. Apply the patch.
10. Restart everything.
11. Refresh the details screen for the imported record.
12. You now get the same as step 7, but with added "Have a look at the "Audit" button in the toolbar" text in the browser pop-up dialog box:
    - A "Processing" dialog box on screen
    - A browser pop-up window/dialog box with:
        Something went wrong when loading the table.
        500: Internal server error
        Have a look at the "Audit" button in the toolbar
    - Click OK
    - The console shows:
      . GET http[URL for the record....] 500 (Internal Server Error)
      . Got 500 (Internal Server Error)
      . DataTables warning: table id=holdings_table - Ajax error. For more information about this error, please see https://datatables.net/tn/7
13. Click on the Audit button.
14. You get a yellow box at the top of the record with:
      Errors found
      Item with itemnumber=973 does not have home and holding library defined
    (Note that there is still the "Processing" dialog box, you can edit the item by select the "Items" option in the sidebar menu and then edit the item details - the home and current library files will be auto-populated with the current library, Centerville with KTD)
15. Repeat step 8, you get the same results.
16. The tests pass: prove t/db_dependent/Koha/Database/DataInconsistency.t
17. Check the details page for another record that doesn't have any data issues - note the 'Audit' toolbar item, and if clicked should show a blue box at the top of the record with "No data inconsistency found."

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 35 Victor Grousset/tuxayo 2026-01-18 14:44:07 UTC
Created attachment 191604 [details] [review]
Bug 40777: Add 'Audit' button in the toolbar

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 36 Victor Grousset/tuxayo 2026-01-18 14:44:10 UTC
Created attachment 191605 [details] [review]
Bug 40777: Add info in the alert

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 37 Victor Grousset/tuxayo 2026-01-18 14:44:13 UTC
Created attachment 191606 [details] [review]
Bug 40777: Move all biblio tests to the new package

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 38 Victor Grousset/tuxayo 2026-01-18 14:44:16 UTC
Created attachment 191607 [details] [review]
Bug 40777: Add tests for invalid_item_library and no_item_type

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 39 Victor Grousset/tuxayo 2026-01-18 14:44:19 UTC
Created attachment 191608 [details] [review]
Bug 40777: Add tests for invalid_item_type

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 40 Victor Grousset/tuxayo 2026-01-18 14:44:23 UTC
Created attachment 191609 [details] [review]
Bug 40777: Add tests for errors_in_marc

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 41 Victor Grousset/tuxayo 2026-01-18 14:44:26 UTC
Created attachment 191610 [details] [review]
Bug 40777: Add tests for nonexistent_AV

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 42 Victor Grousset/tuxayo 2026-01-18 14:44:29 UTC
Created attachment 191611 [details] [review]
Bug 40777: Add tests for empty_title

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 43 Victor Grousset/tuxayo 2026-01-18 14:44:32 UTC
Created attachment 191612 [details] [review]
Bug 40777: Flatten arrays

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 44 Victor Grousset/tuxayo 2026-01-18 14:44:35 UTC
Created attachment 191613 [details] [review]
Bug 40777: Add tests for for_biblio

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 45 Victor Grousset/tuxayo 2026-01-18 14:44:38 UTC
Created attachment 191614 [details] [review]
Bug 40777: Add POD

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 46 Victor Grousset/tuxayo 2026-01-18 14:44:41 UTC
Created attachment 191615 [details] [review]
Bug 40777: (QA follow-up): remove trailing space from translatable string

Otherwise it ends up as "Audit " in the .po
Comment 47 Victor Grousset/tuxayo 2026-01-18 15:03:12 UTC
> the tests are not supposed to pass with UNIMARC

No worries about the tests then. I looked at the failure and it seems it's the test code that somehow puts a @. The business code gets it back so it does it's job even in UNIMARC.

> Re TYPEDOC and LIVR the behavior with this patch should be the same as before. Do you see a change in the behavior or was it the same on main?

From what I understand from above, I got the audit to pass even though my record 099$t value doesn't exist in itemtypes table. I can't compare vs main, it's the new audit.
Wait! it's not new, it's originally from search_for_data_inconsistencies.pl

Ok it's not from this patch. search_for_data_inconsistencies.pl can also be fooled.
Including in MARC21 if I just change in the framework the authorized values from itemtypes to TYPEDOC (or any auth value category) which have not any values in MARC21 sample data.

Having the same bugs is actually a good sign the extraction of business logic didn't mess things up ^^ at least in the scope of my whack tests.

----

Thanks David for the test plan :)

----

Works, makes sense, QA script happy, code looks good, passing QA :)
Comment 48 Victor Grousset/tuxayo 2026-01-18 15:13:03 UTC
QA note: the strings are in Koha/Database/DataInconsistency.pm but I don't see an obvious way without duplicating them between the cron the and web UI.

Feature still works with just "catalogue" permission.