Allow embedding of content within document body

[callumknights]

This allows content items to be embedded within other content, using the syntax {{embed:CONTENT_TYPE:UUID}} in the body field.

There are a number of assumptions here:

• The content being embedded must exist in a live state in the same language as the content it is being embedded within.
• Only the title will currently be embedded. Future work will extend to include other fields.
• Only the body fields can contain embedded content. Future work will extend to include other fields.
• Only documents of type contact can be embedded. This can be extended by updating the SUPPORTED_DOCUMENT_TYPES constant.

This is based on the spike by @pezholio in #2794.

Trello card

[richardTowers]

There's a bit of an n+1 here I think. I'd expect this to make:

• 1 query to get links.where(link_type: "embded")
• n queries to get target documents for each link
• n queries to get the live edition for each target document

This is probably fine for now, as n probably won't be large (as I don't expect documents to have that more than a few embeds).

I think you could do it in one query that gets all the possible locales, and then find the correct locales in ruby.

Something like:

edition_ids = @edition
  .links
  .where(link_type: "embed")
  .joins(target_documents: :live)
  .where(target_documents: { locale: locales })
  .select("editions.id")
# Note ^ the use of select rather than pluck here means this is still
# an `ActiveRecord_AssociationRelation`, rather than an array of ids,
# so it won't make a separate SQL query
  
embedded_editions = Edition.where(id: edition_ids)
# Generates:
# SELECT "editions".* FROM "editions"
# WHERE "editions"."id" IN (
#   SELECT "editions"."id" FROM "links"
#   INNER JOIN "documents" "target_documents"
#     ON "target_documents"."content_id" = "links"."target_content_id"
#   INNER JOIN "editions"
#     ON "editions"."content_store" = 'live'
#     AND "editions"."document_id" = "target_documents"."id"
#   WHERE "links"."edition_id" = 1
#   AND "links"."link_type" = 'embed'
#   AND "target_documents"."locale" IN ('cy', 'en')
# )

# Exercise for the reader: we'll have multiple editions where there are multiple locales. Get the best edition for the locales we want.

[richardTowers]

Alternatively, you could use the same trick as we do here:

publishing-api/app/queries/get_edition_ids_with_fallbacks.rb

Line 14 in c3bfff2

.order(order_by_clause("documents", "locale", locale_fallback_order))

which is complicated but presumably works...

Could even use Queries::GetEditionIdsWithFallbacks directly I guess, but you might end up making one or two unnecessary queries there.

[brucebolt]

I've gone for the approach of using Queries::GetEditionIdsWithFallbacks directly, as that seems to be the least complicated and DRY-est way of getting the embedded editions.

I think that approach should also make it easier for us to deal with embedding draft content (should we wish to) in the future.

[richardTowers]

I guess this means that we won't expand embed references inside linked body elements, so we might end up with things like:

{
  content_type: "person",
  # ...
  links: {
    role: [{
      content_type: "role",
      # ...
      details: {
        body: "Some text where we haven't expanded an embed: {{embed:contact:00000000-0000-0000-0000-000000000000}}"
        # ...
      }
    }]
  }
}

I think that's fine for now, maybe we'll revisit it later.

[brucebolt]

That'a a good spot and a consequence I hadn't considered. It seems easy enough to expand the embedded content in the links, so we might as well do it now. Have added this as a separate commit.

[Gweaton]

This makes sense to me - have left a few comments re: naming, documentation and testing but I like the approach here.

[richardTowers]

A couple of nitpicks, and I'm afraid one actual issue - edition.title is not the value we want for the replacements.

I'm inclined to merge this even with this issue, as it won't cause any harm since the feature won't be used. We'll need to have a bit of a discussion about the best way forward on how to render the HTML for contact embeds I think.

[richardTowers]

nit: could do

Suggested change

	locale_fallback_order: [@edition.locale, Edition::DEFAULT_LOCALE],
	locale_fallback_order: [@edition.locale, Edition::DEFAULT_LOCALE].uniq,

here to avoid passing in %w[en en] in the common case where these are both en (although I presume that wouldn't matter at all)

[brucebolt]

Good spot, I'll use uniq here.

[richardTowers]

nit: I think you could do

Suggested change

	.map(&:target_content_id)
	.pluck(:target_content_id)

which would avoid allocating Link objects for each link and then calling #target_content_id on each one (instead just returning the content ids directly from the SQL query.

[brucebolt]

Nice, will update this.

[richardTowers]

Ah, I think this is actually not the right field on contact to be replacing things with. title will be something like "Government Digital Service", rather than "10 Whitechapel High Street" like we probably want.

The way Whitehall does it is:

• Render the contacts/contact partial
• Which is actually proper complicated
• Complicated further by the fact that the code to render addresses is itself very complicated

🤔 we could merge this with it just using title, but it's probably not actually what we want. On the other hand, I don't think publishing-api having a bunch of knowledge of how to render addresses is desirable - I'd be somewhat resistant to copying the templating code over from Whitehall.

I think the simplest option I can think of would be to add an "embeddable_html" field to Contact when Whitehall presents Contacts to publishing-api, and then use that. That way Whitehall can continue doing the rendering of Contact HTML (even though that's not very nice either), and publishing-api doesn't need to get rendering app capabilities.

Longer term with the design of this feature, I wonder if this is a case for passing the {{embed:...}} syntax all the way through to the frontend, and having the rendering app do the rendering of contact data into HTML (having followed the expanded link).

I'm sort of leaning towards getting this PR merged and then fixing this in a subsequent PR though.

[pezholio]

I like the embeddable_html idea. I struggled with this option in my initial spike too. I agree that we probably should have the rendering app do the actual rendering, but I imagine this will be tricky.

[brucebolt]

We'll leave this as title for now (on the basis that no-one will actually be embedding anything into live content) but will write up a card to implement embedded_html in the schemas and include that in the document instead.

Longer term with the design of this feature, I wonder if this is a case for passing the {{embed:...}} syntax all the way through to the frontend, and having the rendering app do the rendering of contact data into HTML (having followed the expanded link).

This makes a lot of sense to me. Particularly as the frontend app is better placed to do any conversion to HTML (rather than the current situation of the publishing app or Publishing API doing this, then any frontend changes needing all content to be republished).