Bug 15774: Use Koha::Object(s) for additional fields
authorJulian Maurice <julian.maurice@biblibre.com>
Fri, 4 May 2018 15:35:45 +0000 (17:35 +0200)
committerNick Clemens <nick@bywatersolutions.com>
Thu, 7 Mar 2019 20:37:05 +0000 (20:37 +0000)
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 <severine.queune@bulac.fr>

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>

28 files changed:
C4/Acquisition.pm
C4/Serials.pm
Koha/Acquisition/Basket.pm
Koha/Acquisition/Baskets.pm
Koha/AdditionalField.pm
Koha/AdditionalFieldValue.pm [new file with mode: 0644]
Koha/AdditionalFieldValues.pm [new file with mode: 0644]
Koha/AdditionalFields.pm [new file with mode: 0644]
Koha/Object/Mixin/AdditionalFields.pm [new file with mode: 0644]
Koha/Objects/Mixin/AdditionalFields.pm [new file with mode: 0644]
Koha/Schema/Result/Aqbasket.pm
Koha/Schema/Result/Subscription.pm
Koha/Subscription.pm
Koha/Subscriptions.pm
acqui/basket.pl
acqui/basketheader.pl
acqui/histsearch.pl
admin/additional-fields.pl
koha-tmpl/intranet-tmpl/prog/en/includes/additional-fields-entry.inc
koha-tmpl/intranet-tmpl/prog/en/modules/admin/additional-fields.tt
koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-search.tt
serials/claims.pl
serials/serials-search.pl
serials/subscription-add.pl
serials/subscription-batchedit.pl
serials/subscription-detail.pl
t/db_dependent/AdditionalField.t [deleted file]
t/db_dependent/Koha/Objects/Mixin/AdditionalFields.t [new file with mode: 0755]

index 8ed768b..9200318 100644 (file)
@@ -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") ) {
index e929e21..b4452a3 100644 (file)
@@ -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;
     }
index d5e6b8c..5bda3ab 100644 (file)
@@ -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
 
index 2de4683..3dc0414 100644 (file)
@@ -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
 
index 7c4903a..78de775 100644 (file)
@@ -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<id>
-
-    The caller just knows the id of the additional field and want to retrieve all values.
-
-=item B<tablename, name, authorised_value_category, marcfield and searchable>
-
-    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<get_matching_record_ids> and
-C<get_filters_as_values>.
-
-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<get_filters_from_query> into a hashref of C<name> => C<value>, 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 <jonathan.druart at biblibre.com>
diff --git a/Koha/AdditionalFieldValue.pm b/Koha/AdditionalFieldValue.pm
new file mode 100644 (file)
index 0000000..f54c811
--- /dev/null
@@ -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 (file)
index 0000000..fa3576f
--- /dev/null
@@ -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 (file)
index 0000000..2d4761b
--- /dev/null
@@ -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 (file)
index 0000000..090e282
--- /dev/null
@@ -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 (file)
index 0000000..62c784f
--- /dev/null
@@ -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;
index bb1e5a7..0419636 100644 (file)
@@ -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;
index 3666dd6..5ccb9ee 100644 (file)
@@ -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;
index 570ae1f..833f674 100644 (file)
@@ -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
 
index 3538c15..f92b586 100644 (file)
@@ -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
 
index 8a09419..52d3ef8 100755 (executable)
@@ -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 },
     );
 }
 
index acd3d67..da2129f 100755 (executable)
@@ -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;
index a1c7fe6..f5dca49 100755 (executable)
@@ -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.
index 0fc8b5f..5bc722c 100755 (executable)
@@ -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 );
 }
 
index 123e5b3..c8cbd83 100644 (file)
@@ -11,7 +11,7 @@
                     <select name="additional_field_[% field.id %]" id="additional_field_[% field.id %]">
                         <option value="">All</option>
                         [% FOREACH av IN AuthorisedValues.GetAuthValueDropbox( field.authorised_value_category ) %]
-                            [% IF av.authorised_value == values.${field.name} %]
+                            [% IF av.authorised_value == values.${field.id} %]
                                 <option value="[% av.authorised_value %]" selected="selected">[% av.lib %]</option>
                             [% ELSE %]
                                 <option value="[% av.authorised_value %]">[% av.lib %]</option>
                     </select> (Authorised values for [% field.authorised_value_category %])
                 [% ELSE %]
                     [% IF field.marcfield %]
