Bug 29307 - marc record fetched twice in detail.pl
Summary: marc record fetched twice in detail.pl
Status: RESOLVED DUPLICATE of bug 29697
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 29486
Blocks:
  Show dependency treegraph
 
Reported: 2021-10-22 09:41 UTC by Jonathan Druart
Modified: 2022-03-18 10:31 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:


Attachments
Bug 29307: Remove unecessary marc record fetch on detail.pl (1.32 KB, patch)
2021-10-22 11:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29307: Remove unecessary marc record fetch on detail.pl (1.37 KB, patch)
2021-10-23 20:47 UTC, David Nind
Details | Diff | Splinter Review
Bug 29307: Remove unecessary marc record fetch on detail.pl (1.12 KB, patch)
2022-03-18 09:37 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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 ***