Bugzilla – Attachment 124914 Details for
Bug 28948
Add a /public counterpart for the libraries REST endpoints
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 28948: Changes
Bug-28948-Changes.patch (text/plain), 10.70 KB, created by
Marcel de Rooy
on 2021-09-16 08:47:07 UTC
(
hide
)
Description:
Bug 28948: Changes
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2021-09-16 08:47:07 UTC
Size:
10.70 KB
patch
obsolete
>From b241eafbe3c4a925a6e3bfcee8567e870a0f73d0 Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Date: Thu, 16 Sep 2021 07:03:42 +0000 >Subject: [PATCH] Bug 28948: Changes >Content-Type: text/plain; charset=utf-8 > >Touched Objects.t now too. >--- > Koha/Library/Allowlist.pm | 22 ------------ > Koha/Object.pm | 18 ++-------- > Koha/Schema/Result/Branch.pm | 21 +++++++++++ > t/db_dependent/Koha/Object.t | 50 +++++++-------------------- > t/db_dependent/Koha/Objects.t | 65 +++++++---------------------------- > 5 files changed, 49 insertions(+), 127 deletions(-) > delete mode 100644 Koha/Library/Allowlist.pm > >diff --git a/Koha/Library/Allowlist.pm b/Koha/Library/Allowlist.pm >deleted file mode 100644 >index c35fceab0a..0000000000 >--- a/Koha/Library/Allowlist.pm >+++ /dev/null >@@ -1,22 +0,0 @@ >-package Koha::Patron::Allowlist; >- >-use Modern::Perl; >-use parent 'Koha::Allowlist'; >- >-sub load { >- my ($self) = @_; >- my @public_read = qw( >- branchcode branchname branchaddress1 >- branchaddress2 branchaddress3 branchzip >- branchcity branchstate branchcountry >- branchfax branchemail branchurl ); >- $self->add(@public_read); >- >- my @staff_read = qw( >- branchillemail branchreplyto branchreturnpath >- branchip branchnotes marcorgcode >- ); >- $self->add(@staff_read) if $self->{interface} eq 'staff'; >-} >- >-1; >diff --git a/Koha/Object.pm b/Koha/Object.pm >index 5bba55343d..94fa59b2b6 100644 >--- a/Koha/Object.pm >+++ b/Koha/Object.pm >@@ -556,21 +556,9 @@ sub to_api { > $params //= {}; > > # Remove forbidden attributes if required >- # FIXME: All modules should have a corresponding allowlist eventually >- my $allowclass = ref($self) . '::Allowlist'; >- if ( can_load( modules => { $allowclass => undef } ) ) { >- my $allowlist = $allowclass->new( >- { interface => $params->{public} ? 'opac' : 'staff' } ); >- $allowlist->load; >- my $blocked = $allowlist->apply( { input => $json_object } ); >- Koha::Logger->get->debug( >- ref($self) >- . "::to_api allowlist blocked fields: " >- . ( >- join ', ', >- map { $_ . '=' . $blocked->{$_} } sort keys %$blocked >- ) >- ) if keys %$blocked; >+ if( $self->_result->can('get_column_set') ) { #FIXME Temporary measure >+ my $set_name = $params->{public} ? 'public_read' : 'staff_read'; >+ $json_object = $self->filter({ input => $json_object, column_set => $set_name, log_level => 'debug' }); > } > > my $to_api_mapping = $self->to_api_mapping; >diff --git a/Koha/Schema/Result/Branch.pm b/Koha/Schema/Result/Branch.pm >index 500d100d05..be661624ad 100644 >--- a/Koha/Schema/Result/Branch.pm >+++ b/Koha/Schema/Result/Branch.pm >@@ -901,4 +901,25 @@ sub koha_objects_class { > 'Koha::Libraries'; > } > >+sub get_column_set { >+ my ( $self, $name ) = @_; >+ my $column_sets = { >+ public_read => [qw( >+ branchcode branchname branchaddress1 >+ branchaddress2 branchaddress3 branchzip >+ branchcity branchstate branchcountry >+ branchfax branchemail branchurl >+ )], >+ staff_read => [qw( >+ branchcode branchname branchaddress1 >+ branchaddress2 branchaddress3 branchzip >+ branchcity branchstate branchcountry >+ branchfax branchemail branchurl >+ branchillemail branchreplyto branchreturnpath >+ branchip branchnotes marcorgcode >+ )], >+ }; >+ return $column_sets->{$name} // []; >+} >+ > 1; >diff --git a/t/db_dependent/Koha/Object.t b/t/db_dependent/Koha/Object.t >index d89f2c8973..10a9b29d8c 100755 >--- a/t/db_dependent/Koha/Object.t >+++ b/t/db_dependent/Koha/Object.t >@@ -31,7 +31,7 @@ use Koha::Database; > use Koha::Acquisition::Orders; > use Koha::DateUtils qw( dt_from_string ); > use Koha::Libraries; >-use Koha::Library::Allowlist; >+use Koha::Object::ColumnSet; > use Koha::Patrons; > use Koha::ApiKeys; > >@@ -376,49 +376,25 @@ subtest "to_api() tests" => sub { > > subtest 'unprivileged request tests' => sub { > >- my @all_attrs = Koha::Libraries->columns(); >- my $allowlist = Koha::Library::Allowlist->new({ interface => 'opac' })->load(); >- my $public_attrs = $allowlist->{_entries}; >+ # Create a sample library >+ my $library = $builder->build_object({ class => 'Koha::Libraries' }); >+ >+ my @all_attrs = @{ $library->_columns }; >+ my $public_attrs = { map { ( $_ => 1 ) } @{$library->_result->get_column_set('public_read')} }; >+ my $staff_attrs = { map { ( $_ => 1 ) } @{$library->_result->get_column_set('staff_read')} }; > my $mapping = Koha::Library->to_api_mapping; > > plan tests => scalar @all_attrs * 2; > >- # Create a sample library >- my $library = $builder->build_object( { class => 'Koha::Libraries' } ); >- >- my $unprivileged_representation = $library->to_api({ public => 1 }); >+ my $public_representation = $library->to_api({ public => 1 }); > my $privileged_representation = $library->to_api; > > foreach my $attr (@all_attrs) { >- my $mapped = exists $mapping->{$attr} ? $mapping->{$attr} : $attr; >- if ( defined($mapped) ) { >- ok( >- exists $privileged_representation->{$mapped}, >- "Attribute '$attr' is present when privileged" >- ); >- if ( exists $public_attrs->{$attr} ) { >- ok( >- exists $unprivileged_representation->{$mapped}, >- "Attribute '$attr' is present when public" >- ); >- } >- else { >- ok( >- !exists $unprivileged_representation->{$mapped}, >- "Attribute '$attr' is not present when public" >- ); >- } >- } >- else { >- ok( >- !exists $privileged_representation->{$attr}, >- "Unmapped attribute '$attr' is not present when privileged" >- ); >- ok( >- !exists $unprivileged_representation->{$attr}, >- "Unmapped attribute '$attr' is not present when public" >- ); >- } >+ my $mapped = $mapping->{$attr} // $attr; >+ is( exists $privileged_representation->{$mapped}, >+ exists $staff_attrs->{$attr}, "Verify $attr in privileged" ); >+ is( exists $public_representation->{$mapped}, >+ exists $public_attrs->{$attr}, "Verify $attr in public" ); > } > }; > >diff --git a/t/db_dependent/Koha/Objects.t b/t/db_dependent/Koha/Objects.t >index e5168f41c4..587d3fa0b0 100755 >--- a/t/db_dependent/Koha/Objects.t >+++ b/t/db_dependent/Koha/Objects.t >@@ -32,7 +32,6 @@ use Koha::Biblios; > use Koha::Patron::Category; > use Koha::Patron::Categories; > use Koha::Patrons; >-use Koha::Library::Allowlist; > use Koha::Database; > use Koha::DateUtils qw( dt_from_string ); > >@@ -400,67 +399,27 @@ subtest "to_api() tests" => sub { > } > > subtest 'unprivileged request tests' => sub { >- >- my @all_attrs = Koha::Libraries->columns(); >- my $allowlist = Koha::Library::Allowlist->new({ interface => 'opac' })->load(); >- my $public_attrs = $allowlist->{_entries}; >- my $mapping = Koha::Library->to_api_mapping; >+ plan tests => 4; > > # Create sample libraries > my $library_1 = $builder->build_object({ class => 'Koha::Libraries' }); > my $library_2 = $builder->build_object({ class => 'Koha::Libraries' }); >- my $libraries = Koha::Libraries->search( >- { >- branchcode => { >- '-in' => [ >- $library_1->branchcode, $library_2->branchcode >- ] >- } >- } >- ); >- >- plan tests => scalar @all_attrs * 2 * $libraries->count; >+ my $libraries = Koha::Libraries->search({ >+ branchcode => [ $library_1->branchcode, $library_2->branchcode ], >+ }); > > my $libraries_unprivileged_representation = $libraries->to_api({ public => 1 }); > my $libraries_privileged_representation = $libraries->to_api(); >+ my $mapping = Koha::Library->to_api_mapping; > >- for (my $i = 0; $i < $libraries->count; $i++) { >- my $privileged_representation = $libraries_privileged_representation->[$i]; >- my $unprivileged_representation = $libraries_unprivileged_representation->[$i]; >- foreach my $attr (@all_attrs) { >- my $mapped = exists $mapping->{$attr} ? $mapping->{$attr} : $attr; >- if ( defined($mapped) ) { >- ok( >- exists $privileged_representation->{$mapped}, >- "Attribute '$attr' is present when privileged" >- ); >- if ( exists $public_attrs->{$attr} ) { >- ok( >- exists $unprivileged_representation->{$mapped}, >- "Attribute '$attr' is present when public" >- ); >- } >- else { >- ok( >- !exists $unprivileged_representation->{$mapped}, >- "Attribute '$attr' is not present when public" >- ); >- } >- } >- else { >- ok( >- !exists $privileged_representation->{$attr}, >- "Unmapped attribute '$attr' is not present when privileged" >- ); >- ok( >- !exists $unprivileged_representation->{$attr}, >- "Unmapped attribute '$attr' is not present when public" >- ); >- } >- } >- } >- }; >+ my $public_attrs = [ sort map { $mapping->{$_} //$_ } @{$library_1->_result->get_column_set('public_read')} ]; >+ is_deeply( [ sort keys %{$libraries_unprivileged_representation->[0]} ], $public_attrs, 'Check first, public' ); >+ is_deeply( [ sort keys %{$libraries_unprivileged_representation->[1]} ], $public_attrs, 'Check second, public' ); > >+ my $staff_attrs = [ sort map { $mapping->{$_} //$_ } @{$library_1->_result->get_column_set('staff_read')} ]; >+ is_deeply( [ sort keys %{$libraries_privileged_representation->[0]} ], $staff_attrs, 'Check first, staff' ); >+ is_deeply( [ sort keys %{$libraries_privileged_representation->[1]} ], $staff_attrs, 'Check second, staff' ); >+ }; > > $schema->storage->txn_rollback; > }; >-- >2.20.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 28948
:
124479
|
124480
|
124481
|
124482
|
124483
|
124749
|
124911
|
124914
|
125901
|
125902
|
125903
|
125904
|
125905
|
125906
|
125907
|
125908
|
125911
|
125912
|
126111
|
126112
|
126113
|
126114
|
126115
|
126116
|
126117
|
126118
|
126119
|
126120
|
126121
|
126122
|
126123
|
126124
|
126125
|
126126
|
127054
|
127055