diff --git a/app/controllers/api/brands_controller.rb b/app/controllers/api/brands_controller.rb index aca23beb9..464cd0538 100644 --- a/app/controllers/api/brands_controller.rb +++ b/app/controllers/api/brands_controller.rb @@ -1,7 +1,7 @@ class Api::BrandsController < ApplicationController def show expires_in 5.minutes, public: true - render json: brand.as_json(only: %i[name slug]) + render json: brand.as_json(only: %i[name slug header_background_colour border_colour logo_alt_text logo_link copyright_holder]) end private diff --git a/app/controllers/brands_controller.rb b/app/controllers/brands_controller.rb index 11272264f..3fa032207 100644 --- a/app/controllers/brands_controller.rb +++ b/app/controllers/brands_controller.rb @@ -51,10 +51,10 @@ def set_brand end def brand_params - params.require(:brand).permit(:name, :slug) + params.require(:brand).permit(:name, :slug, :header_background_colour, :border_colour, :logo_alt_text, :logo_link, :copyright_holder) end def update_brand_params - params.require(:brand).permit(:name) + params.require(:brand).permit(:name, :header_background_colour, :border_colour, :logo_alt_text, :logo_link, :copyright_holder) end end diff --git a/app/models/brand.rb b/app/models/brand.rb index e03751124..4454261da 100644 --- a/app/models/brand.rb +++ b/app/models/brand.rb @@ -3,4 +3,7 @@ class Brand < ApplicationRecord validates :slug, presence: true, uniqueness: true, format: { with: /\A[a-z0-9]+(?:-[a-z0-9]+)*\z/, allow_blank: true } validates :name, presence: true + validates :header_background_colour, :border_colour, presence: true, format: { with: /\A#[0-9a-f]{6}\z/, allow_blank: true } + validates :logo_link, presence: true, format: { with: %r{\Ahttps?://.*\z}, allow_blank: true } + validates :logo_alt_text, :copyright_holder, presence: true end diff --git a/app/views/brands/edit.html.erb b/app/views/brands/edit.html.erb index 42831ed6d..8d200e8cf 100644 --- a/app/views/brands/edit.html.erb +++ b/app/views/brands/edit.html.erb @@ -15,6 +15,16 @@ <%= f.govuk_text_field :name, label: { size: 'm' } %> + <%= f.govuk_text_field :logo_alt_text, label: { size: 'm' } %> + + <%= f.govuk_text_field :logo_link, label: { size: 'm' } %> + + <%= f.govuk_text_field :header_background_colour, label: { size: 'm' }, width: 5 %> + + <%= f.govuk_text_field :border_colour, label: { size: 'm' }, width: 5 %> + + <%= f.govuk_text_field :copyright_holder, label: { size: 'm' } %> + <%= f.govuk_submit t("save_and_continue") %> <% end %> diff --git a/app/views/brands/new.html.erb b/app/views/brands/new.html.erb index 09e8af924..eca01f182 100644 --- a/app/views/brands/new.html.erb +++ b/app/views/brands/new.html.erb @@ -14,6 +14,16 @@ <%= f.govuk_text_field :slug, label: { size: 'm' } %> + <%= f.govuk_text_field :logo_alt_text, label: { size: 'm' } %> + + <%= f.govuk_text_field :logo_link, label: { size: 'm' } %> + + <%= f.govuk_text_field :header_background_colour, label: { size: 'm' }, width: 5 %> + + <%= f.govuk_text_field :border_colour, label: { size: 'm' }, width: 5 %> + + <%= f.govuk_text_field :copyright_holder, label: { size: 'm' } %> + <%= f.govuk_submit t("save_and_continue") %> <% end %> diff --git a/app/views/brands/show.html.erb b/app/views/brands/show.html.erb index 9a53d4f79..dd9851523 100644 --- a/app/views/brands/show.html.erb +++ b/app/views/brands/show.html.erb @@ -11,6 +11,31 @@ row.with_value { @brand.slug } end + summary_list.with_row do |row| + row.with_key { t('brands.show.summary.logo_alt_text') } + row.with_value { @brand.logo_alt_text } + end + + summary_list.with_row do |row| + row.with_key { t('brands.show.summary.logo_link') } + row.with_value { @brand.logo_link } + end + + summary_list.with_row do |row| + row.with_key { t('brands.show.summary.header_background_colour') } + row.with_value { @brand.header_background_colour } + end + + summary_list.with_row do |row| + row.with_key { t('brands.show.summary.border_colour') } + row.with_value { @brand.border_colour } + end + + summary_list.with_row do |row| + row.with_key { t('brands.show.summary.copyright_holder') } + row.with_value { @brand.copyright_holder } + end + summary_list.with_row do |row| row.with_key { t('brands.show.summary.created_at') } row.with_value { l(@brand.created_at.to_date, format: :long) } diff --git a/config/locales/en.yml b/config/locales/en.yml index 40cf3000b..17feead28 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -158,7 +158,23 @@ en: models: brand: attributes: + border_colour: + blank: Enter the header and footer border colour + invalid: 'Header and footer border colour must be a hex colour code, like #206c49' + copyright_holder: + blank: Enter the copyright holder + header_background_colour: + blank: Enter the header background colour + invalid: 'Header background colour must be a hex colour code, like #206c49' + logo_alt_text: + blank: Enter the logo alt text + logo_link: + blank: Enter the logo link + invalid: Enter the logo link in the correct format, starting with https:// + name: + blank: Enter a brand name slug: + blank: Enter a slug invalid: Slug must only include lowercase letters a to z, numbers and hyphens taken: There is already a brand with this slug draft_question: @@ -308,12 +324,17 @@ en: no_results: No brands found. table_caption: Brands table_headings: - name: Name + name: Brand name slug: Slug show: edit_brand: Edit this brand summary: + border_colour: Header and footer border colour + copyright_holder: Copyright holder created_at: Created + header_background_colour: Header background colour + logo_alt_text: Logo alt text + logo_link: Logo link slug: Slug success_messages: create: The brand has been created @@ -991,6 +1012,13 @@ en: hint: account_name_input: name: You do not need to include a title or any middle names. + brand: + border_colour: 'Enter a hex colour code. For example, #206c49' + copyright_holder: Shown in the copyright notice in the form’s footer. + header_background_colour: 'Enter a hex colour code. For example, #206c49' + logo_alt_text: Describes the logo for people using screen readers. Usually the organisation’s name. + logo_link: The web address people go to when they select the logo. Usually the organisation’s website. + name: Used to identify the brand. Not shown to people filling in forms. forms_copy_input: name: The form name will be shown at the top of each page of the form. Use a name that describes what the form will help people to do. For example ‘Apply for a juggling licence’. page: @@ -1058,6 +1086,13 @@ en: account_terms_of_use_input: agreed_options: '1': I agree to these terms + brand: + border_colour: Header and footer border colour + copyright_holder: Copyright holder + header_background_colour: Header background colour + logo_alt_text: Logo alt text + logo_link: Logo link + name: Brand name forms_batch_submissions_input: batch_frequencies_options: daily: Get a daily CSV of submissions diff --git a/db/migrate/20260812141413_add_branding_attributes_to_brands.rb b/db/migrate/20260812141413_add_branding_attributes_to_brands.rb new file mode 100644 index 000000000..cb141bc86 --- /dev/null +++ b/db/migrate/20260812141413_add_branding_attributes_to_brands.rb @@ -0,0 +1,11 @@ +class AddBrandingAttributesToBrands < ActiveRecord::Migration[8.1] + def change + change_table :brands, bulk: true do |t| + t.string :header_background_colour + t.string :border_colour + t.string :logo_alt_text + t.string :logo_link + t.string :copyright_holder + end + end +end diff --git a/db/schema.rb b/db/schema.rb index e63f53657..ff473944b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,12 +10,17 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_08_11_140129) do +ActiveRecord::Schema[8.1].define(version: 2026_08_12_141413) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" create_table "brands", force: :cascade do |t| + t.string "border_colour" + t.string "copyright_holder" t.datetime "created_at", null: false + t.string "header_background_colour" + t.string "logo_alt_text" + t.string "logo_link" t.string "name", null: false t.string "slug", null: false t.datetime "updated_at", null: false diff --git a/db/seeds.rb b/db/seeds.rb index a4a51dee8..4c7bed61e 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -7,8 +7,24 @@ # Character.create(name: "Luke", movie: movies.first) if (HostingEnvironment.local_development? || HostingEnvironment.review?) && Brand.none? - Brand.create!(slug: "cheshire-east", name: "Cheshire East Council") - Brand.create!(slug: "south-gloucestershire", name: "South Gloucestershire Council") + Brand.create!( + slug: "toadstool-town", + name: "Toadstool Town Council", + header_background_colour: "#ffffff", + border_colour: "#206c49", + logo_alt_text: "Toadstool Town Council", + logo_link: "https://www.toadstooltown.example.com", + copyright_holder: "Toadstool Town Council", + ) + Brand.create!( + slug: "dragonfly-district", + name: "Dragonfly District Council", + header_background_colour: "#ffffff", + border_colour: "#4b0082", + logo_alt_text: "Dragonfly District Council", + logo_link: "https://www.dragonflydistrict.example.com", + copyright_holder: "Dragonfly District Council", + ) end if (HostingEnvironment.local_development? || HostingEnvironment.review?) && User.none? diff --git a/spec/factories/models/brands.rb b/spec/factories/models/brands.rb index 4a57bd05a..fdcbbcb3b 100644 --- a/spec/factories/models/brands.rb +++ b/spec/factories/models/brands.rb @@ -2,5 +2,10 @@ factory :brand do sequence(:slug) { |n| "brand-#{n}" } name { slug.titleize } + header_background_colour { "#ffffff" } + border_colour { "#206c49" } + logo_alt_text { name } + logo_link { "https://www.#{slug}.example.com" } + copyright_holder { name } end end diff --git a/spec/models/brand_spec.rb b/spec/models/brand_spec.rb index 51f0e359c..5026b2730 100644 --- a/spec/models/brand_spec.rb +++ b/spec/models/brand_spec.rb @@ -35,6 +35,48 @@ expect(brand.errors).to be_of_kind(:slug, :taken) end + %i[header_background_colour border_colour logo_alt_text logo_link copyright_holder].each do |attribute| + it "is invalid without a #{attribute.to_s.humanize.downcase}" do + brand.public_send("#{attribute}=", nil) + expect(brand).to be_invalid + expect(brand.errors).to be_of_kind(attribute, :blank) + end + end + + %i[header_background_colour border_colour].each do |attribute| + it "is invalid when the #{attribute.to_s.humanize.downcase} is not a lowercase 6-digit hex colour code" do + ["ffffff", "#FFFFFF", "#fff", "#gggggg", "white"].each do |colour| + brand.public_send("#{attribute}=", colour) + expect(brand).to be_invalid + expect(brand.errors).to be_of_kind(attribute, :invalid) + end + end + + it "is valid when the #{attribute.to_s.humanize.downcase} is a lowercase 6-digit hex colour code" do + brand.public_send("#{attribute}=", "#0b0c0c") + expect(brand).to be_valid + end + end + + it "is invalid when the logo link does not start with http:// or https://" do + ["www.example.com", "example.com", "ftp://example.com"].each do |url| + brand.logo_link = url + expect(brand).to be_invalid + expect(brand.errors).to be_of_kind(:logo_link, :invalid) + end + end + + it "is invalid when the logo link contains more than one line" do + brand.logo_link = "https://www.example.com\nmalicious" + expect(brand).to be_invalid + expect(brand.errors).to be_of_kind(:logo_link, :invalid) + end + + it "is valid when the logo link starts with https://" do + brand.logo_link = "https://www.example.com" + expect(brand).to be_valid + end + it "is an error to insert a brand with an existing slug" do existing_brand = create(:brand) diff --git a/spec/requests/api/brands_controller_spec.rb b/spec/requests/api/brands_controller_spec.rb index d49db49f8..0d58c6804 100644 --- a/spec/requests/api/brands_controller_spec.rb +++ b/spec/requests/api/brands_controller_spec.rb @@ -5,7 +5,16 @@ describe "#show" do context "when the brand exists" do - let(:brand) { create :brand, name: "Golden Zephyr", slug: "golden-zephyr" } + let(:brand) do + create :brand, + name: "Golden Zephyr", + slug: "golden-zephyr", + header_background_colour: "#ffffff", + border_colour: "#206c49", + logo_alt_text: "Golden Zephyr Council", + logo_link: "https://www.goldenzephyr.example.com", + copyright_holder: "Golden Zephyr Council" + end before do get "/api/v2/brands/#{brand.id}", headers: @@ -19,6 +28,11 @@ expect(response.parsed_body).to eq({ "name" => "Golden Zephyr", "slug" => "golden-zephyr", + "header_background_colour" => "#ffffff", + "border_colour" => "#206c49", + "logo_alt_text" => "Golden Zephyr Council", + "logo_link" => "https://www.goldenzephyr.example.com", + "copyright_holder" => "Golden Zephyr Council", }) end diff --git a/spec/requests/brands_controller_spec.rb b/spec/requests/brands_controller_spec.rb index ec4b46874..4a2fd58c9 100644 --- a/spec/requests/brands_controller_spec.rb +++ b/spec/requests/brands_controller_spec.rb @@ -73,6 +73,11 @@ it "shows the brand's properties" do expect(response.body).to include(brand.name) expect(response.body).to include(brand.slug) + expect(response.body).to include(brand.logo_alt_text) + expect(response.body).to include(brand.logo_link) + expect(response.body).to include(brand.header_background_colour) + expect(response.body).to include(brand.border_colour) + expect(response.body).to include(brand.copyright_holder) end end end @@ -93,12 +98,31 @@ expect(response).to have_http_status(:ok) expect(response).to render_template("brands/new") end + + it "has a labelled field for each brand attribute" do + page = Capybara.string(response.body) + ["Brand name", "Slug", "Logo alt text", "Logo link", "Header background colour", "Header and footer border colour", "Copyright holder"].each do |label| + expect(page).to have_field(label) + end + end end end describe "#create" do let(:path) { brands_path } - let(:params) { { brand: { name: "Testshire Council", slug: "testshire" } } } + let(:params) do + { + brand: { + name: "Testshire Council", + slug: "testshire", + header_background_colour: "#ffffff", + border_colour: "#206c49", + logo_alt_text: "Testshire Council", + logo_link: "https://www.testshire.example.com", + copyright_holder: "Testshire Council", + }, + } + end it_behaves_like "unauthorized user is forbidden" do let(:do_request) { post path, params: params } @@ -121,14 +145,21 @@ login_as_super_admin_user end - it "creates a brand with the given name and slug" do + it "creates a brand with the given attributes" do expect { post path, params: params }.to change(Brand, :count).by(1) brand = Brand.last - expect(brand.name).to eq("Testshire Council") - expect(brand.slug).to eq("testshire") + expect(brand).to have_attributes( + name: "Testshire Council", + slug: "testshire", + header_background_colour: "#ffffff", + border_colour: "#206c49", + logo_alt_text: "Testshire Council", + logo_link: "https://www.testshire.example.com", + copyright_holder: "Testshire Council", + ) end it "redirects to the brand page with a success message" do @@ -139,7 +170,24 @@ end context "when the brand is invalid" do - let(:params) { { brand: { name: "", slug: "testshire" } } } + before do + params[:brand][:name] = "" + end + + it "does not create a brand and re-renders the new view" do + expect { + post path, params: params + }.not_to change(Brand, :count) + + expect(response).to have_http_status(:unprocessable_content) + expect(response).to render_template("brands/new") + end + end + + context "when a colour is not a hex colour code" do + before do + params[:brand][:border_colour] = "green" + end it "does not create a brand and re-renders the new view" do expect { @@ -170,13 +218,32 @@ expect(response).to have_http_status(:ok) expect(response).to render_template("brands/edit") end + + it "has a labelled field for each editable brand attribute" do + page = Capybara.string(response.body) + ["Brand name", "Logo alt text", "Logo link", "Header background colour", "Header and footer border colour", "Copyright holder"].each do |label| + expect(page).to have_field(label) + end + end end end describe "#update" do let(:brand) { create :brand, slug: "testshire", name: "Testshire Council" } let(:path) { brand_path(brand) } - let(:params) { { brand: { name: "Greater Testshire Council", slug: "greater-testshire" } } } + let(:params) do + { + brand: { + name: "Greater Testshire Council", + slug: "greater-testshire", + header_background_colour: "#f0f0f0", + border_colour: "#123abc", + logo_alt_text: "Greater Testshire Council", + logo_link: "https://www.greater-testshire.example.com", + copyright_holder: "Greater Testshire Council", + }, + } + end it_behaves_like "unauthorized user is forbidden" do let(:do_request) { put path, params: params } @@ -199,10 +266,18 @@ login_as_super_admin_user end - it "updates the brand's name but not its slug" do + it "updates the brand's attributes but not its slug" do put path, params: params - expect(brand.reload).to have_attributes(name: "Greater Testshire Council", slug: "testshire") + expect(brand.reload).to have_attributes( + name: "Greater Testshire Council", + slug: "testshire", + header_background_colour: "#f0f0f0", + border_colour: "#123abc", + logo_alt_text: "Greater Testshire Council", + logo_link: "https://www.greater-testshire.example.com", + copyright_holder: "Greater Testshire Council", + ) end it "redirects to the brand page with a success message" do