Make use of stale state

@larowlan Happy to add a test in if you think this is valid?

+++ b/src/Entity/PreviewSiteBuild.php
@@ -284,9 +284,31 @@ class PreviewSiteBuild extends ContentEntityBase implements PreviewSiteBuildInte
+    if (!empty($this->original) &&
+      $this->getStatus() === self::STATUS_BUILT &&
+      FALSE === $this->get('contents')->equals($this->original->contents)) {
+      return TRUE;
+    }
+    return FALSE;

any reason not to just return instead of using an if?

return !empty($this->original) && ...

+1 for a test

@larowlan Nope, just me not seeing the obvious. 😂

Adding test.

+++ b/src/Entity/PreviewSiteBuild.php
@@ -284,9 +284,28 @@ class PreviewSiteBuild extends ContentEntityBase implements PreviewSiteBuildInte
+      FALSE === $this->get('contents')->equals($this->original->contents));

should we also be comparing the updated time of the content items where they're instances of EntityChangedInterface?

@larowlan Good point. I think that could be useful, albeit if they're saving the build then it's more likely they've added or removed something so that would take precedent anyway. I'm happy to add it in though.

@larowlan Take a look and let me know what you think. In essence in addition to changing a build, when a content entity is updated it checks to see if there is a preview site build connected to it and sets the status to stale. I've introduced a setting that allows you to turn this behaviour off if it has performance implications. I've also added tests. Happy to discuss further. :)

The last submitted patch, 11: 3349512-4.patch, failed testing. View results →

Let's try this again.

This looks great, just one comment about the update hook and a minor performance optimisation

1. +++ b/preview_site.install
   @@ -31,3 +31,10 @@ function preview_site_update_9001(): TranslatableMarkup {
   +  \Drupal::configFactory()->getEditable('preview_site.settings')->set('proactive_stale_check', 0)->save();
   

   I think we need 'trust data' TRUE for the save call here because this is happening in an update hook

2. +++ b/preview_site.module
   @@ -102,3 +103,32 @@ function preview_site_cron() {
   +    foreach ($preview_site_builds as $preview_site_build) {
   +      $preview_site_build = $preview_site_build_storage->load($preview_site_build);
   

   Can we use loadMultiple outside the loop here, for better performance

3. +++ b/preview_site.module
   @@ -102,3 +103,32 @@ function preview_site_cron() {
   +        $preview_site_build->set('status', PreviewSiteBuildInterface::STATUS_STALE)->save();
   

   yeah this is much cleaner

@larowlan Addressed your feedback.

+++ b/preview_site.module
@@ -102,3 +103,31 @@ function preview_site_cron() {
+      if (!empty($preview_site_build)) {

we don't need this anymore, loadMultiple filters out nulls

Feel free to remove that and commit

@larowlan All done.

Thanks

Automatically closed - issue fixed for 2 weeks with no activity.