-                        <input type="text" value="[% values.${field.name} %]" id="additional_field_[% field.id %]" name="additional_field_[% field.id %]" readonly="readonly" />
+                        <input type="text" value="[% values.${field.id} %]" id="additional_field_[% field.id %]" name="additional_field_[% field.id %]" readonly="readonly" />
                         This value will be filled with the [% field.marcfield %] subfield of the selected biblio.
                     [% ELSE %]
-                        <input type="text" value="[% values.${field.name} %]" id="additional_field_[% field.id %]" name="additional_field_[% field.id %]" />
+                        <input type="text" value="[% values.${field.id} %]" id="additional_field_[% field.id %]" name="additional_field_[% field.id %]" />
                     [% END %]
                 [% END %]
             </li>
index a37401a..bdd6984 100755 (executable)
@@ -12,6 +12,9 @@
 [% Asset.css('css/datatables.css') %]
 </head>
 
+[% marcfield_tables = ['subscription'] %]
+[% show_marcfield = marcfield_tables.grep('^' _ tablename _ '$').size ? 1 : 0 %]
+
 <body id="ser_add_fields" class="ser">
     [% INCLUDE 'header.inc' %]
     [% INCLUDE 'cat-search.inc' %]
@@ -77,7 +80,9 @@
                     <tr>
                         <th>Name</th>
                         <th>Authorised value category</th>
-                        <th>MARC field</th>
+                        [% IF show_marcfield %]
+                            <th>MARC field</th>
+                        [% END %]
                         <th>Searchable</th>
                         <th>Actions</th>
                     </tr>
                         <tr>
                             <td>[% field.name %]</td>
                             <td>[% field.authorised_value_category %]</td>
-                            <td>[% field.marcfield %]</td>
+                            [% IF show_marcfield %]
+                                <td>[% field.marcfield %]</td>
+                            [% END %]
                             <td>
                                 [% IF field.searchable %]Yes[% ELSE %]No[% END %]
                             </td>
                             <td class="actions">
                                 <a class="btn btn-default btn-xs" href="?op=add_form&amp;field_id=[% field.id %]"><i class="fa fa-pencil"></i> Edit</a>
-                                <a class="confirmdelete btn btn-default btn-xs" href="?op=delete&amp;field_id=[% field.id %]"><i class="fa fa-trash"></i> Delete</a>
+                                <a class="confirmdelete btn btn-default btn-xs" href="?op=delete&amp;field_id=[% field.id %]&amp;tablename=[% tablename %]"><i class="fa fa-trash"></i> Delete</a>
                             </td>
                         </tr>
                     [% END %]
                             %]
                         </select>
                     </li>
-                    <li>
-                        <label for="marcfield">MARC field: </label>
-                        <input type="text" name="marcfield" id="marcfield" value="[% field.marcfield %]" />
-                    </li>
+                    [% IF show_marcfield %]
+                        <li>
+                            <label for="marcfield">MARC field: </label>
+                            <input type="text" name="marcfield" id="marcfield" value="[% field.marcfield %]" />
+                        </li>
+                    [% END %]
                     <li>
                         <label for="searchable">Searchable: </label>
                         [% IF field.searchable %]
index 01ebae0..2e2c342 100644 (file)
                       <select id="additional_field_[% field.id | html %]" name="additional_field_[% field.id | html %]">
                         <option value="">All</option>
                         [% FOREACH av IN AuthorisedValues.GetAuthValueDropbox(field.authorised_value_category) %]
-                          [% IF av.authorised_value == additional_field_filters.${field.name} %]
+                          [% IF av.authorised_value == additional_field_filters.${field.id} %]
                             <option value="[% av.authorised_value | html %]" selected="selected">[% av.lib | html %]</option>
                           [% ELSE %]
                             <option value="[% av.authorised_value | html %]">[% av.lib | html %]</option>
                         [% END %]
                       </select>
                     [% ELSE %]
-                      <input id="additional_field_[% field.id | html %]" type="text" value="[% additional_field_filters.${field.name} | html %]" name="additional_field_[% field.id | html %]" />
+                      <input id="additional_field_[% field.id | html %]" type="text" value="[% additional_field_filters.${field.id} | html %]" name="additional_field_[% field.id | html %]" />
                     [% END %]
                   </li>
                 [% END %]
index 44cff74..2ed81b8 100755 (executable)
@@ -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
index 976b128..b1fa40f 100755 (executable)
@@ -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
     );
index 7cc131d..4d3310d 100755 (executable)
@@ -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;
index f0f5690..fa114a1 100755 (executable)
@@ -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'),
 );
 
index 451e5a6..c023c74 100755 (executable)
@@ -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 (file)
index 2a3a680..0000000
+++ /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 (executable)
index 0000000..08bf4e9
--- /dev/null
@@ -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;