From d7935f0e36cf2dc7464b4bcedb333fdbb3a6d7d2 Mon Sep 17 00:00:00 2001 From: Sean Rankine Date: Mon, 13 Jul 2026 16:42:06 +0100 Subject: [PATCH 1/3] Split forms count on organisations list A single forms count on the organisations list hid whether an organisation was running live forms or had only drafted them. Split the column into live and draft counts, excluding archived forms. --- app/controllers/organisations_controller.rb | 7 ++++++- app/views/organisations/index.html.erb | 6 ++++-- config/locales/en.yml | 3 ++- spec/requests/organisations_controller_spec.rb | 12 ++++++++++-- spec/views/organisations/index.html.erb_spec.rb | 14 +++++++++----- 5 files changed, 31 insertions(+), 11 deletions(-) diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index 19aa94ae5..52357db2f 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -12,7 +12,12 @@ def index organisation_ids = @organisations.map(&:id) @user_counts = User.where(organisation_id: organisation_ids).group(:organisation_id).count - @form_counts = GroupForm.joins(:group).where(groups: { organisation_id: organisation_ids }).reorder(nil).group("groups.organisation_id").count + @live_form_counts = GroupForm.joins(:group, :form) + .where(groups: { organisation_id: organisation_ids }, forms: { state: %w[live live_with_draft] }) + .reorder(nil).group("groups.organisation_id").count + @draft_form_counts = GroupForm.joins(:group, :form) + .where(groups: { organisation_id: organisation_ids }, forms: { state: "draft" }) + .reorder(nil).group("groups.organisation_id").count @agreement_types = MouSignature.where(organisation_id: organisation_ids) .distinct .pluck(:organisation_id, :agreement_type) diff --git a/app/views/organisations/index.html.erb b/app/views/organisations/index.html.erb index dcbcf7a61..df5567cff 100644 --- a/app/views/organisations/index.html.erb +++ b/app/views/organisations/index.html.erb @@ -58,7 +58,8 @@ head.with_row do |row| row.with_cell(header: true, text: t("organisations.index.table_headings.name")) row.with_cell(header: true, text: t("organisations.index.table_headings.users"), numeric: true) - row.with_cell(header: true, text: t("organisations.index.table_headings.forms"), numeric: true) + row.with_cell(header: true, text: t("organisations.index.table_headings.live_forms"), numeric: true) + row.with_cell(header: true, text: t("organisations.index.table_headings.draft_forms"), numeric: true) row.with_cell(header: true, text: t("organisations.index.table_headings.agreement_type")) end end %> @@ -70,7 +71,8 @@ govuk_link_to(organisation.name_with_abbreviation, organisation_path(organisation)) end row.with_cell(text: @user_counts.fetch(organisation.id, 0).to_s, numeric: true) - row.with_cell(text: @form_counts.fetch(organisation.id, 0).to_s, numeric: true) + row.with_cell(text: @live_form_counts.fetch(organisation.id, 0).to_s, numeric: true) + row.with_cell(text: @draft_form_counts.fetch(organisation.id, 0).to_s, numeric: true) agreement_types = @agreement_types.fetch(organisation.id, []) if agreement_types.any? row.with_cell(text: agreement_types.map { |agreement_type| t("mou_signatures.index.agreement_type.#{agreement_type}") }.join(", ")) diff --git a/config/locales/en.yml b/config/locales/en.yml index 745c769b7..5b510ebcb 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1524,7 +1524,8 @@ en: table_caption: All organisations table_headings: agreement_type: Agreement type - forms: Forms + draft_forms: Draft forms + live_forms: Live forms name: Name users: Users show: diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb index 8af46f3e4..1360deb35 100644 --- a/spec/requests/organisations_controller_spec.rb +++ b/spec/requests/organisations_controller_spec.rb @@ -28,7 +28,9 @@ context "when the user is a super admin" do before do - create(:form, :with_group, group:) + create(:form, :live, :with_group, group:) + create_list(:form, 2, :with_group, group:) + create(:form, :archived, :with_group, group:) login_as_super_admin_user @@ -45,11 +47,17 @@ expect(response.body).to include(closed_organisation.name) end - it "shows the number of forms in each organisation" do + it "shows the number of live forms in each organisation, not counting archived forms" do page = Capybara.string(response.body) expect(page).to have_xpath "//tbody/tr[2]/td[3]", text: "1" expect(page).to have_xpath "//tbody/tr[1]/td[3]", text: "0" end + + it "shows the number of draft forms in each organisation, not counting archived forms" do + page = Capybara.string(response.body) + expect(page).to have_xpath "//tbody/tr[2]/td[4]", text: "2" + expect(page).to have_xpath "//tbody/tr[1]/td[4]", text: "0" + end end context "when filtering" do diff --git a/spec/views/organisations/index.html.erb_spec.rb b/spec/views/organisations/index.html.erb_spec.rb index c57f95bf8..eb0be119b 100644 --- a/spec/views/organisations/index.html.erb_spec.rb +++ b/spec/views/organisations/index.html.erb_spec.rb @@ -5,7 +5,8 @@ let(:closed_organisation) { create :organisation, slug: "closed-department", closed: true } let(:organisations) { [organisation, closed_organisation] } let(:user_counts) { { organisation.id => 3 } } - let(:form_counts) { { organisation.id => 2 } } + let(:live_form_counts) { { organisation.id => 2 } } + let(:draft_form_counts) { { organisation.id => 4 } } let(:agreement_types) { { organisation.id => %w[crown] } } let(:pagy) { Pagy.new(count: organisations.size, page: 1, limit: 50) } let(:filter_input) { Organisations::FilterInput.new } @@ -15,7 +16,8 @@ assign(:filter_input, filter_input) assign(:organisations, organisations) assign(:user_counts, user_counts) - assign(:form_counts, form_counts) + assign(:live_form_counts, live_form_counts) + assign(:draft_form_counts, draft_form_counts) assign(:agreement_types, agreement_types) render template: "organisations/index" @@ -74,23 +76,25 @@ it "contains the user and form counts" do expect(rendered).to have_xpath "//tbody/tr[1]/td[2]", text: "3" expect(rendered).to have_xpath "//tbody/tr[1]/td[3]", text: "2" + expect(rendered).to have_xpath "//tbody/tr[1]/td[4]", text: "4" end it "shows zero counts for organisations without users or forms" do expect(rendered).to have_xpath "//tbody/tr[2]/td[2]", text: "0" expect(rendered).to have_xpath "//tbody/tr[2]/td[3]", text: "0" + expect(rendered).to have_xpath "//tbody/tr[2]/td[4]", text: "0" end it "shows the agreement type for each organisation" do - expect(rendered).to have_xpath "//tbody/tr[1]/td[4]", text: I18n.t("mou_signatures.index.agreement_type.crown") - expect(rendered).to have_xpath "//tbody/tr[2]/td[4]", text: I18n.t("organisations.index.agreement_type_none") + expect(rendered).to have_xpath "//tbody/tr[1]/td[5]", text: I18n.t("mou_signatures.index.agreement_type.crown") + expect(rendered).to have_xpath "//tbody/tr[2]/td[5]", text: I18n.t("organisations.index.agreement_type_none") end context "when an organisation has both agreement types" do let(:agreement_types) { { organisation.id => %w[crown non_crown] } } it "shows both agreement types" do - expect(rendered).to have_xpath "//tbody/tr[1]/td[4]", text: "Crown MOU, Non-crown agreement" + expect(rendered).to have_xpath "//tbody/tr[1]/td[5]", text: "Crown MOU, Non-crown agreement" end end From 1346a3f4d17c42cd33b6b3228d504b435b571afc Mon Sep 17 00:00:00 2001 From: Sean Rankine Date: Mon, 13 Jul 2026 16:46:06 +0100 Subject: [PATCH 2/3] Sort organisations by live and draft form counts Sorting by total form count no longer matched the table once the forms column was split into live and draft. Replace it with separate sort options for each count. --- app/controllers/organisations_controller.rb | 6 ++- .../organisations/filter_input.rb | 3 +- app/models/organisation.rb | 9 +++- config/locales/en.yml | 3 +- .../organisations/filter_input_spec.rb | 3 +- spec/models/organisation_spec.rb | 54 +++++++++++++++++++ .../requests/organisations_controller_spec.rb | 13 ++++- .../organisations/index.html.erb_spec.rb | 3 +- 8 files changed, 84 insertions(+), 10 deletions(-) diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index 52357db2f..b415f2a68 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -45,8 +45,10 @@ def apply_sort(scope) case filter_params[:sort] when "users" scope.order_by_user_count - when "forms" - scope.order_by_form_count + when "live_forms" + scope.order_by_live_form_count + when "draft_forms" + scope.order_by_draft_form_count when "agreement_date" scope.order_by_first_agreement_date else diff --git a/app/input_objects/organisations/filter_input.rb b/app/input_objects/organisations/filter_input.rb index c0958d951..ecd6881a4 100644 --- a/app/input_objects/organisations/filter_input.rb +++ b/app/input_objects/organisations/filter_input.rb @@ -10,7 +10,8 @@ def sort_options [ OpenStruct.new(label: I18n.t("organisations.index.filter.sort.name"), value: "name"), OpenStruct.new(label: I18n.t("organisations.index.filter.sort.users"), value: "users"), - OpenStruct.new(label: I18n.t("organisations.index.filter.sort.forms"), value: "forms"), + OpenStruct.new(label: I18n.t("organisations.index.filter.sort.live_forms"), value: "live_forms"), + OpenStruct.new(label: I18n.t("organisations.index.filter.sort.draft_forms"), value: "draft_forms"), OpenStruct.new(label: I18n.t("organisations.index.filter.sort.agreement_date"), value: "agreement_date"), ] end diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 57e857604..7c72bb92d 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -36,8 +36,13 @@ class Organisation < ApplicationRecord .order(:name) } - scope :order_by_form_count, lambda { - order(Arel.sql("(SELECT COUNT(*) FROM groups_form_ids INNER JOIN groups ON groups.id = groups_form_ids.group_id WHERE groups.organisation_id = organisations.id) DESC")) + scope :order_by_live_form_count, lambda { + order(Arel.sql("(SELECT COUNT(*) FROM groups_form_ids INNER JOIN groups ON groups.id = groups_form_ids.group_id INNER JOIN forms ON forms.id = groups_form_ids.form_id WHERE groups.organisation_id = organisations.id AND forms.state IN ('live', 'live_with_draft')) DESC")) + .order(:name) + } + + scope :order_by_draft_form_count, lambda { + order(Arel.sql("(SELECT COUNT(*) FROM groups_form_ids INNER JOIN groups ON groups.id = groups_form_ids.group_id INNER JOIN forms ON forms.id = groups_form_ids.form_id WHERE groups.organisation_id = organisations.id AND forms.state = 'draft') DESC")) .order(:name) } diff --git a/config/locales/en.yml b/config/locales/en.yml index 5b510ebcb..236ab5cd0 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1510,9 +1510,10 @@ en: label: Name sort: agreement_date: Date first agreement signed (newest first) - forms: Forms (most first) + draft_forms: Draft forms (most first) hint: Choose how to order the list label: Sort by + live_forms: Live forms (most first) name: Name (A to Z) users: Users (most first) submit: Filter diff --git a/spec/input_objects/organisations/filter_input_spec.rb b/spec/input_objects/organisations/filter_input_spec.rb index 4c880507f..5f4c5a98f 100644 --- a/spec/input_objects/organisations/filter_input_spec.rb +++ b/spec/input_objects/organisations/filter_input_spec.rb @@ -32,7 +32,8 @@ expect(described_class.new.sort_options).to eq([ OpenStruct.new(label: I18n.t("organisations.index.filter.sort.name"), value: "name"), OpenStruct.new(label: I18n.t("organisations.index.filter.sort.users"), value: "users"), - OpenStruct.new(label: I18n.t("organisations.index.filter.sort.forms"), value: "forms"), + OpenStruct.new(label: I18n.t("organisations.index.filter.sort.live_forms"), value: "live_forms"), + OpenStruct.new(label: I18n.t("organisations.index.filter.sort.draft_forms"), value: "draft_forms"), OpenStruct.new(label: I18n.t("organisations.index.filter.sort.agreement_date"), value: "agreement_date"), ]) end diff --git a/spec/models/organisation_spec.rb b/spec/models/organisation_spec.rb index 71b0ee58b..dd9beae70 100644 --- a/spec/models/organisation_spec.rb +++ b/spec/models/organisation_spec.rb @@ -106,6 +106,60 @@ end end + describe ".order_by_live_form_count" do + let!(:organisation_with_two_live_forms) { create :organisation, slug: "org-with-two-live-forms" } + let!(:organisation_with_one_live_form) { create :organisation, slug: "org-with-one-live-form" } + let!(:organisation_with_draft_form) { create :organisation, slug: "org-with-draft-form" } + + before do + two_live_group = create :group, organisation: organisation_with_two_live_forms + create :form, :live, :with_group, group: two_live_group + create :form, :live_with_draft, :with_group, group: two_live_group + + one_live_group = create :group, organisation: organisation_with_one_live_form + create :form, :live, :with_group, group: one_live_group + create :form, :archived, :with_group, group: one_live_group + + draft_group = create :group, organisation: organisation_with_draft_form + create :form, :with_group, group: draft_group + end + + it "orders by live form count, most first, counting live with draft but not draft or archived forms" do + expect(described_class.order_by_live_form_count).to eq [ + organisation_with_two_live_forms, + organisation_with_one_live_form, + organisation_with_draft_form, + ] + end + end + + describe ".order_by_draft_form_count" do + let!(:organisation_with_two_draft_forms) { create :organisation, slug: "org-with-two-draft-forms" } + let!(:organisation_with_one_draft_form) { create :organisation, slug: "org-with-one-draft-form" } + let!(:organisation_with_live_form) { create :organisation, slug: "org-with-live-form" } + + before do + two_draft_group = create :group, organisation: organisation_with_two_draft_forms + create_list :form, 2, :with_group, group: two_draft_group + + one_draft_group = create :group, organisation: organisation_with_one_draft_form + create :form, :with_group, group: one_draft_group + create :form, :live_with_draft, :with_group, group: one_draft_group + create :form, :archived_with_draft, :with_group, group: one_draft_group + + live_group = create :group, organisation: organisation_with_live_form + create :form, :live, :with_group, group: live_group + end + + it "orders by draft form count, most first, not counting live or archived forms with drafts" do + expect(described_class.order_by_draft_form_count).to eq [ + organisation_with_two_draft_forms, + organisation_with_one_draft_form, + organisation_with_live_form, + ] + end + end + describe ".order_by_first_agreement_date" do let!(:organisation_signed_last_week) { create :organisation, slug: "org-signed-last-week" } let!(:organisation_signed_last_year) { create :organisation, slug: "org-signed-last-year" } diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb index 1360deb35..026f13979 100644 --- a/spec/requests/organisations_controller_spec.rb +++ b/spec/requests/organisations_controller_spec.rb @@ -134,8 +134,17 @@ def organisation_row_order(response) expect(organisation_row_order(response).first).to eq(organisation_z.name_with_abbreviation) end - it "sorts by form count descending" do - get path, params: { filter: { sort: "forms" } } + it "sorts by live form count descending" do + group = create :group, organisation: organisation_z + create(:form, :live, :with_group, group:) + + get path, params: { filter: { sort: "live_forms" } } + + expect(organisation_row_order(response).first).to eq(organisation_z.name_with_abbreviation) + end + + it "sorts by draft form count descending" do + get path, params: { filter: { sort: "draft_forms" } } expect(organisation_row_order(response).first).to eq(organisation_n.name_with_abbreviation) end diff --git a/spec/views/organisations/index.html.erb_spec.rb b/spec/views/organisations/index.html.erb_spec.rb index eb0be119b..67b2268c5 100644 --- a/spec/views/organisations/index.html.erb_spec.rb +++ b/spec/views/organisations/index.html.erb_spec.rb @@ -63,7 +63,8 @@ expect(rendered).to have_select("filter[sort]", options: [ I18n.t("organisations.index.filter.sort.name"), I18n.t("organisations.index.filter.sort.users"), - I18n.t("organisations.index.filter.sort.forms"), + I18n.t("organisations.index.filter.sort.live_forms"), + I18n.t("organisations.index.filter.sort.draft_forms"), I18n.t("organisations.index.filter.sort.agreement_date"), ]) end From 513c927d64c82663330e439c44e22581aa8477ca Mon Sep 17 00:00:00 2001 From: Sean Rankine Date: Mon, 13 Jul 2026 16:48:45 +0100 Subject: [PATCH 3/3] Split forms count on organisation page The organisation page summary still showed a single forms count after the organisations list gained the live and draft split. Apply the same split there for consistency. --- app/views/organisations/show.html.erb | 9 +++++++-- config/locales/en.yml | 3 ++- .../views/organisations/show.html.erb_spec.rb | 19 ++++++++++++++----- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/app/views/organisations/show.html.erb b/app/views/organisations/show.html.erb index ad3938560..58541da7f 100644 --- a/app/views/organisations/show.html.erb +++ b/app/views/organisations/show.html.erb @@ -37,8 +37,13 @@ end summary_list.with_row do |row| - row.with_key { t('organisations.show.summary.forms') } - row.with_value { @organisation.group_forms.count.to_s } + row.with_key { t('organisations.show.summary.live_forms') } + row.with_value { @organisation.group_forms.joins(:form).where(forms: { state: %w[live live_with_draft] }).count.to_s } + end + + summary_list.with_row do |row| + row.with_key { t('organisations.show.summary.draft_forms') } + row.with_value { @organisation.group_forms.joins(:form).where(forms: { state: "draft" }).count.to_s } end summary_list.with_row do |row| diff --git a/config/locales/en.yml b/config/locales/en.yml index 236ab5cd0..0c68c3580 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1543,10 +1543,11 @@ en: summary: closed: Closed created_at: Created - forms: Number of forms + draft_forms: Number of draft forms govuk_content_id: GOV.UK content ID groups: Number of groups internal: Internal + live_forms: Number of live forms slug: Slug users: Number of users page_conditions: diff --git a/spec/views/organisations/show.html.erb_spec.rb b/spec/views/organisations/show.html.erb_spec.rb index b89d16136..3f8868b92 100644 --- a/spec/views/organisations/show.html.erb_spec.rb +++ b/spec/views/organisations/show.html.erb_spec.rb @@ -6,7 +6,10 @@ before do organisation_domains - create(:form, :with_group, group: create(:group, organisation:)) + group = create(:group, organisation:) + create(:form, :live, :with_group, group:) + create_list(:form, 2, :with_group, group:) + create(:form, :archived, :with_group, group:) assign(:organisation, organisation) @@ -22,13 +25,19 @@ expect(rendered).to have_css(".govuk-summary-list__value", text: organisation.slug) end - it "shows the number of forms between the user and group counts" do + it "shows the live and draft form counts between the user and group counts" do users_index = rendered.index(I18n.t("organisations.show.summary.users")) - forms_index = rendered.index(I18n.t("organisations.show.summary.forms")) + live_forms_index = rendered.index(I18n.t("organisations.show.summary.live_forms")) + draft_forms_index = rendered.index(I18n.t("organisations.show.summary.draft_forms")) groups_index = rendered.index(I18n.t("organisations.show.summary.groups")) - expect(forms_index).to be_between(users_index, groups_index) - expect(rendered).to have_css(".govuk-summary-list__row", text: /#{I18n.t('organisations.show.summary.forms')}\s*1/) + expect(live_forms_index).to be_between(users_index, draft_forms_index) + expect(draft_forms_index).to be_between(live_forms_index, groups_index) + end + + it "shows the number of live and draft forms, not counting archived forms" do + expect(rendered).to have_css(".govuk-summary-list__row", text: /#{I18n.t('organisations.show.summary.live_forms')}\s*1/) + expect(rendered).to have_css(".govuk-summary-list__row", text: /#{I18n.t('organisations.show.summary.draft_forms')}\s*2/) end it "shows whether the organisation is internal or closed" do