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/input_objects/organisations/filter_input.rb b/app/input_objects/organisations/filter_input.rb index ecd6881a4e..7f5911c151 100644 --- a/app/input_objects/organisations/filter_input.rb +++ b/app/input_objects/organisations/filter_input.rb @@ -19,8 +19,8 @@ def sort_options def agreement_type_options [ OpenStruct.new(label: I18n.t("organisations.index.filter.agreement_type.any")), - OpenStruct.new(label: I18n.t("mou_signatures.index.agreement_type.crown"), value: "crown"), - OpenStruct.new(label: I18n.t("mou_signatures.index.agreement_type.non_crown"), value: "non_crown"), + OpenStruct.new(label: I18n.t("mou_signatures.agreement_type.crown"), value: "crown"), + OpenStruct.new(label: I18n.t("mou_signatures.agreement_type.non_crown"), value: "non_crown"), OpenStruct.new(label: I18n.t("organisations.index.filter.agreement_type.signed"), value: "signed"), OpenStruct.new(label: I18n.t("organisations.index.filter.agreement_type.none"), value: "none"), ] 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 249efb23b6..3d5f4aa178 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, brands_navigation_item, @@ -43,12 +42,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? @@ -103,10 +96,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/organisations/index.html.erb b/app/views/organisations/index.html.erb index df5567cff3..58812b3fb1 100644 --- a/app/views/organisations/index.html.erb +++ b/app/views/organisations/index.html.erb @@ -75,7 +75,7 @@ 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(", ")) + row.with_cell(text: agreement_types.map { |agreement_type| t("mou_signatures.agreement_type.#{agreement_type}") }.join(", ")) else row.with_cell(text: t("organisations.index.agreement_type_none")) end diff --git a/app/views/organisations/show.html.erb b/app/views/organisations/show.html.erb index e5eeb3fa20..4138b714ea 100644 --- a/app/views/organisations/show.html.erb +++ b/app/views/organisations/show.html.erb @@ -69,17 +69,17 @@ <%= govuk_table do |table| %> <%= 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.agreed_at")) + row.with_cell(header: true, text: t("organisations.show.mou_signatures.table_headings.agreement_type")) + row.with_cell(header: true, text: t("organisations.show.mou_signatures.table_headings.name")) + row.with_cell(header: true, text: t("organisations.show.mou_signatures.table_headings.email")) + row.with_cell(header: true, text: t("organisations.show.mou_signatures.table_headings.agreed_at")) end end %> <%= table.with_body do |body| @organisation.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: t("mou_signatures.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)) @@ -93,11 +93,11 @@ <% else %>

<%= t("organisations.show.mou_signatures.none") %>

