Bug 35117 - get_marc_components and get_marc_volumes should be more consistent
Summary: get_marc_components and get_marc_volumes should be more consistent
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Martin Renvoize (ashimema)
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 35099
Blocks:
  Show dependency treegraph
 
Reported: 2023-10-20 13:30 UTC by Martin Renvoize (ashimema)
Modified: 2024-10-18 09:27 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 35117: Improve consistency (3.25 KB, patch)
2023-10-20 13:46 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35117: Improve consistency (3.30 KB, patch)
2024-06-02 21:33 UTC, David Nind
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize (ashimema) 2023-10-20 13:30:17 UTC
Bug 35099 highlighted a number of inconsistencies between these two parallel methods.. we should bring them closer.
Comment 1 Martin Renvoize (ashimema) 2023-10-20 13:46:21 UTC
Created attachment 157516 [details] [review]
Bug 35117: Improve consistency

This patch improves the consistency between get_components_query and
get_get_volumes_query methods so they both check for MARC21.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 2 Martin Renvoize (ashimema) 2023-10-20 13:47:24 UTC
No real test plan as such.. just brings a little more consistency between these two parallel/similar sets of code.
Comment 3 David Nind 2024-06-02 21:33:35 UTC
Created attachment 167327 [details] [review]
Bug 35117: Improve consistency

This patch improves the consistency between get_components_query and
get_get_volumes_query methods so they both check for MARC21.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2024-06-02 21:34:39 UTC
I have no idea about the actual code, but the patch still applies and all the tests pass.

prove t/*
.....
All tests successful.
Files=200, Tests=9395, 497 wallclock secs ( 1.81 usr  0.39 sys + 1068.18 cusr 200.62 csys = 1271.00 CPU)
Result: PASS
Comment 5 Martin Renvoize (ashimema) 2024-08-21 08:51:33 UTC
I can't for the life of me remember what I was doing here at this point :(.
Comment 6 Marcel de Rooy 2024-08-21 11:09:49 UTC
(In reply to Martin Renvoize from comment #5)
> I can't for the life of me remember what I was doing here at this point :(.

LOL
Comment 7 Marcel de Rooy 2024-10-18 09:27:11 UTC
What about 001 not found when pref enabled?
    if ( C4::Context->preference('UseControlNumber') ) {
        my $pf001 = $marc->field('001') || undef;
Then you go to:
my ( $error, $query, $query_str ) = $builder->build_query_compat( undef, [$searchstr], undef, undef, [$sort], 0 );
while $searchstr is undefined..
We should probably not look for [ undef ] ?
In that unusual case, we should be looking for the title, right? So the 001 defined test should go to the toplevel if ?