Added optional days argument to clean command #484
Conversation
|
I like this idea! The only thing is that I would use it as an option and not as argument. Arguments have to be in a fixed order and with every new it get's more complex. That's why I like to have commands with just one argument and the others - primary if optional - as option. |
|
Thank you for this PR. Like @Gummibeer suggested, an option would be better in this case. Could you also add a test to ensure that it works as intended. |
|
Thank you guys! I've updated the code to use an option rather than argument and added test coverage. Will this work? |
|
|
||
| Artisan::call('activitylog:clean', ['--days' => 7]); | ||
|
|
||
| $cutOffDate = Carbon::now()->subDay(7)->format('Y-m-d H:i:s'); |
There was a problem hiding this comment.
For multiple days please the subDays() method.
|
|
||
| $cutOffDate = Carbon::now()->subDay(7)->format('Y-m-d H:i:s'); | ||
|
|
||
| $this->assertCount(0, Activity::where('created_at', '<', $cutOffDate)->get()); |
There was a problem hiding this comment.
Wouldn't a total count be better here? If I'm not wrong there should be 7 left in the database!?
Or better as a second assert.
|
@bradcis and could you please merge the current master in here - this will let pass all unittests again. :) |
|
I've merged it into |
|
Thanks for finishing this up, @Gummibeer! I apologize for not responding sooner. |
Please bear with me, @freekmurze and team! This is my first ever pull request, so I'm hoping I'm following the proper procedure.
I'm requesting we add the ability to set the number of days threshold on the activitylog:clean command if the end user wants to deviate from the default set in the config file. The use case is that you may have multiple logs and want the retention period to be different for each one of them (i.e. user activities are deleted after 7 days where as project activities are deleted after 30 days).
The end user could then add the activitylog:clean command to their schedule with each variation.