Bug 15774: (follow-up) Address QA issues

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This commit is contained in:
Nick Clemens 2018-11-01 18:32:05 +00:00
parent feb866a618
commit 8035151e41
13 changed files with 93 additions and 32 deletions

View File

@ -2445,8 +2445,9 @@ sub GetHistory {
if ( @$ordernumbers ) {
$query .= ' AND (aqorders.ordernumber IN ( ' . join (',', ('?') x @$ordernumbers ) . '))';
push @query_params, @$ordernumbers;
}
if ( @$additional_fields ) {
my @baskets = Koha::Acquisition::Baskets->search_additional_fields($additional_fields);
my @baskets = Koha::Acquisition::Baskets->filter_by_additional_fields($additional_fields);
return [] unless @baskets;

View File

@ -528,7 +528,7 @@ sub SearchSubscriptions {
my $additional_fields = $args->{additional_fields} // [];
my $matching_record_ids_for_additional_fields = [];
if ( @$additional_fields ) {
my @subscriptions = Koha::Subscriptions->search_additional_fields($additional_fields);
my @subscriptions = Koha::Subscriptions->filter_by_additional_fields($additional_fields);
return () unless @subscriptions;

View File

@ -1,5 +1,9 @@
package Koha::AdditionalFieldValue;
use Modern::Perl;
use base 'Koha::Object';
=head1 NAME
Koha::AdditionalFieldValue - Koha::Object derived class for additional field
@ -7,9 +11,27 @@ values
=cut
use Modern::Perl;
=head2 Class methods
use base 'Koha::Object';
=cut
=head3 field
Return the Koha:AdditionalField object for this AdditionalFeidlValue
=cut
sub field {
my ( $self ) = @_;
return Koha::AdditionalField->_new_from_dbic( $self->_result()->field() );
}
=head2 Internal methods
=head3 _type
=cut
sub _type { 'AdditionalFieldValue' }

View File

@ -8,6 +8,7 @@ values
=cut
use Modern::Perl;
use Koha::AdditionalFieldValue;
use base 'Koha::Objects';

View File

@ -1,6 +1,7 @@
package Koha::Object::Mixin::AdditionalFields;
use Modern::Perl;
use Koha::AdditionalFieldValues;
=head1 NAME
@ -43,18 +44,16 @@ Koha::Object::Mixin::AdditionalFields
sub set_additional_fields {
my ($self, $additional_fields) = @_;
my $rs = Koha::Database->new->schema->resultset('AdditionalFieldValue');
my @additional_field_values = $self->additional_field_values->delete;
foreach my $additional_field (@$additional_fields) {
my $field_value = $rs->find_or_new({
field_id => $additional_field->{id},
record_id => $self->id,
});
my $value = $additional_field->{value};
if (defined $value) {
$field_value->set_columns({ value => $value })->update_or_insert;
} elsif ($field_value->in_storage) {
$field_value->delete;
my $field_value = Koha::AdditionalFieldValue->new({
field_id => $additional_field->{id},
record_id => $self->id,
value => $value,
})->store;
}
}
}
@ -70,7 +69,8 @@ Returns additional field values
sub additional_field_values {
my ($self) = @_;
return $self->_result->additional_field_values;
my $afv_rs = $self->_result->additional_field_values;
return Koha::AdditionalFieldValues->_new_from_dbic( $afv_rs );
}
=head1 AUTHOR

View File

@ -20,15 +20,15 @@ Koha::Objects::Mixin::AdditionalFields
use Koha::Foos;
Koha::Foos->search_additional_fields(...)
Koha::Foos->filter_by_additional_fields(...)
=head1 API
=head2 Public methods
=head3 search_additional_fields
=head3 filter_by_additional_fields
my @objects = Koha::Foos->search_additional_fields([
my @objects = Koha::Foos->filter_by_additional_fields([
{
id => 1,
value => 'foo',
@ -41,7 +41,7 @@ Koha::Objects::Mixin::AdditionalFields
=cut
sub search_additional_fields {
sub filter_by_additional_fields {
my ($class, $additional_fields) = @_;
my %conditions;

View File

@ -321,7 +321,6 @@ __PACKAGE__->has_many(
return {
"$args->{foreign_alias}.record_id" => { -ident => "$args->{self_alias}.basketno" },
# TODO Add column additional_field_values.tablename to avoid subquery ?
"$args->{foreign_alias}.field_id" =>
{ -in => \'(SELECT id FROM additional_fields WHERE tablename = "aqbasket")' },
};

View File

@ -464,7 +464,6 @@ __PACKAGE__->has_many(
return {
"$args->{foreign_alias}.record_id" => { -ident => "$args->{self_alias}.subscriptionid" },
# TODO Add column additional_field_values.tablename to avoid subquery ?
"$args->{foreign_alias}.field_id" =>
{ -in => \'(SELECT id FROM additional_fields WHERE tablename = "subscription")' },
};

View File

@ -436,7 +436,7 @@ if ( $op eq 'list' ) {
available_additional_fields => [ Koha::AdditionalFields->search( { tablename => 'aqbasket' } ) ],
additional_field_values => { map {
$_->field->name => $_->value
} Koha::Acquisition::Baskets->find($basketno)->additional_field_values },
} Koha::Acquisition::Baskets->find($basketno)->additional_field_values->as_list },
);
}

View File

@ -101,7 +101,7 @@ if ( $op eq 'add_form' ) {
$template->param(
additional_field_values => { map {
$_->field->id => $_->value
} Koha::Acquisition::Baskets->find($basketno)->additional_field_values },
} Koha::Acquisition::Baskets->find($basketno)->additional_field_values->as_list },
);
} else {
#new basket
@ -171,7 +171,8 @@ if ( $op eq 'add_form' ) {
}
my @additional_fields;
for my $field (Koha::AdditionalFields->search({ tablename => 'aqbasket' })) {
my $basket_fields = Koha::AdditionalFields->search({ tablename => 'aqbasket' });
while ( my $field = $basket_fields->next ) {
my $value = $input->param('additional_field_' . $field->id);
push @additional_fields, {
id => $field->id,

View File

@ -148,7 +148,7 @@ my @additional_fields = Koha::AdditionalFields->search({ tablename => 'subscript
my %additional_field_values;
if ($subscriptionid) {
my $subscription = Koha::Subscriptions->find($subscriptionid);
foreach my $value ($subscription->additional_field_values) {
foreach my $value ($subscription->additional_field_values->as_list) {
$additional_field_values{$value->field_id} = $value->value;
}
}
@ -384,7 +384,8 @@ sub redirect_add_subscription {
my @additional_fields;
my $record = GetMarcBiblio({ biblionumber => $biblionumber, embed_items => 1 });
for my $field (Koha::AdditionalFields->search({ tablename => 'subscription' })) {
my $basket_fields = Koha::AdditionalFields->search({ tablename => 'aqbasket' });
while ( my $field = $basket_fields->next ) {
my $value = $query->param('additional_field_' . $field->id);
if ($field->marcfield) {
my ($field, $subfield) = split /\$/, $field->marcfield;
@ -502,7 +503,8 @@ sub redirect_mod_subscription {
my @additional_fields;
my $record = GetMarcBiblio({ biblionumber => $biblionumber, embed_items => 1 });
for my $field (Koha::AdditionalFields->search({ tablename => 'subscription' })) {
my $basket_fields = Koha::AdditionalFields->search({ tablename => 'aqbasket' });
while ( my $field = $basket_fields->next ) {
my $value = $query->param('additional_field_' . $field->id);
if ($field->marcfield) {
my ($field, $subfield) = split /\$/, $field->marcfield;

View File

@ -19,9 +19,10 @@ use Modern::Perl;
use POSIX qw(strftime);
use Test::More tests => 73;
use Test::More tests => 74;
use t::lib::Mocks;
use Koha::Database;
use Koha::Acquisition::Basket;
use MARC::File::XML ( BinaryEncoding => 'utf8', RecordFormat => 'MARC21' );
@ -754,4 +755,39 @@ subtest 'ModReceiveOrder and subscription' => sub {
is( $order->get_from_storage->order_internalnote, $first_note );
};
subtest 'GetHistory with additional fields' => sub {
plan tests => 3;
my $builder = t::lib::TestBuilder->new;
my $order_basket = $builder->build({ source => 'Aqbasket', value => { is_standing => 0 } });
my $orderinfo ={
basketno => $order_basket->{basketno},
rrp => 19.99,
replacementprice => undef,
quantity => 1,
quantityreceived => 0,
datereceived => undef,
datecancellationprinted => undef,
};
my $order = $builder->build({ source => 'Aqorder', value => $orderinfo });
my $history = GetHistory(ordernumber => $order->{ordernumber});
is( scalar( @$history ), 1, 'GetHistory returns the one order');
my $additional_field = $builder->build({source => 'AdditionalField', value => {
tablename => 'aqbasket',
name => 'snakeoil',
authorised_value_category => "",
}
});
$history = GetHistory( ordernumber => $order->{ordernumber}, additional_fields => [{ id => $additional_field->{id}, value=>'delicious'}]);
is( scalar ( @$history ), 0, 'GetHistory returns no order for an unused additional field');
my $basket = Koha::Acquisition::Baskets->find({ basketno => $order_basket->{basketno} });
$basket->set_additional_fields([{
id => $additional_field->{id},
value => 'delicious',
}]);
$history = GetHistory( ordernumber => $order->{ordernumber}, additional_fields => [{ id => $additional_field->{id}, value=>'delicious'}]);
is( scalar( @$history ), 1, 'GetHistory returns the order when additional field is set');
};
$schema->storage->txn_rollback();

View File

@ -53,7 +53,7 @@ Koha::AdditionalFieldValue->new({
value => 'bar value for basket2',
})->store;
my @baskets = Koha::Acquisition::Baskets->search_additional_fields([
my @baskets = Koha::Acquisition::Baskets->filter_by_additional_fields([
{
id => $foo->id,
value => 'foo value for basket1',
@ -63,7 +63,7 @@ my @baskets = Koha::Acquisition::Baskets->search_additional_fields([
is(scalar @baskets, 1, 'search returns only one result');
is($baskets[0]->basketno, $basket1->basketno, 'result is basket1');
@baskets = Koha::Acquisition::Baskets->search_additional_fields([
@baskets = Koha::Acquisition::Baskets->filter_by_additional_fields([
{
id => $foo->id,
value => 'foo value for basket2',
@ -73,7 +73,7 @@ is($baskets[0]->basketno, $basket1->basketno, 'result is basket1');
is(scalar @baskets, 1, 'search returns only one result');
is($baskets[0]->basketno, $basket2->basketno, 'result is basket2');
@baskets = Koha::Acquisition::Baskets->search_additional_fields([
@baskets = Koha::Acquisition::Baskets->filter_by_additional_fields([
{
id => $foo->id,
value => 'foo value for basket1',
@ -87,7 +87,7 @@ is($baskets[0]->basketno, $basket2->basketno, 'result is basket2');
is(scalar @baskets, 1, 'search returns only one result');
is($baskets[0]->basketno, $basket1->basketno, 'result is basket1');
@baskets = Koha::Acquisition::Baskets->search_additional_fields([
@baskets = Koha::Acquisition::Baskets->filter_by_additional_fields([
{
id => $foo->id,
value => 'foo value for basket1',
@ -100,7 +100,7 @@ is($baskets[0]->basketno, $basket1->basketno, 'result is basket1');
is(scalar @baskets, 0, 'search returns no result');
@baskets = Koha::Acquisition::Baskets->search_additional_fields([
@baskets = Koha::Acquisition::Baskets->filter_by_additional_fields([
{
id => $foo->id,
value => 'foo',
@ -109,7 +109,7 @@ is(scalar @baskets, 0, 'search returns no result');
is(scalar @baskets, 2, 'search returns two results');
@baskets = Koha::Acquisition::Baskets->search_additional_fields([
@baskets = Koha::Acquisition::Baskets->filter_by_additional_fields([
{
id => $foo->id,
value => 'foo',