Allow linking to referenced entities: add `url` property to `EntityReferenceItem::propertyDefinitions()`

Well, wait a sec. Wouldn't that just be a normal entity reference field-powered link? Surely that would be done with a dynamic prop source (perhaps with an adapter on top of it), since those can target any property of any field at any level of a reference chain.

Don't get me wrong, we do need the ability to link to referenced entities for sure, but I don't think that's the intention of HostEntityUrlPropSource.

1. 🙏 Please link to related/relevant issues next time. This is essentially a dangling issue right now. I shouldn't have to go and fix all issue metadata to convey 1) what feature/functionality this is part of, especially if it's a feature you've prioritized (→ #3541000: [META] Content templates UI for 1.0: only nodes, no exposed slots, no replacement for the view mode/display UI), 2) what prior issue introduced the thing that you think is incomplete but which lacked sufficient precision in expectations/requirements (→ #3545859: Add a `host-entity-url` prop source for linking to the host entity).

   Did that for you this time.

2. Also, this is not part of RC2, because the second MR for #3545859: Add a `host-entity-url` prop source for linking to the host entity literally landed yesterday so can't be part of RC2. Plus, until #3555068: Linking a `HostEntityUrlPropSource` to populate a `type: string, format: uri|uri-reference` in a `ContentTemplate` has no effect is fixed, this actually won't work at all in the ContentTemplate UI.
3. ⚠️Note that this means we'll need to rename HostEntityUrlPropSource to EntityUrlPropSource, because it would no longer be limited to the host entity.
4. There's way too much handwaving in this issue. Exactly the kind of handwaving that causes this issue to exist at all, because if #3545859: Add a `host-entity-url` prop source for linking to the host entity had been more precise, this follow-up would've been avoided 😬

   The number of ambiguities in

   Expand this feature to ensure all entities that may be accessed in content templates, not just the host entity, can be used in link props.

   is staggering:

   1. Depth/level of indirection for suggested entity URLs: I think you mean only URLs for the entities referenced directly by the host entity? So, for example, assuming we're looking at the content template for some Node bundle:
      - suggest URL to the author (User) of a Node (1 reference), but NOT URL to the user picture of the author (User) of the Node (2 references)
      - suggest URL to the "primary topic" (Term) of a Node (1 reference), but NOT URL to the "resident expert" (User) of a "primary topic" (Term) of a Node (2 references)

      Or did you really want the entity URL to be available at every one of those levels? (Which your phrasing seems to imply.)

   2. If the answer to the above is "yes, every level", then what about the levels you previously asked to omit? You requested in #3547598: Refine API response with DynamicPropSource suggestions to provide better UX that we hide/omit the "intermediary Files", which also means linking to it becomes harder. Or did you want for a media entity both the "media item canonical URL" and "media item image file canonical URL" to be surfaced?
   3. Speaking of that: what do you expect the labels to become? No guidance existed for this in #3545859: Add a `host-entity-url` prop source for linking to the host entity (because it landed with zero product requirements or design), so I did the simplest possible thing that would clearly need to be improved later: I labeled it Canonical absolute URL — because it's factually true, and because it'd guarantee that we'd improve it later. But if many such suggestions would start appearing, then it'd become a problem, doubly so if to the prior point you'd answer that you expect both suggestions (which would appear as siblings) to get a unique label.
   4. What about multiple cardinality? The typical "Tags" field is multiple-cardinality, and so it's impossible to offer a single URL for it. (This is basically the same problem as #3522718: [later phase] [needs design] UX for associating mismatched cardinality field instance (too little or too much) with a higher cardinality SDC prop (e.g. `type: array, maxItems: 5`) or lower cardinality (e.g. `type: string`).)
      EDIT: or, I guess you would've expected this to return an array of links (type: array, items: {type: string, format: uri|uri-reference}? Even though that's still not yet supported for the much simpler "static value" case: #3546869: Editing support for `type: array`: support multiple-cardinality Field Widgets (`StaticPropSource`s).)
   5. The intent is per #3551455: HostEntityUrlPropSource should be able to support absolute or relative URLs, URL options that you would get multiple choices — right now it's limited to "the canonical absolute URL", but in the future it might be with/without path alias, other link relations (e.g. the "edit form"), etc. How will what you ask scale to support all of those?

   … I'm pretty sure there's more things still, but those are the immediate ones that come to mind.

🙏🙏🙏🙏 Please create a mock-up of what you expect, such as the one in #3547598-23: Refine API response with DynamicPropSource suggestions to provide better UX. Please think through all the above aspects and indicate what is in scope vs what is not.

