Attachment 59711 Details for Bug 17855 – Bug 17855 - Follow up: code clean up

[patch] Bug 17855 - Follow up: code clean up

Bug-17855---Follow-up-code-clean-up.patch (text/plain), 9.50 KB, created by Mark Tompsett on 2017-02-01 02:03:18 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Mark Tompsett

Created: 2017-02-01 02:03:18 UTC

Size: 9.50 KB

patch

obsolete

>From 194d8e0567bfff142d1808dbc0e8094bef0d9609 Mon Sep 17 00:00:00 2001
>From: Alex Buckley <alexbuckley@catalyst.net.nz>
>Date: Mon, 30 Jan 2017 11:09:39 +1300
>Subject: [PATCH] Bug 17855 - Follow up: code clean up
>
>Removed unnecessary use pragmas in the onboarding.pl script
>
>Replaced all sql code in onboarding.pl script with DBIx
>
>Changed the logic of the onshelfholds if statement in onboarding.pl
>
>Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
>
>NOTE: Initially came from 17955
>Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
>---
> installer/onboarding.pl | 138 ++++++++++++------------------------------------
> 1 file changed, 33 insertions(+), 105 deletions(-)
>
>diff --git a/installer/onboarding.pl b/installer/onboarding.pl
>index 1239ac8..6773f8f 100755
>--- a/installer/onboarding.pl
>+++ b/installer/onboarding.pl
>@@ -22,38 +22,29 @@ use Modern::Perl;
> use diagnostics;
> use C4::InstallAuth;
> use CGI qw ( -utf8 );
>-use List::MoreUtils qw/uniq/;
> use Digest::MD5 qw(md5_base64);
> use Encode qw( encode );
> use C4::Koha;
>-use C4::Context;
> use C4::Output;
>-use C4::Form::MessagingPreferences;
> use C4::Members;
> use C4::Members::Attributes;
> use C4::Members::AttributeTypes;
> use C4::Log;
>-use C4::Letters;
>-use C4::Form::MessagingPreferences;
> use Koha::AuthorisedValues;
> use Koha::Patron::Debarments;
>-use Koha::Cities;
> use Koha::Patrons;
> use Koha::Items;
> use Koha::Libraries;
>-use Koha::LibraryCategories;
> use Koha::Database;
> use Koha::DateUtils;
> use Koha::Patron::Categories;
> use Koha::Patron::Category;
> use Koha::Patron::HouseboundRole;
> use Koha::Patron::HouseboundRoles;
>-use Koha::Token;
>-use Email::Valid;
>-use Module::Load;
> use Koha::ItemTypes;
> use Koha::IssuingRule;
> use Koha::IssuingRules;
>+use Data::Dumper;
> 
> #Setting variables
> my $input = new CGI;
>@@ -92,9 +83,11 @@ my $dbh = DBI->connect(
> );
> 
> #Store the value of the template input name='op' in the variable $op so we can check if the user has pressed the button with the name="op" and value="finish" meaning the user has finished the onboarding tool.
>-my $op = $input->param('op') || "";
>+my $op = $input->param('op') || '';
> $template->param( 'op' => $op );
> 
>+my $schema = Koha::Database->new()->schema();
>+
> if ( $op && $op eq 'finish' )
> { #If the value of $op equals 'finish' then redirect user to /cgi-bin/koha/mainpage.pl
>     print $input->redirect("/cgi-bin/koha/mainpage.pl");
>@@ -242,8 +235,8 @@ elsif ( $step && $step == 2 ) {
>     #Create a patron
> }
> elsif ( $step && $step == 3 ) {
>-    my $firstpassword  = $input->param('password') || "";
>-    my $secondpassword = $input->param('password2') || "";
>+    my $firstpassword  = $input->param('password') || '';
>+    my $secondpassword = $input->param('password2') || '';
> 
> 
>     #Find all patron records in the database and hand them to the template
>@@ -281,13 +274,11 @@ elsif ( $step && $step == 3 ) {
>     my $categories = Koha::Patron::Categories->search();
>     $template->param( 'categories' => $categories, );
> 
>+#Incrementing the highest existing patron cardnumber to prevent duplicate cardnumber entry
> 
>+    my $existing_cardnumber = $schema->resultset('Borrower')->get_column('cardnumber')->max();
> 
>-
>-#Incrementing the highest existing patron cardnumber to prevent duplicate cardnumber entry
>-    my $exisiting_cardnumber = my $sth_search =
>-      $dbh->prepare("SELECT MAX(cardnumber) FROM borrowers");
>-    my $new_cardnumber = $exisiting_cardnumber + 1;
>+    my $new_cardnumber = $existing_cardnumber + 1;
>     $template->param( "newcardnumber" => $new_cardnumber );
> 
>     my $op = $input->param('op') // 'list';
>@@ -386,7 +377,7 @@ elsif ( $step && $step == 3 ) {
>             $newdata{address}        = "";
>             $newdata{city}           = "";
> 
>-#Determine the dateexpiry of the patron based on the patron category it is created from
>+#Hand tne the dateexpiry of the patron based on the patron category it is created from
>             my $patron_category = Koha::Patron::Categories->find( $newdata{categorycode} );
>             $newdata{dateexpiry} = $patron_category->get_expiry_date( $newdata{dateenrolled} );
> 
>@@ -415,20 +406,13 @@ elsif ( $step && $step == 3 ) {
>                 }
> 
>                 # construct flags
>-                my $module_flags = 0;
>-                my $sth =
>-                  $dbh->prepare("SELECT bit,flag FROM userflags ORDER BY bit");
>-                $sth->execute();
>-                while ( my ( $bit, $flag ) = $sth->fetchrow_array ) {
>-                    if ( exists $all_module_perms{$flag} ) {
>-                        $module_flags += 2**$bit;
>-                    }
>-                }
>+                my @userflags = $schema->resultset('Userflag')->search({},{
>+                        order_by => { -asc =>'bit'},
>+                        }
>+                );
> 
>-#Set the superlibrarian permission of the newly created patron to superlibrarian
>-                $sth = $dbh->prepare(
>-                    "UPDATE borrowers SET flags=? WHERE borrowernumber=?");
>-                $sth->execute( $module_flags, $borrowernumber );
>+#Setting superlibrarian permissions for new patron
>+                my $flags = Koha::Patrons->find($borrowernumber)->set({flags=>1})->store;
> 
>                 #Error handling checking if the patron was created successfully
>                 if ( !$borrowernumber ) {
>@@ -581,89 +565,33 @@ elsif ( $step && $step == 5 ) {
> 
>         my @messages;
> 
>-#New code from smart-rules.tt starts here. Needs to be added to library
> #Allows for the 'All' option to work when selecting all libraries for a circulation rule to apply to.
>         if ( $branch eq "*" ) {
>-            my $sth_search = $dbh->prepare(
>-                "SELECT count(*) AS total
>-                                        FROM default_circ_rules"
>-            );
>-            my $sth_insert = $dbh->prepare(
>-                "INSERT INTO default_circ_rules
>-                                        (maxissueqty, onshelfholds)
>-                                        VALUES (?, ?)"
>-            );
>-            my $sth_update = $dbh->prepare(
>-                "UPDATE default_circ_rules
>-                                        SET maxissueqty = ?, onshelfholds = ?"
>-            );
>-
>-            $sth_search->execute();
>-            my $res = $sth_search->fetchrow_hashref();
>-            if ( $res->{total} ) {
>-                $sth_update->execute( $maxissueqty, $onshelfholds );
>-            }
>-            else {
>-                $sth_insert->execute( $maxissueqty, $onshelfholds );
>-            }
>+            my $search_default_rules = $schema->resultset('DefaultCircRule')->count();
>+            my $insert_default_rules = $schema->resultset('Issuingrule')->new(
>+                    { maxissueqty => $maxissueqty, onshelfholds => $onshelfholds }
>+                );
>         }
>-
> #Allows for the 'All' option to work when selecting all patron categories for a circulation rule to apply to.
>-        if ( $bor eq "*" ) {
>-            my $sth_search = $dbh->prepare(
>-                "SELECT count(*) AS total
>-                                            FROM default_circ_rules"
>-            );
>-            my $sth_insert = $dbh->prepare(
>-                q|
>-                INSERT INTO default_circ_rules
>-                    (maxissueqty)
>-                    VALUES (?)
>-            |
>-            );
>-            my $sth_update = $dbh->prepare(
>-                q|
>-                UPDATE default_circ_rules
>-                SET maxissueqty = ?
>-            |
>-            );
>+        elsif ( $bor eq "*" ) {
> 
>-            $sth_search->execute();
>-            my $res = $sth_search->fetchrow_hashref();
>-            if ( $res->{total} ) {
>-                $sth_update->execute($maxissueqty);
>-            }
>-            else {
>-                $sth_insert->execute($maxissueqty);
>-            }
>+            my $search_default_rules = $schema->resultset('DefaultCircRule')->count();
>+            my $insert_default_rules = $schema->resultset('Issuingrule')->new(
>+                        { maxissueqty => $maxissueqty}
>+            );
>         }
> 
> #Allows for the 'All' option to work when selecting all itemtypes for a circulation rule to apply to
>-        if ( $itemtype eq "*" ) {
>-            my $sth_search = $dbh->prepare(
>-                "SELECT count(*) AS total
>-                                        FROM default_branch_circ_rules
>-                                        WHERE branchcode = ?"
>-            );
>-            my $sth_insert = $dbh->prepare(
>-                "INSERT INTO default_branch_circ_rules
>-                                        (branchcode, onshelfholds)
>-                                        VALUES (?, ?)"
>-            );
>-            my $sth_update = $dbh->prepare(
>-                "UPDATE default_branch_circ_rules
>-                                        SET onshelfholds = ?
>-                                        WHERE branchcode = ?"
>+        elsif ( $itemtype eq "*" ) {
>+            my $search_default_rules = $schema->resultset('DefaultCircRule')->search({},{
>+                    branchcode => $branch
>+                    }
>+
>             );
>-            $sth_search->execute($branch);
>-            my $res = $sth_search->fetchrow_hashref();
> 
>-            if ( $res->{total} ) {
>-                $sth_update->execute( $onshelfholds, $branch );
>-            }
>-            else {
>-                $sth_insert->execute( $branch, $onshelfholds );
>-            }
>+            my $insert_default_rules = $schema->resultset('Issuingrule')->new(
>+                           { branchcode => $branch, onshelfholds => $onshelfholds }
>+            );
>         }
> 
>         my $issuingrule = Koha::IssuingRules->find(
>-- 
>2.7.4

Actions: View | Diff | Splinter Review

Attachments on bug 17855: 58657 | 58658 | 58659 | 58660 | 58661 | 58662 | 58663 | 58664 | 58665 | 58666 | 58667 | 58668 | 58669 | 58781 | 58782 | 58783 | 58784 | 58785 | 58792 | 58793 | 58794 | 58846 | 58860 | 58861 | 58862 | 58886 | 58892 | 58893 | 58894 | 58895 | 58896 | 58897 | 58898 | 58899 | 58900 | 58901 | 58902 | 58903 | 58904 | 58905 | 58906 | 58907 | 59037 | 59051 | 59140 | 59141 | 59174 | 59188 | 59277 | 59279 | 59280 | 59281 | 59628 | 59703 | 59705 | 59706 | 59707 | 59708 | 59709 | 59710 | 59711 | 59712 | 59742 | 59826 | 59830 | 59840 | 59841 | 59842 | 59856 | 59857 | 60092 | 61667 | 61668 | 61986 | 62186 | 62191 | 62192 | 62193 | 62220 | 62221 | 62222 | 62223 | 62231 | 62233 | 62234 | 62235 | 62236 | 62237 | 62238 | 62239 | 62240 | 62241 | 62242 | 62243 | 62251 | 62252 | 62253 | 62254 | 62255 | 62256 | 62257 | 62258 | 62259 | 62260 | 62261 | 62262 | 62263 | 62281 | 62282 | 62283 | 62284 | 62285 | 62286 | 62287 | 62288 | 62668 | 62669 | 62670 | 62671 | 62672 | 62673 | 62674 | 62675