From 6364986ea44688f879d3fbe33ecd6e3bac54c9e0 Mon Sep 17 00:00:00 2001 From: Sean Rankine Date: Mon, 18 May 2026 21:16:08 +0100 Subject: [PATCH 1/2] Remove volumes from Dockerfile --- Dockerfile | 3 --- 1 file changed, 3 deletions(-) diff --git a/Dockerfile b/Dockerfile index 0c011b83de..127ba49618 100644 --- a/Dockerfile +++ b/Dockerfile @@ -72,9 +72,6 @@ COPY --chown=ruby:ruby --from=build /usr/local/bundle /usr/local/bundle COPY --chown=ruby:ruby --from=build /app /app RUN mkdir -p "/app/tmp/" && chown ruby:ruby "/app/tmp/" && chown ruby:ruby "/app/db/" -VOLUME "/tmp/" -VOLUME "/app/tmp/" -VOLUME "/app/db/" EXPOSE 3000 From 81060a6f2adfa787af090b16e87a2787b7723331 Mon Sep 17 00:00:00 2001 From: Sean Rankine Date: Mon, 13 Jul 2026 12:57:25 +0100 Subject: [PATCH 2/2] Remove the dedicated MOUs list page The organisations pages now show each organisation's signed MOUs and agreements, making the standalone list at /mous redundant. Point the header navigation and sitemap at the organisations pages instead, and drop the now-unused MouSignaturePolicy and locale strings. The MOU signing flow is unchanged. --- app/controllers/mou_signatures_controller.rb | 9 +--- app/controllers/sitemap_controller.rb | 6 +-- app/policies/mou_signature_policy.rb | 5 -- app/services/navigation_items_service.rb | 11 ---- app/views/mou_signatures/index.html.erb | 42 --------------- app/views/sitemap/index.html.erb | 4 +- config/locales/en.yml | 4 -- config/routes.rb | 2 - .../service_navigation_component_preview.rb | 2 +- .../mou/upgrade_user_changes_mou_spec.rb | 46 ++++++---------- spec/features/mou/view_signed_mous_spec.rb | 37 ++++++------- spec/policies/mou_signature_policy_spec.rb | 19 ------- .../services/navigation_items_service_spec.rb | 11 ---- .../mou_signatures/index.html.erb_spec.rb | 53 ------------------- spec/views/sitemap/index.html.erb_spec.rb | 26 ++++++--- 15 files changed, 57 insertions(+), 220 deletions(-) delete mode 100644 app/policies/mou_signature_policy.rb delete mode 100644 app/views/mou_signatures/index.html.erb delete mode 100644 spec/policies/mou_signature_policy_spec.rb delete mode 100644 spec/views/mou_signatures/index.html.erb_spec.rb diff --git a/app/controllers/mou_signatures_controller.rb b/app/controllers/mou_signatures_controller.rb index a360e7b76c..d9ad8a66aa 100644 --- a/app/controllers/mou_signatures_controller.rb +++ b/app/controllers/mou_signatures_controller.rb @@ -1,12 +1,5 @@ class MouSignaturesController < WebController - before_action :set_agreement_type, except: %i[index] - after_action :verify_authorized, only: %i[index] - - def index - authorize MouSignature, :can_manage_mous? - @mou_signatures = MouSignature.all - render template: "mou_signatures/index", locals: { mou_signatures: @mou_signatures } - end + before_action :set_agreement_type def show @mou_signature = current_user.current_organisation_mou_signature diff --git a/app/controllers/sitemap_controller.rb b/app/controllers/sitemap_controller.rb index be5282b0f3..0eea849444 100644 --- a/app/controllers/sitemap_controller.rb +++ b/app/controllers/sitemap_controller.rb @@ -6,7 +6,7 @@ def index render locals: { active_groups:, trial_groups:, should_show_users_link: should_show_users?, - should_show_mous_link: should_show_mous_link?, + should_show_organisations_link: should_show_organisations_link?, should_show_reports_link: should_show_reports_link? } end @@ -14,8 +14,8 @@ def should_show_users? Pundit.policy(current_user, :user).can_manage_user? end - def should_show_mous_link? - Pundit.policy(current_user, :mou_signature).can_manage_mous? + def should_show_organisations_link? + Pundit.policy(current_user, :organisation).can_view_organisations? end def should_show_reports_link? diff --git a/app/policies/mou_signature_policy.rb b/app/policies/mou_signature_policy.rb deleted file mode 100644 index 420c8530bf..0000000000 --- a/app/policies/mou_signature_policy.rb +++ /dev/null @@ -1,5 +0,0 @@ -class MouSignaturePolicy < ApplicationPolicy - def can_manage_mous? - user.super_admin? - end -end diff --git a/app/services/navigation_items_service.rb b/app/services/navigation_items_service.rb index 304c44dd66..f80273c047 100644 --- a/app/services/navigation_items_service.rb +++ b/app/services/navigation_items_service.rb @@ -22,7 +22,6 @@ def navigation_items navigation_items = [ your_groups_navigation_item, - mou_navigation_item, users_navigation_item, organisations_navigation_item, reports_navigation_item, @@ -42,12 +41,6 @@ def your_groups_navigation_item NavigationItem.new(text: I18n.t("header.your_groups"), href: "/", active: false) end - def mou_navigation_item - return nil unless should_show_mous_link? - - NavigationItem.new(text: I18n.t("header.mous"), href: mou_signatures_path, active: false) - end - def users_navigation_item return nil unless should_show_user_profile_link? @@ -96,10 +89,6 @@ def should_show_user_profile_link? Pundit.policy(user, :user).can_manage_user? end - def should_show_mous_link? - Pundit.policy(user, :mou_signature).can_manage_mous? - end - def should_show_organisations_link? Pundit.policy(user, :organisation).can_view_organisations? end diff --git a/app/views/mou_signatures/index.html.erb b/app/views/mou_signatures/index.html.erb deleted file mode 100644 index 94a26d061b..0000000000 --- a/app/views/mou_signatures/index.html.erb +++ /dev/null @@ -1,42 +0,0 @@ -<% set_page_title(t("page_titles.mou_signatures")) %> -

