Bugzilla – Attachment 67934 Details for
Bug 17015
New Koha Calendar
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 17015 - Fix QA tools criticism
Bug-17015---Fix-QA-tools-criticism.patch (text/plain), 16.53 KB, created by
Alex Buckley
on 2017-10-11 09:53:48 UTC
(
hide
)
Description:
Bug 17015 - Fix QA tools criticism
Filename:
MIME Type:
Creator:
Alex Buckley
Created:
2017-10-11 09:53:48 UTC
Size:
16.53 KB
patch
obsolete
>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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 17015
:
53859
|
54318
|
54419
|
59166
|
59167
|
59168
|
59169
|
59170
|
59171
|
59267
|
59268
|
59269
|
59270
|
59271
|
59463
|
59516
|
59517
|
59518
|
59519
|
59520
|
59561
|
59562
|
59586
|
59587
|
59888
|
59889
|
60902
|
60903
|
60904
|
60905
|
60906
|
60986
|
61737
|
62375
|
62376
|
62380
|
63257
|
63282
|
63362
|
63363
|
63364
|
63365
|
63366
|
63367
|
64235
|
65073
|
65074
|
65075
|
65076
|
65077
|
67721
|
67722
|
67723
|
67724
|
67725
|
67879
|
67929
|
67930
|
67931
|
67932
|
67933
|
67934
|
68392
|
68393
|
68394
|
68395
|
68396
|
68397
|
71634
|
71635
|
71636
|
71637
|
71638
|
72890
|
73145
|
74859
|
74860
|
74861
|
74862
|
74863
|
74864
|
74865
|
74866
|
75444
|
75479
|
76594
|
77249
|
77250
|
77607
|
77608
|
77609
|
77610
|
77611
|
77612
|
77613
|
77770
|
77771
|
77772
|
77773
|
77774
|
79035
|
80523
|
80524
|
80525
|
80526
|
80527
|
80528
|
80529
|
80530
|
80531
|
80532
|
80533
|
80534
|
80535
|
83547
|
85394
|
85677
|
85678
|
85679
|
85680
|
85681
|
85682
|
85683
|
85684
|
85685
|
85686
|
85687
|
85688
|
85689
|
85690
|
85691
|
92595
|
92596
|
92597
|
92598
|
100079
|
110383
|
110384
|
110386
|
110387
|
110388
|
110389
|
113541
|
113905
|
115501
|
115502
|
115503
|
115504
|
115505
|
115506
|
115507
|
115508
|
115509
|
115510
|
115511
|
118554
|
118555
|
118556
|
118557
|
118558
|
118559
|
119095
|
119097
|
119099
|
119100
|
119101
|
119102
|
131619
|
131620
|
131621
|
131622
|
131623
|
131624
|
131625
|
131626
|
131634
|
131635
|
131636
|
131637
|
131638
|
131639
|
131640
|
131641
|
131667
|
132199
|
133596
|
133597
|
133598
|
133599
|
133600
|
133601
|
133602
|
133603
|
133604
|
133605
|
133678
|
137219
|
137220
|
137221
|
137222
|
137223
|
137224
|
137225
|
137226
|
137227
|
137228
|
137229
|
139378
|
139379
|
139380
|
139381
|
139382
|
139599
|
139600
|
139601
|
139602
|
139603
|
139851
|
140150
|
141176
|
144257
|
144258
|
144259
|
144260
|
144261
|
144262
|
144264
|
144268
|
144269
|
144270
|
144271
|
144272
|
144273
|
151438
|
151439
|
151440
|
151441
|
151442
|
151443
|
151444
|
151445
|
151446
|
151447
|
151448
|
151449
|
151450
|
156340
|
156341
|
156342
|
156343
|
156344
|
156345
|
156346
|
156347
|
156348
|
156349
|
156350
|
156351
|
156352
|
156353
|
156354
|
156355
|
157656
|
157657
|
157658
|
157659
|
157660
|
157661
|
157662
|
157663
|
157664
|
157665
|
157666
|
157667
|
157668
|
157669
|
157670
|
157671
|
157672
|
167805
|
167806
|
167807
|
167808
|
167809