- <%= t("mou_signatures.index.preamble_html") %> + <%= t("organisations.show.mou_signatures.preamble_html") %> - <%= t("mou_signatures.index.share_mou_link_html", mou_link: link_to(mou_signature_url, mou_signature_url)) %> + <%= t("organisations.show.mou_signatures.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)) %> + <%= t("organisations.show.mou_signatures.share_non_crown_agreement_link_html", agreement_link: link_to(non_crown_agreement_signature_url, non_crown_agreement_signature_url)) %> <% end %>

<%= t("organisations.show.domains.heading") %>

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 82a4ee3b41..fdbd91b9f9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -952,7 +952,6 @@ en:

header: brands: Brands - mous: MOUs organisations: Organisations product_name: Forms reports: Reports @@ -1446,6 +1445,9 @@ en: heading_without_dates: Form metrics percentage: "%{number}%" mou_signatures: + agreement_type: + crown: Crown MOU + non_crown: Non-crown agreement confirmation: crown: body_html: | @@ -1455,20 +1457,6 @@ en: body_html: |

What happens next

We’ll email you with any updates to the agreement that are made in the future.

- index: - agreement_type: - crown: Crown MOU - non_crown: Non-crown agreement - 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: @@ -1558,6 +1546,14 @@ en: mou_signatures: heading: MOUs and agreements none: No one has agreed an MOU or agreement for this organisation. + 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_headings: + agreed_at: Agreed on + agreement_type: Agreement type + email: Email address + name: Name not_set: Not set summary: closed: Closed @@ -1704,7 +1700,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 28d0d5024c..879a3e32ac 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] do resources :brands, controller: :organisation_brands, only: %i[new create destroy] patch "default-brand", to: "organisation_brands#update_default", as: :default_brand 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/input_objects/organisations/filter_input_spec.rb b/spec/input_objects/organisations/filter_input_spec.rb index 5f4c5a98ff..7899514422 100644 --- a/spec/input_objects/organisations/filter_input_spec.rb +++ b/spec/input_objects/organisations/filter_input_spec.rb @@ -43,8 +43,8 @@ it "returns the correct options" do expect(described_class.new.agreement_type_options).to eq([ OpenStruct.new(label: I18n.t("organisations.index.filter.agreement_type.any")), - OpenStruct.new(label: I18n.t("mou_signatures.index.agreement_type.crown"), value: "crown"), - OpenStruct.new(label: I18n.t("mou_signatures.index.agreement_type.non_crown"), value: "non_crown"), + OpenStruct.new(label: I18n.t("mou_signatures.agreement_type.crown"), value: "crown"), + OpenStruct.new(label: I18n.t("mou_signatures.agreement_type.non_crown"), value: "non_crown"), OpenStruct.new(label: I18n.t("organisations.index.filter.agreement_type.signed"), value: "signed"), OpenStruct.new(label: I18n.t("organisations.index.filter.agreement_type.none"), value: "none"), ]) 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/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb index f739ee7b13..307a7c9f0c 100644 --- a/spec/requests/organisations_controller_spec.rb +++ b/spec/requests/organisations_controller_spec.rb @@ -203,7 +203,7 @@ def organisation_row_order(response) expect(response.body).to include(organisation.name) expect(response.body).to include(organisation.slug) expect(response.body).to include(organisation.admin_users.first.email) - expect(response.body).to include(I18n.t("mou_signatures.index.agreement_type.#{organisation.mou_signatures.first.agreement_type}")) + expect(response.body).to include(I18n.t("mou_signatures.agreement_type.#{organisation.mou_signatures.first.agreement_type}")) expect(response.body).to include(organisation_domain.domain) end diff --git a/spec/services/navigation_items_service_spec.rb b/spec/services/navigation_items_service_spec.rb index 0c09378216..07877062c6 100644 --- a/spec/services/navigation_items_service_spec.rb +++ b/spec/services/navigation_items_service_spec.rb @@ -19,14 +19,12 @@ 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 } let(:can_view_brands) { 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(Pundit).to receive(:policy).with(user, :brand).and_return(instance_double(BrandPolicy, can_view_brands?: can_view_brands)) @@ -42,15 +40,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 2fb3750615..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, user: build_stubbed(:user), agreement_type: :crown, created_at: Time.zone.parse("October 12, 2023")), - build(:mou_signature, user: build_stubbed(:user), 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/organisations/index.html.erb_spec.rb b/spec/views/organisations/index.html.erb_spec.rb index 67b2268c58..ab2a647b73 100644 --- a/spec/views/organisations/index.html.erb_spec.rb +++ b/spec/views/organisations/index.html.erb_spec.rb @@ -35,8 +35,8 @@ expect(rendered).to have_field("filter[name]") expect(rendered).to have_select("filter[agreement_type]", options: [ I18n.t("organisations.index.filter.agreement_type.any"), - I18n.t("mou_signatures.index.agreement_type.crown"), - I18n.t("mou_signatures.index.agreement_type.non_crown"), + I18n.t("mou_signatures.agreement_type.crown"), + I18n.t("mou_signatures.agreement_type.non_crown"), I18n.t("organisations.index.filter.agreement_type.signed"), I18n.t("organisations.index.filter.agreement_type.none"), ]) @@ -87,7 +87,7 @@ end it "shows the agreement type for each organisation" do - 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[1]/td[5]", text: I18n.t("mou_signatures.agreement_type.crown") expect(rendered).to have_xpath "//tbody/tr[2]/td[5]", text: I18n.t("organisations.index.agreement_type_none") end diff --git a/spec/views/organisations/show.html.erb_spec.rb b/spec/views/organisations/show.html.erb_spec.rb index a7687bd199..1fd1f49557 100644 --- a/spec/views/organisations/show.html.erb_spec.rb +++ b/spec/views/organisations/show.html.erb_spec.rb @@ -52,7 +52,7 @@ it "lists the MOU signatures" do mou_signature = organisation.mou_signatures.first - expect(rendered).to have_text(I18n.t("mou_signatures.index.agreement_type.#{mou_signature.agreement_type}")) + expect(rendered).to have_text(I18n.t("mou_signatures.agreement_type.#{mou_signature.agreement_type}")) expect(rendered).to have_link(mou_signature.user.email, href: edit_user_path(mou_signature.user)) expect(rendered).to have_text(I18n.l(mou_signature.created_at.to_date, format: :long)) 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")