[Koha-bugs] [Bug 17015] New Koha Calendar

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17015

--- Comment #132 from Josef Moravec <josef.moravec at 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.