Bug 23681: (QA follow-up) Proper handling of default option
This patch removes the 'can_be_added_manually' flag. Only non-system restriction types can be added manually, so we exclude is_system instead of having two flags. (And we set the 'Manual' that's added at install time to default but not system). We then add proper handling for setting the default manual restriction type in the management page and set the dropdown list to use that value by default. Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This commit is contained in:
parent
a50553ff15
commit
3b1c271f78
9 changed files with 160 additions and 147 deletions
|
@ -42,7 +42,7 @@ sub delete {
|
||||||
my ( $self ) = @_;
|
my ( $self ) = @_;
|
||||||
|
|
||||||
# Find out what the default is
|
# Find out what the default is
|
||||||
my $default = Koha::RestrictionTypes->find({ default_value => 1 })->code;
|
my $default = Koha::RestrictionTypes->find({ is_default => 1 })->code;
|
||||||
# Ensure we're not trying to delete a is_system type (this includes
|
# Ensure we're not trying to delete a is_system type (this includes
|
||||||
# the default type)
|
# the default type)
|
||||||
return 0 if $self->is_system == 1;
|
return 0 if $self->is_system == 1;
|
||||||
|
@ -63,6 +63,28 @@ sub delete {
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
=head3 make_default
|
||||||
|
|
||||||
|
Set the current restriction type as the default for manual restrictions
|
||||||
|
|
||||||
|
=cut
|
||||||
|
|
||||||
|
sub make_default {
|
||||||
|
my ( $self ) = @_;
|
||||||
|
|
||||||
|
$self->_result->result_source->schema->txn_do(
|
||||||
|
sub {
|
||||||
|
my $types =
|
||||||
|
Koha::RestrictionTypes->search( { code => { '!=' => $self->code } } );
|
||||||
|
$types->update( { is_default => 0 } );
|
||||||
|
$self->set( { is_default => 1 } );
|
||||||
|
$self->SUPER::store;
|
||||||
|
}
|
||||||
|
);
|
||||||
|
|
||||||
|
return $self;
|
||||||
|
}
|
||||||
|
|
||||||
=head2 Internal methods
|
=head2 Internal methods
|
||||||
|
|
||||||
=head3 type
|
=head3 type
|
||||||
|
|
|
@ -53,7 +53,6 @@ if ( $op eq 'add_form') {
|
||||||
} elsif ( $op eq 'add_validate' ) {
|
} elsif ( $op eq 'add_validate' ) {
|
||||||
|
|
||||||
my $display_text = $input->param('display_text');
|
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");
|
my $is_a_modif = $input->param("is_a_modif");
|
||||||
|
|
||||||
if ($is_a_modif) {
|
if ($is_a_modif) {
|
||||||
|
@ -68,9 +67,6 @@ if ( $op eq 'add_form') {
|
||||||
} else {
|
} else {
|
||||||
my $restriction = Koha::RestrictionTypes->find($code);
|
my $restriction = Koha::RestrictionTypes->find($code);
|
||||||
$restriction->display_text($display_text);
|
$restriction->display_text($display_text);
|
||||||
unless ($restriction->is_system) {
|
|
||||||
$restriction->can_be_added_manually($can_be_added_manually);
|
|
||||||
}
|
|
||||||
$restriction->store;
|
$restriction->store;
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
|
@ -83,13 +79,16 @@ if ( $op eq 'add_form') {
|
||||||
} else {
|
} else {
|
||||||
my $restriction = Koha::RestrictionType->new({
|
my $restriction = Koha::RestrictionType->new({
|
||||||
code => $code,
|
code => $code,
|
||||||
display_text => $display_text,
|
display_text => $display_text
|
||||||
can_be_added_manually => $can_be_added_manually
|
|
||||||
});
|
});
|
||||||
$restriction->store;
|
$restriction->store;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
$op = 'list';
|
$op = 'list';
|
||||||
|
} elsif ( $op eq 'make_default' ) {
|
||||||
|
my $restriction = Koha::RestrictionTypes->find($code);
|
||||||
|
$restriction->make_default;
|
||||||
|
$op = 'list';
|
||||||
} elsif ( $op eq 'delete_confirm' ) {
|
} elsif ( $op eq 'delete_confirm' ) {
|
||||||
$template->param(
|
$template->param(
|
||||||
restriction => scalar Koha::RestrictionTypes->find($code)
|
restriction => scalar Koha::RestrictionTypes->find($code)
|
||||||
|
|
13
installer/data/mysql/atomicupdate/bug_23681.pl
Normal file → Executable file
13
installer/data/mysql/atomicupdate/bug_23681.pl
Normal file → Executable file
|
@ -12,18 +12,17 @@ return {
|
||||||
code varchar(50) NOT NULL PRIMARY KEY,
|
code varchar(50) NOT NULL PRIMARY KEY,
|
||||||
display_text text NOT NULL,
|
display_text text NOT NULL,
|
||||||
is_system tinyint(1) NOT NULL DEFAULT 0,
|
is_system tinyint(1) NOT NULL DEFAULT 0,
|
||||||
default_value tinyint(1) NOT NULL DEFAULT 0,
|
is_default 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;
|
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
|
||||||
});
|
});
|
||||||
say $out "Added debarment_types table";
|
say $out "Added debarment_types table";
|
||||||
|
|
||||||
$dbh->do(q{
|
$dbh->do(q{
|
||||||
INSERT IGNORE INTO debarment_types (code, display_text, is_system, default_value, can_be_added_manually) VALUES
|
INSERT IGNORE INTO debarment_types (code, display_text, is_system, is_default) VALUES
|
||||||
('MANUAL', 'Manual', 1, 1, 0),
|
('MANUAL', 'Manual', 0, 1),
|
||||||
('OVERDUES', 'Overdues', 1, 0, 0),
|
('OVERDUES', 'Overdues', 1, 0),
|
||||||
('SUSPENSION', 'Suspension', 1, 0, 0),
|
('SUSPENSION', 'Suspension', 1, 0),
|
||||||
('DISCHARGE', 'Discharge', 1, 0, 0);
|
('DISCHARGE', 'Discharge', 1, 0);
|
||||||
});
|
});
|
||||||
say $out "Added system debarment_types";
|
say $out "Added system debarment_types";
|
||||||
|
|
||||||
|
|
|
@ -27,24 +27,20 @@ tables:
|
||||||
rows:
|
rows:
|
||||||
- code: "MANUAL"
|
- code: "MANUAL"
|
||||||
display_text: "Manual"
|
display_text: "Manual"
|
||||||
is_system: 1
|
is_system: 0
|
||||||
default: 1
|
is_default: 1
|
||||||
can_be_added_manually: 0
|
|
||||||
|
|
||||||
- code: "OVERDUES"
|
- code: "OVERDUES"
|
||||||
display_text: "Overdues"
|
display_text: "Overdues"
|
||||||
is_system: 1
|
is_system: 1
|
||||||
default: 0
|
is_default: 0
|
||||||
can_be_added_manually: 0
|
|
||||||
|
|
||||||
- code: "SUSPENSION"
|
- code: "SUSPENSION"
|
||||||
display_text: "Suspension"
|
display_text: "Suspension"
|
||||||
is_system: 1
|
is_system: 1
|
||||||
default: 0
|
is_default: 0
|
||||||
can_be_added_manually: 0
|
|
||||||
|
|
||||||
- code: "DISCHARGE"
|
- code: "DISCHARGE"
|
||||||
display_text: "Discharge"
|
display_text: "Discharge"
|
||||||
is_system: 1
|
is_system: 1
|
||||||
default: 0
|
is_default: 0
|
||||||
can_be_added_manually: 0
|
|
||||||
|
|
|
@ -2086,8 +2086,7 @@ CREATE TABLE debarment_types (
|
||||||
code varchar(50) NOT NULL PRIMARY KEY,
|
code varchar(50) NOT NULL PRIMARY KEY,
|
||||||
display_text text NOT NULL,
|
display_text text NOT NULL,
|
||||||
is_system tinyint(1) NOT NULL DEFAULT 0,
|
is_system tinyint(1) NOT NULL DEFAULT 0,
|
||||||
default_value tinyint(1) NOT NULL DEFAULT 0,
|
is_default 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;
|
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
|
||||||
|
|
||||||
--
|
--
|
||||||
|
|
|
@ -58,10 +58,14 @@
|
||||||
<label for="debarred_type">Type:</label>
|
<label for="debarred_type">Type:</label>
|
||||||
<select name="debarred_type">
|
<select name="debarred_type">
|
||||||
[% FOREACH code IN restriction_types.keys %]
|
[% FOREACH code IN restriction_types.keys %]
|
||||||
[% IF restriction_types.$code.can_be_added_manually %]
|
[% IF !restriction_types.$code.is_system %]
|
||||||
|
[% IF restriction_types.$code.is_default %]
|
||||||
|
<option value="[% code | html %]" selected>[% PROCESS restriction_type_description restriction=restriction_types.$code %]</option>
|
||||||
|
[% ELSE %]
|
||||||
<option value="[% code | html %]">[% PROCESS restriction_type_description restriction=restriction_types.$code %]</option>
|
<option value="[% code | html %]">[% PROCESS restriction_type_description restriction=restriction_types.$code %]</option>
|
||||||
[% END %]
|
[% END %]
|
||||||
[% END %]
|
[% END %]
|
||||||
|
[% END %]
|
||||||
</select>
|
</select>
|
||||||
</li>
|
</li>
|
||||||
[% END %]
|
[% END %]
|
||||||
|
|
|
@ -103,16 +103,6 @@
|
||||||
<input type="text" value="[% restriction.display_text | html %]" name="display_text" id="display_text" size="50" maxlength="50" class="required" required="required" />
|
<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>
|
<span class="required">Required</span>
|
||||||
</li>
|
</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 %]
|
[% ELSE %]
|
||||||
<li>
|
<li>
|
||||||
<label for="code" class="required">Code: </label>
|
<label for="code" class="required">Code: </label>
|
||||||
|
@ -124,16 +114,6 @@
|
||||||
<input type="text" name="display_text" id="display_text" size="50" maxlength="50" class="required" required="required" />
|
<input type="text" name="display_text" id="display_text" size="50" maxlength="50" class="required" required="required" />
|
||||||
<span class="required">Required</span>
|
<span class="required">Required</span>
|
||||||
</li>
|
</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 %]
|
[% END %]
|
||||||
</ol>
|
</ol>
|
||||||
</fieldset>
|
</fieldset>
|
||||||
|
@ -180,7 +160,6 @@
|
||||||
<tr>
|
<tr>
|
||||||
<th scope="col">Code</th>
|
<th scope="col">Code</th>
|
||||||
<th scope="col">Label</th>
|
<th scope="col">Label</th>
|
||||||
<th scope="col">Manual</th>
|
|
||||||
<th scope="col">Default</th>
|
<th scope="col">Default</th>
|
||||||
<th scope="col">Actions</th>
|
<th scope="col">Actions</th>
|
||||||
</tr>
|
</tr>
|
||||||
|
@ -195,16 +174,16 @@
|
||||||
[% PROCESS restriction_type_description %]
|
[% PROCESS restriction_type_description %]
|
||||||
</td>
|
</td>
|
||||||
<td>
|
<td>
|
||||||
[% IF restriction.can_be_added_manually %]Yes[% END %]
|
[% IF restriction.is_default %]Yes[% END %]
|
||||||
</td>
|
|
||||||
<td>
|
|
||||||
[% IF restriction.default_value %]Yes[% END %]
|
|
||||||
</td>
|
</td>
|
||||||
<td class="actions">
|
<td class="actions">
|
||||||
<a class="btn btn-default btn-xs" href="/cgi-bin/koha/admin/restrictions.pl?op=add_form&code=[% restriction.code | uri %]"><i class="fa fa-pencil"></i> Edit</a>
|
<a class="btn btn-default btn-xs" href="/cgi-bin/koha/admin/restrictions.pl?op=add_form&code=[% restriction.code | uri %]"><i class="fa fa-pencil"></i> Edit</a>
|
||||||
[% IF !restriction.is_system %]
|
[% IF !restriction.is_system && !restriction.is_default %]
|
||||||
<a class="btn btn-default btn-xs" href="/cgi-bin/koha/admin/restrictions.pl?op=delete_confirm&code=[% restriction.code | uri %]"><i class="fa fa-trash"></i> Delete</a>
|
<a class="btn btn-default btn-xs" href="/cgi-bin/koha/admin/restrictions.pl?op=delete_confirm&code=[% restriction.code | uri %]"><i class="fa fa-trash"></i> Delete</a>
|
||||||
[% END %]
|
[% END %]
|
||||||
|
[% IF !restriction.is_system && !restriction.is_default %]
|
||||||
|
<a class="btn btn-default btn-xs" href="/cgi-bin/koha/admin/restrictions.pl?op=make_default&code=[% restriction.code | uri %]"><i class="fa fa-archive"></i> Make default</a>
|
||||||
|
[% END %]
|
||||||
</td>
|
</td>
|
||||||
</tr>
|
</tr>
|
||||||
[% END %]
|
[% END %]
|
||||||
|
|
|
@ -17,26 +17,30 @@ use_ok('Koha::RestrictionType');
|
||||||
use_ok('Koha::RestrictionTypes');
|
use_ok('Koha::RestrictionTypes');
|
||||||
|
|
||||||
$dbh->do(q|DELETE FROM borrower_debarments|);
|
$dbh->do(q|DELETE FROM borrower_debarments|);
|
||||||
Koha::RestrictionTypes->search->delete;
|
$dbh->do(q|DELETE FROM debarment_types|);
|
||||||
|
|
||||||
$builder->build({
|
$builder->build(
|
||||||
|
{
|
||||||
source => 'DebarmentType',
|
source => 'DebarmentType',
|
||||||
value => {
|
value => {
|
||||||
code => 'ONE',
|
code => 'ONE',
|
||||||
display_text => 'One',
|
display_text => 'One',
|
||||||
readonly => 1,
|
is_system => 1,
|
||||||
is_system => 0
|
is_default => 0
|
||||||
}
|
}
|
||||||
});
|
}
|
||||||
$builder->build({
|
);
|
||||||
|
$builder->build(
|
||||||
|
{
|
||||||
source => 'DebarmentType',
|
source => 'DebarmentType',
|
||||||
value => {
|
value => {
|
||||||
code => 'TWO',
|
code => 'TWO',
|
||||||
display_text => 'Two',
|
display_text => 'Two',
|
||||||
readonly => 1,
|
is_system => 1,
|
||||||
is_system => 1
|
is_default => 1
|
||||||
}
|
}
|
||||||
});
|
}
|
||||||
|
);
|
||||||
|
|
||||||
# keyed_on_code
|
# keyed_on_code
|
||||||
my $keyed = Koha::RestrictionTypes->keyed_on_code;
|
my $keyed = Koha::RestrictionTypes->keyed_on_code;
|
||||||
|
@ -44,14 +48,14 @@ my $expecting = {
|
||||||
ONE => {
|
ONE => {
|
||||||
code => 'ONE',
|
code => 'ONE',
|
||||||
display_text => 'One',
|
display_text => 'One',
|
||||||
readonly => 1,
|
is_system => 1,
|
||||||
is_system => 0
|
is_default => 0
|
||||||
},
|
},
|
||||||
TWO => {
|
TWO => {
|
||||||
code => 'TWO',
|
code => 'TWO',
|
||||||
display_text => 'Two',
|
display_text => 'Two',
|
||||||
readonly => 1,
|
is_system => 1,
|
||||||
is_system => 1
|
is_default => 1
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
|
@ -19,56 +19,61 @@ use_ok('Koha::RestrictionTypes');
|
||||||
$dbh->do(q|DELETE FROM borrower_debarments|);
|
$dbh->do(q|DELETE FROM borrower_debarments|);
|
||||||
$dbh->do(q|DELETE FROM debarment_types|);
|
$dbh->do(q|DELETE FROM debarment_types|);
|
||||||
|
|
||||||
$builder->build({
|
$builder->build(
|
||||||
|
{
|
||||||
source => 'DebarmentType',
|
source => 'DebarmentType',
|
||||||
value => {
|
value => {
|
||||||
code => 'ONE',
|
code => 'ONE',
|
||||||
display_text => 'One',
|
display_text => 'One',
|
||||||
is_system => 1,
|
is_system => 1,
|
||||||
default_value => 0,
|
is_default => 0,
|
||||||
can_be_added_manually => 0
|
|
||||||
}
|
}
|
||||||
});
|
}
|
||||||
$builder->build({
|
);
|
||||||
|
$builder->build(
|
||||||
|
{
|
||||||
source => 'DebarmentType',
|
source => 'DebarmentType',
|
||||||
value => {
|
value => {
|
||||||
code => 'TWO',
|
code => 'TWO',
|
||||||
display_text => 'Two',
|
display_text => 'Two',
|
||||||
is_system => 1,
|
is_system => 1,
|
||||||
default_value => 1,
|
is_default => 1,
|
||||||
can_be_added_manually => 0
|
|
||||||
}
|
}
|
||||||
});
|
}
|
||||||
$builder->build({
|
);
|
||||||
|
$builder->build(
|
||||||
|
{
|
||||||
source => 'DebarmentType',
|
source => 'DebarmentType',
|
||||||
value => {
|
value => {
|
||||||
code => 'THREE',
|
code => 'THREE',
|
||||||
display_text => 'Three',
|
display_text => 'Three',
|
||||||
is_system => 1,
|
is_system => 1,
|
||||||
default_value => 0,
|
is_default => 0,
|
||||||
can_be_added_manually => 0
|
|
||||||
}
|
}
|
||||||
});
|
}
|
||||||
$builder->build({
|
);
|
||||||
|
$builder->build(
|
||||||
|
{
|
||||||
source => 'DebarmentType',
|
source => 'DebarmentType',
|
||||||
value => {
|
value => {
|
||||||
code => 'FOUR',
|
code => 'FOUR',
|
||||||
display_text => 'Four',
|
display_text => 'Four',
|
||||||
is_system => 0,
|
is_system => 0,
|
||||||
default_value => 0,
|
is_default => 0,
|
||||||
can_be_added_manually => 0
|
|
||||||
}
|
}
|
||||||
});
|
}
|
||||||
$builder->build({
|
);
|
||||||
|
$builder->build(
|
||||||
|
{
|
||||||
source => 'DebarmentType',
|
source => 'DebarmentType',
|
||||||
value => {
|
value => {
|
||||||
code => 'FIVE',
|
code => 'FIVE',
|
||||||
display_text => 'Five',
|
display_text => 'Five',
|
||||||
is_system => 0,
|
is_system => 0,
|
||||||
default_value => 0,
|
is_default => 0,
|
||||||
can_be_added_manually => 0
|
|
||||||
}
|
}
|
||||||
});
|
}
|
||||||
|
);
|
||||||
|
|
||||||
# Can we create RestrictionTypes
|
# Can we create RestrictionTypes
|
||||||
my $created = Koha::RestrictionTypes->find( { code => 'ONE' } );
|
my $created = Koha::RestrictionTypes->find( { code => 'ONE' } );
|
||||||
|
@ -84,26 +89,32 @@ ok( !$not_deleted, 'Read only restriction not deleted');
|
||||||
my $library = $builder->build( { source => 'Branch' } );
|
my $library = $builder->build( { source => 'Branch' } );
|
||||||
|
|
||||||
my $patron_category = $builder->build( { source => 'Category' } );
|
my $patron_category = $builder->build( { source => 'Category' } );
|
||||||
my $borrowernumber = Koha::Patron->new({
|
my $borrowernumber = Koha::Patron->new(
|
||||||
|
{
|
||||||
firstname => 'my firstname',
|
firstname => 'my firstname',
|
||||||
surname => 'my surname',
|
surname => 'my surname',
|
||||||
categorycode => $patron_category->{categorycode},
|
categorycode => $patron_category->{categorycode},
|
||||||
branchcode => $library->{branchcode},
|
branchcode => $library->{branchcode},
|
||||||
})->store->borrowernumber;
|
}
|
||||||
|
)->store->borrowernumber;
|
||||||
|
|
||||||
Koha::Patron::Debarments::AddDebarment({
|
Koha::Patron::Debarments::AddDebarment(
|
||||||
|
{
|
||||||
borrowernumber => $borrowernumber,
|
borrowernumber => $borrowernumber,
|
||||||
expiration => '9999-06-10',
|
expiration => '9999-06-10',
|
||||||
type => 'FIVE',
|
type => 'FIVE',
|
||||||
comment => 'Test 1',
|
comment => 'Test 1',
|
||||||
});
|
}
|
||||||
|
);
|
||||||
|
|
||||||
# Now delete a code and check debarments using that code switch to
|
# Now delete a code and check debarments using that code switch to
|
||||||
# using the default
|
# using the default
|
||||||
my $to_delete = Koha::RestrictionTypes->find( { code => 'FIVE' } )->delete;
|
my $to_delete = Koha::RestrictionTypes->find( { code => 'FIVE' } )->delete;
|
||||||
my $debarments = Koha::Patron::Debarments::GetDebarments({
|
my $debarments = Koha::Patron::Debarments::GetDebarments(
|
||||||
|
{
|
||||||
borrowernumber => $borrowernumber
|
borrowernumber => $borrowernumber
|
||||||
});
|
}
|
||||||
|
);
|
||||||
is( $debarments->[0]->{type}, 'TWO', 'Debarments update with restrictions' );
|
is( $debarments->[0]->{type}, 'TWO', 'Debarments update with restrictions' );
|
||||||
|
|
||||||
$schema->storage->txn_rollback;
|
$schema->storage->txn_rollback;
|
||||||
|
|
Loading…
Reference in a new issue