diff --git a/app/controllers/organisation_brands_controller.rb b/app/controllers/organisation_brands_controller.rb index fc3b90348..43c3349ba 100644 --- a/app/controllers/organisation_brands_controller.rb +++ b/app/controllers/organisation_brands_controller.rb @@ -23,11 +23,31 @@ def destroy authorize organisation, :can_manage_organisation_brands? organisation_brand = organisation.organisation_brands.find_by!(brand_id: params[:id]) + + if organisation.default_brand_id == organisation_brand.brand_id + return redirect_to organisation_path(organisation), status: :see_other + end + organisation_brand.destroy! redirect_to organisation_path(organisation), success: t(".success", brand_name: organisation_brand.brand.name) end + def update_default + authorize organisation, :can_manage_organisation_brands? + + if params[:brand_id].present? + brand = organisation.brands.find(params[:brand_id]) + organisation.update!(default_brand: brand) + + redirect_to organisation_path(organisation), success: t(".success", brand_name: brand.name) + else + organisation.update!(default_brand: nil) + + redirect_to organisation_path(organisation), success: t(".success_govuk") + end + end + private def organisation diff --git a/app/models/organisation.rb b/app/models/organisation.rb index b649043a5..15de4899f 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -11,6 +11,10 @@ class Organisation < ApplicationRecord has_many :organisation_brands, dependent: :destroy has_many :brands, -> { order(:name) }, through: :organisation_brands + belongs_to :default_brand, class_name: "Brand", optional: true + + validate :default_brand_is_available, if: :default_brand_id_changed? + scope :not_closed, -> { where(closed: false) } scope :with_users, -> { joins(:users).distinct.order(:name) } @@ -73,4 +77,13 @@ def as_json(options = {}) options[:methods] ||= %i[organisation_admin_users] super(options) end + +private + + def default_brand_is_available + return if default_brand_id.nil? + return if organisation_brands.exists?(brand_id: default_brand_id) + + errors.add(:default_brand, :not_available) + end end diff --git a/app/services/create_form_service.rb b/app/services/create_form_service.rb index ab967b7b6..edee0b606 100644 --- a/app/services/create_form_service.rb +++ b/app/services/create_form_service.rb @@ -34,11 +34,19 @@ def create!(creator:, group:, name:) if event.form_id.present? form = Form.find(event.form_id) else - form = Form.create!(creator_id: creator.id, name:) + form = Form.create!(creator_id: creator.id, name:, brand_id: default_brand_id(group)) GroupForm.create!(group:, form_id: form.id) event.update!(form_id: form.id) end form end + +private + + def default_brand_id(group) + return nil unless FeatureService.new(group:).enabled?(:custom_branding) + + group.organisation.default_brand&.slug + end end diff --git a/app/views/organisations/show.html.erb b/app/views/organisations/show.html.erb index d73011892..e5eeb3fa2 100644 --- a/app/views/organisations/show.html.erb +++ b/app/views/organisations/show.html.erb @@ -113,30 +113,51 @@
<%= t("organisations.show.brands.guidance") %>
- <% if @organisation.brands.any? %> - <%= render ScrollingWrapperComponent::View.new(aria_label: t("organisations.show.brands.heading")) do %> - <%= govuk_table do |table| %> - <%= table.with_head do |head| - head.with_row do |row| - row.with_cell(header: true, text: t("organisations.show.brands.table_headings.name")) - row.with_cell(header: true, text: t("organisations.show.brands.table_headings.actions")) - end - end %> - - <%= table.with_body do |body| %> - <% @organisation.brands.each do |brand| %> - <%= body.with_row do |row| %> - <%= row.with_cell { govuk_link_to(brand.name, brand_path(brand)) } %> - <%= row.with_cell do %> - <%= govuk_button_to(t("organisations.show.brands.remove"), organisation_brand_path(@organisation, brand), method: :delete, secondary: true) %> + <%= render ScrollingWrapperComponent::View.new(aria_label: t("organisations.show.brands.heading")) do %> + <%= govuk_table do |table| %> + <%= table.with_head do |head| + head.with_row do |row| + row.with_cell(header: true, text: t("organisations.show.brands.table_headings.name")) + row.with_cell(header: true, text: t("organisations.show.brands.table_headings.actions")) + end + end %> + + <%= table.with_body do |body| %> + <%= body.with_row do |row| %> + <%= row.with_cell do %> + <%= t("organisations.show.brands.govuk_brand_name") %> + <% if @organisation.default_brand_id.nil? %> + <%= govuk_tag(text: t("organisations.show.brands.default_tag"), colour: "green") %> + <% end %> + <% end %> + <%= row.with_cell do %> + <% if @organisation.default_brand_id.present? %> + <%= govuk_button_to(t("organisations.show.brands.make_default"), organisation_default_brand_path(@organisation), method: :patch, secondary: true, visually_hidden_suffix: t("organisations.show.brands.govuk_brand_name"), class: "govuk-!-margin-bottom-0") %> + <% end %> + <% end %> + <% end %> + <% @organisation.brands.each do |brand| %> + <%= body.with_row do |row| %> + <%= row.with_cell do %> + <%= govuk_link_to(brand.name, brand_path(brand)) %> + <% if brand.id == @organisation.default_brand_id %> + <%= govuk_tag(text: t("organisations.show.brands.default_tag"), colour: "green") %> + <% end %> + <% end %> + <%= row.with_cell do %> + <% if brand.id == @organisation.default_brand_id %> + <%= govuk_button_to(t("organisations.show.brands.remove"), organisation_brand_path(@organisation, brand), method: :delete, secondary: true, disabled: true, visually_hidden_suffix: brand.name, class: "govuk-!-margin-bottom-0") %> + <% else %> + <% end %> <% end %> <% end %> <% end %> <% end %> <% end %> - <% else %> -<%= t("organisations.show.brands.none") %>
<% end %> <%= govuk_button_link_to t("organisations.show.brands.add"), new_organisation_brand_path(@organisation), secondary: true %> diff --git a/config/locales/en.yml b/config/locales/en.yml index dac8e82d6..8ffd10430 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -176,6 +176,10 @@ en: agreed: crown: You must agree to the memorandum of understanding non_crown: You must agree to the agreement to continue + organisation: + attributes: + default_brand: + not_available: The default brand must be one of the organisation’s available brands organisation_domain: attributes: domain: @@ -1501,6 +1505,9 @@ en: prompt: Select a brand submit: Add brand title: Add a brand + update_default: + success: "%{brand_name} is now the default brand for new forms in this organisation" + success_govuk: New forms in this organisation will use GOV.UK branding organisations: boolean: 'false': 'No' @@ -1545,9 +1552,11 @@ en: none: This organisation does not have any organisation admins. brands: add: Add a brand - guidance: Add brands to make them available for forms in this organisation to use. Forms that already use a brand will continue to use it, even if you remove the brand from this organisation. + default_tag: Default + govuk_brand_name: GOV.UK + guidance: Add brands to make them available for forms in this organisation to use. New forms will use the default brand, and people editing a form can change its brand. Forms that already use a brand will continue to use it, even if you remove the brand from this organisation. heading: Available brands - none: This organisation does not have any available brands. Forms use the default GOV.UK branding. + make_default: Make default remove: Remove table_headings: actions: Actions diff --git a/config/routes.rb b/config/routes.rb index 6cf6a3baf..28d0d5024 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -219,6 +219,7 @@ resources :organisations, only: %i[index show] do resources :brands, controller: :organisation_brands, only: %i[new create destroy] + patch "default-brand", to: "organisation_brands#update_default", as: :default_brand end resources :brands, only: %i[index show new create edit update] diff --git a/db/migrate/20260715122206_add_default_brand_to_organisations.rb b/db/migrate/20260715122206_add_default_brand_to_organisations.rb new file mode 100644 index 000000000..3e56b4516 --- /dev/null +++ b/db/migrate/20260715122206_add_default_brand_to_organisations.rb @@ -0,0 +1,5 @@ +class AddDefaultBrandToOrganisations < ActiveRecord::Migration[8.1] + def change + add_reference :organisations, :default_brand, null: true, foreign_key: { to_table: :brands } + end +end diff --git a/db/schema.rb b/db/schema.rb index bbea686ba..36d8e4c63 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_07_14_090000) do +ActiveRecord::Schema[8.1].define(version: 2026_07_15_122206) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" @@ -255,11 +255,13 @@ t.string "abbreviation" t.boolean "closed", default: false t.datetime "created_at", null: false + t.bigint "default_brand_id" t.string "govuk_content_id" t.boolean "internal", default: false t.string "name", null: false t.string "slug", null: false t.datetime "updated_at", null: false + t.index ["default_brand_id"], name: "index_organisations_on_default_brand_id" t.index ["govuk_content_id"], name: "index_organisations_on_govuk_content_id", unique: true t.index ["slug"], name: "index_organisations_on_slug", unique: true end @@ -355,6 +357,7 @@ add_foreign_key "organisation_brands", "brands" add_foreign_key "organisation_brands", "organisations" add_foreign_key "organisation_domains", "organisations" + add_foreign_key "organisations", "brands", column: "default_brand_id" add_foreign_key "page_translations", "pages" add_foreign_key "pages", "forms" add_foreign_key "users", "organisations" diff --git a/spec/models/organisation_spec.rb b/spec/models/organisation_spec.rb index 92edd9614..87e75be39 100644 --- a/spec/models/organisation_spec.rb +++ b/spec/models/organisation_spec.rb @@ -15,6 +15,43 @@ end end + describe "default brand" do + let(:organisation) { create :organisation } + let(:brand) { create :brand } + + it "is valid when the default brand is one of the organisation's brands" do + create(:organisation_brand, organisation:, brand:) + + organisation.default_brand = brand + + expect(organisation).to be_valid + end + + it "is invalid when the default brand is not one of the organisation's brands" do + organisation.default_brand = brand + + expect(organisation).to be_invalid + expect(organisation.errors.of_kind?(:default_brand, :not_available)).to be true + end + + it "is valid to clear the default brand" do + create(:organisation_brand, organisation:, brand:) + organisation.update!(default_brand: brand) + + organisation.default_brand = nil + + expect(organisation).to be_valid + end + + it "does not validate the default brand unless it is changing" do + create(:organisation_brand, organisation:, brand:) + organisation.update!(default_brand: brand) + organisation.organisation_brands.delete_all + + expect { organisation.update!(name: "Renamed Organisation") }.not_to raise_error + end + end + describe "scopes" do describe ".with_users" do it "returns organisations with distinct users" do diff --git a/spec/requests/organisation_brands_controller_spec.rb b/spec/requests/organisation_brands_controller_spec.rb index 96f17af8b..eb56a376b 100644 --- a/spec/requests/organisation_brands_controller_spec.rb +++ b/spec/requests/organisation_brands_controller_spec.rb @@ -192,6 +192,103 @@ expect(response).to have_http_status(:not_found) end end + + context "when the brand is the organisation's default brand" do + before do + organisation.update!(default_brand: brand) + end + + it "does not remove the brand from the organisation" do + expect { + delete path + }.not_to(change { organisation.brands.count }) + end + + it "redirects to the organisation page without a success message" do + delete path + + expect(response).to redirect_to(organisation_path(organisation)) + expect(flash[:success]).to be_nil + end + end + end + end + + describe "#update_default" do + let(:path) { organisation_default_brand_path(organisation) } + let(:params) { { brand_id: brand.id } } + + before do + create :organisation_brand, organisation:, brand: + end + + context "when the user is not a super admin" do + before do + login_as_standard_user + + patch(path, params:) + end + + it "returns http code 403 and renders forbidden" do + expect(response).to have_http_status(:forbidden) + expect(response).to render_template("errors/forbidden") + end + + it "does not change the organisation's default brand" do + expect(organisation.reload.default_brand).to be_nil + end + end + + context "when the user is a super admin" do + before do + login_as_super_admin_user + end + + it "sets the organisation's default brand" do + patch(path, params:) + + expect(organisation.reload.default_brand).to eq(brand) + end + + it "redirects to the organisation page with a success message" do + patch(path, params:) + + expect(response).to redirect_to(organisation_path(organisation)) + expect(flash[:success]).to eq(I18n.t("organisation_brands.update_default.success", brand_name: brand.name)) + end + + context "when no brand is given" do + let(:params) { {} } + + before do + organisation.update!(default_brand: brand) + end + + it "clears the organisation's default brand" do + patch(path, params:) + + expect(organisation.reload.default_brand).to be_nil + end + + it "redirects to the organisation page with a success message" do + patch(path, params:) + + expect(response).to redirect_to(organisation_path(organisation)) + expect(flash[:success]).to eq(I18n.t("organisation_brands.update_default.success_govuk")) + end + end + + context "when the brand is not one of the organisation's brands" do + let(:other_brand) { create :brand } + let(:params) { { brand_id: other_brand.id } } + + it "returns http code 404 and does not change the default brand" do + patch(path, params:) + + expect(response).to have_http_status(:not_found) + expect(organisation.reload.default_brand).to be_nil + end + end end end end diff --git a/spec/services/create_form_service_spec.rb b/spec/services/create_form_service_spec.rb index edd8405e9..3af212d22 100644 --- a/spec/services/create_form_service_spec.rb +++ b/spec/services/create_form_service_spec.rb @@ -24,6 +24,43 @@ expect(GroupForm.last).to have_attributes(form_id: Form.last.id, group_id: 1000) end + context "when the group's organisation has a default brand" do + let(:organisation) { create :organisation } + let(:brand) { create :brand } + let(:group) { build :group, id: 1000, organisation:, custom_branding_enabled: true } + + before do + create(:organisation_brand, organisation:, brand:) + organisation.update!(default_brand: brand) + end + + it "creates the form with the default brand" do + create_form_service.create!(creator:, group:, name:) + + expect(Form.last.brand_id).to eq brand.slug + end + + context "when the group does not have custom branding enabled" do + let(:group) { build :group, id: 1000, organisation:, custom_branding_enabled: false } + + it "creates the form without a brand" do + create_form_service.create!(creator:, group:, name:) + + expect(Form.last.brand_id).to be_nil + end + end + end + + context "when the group's organisation has no default brand" do + let(:group) { build :group, id: 1000, organisation: creator.organisation, custom_branding_enabled: true } + + it "creates the form without a brand" do + create_form_service.create!(creator:, group:, name:) + + expect(Form.last.brand_id).to be_nil + end + end + context "when a form with that name was already created in that group" do before do allow(Form).to receive(:create!).and_call_original diff --git a/spec/views/organisations/show.html.erb_spec.rb b/spec/views/organisations/show.html.erb_spec.rb index 0106054a1..a7687bd19 100644 --- a/spec/views/organisations/show.html.erb_spec.rb +++ b/spec/views/organisations/show.html.erb_spec.rb @@ -73,7 +73,6 @@ expect(rendered).to have_text(I18n.t("organisations.show.admin_users.none")) expect(rendered).to have_text(I18n.t("organisations.show.mou_signatures.none")) expect(rendered).to have_text(I18n.t("organisations.show.domains.none")) - expect(rendered).to have_text(I18n.t("organisations.show.brands.none")) end it "shows brands guidance when there are no brands" do @@ -90,4 +89,74 @@ expect(rendered).to have_css(".govuk-summary-list__value", text: I18n.t("organisations.show.not_set")) end end + + describe "available brands" do + context "when the organisation has no brands" do + it "shows the GOV.UK brand with the default tag" do + expect(rendered).to have_css("td", text: /GOV\.UK\s+Default/) + end + + it "does not show a make default button" do + expect(rendered).not_to have_button(I18n.t("organisations.show.brands.make_default")) + end + end + + context "when the organisation has brands and no default brand" do + let(:brand) { create :brand, name: "Cheshire East Council" } + + let(:organisation) do + create(:organisation, :with_org_admin, slug: "department-for-testing").tap do |organisation| + create(:organisation_brand, organisation:, brand:) + end + end + + it "shows the default tag against the GOV.UK brand" do + expect(rendered).to have_css("td", text: /GOV\.UK\s+Default/) + expect(rendered).to have_css("td", text: /^\s*#{brand.name}\s*$/) + end + + it "shows a make default button for the brand" do + expect(rendered).to have_css("form[action='#{organisation_default_brand_path(organisation)}'] button", text: I18n.t("organisations.show.brands.make_default")) + expect(rendered).to have_css("input[name='brand_id'][value='#{brand.id}']", visible: :hidden) + end + + it "shows an enabled remove button for the brand" do + expect(rendered).to have_css("form[action='#{organisation_brand_path(organisation, brand)}'] button:not([disabled])", text: I18n.t("organisations.show.brands.remove")) + end + end + + context "when the organisation has a default brand" do + let(:brand) { create :brand, name: "Cheshire East Council" } + let(:other_brand) { create :brand, name: "Aberdeenshire Council" } + + let(:organisation) do + create(:organisation, :with_org_admin, slug: "department-for-testing").tap do |organisation| + create(:organisation_brand, organisation:, brand:) + create(:organisation_brand, organisation:, brand: other_brand) + organisation.update!(default_brand: brand) + end + end + + it "shows the default tag against the default brand only" do + expect(rendered).to have_css("td", text: /#{brand.name}\s+Default/) + expect(rendered).to have_css("td", text: /^\s*GOV\.UK\s*$/) + end + + it "disables the remove button for the default brand" do + expect(rendered).to have_css("form[action='#{organisation_brand_path(organisation, brand)}'] button[disabled]", text: I18n.t("organisations.show.brands.remove")) + end + + it "does not show a make default button for the default brand" do + expect(rendered).not_to have_css("form[action='#{organisation_default_brand_path(organisation)}'] input[name='brand_id'][value='#{brand.id}']", visible: :hidden) + end + + it "shows a make default button for the GOV.UK brand" do + expect(rendered).to have_css("form[action='#{organisation_default_brand_path(organisation)}'] button", text: /#{I18n.t('organisations.show.brands.make_default')}\s+GOV\.UK/) + end + + it "shows a make default button for the other brand" do + expect(rendered).to have_css("input[name='brand_id'][value='#{other_brand.id}']", visible: :hidden) + end + end + end end