Add starting-from-id option to regenerate command #2688
Conversation
|
Could you refactor this option to |
|
Could you also update the readme with an example? |
billypoke
changed the title
|
@freekmurze refactored and documented in the readme. Let me know if you have any other changes Thanks for taking the time 😄 |
|
looks like failing tests now. Seems to be failing on |
| $mediaIds = $this->getMediaIds(); | ||
| $noIds = count($mediaIds) === 0; |
There was a problem hiding this comment.
This whole bit feel unreadable to me.
Try to start with the special cases (so starting with determining if you should start from an id or model.
Instead of using &&, introduce new helper functions where you don't use && but work with early returns.
Also make it possible to get models that start with an id greater than some id.
billypoke
force-pushed
the
feature-regenerate-by-date
branch
from
02c063c to
93b657b
Compare
| } | ||
|
|
||
| return $this->mediaRepository->getByIds($mediaIds); | ||
| return $this->mediaRepository->all(); |
There was a problem hiding this comment.
I think this is what you were suggesting? Handle the special cases first, then fall back to ids or all media
There was a problem hiding this comment.
This seems better now.
A case that isn't handled well now is we you pass both modelType and startingFromId. Right now that startingFromId will be ignored.
There was a problem hiding this comment.
This should work without refactoring the existing methods to pass the builder instead of the result and chaining everything depending on the args and options.
I didn't think passing ids and modelType at the same time would make a ton of sense as a use case as the ids may not belong to the passed type.
|
Thanks! |
I have found myself wanting to regenerate recent images, but with potentially thousands of ids to pass as a command option, I thought adding a date option could be useful