PrestaShopCorp / cronjobs

Manage all your automated web tasks from a single interface
18 stars 91 forks source link

Fix day of week in task list #39

Closed mcdado closed 4 years ago

mcdado commented 7 years ago

Since day zero (epoch time) was a Thurday (Thu, 01 Jan 1970 00:00:00) when using mktime() we need to take that into consideration. Thursday is day 4, Monday is day 1; to make our counting start from 1 we need to decrement by 3.

This explains why after selecting a day when creating or editing a task the day shown in the list was wrong.

PierreRambaud commented 5 years ago

Is this pull request still needed?

Look like it's really weird to remove 3 without message. Not sure it's the right way to retrieve the day. Maybe have a look at date('l', $timestamp);

mcdado commented 5 years ago

There's no $timestamp, just a number as weekday. It's a workaround for this issue:

$ php -a
Interactive shell

php > echo date('l', 0);
Thursday

I could refactor it to make it clearer if you want...

mcdado commented 5 years ago

I tried refactoring using $cron['day_of_week'] as an index to an array of weekday names, but it's messed up because it's index zero and all that crap... it's way more elegant to subtract 3 and be done with it. It's not any more a magic number than Epoch is.

mcdado commented 5 years ago
// We don't use mktime for the weekday because timestamp zero is a Thursday.
$weekdays = array(
    self::$module->l('Monday'),
    self::$module->l('Tuesday'),
    self::$module->l('Wednesday'),
    self::$module->l('Thursday'),
    self::$module->l('Friday'),
    self::$module->l('Saturday'),
    self::$module->l('Sunday'),
);

$cron['hour'] = ($cron['hour'] == -1) ? self::$module->l('Every hour', 'CronJobsForms') : date('H:i', mktime((int)$cron['hour'], 0, 0, 0, 1));
$cron['day'] = ($cron['day'] == -1) ? self::$module->l('Every day', 'CronJobsForms') : (int)$cron['day'];
$cron['month'] = ($cron['month'] == -1) ? self::$module->l('Every month', 'CronJobsForms') : self::$module->l(date('F', mktime(0, 0, 0, (int)$cron['month'], 1)));
$cron['day_of_week'] = ($cron['day_of_week'] == -1) ? self::$module->l('Every day of the week', 'CronJobsForms') : $weekdays[(int) $cron['day_of_week'] - 1];
$cron['updated_at'] = ($cron['updated_at'] == 0) ? self::$module->l('Never', 'CronJobsForms') : date('Y-m-d H:i:s', strtotime($cron['updated_at']));
$cron['one_shot'] = (bool)$cron['one_shot'];
$cron['active'] = (bool)$cron['active'];

I can commit this if you prefer it...

PierreRambaud commented 4 years ago

Thanks @mcdado