Attachment 100240 Details for Bug 23971 – Bug 23971: (follow-up) Remove data duplication

[patch] Bug 23971: (follow-up) Remove data duplication

Bug-23971-follow-up-Remove-data-duplication.patch (text/plain), 6.07 KB, created by Andrew Isherwood on 2020-03-06 10:28:41 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Isherwood

Created: 2020-03-06 10:28:41 UTC

Size: 6.07 KB

patch

obsolete

>From c910cdce6a4804b6fc95b6d39b4865321eba3d02 Mon Sep 17 00:00:00 2001
>From: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
>Date: Fri, 6 Dec 2019 10:30:20 +0000
>Subject: [PATCH] Bug 23971: (follow-up) Remove data duplication
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>In response to SÃ©verine observations in comment #10, this patch removes
>the duplicate logging of the borrowernumber
>
>https://bugs.koha-community.org/show_bug.cgi?id=23971
>---
> C4/Acquisition.pm     | 14 ++++----------
> Koha/EDI.pm           |  2 +-
> acqui/addorder.pl     |  3 +--
> acqui/basket.pl       | 13 +++++--------
> acqui/basketheader.pl |  3 +--
> 5 files changed, 12 insertions(+), 23 deletions(-)
>
>diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm
>index 58c4d55f2f..54b787572f 100644
>--- a/C4/Acquisition.pm
>+++ b/C4/Acquisition.pm
>@@ -224,7 +224,7 @@ close a basket (becomes unmodifiable, except for receives)
> =cut
> 
> sub CloseBasket {
>-    my ($basketno, $user, $edi_approval) = @_;
>+    my ($basketno, $edi_approval) = @_;
>     my $dbh        = C4::Context->dbh;
>     $dbh->do('UPDATE aqbasket SET closedate=now() WHERE basketno=?', {}, $basketno );
> 
>@@ -236,12 +236,10 @@ q{UPDATE aqorders SET orderstatus = 'ordered' WHERE basketno = ? AND orderstatus
>     # Log the closure
>     if (C4::Context->preference("AcqLog")) {
>         my $action = $edi_approval ? 'APPROVE_BASKET' : 'CLOSE_BASKET';
>-        my $infos = $user ? sprintf("%010d", $user) : undef;
>         logaction(
>             'ACQUISITIONS',
>             $action,
>-            $basketno,
>-            $infos
>+            $basketno
>         );
>     }
> 
>@@ -570,14 +568,10 @@ sub ModBasket {
> 
>     # Log the basket update
>     if (C4::Context->preference("AcqLog")) {
>-        my $infos = $basketinfo->{borrowernumber} ?
>-            sprintf("%010d", $basketinfo->{borrowernumber}) :
>-            undef;
>         logaction(
>             'ACQUISITIONS',
>             'MODIFY_BASKET',
>-            $basketinfo->{'basketno'},
>-            $infos
>+            $basketinfo->{'basketno'}
>         );
>     }
> 
>@@ -620,7 +614,7 @@ case the AcqCreateItem syspref takes precedence).
> =cut
> 
> sub ModBasketHeader {
>-    my ($basketno, $basketname, $note, $booksellernote, $contractnumber, $booksellerid, $deliveryplace, $billingplace, $is_standing, $create_items, $borrowernumber) = @_;
>+    my ($basketno, $basketname, $note, $booksellernote, $contractnumber, $booksellerid, $deliveryplace, $billingplace, $is_standing, $create_items) = @_;
> 
>     $is_standing ||= 0;
>     my $query = qq{
>diff --git a/Koha/EDI.pm b/Koha/EDI.pm
>index 32abcec893..c1735e98f4 100644
>--- a/Koha/EDI.pm
>+++ b/Koha/EDI.pm
>@@ -576,7 +576,7 @@ sub process_quote {
>             );
>             # Close the basket, passing a flag indicating that this action
>             # originated from an approval
>-            CloseBasket($b, undef, 1);
>+            CloseBasket($b, 1);
>         }
>     }
> 
>diff --git a/acqui/addorder.pl b/acqui/addorder.pl
>index 1c288dae68..dd43092b05 100755
>--- a/acqui/addorder.pl
>+++ b/acqui/addorder.pl
>@@ -367,8 +367,7 @@ if (C4::Context->preference("AcqLog") && $basketno) {
>     logaction(
>         'ACQUISITIONS',
>         'MODIFY_BASKET',
>-        $basketno,
>-        sprintf("%010d", $loggedinuser)
>+        $basketno
>     );
> }
> 
>diff --git a/acqui/basket.pl b/acqui/basket.pl
>index 6b8bb70126..93f9733e29 100755
>--- a/acqui/basket.pl
>+++ b/acqui/basket.pl
>@@ -195,7 +195,7 @@ if ( $op eq 'delete_confirm' ) {
>     if ($confirm) {
>         my $basketno = $query->param('basketno');
>         my $booksellerid = $query->param('booksellerid');
>-        $basketno =~ /^\d+$/ and CloseBasket($basketno, $loggedinuser);
>+        $basketno =~ /^\d+$/ and CloseBasket($basketno);
>         # if requested, create basket group, close it and attach the basket
>         if ($query->param('createbasketgroup')) {
>             my $branchcode;
>@@ -209,8 +209,7 @@ if ( $op eq 'delete_confirm' ) {
>                             closed => 1,
>                             });
>             ModBasket( { basketno => $basketno,
>-                         basketgroupid => $basketgroupid,
>-                         borrowernumber => $loggedinuser } );
>+                         basketgroupid => $basketgroupid } );
>             print $query->redirect('/cgi-bin/koha/acqui/basketgroup.pl?booksellerid='.$booksellerid.'&closed=1');
>         } else {
>             print $query->redirect('/cgi-bin/koha/acqui/booksellers.pl?booksellerid=' . $booksellerid);
>@@ -243,8 +242,7 @@ elsif ( $op eq 'ediorder' ) {
>     $branch = undef if(defined $branch and $branch eq '');
>     ModBasket({
>         basketno => $basket->{basketno},
>-        branch   => $branch,
>-        borrowernumber => $loggedinuser
>+        branch   => $branch
>     });
>     print $query->redirect("/cgi-bin/koha/acqui/basket.pl?basketno=$basketno");
>     exit;
>@@ -538,7 +536,7 @@ sub edi_close_and_order {
>             if ( create_edi_order($edi_params) ) {
>                 #$template->param( edifile => 1 );
>             }
>-        CloseBasket($basketno, $loggedinuser);
>+        CloseBasket($basketno);
> 
>         # if requested, create basket group, close it and attach the basket
>         if ( $query->param('createbasketgroup') ) {
>@@ -560,8 +558,7 @@ sub edi_close_and_order {
>             ModBasket(
>                 {
>                     basketno       => $basketno,
>-                    basketgroupid  => $basketgroupid,
>-                    borrowernumber => $loggedinuser
>+                    basketgroupid  => $basketgroupid
>                 }
>             );
>             print $query->redirect(
>diff --git a/acqui/basketheader.pl b/acqui/basketheader.pl
>index ee4cf647b8..0eb479ff20 100755
>--- a/acqui/basketheader.pl
>+++ b/acqui/basketheader.pl
>@@ -154,8 +154,7 @@ if ( $op eq 'add_form' ) {
>             scalar $input->param('deliveryplace'),
>             scalar $input->param('billingplace'),
>             scalar $input->param('is_standing') ? 1 : undef,
>-            scalar $input->param('create_items'),
>-            $loggedinuser
>+            scalar $input->param('create_items')
>         );
>     } else { #New basket
>         $basketno = NewBasket(
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 23971: 95285 | 95286 | 95885 | 95888 | 96063 | 96064 | 100239 | 100240 | 100241 | 103911 | 103912 | 103913 | 103945 | 108962 | 111300 | 111301 | 111302 | 111303 | 111304 | 111955 | 111956 | 111957 | 111958 | 111959 | 111960 | 112277 | 112278 | 112279 | 112280 | 112281 | 112282 | 112283 | 112284 | 112285 | 112286 | 112287 | 112288 | 112289 | 112290 | 112291 | 112298 | 112299 | 112300 | 112301 | 112302 | 112303 | 112304 | 112305 | 112306 | 112307 | 113384 | 113385 | 113386 | 113387 | 113388 | 113389 | 113390 | 113391 | 113392 | 113393 | 114857 | 114858 | 114859 | 114860 | 114861 | 114862 | 114863 | 114864 | 114865 | 114866 | 114867 | 114905 | 114906 | 114907 | 114908 | 114909 | 114910 | 114911 | 114912 | 114913 | 114914 | 114915 | 114916 | 115976 | 115977 | 115978 | 115979 | 115980 | 115981 | 115982 | 115983 | 115984 | 115985 | 115986 | 119401 | 119402 | 119403 | 119404 | 119405 | 119406 | 119407 | 119408 | 119409 | 119410 | 119411 | 119412 | 119944 | 119946 | 120048