Bug 15777 - Refactor loop in which Record::Processor does not initialize parameters
Summary: Refactor loop in which Record::Processor does not initialize parameters
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Mark Tompsett
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 10589 11592 15870 15871
  Show dependency treegraph
 
Reported: 2016-02-10 01:13 UTC by Mark Tompsett
Modified: 2017-06-14 22:11 UTC (History)
3 users (show)

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


Attachments
Bug 15777 - Test to prove initialization isn't working correctly. (1.68 KB, patch)
2016-02-12 04:21 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 15777 - Fix base class to prove refactor and fix required (951 bytes, patch)
2016-02-12 04:25 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 15777 - Refactor and fix the RecordProcessor class (1.45 KB, patch)
2016-02-12 04:32 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 15777 - Test to prove initialization isn't working correctly. (1.82 KB, patch)
2016-02-12 14:25 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15777 - Fix base class to prove refactor and fix required (1.07 KB, patch)
2016-02-12 14:25 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15777 - Refactor and fix the RecordProcessor class (1.59 KB, patch)
2016-02-12 14:26 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15777 - Test to prove initialization isn't working correctly. (1.87 KB, patch)
2016-02-12 15:11 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 15777 - Fix base class to prove refactor and fix required (1.12 KB, patch)
2016-02-12 15:11 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 15777 - Refactor and fix the RecordProcessor class (1.64 KB, patch)
2016-02-12 15:11 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 15777 - Refactor and fix the RecordProcessor class (1.64 KB, patch)
2016-02-12 15:58 UTC, Mark Tompsett
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Tompsett 2016-02-10 01:13:09 UTC
foreach my $filter (@{ $req_filters }) {
        next unless $filter;
        my $filter_module = $filter =~ m/:/ ? $filter : "Koha::Filter::${schema}::${filter}";
        if (can_load( modules => { $filter_module => undef } )) {
            my $object = $filter_module->new();
            $filter_module->initialize($param);
            push @filters, $object;
        }
    }

"$filter_module->initialize($param);" should be
"$object->initialize($param);

Additionally, object isn't really descriptive, and it isn't clear that $filter is the string name of the filter. So, this loop will have some variable renaming done on it.

And the first commit will include a test that fails to initialize.
And the second commit will include this revision which should then pass.
Comment 1 Mark Tompsett 2016-02-12 04:21:45 UTC Comment hidden (obsolete)
Comment 2 Mark Tompsett 2016-02-12 04:25:59 UTC Comment hidden (obsolete)
Comment 3 Mark Tompsett 2016-02-12 04:32:43 UTC Comment hidden (obsolete)
Comment 4 Marc Véron 2016-02-12 14:25:06 UTC Comment hidden (obsolete)
Comment 5 Marc Véron 2016-02-12 14:25:52 UTC Comment hidden (obsolete)
Comment 6 Marc Véron 2016-02-12 14:26:59 UTC Comment hidden (obsolete)
Comment 7 Kyle M Hall (khall) 2016-02-12 15:11:28 UTC
Created attachment 47988 [details] [review]
Bug 15777 - Test to prove initialization isn't working correctly.

TEST PLAN
---------
1) Apply this testing patch
2) prove -v t/RecordProcessor.t
   -- tests will fail.
3) run koha qa test tools

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 8 Kyle M Hall (khall) 2016-02-12 15:11:36 UTC
Created attachment 47990 [details] [review]
Bug 15777 - Fix base class to prove refactor and fix required

TEST PLAN
---------
1) Apply the testing patch and this patch
2) prove -v t/RecordProcess.t
   -- will complain about string use with "use strict".
      This is because it is using the module name, instead
      of the object instantiated with new().
3) run koha qa test tools.

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 9 Kyle M Hall (khall) 2016-02-12 15:11:38 UTC Comment hidden (obsolete)
Comment 10 Mark Tompsett 2016-02-12 15:58:18 UTC
Created attachment 48000 [details] [review]
Bug 15777 - Refactor and fix the RecordProcessor class

TEST PLAN
---------
1) Apply all three patches
2) prove -v t/RecordProcessor.t
   -- all should be successful
3) run koha qa test tools

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 11 Mark Tompsett 2016-02-12 16:00:41 UTC
Sorry... git bz attach gone awry. Nothing changed.
Comment 12 Brendan Gallagher 2016-02-24 01:59:36 UTC
pushed to Master - Should be in the May 2016 Release.  Thanks
Comment 13 Julian Maurice 2016-02-26 15:31:43 UTC
Patches pushed to 3.22.x, will be in 3.22.4