https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17015
--- Comment #132 from Josef Moravec <josef.mora...@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_should_be_used_as_arguments @@ +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_javascript_at_the_end_of_template @@ +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. _______________________________________________ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
