[Koha-bugs] [Bug 23681] Patron restrictions should be user definable

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

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

Katrin Fischer <katrin.fischer at bsz-bw.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Signed Off                  |Failed QA

--- Comment #114 from Katrin Fischer <katrin.fischer at bsz-bw.de> ---

I am sorry, this got rather long again. It might not look like it, but I think
we are really close and it WOULD make a great starting point for making the
restriction system more granular.

1) Installer data again

(In reply to Martin Renvoize from comment #113)
> (In reply to Katrin Fischer from comment #92)
> > 2) Sample data for installer
> > 
> I think this is dealt with by having it in the templates instead

I had a closer look here and the installer file needs updating one way around
or the other:

I we don't want to translate it, it should be in
https://git.koha-community.org/Koha-community/Koha/src/branch/master/installer/data/mysql/mandatory.

If we don't have it wrong, we should transform to .yml and also add it to
fr-FR, the remaining SQL installer. The .yml file structure looks easy enough
in how to mark the translatable bits and how to add a description for the
installer:
https://git.koha-community.org/Koha-community/Koha/src/branch/master/installer/data/mysql/en/mandatory

I either case the .txt needs to be removed.

I think the latter would be the nicer solution, as the display text translated
would still be nice for reports and such. (I'd also be in favor of moving
account_credit_types and debit_types for that reason, but that's another story
:) )

+INSERT INTO debarment_types (code, display_text, readonly, system) VALUES
+    ('MANUAL', 'Manual', 1, 1),
+    ('OVERDUES', 'Overdues', 1, 0),
+    ('SUSPENSION', 'Suspension', 1, 0),
+    ('DISCHARGE', 'Discharge', 1, 0);

2) CSS update

I believe Bug 23681: CSS Build should be dropped. It's expected that you
generate the CSS files locally. It might create conflicts, but I can just
drop/skip it then.

3) Typo in database update / column names

a)
+                system tinyint(1) NOT NULL DEFAULT 0

Should be: is_system. I've fixed it locally for testing further:
alter table debarment_types rename column system to is_system;

b) I have been thinking more about the different columns and I think there is
an error there:

+------------+--------------+----------+-----------+
| code       | display_text | readonly | is_system |
+------------+--------------+----------+-----------+
| DISCHARGE  | Discharge    |        1 |         0 |
| MANUAL     | Manual       |        1 |         1 |
| OVERDUES   | Overdues     |        1 |         0 |
| SUSPENSION | Suspension   |        1 |         0 |
+------------+--------------+----------+-----------+

read_only should be is_system = It's internal, don't change it. Applies to all
currently added ones.

is_system, was dflt = default = means it's used when the pref is turned off. So
it should be default actually.

AND: I think we need another column:

can_be_added_manually (see account_credit_types for comparison)

Because right now the list of restrictions on the patron account will show ALL
restrictions defined in debarment_types and I feel like we are asking for
trouble if we allow to set restrictions manually that are supposed to be added
automatically. Like a DISCHARGE restriction should be linked to an entry in the
discharges table somehow, an OVERDUES restriction can be lifted automatically
if all overdues are returned, SUSPENSION is calculated on late returns... I'd
feel we might want to leave them out of the pulldown really to keep internal
behaviors untouched.


4) System preference (sorry, only saw this in the GUI)

Currently the pref appears under "Patrons > Patron relationships", but this is
the section for guarantor and family account stuff. I suggest moving it into
the "Accounting > Features" tab, that maybe didn't exist yet when this was
first written.

5) Page title

There is a > left at the beginning that should not be there if you look at the
tab in the browser.

6) Deleting a restriction

When I try to delete a newly created restriction, I see:
Can't locate object method "_new_from_dbic" via package "Koha::DebarmentType"
(perhaps you forgot to load "Koha::DebarmentType"?) at
/kohadevbox/koha/Koha/Object.pm line 237

7) Tests are failing :(

prove t/db_dependent/RestrictionType.t t/db_dependent/RestrictionTypes.t
t/db_dependent/RestrictionType.t ... 1/3 
#   Failed test 'keyed_on_code returns correctly'
#   at t/db_dependent/RestrictionType.t line 58.
#     Structures begin differing at:
#          $got->{MANUAL} = HASH(0x5618db704c68)
#     $expected->{MANUAL} = Does not exist
# Looks like you failed 1 test of 3.
t/db_dependent/RestrictionType.t ... Dubious, test returned 1 (wstat 256,
0x100)
Failed 1/3 subtests 
t/db_dependent/RestrictionTypes.t .. 1/6
DBIx::Class::Storage::DBI::select_single(): Query returned more than one row. 
SQL that returns multiple rows is DEPRECATED for ->find and ->single at
/kohadevbox/koha/Koha/Objects.pm line 96
Can't locate object method "_new_from_dbic" via package "Koha::DebarmentType"
(perhaps you forgot to load "Koha::DebarmentType"?) at
/kohadevbox/koha/Koha/Object.pm line 237.
# Looks like your test exited with 11 just after 3.
t/db_dependent/RestrictionTypes.t .. Dubious, test returned 11 (wstat 2816,
0xb00)
Failed 3/6 subtests 

Test Summary Report
-------------------
t/db_dependent/RestrictionType.t (Wstat: 256 Tests: 3 Failed: 1)
  Failed test:  3
  Non-zero exit status: 1
t/db_dependent/RestrictionTypes.t (Wstat: 2816 Tests: 3 Failed: 0)
  Non-zer

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