https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28924 --- Comment #52 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Forgot David's sign off lines first time round (In reply to Nick Clemens (kidclamp) from comment #29)
Patch doesn't apply, please rebase
Rebased
From reading the code: is_patron_inside_charge_limits checks for no issues charge limits like category/pref/defaults to 0 then you test if the charges are defined - but they always will be, because default 0
Looks like I was confusing a falsy value with an undefined value - good spot. I've amended this now to not set the default
CanBookBeIssued then also checks if they look like a number, and they always will because defaulted to 0 - before these patches leaving the pref blank means it isn't active (except noissuescharge) I will attach a test I think will fail after applying, check if I am right there.
As above
The two new routines in Patron.pm both take borrower arguments - is there a reason not to simply use 'self'?
They should definitely be using $self, not sure what I was doing there
circ/circulation.pl line 590 You add 'noissues=>1' without checking the fines override pref - is this correct?
Don't think that should be there, have removed it
The new GetPatronCharges routine in the Categories template plugin seems misplaced - I think we should be using your new routines directly from the patron object in the templates - it looks like there are other variables related to charges in the scripts - so we should try to avoid double work here and pass more info forward if we have already fetched it.
I've removed the method and passed the data through. The values already existed in the template so just needed adding into the inc files
I don't think the tests are covering all cases and should be expanded a bit.
Can you give an example of a case you think is missing?
This is just a quick pass, so let me know if anything needs clarification
-- You are receiving this mail because: You are watching all bug changes.