Bug 26141 - Duplicated code in search.pl
Summary: Duplicated code in search.pl
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 12478
Blocks:
  Show dependency treegraph
 
Reported: 2020-08-05 07:57 UTC by Jonathan Druart
Modified: 2020-08-16 21:36 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.03


Attachments
Bug 26141: Remove duplicated code from search.pl (3.59 KB, patch)
2020-08-05 07:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 26141: Remove duplicated code from search.pl (3.66 KB, patch)
2020-08-05 11:12 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2020-08-05 07:57:52 UTC
commit 417553a9e47bf31f4cb785162e3dced856a2ac2a
  Bug 12478: starting to add search to staff client

This commit added code related to the itemtypes to the advanced search, but this code already existed.

The weird bit is:
$template->param(advancedsearchesloop => \@advancedsearchesloop);
my $types = C4::Context->preference("AdvancedSearchTypes") || "itemtypes";
my $advancedsearchesloop = prepare_adv_search_types($types);
$template->param(advancedsearchesloop => $advancedsearchesloop);

We are passing twice advancedsearchesloop to the template.
Comment 1 Jonathan Druart 2020-08-05 07:59:27 UTC
Created attachment 107820 [details] [review]
Bug 26141: Remove duplicated code from search.pl

commit 417553a9e47bf31f4cb785162e3dced856a2ac2a
  Bug 12478: starting to add search to staff client

This commit added code related to the itemtypes to the advanced search, but this code already existed.

The weird bit is:
$template->param(advancedsearchesloop => \@advancedsearchesloop);
my $types = C4::Context->preference("AdvancedSearchTypes") || "itemtypes";
my $advancedsearchesloop = prepare_adv_search_types($types);
$template->param(advancedsearchesloop => $advancedsearchesloop);

We are passing twice advancedsearchesloop to the template.

Test plan (for QA):
Review the changes and confirm that we can safely remove this code
Comment 2 Martin Renvoize 2020-08-05 11:12:32 UTC
Created attachment 107828 [details] [review]
Bug 26141: Remove duplicated code from search.pl

commit 417553a9e47bf31f4cb785162e3dced856a2ac2a
  Bug 12478: starting to add search to staff client

This commit added code related to the itemtypes to the advanced search, but this code already existed.

The weird bit is:
$template->param(advancedsearchesloop => \@advancedsearchesloop);
my $types = C4::Context->preference("AdvancedSearchTypes") || "itemtypes";
my $advancedsearchesloop = prepare_adv_search_types($types);
$template->param(advancedsearchesloop => $advancedsearchesloop);

We are passing twice advancedsearchesloop to the template.

Test plan (for QA):
Review the changes and confirm that we can safely remove this code

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 3 Martin Renvoize 2020-08-05 11:14:03 UTC
Another one for a straight QA.. I've compared the code and it is indeed a direct clone of the factored out routine directly below it (with a few variables renamed.. moreover the template variable is actually be overwritten by the second call to set it via the factored out return anyway.

Safe to remove, Passing QA
Comment 4 Jonathan Druart 2020-08-05 15:37:07 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 5 Lucas Gass 2020-08-13 18:05:29 UTC
backported to 20.05.x for 20.05.03
Comment 6 Aleisha Amohia 2020-08-16 21:36:24 UTC
enhancement, not backported to 19.11.x