Bug 19040 - Change prototype of C4::Biblio::GetMarcBiblio
Summary: Change prototype of C4::Biblio::GetMarcBiblio
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: M. Tompsett
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 14385
  Show dependency treegraph
 
Reported: 2017-08-04 14:43 UTC by M. Tompsett
Modified: 2017-12-07 22:24 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 19040: Refactor GetMarcBiblio parameters (72.89 KB, patch)
2017-08-07 03:10 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 19040: print STDERRs to develop test plan (50.70 KB, patch)
2017-08-09 02:28 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 19040: Refactor GetMarcBiblio parameters (72.89 KB, patch)
2017-08-16 14:46 UTC, M. Tompsett
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 19040: Refactor GetMarcBiblio parameters (72.96 KB, patch)
2017-08-17 11:46 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 19040: Refactor GetMarcBiblio parameters (73.12 KB, patch)
2017-08-25 07:45 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19040: [QA Follow-up] Adjust embed_items parameter in showdiffmarc (970 bytes, patch)
2017-08-25 07:45 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19040: Update 2 occurrences of GetMarcBiblio in 22_to_30 (1.66 KB, patch)
2017-08-25 13:35 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 M. Tompsett 2017-08-04 14:43:10 UTC
Because adding parameters to pass to called subroutines is difficult, change parameters to be a hashref.
Comment 1 M. Tompsett 2017-08-07 03:10:22 UTC
Created attachment 65531 [details] [review]
Bug 19040: Refactor GetMarcBiblio parameters

Change parameters to a hashref.
Test plan to come later.
Comment 2 M. Tompsett 2017-08-09 02:28:59 UTC
Created attachment 65695 [details] [review]
Bug 19040: print STDERRs to develop test plan
Comment 3 M. Tompsett 2017-08-11 01:01:56 UTC
TEST PLAN
---------
perldoc C4::Biblio
prove t/Biblio.t
prove t/db_dependent/Acquisition/OrderFromSubscription.t
prove t/db_dependent/AdditionalField.t
prove t/db_dependent/Bookseller.t
prove t/db_dependent/Letters.t
prove t/db_dependent/Serials/Claims.t
prove t/db_dependent/Serials_2.t
prove t/db_dependent/Authorities/Merge.t
prove t/db_dependent/Biblio.t
prove t/db_dependent/Exporter/Record.t
prove t/db_dependent/Record/marcrecord2csv.t
prove t/db_dependent/HoldsQueue.t
prove t/db_dependent/Items.t
prove t/db_dependent/Koha/Filter/EmbedItemsAvailability.t
prove t/db_dependent/OAI/Sets.t
prove t/db_dependent/Koha/BiblioUtils.t
prove t/db_dependent/OAI/Server.t
prove t/db_dependent/Reserves/MultiplePerRecord.t
prove t/db_dependent/ShelfBrowser.t
run the koha qa test tools -- if the changes have a typo, they'll fail.
eyeball each change:
- have the parameters changed? That is, should biblionumber, embeditems, and opac be the same expected values as before?
Comment 4 M. Tompsett 2017-08-11 01:02:30 UTC
Comment on attachment 65695 [details] [review]
Bug 19040: print STDERRs to develop test plan

Not needed to push.
Comment 5 M. Tompsett 2017-08-16 14:46:19 UTC
Created attachment 66101 [details] [review]
Bug 19040: Refactor GetMarcBiblio parameters

Change parameters to a hashref.

Rebased: 2017-08-16
Comment 6 Josef Moravec 2017-08-17 11:46:00 UTC
Created attachment 66145 [details] [review]
[SIGNED-OFF] Bug 19040: Refactor GetMarcBiblio parameters

Change parameters to a hashref.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 7 Marcel de Rooy 2017-08-25 07:45:46 UTC
Created attachment 66457 [details] [review]
Bug 19040: Refactor GetMarcBiblio parameters

Change parameters to a hashref.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Looks good to me.
Two calls in migration_tools/22_to_30 still in old style.
Comment 8 Marcel de Rooy 2017-08-25 07:45:51 UTC
Created attachment 66458 [details] [review]
Bug 19040: [QA Follow-up] Adjust embed_items parameter in showdiffmarc

Trivial adjustment

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Marcel de Rooy 2017-08-25 07:47:12 UTC
carp 'GetMarcBiblio called without parameters';
Should be a Koha::Exception.
See coding guidelines.

git grep -E "GetMarcBiblio\s*\(\s*\$"
No results as expected
git grep -E "GetMarcBiblio\s*\(\s*[^{]"
misc/migration_tools/22_to_30/missing090field.pl: my $record = GetMarcBiblio($biblionumber);
misc/migration_tools/22_to_30/rebuild_unimarc_100.pl:    my $record = GetMarcBiblio($biblionumber);
[OK] t/Biblio.t:warning_is { $ret = GetMarcBiblio() }

Who is going to run these scripts still? And from what version? Should we still have in master ??
Comment 10 Jonathan Druart 2017-08-25 13:23:41 UTC
(In reply to Marcel de Rooy from comment #9)
> Who is going to run these scripts still? And from what version? Should we
> still have in master ??

People upgrading from Koha 2.2?
Comment 11 Jonathan Druart 2017-08-25 13:35:21 UTC
Created attachment 66477 [details] [review]
Bug 19040: Update 2 occurrences of GetMarcBiblio in 22_to_30
Comment 12 Jonathan Druart 2017-08-25 14:02:03 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 13 M. Tompsett 2017-08-25 17:08:03 UTC
(In reply to Jonathan Druart from comment #10)
> (In reply to Marcel de Rooy from comment #9)
> > Who is going to run these scripts still? And from what version? Should we
> > still have in master ??
> 
> People upgrading from Koha 2.2?

People upgrading from 2.2 to current are likely to stop at 3.4 before proceeding to master due to items being stored differently, at least that was my impression. That's why I didn't change those two scripts. But I suppose it can't hurt.
Comment 14 Marcel de Rooy 2017-08-25 19:46:23 UTC
(In reply to Jonathan Druart from comment #10)
> (In reply to Marcel de Rooy from comment #9)
> > Who is going to run these scripts still? And from what version? Should we
> > still have in master ??
> 
> People upgrading from Koha 2.2?

Sure? :)
My impression was that you would call 22_to_30 from 3.0.
So we should not be needing them in master?
Comment 15 Jonathan Druart 2017-08-25 20:49:32 UTC
Then let's remove them :)
Comment 16 Fridolin SOMERS 2017-09-29 11:27:19 UTC
Enhancement not pushed to 17.05.x