https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17015 --- Comment #132 from Josef Moravec <josef.moravec@gmail.com> --- Comment on attachment 71634 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=71634 Bug 17015 - DiscreteCalendar UI, Back-End and necessary scripts Review of attachment 71634: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=17015&attachment=71634) ----------------------------------------------------------------- Just quickly read the code and found few issues which should be addressed I think ::: Koha/DiscreteCalendar.pm @@ +16,5 @@
+# along with Koha; if not, see <http://www.gnu.org/licenses>. + +#####Sets holiday periods for each branch. Datedues will be extended if branch is closed -TG +use strict; +use warnings;
Should be: use Modern::Perl; @@ +72,5 @@
+ +=cut + +sub new { + my ( $classname, %options ) = @_;
You are using hash for options, but should be hashref - we use hashrefs in other packages of Koha namespace at least, see https://wiki.koha-community.org/wiki/Coding_Guidelines#PERL16:_Hashrefs_shou... @@ +156,5 @@
+=head2 add_new_branch + + Koha::DiscreteCalendar->add_new_branch($copyBranch, $newBranch) + +This methode will copy everything from a given branch to a new branch
Typo: "methode" -> "method" ::: installer/data/mysql/atomicupdate/bug_17015_part1_create_discrete_calendar.sql @@ +9,5 @@
+ `holiday_type` varchar(1) DEFAULT '', + `note` varchar(30) DEFAULT '', + `open_hour` time NOT NULL, + `close_hour` time NOT NULL, + PRIMARY KEY (`branchcode`,`date`)
Please specify the encoding of the table. The table is also missing in kohastructure.sql ::: installer/data/mysql/atomicupdate/bug_17015_part2_fill_discrete_calendar.perl @@ +29,5 @@
+ +if ($help) { + print $usage; + exit; +}
I think atomic update should not take params. ::: koha-tmpl/intranet-tmpl/prog/en/modules/tools/discrete_calendar.tt @@ +3,5 @@
+<title>Koha › Tools › [% Branches.GetName( branch ) %] calendar</title> +[% INCLUDE 'doc-head-close.inc' %] +[% INCLUDE 'calendar.inc' %] +<link rel="stylesheet" type="text/css" href="[% interface %]/[% theme %]/css/datatables.css" /> +<script type="text/javascript" src="[% interface %]/lib/jquery/plugins/jquery-ui-timepicker-addon.min.js"></script>
Javascript should be at and of page, see: https://wiki.koha-community.org/wiki/Coding_Guidelines#JS12:_Include_javascr... @@ +376,5 @@
+ }); + //]]> + </script> + <!-- Datepicker colors --> + <style type="text/css">
Please move this to separate css file. ::: misc/cronjobs/add_days_discrete_calendar.pl @@ +3,5 @@
+# +# This script adds one day into discrete_calendar table based on the same day from the week before +# +use strict; +use warnings;
use Modern::Perl; ::: tools/discrete_calendar.pl @@ +17,5 @@
+ +#####Sets holiday periods for each branch. Datedues will be extended if branch is closed -TG +use strict; +use warnings; +
use Modern::Perl; -- You are receiving this mail because: You are watching all bug changes.