-
Notifications
You must be signed in to change notification settings - Fork 4
feat(stories): Add published_at_gt
and other date related query parameters
#65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
src/Domain/Value/QueryParameter/FirstPublishedAtQueryParameter.php
Outdated
Show resolved
Hide resolved
|
||
namespace Storyblok\Api\Domain\Value\QueryParameter; | ||
|
||
final readonly class FirstPublishedAtQueryParameter extends DateQueryParameter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
final readonly class FirstPublishedAtQueryParameter extends DateQueryParameter | |
final readonly class FirstPublishedAt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am using FirstPublishedAtGt
... now. I would prefer new FirstPublishedAtGt
rather than new FirstPublishedAt($dateTime, '<some enum>');
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not passing an argument which is an enum? Such as
new FirstPublishedAt(direction: Direction::LT)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the query parameter is named like this "first_published_at_gt" or "first_published_at_lt". I think it is more intuitive for the developer to use a value object that is directly named like the query parameter itself.
In this case it is not like in the sort_by
parameter where the direction is used as value for some field.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #65 +/- ##
============================================
+ Coverage 93.88% 93.91% +0.02%
- Complexity 461 488 +27
============================================
Files 72 79 +7
Lines 1620 1675 +55
============================================
+ Hits 1521 1573 +52
- Misses 99 102 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
published_at_gt
and other date related query parameters (#64)published_at_gt
and other date related query parameters
Any further feedback? |
Adds support for these query parameters:
published_at_gt
published_at_lt
first_published_at_gt
first_published_at_lt
updated_at_gt
updated_at_lt
More about this topic in the issue: #64