diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index 19aa94ae5..b415f2a68 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) @@ -40,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/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/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 745c769b7..0c68c3580 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 @@ -1524,7 +1525,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: @@ -1541,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/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 8af46f3e4..026f13979 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 @@ -126,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 c57f95bf8..67b2268c5 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" @@ -61,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 @@ -74,23 +77,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 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