Add a `host-entity-url` prop source for linking to the host entity

This could've been avoided if we had #3543408. See #3543408-3: Allow content templates to contain adapted prop sources.

(Please help prevent issue sprawl of hard-to-connect issues! 🙏)

Per #3 😊 Looking forward to your MR!

I tried to manually tests this. I merged this with 1.x but I couldn't find a component that I could link a prop to this field. Is there an existing one? I enabled `sdc_test_all_props` but it didn't have a prop I could find either. Since this mainly for content templates it seems reasonable we should be able to manually test this before merging

Also is there a reason not to create an adapter instead of base field. Won't a base field have the side effect of it being able to used in other module UI's but an adapter won't?

Choices to evaluate

I was asked to check whether:

1. we want to pursue THIS MR's approach (new computed field)
2. or we want to pursue bringing a subset of AdaptedPropSources into Canvas after all (see #3543408: Allow content templates to contain adapted prop sources)
3. … or some other alternative

Evaluation

Of those:

• #1 is simplest — aka this MR is the simplest because it's using all existing infrastructure
• #3 one thing might be simpler still — rather than adding a new computed field, we could add a new computed canonical_url property to the "ID" entity key's field, which would return the /node/1 URL (without path processing), and canonical_url_alias (with path processing). That reduces chances of disruption for contrib, because the set of fields on every Node object remains exactly the same. (Contrib/custom modules are more likely to be iterating over all fields of an entity type+bundle than over the field properties on a particular field instance.)
• #2 would be nicest, because both #1 and #3 just exacerbate future update path pains: it causes even more data needing updating when we'll finally have adapters 🫣 (which is why I opened #3536115: Allow use of same-shape-adapters ahead of general adapter support in #3464003, but it never became a high enough priority for me to work on it 😭)

What if there's a 4th option?

What if … we added a new type of prop source, called HostEntityUrlPropSource? Because DynamicPropSource is about fetching fields/field properties (Typed Data, basically), whereas this is about calling $entity→toUrl('canonical')→setAbsolute(). There's prior art for generating special URLs dynamically, too: DefaultRelativeUrlPropSource

See https://git.drupalcode.org/project/canvas/-/merge_requests/199. WDYT? 😊

Code looks great. Tests pass. #19 confirms it addresses what Drupal CMS needs.

Thanks for that test coverage! 😊

But … why was this merged?!?! It was clearly not ready. I spent a few mins hacking something together sprinkled with many @todos, @phenaproxima added minimal test coverage, and … that's it.

1. Even
   

       // @todo if $content_entity_type_id has a `canonical` URL template, also offer `host-entity-url:absolute:canonical` as a choice. Something like:
       // $suggestions[] = (new HostEntityUrlPropSource())->asChoice();
   

   was merged as-is. 😅 Meaning no human can use it!

2. And there's many more @todos beyond that.
3. Doc updates are missing throughout.
4. \Drupal\Tests\canvas\Kernel\Config\ContentTemplateValidationTest::setUp() should've been updated to use this, to prove it works end-to-end
5. \Drupal\Tests\canvas\Kernel\Config\PatternValidationTest::testInvalidComponentTree() should've gotten a test case that tries to use it, to prove you can't use it in Pattern
6. Same for PageRegion

This was rushed in. 👎

Ah, I found now in private chat that #24.1 was skipped because:

@tedbow: but the issue summary doesn’t actually say whether the goal is to expose this in the UI. it just mentions handcrafting templates
@effulgentsia: No UI for this issue.

Which surprises me, but I guess it does mean Drupal CMS is unblocked 🚀

That does mean this issue should definitely be open again though, for A) tests, B) docs, C) updating our shape matching logic.

I think A + B could largely be handled by @phenaproxima. I am probably the only one who can do C.

Wouldn't the benefit of a computed URL field be that the URL would be available in JSON:API response too? It's currently pretty difficult to get the URL for a given entity from JSON:API. IMO it would make sense to provide it as an API there too.

