diff --git a/cosmetics-web/app/controllers/responsible_persons/delete_notification_controller.rb b/cosmetics-web/app/controllers/responsible_persons/delete_notification_controller.rb index a3ab287871..b493fa69ff 100644 --- a/cosmetics-web/app/controllers/responsible_persons/delete_notification_controller.rb +++ b/cosmetics-web/app/controllers/responsible_persons/delete_notification_controller.rb @@ -3,14 +3,7 @@ class ResponsiblePersons::DeleteNotificationController < SubmitApplicationContro before_action :validate_responsible_person before_action :set_notification - def delete; end - - def destroy - unless params.dig(:confirmation, :yes) == "yes" - @notification.errors.add(:confirmation, "Select yes if you want to delete this #{@notification.notification_complete? ? 'notification' : 'draft'}") - return render(:delete) - end - + def delete NotificationDeleteService.new(@notification, current_user).call tab = @notification.notification_complete? ? "notified" : "incomplete" diff --git a/cosmetics-web/app/helpers/draft_helper.rb b/cosmetics-web/app/helpers/draft_helper.rb index e80e3f8df4..bacd8527b8 100644 --- a/cosmetics-web/app/helpers/draft_helper.rb +++ b/cosmetics-web/app/helpers/draft_helper.rb @@ -47,6 +47,8 @@ def product_badge(notification) if notification.state_lower_than?(NotificationStateConcern::READY_FOR_NANOMATERIALS) in_progress_badge(id, :product) + elsif notification.under_three_years.nil? + in_progress_badge(id, :product) else complete_badge(id, :product) end diff --git a/cosmetics-web/app/helpers/responsible_persons/notifications_helper.rb b/cosmetics-web/app/helpers/responsible_persons/notifications_helper.rb index 2e0b4450da..f95ce59f01 100644 --- a/cosmetics-web/app/helpers/responsible_persons/notifications_helper.rb +++ b/cosmetics-web/app/helpers/responsible_persons/notifications_helper.rb @@ -77,6 +77,13 @@ def notification_summary_product_rows(notification) actions: notification_step_action(notification, :under_three_years), } end, + if notification.under_three_years.nil? + { + key: { text: "For children under 3" }, + value: { text: "Not answered" }, + actions: notification_step_action(notification, :under_three_years), + } + end, { key: { text: "Number of items" }, value: { text: notification.components.length }, diff --git a/cosmetics-web/app/models/secondary_authentication/operations.rb b/cosmetics-web/app/models/secondary_authentication/operations.rb index cb1e9d4a1e..d7ea0c102a 100644 --- a/cosmetics-web/app/models/secondary_authentication/operations.rb +++ b/cosmetics-web/app/models/secondary_authentication/operations.rb @@ -17,7 +17,7 @@ module Operations CHANGE_EMAIL_ADDRESS => 300, # 5 minutes INVITE_USER => 300, # 5 minutes UNLOCK => 300, # 5 minutes - DELETE_NOTIFICATION => 900, # 15 minutes + DELETE_NOTIFICATION => 30, # 30 seconds SETUP_APP_AUTHENTICATION => 300, # 5 minutes SETUP_SMS_AUTHENTICATION => 300, # 5 minutes }.freeze diff --git a/cosmetics-web/app/validators/accept_and_submit_validator.rb b/cosmetics-web/app/validators/accept_and_submit_validator.rb index 6e84135d27..377be61502 100644 --- a/cosmetics-web/app/validators/accept_and_submit_validator.rb +++ b/cosmetics-web/app/validators/accept_and_submit_validator.rb @@ -1,10 +1,15 @@ class AcceptAndSubmitValidator < ActiveModel::Validator def validate(notification) + validate_notification(notification) validate_nano_materials(notification) validate_image_uploads(notification) validate_ingredients(notification) end + def validate_notification(notification) + notification.errors.add :under_three_years, "You have not confirmed if the product is intended to be used on children under 3 years old" if notification.under_three_years.nil? + end + def validate_nano_materials(notification) missing_nano_materials = notification.missing_nano_materials if missing_nano_materials.present? diff --git a/cosmetics-web/app/views/responsible_persons/delete_notification/delete.html.erb b/cosmetics-web/app/views/responsible_persons/delete_notification/delete.html.erb deleted file mode 100644 index 385e62b875..0000000000 --- a/cosmetics-web/app/views/responsible_persons/delete_notification/delete.html.erb +++ /dev/null @@ -1,32 +0,0 @@ -<% type = @notification.notification_complete? ? 'notification' : 'draft' %> -<% - back_path = if @notification.notification_complete? - responsible_person_notification_path(@notification.responsible_person, @notification) - else - responsible_person_notification_draft_path(@notification.responsible_person, @notification) - end -%> -<% page_title "Delete #{type}", errors: @notification.errors.any? %> -<% content_for :after_header do %> - <%= link_to "Back", back_path, class: "govuk-back-link" %> -<% end %> - -
-
- <%= error_summary(@notification.errors, map_errors: { confirmation: :yes }) %> - <%= form_with model: @notification, scope: :confirmation, url: responsible_person_delete_notification_path(@responsible_person, @notification), html: { novalidate: true}, method: :delete do |form| %> - <%= govukCheckboxes( - form: form, - id: "confirmation", - key: :confirmation, - hint: { text: "The #{@notification.product_name} product notification #{ @notification.notification_complete? ? '' : 'draft ' }will be deleted." }, - fieldset: { legend: { text: "Do you want to delete this #{type}?", classes: "govuk-fieldset__legend--l", isPageHeading: true } }, - items: [{ id: "yes", key: :yes, value: "yes", text: "Yes", checked: false, disable_ghost: true }]) %> - -
- <%= govukButton(text: "Delete") %> - <%= link_to("Cancel", back_path, class: "govuk-link govuk-link--no-visited-state" ) %> -
- <% end %> -
-
diff --git a/cosmetics-web/app/views/responsible_persons/notifications/_show.html.erb b/cosmetics-web/app/views/responsible_persons/notifications/_show.html.erb index f57a703d13..a85a1b6258 100644 --- a/cosmetics-web/app/views/responsible_persons/notifications/_show.html.erb +++ b/cosmetics-web/app/views/responsible_persons/notifications/_show.html.erb @@ -46,14 +46,16 @@
- <% if @notification.archived? %> - <%= govukButton(text: "Unarchive this notification", href: responsible_person_notification_unarchive_path(@responsible_person, @notification), classes: "govuk-button--secondary") %> - <% else %> - <%= govukButton(text: "Archive this notification", href: responsible_person_notification_choose_archive_reason_path(@responsible_person, @notification, back_to: "notification"), classes: "govuk-button--secondary") %> - <% end %> - <% if @notification.can_be_deleted? %> - <%= govukButton(text: "Delete this notification", href: delete_responsible_person_delete_notification_path(@responsible_person, @notification), classes: "govuk-button--warning") %> - <% end %> +
+ <% if @notification.archived? %> + <%= link_to "Unarchive this notification", responsible_person_notification_unarchive_path(@responsible_person, @notification), class: "govuk-button govuk-button--secondary" %> + <% else %> + <%= link_to "Archive this notification", responsible_person_notification_choose_archive_reason_path(@responsible_person, @notification, back_to: "notification"), class: "govuk-button govuk-button--secondary" %> + <% end %> + <% if @notification.can_be_deleted? %> + <%= link_to "Delete this notification", delete_responsible_person_delete_notification_path(@responsible_person, @notification), class: "govuk-button govuk-button--warning" %> + <% end %> +
<% unless @notification.can_be_deleted? %> diff --git a/cosmetics-web/config/routes.rb b/cosmetics-web/config/routes.rb index 2f62162c00..901b5fe511 100644 --- a/cosmetics-web/config/routes.rb +++ b/cosmetics-web/config/routes.rb @@ -236,7 +236,6 @@ resources :delete_notification, param: :reference_number, controller: "responsible_persons/delete_notification", only: [] do member do get :delete - delete :destroy end end end diff --git a/cosmetics-web/spec/factories/notification.rb b/cosmetics-web/spec/factories/notification.rb index 7468b49f17..5019b8258e 100644 --- a/cosmetics-web/spec/factories/notification.rb +++ b/cosmetics-web/spec/factories/notification.rb @@ -3,6 +3,7 @@ responsible_person sequence(:product_name) { |n| "Product #{n}" } cpnp_reference { nil } + under_three_years { true } factory :draft_notification do state { NotificationStateConcern::COMPONENTS_COMPLETE } diff --git a/cosmetics-web/spec/features/submit/notifications_deletion_spec.rb b/cosmetics-web/spec/features/submit/notifications_deletion_spec.rb index a1d12b8526..a36978fe53 100644 --- a/cosmetics-web/spec/features/submit/notifications_deletion_spec.rb +++ b/cosmetics-web/spec/features/submit/notifications_deletion_spec.rb @@ -17,15 +17,6 @@ click_on draft_notification.product_name click_on "Delete this draft" - expect(page).to have_h1("Do you want to delete this draft?") - expect(page).to have_text("The #{draft_notification.product_name} product notification draft will be deleted.") - click_button "Delete" - - expect(page).to have_css("h2#error-summary-title", text: "There is a problem") - expect(page).to have_link("Select yes if you want to delete this draft", href: "#yes") - check "Yes" - click_button "Delete" - assert_text "#{draft_notification.product_name} notification deleted" end @@ -35,15 +26,6 @@ click_on "View #{notification.product_name}" click_on "Delete this notification" - expect(page).to have_h1("Do you want to delete this notification?") - expect(page).to have_text("The #{notification.product_name} product notification will be deleted.") - click_button "Delete" - - expect(page).to have_css("h2#error-summary-title", text: "There is a problem") - expect(page).to have_link("Select yes if you want to delete this notification", href: "#yes") - check "Yes" - click_button "Delete" - assert_text "#{notification.product_name} notification deleted" log = NotificationDeleteLog.first diff --git a/cosmetics-web/spec/helpers/responsible_persons/notifications_helper_spec.rb b/cosmetics-web/spec/helpers/responsible_persons/notifications_helper_spec.rb index e57000d2ff..1dc83a2025 100644 --- a/cosmetics-web/spec/helpers/responsible_persons/notifications_helper_spec.rb +++ b/cosmetics-web/spec/helpers/responsible_persons/notifications_helper_spec.rb @@ -162,9 +162,9 @@ end describe "for children under 3" do - it "not included when not available for the notification" do + it "included when not available for the notification" do notification.under_three_years = nil - expect(summary_product_rows).not_to include(hash_including(key: { text: "For children under 3" })) + expect(summary_product_rows).to include(hash_including({ key: { text: "For children under 3" }, value: { text: "Not answered" } })) end it "included when notification product is for children under 3" do @@ -277,9 +277,9 @@ end describe "for children under 3" do - it "not included when not available for the notification" do + it "included when not available for the notification" do notification.under_three_years = nil - expect(summary_product_rows).not_to include(hash_including(key: { text: "For children under 3" })) + expect(summary_product_rows).to include(hash_including({ key: { text: "For children under 3" }, value: { text: "Not answered" }, actions: { items: [hash_including({ href: "#{product_href}/under_three_years" })] } })) end it "included when notification product is for children under 3" do diff --git a/cosmetics-web/spec/requests/responsible_persons/delete_notification_spec.rb b/cosmetics-web/spec/requests/responsible_persons/delete_notification_spec.rb index ae4fac4afe..4554518841 100644 --- a/cosmetics-web/spec/requests/responsible_persons/delete_notification_spec.rb +++ b/cosmetics-web/spec/requests/responsible_persons/delete_notification_spec.rb @@ -16,16 +16,10 @@ end context "when deletion is confirmed" do - it "redirects" do - delete responsible_person_delete_notification_path(responsible_person, draft_notification, confirmation: { yes: "yes" }) - - expect(response.status).to eq 302 - end - it "creates log record with current user" do expect(NotificationDeleteLog.count).to eq 0 - delete responsible_person_delete_notification_path(responsible_person, notification, confirmation: { yes: "yes" }) + get delete_responsible_person_delete_notification_path(responsible_person, notification) expect(NotificationDeleteLog.first.submit_user).to eq user end @@ -33,36 +27,14 @@ it "removes record" do draft_notification expect { - delete responsible_person_delete_notification_url(responsible_person, draft_notification, confirmation: { yes: "yes" }) + get delete_responsible_person_delete_notification_path(responsible_person, notification) }.to change(Notification.deleted, :count).from(0).to(1) end end - context "when deletion is not confirmed" do - it "renders the deletion page" do - delete responsible_person_delete_notification_path(responsible_person, draft_notification, confirmation: { yes: "0" }) - - expect(response.status).to eq 200 - expect(response).to render_template("responsible_persons/delete_notification/delete") - end - - it "does not create a notification delete log" do - expect { - delete responsible_person_delete_notification_path(responsible_person, notification, confirmation: { yes: "0" }) - }.not_to change(NotificationDeleteLog, :count) - end - - it "does not remove record" do - notification - expect { - delete responsible_person_delete_notification_url(responsible_person, notification, confirmation: { yes: "0" }) - }.not_to change(Notification, :count) - end - end - context "when 2FA time passed", :with_2fa do before do - get delete_responsible_person_delete_notification_url(responsible_person, draft_notification, confirmation: { yes: "yes" }) + get delete_responsible_person_delete_notification_url(responsible_person, draft_notification) post secondary_authentication_sms_url, params: { secondary_authentication_form: { @@ -76,18 +48,18 @@ it "does not remove record" do expect { - delete responsible_person_delete_notification_url(responsible_person, draft_notification, confirmation: { yes: "yes" }) + get delete_responsible_person_delete_notification_url(responsible_person, draft_notification) }.not_to change(Notification, :count) end it "redirects to 2FA" do draft_notification - delete responsible_person_delete_notification_path(responsible_person, draft_notification, confirmation: { yes: "yes" }) + get delete_responsible_person_delete_notification_path(responsible_person, draft_notification) expect(response).to redirect_to("/two-factor/sms") end it "redirects info page to 2FA" do - get delete_responsible_person_delete_notification_path(responsible_person, draft_notification, confirmation: { yes: "yes" }) + get delete_responsible_person_delete_notification_path(responsible_person, draft_notification) expect(response.status).to be(302) end @@ -101,7 +73,7 @@ it "does not succeed" do expect { - delete responsible_person_delete_notification_path(other_responsible_person, draft_notification) + get delete_responsible_person_delete_notification_path(other_responsible_person, draft_notification) }.to raise_error(Pundit::NotAuthorizedError) end end