From 7af422940051e176562b2fe3941f7ec4fa509419 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 4 Dec 2013 16:26:59 +0100 Subject: [PATCH] Bug 10855: Squash several fixes Bug 10855: FIX: Add additional fields for closed subscriptions Bug 10855: Fix instance vs static method This patch fixes the error message. The 3 modified routines *are* static methods. Bug 10855: FIX conflicts with bug 7688 Bug 10855: Fix typo addition_fields -> additional_fields Bug 10855: A partial search should return the subscriptions If a search on an additional fields is done using a partial string ("foo" and the defined value is "foobar"), the subscription should appear in the result list. Test plan: Try to search a part of the string for an additional field. Bug 10855: Filtering on additional fields don't work if value is equal to 0 If you tried to filter on an additional field linked to an authorised value, that did not work if the value was 0. Bug 10855: Remove the advanced serial search box on the serial home page Bug 10855: FIX an add field should not be created if the marc field does not exist. This patch fixes the following: Create an add fields linked to a nonexistent marc field (does not exist in serials). Edit a subscription and save. Without this patch, an error occured: Software error: DBD::mysql::db do failed: Column 'value' cannot be null at /var/root-koha/bug-10855/Koha/AdditionalField.pm line 107. Bug 10855: Fix Type table vs tablename Koha::AdditionalField->all method take "tablename" not "table" in parameter". --- C4/Serials.pm | 5 +- Koha/AdditionalField.pm | 12 +++-- .../prog/en/modules/serials/serials-home.tt | 1 - .../prog/en/modules/serials/serials-search.tt | 29 ++++++++--- .../prog/en/modules/serials/subscription-add.tt | 8 +-- serials/claims.pl | 2 +- serials/serials-search.pl | 5 +- serials/subscription-add.pl | 12 ++--- t/db_dependent/AdditionalField.t | 51 ++++++++++++++------ 9 files changed, 82 insertions(+), 43 deletions(-) diff --git a/C4/Serials.pm b/C4/Serials.pm index 409fa10..da49a2e 100644 --- a/C4/Serials.pm +++ b/C4/Serials.pm @@ -655,8 +655,9 @@ sub SearchSubscriptions { my $matching_record_ids_for_additional_fields = []; if ( @$additional_fields ) { $matching_record_ids_for_additional_fields = Koha::AdditionalField->get_matching_record_ids({ - fields => $args->{additional_fields}, + fields => $additional_fields, tablename => 'subscription', + exact_match => 0, }); return () unless @$matching_record_ids_for_additional_fields; } @@ -746,7 +747,7 @@ sub SearchSubscriptions { record_id => $subscription->{subscriptionid}, tablename => 'subscription' }); - $subscription->{addition_fields} = $additional_field_values->{$subscription->{subscriptionid}}; + $subscription->{additional_fields} = $additional_field_values->{$subscription->{subscriptionid}}; } return @$results; diff --git a/Koha/AdditionalField.pm b/Koha/AdditionalField.pm index de6f763..2f3f44b 100644 --- a/Koha/AdditionalField.pm +++ b/Koha/AdditionalField.pm @@ -97,6 +97,7 @@ sub insert_values { my $dbh = C4::Context->dbh; local $dbh->{RaiseError} = 1; while ( my ( $record_id, $value ) = each %{$self->{values}} ) { + next unless defined $value; my $updated = $dbh->do(q| UPDATE additional_field_values SET value = ? @@ -135,7 +136,7 @@ sub fetch_values { sub all { my ( $class, $args ) = @_; - die "BAD CALL: Don't use fetch_all_values as a static method" + die "BAD CALL: Don't use fetch_all_values as an instance method" if ref $class and UNIVERSAL::can($class,'can'); my $tablename = $args->{tablename}; my $searchable = $args->{searchable}; @@ -172,7 +173,7 @@ sub all { sub fetch_all_values { my ( $class, $args ) = @_; - die "BAD CALL: Don't use fetch_all_values as a static method" + die "BAD CALL: Don't use fetch_all_values as an instance method" if ref $class and UNIVERSAL::can($class,'can'); my $record_id = $args->{record_id}; @@ -199,11 +200,12 @@ sub fetch_all_values { sub get_matching_record_ids { my ( $class, $args ) = @_; - die "BAD CALL: Don't use fetch_all_values as a static method" + die "BAD CALL: Don't use fetch_all_values as an instance method" if ref $class and UNIVERSAL::can($class,'can'); my $fields = $args->{fields} // []; my $tablename = $args->{tablename}; + my $exact_match = $args->{exact_match} // 1; return [] unless @$fields; my $dbh = C4::Context->dbh; @@ -222,13 +224,13 @@ sub get_matching_record_ids { WHERE afv.field_id = af.id AND af.name = ? AND af.tablename = ? - AND value = ? + AND value LIKE ? ) AS field$i USING (id) WHERE field$i.id IS NOT NULL ) AS values$i |; $subquery .= ' USING (record_id)' if $i > 1; push @subqueries, $subquery; - push @args, $field->{name}, $tablename, $field->{value}; + push @args, $field->{name}, $tablename, ( $exact_match ? $field->{value} : "%$field->{value}%" ); } $query .= join( ' LEFT JOIN ', @subqueries ) . ' WHERE 1'; for my $j ( 1 .. $i ) { diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-home.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-home.tt index 4037b45..eaafc94 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-home.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-home.tt @@ -17,7 +17,6 @@
- [% INCLUDE 'subscriptions-search.inc' %] [% INCLUDE 'serials-menu.inc' %]
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-search.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-search.tt index 14ba7d2..089311a 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-search.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-search.tt @@ -98,16 +98,16 @@ [% FOR field IN additional_fields_for_subscription %]
  • - + [% IF field.authorised_value_choices %] - [% FOREACH av IN field.authorised_value_choices %] [% END %] [% ELSE %] - + [% END %]
    • [% END %] @@ -186,8 +186,6 @@ [% IF ( subscription.enddate ) %][% subscription.enddate | $KohaDates %][% END %] - [% IF ( routing && CAN_user_serials_routing ) %] - [% END %] [% FOR field IN additional_fields_for_subscription %] [% IF field.authorised_value_category %] [% KohaAuthorisedValues.GetByCode( field.authorised_value_category, subscription.additional_fields.${field.name} ) %] @@ -197,6 +195,7 @@ [% END %] Issue history + [% IF ( routing && CAN_user_serials_routing ) %] [% IF ( subscription.cannotedit ) %]   @@ -209,6 +208,7 @@ [% END %] [% END %] + [% END %] [% IF ( CAN_user_serials_receive_serials ) %] Serial receive @@ -236,6 +236,9 @@ Notes Library Call number + [% FOR field IN additional_fields_for_subscription %] + [% field.name %] + [% END %]     @@ -247,6 +250,9 @@ + [% FOR field IN additional_fields_for_subscription %] + + [% END %] @@ -273,6 +279,13 @@ [% IF ( subscription.callnumber ) %][% subscription.callnumber %][% END %] + [% FOR field IN additional_fields_for_subscription %] + [% IF field.authorised_value_category %] + [% KohaAuthorisedValues.GetByCode( field.authorised_value_category, subscription.additional_fields.${field.name} ) %] + [% ELSE %] + [% subscription.additional_fields.${field.name} %] + [% END %] + [% END %] [% UNLESS subscription.cannotedit %] Reopen @@ -349,9 +362,9 @@ [% FOR field IN additional_fields_for_subscription %]
  • - + [% IF field.authorised_value_choices %] - [% FOREACH av IN field.authorised_value_choices %] [% IF av.authorised_value == additional_field_filters.${field.name} %] @@ -362,7 +375,7 @@ [% END %] [% ELSE %] - + [% END %]
    • [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt index 2a963bf..f52cc72 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt @@ -841,9 +841,9 @@ $(document).ready(function() {
      [% FOR field IN additional_fields_for_subscription %]
    1. - + [% IF field.authorised_value_choices %] - [% FOREACH av IN field.authorised_value_choices %] [% IF av.authorised_value == additional_fields.${field.name} %] @@ -854,10 +854,10 @@ $(document).ready(function() { (Authorised values for [% field.authorised_value_category %]) [% ELSE %] [% IF field.marcfield %] - + This value will be filled with the [% field.marcfield %] subfield of the selected biblio. [% ELSE %] - + [% END %] [% END %]
      2. diff --git a/serials/claims.pl b/serials/claims.pl index d1af723..443b3b3 100755 --- a/serials/claims.pl +++ b/serials/claims.pl @@ -60,7 +60,7 @@ for my $s (@{$supplierlist} ) { } } -my $additional_fields = Koha::AdditionalField->all( { table => 'subscription', searchable => 1 } ); +my $additional_fields = Koha::AdditionalField->all( { tablename => 'subscription', searchable => 1 } ); for my $field ( @$additional_fields ) { if ( $field->{authorised_value_category} ) { $field->{authorised_value_choices} = GetAuthorisedValues( $field->{authorised_value_category} ); diff --git a/serials/serials-search.pl b/serials/serials-search.pl index 881ef8c..f1b48f8 100755 --- a/serials/serials-search.pl +++ b/serials/serials-search.pl @@ -73,10 +73,11 @@ if ( $op and $op eq "close" ) { } -my $additional_fields = Koha::AdditionalField->all( { table => 'subscription', searchable => 1 } ); +my $additional_fields = Koha::AdditionalField->all( { tablename => 'subscription', searchable => 1 } ); my $additional_field_filters; for my $field ( @$additional_fields ) { - if ( my $filter_value = $query->param('additional_field_' . $field->{name} . '_filter') ) { + my $filter_value = $query->param('additional_field_' . $field->{id} . '_filter'); + if ( defined ( $filter_value ) ) { $additional_field_filters->{ $field->{name} } = $filter_value; } if ( $field->{authorised_value_category} ) { diff --git a/serials/subscription-add.pl b/serials/subscription-add.pl index 5b8e352..ff7e006 100755 --- a/serials/subscription-add.pl +++ b/serials/subscription-add.pl @@ -155,7 +155,7 @@ $template->param(branchloop => $branchloop, ); -my $additional_fields = Koha::AdditionalField->all( { table => 'subscription' } ); +my $additional_fields = Koha::AdditionalField->all( { tablename => 'subscription' } ); for my $field ( @$additional_fields ) { if ( $field->{authorised_value_category} ) { $field->{authorised_value_choices} = GetAuthorisedValues( $field->{authorised_value_category} ); @@ -349,13 +349,13 @@ sub redirect_add_subscription { $skip_serialseq ); - my $additional_fields = Koha::AdditionalField->all( { table => 'subscription' } ); + my $additional_fields = Koha::AdditionalField->all( { tablename => 'subscription' } ); my @additional_field_values; for my $field ( @$additional_fields ) { my $af = Koha::AdditionalField->new({ id => $field->{id} }); $af->{values} = { - $subscriptionid => $query->param('additional_fields_' . $field->{name}) - }; + $subscriptionid => $query->param('additional_field_' . $field->{id}) + } if defined $query->param('additional_field_' . $field->{id}); $af->insert_values; } @@ -434,7 +434,7 @@ sub redirect_mod_subscription { $skip_serialseq ); - my $additional_fields = Koha::AdditionalField->all( { table => 'subscription' } ); + my $additional_fields = Koha::AdditionalField->all( { tablename => 'subscription' } ); my @additional_field_values; for my $field ( @$additional_fields ) { my $af = Koha::AdditionalField->new({ id => $field->{id} })->fetch; @@ -448,7 +448,7 @@ sub redirect_mod_subscription { }; } else { $af->{values} = { - $subscriptionid => $query->param('additional_fields_' . $field->{name}) + $subscriptionid => $query->param('additional_field_' . $field->{id}) }; } $af->insert_values; diff --git a/t/db_dependent/AdditionalField.t b/t/db_dependent/AdditionalField.t index f7f5227..df307a4 100644 --- a/t/db_dependent/AdditionalField.t +++ b/t/db_dependent/AdditionalField.t @@ -1,7 +1,7 @@ #!/usr/bin/perl use Modern::Perl; -use Test::More tests => 34; +use Test::More tests => 37; use C4::Context; use Koha::AdditionalField; @@ -104,6 +104,8 @@ use C4::Acquisition; use C4::Biblio; use C4::Budgets; use C4::Serials; +use C4::Serials::Frequency; +use C4::Serials::Numberpattern; my $booksellerid = C4::Bookseller::AddBookseller( { @@ -118,7 +120,7 @@ my ($biblionumber, $biblioitemnumber) = AddBiblio(MARC::Record->new, ''); my $budgetid; my $bpid = AddBudgetPeriod({ budget_period_startdate => '01-01-2015', - budget_period_enddate => '12-31-2015', + budget_period_enddate => '01-01-2016', budget_description => "budget desc" }); @@ -132,22 +134,28 @@ my $budget_id = AddBudget({ budget_period_id => $bpid }); +my $frequency_id = AddSubscriptionFrequency({ description => "Test frequency 1" }); +my $pattern_id = AddSubscriptionNumberpattern({ + label => 'Test numberpattern 1', + numberingmethod => '{X}' +}); + my $subscriptionid1 = NewSubscription( - undef, "", undef, undef, $budget_id, $biblionumber, '01-01-2013',undef, - undef, undef, undef, undef, undef, undef, undef, undef, - undef, undef, undef, undef, undef, undef, undef, undef, - undef, undef, undef, undef, undef, undef, undef, 1, - "notes", undef, undef, undef, undef, undef, undef, 0, - "intnotes", 0, undef, undef, 0, undef, '31-12-2013', + undef, "", undef, undef, $budget_id, $biblionumber, + '2013-01-01', $frequency_id, undef, undef, undef, + undef, undef, undef, undef, undef, undef, + 1, "notes",undef, '2013-01-01', undef, $pattern_id, + undef, undef, 0, "intnotes", 0, + undef, undef, 0, undef, '2013-01-01', 0 ); my $subscriptionid2 = NewSubscription( - undef, "", undef, undef, $budget_id, $biblionumber, '01-01-2013',undef, - undef, undef, undef, undef, undef, undef, undef, undef, - undef, undef, undef, undef, undef, undef, undef, undef, - undef, undef, undef, undef, undef, undef, undef, 1, - "notes", undef, undef, undef, undef, undef, undef, 0, - "intnotes", 0, undef, undef, 0, undef, '31-12-2013', + undef, "", undef, undef, $budget_id, $biblionumber, + '2013-01-01', $frequency_id, undef, undef, undef, + undef, undef, undef, undef, undef, undef, + 1, "notes",undef, '2013-01-01', undef, $pattern_id, + undef, undef, 0, "intnotes", 0, + undef, undef, 0, undef, '2013-01-01', 0 ); # insert @@ -268,4 +276,19 @@ is ( $exists, 1, "get_matching_record_ids: field common: common_value matches su $exists = grep /not_existent_id/, @$matching_record_ids; is ( $exists, 0, "get_matching_record_ids: field common: common_value does not inexistent id" ); +$fields = [ + { + name => 'common', + value => q|common|, + } +]; +$matching_record_ids = Koha::AdditionalField->get_matching_record_ids({ tablename => 'subscription', fields => $fields, exact_match => 0 }); +$exists = grep /$subscriptionid1/, @$matching_record_ids; +is ( $exists, 1, "get_matching_record_ids: field common: common% matches subscription1" ); +$exists = grep /$subscriptionid2/, @$matching_record_ids; +is ( $exists, 1, "get_matching_record_ids: field common: common% matches subscription2 too" ); +$exists = grep /not_existent_id/, @$matching_record_ids; +is ( $exists, 0, "get_matching_record_ids: field common: common% does not inexistent id" ); + + $dbh->rollback; -- 1.7.10.4