IMO it would make sense to provide it as an API there too.

That'd be a core issue. It's not Canvas' responsibility to do this on behalf of all entities in all of Drupal.

#27:

1. 👍
2. 👍, and thanks for creating that follow-up issue!
3. Thanks!
4. 🙏 Thanks!
5. 🙏 Thanks!
6. 🙏 Thanks!

I'll get on reviewing this 👍

1. This is for sure the hard part, because FieldForComponentSuggester (as the name implies) is only designed for suggesting field data (field properties in fields on the entity, or from a referenced entity). But due to the MR that was merged here, it needs to be reorganized significantly, to allow for not only DynamicPropSources but also HostEntityUrlPropSources.

   It doesn't help that all of #3551455: HostEntityUrlPropSource should be able to support absolute or relative URLs, URL options was deferred to a follow-up: that just makes it harder to get everything right, because currently HostEntityUrlPropSource is de facto a one-trick pony, whereas it clearly will need to accept configurability in the future (even if only to be able to generate a relative OR an absolute URL, whereas currently it ALWAYS generates an absolute URL).

2. 👍 — but … ⚠️ since I wrote #24, @f.mazeikis pointed out an important flaw in (Static|Dynamic)PropSource, and the same flaw also applies to this new prop source: it's failing to bubble cacheability (an absolute URL varies by the url.site cache context) → #3554184: Bubble cacheability of resolved props values and access results + `PropSourceBase::evaluate()` does not return cacheability at all should tackle it for all prop sources at once
3. Lots of docs still needed updating. Fixed in https://git.drupalcode.org/project/canvas/-/merge_requests/222/diffs?com... + https://git.drupalcode.org/project/canvas/-/merge_requests/222/diffs?com...
4. End-to-end tests were still missing, added those in https://git.drupalcode.org/project/canvas/-/merge_requests/222/diffs?com...

I've got #1 WIP locally, but am currently constantly distracted by a very sick baby to take care of 😅 Still, progress is being made:

Merging, given @phenaproxima's +1.

Three follow-ups:

1. MR !222 causes the new choice to appear in the ContentTemplate UI, but clicking it has no effect, due to a bug in the UI — the child issues of #3541000: [META] Content templates UI for 1.0: only nodes, no exposed slots, no replacement for the view mode/display UI that built the "select a suggestion from the server-side provided list of suggestions" specifically was designed to have the client side treat the suggested sources as opaque JSON objects (see #/components/schemas/StructuredDataForPropShapeHierarchicalSuggestionPathWithChoice in /openapi.yml). Apparently the /ui code does not do that. Follow-up bug report filed: #3555068: Linking a `HostEntityUrlPropSource` to populate a `type: string, format: uri|uri-reference` in a `ContentTemplate` has no effect — until that is fixed, the choice will appear in the UI but clicking it won't do a thing:

   

2. Make this new prop source actually have flags/options/settings to configure what kind of URL to generate: #3551455: HostEntityUrlPropSource should be able to support absolute or relative URLs, URL options.
3. (long predates this issue, but this issue made that more obviously necessary) #3523446: Rename `FieldForComponentSuggester` to `PropSourceSuggester`

Fixing the title, because what the merged MR !199 landed is specifically not a permalink (because path processing is enabled by default, and path aliases are literally the opposite of permalinks — unless you install the contrib Redirect module) — see #3551455-3: HostEntityUrlPropSource should be able to support absolute or relative URLs, URL options.

Looks like @lauriii thinks this MR didn't go far enough: #3555413: Allow linking to referenced entities: add `url` property to `EntityReferenceItem::propertyDefinitions()`. Which in turn may call into question the entire premise of the new prop source. As I wrote in #15 here: aka this MR is the simplest because it's using all existing infrastructure — which given @lauriii's latest remark over there actually is literally what he expects.

Which is totally reasonable, but it would mean that a new prop source makes less sense than adding a computed base field to every entity type — aka what this issue originally did 😅

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?

