[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 #119 from Jonathan Druart <jonathan.druart at bugs.koha-community.org> ---
Very quick QA feedbacks:

* +use Koha::Account::Line;
added in C4/Overdues.pm

* in getDatesInfo
+            select  => [ 'date', { DATE => 'date' } ],
+            as      => [qw/ date date /],
What does this?

* Mix of snake_case and camelCase in method names (stick to snake_case)

* Same for columns in DB (isopened => is_opened)

* "DFLT" for the default is confusing, use empty string instead like in other
areas of Koha

* Why did not you use Koha::Object[s]?

* +    my ($self, $title, $weekday, $holidaytype, $openHour, $closeHour,
$startDate, $endDate, $deleteType, $today) = @_;

You should use a hashref instead

* Types of holidays (E, F, N, R, W): please use global variables with readable
value ($holidays->{Exception} = "E"; for instance, or $EXCEPTION_HOLIDAY = 'E')

It will make this kind of lines easier to read:
+    }elsif ($holidaytype eq 'E' || $holidaytype eq 'F' || $holidaytype eq 'N')
{

* When you use DBIx::Class you should avoid to write SQL, for instance:
  date BETWEEN DATE(?) AND DATE(?)
should be:
  date => { '>=' => $from, '<=' => $to }
or:
  date => { -between => [$from, $to] }

* Isn't DAYOFWEEK a MySQLism? We think we should find another way to do that.

* At first glance DateTime::Format::Strptime->new should be replaced with my
$dtf = $schema->storage->datetime_parser

* misc/cronjobs/add_days_discrete_calendar.pl is using SQL statements, use your
new module instead

* This same cronjob must use $schema->storage->txn_begin or ->txn_do instead of
turning off the AutoCommit flag of DBI

* t/db_dependent/Fines.t: Do not use DateTime directly, dt_from_string instead:
dt_from_string->add( day => 42 )

* tools/discrete_calendar.pl
Use C4::Context::only_my_library for $onlymine

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