Bug 36917

Summary: Many warnings from t/db_dependent/Authority/Merge.t
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Test SuiteAssignee: Jonathan Druart <jonathan.druart>
Status: RESOLVED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: minor    
Priority: P5 - low CC: caroline.cyr-la-rose, fridolin.somers, m.de.rooy, nick
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00
Circulation function:
Bug Depends on: 30047    
Bug Blocks: 25515    
Attachments: Bug 36917: Remove some warnings from t/db_dependent/Authority/Merge.t
Bug 36917: Remove some warnings from t/db_dependent/Authority/Merge.t
Bug 36917: Remove some warnings from t/db_dependent/Authority/Merge.t
Bug 36917: (follow-up) Remove Elasticsearch warnings and tidy
Bug 36917: (follow-up) Add FIXME for marcflavour
Bug 36917: (follow-up) Remove Elasticsearch warnings and tidy
Bug 36917: (follow-up) Add FIXME for marcflavour
Bug 36917: (follow-up) Remove Elasticsearch warnings and tidy
Bug 36917: (follow-up) Add FIXME for marcflavour

Description Jonathan Druart 2024-05-22 08:54:49 UTC
Cannot determine authority type for record: 1709 at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch.pm line 589.                                                                                                    
Use of uninitialized value $subfields in pattern match (m//) at /kohadevbox/koha/C4/Heading/MARC21.pm line 412.                                                                                                      
Use of uninitialized value $subfields in pattern match (m//) at /kohadevbox/koha/C4/Heading/MARC21.pm line 448.
Comment 1 Jonathan Druart 2024-05-22 08:55:59 UTC
I think this is because tag 109 is not defined in C4::Heading::MARC21::$auth_heading_fields

What is 109 here?
Comment 2 Jonathan Druart 2024-05-22 08:57:37 UTC
Created attachment 167012 [details] [review]
Bug 36917: Remove some warnings from t/db_dependent/Authority/Merge.t

Cannot determine authority type for record: 1709 at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch.pm line 589.
Use of uninitialized value $subfields in pattern match (m//) at /kohadevbox/koha/C4/Heading/MARC21.pm line 412.
Use of uninitialized value $subfields in pattern match (m//) at /kohadevbox/koha/C4/Heading/MARC21.pm line 448.

This change removes the 2 "Use of uninitialized value $subfields in
pattern match (m//)" warnings

To be honest I am not really sure what I am doing here but this looks
safe enough.

Can one of you confirm?
Comment 3 Marcel de Rooy 2024-05-22 11:12:42 UTC
(In reply to Jonathan Druart from comment #2)
> Can one of you confirm?

Will have a look.
Note that this test ran fine for years so there must be some new trigger ?
Comment 4 Nick Clemens (kidclamp) 2024-05-22 11:29:08 UTC
Probably storing the heading from bug 30047
Comment 5 Marcel de Rooy 2024-05-22 11:32:55 UTC
This might be an issue:

return {} if !$authtype;    # very exceptional
Comment 6 Marcel de Rooy 2024-05-22 11:37:54 UTC
Created attachment 167023 [details] [review]
Bug 36917: Remove some warnings from t/db_dependent/Authority/Merge.t

Cannot determine authority type for record: 1709 at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch.pm line 589.
Use of uninitialized value $subfields in pattern match (m//) at /kohadevbox/koha/C4/Heading/MARC21.pm line 412.
Use of uninitialized value $subfields in pattern match (m//) at /kohadevbox/koha/C4/Heading/MARC21.pm line 448.

This change removes the 2 "Use of uninitialized value $subfields in
pattern match (m//)" warnings

To be honest I am not really sure what I am doing here but this looks
safe enough.

Can one of you confirm?

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Nick Clemens (kidclamp) 2024-05-22 12:02:53 UTC
Created attachment 167025 [details] [review]
Bug 36917: Remove some warnings from t/db_dependent/Authority/Merge.t

Cannot determine authority type for record: 1709 at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch.pm line 589.
Use of uninitialized value $subfields in pattern match (m//) at /kohadevbox/koha/C4/Heading/MARC21.pm line 412.
Use of uninitialized value $subfields in pattern match (m//) at /kohadevbox/koha/C4/Heading/MARC21.pm line 448.

This change removes the 2 "Use of uninitialized value $subfields in
pattern match (m//)" warnings

The warnings are gnerated because the subfields for headings are hardcoded variables in C4::Heading::MARC21
for any genuine authority record we should receive the correct list of subfields. The tests in this case are
mocking a new authtype, and so do not find the correct subfields. Skipping the generation here silences the
warnings and will prevent user defined types from throwing errors as well.

NOTE: Koha allows the user to define their own authority types, however, we have various features hardcoded
that will prevent them from working as expected

WNC amended commit message

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 8 Nick Clemens (kidclamp) 2024-05-22 12:02:55 UTC
Created attachment 167026 [details] [review]
Bug 36917: (follow-up) Remove Elasticsearch warnings and tidy

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 9 Nick Clemens (kidclamp) 2024-05-22 12:02:57 UTC
Created attachment 167027 [details] [review]
Bug 36917: (follow-up) Add FIXME for marcflavour

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 10 Nick Clemens (kidclamp) 2024-05-22 12:03:20 UTC
Back to you to review my follow-ups Joubu
Comment 11 Jonathan Druart 2024-05-22 13:05:27 UTC
Created attachment 167033 [details] [review]
Bug 36917: (follow-up) Remove Elasticsearch warnings and tidy

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 12 Jonathan Druart 2024-05-22 13:05:30 UTC
Created attachment 167034 [details] [review]
Bug 36917: (follow-up) Add FIXME for marcflavour

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Jonathan Druart 2024-05-22 13:05:45 UTC
(In reply to Nick Clemens (kidclamp) from comment #10)
> Back to you to review my follow-ups Joubu

Done, thanks!
Comment 14 Marcel de Rooy 2024-05-24 09:27:32 UTC
Created attachment 167154 [details] [review]
Bug 36917: (follow-up) Remove Elasticsearch warnings and tidy

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 15 Marcel de Rooy 2024-05-24 09:27:34 UTC
Created attachment 167155 [details] [review]
Bug 36917: (follow-up) Add FIXME for marcflavour

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 16 Katrin Fischer 2024-05-24 13:40:23 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 17 Fridolin Somers 2024-05-30 08:10:55 UTC
Depends on Bug 30047 not in 23.11.x
Comment 18 Caroline Cyr La Rose 2024-07-04 18:36:02 UTC
Test suite changes don't induce changes to the manual afaik. Closing, but feel free to reopen if the status was put there to document something in the coding guidelines