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

Katrin Fischer <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Allow NoIssuesCharge to be  |Allow checkout fine limit
                   |determined by patron        |to be determined by patron
                   |category                    |category
             Status|Passed QA                   |Failed QA

--- Comment #66 from Katrin Fischer <[email protected]> ---
1) Is this correct?

         [% IF ( charges ) %]
-            [% INCLUDE 'blocked-fines.inc' borrowernumber =
patron.borrowernumber %]
+            [% INCLUDE 'blocked-fines.inc' NoIssuesCharge = chargesamount %]
         [% END %]

It looks like we are losing the borrowernumber? 

2) Terminology

https://wiki.koha-community.org/wiki/Terminology

a) +=head3 can_borrow
...
+This method determines whether a borrower is able to borrow based on various
parameters.
+- Debarrments

I feel like this should be "can_checkout" to follow our terminology guide and
because borrow/lend is a common one to get confused by non-native speakers.

Also some small stuff borrower = patron etc.

b) noissuecharge

The noissuecharge parameter is badly named and has always been. I feel like we
will never will fully recover from it as it has spread to other system
preferences by now, but I wonder if we could at least start to move into a
better direction with the new database table columns:

noissuescharge
noissueschargeguarantees
noissueschargeguarantorswithguarantees

I can see the consistency here, so this is more a question than a blocker.
I was thinking along the lines of checkout_charge_limit or so.

c) Templates

So this is a blocker for me, because I can see the consistency with the
codes/prefs, but I think we can do better in the templates and stick to the
agreed on terms:

+                    <th scope="col">No issues charge</th>
+                    <th scope="col">No issues charge guarantees</th>
+                    <th scope="col">No issues charge guarantors with
guarantees</th>

Also in the input form.

What about: 
Checkout charge limit... ? And maybe a good hint for the input form with
reference and link to noissuescharge and the other preferences if we want to
make that connection visible.

3) Don't include api/v1/swagger/swagger_bundle.json

It's in this patch:
Subject: [PATCH] Bug 28924: Add new columns to UI and controller

It will be automatically generated.

4) Translatability

This smells untranslatable:

-        my $noissuescharge = C4::Context->preference("noissuescharge") || 5;
-        $flaginfo{'message'} = sprintf 'Patron owes %.02f', $owing;
-        $flaginfo{'amount'}  = sprintf "%.02f", $owing;
-        if ( $owing > $noissuescharge &&
!C4::Context->preference("AllowFineOverride") ) {
+        my $noissuescharge = $patron_charge_limits->{noissuescharge}->{limit}
|| 5;
+        $flaginfo{'message'} = sprintf 'Patron owes %.02f',
$patron_charge_limits->{noissuescharge}->{charge};
+        $flaginfo{'amount'}  = sprintf "%.02f",
$patron_charge_limits->{noissuescharge}->{charge};

-            $flaginfo{'message'} = sprintf 'patron guarantees owe %.02f',
$guarantees_non_issues_charges;
-            $flaginfo{'amount'}  = $guarantees_non_issues_charges;
+            $flaginfo{'message'} = sprintf 'patron guarantees owe %.02f',
$patron_charge_limits->{NoIssuesChargeGuarantees}->{charge};
+            $flaginfo{'amount'}  =
$patron_charge_limits->{NoIssuesChargeGuarantees}->{charge};


Also the Price formatting will be wrong (CurrentyFormat). I am not sure where
this is used?

5) Documentation

+  `noissuescharge` int(11) DEFAULT NULL COMMENT 'define maximum amount
withstanding before checkouts are blocked',
+  `noissueschargeguarantees` int(11) DEFAULT NULL COMMENT 'define maximum
amount withstanding before checkouts are blocked',
+  `noissueschargeguarantorswithguarantees` int(11) DEFAULT NULL COMMENT
'define maximum amount withstanding before checkouts are blocked',

These all have the same description - what is the difference?

6) Formatting

Please use the $Price TT filter for proper formatting of the prices on display,
including the 0.00 default:

+                        [% IF (category.noissuescharge) %]
+                            <td>[% category.noissuescharge | html %]</td>
+                        [% ELSE %]
+                            <td>0.00</td>
+                        [% END %]
+                        [% IF (category.noissueschargeguarantees) %]
+                            <td>[% category.noissueschargeguarantees | html
%]</td>
+                        [% ELSE %]
+                            <td>0.00</td>
+                        [% END %]
+                        [% IF
(category.noissueschargeguarantorswithguarantees) %]
+                            <td>[%
category.noissueschargeguarantorswithguarantees | html %]</td>
+                        [% ELSE %]
+                            <td>0.00</td>
+                        [% END %]

7) Price handling on input fields

You need to use $Price on_editing on the input fields, I believe:

Example:
<input class="decimal" type="text" size="20" name="rrp" id="rrp" value="[% rrp
| $Price on_editing => 1 %]

-- 
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[email protected]
https://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/

Reply via email to