https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14048 --- Comment #76 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Tomás Cohen Arazi from comment #73)

(In reply to Jonathan Druart from comment #72)

...

I have squashed the patches for the review, so I won't use the splinter:

1/ my $item_holding_branch = $item->{ holdingbranch }; => would be good to have a comment here to explain the last patch.

I thought the commit message would be enough. Do u require a comment on the code?

Would be good, but not required. Forget it

...

2/ Koha::Config::SysPrefs->find should not be used, C4::Context->preference should continue to be used everywhere, to take advantage of the syspref cache.

Ok, followup coming.

I added one also to fix the tests.

...

3/ $schema->resultset('RefundLostItemFeeRule')->search()->delete; should use Koha::RefundLostItemFeeRules

This is done on the tests, instead of the usual DELETE FROM ... The problem with Koha::RefundLostItemFeeRules is that it explicitly forbis deleting the default rule. I'd leave it as it is.

Correct, and I was wondering if it would not be easier to assume that default is 1 if no rule exist. It will permit to remove the sql file in the mysql/mandatory dir (and then no need to manage it from the installer) and no need to overwrite delete. Why do you think?

...

4/ Tests only cover RefundLostOnReturnControl = 'CheckinLibrary'

This is incorrect:

subtest 'Koha::RefundLostItemFeeRules::_choose_branch() tests' => Tries the three possible values

subtest 'Koha::RefundLostItemFeeRules::should_refund() tests' => Tries the three possible values

-- You are receiving this mail because: You are watching all bug changes.