<%= t("page_titles.mou_signatures") %>

- -
- <%= t("mou_signatures.index.preamble_html") %> - - <%= t("mou_signatures.index.share_mou_link_html", mou_link: link_to(mou_signature_url, mou_signature_url)) %> - - <%= t("mou_signatures.index.share_non_crown_agreement_link_html", agreement_link: link_to(non_crown_agreement_signature_url, non_crown_agreement_signature_url)) %> -
- -<% if mou_signatures.any? %> - <%= render ScrollingWrapperComponent::View.new(aria_label: t("mou_signatures.index.table_caption")) do |table| %> - <%= govuk_table do |table| %> - <%= table.with_caption(size: 'm', text: t("mou_signatures.index.table_caption")) %> - - <%= table.with_head do |head| - head.with_row do |row| - row.with_cell(header: true, text: t("mou_signatures.index.table_headings.agreement_type")) - row.with_cell(header: true, text: t("mou_signatures.index.table_headings.name")) - row.with_cell(header: true, text: t("mou_signatures.index.table_headings.email")) - row.with_cell(header: true, text: t("mou_signatures.index.table_headings.organisation")) - row.with_cell(header: true, text: t("mou_signatures.index.table_headings.agreed_at")) - end - end %> - - <%= table.with_body do |body| - mou_signatures.each do |mou_signature| - body.with_row do |row| - row.with_cell(text: t("mou_signatures.index.agreement_type.#{mou_signature.agreement_type}")) - row.with_cell(text: mou_signature.user.name.presence || t("users.index.name_blank")) - row.with_cell do - govuk_link_to(mou_signature.user.email, edit_user_path(mou_signature.user)) - end - row.with_cell(text: mou_signature.organisation&.name.presence || t("users.index.organisation_blank")) - row.with_cell(text: l(mou_signature.created_at.to_date, :format => :long)) - end - end - end %> - <% end %> - <% end %> -<% end %> diff --git a/app/views/sitemap/index.html.erb b/app/views/sitemap/index.html.erb index f18ceb632b..a1686bfec5 100644 --- a/app/views/sitemap/index.html.erb +++ b/app/views/sitemap/index.html.erb @@ -20,8 +20,8 @@ <% end %> -<% if should_show_mous_link %> -

<%= link_to t("page_titles.mou_signatures"), mou_signatures_path %>

+<% if should_show_organisations_link %> +

<%= link_to t("page_titles.organisations"), organisations_path %>

diff --git a/config/locales/en.yml b/config/locales/en.yml index f1c081b549..badd897225 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -928,7 +928,6 @@ en: Guidance will be displayed at the top of the page, above your question.

header: - mous: MOUs organisations: Organisations product_name: Forms reports: Reports @@ -1443,13 +1442,11 @@ en: preamble_html: "

