Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
eYinka wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions app/helpers/admin/tabbed_nav_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ def config_driven_nav_items(edition, current_path, current_tab)
current: on_dynamic_tab && current_tab == tab["id"],
}
end
nav_items.concat(linked_tabs_nav_items(edition, current_path))
end
end

Expand Down Expand Up @@ -216,4 +217,20 @@ def worldwide_organisation_page_nav_items(page, current_path)
},
]
end

def linked_tabs_nav_items(edition, current_path)
edition.type_instance.linked_tabs.map do |tab|
href = href_for_linked_tab(tab, edition)
{ label: tab["label"], href:, current: current_path == href }
end
end

def href_for_linked_tab(tab, edition)
Copy link
Copy Markdown
Contributor

@ChrisBAshton ChrisBAshton May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

@eYinka eYinka May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fairs

builders = {
"edition_attachments" => -> { admin_edition_attachments_path(edition) },
"edition_images" => -> { admin_edition_images_path(edition) },
"edition_features" => -> { features_admin_standard_edition_path(edition, locale: edition.primary_locale) },
}
builders[tab["linked_to"]]&.call
end
end
10 changes: 10 additions & 0 deletions app/models/configurable_document_type.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,16 @@ def dynamic_tabs
end
end

def linked_tabs
@linked_tabs ||= @forms.filter_map do |id, form|
{ "id" => id, "label" => form["label"], "linked_to" => form["linked_to"] } if form["linked_to"]
end
end

def has_linked_tab?(tab_key)
linked_tabs.any? { |tab| tab["linked_to"] == tab_key }
end

def schema_for_fields(field_keys)
field_keys = field_keys.map(&:to_s)

Expand Down
5 changes: 4 additions & 1 deletion app/models/configurable_document_types/news_story.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@
"translatable": false
}
}
},
"attachments": {
"label": "Attachments",
"linked_to": "edition_attachments"
}
},
"schema": {
Expand Down Expand Up @@ -122,7 +126,6 @@
}
},
"send_change_history": true,
"file_attachments_enabled": true,
Copy link
Copy Markdown
Contributor

@ChrisBAshton ChrisBAshton May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

@eYinka eYinka May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

"organisations": null,
"backdating_enabled": true,
"history_mode_enabled": true,
Expand Down
2 changes: 1 addition & 1 deletion app/presenters/publishing_api/standard_edition_presenter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def details
details = PayloadBuilder::BlockContent.for(item)
details.merge!(PayloadBuilder::ChangeHistory.for(item)) if type.settings["send_change_history"] == true
details.merge!(PayloadBuilder::PoliticalDetails.for(item)) if type.settings["history_mode_enabled"] == true
details.merge!(PayloadBuilder::Attachments.for(item)) if type.settings["file_attachments_enabled"] == true
details.merge!(PayloadBuilder::Attachments.for(item)) if type.settings["file_attachments_enabled"] == true || type.has_linked_tab?("edition_attachments")
details.merge!(PayloadBuilder::EmphasisedOrganisations.for(item)) if item.organisation_association_enabled?
details.merge!(PayloadBuilder::StandardEditionImages.for(item)) if type.settings.dig("images", "enabled")
details.merge!(PayloadBuilder::Features.for(item)) if type.settings["features_enabled"] == true
Expand Down
6 changes: 5 additions & 1 deletion app/validators/schema_validator.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,11 @@ def required_form_fields
end

def form_fields
(@document["forms"] || [])&.keys&.flat_map { |key| obj_dig(@document, "fields", ["forms", key]) }
(@document["forms"] || [])&.keys&.flat_map do |key|
next [] if @document.dig("forms", key, "linked_to") # linked tabs have no block content fields, so skip it

obj_dig(@document, "fields", ["forms", key])
end
end

def obj_dig(obj, attr, keys, &visitor)
Expand Down
17 changes: 14 additions & 3 deletions public/configurable-document-type.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,6 @@
},
"additionalProperties": {
"type": "object",
"required": ["fields"],
"description": "Definition of a group of form fields to group.",
"properties": {
"dynamic": {
Expand All @@ -262,8 +261,21 @@
"$ref": "#/$defs/form_field"
}
}
},
"linked_to": {
"type": "string",
"description": "Named route this tab links to. Mutually exclusive with fields. Used for tabs whose UI is owned by an existing controller (attachments, features, images).",
"enum": [
"edition_attachments",
"edition_features",
"edition_images"
]
}
}
},
"oneOf": [
{ "required": ["fields"] },
{ "required": ["linked_to"] }
]
}
},
"presenters": {
Expand Down Expand Up @@ -471,7 +483,6 @@
"organisations",
"backdating_enabled",
"history_mode_enabled",
"file_attachments_enabled",
"translations_enabled"
]
}
Expand Down
Loading