Bug 29307

Summary: marc record fetched twice in detail.pl
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: RESOLVED DUPLICATE QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: david, dcook, joonas.kylmala
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29697
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 29486    
Bug Blocks:    
Attachments: Bug 29307: Remove unecessary marc record fetch on detail.pl
Bug 29307: Remove unecessary marc record fetch on detail.pl
Bug 29307: Remove unecessary marc record fetch on detail.pl

Description Jonathan Druart 2021-10-22 09:41:17 UTC
86 my $record       = GetMarcBiblio({ biblionumber => $biblionumber });
 87 my $biblio = Koha::Biblios->find( $biblionumber );
 88 $template->param( 'biblio', $biblio );
 89 
 90 if ( not defined $record ) {
 91     # biblionumber invalid -> report and exit
 92     $template->param( unknownbiblionumber => 1,
 93                       biblionumber => $biblionumber );
 94     output_html_with_http_headers $query, $cookie, $template->output;
 95     exit;
 96 }
 97 
 98 eval { $biblio->metadata->record };
 99 $template->param( decoding_error => $@ );
Comment 1 Jonathan Druart 2021-10-22 11:43:27 UTC
Created attachment 126749 [details] [review]
Bug 29307: Remove unecessary marc record fetch on detail.pl

We were fetching the MARC::Record twice here.
Comment 2 David Nind 2021-10-23 20:47:22 UTC
Created attachment 126791 [details] [review]
Bug 29307: Remove unecessary marc record fetch on detail.pl

We were fetching the MARC::Record twice here.

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 David Nind 2021-10-23 20:48:48 UTC
Tested by checking that the MARC detail pages in the staff interface work as expected after the patch is applied.
Comment 4 Joonas Kylmälä 2021-10-31 11:07:45 UTC
They don't do quite the same thing, GetMarcBiblio calls C4::Biblio::_koha_marc_update_bib_ids() and $biblio->metadata->record doesn't.
Comment 5 Jonathan Druart 2021-11-02 12:05:28 UTC
(In reply to Joonas Kylmälä from comment #4)
> They don't do quite the same thing, GetMarcBiblio calls
> C4::Biblio::_koha_marc_update_bib_ids() and $biblio->metadata->record
> doesn't.

What differences do you expect?

The point of those tests is to not explode later basically, so we must know if the record has a valid MARCXML stored in the biblio_metadata table.
Comment 6 Joonas Kylmälä 2021-11-06 16:32:28 UTC
(In reply to Jonathan Druart from comment #5)
> (In reply to Joonas Kylmälä from comment #4)
> > They don't do quite the same thing, GetMarcBiblio calls
> > C4::Biblio::_koha_marc_update_bib_ids() and $biblio->metadata->record
> > doesn't.
> 
> What differences do you expect?

Commenting on this just based on the code I read: Before this patch the detail page included biblionumbers if they were not in the original data, after this patch it doesn't.
Comment 7 Jonathan Druart 2021-11-15 15:06:18 UTC
(In reply to Joonas Kylmälä from comment #6)
> (In reply to Jonathan Druart from comment #5)
> > (In reply to Joonas Kylmälä from comment #4)
> > > They don't do quite the same thing, GetMarcBiblio calls
> > > C4::Biblio::_koha_marc_update_bib_ids() and $biblio->metadata->record
> > > doesn't.
> > 
> > What differences do you expect?
> 
> Commenting on this just based on the code I read: Before this patch the
> detail page included biblionumbers if they were not in the original data,
> after this patch it doesn't.

Yes, I don't think this call in GetMarcBiblio is relevant, I opened bug 29486.
Comment 8 Jonathan Druart 2022-03-18 09:37:09 UTC
Created attachment 131864 [details] [review]
Bug 29307: Remove unecessary marc record fetch on detail.pl

We were fetching the MARC::Record twice here.

Signed-off-by: David Nind <david@davidnind.com>
Comment 9 Jonathan Druart 2022-03-18 10:31:12 UTC
Will be fixed by bug 29697.

*** This bug has been marked as a duplicate of bug 29697 ***