From f015da347dbe1255a5c2d6b0a0b580f1d2ee30fc Mon Sep 17 00:00:00 2001
From: Lari Taskula <lari.taskula@jns.fi>
Date: Mon, 22 May 2017 18:05:24 +0300
Subject: [PATCH] Bug 16330: Spot unchanged modification-request in
 Koha::Patron::Modification

Throw Koha::Exceptions::NoChanges if attempting to create a modification request
without changing anything.

This exception is caught in Patron REST API controller.

Also, validate changes via Koha::Patron->_validate

To test:
1. prove t/db_dependent/Koha/Patron/Modifications.t
---
 Koha/Patron/Modification.pm                | 40 ++++++++++++++++++++++++++++++
 t/db_dependent/Koha/Patron/Modifications.t | 38 +++++++++++++++++++++++++---
 2 files changed, 75 insertions(+), 3 deletions(-)

diff --git a/Koha/Patron/Modification.pm b/Koha/Patron/Modification.pm
index 1381a6a..024cc4a 100644
--- a/Koha/Patron/Modification.pm
+++ b/Koha/Patron/Modification.pm
@@ -1,6 +1,7 @@
 package Koha::Patron::Modification;
 
 # Copyright ByWater Solutions 2014
+# Copyright Koha-Suomi Oy 2016
 #
 # This file is part of Koha.
 #
@@ -22,6 +23,9 @@ use Modern::Perl;
 use Carp;
 
 use Koha::Database;
+use Koha::Exceptions;
+use Koha::Patrons;
+
 use Koha::Exceptions::Patron::Modification;
 use Koha::Patron::Attribute;
 use Koha::Patron::Attributes;
@@ -94,6 +98,42 @@ sub store {
         };
     }
 
+    my $patron = Koha::Patrons->find( $self->borrowernumber );
+    if ( $patron ) {
+        my %columns = map { $_ => 1 } Koha::Patrons->columns;
+
+        my $changes_made = 0;
+        foreach my $column ( keys %{$self->unblessed} ) {
+            if (exists $columns{$column}) {
+                if ($patron->$column ne $self->$column) {
+                    $changes_made = 1;
+                    last;
+                }
+                # Set the changes for patron without commiting - this will be used
+                # for validating the given parameters via Koha::Patron.
+                # Do not ->store the patron!
+                $patron->$column($self->$column);
+            }
+        }
+        my $extended_attributes = try { from_json( $self->extended_attributes ) };
+        if (!$changes_made && $extended_attributes) {
+            my %codes = map { $_->{code} => $_->{value} } @{$extended_attributes};
+            foreach my $code (keys %codes) {
+                delete $codes{$code} if Koha::Patron::Attributes->search({
+                        borrowernumber => $patron->borrowernumber,
+                        code           => $code,
+                        attribute      => $codes{$code}
+                    })->count > 0;
+            }
+            $changes_made = 1 if keys %codes > 0;
+        }
+        Koha::Exceptions::NoChanges->throw(
+            error => "No changes have been made",
+        ) unless $changes_made;
+        # Validate changes via Koha::Patron
+        $patron->_validate;
+    }
+
     return $self->SUPER::store();
 }
 
diff --git a/t/db_dependent/Koha/Patron/Modifications.t b/t/db_dependent/Koha/Patron/Modifications.t
index b24cfe3..d021bc1 100755
--- a/t/db_dependent/Koha/Patron/Modifications.t
+++ b/t/db_dependent/Koha/Patron/Modifications.t
@@ -97,7 +97,7 @@ subtest 'new() tests' => sub {
 
 subtest 'store( extended_attributes ) tests' => sub {
 
-    plan tests => 4;
+    plan tests => 6;
 
     $schema->storage->txn_begin;
 
@@ -106,8 +106,14 @@ subtest 'store( extended_attributes ) tests' => sub {
     my $patron
         = $builder->build( { source => 'Borrower' } )->{borrowernumber};
     my $verification_token = md5_hex( time().{}.rand().{}.$$ );
-    my $valid_json_text    = '[{"code":"CODE","value":"VALUE"}]';
-    my $invalid_json_text  = '[{"code":"CODE";"value":"VALUE"}]';
+    $builder->build(
+        { source => 'BorrowerAttributeType', value => { code => 'CODE_1' } }
+    );
+    $builder->build(
+        { source => 'BorrowerAttributeType', value => { code => 'CODE_2' } }
+    );
+    my $valid_json_text    = '[{"code":"CODE_1","value":"VALUE"}]';
+    my $invalid_json_text  = '[{"code":"CODE_2";"value":"VALUE"}]';
 
     Koha::Patron::Modification->new(
         {   verification_token  => $verification_token,
@@ -142,6 +148,32 @@ subtest 'store( extended_attributes ) tests' => sub {
 
     is( $@, 'The passed extended_attributes is not valid JSON' );
 
+    Koha::Patrons->find($patron)->surname('Hall')->store;
+    throws_ok {
+        Koha::Patron::Modification->new(
+            {   verification_token  => $verification_token,
+                borrowernumber      => $patron,
+                surname             => 'Hall',
+            }
+        )->store();
+    }
+    'Koha::Exceptions::NoChanges',
+        'Trying to create a modification request without changing anything'
+        .' raises exception';
+
+    $patron_modification->approve;
+    throws_ok {
+        Koha::Patron::Modification->new(
+            {   verification_token  => $verification_token,
+                borrowernumber      => $patron,
+                extended_attributes => $valid_json_text,
+            }
+        )->store();
+    }
+    'Koha::Exceptions::NoChanges',
+        'Trying to create a modification request without changing anything'
+        .' raises exception';
+
     $schema->storage->txn_rollback;
 };
 
-- 
2.7.4