Skip to content
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

Support the addition of a query scope #2

Merged
merged 4 commits into from
Jan 16, 2024

Conversation

ryanmitchell
Copy link
Contributor

Requires statamic/cms#5927

This PR adds a config statamic-scheduled-cache-invalidator.query_scope that accepts a snake cased query scope defined by the developer. This allows the query to be extended with other conditions, for example an end date.

The query scope receives both collection and now in the values param, which it may use to determine what restrictions to place.

@martyf
Copy link
Contributor

martyf commented Jan 15, 2024

Thanks Ryan - in the event of a user wanting to use multiple scopes (i.e. different rules for different collections), would it make sense for the config param to be an array, with the key the collection handle, and the value the scope as you've done?

Such as:

'query_scopes' => [
    'blog' => 'my_blog_scope',
    'news' => 'my_news_scope',
],

The only other thought would be to also allow passing the class name, and having the addon translate that accordingly. Such as:

'query_scopes' => [
    'blog' => \App\Scopes\MyBlogScope::class,
    'news' => \App\Scopes\MyNewsScope::class,
],

Then calling with:

app($scope)->apply($query, $feedConfig);

I went down this path with Feedamic (see the latest update) as I felt it was neater - could just be my personal opinion on that though 😉 I like this because it means within the IDE I can click on the scope to jump to it.

Thoughts?

@ryanmitchell
Copy link
Contributor Author

Yep makes sense, and avoids the need to wait for that PR.
Updated!

@ryanmitchell ryanmitchell marked this pull request as ready for review January 15, 2024 21:09
@martyf
Copy link
Contributor

martyf commented Jan 16, 2024

You're a legend, thanks @ryanmitchell!

@martyf martyf merged commit d37fb90 into mitydigital:main Jan 16, 2024
@martyf
Copy link
Contributor

martyf commented Jan 16, 2024

Just a note @ryanmitchell, after merging I tweaked the behaviour for "all" vs collection-specific.

For all:

'query_scopes' => \Path\To\Scope::class

For collections:

'query_scopes' => [
    'collection_a' => \Path\To\Scope::class,
    'collection_b' => \Path\To\DifferentScope::class
],

Just in case for some bizarre reason someone has a collection called "all". Me being glass half empty here ha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants