[SPIKE] Prototype targeted approach for config-driven tabs [WHIT-3351]

[eYinka]

Tabs like Attachments, Images, and Featured are currently in legacy hardwired form and they're toggled by boolean flags in settings; then added to the nav by some if edition.allows_attachments? checks in the helper. The goal of this spike was to understand whether we could instead declare them in forms (alongside dynamic tabs like Social Media Accounts), making the JSON config the single source of truth for all tabs on a StandardEdition.

As part of WHIT-3351, I've made a quick prototype on how we can make these legacy tabs (Featured, Images, Attachements) expressed in the config-driven way; maintaining a minimal approach to support existing code.

In this approach, I have introduced "linked_to": "foo" in the forms object to declare a tab that routes to a separate controller rather than rendering inline.

I struggled with the naming here. Had to decide between linked_tabs and external_tabs. I thought linked_tabs will communicate the idea more effectively.

Key things I learned:

• The targeted approach works well with the dynamic tab (Understood as in-line tabs) pipeline with minimal changes and no new abstractions
• Linked tabs don't need valid_tab_key? changes, since they never submit via ?current_tab=, so scoped validation is not a concern for them. Or not?
• We can safely (I guess) get rid of settings.file_attachments_enabled in a configurable document type.

Note: The type.settings["..."] == true fallback on the Attachments line is intentional for transition purposes. Once all document type configs have been updated to declare their tabs via linked_to, the settings flags (file_attachments_enabled, features_enabled, etc) and the || fallbacks can be removed from the presenter and StandardEdition model methods.

To add other tabs, follow the approach in news_story.json. For example, to add features tab to News Story:

  "forms": {
    "documents": {
      "fields": { ... }
    },
    "attachments": {
      "label": "Attachments",
      "linked_to": "edition_attachments"
    },
    "features": {
      "label": "Features",
      "linked_to": "edition_features"
    },
  }

Then update the presenter: app/presenters/publishing_api/standard_edition_presenter.rb:

Existing code:
details.merge!(PayloadBuilder::Features.for(item)) if type.settings["features_enabled"] == true

Updated code:
details.merge!(PayloadBuilder::Features.for(item)) if type.settings["features_enabled"] == true || type.has_linked_tab?("edition_features")

TODO:

• Run test suites to catch edge cases
• Thorough testing on the UI

JIRA

---

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

This application is owned by the Whitehall Experience team. Please let us know in #govuk-whitehall-experience-tech when you raise any PRs.

Follow these steps if you are doing a Rails upgrade.

[ChrisBAshton]

I really like this approach! Think it's a solid improvement, and minimally invasive code-wise 🎉 Nice job ⭐

To answer some points in your PR:

Linked tabs don't need valid_tab_key? changes, since they never submit via ?current_tab=, so scoped validation is not a concern for them. Or not?

I think that's correct, yes. The hardcoded tabs basically already have their own validation scope.

In this approach, I have introduced "linked": true

Have you? I only see "linked_to": "<kind>"

Btw, I can see us skipping the fallback you've added and just migrating all configurable content type schemas en-masse.

[ChrisBAshton]

I think you're probably right that we can remove this. Just needs double-checking.

The presence of the Attachments tab doesn't fully describe the attachments behaviour we want for the content type: historically we sometimes allow HtmlAttachment, sometimes not, sometimes allow InlineAttachment, sometimes not, etc. I know file_attachments_enabled doesn't cover all that either, but we may still need some sort of toggling in the settings hash.

[eYinka]

Agreed. We will probably have to do something like this later on:

  "settings": { 
    "attachments": { 
      "file": true, 
      "html": false 
    } 
  }

[ChrisBAshton]

I can see why you went for 'linked tab', and why you considered 'external tab'.

Tbh I think 'hardcoded tab' is more expressive here (since they are!).

[eYinka]

Fairs

[eYinka]

@ChrisBAshton

Have you? I only see "linked_to": ""

My bad. I refactored to get rid of the linked property as it was redundant. Will update the description.