From 904a4884601588ca89faaaee5e0fd53d3907ec10 Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Fri, 4 May 2018 17:35:45 +0200 Subject: [PATCH] Bug 15774: Use Koha::Object(s) for additional fields MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit A lot of code can be removed just by using Koha::Object It also makes fetching and updating additional field values easier. Signed-off-by: Séverine QUEUNE Signed-off-by: Jonathan Druart Signed-off-by: Josef Moravec Signed-off-by: Nick Clemens --- C4/Acquisition.pm | 12 +- C4/Serials.pm | 51 +- Koha/Acquisition/Basket.pm | 2 +- Koha/Acquisition/Baskets.pm | 2 +- Koha/AdditionalField.pm | 500 +----------------- Koha/AdditionalFieldValue.pm | 9 + Koha/AdditionalFieldValues.pm | 10 + Koha/AdditionalFields.pm | 12 + Koha/Object/Mixin/AdditionalFields.pm | 68 +++ Koha/Objects/Mixin/AdditionalFields.pm | 59 +++ Koha/Schema/Result/Aqbasket.pm | 16 + Koha/Schema/Result/Subscription.pm | 16 + Koha/Subscription.pm | 2 +- Koha/Subscriptions.pm | 2 +- acqui/basket.pl | 12 +- acqui/basketheader.pl | 26 +- acqui/histsearch.pl | 20 +- admin/additional-fields.pl | 18 +- .../en/includes/additional-fields-entry.inc | 6 +- .../en/modules/admin/additional-fields.tt | 23 +- .../prog/en/modules/serials/serials-search.tt | 4 +- serials/claims.pl | 10 +- serials/serials-search.pl | 24 +- serials/subscription-add.pl | 48 +- serials/subscription-batchedit.pl | 30 +- serials/subscription-detail.pl | 8 +- t/db_dependent/AdditionalField.t | 293 ---------- .../Koha/Objects/Mixin/AdditionalFields.t | 126 +++++ 28 files changed, 484 insertions(+), 925 deletions(-) create mode 100644 Koha/AdditionalFieldValue.pm create mode 100644 Koha/AdditionalFieldValues.pm create mode 100644 Koha/AdditionalFields.pm create mode 100644 Koha/Object/Mixin/AdditionalFields.pm create mode 100644 Koha/Objects/Mixin/AdditionalFields.pm delete mode 100644 t/db_dependent/AdditionalField.t create mode 100755 t/db_dependent/Koha/Objects/Mixin/AdditionalFields.t diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm index 8ed768b2db..9200318ce3 100644 --- a/C4/Acquisition.pm +++ b/C4/Acquisition.pm @@ -28,6 +28,7 @@ use C4::Contract; use C4::Debug; use C4::Templates qw(gettemplate); use Koha::DateUtils qw( dt_from_string output_pref ); +use Koha::Acquisition::Baskets; use Koha::Acquisition::Booksellers; use Koha::Acquisition::Orders; use Koha::Biblios; @@ -2445,15 +2446,12 @@ sub GetHistory { $query .= ' AND (aqorders.ordernumber IN ( ' . join (',', ('?') x @$ordernumbers ) . '))'; push @query_params, @$ordernumbers; if ( @$additional_fields ) { - my $matching_record_ids_for_additional_fields = Koha::AdditionalField->get_matching_record_ids( { - fields => $additional_fields, - tablename => 'aqbasket', - exact_match => 0, - } ); - return [] unless @$matching_record_ids_for_additional_fields; + my @baskets = Koha::Acquisition::Baskets->search_additional_fields($additional_fields); + + return [] unless @baskets; # No parameterization because record IDs come directly from DB - $query .= ' AND aqbasket.basketno IN ( ' . join( ',', @$matching_record_ids_for_additional_fields ) . ' )'; + $query .= ' AND aqbasket.basketno IN ( ' . join( ',', map { $_->basketno } @baskets ) . ' )'; } if ( C4::Context->preference("IndependentBranches") ) { diff --git a/C4/Serials.pm b/C4/Serials.pm index e929e219ef..b4452a31dc 100644 --- a/C4/Serials.pm +++ b/C4/Serials.pm @@ -30,7 +30,7 @@ use C4::Log; # logaction use C4::Debug; use C4::Serials::Frequency; use C4::Serials::Numberpattern; -use Koha::AdditionalField; +use Koha::AdditionalFieldValues; use Koha::DateUtils; use Koha::Serial; use Koha::Subscriptions; @@ -275,11 +275,10 @@ sub GetSubscription { $subscription->{cannotedit} = not can_edit_subscription( $subscription ); # Add additional fields to the subscription into a new key "additional_fields" - my $additional_field_values = Koha::AdditionalField->fetch_all_values({ - tablename => 'subscription', - record_id => $subscriptionid, - }); - $subscription->{additional_fields} = $additional_field_values->{$subscriptionid}; + my %additional_field_values = map { + $_->field->name => $_->value + } Koha::Subscriptions->find($subscriptionid)->additional_field_values; + $subscription->{additional_fields} = \%additional_field_values; if ( my $mana_id = $subscription->{mana_id} ) { my $mana_subscription = Koha::SharedContent::get_entity_by_id( @@ -529,12 +528,13 @@ sub SearchSubscriptions { my $additional_fields = $args->{additional_fields} // []; my $matching_record_ids_for_additional_fields = []; if ( @$additional_fields ) { - $matching_record_ids_for_additional_fields = Koha::AdditionalField->get_matching_record_ids({ - fields => $additional_fields, - tablename => 'subscription', - exact_match => 0, - }); - return () unless @$matching_record_ids_for_additional_fields; + my @subscriptions = Koha::Subscriptions->search_additional_fields($additional_fields); + + return () unless @subscriptions; + + $matching_record_ids_for_additional_fields = [ map { + $_->subscriptionid + } @subscriptions ]; } my $query = q| @@ -631,11 +631,10 @@ sub SearchSubscriptions { $subscription->{cannotedit} = not can_edit_subscription( $subscription ); $subscription->{cannotdisplay} = not can_show_subscription( $subscription ); - my $additional_field_values = Koha::AdditionalField->fetch_all_values({ - record_id => $subscription->{subscriptionid}, - tablename => 'subscription' - }); - $subscription->{additional_fields} = $additional_field_values->{$subscription->{subscriptionid}}; + my %additional_field_values = map { + $_->field->name => $_->value + } Koha::Subscriptions->find($subscription->{subscriptionid})->additional_field_values; + $subscription->{additional_fields} = \%additional_field_values; } return @$results; @@ -1700,10 +1699,10 @@ sub DelSubscription { $dbh->do("DELETE FROM subscriptionhistory WHERE subscriptionid=?", undef, $subscriptionid); $dbh->do("DELETE FROM serial WHERE subscriptionid=?", undef, $subscriptionid); - my $afs = Koha::AdditionalField->all({tablename => 'subscription'}); - foreach my $af (@$afs) { - $af->delete_values({record_id => $subscriptionid}); - } + Koha::AdditionalFieldValues->search({ + 'field.tablename' => 'subscription', + 'me.record_id' => $subscriptionid, + }, { join => 'field' })->delete; logaction( "SERIAL", "DELETE", $subscriptionid, "" ) if C4::Context->preference("SubscriptionLog"); } @@ -1833,11 +1832,11 @@ sub GetLateOrMissingIssues { } $line->{"status".$line->{status}} = 1; - my $additional_field_values = Koha::AdditionalField->fetch_all_values({ - record_id => $line->{subscriptionid}, - tablename => 'subscription' - }); - %$line = ( %$line, additional_fields => $additional_field_values->{$line->{subscriptionid}} ); + my $subscription = Koha::Subscriptions->find($line->{subscriptionid}); + my %additional_field_values = map { + $_->field->name => $_->value + } $subscription->additional_field_values; + %$line = ( %$line, additional_fields => \%additional_field_values ); push @issuelist, $line; } diff --git a/Koha/Acquisition/Basket.pm b/Koha/Acquisition/Basket.pm index d5e6b8ccb1..5bda3ab303 100644 --- a/Koha/Acquisition/Basket.pm +++ b/Koha/Acquisition/Basket.pm @@ -22,7 +22,7 @@ use Modern::Perl; use Koha::Database; use Koha::Acquisition::BasketGroups; -use base qw( Koha::Object ); +use base qw( Koha::Object Koha::Object::Mixin::AdditionalFields ); =head1 NAME diff --git a/Koha/Acquisition/Baskets.pm b/Koha/Acquisition/Baskets.pm index 2de46835a7..3dc041401e 100644 --- a/Koha/Acquisition/Baskets.pm +++ b/Koha/Acquisition/Baskets.pm @@ -22,7 +22,7 @@ use Modern::Perl; use Koha::Database; use Koha::Acquisition::Basket; -use base qw( Koha::Objects ); +use base qw( Koha::Objects Koha::Objects::Mixin::AdditionalFields ); =head1 NAME diff --git a/Koha/AdditionalField.pm b/Koha/AdditionalField.pm index 7c4903a65a..78de775d5e 100644 --- a/Koha/AdditionalField.pm +++ b/Koha/AdditionalField.pm @@ -2,325 +2,11 @@ package Koha::AdditionalField; use Modern::Perl; -use base qw(Class::Accessor); +use base qw(Koha::Object); use C4::Context; -__PACKAGE__->mk_accessors(qw( id tablename name authorised_value_category marcfield searchable values )); - -sub new { - my ( $class, $args ) = @_; - - my $additional_field = { - id => $args->{id} // q||, - tablename => $args->{tablename} // q||, - name => $args->{name} // q||, - authorised_value_category => $args->{authorised_value_category} // q||, - marcfield => $args->{marcfield} // q||, - searchable => $args->{searchable} // 0, - values => $args->{values} // {}, - }; - - my $self = $class->SUPER::new( $additional_field ); - - bless $self, $class; - return $self; -} - -sub fetch { - my ( $self ) = @_; - my $dbh = C4::Context->dbh; - my $field_id = $self->id; - return unless $field_id; - my $data = $dbh->selectrow_hashref( - q| - SELECT id, tablename, name, authorised_value_category, marcfield, searchable - FROM additional_fields - WHERE id = ? - |, - {}, ( $field_id ) - ); - - die "This additional field does not exist (id=$field_id)" unless $data; - $self->{id} = $data->{id}; - $self->{tablename} = $data->{tablename}; - $self->{name} = $data->{name}; - $self->{authorised_value_category} = $data->{authorised_value_category}; - $self->{marcfield} = $data->{marcfield}; - $self->{searchable} = $data->{searchable}; - return $self; -} - -sub update { - my ( $self ) = @_; - - die "There is no id defined for this additional field. I cannot update it" unless $self->{id}; - - my $dbh = C4::Context->dbh; - local $dbh->{RaiseError} = 1; - - return $dbh->do(q| - UPDATE additional_fields - SET name = ?, - authorised_value_category = ?, - marcfield = ?, - searchable = ? - WHERE id = ? - |, {}, ( $self->{name}, $self->{authorised_value_category}, $self->{marcfield}, $self->{searchable}, $self->{id} ) ); -} - -sub delete { - my ( $self ) = @_; - return unless $self->{id}; - my $dbh = C4::Context->dbh; - local $dbh->{RaiseError} = 1; - return $dbh->do(q| - DELETE FROM additional_fields WHERE id = ? - |, {}, ( $self->{id} ) ); -} - -sub insert { - my ( $self ) = @_; - my $dbh = C4::Context->dbh; - local $dbh->{RaiseError} = 1; - $dbh->do(q| - INSERT INTO additional_fields - ( tablename, name, authorised_value_category, marcfield, searchable ) - VALUES ( ?, ?, ?, ?, ? ) - |, {}, ( $self->{tablename}, $self->{name}, $self->{authorised_value_category}, $self->{marcfield}, $self->{searchable} ) ); - $self->{id} = $dbh->{mysql_insertid}; -} - -sub insert_values { - my ( $self ) = @_; - - 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 = ? - WHERE field_id = ? - AND record_id = ? - |, {}, ( $value, $self->{id}, $record_id )); - if ( $updated eq '0E0' ) { - $dbh->do(q| - INSERT INTO additional_field_values( field_id, record_id, value ) - VALUES( ?, ?, ?) - |, {}, ( $self->{id}, $record_id, $value )); - } - } -} - -sub fetch_values { - my ( $self, $args ) = @_; - my $record_id = $args->{record_id}; - my $dbh = C4::Context->dbh; - my $values = $dbh->selectall_arrayref( - q| - SELECT * - FROM additional_fields af, additional_field_values afv - WHERE af.id = afv.field_id - AND af.tablename = ? - AND af.name = ? - | . ( $record_id ? q|AND afv.record_id = ?| : '' ), - {Slice => {}}, ( $self->{tablename}, $self->{name}, ($record_id ? $record_id : () ) ) - ); - - $self->{values} = {}; - for my $v ( @$values ) { - $self->{values}{$v->{record_id}} = $v->{value}; - } -} - -sub delete_values { - my ($self, $args) = @_; - - my $record_id = $args->{record_id}; - - my $dbh = C4::Context->dbh; - - my @where_strs = ('field_id = ?'); - my @where_args = ($self->{id}); - - if ($record_id) { - push @where_strs, 'record_id = ?'; - push @where_args, $record_id; - } - - my $query = q{ - DELETE FROM additional_field_values - WHERE - } . join (' AND ', @where_strs); - - $dbh->do($query, undef, @where_args); -} - -sub all { - my ( $class, $args ) = @_; - 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}; - my $dbh = C4::Context->dbh; - my $query = q| - SELECT * FROM additional_fields WHERE 1 - |; - $query .= q| AND tablename = ?| - if $tablename; - - $query .= q| AND searchable = ?| - if defined $searchable; - - my $results = $dbh->selectall_arrayref( - $query, {Slice => {}}, ( - $tablename ? $tablename : (), - defined $searchable ? $searchable : () - ) - ); - my @fields; - for my $r ( @$results ) { - push @fields, Koha::AdditionalField->new({ - id => $r->{id}, - tablename => $r->{tablename}, - name => $r->{name}, - authorised_value_category => $r->{authorised_value_category}, - marcfield => $r->{marcfield}, - searchable => $r->{searchable}, - }); - } - return \@fields; - -} - -sub fetch_all_values { - my ( $class, $args ) = @_; - 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}; - my $tablename = $args->{tablename}; - return unless $tablename; - - my $dbh = C4::Context->dbh; - my $values = $dbh->selectall_arrayref( - q| - SELECT afv.record_id, af.name, afv.value - FROM additional_fields af, additional_field_values afv - WHERE af.id = afv.field_id - AND af.tablename = ? - | . ( $record_id ? q| AND afv.record_id = ?| : q|| ), - {Slice => {}}, ( $tablename, ($record_id ? $record_id : ()) ) - ); - - my $r; - for my $v ( @$values ) { - $r->{$v->{record_id}}{$v->{name}} = $v->{value}; - } - return $r; -} - -sub get_matching_record_ids { - my ( $class, $args ) = @_; - 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; - my $query = q|SELECT * FROM |; - my ( @subqueries, @args ); - my $i = 0; - for my $field ( @$fields ) { - $i++; - my $subquery = qq|( - SELECT record_id, field$i.name AS field${i}_name - FROM additional_field_values afv - LEFT JOIN - ( - SELECT afv.id, af.name, afv.value - FROM additional_field_values afv, additional_fields af - WHERE afv.field_id = af.id - AND af.name = ? - AND af.tablename = ? - 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, ( ( $exact_match or $field->{authorised_value_category} ) ? $field->{value} : "%$field->{value}%" ); - } - $query .= join( ' LEFT JOIN ', @subqueries ) . ' WHERE 1'; - for my $j ( 1 .. $i ) { - $query .= qq| AND field${j}_name IS NOT NULL|; - } - my $values = $dbh->selectall_arrayref( $query, {Slice => {}}, @args ); - return [ - map { $_->{record_id} } @$values - ] -} - -sub update_fields_from_query { - my ( $class, $args ) = @_; - die "BAD CALL: Don't use update_fields_from_query as an instance method" - if ref $class and UNIVERSAL::can($class,'can'); - - my $query = $args->{query}; - my $additional_fields = Koha::AdditionalField->all( { tablename => $args->{tablename} } ); - for my $field ( @$additional_fields ) { - my $af = Koha::AdditionalField->new({ id => $field->{id} })->fetch; - if ( $af->{marcfield} ) { - my ( $field, $subfield ) = split /\$/, $af->{marcfield}; - $af->{values} = undef; - if ( $args->{marc_record} and $field and $subfield ) { - my $value = $args->{marc_record}->subfield( $field, $subfield ); - $af->{values} = { - $args->{record_id} => $value - }; - } - } else { - $af->{values} = { - $args->{record_id} => $query->param( 'additional_field_' . $field->{id} ) - } if defined $query->param( 'additional_field_' . $field->{id} ); - } - $af->insert_values; - } -} - -sub get_filters_from_query { - my ( $class, $args ) = @_; - die "BAD CALL: Don't use get_filters_from_query as an instance method" - if ref $class and UNIVERSAL::can($class,'can'); - - my $query = $args->{query}; - my $additional_fields = Koha::AdditionalField->all( { tablename => $args->{tablename}, searchable => 1 } ); - my @additional_field_filters; - for my $field ( @$additional_fields ) { - my $filter_value = $query->param( 'additional_field_' . $field->{id} ); - if ( defined $filter_value and $filter_value ne q|| ) { - push @additional_field_filters, { - name => $field->{name}, - value => $filter_value, - authorised_value_category => $field->{authorised_value_category}, - }; - } - } - - return \@additional_field_filters; -} - -sub get_filters_as_values { - my ( $class, $filters ) = @_; - die "BAD CALL: Don't use get_filters_as_values as an instance method" - if ref $class and UNIVERSAL::can($class,'can'); - - return { map { $_->{name} => $_->{value} } @$filters }; -} +sub _type { 'AdditionalField' } 1; @@ -330,188 +16,6 @@ __END__ Koha::AdditionalField -=head1 SYNOPSIS - - use Koha::AdditionalField; - my $af1 = Koha::AdditionalField->new({id => $id}); - my $af2 = Koha::AuthorisedValue->new({ - tablename => 'my_table', - name => 'a_name', - authorised_value_category => 'LOST', - marcfield => '200$a', - searchable => 1, - }); - $av1->delete; - $av2->{name} = 'another_name'; - $av2->update; - -=head1 DESCRIPTION - -Class for managing additional fields into Koha. - -=head1 METHODS - -=head2 new - -Create a new Koha::AdditionalField object. This method can be called using several ways. -Either with the id for existing field or with different values for a new one. - -=over 4 - -=item B - - The caller just knows the id of the additional field and want to retrieve all values. - -=item B - - The caller wants to create a new additional field. - -=back - -=head2 fetch - -The information will be retrieved from the database. - -=head2 update - -If the AdditionalField object has been modified and the values have to be modified into the database, call this method. - -=head2 delete - -Remove a the record in the database using the id the object. - -=head2 insert - -Insert a new AdditionalField object into the database. - -=head2 insert_values - -Insert new values for a record. - - my $af = Koha::AdditionalField({ id => $id })->fetch; - my $af->{values} = { - record_id1 => 'my value', - record_id2 => 'another value', - }; - $af->insert_values; - -=head2 fetch_values - -Retrieve values from the database for a given record_id. -The record_id argument is optional. - - my $af = Koha::AdditionalField({ id => $id })->fetch; - my $values = $af->fetch_values({record_id => $record_id}); - - $values will be equal to something like: - { - record_id => { - field_name1 => 'value1', - field_name2 => 'value2', - } - } - -=head2 delete_values - -Delete values from the database for a given record_id. -The record_id argument is optional. - - my $af = Koha::AdditionalField({ id => $id })->fetch; - $af->delete_values({record_id => $record_id}); - -=head2 all - -Retrieve all additional fields in the database given some parameters. -Parameters are optional. -This method returns a list of AdditionalField objects. -This is a static method. - - my $fields = Koha::AdditionalField->all; - or - my $fields = Koha::AdditionalField->all{(tablename => 'my_table'}); - or - my $fields = Koha::AdditionalField->all({searchable => 1}); - -=head2 fetch_all_values - -Retrieve all values for a table name. -This is a static method. - - my $values = Koha::AdditionalField({ tablename => 'my_table' }); - - $values will be equel to something like: - { - record_id1 => { - field_name1 => 'value1', - field_name2 => 'value2', - }, - record_id2 => { - field_name1 => 'value3', - field_name2 => 'value4', - } - - } - -=head2 get_matching_record_ids - -Retrieve all record_ids for records matching the field values given in parameter. -This method returns a list of ids. -This is a static method. - - my $fields = [ - { - name => 'field_name', - value => 'field_value', - } - ]; - my $ids = Koha::AdditionalField->get_matching_record_ids( - { - tablename => 'subscription', - fields => $fields - } - ); - -=head2 update_fields_from_query - -Updates fields based on user input (best paired with additional-fields-entry.pl) and optionally a MARC record. - -This is a static method. - - Koha::AdditionalField->update_fields_from_query( { - tablename => 'aqbasket', - input => $input, - record_id => $basketno, - marc_record => GetBiblio( $biblionumber ), - } ); - -=head2 get_filters_from_query - -Extracts a list of search filters from user input, to be used with C and -C. - -This is a static method. - - my $filters = Koha::AdditionalField->get_filters_from_query( { - tablename => 'aqbasket', - input => $input, - } ); - -=head2 get_filters_as_values - -Transforms the result of C into a hashref of C => C, so -user-entered filters can be redisplayed. - - $template->param( - additional_field_values => Koha::AdditionalField->get_filters_as_values( - $filters - ), - ); - - [% INCLUDE 'additional-field-entry.inc' - values=additional_field_values - available=available_additional_fields - %] - =head1 AUTHOR Jonathan Druart diff --git a/Koha/AdditionalFieldValue.pm b/Koha/AdditionalFieldValue.pm new file mode 100644 index 0000000000..f54c811ad8 --- /dev/null +++ b/Koha/AdditionalFieldValue.pm @@ -0,0 +1,9 @@ +package Koha::AdditionalFieldValue; + +use Modern::Perl; + +use base 'Koha::Object'; + +sub _type { 'AdditionalFieldValue' } + +1; diff --git a/Koha/AdditionalFieldValues.pm b/Koha/AdditionalFieldValues.pm new file mode 100644 index 0000000000..fa3576fa4f --- /dev/null +++ b/Koha/AdditionalFieldValues.pm @@ -0,0 +1,10 @@ +package Koha::AdditionalFieldValues; + +use Modern::Perl; + +use base 'Koha::Objects'; + +sub _type { 'AdditionalFieldValue' } +sub object_class { 'Koha::AdditionalFieldValue' } + +1; diff --git a/Koha/AdditionalFields.pm b/Koha/AdditionalFields.pm new file mode 100644 index 0000000000..2d4761bbec --- /dev/null +++ b/Koha/AdditionalFields.pm @@ -0,0 +1,12 @@ +package Koha::AdditionalFields; + +use Modern::Perl; + +use base 'Koha::Objects'; + +use Koha::AdditionalField; + +sub _type { 'AdditionalField' } +sub object_class { 'Koha::AdditionalField' } + +1; diff --git a/Koha/Object/Mixin/AdditionalFields.pm b/Koha/Object/Mixin/AdditionalFields.pm new file mode 100644 index 0000000000..090e2824c2 --- /dev/null +++ b/Koha/Object/Mixin/AdditionalFields.pm @@ -0,0 +1,68 @@ +package Koha::Object::Mixin::AdditionalFields; + +use Modern::Perl; + +=head1 NAME + +Koha::Object::Mixin::AdditionalFields + +=head1 SYNOPSIS + + package Koha::Foo; + + use parent qw( Koha::Object Koha::Object::Mixin::AdditionalFields ); + + sub _type { 'Foo' } + + + package main; + + use Koha::Foo; + + Koha::Foos->find($id)->set_additional_fields(...); + +=head1 API + +=head2 Public methods + +=head3 set_additional_fields + + $foo->set_additional_fields([ + { + id => 1, + value => 'foo', + }, + { + id => 2, + value => 'bar', + } + ]); + +=cut + +sub set_additional_fields { + my ($self, $additional_fields) = @_; + + my $rs = Koha::Database->new->schema->resultset('AdditionalFieldValue'); + + 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; + } + } +} + +sub additional_field_values { + my ($self) = @_; + + return $self->_result->additional_field_values; +} + +1; diff --git a/Koha/Objects/Mixin/AdditionalFields.pm b/Koha/Objects/Mixin/AdditionalFields.pm new file mode 100644 index 0000000000..62c784f183 --- /dev/null +++ b/Koha/Objects/Mixin/AdditionalFields.pm @@ -0,0 +1,59 @@ +package Koha::Objects::Mixin::AdditionalFields; + +use Modern::Perl; + +=head1 NAME + +Koha::Objects::Mixin::AdditionalFields + +=head1 SYNOPSIS + + package Koha::Foos; + + use parent qw( Koha::Objects Koha::Objects::Mixin::AdditionalFields ); + + sub _type { 'Foo' } + sub object_class { 'Koha::Foo' } + + + package main; + + use Koha::Foos; + + Koha::Foos->search_additional_fields(...) + +=head1 API + +=head2 Public methods + +=head3 search_additional_fields + + my @objects = Koha::Foos->search_additional_fields([ + { + id => 1, + value => 'foo', + }, + { + id => 2, + value => 'bar', + } + ]); + +=cut + +sub search_additional_fields { + my ($class, $additional_fields) = @_; + + my %conditions; + my $idx = 0; + foreach my $additional_field (@$additional_fields) { + ++$idx; + my $alias = $idx > 1 ? "additional_field_values_$idx" : "additional_field_values"; + $conditions{"$alias.field_id"} = $additional_field->{id}; + $conditions{"$alias.value"} = { -like => '%' . $additional_field->{value} . '%'}; + } + + return $class->search(\%conditions, { join => [ ('additional_field_values') x $idx ] }); +} + +1; diff --git a/Koha/Schema/Result/Aqbasket.pm b/Koha/Schema/Result/Aqbasket.pm index bb1e5a78aa..0419636955 100644 --- a/Koha/Schema/Result/Aqbasket.pm +++ b/Koha/Schema/Result/Aqbasket.pm @@ -312,6 +312,22 @@ __PACKAGE__->many_to_many("borrowernumbers", "aqbasketusers", "borrowernumber"); # Created by DBIx::Class::Schema::Loader v0.07042 @ 2018-02-16 17:54:53 # DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:gSw/f4JmMBzEssEFRg2fAQ +__PACKAGE__->has_many( + "additional_field_values", + "Koha::Schema::Result::AdditionalFieldValue", + sub { + my ($args) = @_; + + 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")' }, + }; + }, + { cascade_copy => 0, cascade_delete => 0 }, +); # You can replace this text with custom code or comments, and it will be preserved on regeneration 1; diff --git a/Koha/Schema/Result/Subscription.pm b/Koha/Schema/Result/Subscription.pm index 3666dd6fd5..5ccb9ee44f 100644 --- a/Koha/Schema/Result/Subscription.pm +++ b/Koha/Schema/Result/Subscription.pm @@ -455,6 +455,22 @@ __PACKAGE__->has_many( # Created by DBIx::Class::Schema::Loader v0.07046 @ 2019-01-23 12:56:39 # DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:dTb/JOO3KQ3NZGypFbRiEw +__PACKAGE__->has_many( + "additional_field_values", + "Koha::Schema::Result::AdditionalFieldValue", + sub { + my ($args) = @_; + + 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")' }, + }; + }, + { cascade_copy => 0, cascade_delete => 0 }, +); # You can replace this text with custom content, and it will be preserved on regeneration 1; diff --git a/Koha/Subscription.pm b/Koha/Subscription.pm index 570ae1f590..833f674ad4 100644 --- a/Koha/Subscription.pm +++ b/Koha/Subscription.pm @@ -30,7 +30,7 @@ use Koha::Subscription::Frequencies; use Koha::Subscription::Numberpatterns; use JSON; -use base qw(Koha::Object); +use base qw(Koha::Object Koha::Object::Mixin::AdditionalFields); =head1 NAME diff --git a/Koha/Subscriptions.pm b/Koha/Subscriptions.pm index 3538c15cd6..f92b586dac 100644 --- a/Koha/Subscriptions.pm +++ b/Koha/Subscriptions.pm @@ -25,7 +25,7 @@ use Koha::Database; use Koha::Subscription; -use base qw(Koha::Objects); +use base qw(Koha::Objects Koha::Objects::Mixin::AdditionalFields); =head1 NAME diff --git a/acqui/basket.pl b/acqui/basket.pl index 8a09419155..52d3ef8a2e 100755 --- a/acqui/basket.pl +++ b/acqui/basket.pl @@ -33,6 +33,7 @@ use C4::Biblio; use C4::Items; use C4::Suggestions; use Koha::Biblios; +use Koha::Acquisition::Baskets; use Koha::Acquisition::Booksellers; use Koha::Acquisition::Orders; use Koha::Libraries; @@ -43,7 +44,7 @@ use Koha::EDI qw( create_edi_order get_edifact_ean ); use Koha::CsvProfiles; use Koha::Patrons; -use Koha::AdditionalField; +use Koha::AdditionalFields; =head1 NAME @@ -432,11 +433,10 @@ if ( $op eq 'list' ) { has_budgets => $has_budgets, duplinbatch => $duplinbatch, csv_profiles => [ Koha::CsvProfiles->search({ type => 'sql', used_for => 'export_basket' }) ], - available_additional_fields => Koha::AdditionalField->all( { tablename => 'aqbasket' } ), - additional_field_values => Koha::AdditionalField->fetch_all_values( { - tablename => 'aqbasket', - record_id => $basketno, - } )->{$basketno}, + available_additional_fields => [ Koha::AdditionalFields->search( { tablename => 'aqbasket' } ) ], + additional_field_values => { map { + $_->field->name => $_->value + } Koha::Acquisition::Baskets->find($basketno)->additional_field_values }, ); } diff --git a/acqui/basketheader.pl b/acqui/basketheader.pl index acd3d67c51..da2129f485 100755 --- a/acqui/basketheader.pl +++ b/acqui/basketheader.pl @@ -54,7 +54,8 @@ use C4::Acquisition qw/GetBasket NewBasket ModBasketHeader/; use C4::Contract qw/GetContracts/; use Koha::Acquisition::Booksellers; -use Koha::AdditionalField; +use Koha::Acquisition::Baskets; +use Koha::AdditionalFields; my $input = new CGI; my ( $template, $loggedinuser, $cookie ) = get_template_and_user( @@ -75,7 +76,7 @@ my $basket; my $op = $input->param('op'); my $is_an_edit = $input->param('is_an_edit'); -$template->param( available_additional_fields => scalar Koha::AdditionalField->all( { tablename => 'aqbasket' } ) ); +$template->param( available_additional_fields => [ Koha::AdditionalFields->search( { tablename => 'aqbasket' } ) ] ); if ( $op eq 'add_form' ) { my @contractloop; @@ -98,10 +99,9 @@ if ( $op eq 'add_form' ) { } $template->param( is_an_edit => 1); $template->param( - additional_field_values => Koha::AdditionalField->fetch_all_values( { - tablename => 'aqbasket', - record_id => $basketno, - } )->{$basketno}, + additional_field_values => { map { + $_->field->name => $_->value + } Koha::Acquisition::Baskets->find($basketno)->additional_field_values }, ); } else { #new basket @@ -170,11 +170,15 @@ if ( $op eq 'add_form' ) { ); } - Koha::AdditionalField->update_fields_from_query( { - tablename => 'aqbasket', - record_id => $basketno, - query => $input, - } ); + my @additional_fields; + for my $field (Koha::AdditionalFields->search({ tablename => 'aqbasket' })) { + my $value = $input->param('additional_field_' . $field->id); + push @additional_fields, { + id => $field->id, + value => $value, + }; + } + Koha::Acquisition::Baskets->find($basketno)->set_additional_fields(\@additional_fields); print $input->redirect('basket.pl?basketno='.$basketno); exit 0; diff --git a/acqui/histsearch.pl b/acqui/histsearch.pl index a1c7fe6708..f5dca499c8 100755 --- a/acqui/histsearch.pl +++ b/acqui/histsearch.pl @@ -56,7 +56,7 @@ use C4::Output; use C4::Acquisition; use C4::Debug; use C4::Koha; -use Koha::AdditionalField; +use Koha::AdditionalFields; use Koha::DateUtils; my $input = new CGI; @@ -98,12 +98,20 @@ unless ( $input->param('from') ) { } $filters->{from_placed_on} = output_pref( { dt => $from_placed_on, dateformat => 'iso', dateonly => 1 } ), $filters->{to_placed_on} = output_pref( { dt => $to_placed_on, dateformat => 'iso', dateonly => 1 } ), -$filters->{additional_fields} = Koha::AdditionalField->get_filters_from_query({ - tablename => 'aqbasket', - query => $input, -} ); +my @additional_fields = Koha::AdditionalFields->search( { tablename => 'aqbasket', searchable => 1 } ); +$template->param( available_additional_fields => \@additional_fields ); +my @additional_field_filters; +foreach my $additional_field (@additional_fields) { + my $value = $input->param('additional_field_' . $additional_field->id); + if (defined $value and $value ne '') { + push @additional_field_filters, { + id => $additional_field->id, + value => $value, + }; + } +} +$filters->{additional_fields} = \@additional_field_filters; -$template->param( available_additional_fields => scalar Koha::AdditionalField->all( { tablename => 'aqbasket', searchable => 1 } ) ); my $order_loop; # If we're supplied any value then we do a search. Otherwise we don't. diff --git a/admin/additional-fields.pl b/admin/additional-fields.pl index 0fc8b5f09a..5bc722c09d 100755 --- a/admin/additional-fields.pl +++ b/admin/additional-fields.pl @@ -22,6 +22,7 @@ use C4::Auth; use C4::Koha; use C4::Output; use Koha::AdditionalField; +use Koha::AdditionalFields; my $input = new CGI; @@ -49,14 +50,14 @@ if ( $op eq 'add' ) { if ( $field_id and $name ) { my $updated = 0; eval { - my $af = Koha::AdditionalField->new({ - id => $field_id, + my $af = Koha::AdditionalFields->find($field_id); + $af->set({ name => $name, authorised_value_category => $authorised_value_category, marcfield => $marcfield, searchable => $searchable, }); - $updated = $af->update; + $updated = $af->store ? 1 : 0; }; push @messages, { code => 'update', @@ -72,7 +73,7 @@ if ( $op eq 'add' ) { marcfield => $marcfield, searchable => $searchable, }); - $inserted = $af->insert; + $inserted = $af->store ? 1 : 0; }; push @messages, { code => 'insert', @@ -90,9 +91,8 @@ if ( $op eq 'add' ) { if ( $op eq 'delete' ) { my $deleted = 0; eval { - my $af = Koha::AdditionalField->new( { id => $field_id } ); + my $af = Koha::AdditionalFields->find($field_id); $deleted = $af->delete; - $deleted = 0 if $deleted eq '0E0'; }; push @messages, { code => 'delete', @@ -104,10 +104,10 @@ if ( $op eq 'delete' ) { if ( $op eq 'add_form' ) { my $field; if ( $field_id ) { - $field = Koha::AdditionalField->new( { id => $field_id } )->fetch; + $field = Koha::AdditionalFields->find($field_id); } - $tablename = $field->{tablename} if $field; + $tablename = $field->tablename if $field; $template->param( field => $field, @@ -115,7 +115,7 @@ if ( $op eq 'add_form' ) { } if ( $op eq 'list' ) { - my $fields = Koha::AdditionalField->all( { tablename => $tablename } ); + my $fields = Koha::AdditionalFields->search( { tablename => $tablename } ); $template->param( fields => $fields ); } diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/additional-fields-entry.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/additional-fields-entry.inc index 123e5b38c2..c8cbd83bcd 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/additional-fields-entry.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/additional-fields-entry.inc @@ -11,7 +11,7 @@ (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 %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/additional-fields.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/additional-fields.tt index a37401aa80..bdd6984be5 100755 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/additional-fields.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/additional-fields.tt @@ -12,6 +12,9 @@ [% Asset.css('css/datatables.css') %] +[% marcfield_tables = ['subscription'] %] +[% show_marcfield = marcfield_tables.grep('^' _ tablename _ '$').size ? 1 : 0 %] + [% INCLUDE 'header.inc' %] [% INCLUDE 'cat-search.inc' %] @@ -77,7 +80,9 @@ Name Authorised value category - MARC field + [% IF show_marcfield %] + MARC field + [% END %] Searchable Actions @@ -87,13 +92,15 @@ [% field.name %] [% field.authorised_value_category %] - [% field.marcfield %] + [% IF show_marcfield %] + [% field.marcfield %] + [% END %] [% IF field.searchable %]Yes[% ELSE %]No[% END %] Edit - Delete + Delete [% END %] @@ -127,10 +134,12 @@ %] -
  • - - -
    • + [% IF show_marcfield %] +
  • + + +
    • + [% END %]
  • [% IF field.searchable %] 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 01ebae0b1f..2e2c3420c0 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 @@ -376,7 +376,7 @@ [% ELSE %] - + [% END %]
    • [% END %] diff --git a/serials/claims.pl b/serials/claims.pl index 44cff74865..2ed81b8745 100755 --- a/serials/claims.pl +++ b/serials/claims.pl @@ -27,7 +27,7 @@ use C4::Context; use C4::Letters; use C4::Koha qw( GetAuthorisedValues ); -use Koha::AdditionalField; +use Koha::AdditionalFields; use Koha::CsvProfiles; my $input = CGI->new; @@ -57,13 +57,7 @@ for my $s (@{$supplierlist} ) { } } -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} ); - } -} - +my $additional_fields = Koha::AdditionalFields->search( { tablename => 'subscription', searchable => 1 } ); my @serialnums=$input->multi_param('serialid'); if (@serialnums) { # i.e. they have been flagged to generate claims diff --git a/serials/serials-search.pl b/serials/serials-search.pl index 976b12815c..b1fa40f01a 100755 --- a/serials/serials-search.pl +++ b/serials/serials-search.pl @@ -35,7 +35,7 @@ use C4::Context; use C4::Koha qw( GetAuthorisedValues ); use C4::Output; use C4::Serials; -use Koha::AdditionalField; +use Koha::AdditionalFields; use Koha::DateUtils; use Koha::SharedContent; @@ -79,11 +79,17 @@ if ( $op and $op eq "close" ) { } -my $additional_fields = Koha::AdditionalField->all( { tablename => 'subscription', searchable => 1 } ); -my $additional_field_filters = Koha::AdditionalField->get_filters_from_query( { - tablename => 'subscription', - query => $query, -} ); +my @additional_fields = Koha::AdditionalFields->search( { tablename => 'subscription', searchable => 1 } ); +my @additional_field_filters; +for my $field ( @additional_fields ) { + my $value = $query->param( 'additional_field_' . $field->id ); + if ( defined $value and $value ne '' ) { + push @additional_field_filters, { + id => $field->id, + value => $value, + }; + } +} my $expiration_date_dt = $expiration_date ? dt_from_string( $expiration_date ) : undef; my @subscriptions; @@ -110,7 +116,7 @@ if ($searched){ publisher => $publisher, bookseller => $bookseller, branch => $branch, - additional_fields => $additional_field_filters, + additional_fields => \@additional_field_filters, location => $location, expiration_date => $expiration_date_dt, }); @@ -188,8 +194,8 @@ else branches_loop => \@branches_loop, done_searched => $searched, routing => $routing, - additional_field_filters => Koha::AdditionalField->get_filters_as_values( $additional_field_filters ), - additional_fields_for_subscription => $additional_fields, + additional_field_filters => { map { $_->{id} => $_->{value} } @additional_field_filters }, + additional_fields_for_subscription => \@additional_fields, marcflavour => (uc(C4::Context->preference("marcflavour"))), mana => $mana ); diff --git a/serials/subscription-add.pl b/serials/subscription-add.pl index 7cc131da5a..4d3310d433 100755 --- a/serials/subscription-add.pl +++ b/serials/subscription-add.pl @@ -29,7 +29,7 @@ use C4::Serials; use C4::Serials::Frequency; use C4::Serials::Numberpattern; use C4::Letters; -use Koha::AdditionalField; +use Koha::AdditionalFields; use Koha::Biblios; use Koha::DateUtils; use Koha::ItemTypes; @@ -144,7 +144,7 @@ $template->param( locations_loop=>$locations_loop, ); -$template->param( additional_fields_for_subscription => scalar Koha::AdditionalField->all( { tablename => 'subscription' } ) ); +$template->param( additional_fields_for_subscription => [ Koha::AdditionalFields->search( { tablename => 'subscription' } ) ] ); my $typeloop = { map { $_->{itemtype} => $_ } @{ Koha::ItemTypes->search_with_localization->unblessed } }; @@ -370,12 +370,22 @@ sub redirect_add_subscription { $template->param( mana_msg => $result->{msg} ); } - Koha::AdditionalField->update_fields_from_query( { - tablename => 'subscription', - record_id => $subscriptionid, - query => $query, - marc_record => GetMarcBiblio({ biblionumber => $biblionumber, embed_items => 1 }) - } ); + my @additional_fields; + my $record = GetMarcBiblio({ biblionumber => $biblionumber, embed_items => 1 }); + for my $field (Koha::AdditionalFields->search({ tablename => 'subscription' })) { + my $value = $query->param('additional_field_' . $field->id); + if ($field->marcfield) { + my ($field, $subfield) = split /\$/, $field->marcfield; + if ( $record and $field and $subfield ) { + $value = $record->subfield( $field, $subfield ); + } + } + push @additional_fields, { + id => $field->id, + value => $value, + }; + } + Koha::Subscriptions->find($subscriptionid)->set_additional_fields(\@additional_fields); print $query->redirect("/cgi-bin/koha/serials/subscription-detail.pl?subscriptionid=$subscriptionid"); return; @@ -478,12 +488,22 @@ sub redirect_mod_subscription { $skip_serialseq, $itemtype, $previousitemtype, $mana_id ); - Koha::AdditionalField->update_fields_from_query( { - tablename => 'subscription', - record_id => $subscriptionid, - query => $query, - marc_record => GetMarcBiblio({ biblionumber => $biblionumber, embed_items => 1 }) - } ); + my @additional_fields; + my $record = GetMarcBiblio({ biblionumber => $biblionumber, embed_items => 1 }); + for my $field (Koha::AdditionalFields->search({ tablename => 'subscription' })) { + my $value = $query->param('additional_field_' . $field->id); + if ($field->marcfield) { + my ($field, $subfield) = split /\$/, $field->marcfield; + if ( $record and $field and $subfield ) { + $value = $record->subfield( $field, $subfield ); + } + } + push @additional_fields, { + id => $field->id, + value => $value, + }; + } + Koha::Subscriptions->find($subscriptionid)->set_additional_fields(\@additional_fields); print $query->redirect("/cgi-bin/koha/serials/subscription-detail.pl?subscriptionid=$subscriptionid"); return; diff --git a/serials/subscription-batchedit.pl b/serials/subscription-batchedit.pl index f0f56906e0..fa114a18e2 100755 --- a/serials/subscription-batchedit.pl +++ b/serials/subscription-batchedit.pl @@ -26,7 +26,7 @@ use C4::Output; use C4::Serials; use Koha::Subscriptions; use Koha::Acquisition::Booksellers; -use Koha::AdditionalField; +use Koha::AdditionalFields; use Koha::DateUtils; my $cgi = new CGI; @@ -48,7 +48,7 @@ foreach my $subscriptionid (@subscriptionids) { push @subscriptions, $subscription if $subscription; } -my $additional_fields = Koha::AdditionalField->all({tablename => 'subscription'}); +my @additional_fields = Koha::AdditionalFields->search({tablename => 'subscription'}); my $batchedit = $cgi->param('batchedit'); if ($batchedit) { @@ -64,9 +64,9 @@ if ($batchedit) { ); my $field_values = {}; - foreach my $field (@$additional_fields) { - my $value = $cgi->param('field_' . $field->{id}); - $field_values->{$field->{id}} = $value; + foreach my $field (@additional_fields) { + my $value = $cgi->param('field_' . $field->id); + $field_values->{$field->id} = $value; } foreach my $subscription (@subscriptions) { @@ -77,23 +77,21 @@ if ($batchedit) { } } - foreach my $field (@$additional_fields) { - my $value = $field_values->{$field->{id}}; + my @additional_field_values; + foreach my $field (@additional_fields) { + my $value = $field_values->{$field->id}; if (defined $value and $value ne '') { - $field->{values} //= {}; - $field->{values}->{$subscription->subscriptionid} = $value; + push @additional_field_values, { + id => $field->id, + value => $value, + }; } } + $subscription->set_additional_fields(\@additional_field_values); $subscription->store; } - foreach my $field (@$additional_fields) { - if (defined $field->{values}) { - $field->insert_values(); - } - } - my $redirect_url = $cgi->param('referrer') // '/cgi-bin/koha/serials/serials-home.pl'; print $cgi->redirect($redirect_url); exit; @@ -102,7 +100,7 @@ if ($batchedit) { $template->param( subscriptions => \@subscriptions, booksellers => [ Koha::Acquisition::Booksellers->search() ], - additional_fields => $additional_fields, + additional_fields => \@additional_fields, referrer => scalar $cgi->param('referrer'), ); diff --git a/serials/subscription-detail.pl b/serials/subscription-detail.pl index 451e5a6baf..c023c748e2 100755 --- a/serials/subscription-detail.pl +++ b/serials/subscription-detail.pl @@ -26,6 +26,7 @@ use C4::Output; use C4::Context; use C4::Search qw/enabled_staff_search_views/; +use Koha::AdditionalFields; use Koha::AuthorisedValues; use Koha::DateUtils; use Koha::Acquisition::Bookseller; @@ -128,12 +129,7 @@ my $numberpattern = C4::Serials::Numberpattern::GetSubscriptionNumberpattern($su my $default_bib_view = get_default_view(); -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} ); - } -} +my $additional_fields = Koha::AdditionalFields->search( { tablename => 'subscription' } ); $template->param( additional_fields_for_subscription => $additional_fields ); # FIXME Do we want to hide canceled orders? diff --git a/t/db_dependent/AdditionalField.t b/t/db_dependent/AdditionalField.t deleted file mode 100644 index 2a3a680fcc..0000000000 --- a/t/db_dependent/AdditionalField.t +++ /dev/null @@ -1,293 +0,0 @@ -#!/usr/bin/perl - -use Modern::Perl; -use Test::More tests => 40; - -use C4::Context; -use Koha::AdditionalField; -use Koha::Database; - -my $schema = Koha::Database->new->schema; -$schema->storage->txn_begin; -my $dbh = C4::Context->dbh; - -$dbh->do( q|DELETE FROM additional_fields| ); -$dbh->do( q|DELETE FROM additional_field_values| ); - -my $afs = Koha::AdditionalField->all; -is( scalar( @$afs ), 0, "all: there is no additional field" ); - -my $af1_name = q|af1|; -my $af1 = Koha::AdditionalField->new({ - tablename => 'subscription', - name => $af1_name, - authorised_values_category => '', - marcfield => '', - searchable => 1, -}); -is ( $af1->name, $af1_name, "new: name value is kept" ); - -$af1->insert; -like ( $af1->id, qr|^\d+$|, "new: populate id value" ); - -my $af2_name = q|af2|; -my $af2_marcfield = q|200$a|; -my $af2_searchable = 1; -my $af2_tablename = q|subscription|; -my $af2_avc = q|LOST|; -my $af2 = Koha::AdditionalField->new({ - tablename => $af2_tablename, - name => $af2_name, - authorised_value_category => $af2_avc, - marcfield => $af2_marcfield, - searchable => $af2_searchable, -}); -$af2->insert; -my $af2_id = $af2->id; -$af2 = Koha::AdditionalField->new({ id => $af2_id })->fetch; -is( ref($af2) , q|Koha::AdditionalField|, "fetch: return an object" ); -is( $af2->id, $af2_id, "fetch: id for af2" ); -is( $af2->tablename, $af2_tablename, "fetch: tablename for af2" ); -is( $af2->name, $af2_name, "fetch: name for af2" ); -is( $af2->authorised_value_category, $af2_avc, "fetch: authorised_value_category for af2" ); -is( $af2->marcfield, $af2_marcfield, "fetch: marcfield for af2" ); -is( $af2->searchable, $af2_searchable, "fetch: searchable for af2" ); - -my $af3 = Koha::AdditionalField->new({ - tablename => 'a_table', - name => q|af3|, - authorised_value_category => '', - marcfield => '', - searchable => 1, -}); -$af3->insert; - -my $af_common = Koha::AdditionalField->new({ - tablename => 'subscription', - name => q|common|, - authorised_value_category => '', - marcfield => '', - searchable => 1, -}); -$af_common->insert; - -# update -$af3->{tablename} = q|another_table|; -$af3->{name} = q|af3_mod|; -$af3->{authorised_value_category} = q|LOST|; -$af3->{marcfield} = q|200$a|; -$af3->{searchable} = 0; -my $updated = $af3->update; -$af3 = Koha::AdditionalField->new({ id => $af3->id })->fetch; -is( $updated, 1, "update: return number of affected rows" ); -is( $af3->tablename, q|a_table|, "update: tablename is *not* updated, there is no sense to copy a field to another table" ); -is( $af3->name, q|af3_mod|, "update: name" ); -is( $af3->authorised_value_category, q|LOST|, "update: authorised_value_category" ); -is( $af3->marcfield, q|200$a|, "update: marcfield" ); -is( $af3->searchable, q|0|, "update: searchable" ); - -# fetch all -$afs = Koha::AdditionalField->all; -is( scalar( @$afs ), 4, "all: got 4 additional fields" ); -$afs = Koha::AdditionalField->all({tablename => 'subscription'}); -is( scalar( @$afs ), 3, "all: got 3 additional fields for the subscription table" ); -$afs = Koha::AdditionalField->all({searchable => 1}); -is( scalar( @$afs ), 3, "all: got 3 searchable additional fields" ); -$af3->delete; -$afs = Koha::AdditionalField->all; -is( scalar( @$afs ), 3, "all: got 3 additional fields after deleting one" ); - - -# Testing additional field values - -## Creating 2 subscriptions -use C4::Acquisition; -use C4::Biblio; -use C4::Budgets; -use C4::Serials; -use C4::Serials::Frequency; -use C4::Serials::Numberpattern; - -my ($biblionumber, $biblioitemnumber) = AddBiblio(MARC::Record->new, ''); -my $budgetid; -my $bpid = AddBudgetPeriod({ - budget_period_startdate => '2015-01-01', - budget_period_enddate => '2016-01-01', -}); - -my $budget_id = AddBudget({ - budget_code => "ABCD", - budget_amount => "123.132", - budget_name => "Périodiques", - budget_notes => "This is a note", - budget_period_id => $bpid -}); - -my $frequency_id = AddSubscriptionFrequency({ description => "Test frequency 1" }); -my $pattern_id = AddSubscriptionNumberpattern({ - label => 'Test numberpattern 1', - description => 'Description for numberpattern 1', - numberingmethod => '{X}' -}); - -my $subscriptionid1 = NewSubscription( - 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, - '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 -my $af1_values = { - $subscriptionid1 => "value_for_af1_$subscriptionid1", - $subscriptionid2 => "value_for_af1_$subscriptionid2", -}; -$af1->{values} = $af1_values; -$af1->insert_values; - -my $af2_values = { - $subscriptionid1 => "old_value_for_af2_$subscriptionid1", - $subscriptionid2 => "old_value_for_af2_$subscriptionid2", -}; -$af2->{values} = $af2_values; -$af2->insert_values; -my $new_af2_values = { - $subscriptionid1 => "value_for_af2_$subscriptionid1", - $subscriptionid2 => "value_for_af2_$subscriptionid2", -}; -$af2->{values} = $new_af2_values; -$af2->insert_values; # Insert should replace old values - -my $common_values = { - $subscriptionid1 => 'common_value', - $subscriptionid2 => 'common_value', -}; - -$af_common->{values} = $common_values; -$af_common->insert_values; - -# fetch_values -$af1 = Koha::AdditionalField->new({ id => $af1->id })->fetch; -$af2 = Koha::AdditionalField->new({ id => $af2->id })->fetch; - -$af1->fetch_values; -is_deeply ( $af1->values, {$subscriptionid1 => qq|value_for_af1_$subscriptionid1|, $subscriptionid2 => qq|value_for_af1_$subscriptionid2| }, "fetch_values: without argument, returns 2 records" ); -$af1->fetch_values({ record_id => $subscriptionid1 }); -is_deeply ( $af1->values, {$subscriptionid1 => qq|value_for_af1_$subscriptionid1|}, "fetch_values: values for af1 and subscription1" ); -$af2->fetch_values({ record_id => $subscriptionid2 }); -is_deeply ( $af2->values, {$subscriptionid2 => qq|value_for_af2_$subscriptionid2|}, "fetch_values: values for af2 and subscription2" ); - -# fetch_all_values -eval{ - $af1->fetch_all_values; -}; -like ( $@, qr|^BAD CALL|, 'fetch_all_values: fail if called with a blessed object' ); - -my $fetched_values = Koha::AdditionalField->fetch_all_values({ tablename => 'subscription' }); -my $expected_values = { - $subscriptionid1 => { - $af1_name => qq|value_for_af1_$subscriptionid1|, - $af2_name => qq|value_for_af2_$subscriptionid1|, - 'common' => q|common_value|, - }, - $subscriptionid2 => { - $af1_name => qq|value_for_af1_$subscriptionid2|, - $af2_name => qq|value_for_af2_$subscriptionid2|, - 'common' => q|common_value|, - } -}; -is_deeply ( $fetched_values, $expected_values, "fetch_all_values: values for table subscription" ); - -my $expected_values_1 = { - $subscriptionid1 => { - $af1_name => qq|value_for_af1_$subscriptionid1|, - $af2_name => qq|value_for_af2_$subscriptionid1|, - common => q|common_value|, - } -}; -my $fetched_values_1 = Koha::AdditionalField->fetch_all_values({ tablename => 'subscription', record_id => $subscriptionid1 }); -is_deeply ( $fetched_values_1, $expected_values_1, "fetch_all_values: values for subscription1" ); - -# get_matching_record_ids -eval{ - $af1->get_matching_record_ids; -}; -like ( $@, qr|^BAD CALL|, 'get_matching_record_ids: fail if called with a blessed object' ); - -my $matching_record_ids = Koha::AdditionalField->get_matching_record_ids; -is_deeply ( $matching_record_ids, [], "get_matching_record_ids: return [] if no argument given" ); -$matching_record_ids = Koha::AdditionalField->get_matching_record_ids({ tablename => 'subscription' }); -is_deeply ( $matching_record_ids, [], "get_matching_record_ids: return [] if no field given" ); - -my $fields = [ - { - name => $af1_name, - value => qq|value_for_af1_$subscriptionid1| - } -]; -$matching_record_ids = Koha::AdditionalField->get_matching_record_ids({ tablename => 'subscription', fields => $fields }); -is_deeply ( $matching_record_ids, [ $subscriptionid1 ], "get_matching_record_ids: field $af1_name: value_for_af1_$subscriptionid1 matches subscription1" ); - -$fields = [ - { - name => $af1_name, - value => qq|value_for_af1_$subscriptionid1| - }, - { - name => $af2_name, - value => qq|value_for_af2_$subscriptionid1|, - } -]; -$matching_record_ids = Koha::AdditionalField->get_matching_record_ids({ tablename => 'subscription', fields => $fields }); -is_deeply ( $matching_record_ids, [ $subscriptionid1 ], "get_matching_record_ids: fields $af1_name:value_for_af1_$subscriptionid1 and $af2_name:value_for_af2_$subscriptionid1 match subscription1" ); - -$fields = [ - { - name => 'common', - value => q|common_value|, - } -]; -$matching_record_ids = Koha::AdditionalField->get_matching_record_ids({ tablename => 'subscription', fields => $fields }); -my $exists = grep /$subscriptionid1/, @$matching_record_ids; -is ( $exists, 1, "get_matching_record_ids: field common: common_value matches subscription1" ); -$exists = grep /$subscriptionid2/, @$matching_record_ids; -is ( $exists, 1, "get_matching_record_ids: field common: common_value matches subscription2 too" ); -$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" ); - -# delete_values -$af1 = Koha::AdditionalField->new({ id => $af1->id })->fetch; - -$af1->fetch_values; -is_deeply ( $af1->values, {$subscriptionid1 => qq|value_for_af1_$subscriptionid1|, $subscriptionid2 => qq|value_for_af1_$subscriptionid2| }, "fetch_values: without argument, returns 2 records" ); -$af1->delete_values({record_id => $subscriptionid1}); -$af1->fetch_values; -is_deeply ( $af1->values, {$subscriptionid2 => qq|value_for_af1_$subscriptionid2|}, "fetch_values: values for af2 and subscription2" ); -$af1->delete_values; -$af1->fetch_values; -is_deeply ( $af1->values, {}, "fetch_values: no values" ); diff --git a/t/db_dependent/Koha/Objects/Mixin/AdditionalFields.t b/t/db_dependent/Koha/Objects/Mixin/AdditionalFields.t new file mode 100755 index 0000000000..08bf4e9fad --- /dev/null +++ b/t/db_dependent/Koha/Objects/Mixin/AdditionalFields.t @@ -0,0 +1,126 @@ +#!/usr/bin/env perl + +use Modern::Perl; + +use Test::More tests => 10; + +use Koha::Acquisition::Baskets; # Koha::Acquisition::Baskets uses the mixin +use Koha::AdditionalFields; +use Koha::AdditionalField; +use Koha::AdditionalFieldValue; +use Koha::Database; + +use t::lib::TestBuilder; + +my $storage = Koha::Database->new->schema->storage; +$storage->txn_begin; + +my $builder = t::lib::TestBuilder->new; +my $basket1 = $builder->build_object({ + class => 'Koha::Acquisition::Baskets', +}); +my $basket2 = $builder->build_object({ + class => 'Koha::Acquisition::Baskets', +}); + +my $foo = Koha::AdditionalField->new({ + tablename => 'aqbasket', + name => 'basket_foo', +})->store; +my $bar = Koha::AdditionalField->new({ + tablename => 'aqbasket', + name => 'basket_bar', +})->store; + +Koha::AdditionalFieldValue->new({ + field_id => $foo->id, + record_id => $basket1->basketno, + value => 'foo value for basket1', +})->store; +Koha::AdditionalFieldValue->new({ + field_id => $bar->id, + record_id => $basket1->basketno, + value => 'bar value for basket1', +})->store; +Koha::AdditionalFieldValue->new({ + field_id => $foo->id, + record_id => $basket2->basketno, + value => 'foo value for basket2', +})->store; +Koha::AdditionalFieldValue->new({ + field_id => $bar->id, + record_id => $basket2->basketno, + value => 'bar value for basket2', +})->store; + +my @baskets = Koha::Acquisition::Baskets->search_additional_fields([ + { + id => $foo->id, + value => 'foo value for basket1', + }, +]); + +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([ + { + id => $foo->id, + value => 'foo value for basket2', + }, +]); + +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([ + { + id => $foo->id, + value => 'foo value for basket1', + }, + { + id => $bar->id, + value => 'bar value for basket1', + }, +]); + +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([ + { + id => $foo->id, + value => 'foo value for basket1', + }, + { + id => $bar->id, + value => 'bar value for basket2', + }, +]); + +is(scalar @baskets, 0, 'search returns no result'); + +@baskets = Koha::Acquisition::Baskets->search_additional_fields([ + { + id => $foo->id, + value => 'foo', + }, +]); + +is(scalar @baskets, 2, 'search returns two results'); + +@baskets = Koha::Acquisition::Baskets->search_additional_fields([ + { + id => $foo->id, + value => 'foo', + }, + { + id => $foo->id, + value => 'basket1', + }, +]); + +is(scalar @baskets, 1, 'search returns only one result'); +is($baskets[0]->basketno, $basket1->basketno, 'result is basket1'); + +$storage->txn_rollback; -- 2.39.5