From 7061a8a19030ab46ef64aa0444ae67cedf2a39cb Mon Sep 17 00:00:00 2001
From: David Bourgault <david.bourgault@inlibro.com>
Date: Tue, 10 Oct 2017 09:15:07 -0400
Subject: [PATCH] Bug 17015 - Fix QA tools criticism

Adds POD to Koha/DiscreteCalendar.pm
Fixes spelling errors/bad pratices in various files.

QA tools now passes all green with the exception of Koha::Schema::Result::DiscreteCalendar. I'm not sure what to do here, this file did not exist before, and is (as far as I understand) generated automatically. Any ideas?

All files including Koha::Schema::Result::DiscreteCalendar pass QA test
tool for me

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
---
 Koha/DiscreteCalendar.pm                           | 285 +++++++++++++++++++--
 .../prog/en/modules/tools/discrete_calendar.tt     |   2 +-
 t/db_dependent/DiscreteCalendar.t                  |   6 +-
 tools/discrete_calendar.pl                         |   4 +-
 4 files changed, 276 insertions(+), 21 deletions(-)

diff --git a/Koha/DiscreteCalendar.pm b/Koha/DiscreteCalendar.pm
index 49becee..8718137 100644
--- a/Koha/DiscreteCalendar.pm
+++ b/Koha/DiscreteCalendar.pm
@@ -29,6 +29,38 @@ use C4::Output;
 use Koha::Database;
 use Koha::DateUtils;
 
