[Koha-bugs] [Bug 14048] Change RefundLostItemFeeOnReturn to be branch specific

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14048

--- Comment #76 from Jonathan Druart <jonathan.druart at 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.