[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 #213 from Josef Moravec <josef.moravec at gmail.com> ---
Hi Charles, I just look to your code quickly and have some comments:

1) Bug 22015 added datatables.css to doc-head-close include file, so it is no
more needed to add it to every template and should be removed from
koha-tmpl/intranet-tmpl/prog/en/modules/tools/discrete_calendar.tt

2) Why are you using DateTime::Format::Strptime instead of datetime_parser from
schema when working with database? I don't think it is right

3) Koha/Schema/* files should always be in they own patch, this patch is not
pushed, but is here for ease testing, and schema is regenerated by release
manager when patchset is pushed

4) Use DateTime->clone->subtract or add in
bug_17015_part2_fill_discrete_calendar.perl when setting $startDate and
$endDate, it would be much more readable
my $startDate = DateTime->new(
day       => $currentDate->day(),
month     => $currentDate->month(),
year      => $currentDate->year()-2,
time_zone => C4::Context->tz()
)->truncate( to => 'day' );

5) Is this really needed?
#Added a default (standard) branch.
my $add_default_branch = 'INSERT IGNORE INTO branches (branchname, branchcode)
VALUES(?,?)';
my $add_Branch_Sth = $dbh->prepare($add_default_branch);
$add_Branch_Sth->execute('Default', '');

I don't think it is good idea to add a fake data to branches...

6) t/db_dependent/Circulation.t still references C4::Calendar

7) koha-tmpl/intranet-tmpl/prog/en/modules/tools/discrete_calendar.tt has many
missing filters

These are just few things which would need attention, but first please, write
down a short description/summary and bring it to meeting to discuss and vote
for this by community.

The description is needed for people who are not aware of this report to
summary the pros and cons to make them able to decide how this would continue.

Thanks for your contribution!

-- 
You are receiving this mail because:
You are watching all bug changes.