Merge branch 'develop' into feature/2288-2291-fix-ph-fields

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"

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

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 },

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

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?

cosmetics-web/app/views/responsible_persons/notifications/_show.html.erb

@@ -46,14 +46,16 @@
   <div class="govuk-grid-column-full govuk-!-margin-top-4">
     <div class="govuk-grid-row">
       <div class="govuk-grid-column-two-thirds">
-        <% 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 %>
+        <div class="govuk-button-group">
+          <% 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 %>
+        </div>
       </div>
     </div>
     <% unless @notification.can_be_deleted? %>

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

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 }

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

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

cosmetics-web/spec/requests/responsible_persons/delete_notification_spec.rb

@@ -16,53 +16,25 @@
     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
 
       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