Koha::RecordProcessor is designed and documented to accept more than one Koha::Filter as parameter. The current code only support one filter to be passed. Regression test coming-
Created attachment 41388 [details] [review] Bug 14646: (regression test) Koha::RecordProcessor only accepts one filter at a time This patch introduces new tests to t/RecordProcessor.t so it tests for creating processors with more than one filter. It does so by running my $processor = new Koha::RecordProcessor({ filters => ['Null','Dummy'] }); and testing the results. To test: - Apply the patch - Run: $ prove t/RecordProcessor.t => FAIL: tests related to multiple filters fail.
Created attachment 41389 [details] [review] Bug 14646: Koha::RecordProcessor should accept more than one filter The docs say that Koha::RecordProcessor accepts more than one filter at a time. But as the regression tests show it doesn't. This is really important to extend its usage in record processing (either to enforce policy, transform, etc). This patch makes ->new evaluate the reference type of the passed filters and builds an array with a single item if a scalar has been passed. The loop now explicitly casts the filters as an array. To test: - Run $ prove t/RecordProcessor.t => FAIL: tests fail because Koha::RecordProcessor doesn't hanlde more than one filter at a time. - Apply the patch - Run $ prove t/RecordProcessor.t => SUCCESS: tests now pass - Easy, right? Sign off :-D
Created attachment 41391 [details] [review] Bug 14646: (regression test) Koha::RecordProcessor only accepts one filter at a time This patch introduces new tests to t/RecordProcessor.t so it tests for creating processors with more than one filter. It does so by running my $processor = new Koha::RecordProcessor({ filters => ['Null','Dummy'] }); and testing the results. To test: - Apply the patch - Run: $ prove t/RecordProcessor.t => FAIL: tests related to multiple filters fail. Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 41392 [details] [review] Bug 14646: Koha::RecordProcessor should accept more than one filter The docs say that Koha::RecordProcessor accepts more than one filter at a time. But as the regression tests show it doesn't. This is really important to extend its usage in record processing (either to enforce policy, transform, etc). This patch makes ->new evaluate the reference type of the passed filters and builds an array with a single item if a scalar has been passed. The loop now explicitly casts the filters as an array. To test: - Apply the test patch - Run $ prove t/RecordProcessor.t => FAIL: tests fail because Koha::RecordProcessor doesn't hanlde more than one filter at a time. - Apply this patch - Run $ prove t/RecordProcessor.t => SUCCESS: tests now pass - Easy, right? Sign off :-D NOTE: Read code. Don't like the ? operator logic, but it is functional despite readability issues. Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 41655 [details] [review] Bug 14646: (regression test) Koha::RecordProcessor only accepts one filter at a time This patch introduces new tests to t/RecordProcessor.t so it tests for creating processors with more than one filter. It does so by running my $processor = new Koha::RecordProcessor({ filters => ['Null','Dummy'] }); and testing the results. To test: - Apply the patch - Run: $ prove t/RecordProcessor.t => FAIL: tests related to multiple filters fail. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 41656 [details] [review] Bug 14646: Koha::RecordProcessor should accept more than one filter The docs say that Koha::RecordProcessor accepts more than one filter at a time. But as the regression tests show it doesn't. This is really important to extend its usage in record processing (either to enforce policy, transform, etc). This patch makes ->new evaluate the reference type of the passed filters and builds an array with a single item if a scalar has been passed. The loop now explicitly casts the filters as an array. To test: - Apply the test patch - Run $ prove t/RecordProcessor.t => FAIL: tests fail because Koha::RecordProcessor doesn't hanlde more than one filter at a time. - Apply this patch - Run $ prove t/RecordProcessor.t => SUCCESS: tests now pass - Easy, right? Sign off :-D NOTE: Read code. Don't like the ? operator logic, but it is functional despite readability issues. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(In reply to M. Tompsett from comment #4) > NOTE: Read code. Don't like the ? operator logic, but it is > functional despite readability issues. Something better to suggest?
(In reply to Jonathan Druart from comment #7) > (In reply to M. Tompsett from comment #4) > > NOTE: Read code. Don't like the ? operator logic, but it is > > functional despite readability issues. > > Something better to suggest? As noted it is merely readability issues. I do not have anything better to suggest.
Patches pushed to master.