Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 23681: Fix QA issues

Browse source 
This patch fixes following QA issues:
- convert intaller files as .yml
- change column name readonly as is_system
- change column name is_system as default_value
- add column can_be_added_manually (testplan for this below)
- move syspref "PatronRestrictionTypes" to "Accounting > Features" tab
- tweak page title
- tweak tests to apply these changes

Also atomicupdate files have been updated. Issues with delete and tests
is fixed with adding additional schema change file.

To test:
1) Add new restriction type and make it manual.
2) Navigate to patron details page.
3) Add new restriction to patron.
=> Only selectable restriction should be the one
you just created.

Also prove t/db_dependent/RestrictionTypes.t.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This commit is contained in:
Emmi Takkinen 2022-05-02 15:40:35 +03:00 committed by Tomas Cohen Arazi
parent eb28de78fe
commit 20349d8737
Signed by: tomascohen
GPG key ID: 0A272EA1B2F3C15F
18 changed files with 191 additions and 89 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
Koha/RestrictionType.pm
View file

@ -42,10 +42,10 @@ sub delete {
my ( $self ) = @_;
# Find out what the default is
my $default = Koha::RestrictionTypes->find({ is_system => 1 })->code;
# Ensure we're not trying to delete a readonly type (this includes
my $default = Koha::RestrictionTypes->find({ default_value => 1 })->code;
# Ensure we're not trying to delete a is_system type (this includes
# the default type)
return 0 if $self->readonly == 1;
return 0 if $self->is_system == 1;
# We can't use Koha objects here because Koha::Patron::Debarments
# is not a Koha object. So we'll do it old skool
my $rows = C4::Context->dbh->do(

9
admin/restrictions.pl
View file

@ -53,6 +53,7 @@ if ( $op eq 'add_form') {
} elsif ( $op eq 'add_validate' ) {
my $display_text = $input->param('display_text');
my $can_be_added_manually = $input->param('can_be_added_manually') || 0;
my $is_a_modif = $input->param("is_a_modif");
if ($is_a_modif) {
@ -66,7 +67,10 @@ if ( $op eq 'add_form') {
};
} else {
my $restriction = Koha::RestrictionTypes->find($code);
$restriction->display_text($display_text);
unless ($restriction->is_system) {
$restriction->display_text($display_text);
$restriction->can_be_added_manually($can_be_added_manually);
}
$restriction->store;
}
} else {
@ -79,7 +83,8 @@ if ( $op eq 'add_form') {
} else {
my $restriction = Koha::RestrictionType->new({
code => $code,
display_text => $display_text
display_text => $display_text,
can_be_added_manually => $can_be_added_manually
});
$restriction->store;
}

6
installer/data/mysql/atomicupdate/bug_23681_add_PatronRestrictionTypes_syspref.perl
View file

@ -1,6 +0,0 @@
$DBversion = 'XXX'; # will be replaced by the RM
if( CheckVersion( $DBversion ) ) {
$dbh->do( q| INSERT IGNORE INTO systempreferences (variable, value, explanation, options, type) VALUES ('PatronRestrictionTypes', '0', 'If enabled, it is possible to specify the "type" of patron restriction being applied.', '', 'YesNo'); | );
SetVersion( $DBversion );
print "Upgrade to $DBversion done (Bug 23681 - Add PatronRestrictionTypes syspref)\n";
}

14
installer/data/mysql/atomicupdate/bug_23681_add_PatronRestrictionTypes_syspref.pl Normal file
View file

@ -0,0 +1,14 @@
use Modern::Perl;
return {
bug_number => "23681",
description => "Add PatronRestrictionTypes syspref",
up => sub {
my ($args) = @_;
my ($dbh, $out) = @$args{qw(dbh out)};
# Do you stuffs here
$dbh->do(q{INSERT IGNORE INTO systempreferences (variable, value, explanation, options, type) VALUES ('PatronRestrictionTypes', '0', 'If enabled, it is possible to specify the "type" of patron restriction being applied.', '', 'YesNo');});
# Print useful stuff here
say $out "Update is going well so far";
},
};

33
installer/data/mysql/atomicupdate/bug_23681_add_debarment_types.perl
View file

@ -1,33 +0,0 @@
$DBversion = 'XXX';
if ( CheckVersion( $DBversion ) ) {
if ( !TableExists( 'debarment_types' ) ) {
$dbh->do( q|
CREATE TABLE debarment_types (
code varchar(50) NOT NULL PRIMARY KEY,
display_text text NOT NULL,
readonly tinyint(1) NOT NULL DEFAULT 0,
system tinyint(1) NOT NULL DEFAULT 0
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
| );
$dbh->do( q|
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);
|);
$dbh->do( q|
ALTER TABLE borrower_debarments
MODIFY COLUMN type varchar(50) NOT NULL
| );
$dbh->do( q|
ALTER TABLE borrower_debarments
ADD CONSTRAINT borrower_debarments_ibfk_2 FOREIGN KEY (type) REFERENCES debarment_types(code) ON DELETE NO ACTION ON UPDATE CASCADE;
| );
}
SetVersion( $DBversion );
print "Upgrade to $DBversion done (Bug 23681 - Add debarment_types)\n";
}

37
installer/data/mysql/atomicupdate/bug_23681_add_debarment_types.pl Normal file
View file

@ -0,0 +1,37 @@
use Modern::Perl;
return {
bug_number => "23681",
description => "Add debarment_types",
up => sub {
my ($args) = @_;
my ($dbh, $out) = @$args{qw(dbh out)};
# Do you stuffs here
$dbh->do(q{
CREATE TABLE IF NOT EXISTS debarment_types (
code varchar(50) NOT NULL PRIMARY KEY,
display_text text NOT NULL,
is_system tinyint(1) NOT NULL DEFAULT 0,
default_value tinyint(1) NOT NULL DEFAULT 0,
can_be_added_manually tinyint(1) NOT NULL DEFAULT 0
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
});
$dbh->do(q{
INSERT IGNORE INTO debarment_types (code, display_text, is_system, default_value, can_be_added_manually) VALUES
('MANUAL', 'Manual', 1, 1, 0),
('OVERDUES', 'Overdues', 1, 0, 0),
('SUSPENSION', 'Suspension', 1, 0, 0),
('DISCHARGE', 'Discharge', 1, 0, 0);
});
$dbh->do(q{
ALTER TABLE borrower_debarments
MODIFY COLUMN type varchar(50) NOT NULL
});
$dbh->do(q{
ALTER TABLE borrower_debarments
ADD CONSTRAINT borrower_debarments_ibfk_2 FOREIGN KEY (type) REFERENCES debarment_types(code) ON DELETE NO ACTION ON UPDATE CASCADE;
});
# Print useful stuff here
say $out "Update is going well so far";
},
};

9
installer/data/mysql/atomicupdate/bug_23681_add_manage_patron_restrictions_perm.perl
View file

@ -1,9 +0,0 @@
$DBversion = 'XXX';
if( CheckVersion( $DBversion ) ) {
$dbh->do(q{
INSERT IGNORE INTO permissions (module_bit, code, description) VALUES
( 3, 'manage_patron_restrictions', 'Manage patron restrictions')
});
SetVersion( $DBversion );
print "Upgrade to $DBversion done (Bug 23681 - Add manage_patron_restrictions_permission)\n";
}

14
installer/data/mysql/atomicupdate/bug_23681_add_manage_patron_restrictions_perm.pl Normal file
View file

@ -0,0 +1,14 @@
use Modern::Perl;
return {
bug_number => "23681",
description => "Add manage_patron_restrictions_permission",
up => sub {
my ($args) = @_;
my ($dbh, $out) = @$args{qw(dbh out)};
# Do you stuffs here
$dbh->do(q{ INSERT IGNORE INTO permissions (module_bit, code, description) VALUES ( 3, 'manage_patron_restrictions', 'Manage patron restrictions')});
# Print useful stuff here
say $out "Update is going well so far";
},
};

5
installer/data/mysql/en/mandatory/patron_restrictions.sql
View file

@ -1,5 +0,0 @@
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);

1
installer/data/mysql/en/mandatory/patron_restrictions.txt
View file

@ -1 +0,0 @@
Default Koha system patron restriction types

50
installer/data/mysql/en/mandatory/patron_restrictions.yml Normal file
View file

@ -0,0 +1,50 @@
---
#
# Copyright 2020 Koha Development Team
#
# This file is part of Koha.
#
# Koha is free software; you can redistribute it and/or modify it
# under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 3 of the License, or
# (at your option) any later version.
#
# Koha is distributed in the hope that it will be useful, but
# WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with Koha; if not, see <http://www.gnu.org/licenses>.
description:
- "Default Koha system patron restriction types"
tables:
- debarment_types:
translatable: [ display_text ]
multiline: []
rows:
- code: "MANUAL"
display_text: "Manual"
is_system: 1
default: 1
can_be_added_manually: 0
- code: "OVERDUES"
display_text: "Overdues"
is_system: 1
default: 0
can_be_added_manually: 0
- code: "SUSPENSION"
display_text: "Suspension"
is_system: 1
default: 0
can_be_added_manually: 0
- code: "DISCHARGE"
display_text: "Discharge"
is_system: 1
default: 0
can_be_added_manually: 0

5
installer/data/mysql/kohastructure.sql
View file

@ -2085,8 +2085,9 @@ DROP TABLE IF EXISTS `debarment_types`;
CREATE TABLE debarment_types (
code varchar(50) NOT NULL PRIMARY KEY,
display_text text NOT NULL,
readonly tinyint(1) NOT NULL DEFAULT 0,
is_system tinyint(1) NOT NULL DEFAULT 0
is_system tinyint(1) NOT NULL DEFAULT 0,
default_value tinyint(1) NOT NULL DEFAULT 0,
can_be_added_manually tinyint(1) NOT NULL DEFAULT 0
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
--

4
koha-tmpl/intranet-tmpl/prog/en/includes/borrower_debarments.inc
View file

@ -64,7 +64,9 @@
<label for="debarred_type">Type:</label>
<select name="debarred_type">
[% FOREACH code IN restriction_types.keys %]
<option value="[% code | html %]">[% restriction_types.$code.display_text | html %]</option>
[% IF restriction_types.$code.can_be_added_manually %]
<option value="[% code | html %]">[% restriction_types.$code.display_text | html %]</option>
[% END %]
[% END %]
</select>
</li>

6
koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/accounting.pref
View file

@ -57,3 +57,9 @@ Accounting:
branchyyyymmincr: 'Automatically generate credit numbers in the form <branchcode>yyyymm0001'
incremental: 'Automatically generate credit numbers in the form 1, 2, 3'
- Automatic generation also has to be enabled for each credit type (<a href="/cgi-bin/koha/admin/credit_types.pl">Configure credit types</a>)
-
- pref: PatronRestrictionTypes
choices:
1: Allow
0: Don't allow
- "the type of patron restriction to be specified when applying manually."

6
koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref
View file

@ -341,12 +341,6 @@ Patrons:
1: Allow
0: "Don't allow"
- staff to set the ability for a patron's fines to be viewed by linked patrons in the OPAC.
-
- pref: PatronRestrictionTypes
choices:
1: Allow
0: Don't allow
- "the type of patron restriction to be specified when applying manually."
Privacy:
-

39
koha-tmpl/intranet-tmpl/prog/en/modules/admin/restrictions.tt
View file

@ -21,19 +21,19 @@
<li>
<a href="/cgi-bin/koha/admin/admin-home.pl">Administration</a>
</li>
<li>
<a href="/cgi-bin/koha/admin/restrictions.pl">Patron restrictions</a>
</li>
[% IF op == 'list' %]
<li>
<a href="#" aria-current="page">
All restrictions
Patron restrictions
</a>
</li>
[% END %]
[% IF op == 'add_form' %]
<li>
<a href="/cgi-bin/koha/admin/restrictions.pl">Patron restrictions</a>
</li>
[% IF restriction %]
<li>
<a href="#" aria-current="page">
@ -50,6 +50,9 @@
[% END %]
[% IF op == 'delete_confirm' %]
<li>
<a href="/cgi-bin/koha/admin/restrictions.pl">Patron restrictions</a>
</li>
<li>
<a href="#" aria-current="page">
Delete restriction?
@ -99,6 +102,16 @@
<input type="text" value="[% restriction.display_text | html %]" name="display_text" id="display_text" size="50" maxlength="50" class="required" required="required" />
<span class="required">Required</span>
</li>
<li>
<label for="can_be_added_manually">Can be manually added?</label>
[% IF restriction && restriction.is_system %]
[% IF restriction.can_be_added_manually %]Yes[% ELSE %]No[% END %]
[% ELSIF restriction.can_be_added_manually %]
<input type="checkbox" name="can_be_added_manually" id="can_be_added_manually" checked="checked" value="1" />
[% ELSE %]
<input type="checkbox" name="can_be_added_manually" id="can_be_added_manually" value="1" />
[% END %]
</li>
[% ELSE %]
<li>
<label for="code" class="required">Code: </label>
@ -110,6 +123,16 @@
<input type="text" name="display_text" id="display_text" size="50" maxlength="50" class="required" required="required" />
<span class="required">Required</span>
</li>
<li>
<label for="can_be_added_manually">Can be manually added?</label>
[% IF restriction && restriction.is_system %]
[% IF restriction.can_be_added_manually %]Yes[% ELSE %]No[% END %]
[% ELSIF restriction.can_be_added_manually %]
<input type="checkbox" name="can_be_added_manually" id="can_be_added_manually" checked="checked" value="1" />
[% ELSE %]
<input type="checkbox" name="can_be_added_manually" id="can_be_added_manually" value="1" />
[% END %]
</li>
[% END %]
</ol>
</fieldset>
@ -156,6 +179,7 @@
<tr>
<th scope="col">Code</th>
<th scope="col">Label</th>
<th scope="col">Manual</th>
<th scope="col">Default</th>
<th scope="col">Actions</th>
</tr>
@ -170,11 +194,14 @@
[% restriction.display_text | html %]
</td>
<td>
[% IF restriction.is_system %]Yes[% END %]
[% IF restriction.can_be_added_manually %]Yes[% END %]
</td>
<td>
[% IF restriction.default_value %]Yes[% END %]
</td>
<td class="actions">
<a class="btn btn-default btn-xs" href="/cgi-bin/koha/admin/restrictions.pl?op=add_form&amp;code=[% restriction.code | uri %]"><i class="fa fa-pencil"></i> Edit</a>
[% IF !restriction.readonly %]
[% IF !restriction.is_system %]
<a class="btn btn-default btn-xs" href="/cgi-bin/koha/admin/restrictions.pl?op=delete_confirm&amp;code=[% restriction.code | uri %]"><i class="fa fa-trash"></i> Delete</a>
[% END %]
</td>

4
koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
View file

@ -1402,7 +1402,9 @@ legend:hover {
<label for="debarred_type">Type:</label>
<select name="debarred_type">
[% FOREACH code IN restriction_types.keys %]
<option value="[% code | html %]">[% restriction_types.$code.display_text | html %]</option>
[% IF restriction_types.$code.can_be_added_manually %]
<option value="[% code | html %]">[% restriction_types.$code.display_text | html %]</option>
[% END %]
[% END %]
</select>
</li>

32
t/db_dependent/RestrictionTypes.t
View file

@ -17,15 +17,16 @@ use_ok('Koha::RestrictionType');
use_ok('Koha::RestrictionTypes');
$dbh->do(q|DELETE FROM borrower_debarments|);
Koha::RestrictionTypes->search->delete;
$dbh->do(q|DELETE FROM debarment_types|);
$builder->build({
source => 'DebarmentType',
value => {
code => 'ONE',
display_text => 'One',
readonly => 1,
is_system => 0
is_system => 1,
default_value => 0,
can_be_added_manually => 0
}
});
$builder->build({
@ -33,8 +34,9 @@ $builder->build({
value => {
code => 'TWO',
display_text => 'Two',
readonly => 1,
is_system => 1
is_system => 1,
default_value => 1,
can_be_added_manually => 0
}
});
$builder->build({
@ -42,8 +44,9 @@ $builder->build({
value => {
code => 'THREE',
display_text => 'Three',
readonly => 1,
is_system => 0
is_system => 1,
default_value => 0,
can_be_added_manually => 0
}
});
$builder->build({
@ -51,8 +54,9 @@ $builder->build({
value => {
code => 'FOUR',
display_text => 'Four',
readonly => 0,
is_system => 0
is_system => 0,
default_value => 0,
can_be_added_manually => 0
}
});
$builder->build({
@ -60,8 +64,9 @@ $builder->build({
value => {
code => 'FIVE',
display_text => 'Five',
readonly => 0,
is_system => 0
is_system => 0,
default_value => 0,
can_be_added_manually => 0
}
});
@ -71,10 +76,9 @@ ok( $created->display_text eq 'One', 'Restrictions created');
# Can we delete RestrictionTypes, when appropriate
my $deleted = Koha::RestrictionTypes->find({ code => 'FOUR' })->delete;
ok( $deleted == 1, 'Restriction deleted');
ok( $deleted, 'Restriction deleted');
my $not_deleted = Koha::RestrictionTypes->find({ code => 'TWO' })->delete;
ok( $not_deleted == 0, 'Read only restriction not deleted');
ok( !$not_deleted, 'Read only restriction not deleted');
# Add a patron with a debarment
my $library = $builder->build({ source => 'Branch' });