{"action":"created","issue":{"url":"https://api.github.com/repos/laravel/framework/issues/26753","repository_url":"https://api.github.com/repos/laravel/framework","labels_url":"https://api.github.com/repos/laravel/framework/issues/26753/labels{/name}","comments_url":"https://api.github.com/repos/laravel/framework/issues/26753/comments","events_url":"https://api.github.com/repos/laravel/framework/issues/26753/events","html_url":"https://github.com/laravel/framework/pull/26753","id":387792444,"node_id":"MDExOlB1bGxSZXF1ZXN0MjM2MjAyMzA0","number":26753,"title":"[5.8] fix Illuminate\\Console\\Scheduling\\ManagesFrequencies::dailyAt() accepts a parameter specific to the second, which happened error.it  happen in the initial minute of the corresponding hour.","user":{"login":"yiranzai","id":20548510,"node_id":"MDQ6VXNlcjIwNTQ4NTEw","avatar_url":"https://avatars0.githubusercontent.com/u/20548510?v=4","gravatar_id":"","url":"https://api.github.com/users/yiranzai","html_url":"https://github.com/yiranzai","followers_url":"https://api.github.com/users/yiranzai/followers","following_url":"https://api.github.com/users/yiranzai/following{/other_user}","gists_url":"https://api.github.com/users/yiranzai/gists{/gist_id}","starred_url":"https://api.github.com/users/yiranzai/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/yiranzai/subscriptions","organizations_url":"https://api.github.com/users/yiranzai/orgs","repos_url":"https://api.github.com/users/yiranzai/repos","events_url":"https://api.github.com/users/yiranzai/events{/privacy}","received_events_url":"https://api.github.com/users/yiranzai/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2018-12-05T14:51:41Z","updated_at":"2018-12-06T02:35:34Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/laravel/framework/pulls/26753","html_url":"https://github.com/laravel/framework/pull/26753","diff_url":"https://github.com/laravel/framework/pull/26753.diff","patch_url":"https://github.com/laravel/framework/pull/26753.patch"},"body":"fix Illuminate\\Console\\Scheduling\\ManagesFrequencies::dailyAt() accepts a parameter specific to the second, which happened error.it  happen in the initial minute of the corresponding hour.\r\nsee [#26752](https://github.com/laravel/framework/issues/26752)\r\n- Laravel Version: 5.*\r\n- PHP Version: 7.1.19\r\n- Database Driver & Version: \r\n\r\n### Description:\r\n\r\nFirst of all, my English is very poor.\r\n`Illuminate\\Console\\Scheduling\\ManagesFrequencies::dailyAt()` accepts a parameter specific to the second, which happened error.\r\nFor example \r\n```\r\n<?php\r\n\r\nnamespace App\\Console;\r\n\r\nuse App\\Console\\Commands\\Test;\r\nuse Illuminate\\Console\\Scheduling\\Schedule;\r\nuse Illuminate\\Foundation\\Console\\Kernel as ConsoleKernel;\r\n\r\nclass Kernel extends ConsoleKernel\r\n{\r\n    /**\r\n     * The Artisan commands provided by your application.\r\n     * @var array\r\n     */\r\n    protected $commands = [\r\n        Test::class,\r\n    ];\r\n\r\n    /**\r\n     * Define the application's command schedule.\r\n     * @param  \\Illuminate\\Console\\Scheduling\\Schedule $schedule\r\n     * @return void\r\n     */\r\n    protected function schedule(Schedule $schedule)\r\n    {\r\n        $schedule->command('test:test')->dailyAt('12:59:00');\r\n        //$schedule->command('test:test')->dailyAt('12:00');\r\n    }\r\n\r\n    /**\r\n     * Register the commands for the application.\r\n     * @return void\r\n     */\r\n    protected function commands()\r\n    {\r\n        $this->load(__DIR__ . '/Commands');\r\n\r\n        require base_path('routes/console.php');\r\n    }\r\n}\r\n```\r\nThat is, `dailyAt('12:59:00')` => `dailyAt('12:00')`.\r\nThis should not happen, although the cron task is executed every minute, not specific to the second, but it should not happen in the initial minute of the corresponding hour.\r\nLet's look at the code\r\n```\r\n    /**\r\n     * Schedule the event to run daily at a given time (10:00, 19:30, etc).\r\n     *\r\n     * @param  string  $time\r\n     * @return $this\r\n     */\r\n    public function dailyAt($time)\r\n    {\r\n        $segments = explode(':', $time);\r\n\r\n        return $this->spliceIntoPosition(2, (int) $segments[0])\r\n                    ->spliceIntoPosition(1, count($segments) === 2 ? (int) $segments[1] : '0');\r\n    }\r\n```\r\nI can understand the meaning here: when the developer is lazy, `dailyAt('12')` = `dailyAt('12:00')`.\r\nbut `12:59:00` is also a standard time, so I think\r\n`dailyAt('12:59:00')` = `dailyAt('12:59')`\r\n`dailyAt('12:00:00')` = `dailyAt('12:00')` = `dailyAt('12')`\r\nThis is reasonable.\r\nTherefore, `dailyAt('12:59:00')` = `dailyAt('12:00')` is a serious bug.\r\nI recommend\r\n```\r\n    public function dailyAt($time)\r\n    {\r\n        $segments = explode(':', $time);\r\n\r\n        return $this->spliceIntoPosition(2, (int) $segments[0])\r\n                    ->spliceIntoPosition(1, count($segments) === 1 ? '0' : (int) $segments[1]);\r\n    }\r\n```\r\n### Steps To Reproduce:\r\nI have a timed task, as follows\r\n```\r\n<?php\r\n\r\nnamespace App\\Console;\r\n\r\nuse App\\Console\\Commands\\Test;\r\nuse Illuminate\\Console\\Scheduling\\Schedule;\r\nuse Illuminate\\Foundation\\Console\\Kernel as ConsoleKernel;\r\n\r\nclass Kernel extends ConsoleKernel\r\n{\r\n    /**\r\n     * The Artisan commands provided by your application.\r\n     * @var array\r\n     */\r\n    protected $commands = [\r\n        Test::class,\r\n    ];\r\n\r\n    /**\r\n     * Define the application's command schedule.\r\n     * @param  \\Illuminate\\Console\\Scheduling\\Schedule $schedule\r\n     * @return void\r\n     */\r\n    protected function schedule(Schedule $schedule)\r\n    {\r\n        $schedule->command('test:test')->dailyAt('12:59:00');\r\n    }\r\n\r\n    /**\r\n     * Register the commands for the application.\r\n     * @return void\r\n     */\r\n    protected function commands()\r\n    {\r\n        $this->load(__DIR__ . '/Commands');\r\n\r\n        require base_path('routes/console.php');\r\n    }\r\n}\r\n```\r\nBut I found that it didn't execute at 12:59\r\nIt was executed at 12:00.\r\n#### end\r\nso,I want to submit a pull request to fix it\r\n"},"comment":{"url":"https://api.github.com/repos/laravel/framework/issues/comments/444727128","html_url":"https://github.com/laravel/framework/pull/26753#issuecomment-444727128","issue_url":"https://api.github.com/repos/laravel/framework/issues/26753","id":444727128,"node_id":"MDEyOklzc3VlQ29tbWVudDQ0NDcyNzEyOA==","user":{"login":"yiranzai","id":20548510,"node_id":"MDQ6VXNlcjIwNTQ4NTEw","avatar_url":"https://avatars0.githubusercontent.com/u/20548510?v=4","gravatar_id":"","url":"https://api.github.com/users/yiranzai","html_url":"https://github.com/yiranzai","followers_url":"https://api.github.com/users/yiranzai/followers","following_url":"https://api.github.com/users/yiranzai/following{/other_user}","gists_url":"https://api.github.com/users/yiranzai/gists{/gist_id}","starred_url":"https://api.github.com/users/yiranzai/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/yiranzai/subscriptions","organizations_url":"https://api.github.com/users/yiranzai/orgs","repos_url":"https://api.github.com/users/yiranzai/repos","events_url":"https://api.github.com/users/yiranzai/events{/privacy}","received_events_url":"https://api.github.com/users/yiranzai/received_events","type":"User","site_admin":false},"created_at":"2018-12-06T02:35:34Z","updated_at":"2018-12-06T02:35:34Z","author_association":"NONE","body":"@laurencei emmmmm. \r\nIt's not a proper way to run `dailyAt('12:59:00')` at 12:00. If `dailyAt('12:59:00')` is considered as an invalid parameter, instead of identifying it as `dailyAt('12:00')`. It's better to throw an exception.\r\n\r\nProposed Changes:\r\n1. Throw an exception (warning the developer)\r\n2. Intercept to the minute (I recommend this way)\r\n\r\n`count($segments) === 2 ? (int) $segments[1] : '0'` is a very bad treatment.\r\nAnyway, It does not make sense to treat the time specific to the seconds as the first minute of the hour it belongs to."}}
{"actor":{"display_login":"yiranzai"}}