Make use of stale state

+++ 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

+++ 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?

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

+++ 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

Thanks