Someone from each organisation needs to agree to one of the GOV.UK Forms agreements before an organisation admin can be assigned or any groups upgraded to ‘active’.

\n" share_mou_link_html: "

For Crown organisations, like government departments and executive agencies, share the Memorandum of Understanding:
%{mou_link}

\n" share_non_crown_agreement_link_html: "

For non-crown organisations, like local authorities, share the GOV.UK Forms agreement:
%{agreement_link}

\n" - table_caption: Agreed MOUs and agreements table_headings: agreed_at: Agreed on agreement_type: Agreement type email: Email address name: Name - organisation: Organisation show: crown: banner: @@ -1652,7 +1649,6 @@ en: missing_draft_question: There’s a problem with this page mou_signature_confirmation: You’ve agreed to the MOU mou_signature_new: GOV.UK Forms Memorandum of Understanding - mou_signatures: MOUs and agreements move_form: Move this form to a different group name_settings: Ask for a person’s name new_group: Give your group a name diff --git a/config/routes.rb b/config/routes.rb index ed1675a0c9..83cd523d5e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -215,8 +215,6 @@ resource :contact_for_research, controller: :contact_for_research, only: %i[edit update] end - resources :mou_signatures, only: %i[index], path: "mous" - resources :organisations, only: %i[index show] resource :mou_signature, only: %i[new show create], path: "/memorandum-of-understanding", defaults: { agreement_type: :crown }, as: :mou_signature do diff --git a/spec/components/service_navigation_component/service_navigation_component_preview.rb b/spec/components/service_navigation_component/service_navigation_component_preview.rb index e79de933cc..511e1e9954 100644 --- a/spec/components/service_navigation_component/service_navigation_component_preview.rb +++ b/spec/components/service_navigation_component/service_navigation_component_preview.rb @@ -15,8 +15,8 @@ def with_navigation_links def with_super_admin_navigation_links render(ServiceNavigationComponent::View.new(navigation_items: [ { text: "Your groups", href: "/" }, - { text: "MOUs", href: "/mous" }, { text: "Users", href: "/users" }, + { text: "Organisations", href: "/organisations" }, { text: "Reports", href: "/reports" }, { text: "Support", href: "/support" }, { text: "A User", href: nil, classes: ["app-service-navigation__item--featured"] }, diff --git a/spec/features/mou/upgrade_user_changes_mou_spec.rb b/spec/features/mou/upgrade_user_changes_mou_spec.rb index 5adf9b31b2..1e70a2e474 100644 --- a/spec/features/mou/upgrade_user_changes_mou_spec.rb +++ b/spec/features/mou/upgrade_user_changes_mou_spec.rb @@ -3,48 +3,34 @@ describe "Assign an organisation to a user with a signed MOU", type: :feature do let(:user) { create :user, name: "Test User", organisation: nil } let!(:mou_signature) { create(:mou_signature, user:, organisation: nil, created_at: Time.zone.parse("September 1, 2023")) } - let(:organisation) { create :organisation } + let!(:organisation) { create :organisation, slug: "department-for-testing" } - it "a logged in user can sign an MOU" do + it "assigning an organisation to a user adds it to their MOU" do login_as_super_admin_user - when_i_visit_the_mou_index_page - then_i_can_see_the_mou_page - then_i_see_the_mou_with_no_organisation - then_i_visit_the_users_page - then_i_update_the_user_organisation - when_i_visit_the_mou_index_page - then_i_can_see_the_mou_page + when_i_update_the_user_organisation + then_i_visit_the_organisation_page then_i_see_the_mou_with_organisation end private - def when_i_visit_the_mou_index_page - visit mou_signatures_path - end - - def then_i_can_see_the_mou_page - expect(page).to have_text "MOUs and agreements" - end - - def then_i_see_the_mou_with_no_organisation - expect(page).to have_text mou_signature.user.name - expect(page).to have_text mou_signature.user.email - expect(page).to have_text "No organisation" + def when_i_update_the_user_organisation + visit edit_user_path(user) + organisation_field = find_field "Organisation" + # The \n is important, it "presses enter" to confirm the autocomplete selection + organisation_field.fill_in with: "#{organisation.name}\n" + expect(organisation_field.value).to start_with organisation.name + click_button "Save" end - def then_i_visit_the_users_page - click_link mou_signature.user.email - end - - def then_i_update_the_user_organisation - fill_in "Organisation", with: "#{organisation.name}\n" - click_button "Save" - sleep 1 + def then_i_visit_the_organisation_page + visit organisation_path(organisation) end def then_i_see_the_mou_with_organisation + expect(page).to have_text "MOUs and agreements" expect(page).to have_text mou_signature.user.name - expect(page).to have_text organisation.name + expect(page).to have_link mou_signature.user.email + expect(page).to have_text "September 01, 2023" end end diff --git a/spec/features/mou/view_signed_mous_spec.rb b/spec/features/mou/view_signed_mous_spec.rb index cb4773617e..5b7ef49a89 100644 --- a/spec/features/mou/view_signed_mous_spec.rb +++ b/spec/features/mou/view_signed_mous_spec.rb @@ -1,42 +1,35 @@ require "rails_helper" describe "Check which MOUs have been signed", type: :feature do - let(:user) { super_admin_user } - let!(:mou_signatures) do - [create(:mou_signature, created_at: Time.zone.parse("October 12, 2023")), - create(:mou_signature, created_at: Time.zone.parse("September 1, 2023"))] - end + let(:organisation) { create :organisation, slug: "department-for-testing" } + let!(:mou_signature) { create(:mou_signature_for_organisation, organisation:, created_at: Time.zone.parse("October 12, 2023")) } before do login_as_super_admin_user end - it "super_admin's can see the MOUs page" do - then_i_click_on_the_mou_link - then_i_can_see_the_mou_page + it "super_admin's can see an organisation's signed MOUs" do + then_i_click_on_the_organisations_link + then_i_click_on_the_organisation then_i_can_see_the_mou_list end private - def then_i_can_see_the_mou_list - expect(page).to have_text mou_signatures.first.organisation.name - expect(page).to have_link mou_signatures.first.user.email - expect(page).to have_text mou_signatures.first.user.name - expect(page).to have_text "October 12, 2023" - - expect(page).to have_text mou_signatures.second.organisation.name - expect(page).to have_link mou_signatures.second.user.email - expect(page).to have_text mou_signatures.second.user.name - expect(page).to have_text "September 01, 2023" + def then_i_click_on_the_organisations_link + visit root_path + click_link("Organisations") end - def then_i_click_on_the_mou_link - visit root_path - click_link("MOUs") + def then_i_click_on_the_organisation + click_link organisation.name_with_abbreviation end - def then_i_can_see_the_mou_page + def then_i_can_see_the_mou_list expect(page).to have_text "MOUs and agreements" + expect(page).to have_text "Crown MOU" + expect(page).to have_text mou_signature.user.name + expect(page).to have_link mou_signature.user.email + expect(page).to have_text "October 12, 2023" end end diff --git a/spec/policies/mou_signature_policy_spec.rb b/spec/policies/mou_signature_policy_spec.rb deleted file mode 100644 index 75d7cc1062..0000000000 --- a/spec/policies/mou_signature_policy_spec.rb +++ /dev/null @@ -1,19 +0,0 @@ -require "rails_helper" - -describe MouSignaturePolicy do - subject(:policy) { described_class.new(user, :mouSignature) } - - let(:user) { build :super_admin_user } - - context "with super admin" do - it { is_expected.to permit_actions(%i[can_manage_mous]) } - end - - (User.roles.keys - %w[super_admin]).each do |role| - context "with #{role}" do - let(:user) { build :user, role: } - - it { is_expected.to forbid_actions(%i[can_manage_mous]) } - end - end -end diff --git a/spec/services/navigation_items_service_spec.rb b/spec/services/navigation_items_service_spec.rb index 2591101cdb..13d8f0401e 100644 --- a/spec/services/navigation_items_service_spec.rb +++ b/spec/services/navigation_items_service_spec.rb @@ -19,13 +19,11 @@ context "when user is present" do let(:can_manage_user) { false } - let(:can_manage_mous) { false } let(:can_view_reports) { false } let(:can_view_organisations) { false } before do allow(Pundit).to receive(:policy).with(user, :user).and_return(instance_double(UserPolicy, can_manage_user?: can_manage_user)) - allow(Pundit).to receive(:policy).with(user, :mou_signature).and_return(instance_double(MouSignaturePolicy, can_manage_mous?: can_manage_mous)) allow(Pundit).to receive(:policy).with(user, :report).and_return(instance_double(ReportPolicy, can_view_reports?: can_view_reports)) allow(Pundit).to receive(:policy).with(user, :organisation).and_return(instance_double(OrganisationPolicy, can_view_organisations?: can_view_organisations)) allow(Settings.forms_product_page).to receive(:support_url).and_return(support_url) @@ -40,15 +38,6 @@ end end - context "when user can manage mous" do - let(:can_manage_mous) { true } - - it "includes mous in navigation items" do - mous_item = NavigationItemsService::NavigationItem.new(text: I18n.t("header.mous"), href: mou_signatures_path, active: false) - expect(service.navigation_items).to include(mous_item) - end - end - context "when user can view organisations" do let(:can_view_organisations) { true } diff --git a/spec/views/mou_signatures/index.html.erb_spec.rb b/spec/views/mou_signatures/index.html.erb_spec.rb deleted file mode 100644 index 623e036f5f..0000000000 --- a/spec/views/mou_signatures/index.html.erb_spec.rb +++ /dev/null @@ -1,53 +0,0 @@ -require "rails_helper" - -describe "mou_signatures/index.html.erb" do - let(:mou_signatures) do - [ - build(:mou_signature, agreement_type: :crown, created_at: Time.zone.parse("October 12, 2023")), - build(:mou_signature, agreement_type: :non_crown, created_at: Time.zone.parse("October 13, 2023")), - ] - end - - before do - render template: "mou_signatures/index", locals: { mou_signatures: } - end - - it "contains page heading" do - expect(rendered).to have_css("h1.govuk-heading-l", text: I18n.t("page_titles.mou_signatures")) - end - - it "contains a scrollable wrapper with a table in it" do - expect(rendered).to have_css(".app-scrolling-wrapper > table") - end - - it "contains the agreement type" do - expect(rendered).to have_xpath "//thead/tr/th[1]", text: I18n.t("mou_signatures.index.table_headings.agreement_type") - expect(rendered).to have_xpath "//tbody/tr[1]/td[1]", text: I18n.t("mou_signatures.index.agreement_type.crown") - expect(rendered).to have_xpath "//tbody/tr[2]/td[1]", text: I18n.t("mou_signatures.index.agreement_type.non_crown") - end - - it "contains the user's name" do - expect(rendered).to have_text(mou_signatures.first.user.name) - end - - it "contains the user's email as a link to the edit page" do - expect(rendered).to have_link(mou_signatures.first.user.email, href: edit_user_path(mou_signatures.first.user)) - end - - it "contains organisation name" do - expect(rendered).to have_text(mou_signatures.first.user.organisation.name) - end - - it "contains the date the MOU was signed" do - expect(rendered).to have_text(I18n.l(mou_signatures.first.created_at.to_date, format: :long)) - end - - context "when there are no signed MOUs" do - let(:mou_signatures) { [] } - - it "does not show the MOU table" do - expect(rendered).not_to have_text(I18n.t("mou_signatures.table_caption")) - expect(rendered).not_to have_css("table") - end - end -end diff --git a/spec/views/sitemap/index.html.erb_spec.rb b/spec/views/sitemap/index.html.erb_spec.rb index 604fa066a9..b435b8f3eb 100644 --- a/spec/views/sitemap/index.html.erb_spec.rb +++ b/spec/views/sitemap/index.html.erb_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" describe "sitemap/index.html.erb" do - let(:locals) { { active_groups: [], trial_groups: [], should_show_users_link: false, should_show_mous_link: false, should_show_reports_link: false } } + let(:locals) { { active_groups: [], trial_groups: [], should_show_users_link: false, should_show_organisations_link: false, should_show_reports_link: false } } def render_template render template: "sitemap/index", locals: @@ -59,20 +59,32 @@ def render_template end end - context "when the should_show_mous_link is true" do - let(:locals) { super().merge(should_show_mous_link: true) } + context "when the should_show_organisations_link is true" do + let(:locals) { super().merge(should_show_organisations_link: true) } - it "includes a link to the MOUs page" do + it "includes a link to the organisations page" do + render_template + + expect(rendered).to have_link("Organisations", href: organisations_path) + end + + it "includes a link to the MOU signing page" do render_template expect(rendered).to have_link("Memorandum of Understanding", href: mou_signature_url) end end - context "when should_show_mous_link is false" do - let(:locals) { super().merge(should_show_mous_link: false) } + context "when should_show_organisations_link is false" do + let(:locals) { super().merge(should_show_organisations_link: false) } + + it "does not include a link to the organisations page" do + render_template + + expect(rendered).not_to have_link("Organisations") + end - it "does not include a link to the MOUs page" do + it "does not include a link to the MOU signing page" do render_template expect(rendered).not_to have_link("Memorandum of Understanding")