From e4be0920543db3a8ded2890464db8792a129ac5b Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Fri, 28 Oct 2022 11:50:13 -0300
Subject: [PATCH] Bug 26635: AV expansion should ackowledge $params->{public}

This patch simplifies the original implementation so it is more
readable, removing extra loops and methods and just dealing with the
coded values inline on the existing loops.

It also adds non-public attributes removal from the original list, to
address the changes introduced by bug 28948.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Object.t \
           t/db_dependent/Koha/REST/Plugin/Objects.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
---
 Koha/Object.pm | 78 +++++++++++++++++---------------------------------
 1 file changed, 26 insertions(+), 52 deletions(-)

diff --git a/Koha/Object.pm b/Koha/Object.pm
index 384e7fd59a..a574bf8fc7 100644
--- a/Koha/Object.pm
+++ b/Koha/Object.pm
@@ -553,35 +553,52 @@ sub to_api {
     my ( $self, $params ) = @_;
     my $json_object = $self->TO_JSON;
 
-    # Remove forbidden attributes if required
+    # coded values handling
+    my $avs = {};
+    if ( $params->{av_expand} and $self->can('_fetch_authorised_values') ) {
+        $avs = $self->_fetch_authorised_values;
+    }
+
+    # Remove forbidden attributes if required (including their coded values)
     if ( $params->{public} ) {
         for my $field ( keys %{$json_object} ) {
             delete $json_object->{$field} unless any { $_ eq $field } @{ $self->public_read_list };
         }
+
+        if ( $params->{av_expand} ) {
+            foreach my $field (keys %{$avs}) {
+                delete $avs->{$field}
+                    unless any { $_ eq $field } @{ $self->public_read_list };
+            }
+        }
     }
 
     my $to_api_mapping = $self->to_api_mapping;
 
-    # Rename attributes if there's a mapping
+    # Rename attributes and coded values if there's a mapping
     if ( $self->can('to_api_mapping') ) {
         foreach my $column ( keys %{ $self->to_api_mapping } ) {
             my $mapped_column = $self->to_api_mapping->{$column};
             if ( exists $json_object->{$column}
-                && defined $mapped_column )
-            {
+                && defined $mapped_column ) {
+
                 # key != undef
                 $json_object->{$mapped_column} = delete $json_object->{$column};
-            }
-            elsif ( exists $json_object->{$column}
-                && !defined $mapped_column )
-            {
+                $avs->{$mapped_column}         = delete $avs->{$column}
+                  if exists $avs->{$column};
+
+            } elsif ( exists $json_object->{$column}
+                && !defined $mapped_column ) {
+
                 # key == undef
                 delete $json_object->{$column};
+                delete $avs->{$column};
             }
         }
     }
 
-    my $av_expand = $params->{av_expand};
+    $json_object->{_authorised_values} = $avs
+      if $params->{av_expand};
 
     # Make sure we duplicate the $params variable to avoid
     # breaking calls in a loop (Koha::Objects->to_api)
@@ -629,49 +646,6 @@ sub to_api {
         }
     }
 
-    if ( $av_expand && $self->can('_fetch_authorised_values') ) {
-
-        # _fetch_authorised_values should return a hash as the following
-        # {
-        #  column_name => <authorised_value>->unblessed
-        #  ...
-        # }
-        my $avs = $self->_fetch_authorised_values($av_expand);
-
-        # Language selection will be implemented when lang overlay for av is ready
-        # Now we will just fetch plain authorised values from the Koha::AuthorisedValues
-        $avs = $self->_do_api_mapping($avs);
-
-        $json_object->{_authorised_values} = $avs || {};
-    }
-
-    return $json_object;
-}
-
-=head3 _do_api_mapping
-
-=cut
-
-sub _do_api_mapping {
-    my ($self, $json_object) = @_;
-    # Rename attributes if there's a mapping
-    if ( $self->can('to_api_mapping') ) {
-        foreach my $column ( keys %{ $self->to_api_mapping } ) {
-            my $mapped_column = $self->to_api_mapping->{$column};
-            if ( exists $json_object->{$column}
-                && defined $mapped_column )
-            {
-                # key != undef
-                $json_object->{$mapped_column} = delete $json_object->{$column};
-            }
-            elsif ( exists $json_object->{$column}
-                && !defined $mapped_column )
-            {
-                # key == undef
-                delete $json_object->{$column};
-            }
-        }
-    }
     return $json_object;
 }
 
-- 
2.39.5