https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14048
--- Comment #76 from Jonathan Druart <[email protected]> --- (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. _______________________________________________ Koha-bugs mailing list [email protected] http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
