diff --git a/app/controllers/assessor_interface/application_forms_controller.rb b/app/controllers/assessor_interface/application_forms_controller.rb index 93f0c6c601..627e40ccba 100644 --- a/app/controllers/assessor_interface/application_forms_controller.rb +++ b/app/controllers/assessor_interface/application_forms_controller.rb @@ -50,20 +50,37 @@ def timeline .order(created_at: :desc) end - def edit + def edit_email + @form = ApplicationFormEmailForm.new + end + + def update_email + @form = + ApplicationFormEmailForm.new( + email_form_params.merge(application_form:, user: current_staff), + ) + + if @form.save + redirect_to [:assessor_interface, application_form] + else + render :edit_email, status: :unprocessable_entity + end + end + + def edit_name @form = ApplicationFormNameForm.new end - def update + def update_name @form = ApplicationFormNameForm.new( - form_params.merge(application_form:, user: current_staff), + name_form_params.merge(application_form:, user: current_staff), ) if @form.save redirect_to [:assessor_interface, application_form] else - render :edit, status: :unprocessable_entity + render :edit_name, status: :unprocessable_entity end end @@ -99,7 +116,13 @@ def application_form @application_form ||= show_view_object.application_form end - def form_params + def email_form_params + params.require(:assessor_interface_application_form_email_form).permit( + :email, + ) + end + + def name_form_params params.require(:assessor_interface_application_form_name_form).permit( :given_names, :family_name, diff --git a/app/helpers/application_form_helper.rb b/app/helpers/application_form_helper.rb index dcb53e814f..dd13d4e353 100644 --- a/app/helpers/application_form_helper.rb +++ b/app/helpers/application_form_helper.rb @@ -32,10 +32,10 @@ def application_form_summary_rows( if AssessorInterface::ApplicationFormPolicy.new( current_staff, application_form, - ).edit? + ).edit_name? { visually_hidden_text: I18n.t("application_form.summary.name"), - href: [:edit, :assessor_interface, application_form], + href: [:name, :assessor_interface, application_form], } end, ].compact, @@ -76,6 +76,17 @@ def application_form_summary_rows( end ), }, + actions: [ + if AssessorInterface::ApplicationFormPolicy.new( + current_staff, + application_form, + ).edit_email? + { + visually_hidden_text: I18n.t("application_form.summary.email"), + href: [:email, :assessor_interface, application_form], + } + end, + ].compact, }, { key: { diff --git a/app/policies/assessor_interface/application_form_policy.rb b/app/policies/assessor_interface/application_form_policy.rb index b168ba3040..0d00bb1d3a 100644 --- a/app/policies/assessor_interface/application_form_policy.rb +++ b/app/policies/assessor_interface/application_form_policy.rb @@ -16,10 +16,18 @@ def show? alias_method :timeline?, :show? alias_method :show_pdf?, :show? - def update? + def update_email? + user.change_email_permission + end + + alias_method :edit_email?, :update_email? + + def update_name? user.change_name_permission end + alias_method :edit_name?, :update_name? + def destroy? user.withdraw_permission end diff --git a/app/views/assessor_interface/application_forms/edit_email.html.erb b/app/views/assessor_interface/application_forms/edit_email.html.erb new file mode 100644 index 0000000000..42cbf14a1b --- /dev/null +++ b/app/views/assessor_interface/application_forms/edit_email.html.erb @@ -0,0 +1,26 @@ +<% title = "Change email for ‘#{application_form_full_name(@application_form)}’" %> + +<% content_for :page_title, title_with_error_prefix(title, error: @form.errors.any?) %> +<% content_for :back_link_url, back_history_path(default: assessor_interface_application_form_path(@application_form)) %> + +<%= form_with model: @form, url: [:email, :assessor_interface, @application_form] do |f| %> + <%= f.govuk_error_summary %> + +

<%= title %>

+ +

Current email

+ + <%= govuk_summary_list(actions: false) do |summary_list| + summary_list.with_row do |row| + row.with_key { "Email" } + row.with_value { @application_form.teacher.email } + end + end %> + +

Change email

+

Use this form to change the email address.

+ + <%= f.govuk_text_field :email %> + + <%= render "shared/assessor_interface/continue_cancel_button", f: %> +<% end %> diff --git a/app/views/assessor_interface/application_forms/edit.html.erb b/app/views/assessor_interface/application_forms/edit_name.html.erb similarity index 83% rename from app/views/assessor_interface/application_forms/edit.html.erb rename to app/views/assessor_interface/application_forms/edit_name.html.erb index 1bc86c5e1c..8b6537e611 100644 --- a/app/views/assessor_interface/application_forms/edit.html.erb +++ b/app/views/assessor_interface/application_forms/edit_name.html.erb @@ -1,9 +1,9 @@ -<% title = "Change details for ‘#{application_form_full_name(@application_form)}’" %> +<% title = "Change name for ‘#{application_form_full_name(@application_form)}’" %> <% content_for :page_title, title_with_error_prefix(title, error: @form.errors.any?) %> <% content_for :back_link_url, back_history_path(default: assessor_interface_application_form_path(@application_form)) %> -<%= form_with model: @form, url: [:assessor_interface, @application_form], method: :put do |f| %> +<%= form_with model: @form, url: [:name, :assessor_interface, @application_form] do |f| %> <%= f.govuk_error_summary %>

<%= title %>

diff --git a/config/routes.rb b/config/routes.rb index 9051b5d2be..97d6f3b76c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -16,7 +16,7 @@ resources :application_forms, path: "/applications", - except: :new, + only: %i[index show destroy], param: :reference do collection do post "filters/apply", to: "application_forms#apply_filters" @@ -24,6 +24,12 @@ end member do + get "email", to: "application_forms#edit_email" + post "email", to: "application_forms#update_email" + + get "name", to: "application_forms#edit_name" + post "name", to: "application_forms#update_name" + get "status" get "timeline" get "withdraw" diff --git a/spec/factories/staff.rb b/spec/factories/staff.rb index ebe498b11b..96a915d977 100644 --- a/spec/factories/staff.rb +++ b/spec/factories/staff.rb @@ -65,6 +65,10 @@ assess_permission { true } end + trait :with_change_email_permission do + change_email_permission { true } + end + trait :with_change_name_permission do change_name_permission { true } end diff --git a/spec/helpers/application_form_helper_spec.rb b/spec/helpers/application_form_helper_spec.rb index 0166f35e65..7297ed72a5 100644 --- a/spec/helpers/application_form_helper_spec.rb +++ b/spec/helpers/application_form_helper_spec.rb @@ -77,6 +77,7 @@ value: { text: application_form.teacher.email, }, + actions: [], }, { key: { text: "Created on" }, value: { text: " 1 January 2020" } }, { @@ -129,6 +130,31 @@ ) end + context "user has change email permission" do + let(:current_staff) { create(:staff, :with_change_email_permission) } + + it "has an action to change the name" do + email_row = summary_rows.find { |row| row[:key][:text] == "Email" } + + expect(email_row).to eq( + { + key: { + text: "Email", + }, + value: { + text: application_form.teacher.email, + }, + actions: [ + { + visually_hidden_text: "Email", + href: [:email, :assessor_interface, application_form], + }, + ], + }, + ) + end + end + context "user has change name permission" do let(:current_staff) { create(:staff, :with_change_name_permission) } @@ -146,7 +172,7 @@ actions: [ { visually_hidden_text: "Name", - href: [:edit, :assessor_interface, application_form], + href: [:name, :assessor_interface, application_form], }, ], }, diff --git a/spec/policies/assessor_interface/application_form_policy_spec.rb b/spec/policies/assessor_interface/application_form_policy_spec.rb index 0e3041f8f5..107a1898e2 100644 --- a/spec/policies/assessor_interface/application_form_policy_spec.rb +++ b/spec/policies/assessor_interface/application_form_policy_spec.rb @@ -62,6 +62,26 @@ describe "#edit?" do subject(:edit?) { policy.edit? } + it_behaves_like "a policy method without permission" + end + + describe "#update_email?" do + subject(:update?) { policy.update_email? } + it_behaves_like "a policy method requiring the change email permission" + end + + describe "#edit_email?" do + subject(:edit?) { policy.edit_email? } + it_behaves_like "a policy method requiring the change email permission" + end + + describe "#update_name?" do + subject(:update?) { policy.update_name? } + it_behaves_like "a policy method requiring the change name permission" + end + + describe "#edit_name?" do + subject(:edit?) { policy.edit_name? } it_behaves_like "a policy method requiring the change name permission" end diff --git a/spec/support/autoload/page_objects/assessor_interface/edit_application_email.rb b/spec/support/autoload/page_objects/assessor_interface/edit_application_email.rb new file mode 100644 index 0000000000..c139374464 --- /dev/null +++ b/spec/support/autoload/page_objects/assessor_interface/edit_application_email.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module PageObjects + module AssessorInterface + class EditApplicationEmail < SitePrism::Page + set_url "/assessor/applications/{reference}/email" + + section :form, "form" do + element :email_field, + "#assessor-interface-application-form-email-form-email-field" + element :submit_button, ".govuk-button" + end + end + end +end diff --git a/spec/support/autoload/page_objects/assessor_interface/edit_application.rb b/spec/support/autoload/page_objects/assessor_interface/edit_application_name.rb similarity index 80% rename from spec/support/autoload/page_objects/assessor_interface/edit_application.rb rename to spec/support/autoload/page_objects/assessor_interface/edit_application_name.rb index aaa97964b1..e1e32ab631 100644 --- a/spec/support/autoload/page_objects/assessor_interface/edit_application.rb +++ b/spec/support/autoload/page_objects/assessor_interface/edit_application_name.rb @@ -2,8 +2,8 @@ module PageObjects module AssessorInterface - class EditApplication < SitePrism::Page - set_url "/assessor/applications/{reference}/edit" + class EditApplicationName < SitePrism::Page + set_url "/assessor/applications/{reference}/name" section :form, "form" do element :given_names_field, diff --git a/spec/support/page_helpers.rb b/spec/support/page_helpers.rb index 0967b73dbd..77cb01c8b8 100644 --- a/spec/support/page_helpers.rb +++ b/spec/support/page_helpers.rb @@ -107,9 +107,14 @@ def assessor_edit_age_range_subjects_assessment_recommendation_award_page PageObjects::AssessorInterface::EditAgeRangeSubjectsAssessmentRecommendationAward.new end - def assessor_edit_application_page - @assessor_edit_application_page ||= - PageObjects::AssessorInterface::EditApplication.new + def assessor_edit_application_email_page + @assessor_edit_application_email_page ||= + PageObjects::AssessorInterface::EditApplicationEmail.new + end + + def assessor_edit_application_name_page + @assessor_edit_application_name_page ||= + PageObjects::AssessorInterface::EditApplicationName.new end def assessor_edit_work_history_page diff --git a/spec/support/shared_examples/policy.rb b/spec/support/shared_examples/policy.rb index 13d575d61b..b0b25b334d 100644 --- a/spec/support/shared_examples/policy.rb +++ b/spec/support/shared_examples/policy.rb @@ -37,6 +37,18 @@ end end +RSpec.shared_examples "a policy method requiring the change email permission" do + context "without permission" do + let(:user) { create(:staff) } + it { is_expected.to be false } + end + + context "with permission" do + let(:user) { create(:staff, :with_change_email_permission) } + it { is_expected.to be true } + end +end + RSpec.shared_examples "a policy method requiring the change name permission" do context "without permission" do let(:user) { create(:staff) } diff --git a/spec/system/assessor_interface/change_application_form_email_spec.rb b/spec/system/assessor_interface/change_application_form_email_spec.rb new file mode 100644 index 0000000000..0dd04fedb0 --- /dev/null +++ b/spec/system/assessor_interface/change_application_form_email_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe "Assessor change application form email", type: :system do + before { given_there_is_an_application_form } + + it "checks manage applications permission" do + given_i_am_authorized_as_a_user(assessor) + + when_i_visit_the(:assessor_edit_application_email_page, reference:) + then_i_see_the_forbidden_page + end + + it "allows changing application form name" do + given_i_am_authorized_as_a_user(manager) + + when_i_visit_the(:assessor_application_page, reference:) + then_i_see_the(:assessor_application_page) + + when_i_click_on_change_email + then_i_see_the(:assessor_edit_application_email_page, reference:) + + when_i_fill_in_the_email + then_i_see_the(:assessor_application_page, reference:) + end + + def given_there_is_an_application_form + application_form + end + + def when_i_click_on_change_email + assessor_application_page + .summary_list + .find_row(key: "Email") + .actions + .link + .click + end + + def when_i_fill_in_the_email + assessor_edit_application_email_page.form.email_field.fill_in with: + "new@example.com" + assessor_edit_application_email_page.form.submit_button.click + end + + def application_form + @application_form ||= + create( + :application_form, + :submitted, + :with_personal_information, + :with_assessment, + ) + end + + delegate :reference, to: :application_form + + def assessor + create(:staff, :confirmed, :with_assess_permission) + end + + def manager + create(:staff, :confirmed, :with_change_email_permission) + end +end diff --git a/spec/system/assessor_interface/change_application_form_name_spec.rb b/spec/system/assessor_interface/change_application_form_name_spec.rb index 66d68216c6..90b7cc08dc 100644 --- a/spec/system/assessor_interface/change_application_form_name_spec.rb +++ b/spec/system/assessor_interface/change_application_form_name_spec.rb @@ -8,7 +8,7 @@ it "checks manage applications permission" do given_i_am_authorized_as_a_user(assessor) - when_i_visit_the(:assessor_edit_application_page, reference:) + when_i_visit_the(:assessor_edit_application_name_page, reference:) then_i_see_the_forbidden_page end @@ -19,7 +19,7 @@ then_i_see_the(:assessor_application_page) when_i_click_on_change_name - then_i_see_the(:assessor_edit_application_page, reference:) + then_i_see_the(:assessor_edit_application_name_page, reference:) when_i_fill_in_the_name then_i_see_the(:assessor_application_page, reference:) @@ -39,11 +39,11 @@ def when_i_click_on_change_name end def when_i_fill_in_the_name - assessor_edit_application_page.form.given_names_field.fill_in with: + assessor_edit_application_name_page.form.given_names_field.fill_in with: "New given names" - assessor_edit_application_page.form.family_name_field.fill_in with: + assessor_edit_application_name_page.form.family_name_field.fill_in with: "New family name" - assessor_edit_application_page.form.submit_button.click + assessor_edit_application_name_page.form.submit_button.click end def application_form