Calculating dependencies of `ReferenceField(Type)PropExpression` is missing intermediate dependencies

Fantastic find!

@penyaskito discovered this in #3534561: Integrate with core's default content exporter.

Not every stable blocker needs to block an RC, but this one does.

Per https://git.drupalcode.org/project/canvas/-/merge_requests/23/diffs#note....

I think I've got all the necessary remaining pieces ready locally. Too tired to finish this today though. Will finish tomorrow AM.

Follow-up created: #3547579: Introduce a new cache tags aware prop shape repository, so changes affecting prop shape calculation can force the re-invoke of hook_canvas_storable_shape_prop_alter. Really, it's not a follow-up for this issue, but just a completely separate bug that was surfaced by this issue's new functional test coverage 😅

All done and fixed. Sprinkled ℹ️ remarks for @penyaskito to review and do the final sign-off. He found this, he reported it, he wrote the test coverage to first prove this was broken.

So I think he's the perfect person to confirm that what I did is correct 😊

Thanks for all the due diligence you did at #3534561-37: Integrate with core's default content exporter, @penyaskito! 👏🙏

This indeed allows us to merge this with confidence 😊

Actually, this needs update path tests. Similar to the update path + test coverage at https://www.drupal.org/node/3539207.

(We got rid of that update path test coverage in the XB → Canvas rename.)

That'll be a good baseline to establish 👍

1. This looks fantastic! 🤩
2. AFAICT no update path is necessary for JavaScriptComponent config entities 😇
3. I'm a bit worried about recalculating all dependencies to check whether an update is necessary; why not rely on the presence of ReferenceField(Type)PropExpression instead? Described how I think that can work.
4. I think the various add-* files to construct a config entity with a component instance whose dependencies would've been incorrectly calculated prior to !23 are fine, but unnecessarily complex. I think we could instead be using the exact same component tree with a ReferenceFieldTypePropExpression in a StaticPropSource in all of them. Special case for ContentTemplate, where I'd want to see a <code>ReferenceFieldPropExpression in a DynamicPropSource in addition.
   That would then mean that UpdateIntermediateDependenciesUpdateTest could become 10x simpler: it'd just check for the same component instance UUID in a Pattern, a PageRegion, a ContentTemplate and a FieldConfig, and verify that additional dependencies appeared after the update. For ContentTemplate, you'd do an additional check.
   IOW: the same before vs after expectations could be applied to all of the config-entities-under-test. It'd make the update path test coverage much easier to review and maintain. (And would be an even better reference for future update paths!)
5. Last but not least: shouldn't we also be updating all content entities; at minimum the default revision, preferably all revisions?

   https://www.drupal.org/node/3539207 didn't need to do that because it was an optimization, but here it is a fix that impacts data integrity: without such dependencies, in theory it's possible that e.g. the intermediary Media entity gets deleted because the content entity's component tree depended only on the File entity that the Media pointed to.

   However: A) I remember us discussing this and @effulgentsia convincing us we don't need that?, B) deleting content entities doesn't actually check the impact on things depending on it, see \Drupal\Core\Entity\ContentEntityDeleteForm and \Drupal\Core\Entity\EntityDeleteForm::buildForm(), so even if we did update all revisions' inputs' dependencies in the DB, it would have zero effect. The only effect it would have, is to make Canvas' Audit functionality more precise.

   Conclusion: I'm fine with proceeding without this, especially given this is not yet a stable module.

#24.4 has not yet been addressed yet. 🙏 That'd make these tests much easier to understand and much more maintainable.

After spending more hours than either of us really wanted pairing on this (and the blocking https://git.drupalcode.org/project/canvas/-/merge_requests/219 MR), we both think this is finally ready to go!

🌇