Attachment 113374 Details for Bug 23916 – Bug 23916: (follow-up) Fix terminology, use patron-title and make column consistent in issues and old_issues

[patch] Bug 23916: (follow-up) Fix terminology, use patron-title and make column consistent in issues and old_issues

Bug-23916-follow-up-Fix-terminology-use-patron-tit.patch (text/plain), 12.54 KB, created by Tomás Cohen Arazi (tcohen) on 2020-11-10 12:00:08 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2020-11-10 12:00:08 UTC

Size: 12.54 KB

patch

obsolete

>From 8f6be217a221dd0c38d80d92b07c2aec17769060 Mon Sep 17 00:00:00 2001
>From: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
>Date: Mon, 12 Oct 2020 10:28:33 +0100
>Subject: [PATCH] Bug 23916: (follow-up) Fix terminology, use patron-title and
> make column consistent in issues and old_issues
>
>This commit makes the changes suggested by Katrin in comment #50:
>
>- Changed syspref from RecordIssuer to RecordStaffUserOnCheckout
>- Changed terminology from "issue" to "check out" and variations
>- Fixed name display to use patron-title.inc
>- Made issuer column DEFAULT NULL consistently between issues and
>old_issues and between the DB update and kohastructure.sql
>
>Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>---
> C4/Circulation.pm                                    |  2 +-
> .../bug_23916_add_RecordIssuer_syspref.perl          |  4 ++--
> .../atomicupdate/bug_23916_add_issues_issuer.perl    |  4 ++--
> installer/data/mysql/kohastructure.sql               |  2 +-
> installer/data/mysql/mandatory/sysprefs.sql          |  2 +-
> .../en/modules/admin/preferences/circulation.pref    |  6 +++---
> .../prog/en/modules/catalogue/issuehistory.tt        |  6 +++---
> .../prog/en/modules/members/readingrec.tt            |  4 ++--
> t/db_dependent/Circulation.t                         | 12 ++++++------
> 9 files changed, 21 insertions(+), 21 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index 0dac77f95c2..925c6f7b527 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -1523,7 +1523,7 @@ sub AddIssue {
>             # Get ID of logged in user.  if called from a batch job,
>             # no user session exists and C4::Context->userenv() returns
>             # the scalar '0'. Only do this is the syspref says so
>-            if ( C4::Context->preference('RecordIssuer') ) {
>+            if ( C4::Context->preference('RecordStaffUserOnCheckout') ) {
>                 my $userenv = C4::Context->userenv();
>                 my $usernumber = (ref($userenv) eq 'HASH') ? $userenv->{'number'} : undef;
>                 if ($usernumber) {
>diff --git a/installer/data/mysql/atomicupdate/bug_23916_add_RecordIssuer_syspref.perl b/installer/data/mysql/atomicupdate/bug_23916_add_RecordIssuer_syspref.perl
>index ad3dca7b186..aa1dc6960e1 100644
>--- a/installer/data/mysql/atomicupdate/bug_23916_add_RecordIssuer_syspref.perl
>+++ b/installer/data/mysql/atomicupdate/bug_23916_add_RecordIssuer_syspref.perl
>@@ -1,8 +1,8 @@
> $DBversion = 'XXX'; # will be replaced by the RM
> if( CheckVersion( $DBversion ) ) {
> 
>-    $dbh->do( q| INSERT IGNORE INTO systempreferences (variable, value, explanation, options, type) VALUES ('RecordIssuer', '0', 'If enabled, when an item is issued, the user who issued the item is recorded', '', 'YesNo'); | );
>+    $dbh->do( q| INSERT IGNORE INTO systempreferences (variable, value, explanation, options, type) VALUES ('RecordStaffUserOnCheckout', '0', 'If enabled, when an item is checked out, the user who checked out the item is recorded', '', 'YesNo'); | );
> 
>     SetVersion( $DBversion );
>-    print "Upgrade to $DBversion done (Bug 23916 - Add RecordIssuer syspref)\n";
>+    print "Upgrade to $DBversion done (Bug 23916 - Add RecordStaffUserOnCheckout syspref)\n";
> }
>diff --git a/installer/data/mysql/atomicupdate/bug_23916_add_issues_issuer.perl b/installer/data/mysql/atomicupdate/bug_23916_add_issues_issuer.perl
>index 589d3a91a89..08ede0aa745 100644
>--- a/installer/data/mysql/atomicupdate/bug_23916_add_issues_issuer.perl
>+++ b/installer/data/mysql/atomicupdate/bug_23916_add_issues_issuer.perl
>@@ -1,13 +1,13 @@
> $DBversion = 'XXX'; # will be replaced by the RM
> if( CheckVersion( $DBversion ) ) {
>     if( !column_exists( 'issues', 'issuer' ) ) {
>-        $dbh->do( q| ALTER TABLE issues ADD issuer INT(11) AFTER borrowernumber | );
>+        $dbh->do( q| ALTER TABLE issues ADD issuer INT(11) DEFAULT NULL AFTER borrowernumber | );
>     }
>     if (!foreign_key_exists( 'issues', 'issues_ibfk_borrowers_borrowernumber' )) {
>         $dbh->do( q| ALTER TABLE issues ADD CONSTRAINT `issues_ibfk_borrowers_borrowernumber` FOREIGN KEY (`issuer`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE SET NULL ON UPDATE CASCADE | );
>     }
>     if( !column_exists( 'old_issues', 'issuer' ) ) {
>-        $dbh->do( q| ALTER TABLE old_issues ADD issuer INT(11) AFTER borrowernumber | );
>+        $dbh->do( q| ALTER TABLE old_issues ADD issuer INT(11) DEFAULT NULL AFTER borrowernumber | );
>     }
>     if (!foreign_key_exists( 'old_issues', 'old_issues_ibfk_borrowers_borrowernumber' )) {
>         $dbh->do( q| ALTER TABLE old_issues ADD CONSTRAINT `old_issues_ibfk_borrowers_borrowernumber` FOREIGN KEY (`issuer`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE SET NULL ON UPDATE CASCADE | );
>diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql
>index 02c701724d5..6087b42159a 100644
>--- a/installer/data/mysql/kohastructure.sql
>+++ b/installer/data/mysql/kohastructure.sql
>@@ -1618,7 +1618,7 @@ DROP TABLE IF EXISTS `issues`;
> CREATE TABLE `issues` ( -- information related to check outs or issues
>   `issue_id` int(11) NOT NULL AUTO_INCREMENT, -- primary key for issues table
>   `borrowernumber` int(11), -- foreign key, linking this to the borrowers table for the patron this item was checked out to
>-  `issuer` int(11), -- foreign key, linking this to the borrowers table for the user who checked out this item
>+  `issuer` int(11) default NULL, -- foreign key, linking this to the borrowers table for the user who checked out this item
>   `itemnumber` int(11), -- foreign key, linking this to the items table for the item that was checked out
>   `date_due` datetime default NULL, -- datetime the item is due (yyyy-mm-dd hh:mm::ss)
>   `branchcode` varchar(10) default NULL, -- foreign key, linking to the branches table for the location the item was checked out
>diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql
>index 6e35b03a1c7..234c68da33e 100644
>--- a/installer/data/mysql/mandatory/sysprefs.sql
>+++ b/installer/data/mysql/mandatory/sysprefs.sql
>@@ -536,7 +536,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `
> ('QueryWeightFields','1',NULL,'If ON, enables field weighting','YesNo'),
> ('QuoteOfTheDay','','intranet,opac','Enable or disable display of Quote of the Day on the OPAC and staff interface home page','multiple'),
> ('RandomizeHoldsQueueWeight','0',NULL,'if ON, the holds queue in circulation will be randomized, either based on all location codes, or by the location codes specified in StaticHoldsQueueWeight','YesNo'),
>-('RecordIssuer','0',NULL,'If ON, when an item is issued, the user who issued the item is recorded','YesNo'),
>+('RecordStaffUserOnCheckout','0',NULL,'If ON, when an item is checked out, the user who checked out the item is recorded','YesNo'),
> ('RecordLocalUseOnReturn','0',NULL,'If ON, statistically record returns of unissued items as local use, instead of return','YesNo'),
> ('RefundLostOnReturnControl','CheckinLibrary','CheckinLibrary|ItemHomeBranch|ItemHoldingBranch','If a lost item is returned, choose which branch to pick rules for refunding.','Choice'),
> ('RenewAccruingItemWhenPaid','0','','If enabled, when the fines on an item accruing is paid off, attempt to renew that item. If the syspref "RenewalPeriodBase" is set to "due date", renewed items may still be overdue','YesNo'),
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
>index 24eea974e5c..9e620b04661 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
>@@ -446,12 +446,12 @@ Circulation:
>                   nothing : "do nothing"
>             - .
>         -
>-            - "When issuing an item, "
>-            - pref: RecordIssuer
>+            - "When checking out an item, "
>+            - pref: RecordStaffUserOnCheckout
>               choices:
>                   yes: "record"
>                   no: "don't record"
>-            - "the user who issued the item."
>+            - "the user who checked out the item."
>         -
>             - "Mark items as returned when flagged as lost "
>             - pref: MarkLostItemsAsReturned
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/issuehistory.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/issuehistory.tt
>index a79feb04de2..796c5e62e63 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/issuehistory.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/issuehistory.tt
>@@ -37,7 +37,7 @@
>             [% END %]
>             <th>Barcode</th>
>             <th>Checked out from</th>
>-            [% IF Koha.Preference('RecordIssuer') %]
>+            [% IF Koha.Preference('RecordStaffUserOnCheckout') %]
>             <th>Checked out by</th>
>             [% END %]
>             <th>Renewed</th>
>@@ -66,10 +66,10 @@
>                     [% ELSE %]
>                         &nbsp;
>                     [% END %]</td>
>-                [% IF Koha.Preference('RecordIssuer') %]
>+                [% IF Koha.Preference('RecordStaffUserOnCheckout') %]
>                 <td>[% IF checkout.issuer %]
>                     <a href="/cgi-bin/koha/circ/circulation.pl?borrowernumber=[% checkout.issuer | uri %]">
>-                    [% checkout.issued_by.firstname | html %] [% checkout.issued_by.surname | html %]
>+                    [% INCLUDE 'patron-title.inc' patron=checkout.issued_by %]
>                     </a>
>                     [% END %]</td>
>                 [% END %]
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt
>index 59e84be4d0e..019e3b78cb9 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt
>@@ -58,7 +58,7 @@
>         <th>Number of renewals</th>
>         <th class="title-string">Checked out on</th>
>         <th>Checked out from</th>
>-        [% IF Koha.Preference('RecordIssuer') %]
>+        [% IF Koha.Preference('RecordStaffUserOnCheckout') %]
>         <th>Checked out by</th>
>         [% END %]
>         <th class="title-string">Date due</th>
>@@ -97,7 +97,7 @@
>             <span title="[% issue.issuedate | html %]">[% issue.issuedate |$KohaDates  with_hours => 1 %]</span>
>           </td>
>           <td>[% Branches.GetName( issue.branchcode ) | html %]</td>
>-          [% IF Koha.Preference('RecordIssuer') %]
>+          [% IF Koha.Preference('RecordStaffUserOnCheckout') %]
>           <td><a href="/cgi-bin/koha/circ/circulation.pl?borrowernumber=[% issue.issuer | uri %]">[% issue.firstname | html %] [% issue.surname | html %]</a></td>
>           [% END %]
>           <td>
>diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t
>index c1d4ba1457d..104ed7cb7c2 100755
>--- a/t/db_dependent/Circulation.t
>+++ b/t/db_dependent/Circulation.t
>@@ -122,8 +122,8 @@ for my $branch ( $branches->next ) {
> $dbh->do('DELETE FROM issues');
> $dbh->do('DELETE FROM borrowers');
> 
>-# Disable recording of issuer until we're ready for it
>-t::lib::Mocks::mock_preference('RecordIssuer', 0);
>+# Disable recording of the staff who checked out an item until we're ready for it
>+t::lib::Mocks::mock_preference('RecordStaffUserOnCheckout', 0);
> 
> my $module = new Test::MockModule('C4::Context');
> 
>@@ -4605,7 +4605,7 @@ subtest 'Checkout should correctly terminate a transfer' => sub {
>     is( $hold->priority, 1, );
> }
> 
>-subtest 'AddIssue records issuer if appropriate' => sub  {
>+subtest 'AddIssue records staff who checked out item if appropriate' => sub  {
>     plan tests => 2;
> 
>     $module->mock( 'userenv', sub { { branch => $library->{id} } } );
>@@ -4638,14 +4638,14 @@ subtest 'AddIssue records issuer if appropriate' => sub  {
>     my $issue =
>       AddIssue( $patron->unblessed, $item->barcode, $dt_to, undef, $dt_from );
> 
>-    is( $issue->issuer, undef, "Issuer not recorded when RecordIssuer turned off" );
>+    is( $issue->issuer, undef, "Staff who checked out the item not recorded when RecordStaffUserOnCheckout turned off" );
> 
>-    t::lib::Mocks::mock_preference('RecordIssuer', 1);
>+    t::lib::Mocks::mock_preference('RecordStaffUserOnCheckout', 1);
> 
>     my $issue2 =
>       AddIssue( $patron->unblessed, $item->barcode, $dt_to, undef, $dt_from );
> 
>-    is( $issue->issuer, $issuer->{borrowernumber}, "Issuer recorded when RecordIssuer turned on" );
>+    is( $issue->issuer, $issuer->{borrowernumber}, "Staff who checked out the item recorded when RecordStaffUserOnCheckout turned on" );
> };
> 
> $schema->storage->txn_rollback;
>-- 
>2.29.2

Actions: View | Diff | Splinter Review

Attachments on bug 23916: 94914 | 94915 | 94916 | 94917 | 94918 | 95503 | 95504 | 95505 | 95506 | 95507 | 95508 | 95509 | 97719 | 97720 | 97721 | 97722 | 97723 | 97724 | 98586 | 98587 | 98588 | 98589 | 98590 | 98591 | 98594 | 99886 | 99887 | 99888 | 99889 | 99890 | 99891 | 110649 | 110650 | 110651 | 110652 | 110653 | 110654 | 110655 | 111485 | 111486 | 111487 | 111488 | 111489 | 111490 | 111491 | 111492 | 111966 | 111967 | 111968 | 111969 | 111970 | 111971 | 111972 | 111973 | 111974 | 112248 | 112249 | 112250 | 112251 | 112252 | 112253 | 112254 | 112255 | 112256 | 112257 | 112363 | 112364 | 112365 | 112366 | 112367 | 112368 | 112369 | 112370 | 112371 | 112372 | 112373 | 113035 | 113036 | 113037 | 113038 | 113039 | 113040 | 113041 | 113042 | 113043 | 113044 | 113045 | 113316 | 113317 | 113318 | 113319 | 113320 | 113321 | 113322 | 113323 | 113324 | 113325 | 113326 | 113327 | 113367 | 113368 | 113369 | 113370 | 113371 | 113372 | 113373 | 113374 | 113375 | 113376 | 113377 | 113378 | 113379 | 113380 | 113381 | 113382 | 113396