+=head1 NAME
+
+Koha::DiscreteCalendar - Object containing a branches calendar, working with the SQL database
+
+=head1 SYNOPSIS
+
+  use Koha::DiscreteCalendar
+
+  my $c = Koha::DiscreteCalendar->new( branchcode => 'MAIN' );
+  my $dt = DateTime->now();
+
+  # are we open
+  $open = $c->is_holiday($dt);
+  # when will item be due if loan period = $dur (a DateTime::Duration object)
+  $duedate = $c->addDate($dt,$dur,'days');
+
+
+=head1 DESCRIPTION
+
+  Implements a new Calendar object, but uses the SQL database to keep track of days and holidays.
+  This results in a performance gain since the optimization is done by the MySQL database/team.
+
+=head1 METHODS
+
+=head2 new : Create a (discrete) calendar object
+
+my $calendar = Koha::DiscreteCalendar->new( branchcode => 'MAIN' );
+
+The option branchcode is required
+
+=cut
+
 sub new {
     my ( $classname, %options ) = @_;
     my $self = {};
@@ -67,7 +99,16 @@ sub _init {
 
 }
 
-# Returns an array of all the date objects of current branch.
+=head2 getDatesInfo
+
+  my @dates = $calendar->getDatesInfo();
+
+Returns an array of hashes representing the dates in this calendar. The hash
+contains the fields C<$date>, C<$outputdate>, C<$holidaytype>, C<$openhour>,
+C<$closehour> and C<$note>.
+
+=cut
+
 sub getDatesInfo {
     my $self = shift;
     my $branchcode = $self->{branchcode};
@@ -101,9 +142,19 @@ sub getDatesInfo {
     return @datesInfos;
 }
 
-# This methode will copy everything from a given branch to a new branch
+=head2 add_new_branch
+
+    Koha::DiscreteCalendar->add_new_branch($copyBranch, $newBranch)
+
+This methode will copy everything from a given branch to a new branch
+C<$copyBranch> is the branch to copy from
+C<$newBranch> is the branch to be created, and copy into
+
+=cut
+
 sub add_new_branch {
-    my ($self, $copyBranch, $newBranch) = @_;
+    my ( $classname, $copyBranch, $newBranch) = @_;
+
     $copyBranch = 'DFLT' unless $copyBranch;
     my $schema = Koha::Database->new->schema;
 
@@ -124,7 +175,16 @@ sub add_new_branch {
 
 }
 
-# DiscreteCalendar data transfer object (DTO)
+=head2 get_date_info
+
+    my $date = $calendar->get_date_info;
+
+Returns a reference-to-hash representing a DiscreteCalendar date data object.
+The hash contains the fields C<$date>, C<$outputdate>, C<$holidaytype>,
+C<$openhour>, C<$closehour> and C<$note>.
+
+=cut
+
 sub get_date_info {
     my ($self, $date) = @_;
     my $branchcode = $self->{branchcode};
@@ -159,7 +219,14 @@ sub get_date_info {
     return $dateDTO;
 }
 
-# Returns the furthest date available in the databse of current branch.
+=head2 getMaxDate
+
+    my $maxDate = $calendar->getMaxDate();
+
+Returns the furthest date available in the databse of current branch.
+
+=cut
+
 sub getMaxDate {
     my $self       = shift;
     my $branchcode     = $self->{branchcode};
@@ -178,7 +245,14 @@ sub getMaxDate {
     return $rs->next()->get_column('max');
 }
 
-# Returns the onldest date available in the databse of current branch.
+=head2 getMinDate
+
+    my $minDate = $calendar->getMinDate();
+
+Returns the oldest date available in the databse of current branch.
+
+=cut
+
 sub getMinDate {
     my $self       = shift;
     my $branchcode     = $self->{branchcode};
@@ -197,7 +271,14 @@ sub getMinDate {
     return $rs->next()->get_column('min');
 }
 
-# Returns an array of all the date objects that are unique holidays.
+=head2 get_unique_holidays
+
+  my @unique_holidays = $calendar->get_unique_holidays();
+
+Returns an array of all the date objects that are unique holidays.
+
+=cut
+
 sub get_unique_holidays {
     my $self = shift;
     my $branchcode = $self->{branchcode};
@@ -227,7 +308,14 @@ sub get_unique_holidays {
     return @unique_holidays;
 }
 
-# Returns an array of all the date objects  that are float holidays.
+=head2 get_float_holidays
+
+  my @float_holidays = $calendar->get_float_holidays();
+
+Returns an array of all the date objects that are float holidays.
+
+=cut
+
 sub get_float_holidays {
     my $self = shift;
     my $branchcode = $self->{branchcode};
@@ -257,8 +345,15 @@ sub get_float_holidays {
     return @float_holidays;
 }
 
-# Returns an array of all the date objects  that are need validation holidays.
-sub get_need_valdation_holidays {
+=head2 get_need_validation_holidays
+
+  my @need_validation_holidays = $calendar->get_need_validation_holidays();
+
+Returns an array of all the date objects that are float holidays in need of validation.
+
+=cut
+
+sub get_need_validation_holidays {
     my $self = shift;
     my $branchcode = $self->{branchcode};
     my @need_validation_holidays;
@@ -287,7 +382,14 @@ sub get_need_valdation_holidays {
     return @need_validation_holidays;
 }
 
-# Returns an array of all the date objects  that are repeatable holidays.
+=head2 get_repeatable_holidays
+
+  my @repeatable_holidays = $calendar->get_repeatable_holidays();
+
+Returns an array of all the date objects that are repeatable holidays.
+
+=cut
+
 sub get_repeatable_holidays {
     my $self = shift;
     my $branchcode = $self->{branchcode};
@@ -317,7 +419,14 @@ sub get_repeatable_holidays {
     return @repeatable_holidays;
 }
 
-# Returns an array of all the date objects  that are weekly holidays.
+=head2 get_week_days_holidays
+
+  my @week_days_holidays = $calendar->get_week_days_holidays;
+
+Returns an array of all the date objects that are weekly holidays.
+
+=cut
+
 sub get_week_days_holidays {
     my $self = shift;
     my $branchcode = $self->{branchcode};
@@ -346,7 +455,7 @@ sub get_week_days_holidays {
     return @week_days;
 }
 
-=head1 edit_holiday
+=head2 edit_holiday
 
 Modifies a date or a range of dates
 
@@ -484,6 +593,17 @@ sub edit_holiday {
     $schema->storage->txn_commit;
 }
 
+=head2 remove_weekly_holidays
+
+    $calendar->remove_weekly_holidays($weekday, $updateValues, $today);
+
+Removes a weekly holiday and updates the days' parameters
+C<$weekday> is the weekday to un-holiday
+C<$updatevalues> is hashref containing the new parameters
+C<$today> is today's date
+
+=cut
+
 sub remove_weekly_holidays {
     my ($self, $weekday, $updateValues, $today) = @_;
     my $branchcode = $self->{branchcode};
@@ -505,6 +625,18 @@ sub remove_weekly_holidays {
     }
 }
 
+=head2 remove_repeatable_holidays
+
+    $calendar->remove_repeatable_holidays($startDate, $endDate, $today);
+
+Removes a repeatable holiday and updates the days' parameters
+C<$startDatey> is the start date of the repeatable holiday
+C<$endDate> is the end date of the repeatble holiday
+C<$updatevalues> is hashref containing the new parameters
+C<$today> is today's date
+
+=cut
+
 sub remove_repeatable_holidays {
     my ($self, $startDate, $endDate, $updateValues, $today) = @_;
     my $branchcode = $self->{branchcode};
@@ -533,6 +665,17 @@ sub remove_repeatable_holidays {
     }
 }
 
+=head2 copyToBranch
+
+  $calendar->copyToBranch($branch2);
+
+Copies the days and holidays from this branch to $branch2, ignoring dates in C<$self>
+but not in C<$branch2>
+
+C<$branch2> the branch to copy into
+
+=cut
+
 sub copyToBranch {
     my ($self,$newBranch) =@_;
     my $branchcode = $self->{branchcode};
@@ -570,6 +713,14 @@ sub copyToBranch {
     }
 }
 
+=head2 isOpened
+
+    $calendar->isOpened($date)
+
+Returns whether the library is open on C<$date>
+
+=cut
+
 sub isOpened {
     my($self, $date) = @_;
     my $branchcode = $self->{branchcode};
@@ -591,6 +742,14 @@ sub isOpened {
     return $isOpened;
 }
 
+=head2 is_holiday
+
+    $calendar->is_holiday($date)
+
+Returns whether C<$date> is a holiday or not
+
+=cut
+
 sub is_holiday {
     my($self, $date) = @_;
     my $branchcode = $self->{branchcode};
@@ -616,6 +775,21 @@ sub is_holiday {
     return $isHoliday;
 }
 
+=head2 copyHoliday
+
+  $calendar->copyHoliday($from_startDate, $from_endDate, $to_startDate, $to_endDate, $daysnumber);
+
+Copies a holiday's parameters from a range to a new range
+C<$from_startDate> the source holiday's start date
+C<$from_endDate> the source holiday's end date
+C<$to_startDate> the destination holiday's start date
+C<$to_endDate> the destination holiday's end date
+C<$daysnumber> the number of days in the range.
+
+Both ranges should have the same number of days in them.
+
+=cut
+
 sub copyHoliday {
     my ($self, $from_startDate, $from_endDate, $to_startDate, $to_endDate, $daysnumber) = @_;
     my $branchcode = $self->{branchcode};
@@ -664,7 +838,7 @@ sub copyHoliday {
         my $endDate = dt_from_string($from_endDate);
         $to_startDate = $dtf->format_datetime($to_startDate);
         $to_endDate = $dtf->format_datetime($to_endDate);
-        if($daysnumber ==7){
+        if($daysnumber == 7){
             for (my $tempDate = $from_startDate->clone(); $tempDate <= $endDate;$tempDate->add(days => 1)){
                 my $formatedDate = $dtf->format_datetime($tempDate);
                 my $fromDate = $schema->resultset('DiscreteCalendar')->search(
@@ -740,6 +914,14 @@ sub copyHoliday {
     }
 }
 
+=head2 days_between
+
+   $cal->days_between( $start_date, $end_date )
+
+Calculates the number of days the library is opened between C<$start_date> and C<$end_date>
+
+=cut
+
 sub days_between {
     my ($self, $start_date, $end_date, ) = @_;
     my $branchcode = $self->{branchcode};
@@ -769,6 +951,14 @@ sub days_between {
     return DateTime::Duration->new( days => $days_between->count());
 }
 
+=head2 next_open_day
+
+   $open_date = $self->next_open_day($base_date);
+
+Returns a string representing the next day the library is open, starting from C<$base_date>
+
+=cut
+
 sub next_open_day {
     my ( $self, $date ) = @_;
     my $branchcode = $self->{branchcode};
@@ -790,6 +980,14 @@ sub next_open_day {
     return dt_from_string( $rs->next()->date(), 'iso');
 }
 
+=head2 prev_open_day
+
+   $open_date = $self->prev_open_day($base_date);
+
+Returns a string representing the closest previous day the library was open, starting from C<$base_date>
+
+=cut
+
 sub prev_open_day {
     my ( $self, $date ) = @_;
     my $branchcode = $self->{branchcode};
@@ -811,6 +1009,14 @@ sub prev_open_day {
     return dt_from_string( $rs->next()->date(), 'iso');
 }
 
+=head2 days_forward
+
+    $fwrd_date = $calendar->days_forward($start, $count)
+
+Returns the date C<$count> days in the future from C<$start>, ignoring days where the library is closed.
+
+=cut
+
 sub days_forward {
     my $self     = shift;
     my $start_dt = shift;
@@ -827,6 +1033,16 @@ sub days_forward {
     return $base_dt;
 }
 
+=head2 hours_between
+
+    $hours = $calendar->hours_between($start_dt, $end_dt)
+
+Returns the number of hours between C<$start_dt> and C<$end_dt>. This is the imprecise
+version, which simply calculates the number of day times 24. To take opening hours into account
+see C<open_hours_between>/
+
+=cut
+
 sub hours_between {
     my ($self, $start_dt, $end_dt) = @_;
     my $branchcode = $self->{branchcode};
@@ -862,6 +1078,15 @@ sub hours_between {
     return $duration;
 }
 
+=head2 open_hours_between
+
+  $hours = $calendar->open_hours_between($start_date, $end_date)
+
+Returns the number of hours between C<$start_date> and C<$end_date>, taking into
+account the opening hours of the library.
+
+=cut
+
 sub open_hours_between {
     my ($self, $start_date, $end_date) = @_;
     my $branchcode = $self->{branchcode};
@@ -919,6 +1144,17 @@ sub open_hours_between {
 
     return ($working_hours_between->next()->get_column('hours_between') - $not_used_hours->next()->get_column('not_used_hours'));
 }
+
+=head2 addDate
+
+  my $dt = $calendar->addDate($date, $dur, $unit)
+
+C<$date> is a DateTime object representing the starting date of the interval.
+C<$offset> is a duration to add to it (DateTime::Duration objects are supported as legacy)
+C<$unit> is a string value 'days' or 'hours' toflag granularity of duration
+
+=cut
+
 sub addDate {
     my ( $self, $startdate, $add_duration, $unit ) = @_;
 
@@ -943,6 +1179,15 @@ sub addDate {
     return $dt;
 }
 
+=head2 addHours
+
+  $end = $calendar->addHours($start, $hours_duration, $return_by_hour)
+
+Add C<$hours_duration> to C<$start> date.
+C<$return_by_hour> is an integer value representing the opening hour for the branch
+
+=cut
+
 sub addHours {
     my ( $self, $startdate, $hours_duration, $return_by_hour ) = @_;
     my $base_date = $startdate->clone();
@@ -968,6 +1213,16 @@ sub addHours {
     return $base_date;
 }
 
+=head2 addDays
+
+  $date = $calendar->addDays($start, $duration)
+
+Add C<$days_duration> to C<$start> date. If the calendar's days_mode is set
+to 'Calendar', it ignores closed days. Else if the calendar is set to 'Datedue'
+it calculates the date normally, and then pushes to result to the next open day.
+
+=cut
+
 sub addDays {
     my ( $self, $startdate, $days_duration ) = @_;
     my $base_date = $startdate->clone();
@@ -1013,4 +1268,4 @@ sub addDays {
     return $base_date;
 }
 
-1;
+1;
\ No newline at end of file
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/discrete_calendar.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/discrete_calendar.tt
index 81ccd10..d352068 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/discrete_calendar.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/discrete_calendar.tt
@@ -155,7 +155,7 @@
         }
 
         /**
-        * This function seperate a given date object a returns an array containing all needed information about the date.
+        * This function separate a given date object a returns an array containing all needed information about the date.
         */
         function getSeparetedDate(date){
             var mydate = new Array();
diff --git a/t/db_dependent/DiscreteCalendar.t b/t/db_dependent/DiscreteCalendar.t
index b320d9d..b155dcc 100644
--- a/t/db_dependent/DiscreteCalendar.t
+++ b/t/db_dependent/DiscreteCalendar.t
@@ -46,11 +46,11 @@ $schema->resultset('DiscreteCalendar')->search({
 });
 
 #Added entries for the branches.
-Koha::DiscreteCalendar->new( branchcode => 'DFLT' )->add_new_branch('DFLT', $branch1);
-Koha::DiscreteCalendar->new( branchcode => 'DFLT' )->add_new_branch('DFLT', $branch2);
+Koha::DiscreteCalendar->add_new_branch('DFLT', $branch1);
+Koha::DiscreteCalendar->add_new_branch('DFLT', $branch2);
 
 isnt($branch1,'', "First branch to do tests. BranchCode => $branch1");
-isnt($branch1,'', "Second branch  to do tests. BranchCode => $branch2");
+isnt($branch2,'', "Second branch to do tests. BranchCode => $branch2");
 
 #2 Calendars to make sure branches are treated separately.
 my $calendar = Koha::DiscreteCalendar->new(branchcode => $branch1);
diff --git a/tools/discrete_calendar.pl b/tools/discrete_calendar.pl
index 6a306da..0104c72 100755
--- a/tools/discrete_calendar.pl
+++ b/tools/discrete_calendar.pl
@@ -41,7 +41,7 @@ my ($template, $loggedinuser, $cookie)
 
 my $branch = $input->param('branch') || C4::Context->userenv->{'branch'};
 my $calendar = Koha::DiscreteCalendar->new(branchcode => $branch);
-#alert the user that he is using the default calendar because he does not have a library set
+#alert the user that they are using the default calendar because they do not have a library set
 my $no_branch_selected = $calendar->{no_branch_selected};
 
 my $weekday = $input->param('day_of_week');
@@ -120,7 +120,7 @@ my @unique_holidays =$calendar->get_unique_holidays();
 #discrete_calendar floating holidays
 my @float_holidays =$calendar->get_float_holidays();
 #discrete_caledar need validation holidays
-my @need_validation_holidays =$calendar->get_need_valdation_holidays();
+my @need_validation_holidays =$calendar->get_need_validation_holidays();
 
 #Calendar maximum date
 my $minDate = $calendar->getMinDate($branch);
-- 
2.1.4