[4.x] Add isDirty / isClean

[ryanmitchell]

This PR adds a trait for isDirty() / isClean() support on Entries through the use of a trait. I set it up this way to allow it to be extended to other stores if the approach is felt to be correct.

Tests are failing as the test environment doesn't seem to like using $entry->fresh() - not sure how to get around that!

You can use the functions as follows:

$entry->isDirty();
$entry->isDirty('field');
$entry->isDirty(['field1', 'field2']);

$entry->isClean();
$entry->isClean('field');
$entry->isClean(['field1', 'field2']);

Closes statamic/ideas#7

[jasonvarga]

Nice. Although the toCacheableArray method is never used, I think that's a leftover relic from v2. I'm going to sneak the removal of it into 3.3, and you can make your own dedicated method in this PR.

[ryanmitchell]

No problem - I've updated that method to be called getDirtyArray() (please come up with something better), and put some of the data logic in there.

Also separated out a separate getOriginal() seeing as the logic was there for it anyway.

[jasonvarga]

In other classes, for example, Asset, we already have the concept of "original" data. I'd want to use that here. There's a trait, etc.

[ryanmitchell]

Yep I can see the syncOriginal function - see how I've updated to look for getDirtyArray if it exists (seemed like the best approach). However, I've no idea where in the entry 'lifecycle' to insert syncOriginal() to get it to populate with any yaml-stored values?

[jasonvarga]

That would need to go in the Stache where it initially makes an instance from the file. Right about here:

cms/src/Stache/Stores/CollectionEntriesStore.php

Line 100 in 71fac6d

[ryanmitchell]

Thank you! Thats updated now - let me know what you think.

[ryanmitchell]

Not sure why tests are failing on this (and they seem unrelated to the changes made). They are all passing for me locally.

[ryanmitchell]

@jasonvarga Struggling a bit with the tests on this one. On the two asset failures, do I just update the tests to expect the .meta files? Or do I handle it some other way?

[ryanmitchell]

I've got all tests passing locally now, but I cant get them working here in GitHub actions. If you can see why that would be really helpful.

[duncanmcclean]

I've got all tests passing locally now, but I cant get them working here in GitHub actions. If you can see why that would be really helpful.

Looks like an error on GitHub's side, it doesn't get past setting up the jobs.. probably needs to be re-run.

[jasonvarga]

Thanks for this PR. Another banger. And sorry for the delay, it's pretty substantial.

I made a handful of changes:

• Moved HasDirtyState into Statamic\Data where there are similar data-related traits.
• Use HasDirtyState everywhere, and deprecate the existing SyncsOriginalState.
• Ensures syncOriginal() is called at the end of save() so that events etc get the appropriate dirty state, and added tests for this.
• Renamed getDirtyArray to a wordier but more explicit getCurrentDirtyStateAttributes so it doesn't get confused with getDirty which Eloquent models have and I imagine we might add down the road.
• Add tests for isClean
• Moved all the stache syncOriginal stuff to a consistent spot in one class - when it's created from the file or loaded from the cache.

Some additional notes:

• The user events test should be testing creating/created but it would mean changing unrelated tests, so I just left a comment instead.
• Since assets store "data" in a sub-array, you needed to add extra logic in isDirty to handle that. I'd prefer it to be a flat array like everywhere else but that would have made this PR grow in scope, and might've been a breaking change. We'll consider changing that for v5.

Changes made

[ryanmitchell]

Thanks for all the changes. As always you’ve made it much better.

[ryanmitchell]

❤️

[edalzell]

KA
BOOM

[afonic]

Thanks for this PR, it will be massively helpful.

Not sure where to ask so I'll try here! This breaks some tests for an addon of mine and I am struggling to understand why. For example this test: https://github.com/reachweb/locale-lander/blob/main/tests/Feature/LocaleLanderTest.php#L61 works fine if I remove the HasDirtyState trait from the Entry model (or downgrade to v4.50.0).

It seems that when using the trait, when checking for the translation here: https://github.com/reachweb/locale-lander/blob/main/src/Support/LocaleHelper.php#L63 it will return null. To make matters even worse, if I run the test 10 times, it will find the Entry 2 times out of 10 (or something like that).

Any ideas on what causes this and how to fix the tests?

Note that this only affects the test environment, the addon works fine in the actually Statamic install.

[duncanmcclean]

@afonic I haven't dug into it but could your issue be related to #9651? If not, maybe start a new Discussion? (to avoid everyone in this PR being notified of any comments 😄 )