$node->url->canonical->setAbsolute()->setOption('query', ['foo' => 'bar'])->toString()

Exactly!

1. Well, almost — we wouldn't be able to return a \Drupal\Core\Url object as a property directly. So the ->setAbsolute()->setOption('query', ['foo' => 'bar'])->toString() part is not realistic I think, because how would we know to call those methods? ⇒ it should be available as a field property directly, with no choices available. So:
   

   $node->url->canonical_absolute
   

   +

   $node->url->canonical_relative
   

   (with url the new base field on all entity types, added by Canvas, and canonical_absolute and canonical_relative the 2 field properties.

2. … and thanks to the static \Drupal\Core\Field\FieldItemInterface::propertyDefinitions(FieldStorageDefinitionInterface $field_definition) being able to determine the target entity type ID using the given field storage definition, it's possible to determine which link relations/templates exist for a given entity type, and hence expose also edit-form_absolute+<code>edit-form_relative, and so on. AKA what #3551455: HostEntityUrlPropSource should be able to support absolute or relative URLs, URL options was about.
   (Plus perhaps a special case for the canonical URL with + without path processing aka path aliases?)

Similar sentiment @phenaproxima here in #43 and from @penyaskito at #3555413-12: Allow linking to referenced entities: add `url` property to `EntityReferenceItem::propertyDefinitions()`.

⇒marking unfixed, and flagging the likely need for a revert

⚠️ Note that this will make any future update path towards adapters much more painful. Because anything that uses a Canvas-altered-in field property that SHOULD use an adapter would then need to be updated later.

A simpler, clearer example of that general problem is an SDC's need (e.g. #3547303: The hero-blog component's `date` prop needs to be a timestamp) for a type: string, format: date prop to be populated by a Node's created</em> field. That field contains a UNIX timestamp, which is a <code>type: integer. That was intended to be solved by \Drupal\canvas\Plugin\Adapter\UnixTimestampToDateAdapter, but adapters are out of scope for 1.0.

But @lauriii just told me in a meeting that he expects this to work in 1.0, despite that not being part of #3541000: [META] Content templates UI for 1.0: only nodes, no exposed slots, no replacement for the view mode/display UI. 😅😬

So we might just have to accept that such a painful update path will be necessary. Thanks to the ComponentAudit service, we'll know how to find affected component instances (per revision, even). But it'll mean we won't be able to drop whichever computed field properties (or fields) we add in Canvas 1.x until at least 2.x, and realistically until much later.

Is that something we're willing to commit to? For content templates, the update path wouldn't be too painful (at most dozens of entities to update), but for content entities it'd be very complex.

To give a sense of the complexity & scale of such an update path: essentially, we'd have to update potentially millions of rows (every component instance DB row that uses a computed field/field property added by Canvas) from something like:

{
"heading_text": "Sensational title!",
"date": 1770856866,
…
}

(👆 stored for a single component instance in its inputs column of the field table for a content entity)
+

expression: ℹ︎timestamp␟as_string_format_date

(👆 stored in the Component config entity)

to:

expression: ℹ︎timestamp␟value

+ a change to the per-component instance inputs column for every revision that uses it, to convey that this uses a static prop source + adapter, so something like:

{
"heading_text": "Sensational title!",
"date": {
  "sourceType": "adapter:day_count",
  "adapterInputs": {
    "unix": {
      "sourceType": "static",
      "expression": "ℹ︎timestamp␟value",
      "value": 1770856866
    }
  }
}

Discussed with @effulgentsia in detail.

I discussed with him the full range of possible implementation choices (they're listed in #3555413: Allow linking to referenced entities: add `url` property to `EntityReferenceItem::propertyDefinitions()`).

Alex independently proposed what I previously proposed at #3555413-5: Allow linking to referenced entities: add `url` property to `EntityReferenceItem::propertyDefinitions()`:

[…] #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.)

But he phrased it better than I did — this is our joint conclusion, which uses his phrasing:

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.)

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