<%= t("organisations.show.domains.none") %>
<% end %> + +<%= 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) %> + <% 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 745c769b7..f1c676704 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1491,6 +1491,16 @@ en: If you pasted the web address, check you copied the entire address. title: Page not found + organisation_brands: + create: + success: "%{brand_name} has been added to this organisation’s brands" + destroy: + success: "%{brand_name} has been removed from this organisation’s brands" + new: + label: Brand + prompt: Select a brand + submit: Add brand + title: Add a brand organisations: boolean: 'false': 'No' @@ -1531,6 +1541,15 @@ en: admin_users: heading: Organisation admins 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. + heading: Available brands + none: This organisation does not have any available brands. Forms use the default GOV.UK branding. + remove: Remove + table_headings: + actions: Actions + name: Name domains: heading: Email domains none: This organisation does not have any email domains. diff --git a/config/locales/input_objects/organisation_brand.yml b/config/locales/input_objects/organisation_brand.yml new file mode 100644 index 000000000..c7ccb08c0 --- /dev/null +++ b/config/locales/input_objects/organisation_brand.yml @@ -0,0 +1,10 @@ +--- +en: + activemodel: + errors: + models: + organisations/brand_input: + attributes: + brand_id: + blank: Select a brand + inclusion: Select a brand from the list of available brands diff --git a/config/routes.rb b/config/routes.rb index bc58cd57e..6cf6a3baf 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -217,7 +217,9 @@ resources :mou_signatures, only: %i[index], path: "mous" - resources :organisations, only: %i[index show] + resources :organisations, only: %i[index show] do + resources :brands, controller: :organisation_brands, only: %i[new create destroy] + end resources :brands, only: %i[index show new create edit update] diff --git a/config/settings.yml b/config/settings.yml index 832b978cc..64913349e 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -9,15 +9,6 @@ features: send_filler_answers: enabled_by_group: true -# Brands that a form can be styled with in forms-runner. A form with no -# brand_id uses the default GOV.UK branding. -branding: - available_brands: - - id: cheshire-east - name: Cheshire East Council - - id: south-gloucestershire - name: South Gloucestershire Council - forms_api: # Authentication key to authenticate with forms-api auth_key: development_key diff --git a/db/migrate/20260714090000_create_organisation_brands.rb b/db/migrate/20260714090000_create_organisation_brands.rb new file mode 100644 index 000000000..4b382bcac --- /dev/null +++ b/db/migrate/20260714090000_create_organisation_brands.rb @@ -0,0 +1,11 @@ +class CreateOrganisationBrands < ActiveRecord::Migration[8.1] + def change + create_table :organisation_brands do |t| + t.references :organisation, null: false, foreign_key: true + t.references :brand, null: false, foreign_key: true + t.timestamps + end + + add_index :organisation_brands, %i[organisation_id brand_id], unique: true, name: "index_organisation_brands_unique" + end +end diff --git a/db/schema.rb b/db/schema.rb index a419bb51d..bbea686ba 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_10_080000) do +ActiveRecord::Schema[8.1].define(version: 2026_07_14_090000) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" @@ -231,6 +231,16 @@ t.index ["user_id"], name: "index_mou_signatures_on_user_id_unique_without_organisation_id", unique: true, where: "(organisation_id IS NULL)", comment: "Users can only sign a single MOU without an organisation" end + create_table "organisation_brands", force: :cascade do |t| + t.bigint "brand_id", null: false + t.datetime "created_at", null: false + t.bigint "organisation_id", null: false + t.datetime "updated_at", null: false + t.index ["brand_id"], name: "index_organisation_brands_on_brand_id" + t.index ["organisation_id", "brand_id"], name: "index_organisation_brands_unique", unique: true + t.index ["organisation_id"], name: "index_organisation_brands_on_organisation_id" + end + create_table "organisation_domains", force: :cascade do |t| t.datetime "created_at", null: false t.string "domain", null: false @@ -342,6 +352,8 @@ add_foreign_key "memberships", "users", column: "added_by_id" add_foreign_key "mou_signatures", "organisations" add_foreign_key "mou_signatures", "users" + add_foreign_key "organisation_brands", "brands" + add_foreign_key "organisation_brands", "organisations" add_foreign_key "organisation_domains", "organisations" add_foreign_key "page_translations", "pages" add_foreign_key "pages", "forms" diff --git a/db/seeds.rb b/db/seeds.rb index 4a06fb129..6fad255d9 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -21,6 +21,7 @@ ) gds.organisation_domains.create! domain: "digital.cabinet-office.gov.uk" gds.organisation_domains.create! domain: "dsit.gov.uk" + gds.brands = Brand.all # Create default super-admin default_user = User.create!({ email: "example@example.com", diff --git a/spec/factories/models/forms.rb b/spec/factories/models/forms.rb index 1c467ea0c..fb0445f86 100644 --- a/spec/factories/models/forms.rb +++ b/spec/factories/models/forms.rb @@ -24,7 +24,7 @@ brand_id { nil } trait :with_brand do - brand_id { Settings.branding.available_brands.first.id } + brand_id { "test-brand" } end trait :with_group do diff --git a/spec/factories/models/organisation_brands.rb b/spec/factories/models/organisation_brands.rb new file mode 100644 index 000000000..0b1b392d7 --- /dev/null +++ b/spec/factories/models/organisation_brands.rb @@ -0,0 +1,6 @@ +FactoryBot.define do + factory :organisation_brand do + organisation + brand + end +end diff --git a/spec/input_objects/forms/brand_input_spec.rb b/spec/input_objects/forms/brand_input_spec.rb index 0f0ce21b8..69cdf6e92 100644 --- a/spec/input_objects/forms/brand_input_spec.rb +++ b/spec/input_objects/forms/brand_input_spec.rb @@ -1,8 +1,18 @@ require "rails_helper" RSpec.describe Forms::BrandInput, type: :model do + let(:organisation) { create(:organisation) } + let(:group) { create(:group, organisation:) } let(:form) do - create(:form, :live) + create(:form, :live, :with_group, group:) + end + + let(:cheshire_east) { create(:brand, slug: "cheshire-east", name: "Cheshire East Council") } + let(:south_gloucestershire) { create(:brand, slug: "south-gloucestershire", name: "South Gloucestershire Council") } + + before do + create(:organisation_brand, organisation:, brand: cheshire_east) + create(:organisation_brand, organisation:, brand: south_gloucestershire) end describe "validations" do @@ -14,15 +24,29 @@ end end - context "when given a brand_id from the configured list" do + context "when given a brand_id from the organisation's brands" do it "validates successfully" do - brand_input = described_class.new(form:, brand_id: Settings.branding.available_brands.first.id) + brand_input = described_class.new(form:, brand_id: "cheshire-east") expect(brand_input).to be_valid end end - context "when given a brand_id that is not in the configured list" do + context "when given a brand_id that is not one of the organisation's brands" do + it "returns a validation error" do + create(:brand, slug: "not-allowed-for-organisation") + + brand_input = described_class.new(form:, brand_id: "not-allowed-for-organisation") + + brand_input.validate(:brand_id) + + expect(brand_input.errors.full_messages_for(:brand_id)).to include( + "Brand Select a brand", + ) + end + end + + context "when given a brand_id that does not exist" do it "returns a validation error" do brand_input = described_class.new(form:, brand_id: "not-a-brand") @@ -36,11 +60,32 @@ end describe "#brand_options" do - it "starts with the GOV.UK default option followed by the configured brands" do + it "starts with the GOV.UK default option followed by the organisation's brands" do brand_input = described_class.new(form:) expect(brand_input.brand_options.first).to have_attributes(id: "", name: "GOV.UK (default)") - expect(brand_input.brand_options.drop(1).map(&:id)).to eq(Settings.branding.available_brands.map(&:id)) + expect(brand_input.brand_options.drop(1).map(&:id)).to eq %w[cheshire-east south-gloucestershire] + end + + context "when the organisation has no brands" do + let(:other_organisation) { create(:organisation, slug: "other-org") } + let(:group) { create(:group, organisation: other_organisation) } + + it "only includes the GOV.UK default option" do + brand_input = described_class.new(form:) + + expect(brand_input.brand_options.map(&:id)).to eq [""] + end + end + + context "when the form is not in a group" do + let(:form) { create(:form, :live) } + + it "only includes the GOV.UK default option" do + brand_input = described_class.new(form:) + + expect(brand_input.brand_options.map(&:id)).to eq [""] + end end end diff --git a/spec/models/organisation_brand_spec.rb b/spec/models/organisation_brand_spec.rb new file mode 100644 index 000000000..4c40ee790 --- /dev/null +++ b/spec/models/organisation_brand_spec.rb @@ -0,0 +1,32 @@ +require "rails_helper" + +RSpec.describe OrganisationBrand do + describe "validations" do + it "has a valid factory" do + expect(build(:organisation_brand)).to be_valid + end + + it "is invalid without an organisation" do + organisation_brand = described_class.new(brand: create(:brand)) + expect(organisation_brand).not_to be_valid + end + + it "is invalid without a brand" do + organisation_brand = described_class.new(organisation: create(:organisation)) + expect(organisation_brand).not_to be_valid + end + + it "is invalid when the brand is already added to the organisation" do + organisation_brand = create(:organisation_brand) + duplicate = described_class.new(organisation: organisation_brand.organisation, brand: organisation_brand.brand) + expect(duplicate).not_to be_valid + end + + it "is valid when the brand is added to a different organisation" do + organisation_brand = create(:organisation_brand) + other_organisation = create(:organisation, slug: "other-org") + other_organisation_brand = build(:organisation_brand, organisation: other_organisation, brand: organisation_brand.brand) + expect(other_organisation_brand).to be_valid + end + end +end diff --git a/spec/requests/forms/brand_controller_spec.rb b/spec/requests/forms/brand_controller_spec.rb index 796e910ee..888222b32 100644 --- a/spec/requests/forms/brand_controller_spec.rb +++ b/spec/requests/forms/brand_controller_spec.rb @@ -8,6 +8,11 @@ let(:custom_branding_enabled) { true } before do + cheshire_east = create(:brand, slug: "cheshire-east", name: "Cheshire East Council") + south_gloucestershire = create(:brand, slug: "south-gloucestershire", name: "South Gloucestershire Council") + create(:organisation_brand, organisation: standard_user.organisation, brand: cheshire_east) + create(:organisation_brand, organisation: standard_user.organisation, brand: south_gloucestershire) + Membership.create!(group_id: group.id, user: standard_user, added_by: standard_user) GroupForm.create!(form_id: form.id, group_id: group.id) diff --git a/spec/requests/organisation_brands_controller_spec.rb b/spec/requests/organisation_brands_controller_spec.rb new file mode 100644 index 000000000..96f17af8b --- /dev/null +++ b/spec/requests/organisation_brands_controller_spec.rb @@ -0,0 +1,197 @@ +require "rails_helper" + +RSpec.describe OrganisationBrandsController, type: :request do + let(:organisation) { create :organisation, slug: "department-for-testing" } + let(:brand) { create :brand, slug: "cheshire-east", name: "Cheshire East Council" } + + describe "#new" do + let(:path) { new_organisation_brand_path(organisation) } + + context "when the user is not a super admin" do + before do + login_as_standard_user + + get path + 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 + end + + context "when the user is a super admin" do + let!(:added_brand) { create :brand, name: "Already Added Council" } + let!(:available_brand) { create :brand, name: "Available Council" } + + before do + create :organisation_brand, organisation:, brand: added_brand + + login_as_super_admin_user + + get path + end + + it "returns http code 200 and renders the new view" do + expect(response).to have_http_status(:ok) + expect(response).to render_template("organisation_brands/new") + end + + it "includes brands that have not been added to the organisation" do + expect(response.body).to include(available_brand.name) + end + + it "does not include brands that have already been added to the organisation" do + expect(response.body).not_to include(added_brand.name) + end + end + end + + describe "#create" do + let(:path) { organisation_brands_path(organisation) } + let(:params) { { organisations_brand_input: { brand_id: brand.id } } } + + context "when the user is not a super admin" do + before do + login_as_standard_user + + post(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 add the brand to the organisation" do + expect(organisation.brands).to be_empty + end + end + + context "when the user is a super admin" do + before do + login_as_super_admin_user + end + + it "adds the brand to the organisation" do + expect { + post(path, params:) + }.to change { organisation.brands.count }.by(1) + end + + it "redirects to the organisation page with a success message" do + post(path, params:) + + expect(response).to redirect_to(organisation_path(organisation)) + expect(flash[:success]).to eq(I18n.t("organisation_brands.create.success", brand_name: brand.name)) + end + + context "when no brand is selected" do + let(:params) { { organisations_brand_input: { brand_id: "" } } } + + it "re-renders the page with an error" do + post(path, params:) + + expect(response).to have_http_status(:unprocessable_content) + expect(response.body).to include(I18n.t("activemodel.errors.models.organisations/brand_input.attributes.brand_id.blank")) + end + end + + context "when the brand does not exist" do + let(:params) { { organisations_brand_input: { brand_id: "999999" } } } + + it "re-renders the page with an error" do + post(path, params:) + + expect(response).to have_http_status(:unprocessable_content) + expect(response.body).to include(I18n.t("activemodel.errors.models.organisations/brand_input.attributes.brand_id.inclusion")) + end + end + + context "when the brand has already been added to the organisation" do + before do + create :organisation_brand, organisation:, brand: + end + + it "re-renders the page with an error" do + expect { + post(path, params:) + }.not_to(change { organisation.brands.count }) + + expect(response).to have_http_status(:unprocessable_content) + expect(response.body).to include(I18n.t("activemodel.errors.models.organisations/brand_input.attributes.brand_id.inclusion")) + end + end + + context "when the autocomplete text input has been cleared" do + let(:params) { { organisations_brand_input: { brand_id: brand.id, brand_id_raw: "" } } } + + it "ignores the stale select value and re-renders the page with an error" do + expect { + post(path, params:) + }.not_to(change { organisation.brands.count }) + + expect(response).to have_http_status(:unprocessable_content) + expect(response.body).to include(I18n.t("activemodel.errors.models.organisations/brand_input.attributes.brand_id.blank")) + end + end + end + end + + describe "#destroy" do + let(:path) { organisation_brand_path(organisation, brand) } + + before do + create :organisation_brand, organisation:, brand: + end + + context "when the user is not a super admin" do + before do + login_as_standard_user + + delete path + 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 remove the brand from the organisation" do + expect(organisation.brands).to include(brand) + end + end + + context "when the user is a super admin" do + before do + login_as_super_admin_user + end + + it "removes the brand from the organisation without deleting the brand" do + expect { + delete path + }.to change { organisation.brands.count }.by(-1) + + expect(Brand.exists?(brand.id)).to be true + end + + it "redirects to the organisation page with a success message" do + delete path + + expect(response).to redirect_to(organisation_path(organisation)) + expect(flash[:success]).to eq(I18n.t("organisation_brands.destroy.success", brand_name: brand.name)) + end + + context "when the brand is not one of the organisation's brands" do + let(:other_brand) { create :brand } + let(:path) { organisation_brand_path(organisation, other_brand) } + + it "returns http code 404" do + delete path + + expect(response).to have_http_status(:not_found) + end + end + end + end +end diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb index 8af46f3e4..ff1878f40 100644 --- a/spec/requests/organisations_controller_spec.rb +++ b/spec/requests/organisations_controller_spec.rb @@ -166,6 +166,7 @@ def organisation_row_order(response) let(:path) { organisation_path(organisation) } let!(:organisation_domain) { create :organisation_domain, organisation: } + let!(:organisation_brand) { create :organisation_brand, organisation: } include_examples "unauthorized user is forbidden" @@ -188,6 +189,13 @@ def organisation_row_order(response) expect(response.body).to include(I18n.t("mou_signatures.index.agreement_type.#{organisation.mou_signatures.first.agreement_type}")) expect(response.body).to include(organisation_domain.domain) end + + it "shows the organisation's brands with a link to add a brand" do + expect(response.body).to include(organisation_brand.brand.name) + expect(response.body).to include(I18n.t("organisations.show.brands.guidance")) + expect(response.body).to include(I18n.t("organisations.show.brands.add")) + expect(response.body).to include(new_organisation_brand_path(organisation)) + end end end end diff --git a/spec/views/organisations/show.html.erb_spec.rb b/spec/views/organisations/show.html.erb_spec.rb index b89d16136..3773a3cd3 100644 --- a/spec/views/organisations/show.html.erb_spec.rb +++ b/spec/views/organisations/show.html.erb_spec.rb @@ -64,6 +64,11 @@ 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 + expect(rendered).to have_text(I18n.t("organisations.show.brands.guidance")) end it "shows the MOU guidance with links to the agreements" do