d.o's "cross-posting causes unpublishing" bug strikes again 🫣

@phenaproxima in #2:

Wouldn't that just be a normal entity reference field-powered link?

Conceptually, @lauriii is asking to make for every entity reference encountered (not sure if at the top level or also deeper, see my prior comment), we also offer a HostEntityUrlPropSource for the referenced entity.

\Drupal\Core\Field\Plugin\Field\FieldType\EntityReferenceItem::propertyDefinitions() does not have a property that generates a URL, so relying on the existing reference fields is insufficient.

🤔 But … perhaps that is reasonable to do? #3545859: Add a `host-entity-url` prop source for linking to the host entity did what it did because we didn't want to add a computed field to all of the entities. But for an entity reference to provide a canonical URL to the referenced empty … that seems kinda reasonable? Probably only the canonical and relative (so: type: string, format: uri-reference) one, and that'd be sufficient for 99% of cases?

(That would also be compatible with #3551455: HostEntityUrlPropSource should be able to support absolute or relative URLs, URL options, because you'd be unlikely to ever want to link to "the edit form of the referenced user/term" — that'd be more likely to only occur for the host entity itself.)

FYI: tagged stable blocker because of

⚠️Note that this means we'll need to rename HostEntityUrlPropSource to EntityUrlPropSource, because it would no longer be limited to the host entity.

i.e. we shouldn't be doing such a significant change after 1.0. That'd be too disruptive.

I would expect this to inherit our existing limitations for level of depth, i.e. the link should be available for all of the entity references that are accessible there. So if user can map an image field to a user picture, they should be able to map a link field to the user pictures URL. Same is true for the resident expert for the primary topic.

This will require a significant refactor, then:

1. HostEntityUrlPropSource → EntityUrlPropSource and for anything except the host entity, we'd require an expression (that explains how to traverse the Typed Data tree), just like for DynamicPropSource
2. the logic to suggest HostEntityUrlPropSource today is stupid simple, because it doesn't need to traverse Typed Data. That will need to be refactored to share a subset of that infrastructure with DynamicPropSource's matching logic.

You casually say "inherit our existing limitations of level of depth", but this is quite complex to achieve, because these URLs that are offered to users are NOT part of Typed Data! If on #3545859: Add a `host-entity-url` prop source for linking to the host entity these product requirements had been conveyed, I would never even have suggested this new prop source type! 😬 Because if we expect such close integration, just adding new computed fields would be a LOT simpler 😅 In fact, it might still be preferable to backtrack/revert our prior decision then…

Do you expect this all to be done before 1.0, or after?

Could we just call it URL and Slug? They should not appear as siblings; they should appear under a child menu. For example, Image => URL or Authored by => URL.

I think by "slug" you refer to path alias and by "URL" you refer to /node/<nid>? If so, please chime in on #3551455: HostEntityUrlPropSource should be able to support absolute or relative URLs, URL options for that.

But that's not what I was saying/asking there. I was asking specifically about the collapsing/simplifying you asked us to implement for File entities into the containing Media entity. Which starts to fall apart when there's then 2 URLs that are supposed to appear: how do you expect those to be labeled and ordered relative to other suggestions?

So, to make #9 very concrete, I am proposing to remove HostEntityUrlPropSource and add computed fields to all entities instead:

IOW: why go through the pains of simulating the presence of entity URLs in the right place in the Typed Data tree (which is the path we're currently on) when we could much more simply just expose it as a computed field with multiple properties to offer multiple kinds of URLs?

— me, at #3545859-41: Add a `host-entity-url` prop source for linking to the host entity

Postponed #3551455: HostEntityUrlPropSource should be able to support absolute or relative URLs, URL options and #3555068: Linking a `HostEntityUrlPropSource` to populate a `type: string, format: uri|uri-reference` in a `ContentTemplate` has no effect on #10, to avoid even more potentially wasted effort.

Meeting with @lauriii on Monday. Pinged @effulgentsia and @phenaproxima, who led the charge on landing #3545859: Add a `host-entity-url` prop source for linking to the host entity, and so should definitely chime in.

After reading this #10 makes sense to me. And if this needs to be solved for Drupal CMS, I wonder if providing that computed field should be the responsibility of canvas or a different specific contrib module.

Marked #3545859 unfixed in #3545859-45: Add a `host-entity-url` prop source for linking to the host entity.

Discussed yesterday with @lauriii, and then with @effulgentsia.

Conclusion: a hybrid between what we have in HEAD (we'd keep HostEntityUrlPropSource), but add a new computed field property to entity reference fields.

Details:

1. Let's keep HostEntityUrlPropSource this issue added, only for the host entity. There is indeed a likely future need for e.g. linking to the edit-form, or with/without path processing (path aliases), etc. → we have #3551455: HostEntityUrlPropSource should be able to support absolute or relative URLs, URL options for that.
2. Its position in the suggestions available to the Site Builder when constructing a ContentTemplate is fine as-is (at the very top of the root level — only if the target prop shape is type: string, format: uri|uri-reference, of course).
3. Let's add a url field property to \Drupal\Core\Field\Plugin\Field\FieldType\EntityReferenceItem::propertyDefinitions(), and make it only ever return the canonical URL. This would then simply reuse the existing Typed Data-based DynamicPropSource matching+suggesting logic — no changes needed.
4. Rationale: a clash of field names (which we'd need to add if reverted HostEntityUrlPropSource and added a computed field instead) is far more likely than adding a computed field property. Any module could be altering base fields, but only one module can alter a field type's properties, which also exceedingly few modules do. This would mean it is feasible to bring this part into core. (Whereas making core auto-add a new (computed) base field to all (content) entity types would be fairly unlikely.)

— #3545859-48: Add a `host-entity-url` prop source for linking to the host entity

AFAICT this addresses all concerns by both @lauriii from a product POV and the technical/feasibility concerns I raised here 😊

MR up with a first commit that starts building this. The first commit gets us to the point where indeed all entity reference fields are suggested, including nonsensical ones:

🐛 REQUIRED, type=string&format=uri-reference — either extraneous field instance matches found, or missing expectations
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
-Array &0 []
+Array &0 [
+    2 => 'ℹ︎␜entity:media:baby_videos␝bundle␞␟url',
+    5 => 'ℹ︎␜entity:media:press_releases␝bundle␞␟url',
+    6 => 'ℹ︎␜entity:media:vacation_videos␝bundle␞␟url',
+    13 => 'ℹ︎␜entity:node:foo␝marketing_docs␞␟entity␜␜entity:media␝bundle␞␟url',
+    14 => 'ℹ︎␜entity:node:foo␝marketing_docs␞␟url',
+    19 => 'ℹ︎␜entity:node:foo␝media_video_field␞␟entity␜␜entity:media␝bundle␞␟url',
+    20 => 'ℹ︎␜entity:node:foo␝media_video_field␞␟url',
+    21 => 'ℹ︎␜entity:node:foo␝type␞␟url',
+]

It works.

Tested while editing the ContentTemplate for article Nodes, using the type: string, format: uri-reference prop in the "all props" test SDC:

Before

After

I have almost nothing left to complain about; just moving a couple of comments around, and adding a little more high-level explanation to one thing. But otherwise, I say SHIP THIS CHUNGUS!

1. That is best explained by quoting the test coverage, which makes this very explicit:
   

               'Silly image 🤡 → URI → Root-relative file URL' => 'ℹ︎␜entity:node:foo␝field_silly_image␞␟entity␜␜entity:file␝uri␞␟url',
               'Silly image 🤡 → URI' => 'ℹ︎␜entity:node:foo␝field_silly_image␞␟entity␜␜entity:file␝uri␞␟value',
               "Silly image 🤡" => 'ℹ︎␜entity:node:foo␝field_silly_image␞␟src_with_alternate_widths',
   

   1. The first ("URI → root-relative file URL"): the image field's reference to the File entity is followed, then the "url" property on the "uri" field on the File entity is retrieved.
   2. The second ("URI"): the image field's reference to the File entity is followed, then the "value" property on the "uri" field on the File entity is retrieved — aka the main property on that field, which is why there's one less level in the hierarchy for this one.
   3. The third ("just the image field itself"): the image field's src_with_alternate_widths computed property that Canvas added.

   You can only see all 3 if you have an SDC/code component prop with a very open-ended prop, because the second one returns a public:// URI. but, arguably, it does not make sense to expose "root-relative file URL" for an image field, either, because src_with_alternate_widths is likely preferable. Except … if you want to link to the original image, because then the query string is just noise.

   ⚠️ This is confusing because of the simplification you requested previously: the omitting of intermediary levels (which I referred to in #3.2). Because if it'd said "Image → File → URI → Root-relative file URL", it'd have been much clearer that "URI" was a field on the File entity. This is tricky to get just right from a UX/Product POV 😬

   Proposal: special-case the "URI" field on the File entity type to always show the specific description of the field property, even for the main property. And possibly also change the label of that base field to "File URI". Which would then result in:

               'Silly image 🤡 → File URI → File URL' => 'ℹ︎␜entity:node:foo␝field_silly_image␞␟entity␜␜entity:file␝uri␞␟url',
               'Silly image 🤡 → File URI → Stream wrapper URI ' => 'ℹ︎␜entity:node:foo␝field_silly_image␞␟entity␜␜entity:file␝uri␞␟value',
               "Silly image 🤡" => 'ℹ︎␜entity:node:foo␝field_silly_image␞␟src_with_alternate_widths',
   

   What do you think?

2. That's an excellent addition for #3551339: Suggest only relevant DynamicPropSources — where I'd been waiting for exactly this kind of product decision 😊👍
3. Because "Authored by" is the label of the entity reference field pointing to the User entity, and it's the entity reference field itself that provides the new url property. To achieve Authored by => User => URL, we'd need to add a computed base field to all entity types, which is exactly the thing that is so problematic: likely clashes with contrib/custom modules, as @effulgentsia and I explained at #3545859-48: Add a `host-entity-url` prop source for linking to the host entity.

This is great, and useful, and clear as a bell. I'm shipping it with my newfound commit powers! Mwahahahahahaha!!!

Okay, looks like I can't merge this without it being approved by the correct code owners. Boo!

RTBC+1. I approved for the code groups that I'm a code owner of but that still leaves a couple groups that need approval.

Left two comments that I think can likely be followups given this is RTBC and critical

@phenaproxima & @effulgentsia: thanks both!

@larowlan in #24:

Left two comments that I think can likely be followups given this is RTBC and critical

Addressed both 😊 Keeping technical debt/issue queue sprawl under control by spending 15 mins addressing those here is worth it IMHO 👍

@lauriii in #25:

1. The code component editor is correct, it's just coarse. I think we need to expand the code component editor UI (and the JSON schema for props that code components are allowed to have) to allow for optional granularity. This is especially true for links. Because the hyperlink is literally the foundation of the web, and it's also literally what a CMS is about: managing content and relations between them. So it's important that the author of a code component can actually specify what kind of link they expect. Because only then can Canvas offer relevant links!

   This is what that UI currently looks like, annotated to convey what the coarse prop shapes are that you're allowed to pick:

   👉 Detailed proposal in new issue: #3556144: Code components "link" props can only be coarse, resulting in irrelevant shape matches when building `ContentTemplate`s

2. 👍
3. I believe here too labels could make all the difference. Instead of just "URL", I could change it to "Entity URL" or even something relevant to the target entity type: "User URL", "Content URL", etc.

This is now finally fully ready.

Note that manually testing this in a ContentTemplate when the referenced entity is inaccessible reveals that the resulting NULL value causes an SDC render crash.

The reason: \Drupal\Core\Theme\Component\ComponentValidator::validateProps() requires optional SDC props to be ABSENT or have a valid value. So what is NOT allowed is a value sometimes resolving to a a string (e.g. node 5's referenced entity is accessible ⇒ result = URL) and sometimes not (e.g. node 6's referenced entity being inaccessible ⇒ result = NULL).

We've already got #3541361: Find optional field instance matches for `type: object` props (images + videos), including for optional fields on bundleless entity types (e.g. `User`'s `user_picture`) for that, and that already has test infrastructure for it. So keeping that out of the scope here — this MR is already plenty complex as-is 😅

Gave this another look. The comments make fairly clear what's going on here, and the MaybeUrl value object makes sense as something we can adapt in the future if we run into a similar need for a "this could a useful value, or it could NULL because of an access problem" case. I appreciate the added clarity (in the class doc comment) about how Evaluator will handle a required value that fails to produce a value. These are really complex problems, and I hope that core will be able to improve the status quo over time, but for now...

...ship it.

Thanks! Now see y'all in #3541361: Find optional field instance matches for `type: object` props (images + videos), including for optional fields on bundleless entity types (e.g. `User`'s `user_picture`) per #27, in #3551339: Suggest only relevant DynamicPropSources for omitting irrelevant suggestions, and in #3551455: HostEntityUrlPropSource should be able to support absolute or relative URLs, URL options to make HostEntityUrlPropSource feature-complete and update-path-free.

#27 would also be alleviated if #3531905: Validation error on optional properties. lands

@penyaskito: #3531905-23: Validation error on optional properties.. 🏓

Verified below scenarios on 1.x :

Please check TC05, failed test case.

TC05 (selecting Canonical absolute URL) is explicitly not in scope of this issue.

The fix for that landed in the past hour: #3555068: Linking a `HostEntityUrlPropSource` to populate a `type: string, format: uri|uri-reference` in a `ContentTemplate` has no effect.