From 761a5597bd2592bc42e0f21a127fb577483b03b5 Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Thu, 2 Jan 2020 11:04:10 -0300
Subject: [PATCH] Bug 24321: Clean /patrons
---
Koha/REST/V1/Patrons.pm | 245 ++++++----------------------------------
1 file changed, 35 insertions(+), 210 deletions(-)
diff --git a/Koha/REST/V1/Patrons.pm b/Koha/REST/V1/Patrons.pm
index 1345d38046..0ffd5e6545 100644
--- a/Koha/REST/V1/Patrons.pm
+++ b/Koha/REST/V1/Patrons.pm
@@ -43,29 +43,46 @@ sub list {
my $c = shift->openapi->valid_input or return;
return try {
- my $attributes = {};
+
+ my $patrons_rs = Koha::Patrons->new;
my $args = $c->validation->output;
- my ( $params, $reserved_params ) = $c->extract_reserved_params( $args );
+ my $attributes = {};
+
+ # Extract reserved params
+ my ( $filtered_params, $reserved_params ) = $c->extract_reserved_params($args);
+
+ my $restricted = delete $filtered_params->{restricted};
# Merge sorting into query attributes
- $c->dbic_merge_sorting({ attributes => $attributes, params => $reserved_params });
+ $c->dbic_merge_sorting(
+ {
+ attributes => $attributes,
+ params => $reserved_params,
+ result_set => $patrons_rs
+ }
+ );
# Merge pagination into query attributes
- $c->dbic_merge_pagination({ filter => $attributes, params => $reserved_params });
+ $c->dbic_merge_pagination(
+ {
+ filter => $attributes,
+ params => $reserved_params
+ }
+ );
- my $restricted = $args->{restricted};
+ if ( defined $filtered_params ) {
- $params = _to_model($params)
- if defined $params;
- # deal with string params
- $params = $c->build_query_params( $params, $reserved_params );
+ # Apply the mapping function to the passed params
+ $filtered_params = $patrons_rs->attributes_from_api($filtered_params);
+ $filtered_params = $c->build_query_params( $filtered_params, $reserved_params );
+ }
# translate 'restricted' => 'debarred'
- $params->{debarred} = { '!=' => undef }
+ $filtered_params->{debarred} = { '!=' => undef }
if $restricted;
- my $patrons = Koha::Patrons->search( $params, $attributes );
- if ( $patrons->is_paged ) {
+ my $patrons = $patrons_rs->search( $filtered_params, $attributes );
+ if ( $patrons_rs->is_paged ) {
$c->add_pagination_headers(
{
total => $patrons->pager->total_entries,
@@ -132,6 +149,9 @@ sub add {
);
}
catch {
+
+ my $to_api_mapping = Koha::Patron->new->to_api_mapping;
+
unless ( blessed $_ && $_->can('rethrow') ) {
return $c->render(
status => 500,
@@ -149,7 +169,7 @@ sub add {
status => 400,
openapi => {
error => "Given "
- . $Koha::REST::V1::Patrons::to_api_mapping->{ $_->broken_fk }
+ . $to_api_mapping->{ $_->broken_fk }
. " does not exist"
}
);
@@ -159,7 +179,7 @@ sub add {
status => 400,
openapi => {
error => "Given "
- . $Koha::REST::V1::Patrons::to_api_mapping->{ $_->parameter }
+ . $to_api_mapping->{ $_->parameter }
. " does not exist"
}
);
@@ -218,7 +238,7 @@ sub update {
return $c->render(
status => 400,
openapi => { error => "Given " .
- $Koha::REST::V1::Patrons::to_api_mapping->{$_->broken_fk}
+ $patron->to_api_mapping->{$_->broken_fk}
. " does not exist" }
);
}
@@ -379,199 +399,4 @@ sub guarantors_can_see_checkouts {
};
}
-=head2 Internal methods
-
-=head3 _to_api
-
-Helper function that maps unblessed Koha::Patron objects into REST api
-attribute names.
-
-=cut
-
-sub _to_api {
- my $patron = shift;
- my $patron_id = $patron->{ borrowernumber };
-
- # Rename attributes
- foreach my $column ( keys %{ $Koha::REST::V1::Patrons::to_api_mapping } ) {
- my $mapped_column = $Koha::REST::V1::Patrons::to_api_mapping->{$column};
- if ( exists $patron->{ $column }
- && defined $mapped_column )
- {
- # key != undef
- $patron->{ $mapped_column } = delete $patron->{ $column };
- }
- elsif ( exists $patron->{ $column }
- && !defined $mapped_column )
- {
- # key == undef
- delete $patron->{ $column };
- }
- }
-
- # Calculate the 'restricted' field
- my $patron_obj = Koha::Patrons->find( $patron_id );
- $patron->{ restricted } = ($patron_obj->is_debarred) ? Mojo::JSON->true : Mojo::JSON->false;
-
- return $patron;
-}
-
-=head3 _to_model
-
-Helper function that maps REST api objects into Koha::Patron
-attribute names.
-
-=cut
-
-sub _to_model {
- my $patron = shift;
-
- foreach my $attribute ( keys %{ $Koha::REST::V1::Patrons::to_model_mapping } ) {
- my $mapped_attribute = $Koha::REST::V1::Patrons::to_model_mapping->{$attribute};
- if ( exists $patron->{ $attribute }
- && defined $mapped_attribute )
- {
- # key => !undef
- $patron->{ $mapped_attribute } = delete $patron->{ $attribute };
- }
- elsif ( exists $patron->{ $attribute }
- && !defined $mapped_attribute )
- {
- # key => undef / to be deleted
- delete $patron->{ $attribute };
- }
- }
-
- # TODO: Get rid of this once write operations are based on Koha::Patron
- if ( exists $patron->{lost} ) {
- $patron->{lost} = ($patron->{lost}) ? 1 : 0;
- }
-
- if ( exists $patron->{ gonenoaddress} ) {
- $patron->{gonenoaddress} = ($patron->{gonenoaddress}) ? 1 : 0;
- }
-
- if ( exists $patron->{lastseen} ) {
- $patron->{lastseen} = output_pref({ str => $patron->{lastseen}, dateformat => 'sql' });
- }
-
- if ( exists $patron->{updated_on} ) {
- $patron->{updated_on} = output_pref({ str => $patron->{updated_on}, dateformat => 'sql' });
- }
-
- return $patron;
-}
-
-=head2 Global variables
-
-=head3 $to_api_mapping
-
-=cut
-
-our $to_api_mapping = {
- borrowernotes => 'staff_notes',
- borrowernumber => 'patron_id',
- branchcode => 'library_id',
- categorycode => 'category_id',
- checkprevcheckout => 'check_previous_checkout',
- contactfirstname => undef, # Unused
- contactname => undef, # Unused
- contactnote => 'altaddress_notes',
- contacttitle => undef, # Unused
- dateenrolled => 'date_enrolled',
- dateexpiry => 'expiry_date',
- dateofbirth => 'date_of_birth',
- debarred => undef, # replaced by 'restricted'
- debarredcomment => undef, # calculated, API consumers will use /restrictions instead
- emailpro => 'secondary_email',
- flags => undef, # permissions manipulation handled in /permissions
- gonenoaddress => 'incorrect_address',
- guarantorid => 'guarantor_id',
- lastseen => 'last_seen',
- lost => 'patron_card_lost',
- opacnote => 'opac_notes',
- othernames => 'other_name',
- password => undef, # password manipulation handled in /password
- phonepro => 'secondary_phone',
- relationship => 'relationship_type',
- sex => 'gender',
- smsalertnumber => 'sms_number',
- sort1 => 'statistics_1',
- sort2 => 'statistics_2',
- streetnumber => 'street_number',
- streettype => 'street_type',
- zipcode => 'postal_code',
- B_address => 'altaddress_address',
- B_address2 => 'altaddress_address2',
- B_city => 'altaddress_city',
- B_country => 'altaddress_country',
- B_email => 'altaddress_email',
- B_phone => 'altaddress_phone',
- B_state => 'altaddress_state',
- B_streetnumber => 'altaddress_street_number',
- B_streettype => 'altaddress_street_type',
- B_zipcode => 'altaddress_postal_code',
- altcontactaddress1 => 'altcontact_address',
- altcontactaddress2 => 'altcontact_address2',
- altcontactaddress3 => 'altcontact_city',
- altcontactcountry => 'altcontact_country',
- altcontactfirstname => 'altcontact_firstname',
- altcontactphone => 'altcontact_phone',
- altcontactsurname => 'altcontact_surname',
- altcontactstate => 'altcontact_state',
- altcontactzipcode => 'altcontact_postal_code'
-};
-
-=head3 $to_model_mapping
-
-=cut
-
-our $to_model_mapping = {
- altaddress_notes => 'contactnote',
- category_id => 'categorycode',
- check_previous_checkout => 'checkprevcheckout',
- date_enrolled => 'dateenrolled',
- date_of_birth => 'dateofbirth',
- expiry_date => 'dateexpiry',
- gender => 'sex',
- guarantor_id => 'guarantorid',
- incorrect_address => 'gonenoaddress',
- last_seen => 'lastseen',
- library_id => 'branchcode',
- opac_notes => 'opacnote',
- other_name => 'othernames',
- patron_card_lost => 'lost',
- patron_id => 'borrowernumber',
- postal_code => 'zipcode',
- relationship_type => 'relationship',
- restricted => undef,
- secondary_email => 'emailpro',
- secondary_phone => 'phonepro',
- sms_number => 'smsalertnumber',
- staff_notes => 'borrowernotes',
- statistics_1 => 'sort1',
- statistics_2 => 'sort2',
- street_number => 'streetnumber',
- street_type => 'streettype',
- altaddress_address => 'B_address',
- altaddress_address2 => 'B_address2',
- altaddress_city => 'B_city',
- altaddress_country => 'B_country',
- altaddress_email => 'B_email',
- altaddress_phone => 'B_phone',
- altaddress_state => 'B_state',
- altaddress_street_number => 'B_streetnumber',
- altaddress_street_type => 'B_streettype',
- altaddress_postal_code => 'B_zipcode',
- altcontact_firstname => 'altcontactfirstname',
- altcontact_surname => 'altcontactsurname',
- altcontact_address => 'altcontactaddress1',
- altcontact_address2 => 'altcontactaddress2',
- altcontact_city => 'altcontactaddress3',
- altcontact_state => 'altcontactstate',
- altcontact_postal_code => 'altcontactzipcode',
- altcontact_country => 'altcontactcountry',
- altcontact_phone => 'altcontactphone'
-};
-
1;
--
2.24.1