mitydigital / statamic-scheduled-cache-invalidator

MIT License
2 stars 3 forks source link

Add support for undated collections #4

Closed aerni closed 7 months ago

aerni commented 7 months ago

This PR adds the ability to invalidate entries of any collection (not just dated) based on provided query scopes. This opens up countless possibilities to invalidate entries based on other fields than the date.

I also took the chance to make the query scopes config more powerful by allowing granular scopes.

How it works

Examples

Apply a scope to all collections.

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

Apply multiple scopes to all collections:

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

Apply scopes to specific collections:

'query_scopes' => [
    'collection_handle' => \Path\To\Scope::class,
    'another_collection' => \Path\To\DifferentScope::class,
],

Apply multiple scopes to specific collections:

'query_scopes' => [
    'collection_handle' => [
        \Path\To\Scope::class,
        \Path\To\DifferentScope::class,
    ],
    'another_collection' => [
        \Path\To\Scope::class,
        \Path\To\DifferentScope::class,
    ],
],

Disclaimer

This PR is technically a breaking change in the case of a user applying a global query scope without specifying the collection. (As in config option 1 and 2). After this PR, global query scopes will be applied to dated AND undated collections. So this could result in undesired invalidation of undated entries.

There are a couple of ways we could handle this:

martyf commented 7 months ago

Thanks for this @aerni. Regarding the disclaimer, I think points 1, 2 or 4 are valid options.

The issue with 4 is people may miss it - but it is also the cleanest way to say "forget the past, just maintain that base going forward" which I like.

From another person's perspective, what would you like to see as a user?

aerni commented 7 months ago

I don't mind a major release with a breaking change. Especially in a situation like this, where it's a fundamental change upon which new features will be built. A feature flag feels a little complicated.

One thing to consider before releasing a major version is if this is really the best way to do things before moving forward. There might be some other features that would be nice to implement, but which might not work with the current state of this PR either. Resulting in a potential other major release sooner than later. So a major release should be thoughtful.

martyf commented 7 months ago

Did you have thoughts or suggestions in mind, whether for now or for later?

The whole concept is really opinionated to start with so definition of "best way" comes with that caveat anyway ;)

aerni commented 7 months ago

One feature that would be handy is the ability to invalidate an entry based on a boolean. This is particularly useful in combination with computed properties.

Collection::computed('events', 'is_live', fn ($entry) => $entry->is_digital_event ? Carbon::parse($entry->date)->subMinutes(15) < now() : null);

So instead of having to duplicate the logic of the computed property in a query scope, we can just query by is_live and validate the entry when it returns true.

You can already query by is_live but the entry would be invalidated every time the command is run as the entry would be queried every time. So we'd need a way to only invalidate it the first time is_live returns true.

This is just icing on the cake, though.

martyf commented 7 months ago

But would you see that a v2 extension to this, or something that is needed before considering a v2?

aerni commented 7 months ago

Depends. I don't think this is needed for v2. But to implement this feature, we might have to refactor and introduce another breaking change, hence v3. So if this is a feature that you'd consider, it might be best to first dig into it to see if it's doable with the current code base before releasing v2.

martyf commented 7 months ago

I'm not sure I have a use case for it personally (and quite have my head around what you've described) - but if it is something you wanted to look at because it would help with how you could use the addon, more than happy to include it. The initial purpose was really just to flush based on dates - and I love how this has grown with query scopes and the work you've done, so can see potential in future expansions for sure.

aerni commented 7 months ago

I might look at implementing this feature in the future. For now, I'm fine with the state of this PR. I'd release v2. If anything breaks in the future, we can always release v3.