Notification creation from the check form

app/controllers/checks_controller.rb

@@ -29,9 +29,8 @@ class ChecksController < ApplicationController
     build_empty_notification
   end
 
-  def create
-    @check = Check.new(new_check_params)
-    @check.user = current_user
+  def create # rubocop:disable Metrics/AbcSize
+    @check = Check.new(new_check_params.merge(user: current_user))
     authorize @check
 
     if @check.save
@@ -39,6 +38,8 @@ class ChecksController < ApplicationController
       redirect_to checks_path
     else
       flash.now[:alert] = t("checks.invalid", scope: :flashes)
+
+      fill_or_build_new_notification
       render :new
     end
   end
@@ -53,7 +54,8 @@ class ChecksController < ApplicationController
       redirect_to checks_path
     else
       flash.now[:alert] = t("checks.invalid", scope: :flashes)
-      build_empty_notification
+
+      fill_or_build_new_notification
       render :edit
     end
   end
@@ -81,13 +83,32 @@ class ChecksController < ApplicationController
   end
 
   def check_params(*others)
-    params.require(:check)
-          .permit(:domain, :domain_created_at, :comment, :vendor, :round_robin, *others,
-                  notifications_attributes: [:id, :channel, :recipient, :interval])
+    permitted_params = params.require(:check)
+                             .permit(:domain, :domain_created_at, :comment, :vendor, :round_robin, *others,
+                             notification_ids: [],
+                             notifications_attributes: [:channel, :label, :recipient, :interval])
+
+    permitted_params[:notifications_attributes].each_pair do |_key, attributes|
+      attributes.merge!(user: current_user)
+    end
+
+    permitted_params
   end
 
   def build_empty_notification
-    @check.notifications.build
+    @new_notification = @check.notifications.build
+    @new_notification.recipient = current_user.email
+  end
+
+  def fill_or_build_new_notification
+    last_notification = @check.notifications.last
+
+    # user has filled a new notification: we use it for the form
+    if last_notification.new_record?
+      @new_notification = last_notification
+    else # otherwise, set a new empty notification
+      build_empty_notification
+    end
   end
 
   def current_sort

app/controllers/notifications_controller.rb

@@ -11,7 +11,7 @@ class NotificationsController < ApplicationController
     @notifications = policy_scope(Notification).order(checks_count: :desc)
   end
 
-  def new;
+  def new
     @notification = Notification.new
     authorize @notification
     @notification.recipient = current_user.email
@@ -31,7 +31,6 @@ class NotificationsController < ApplicationController
     end
   end
 
-
   def edit; end
 
   def update

app/helpers/check_notifications_helper.rb

@@ -2,7 +2,4 @@
 # License: GNU AGPL-3+ (see full text in LICENSE file)
 
 module CheckNotificationsHelper
-  def recipient_col_class
-    many_channels_available? ? "col-md-7" : "col-md-9"
-  end
 end

app/helpers/notifications_helper.rb

@@ -2,4 +2,8 @@ module NotificationsHelper
   def many_channels_available?
     Notification.channels.many?
   end
+
+  def notification_variable_col_class
+    many_channels_available? ? "col-md-4" : "col-md-5"
+  end
 end

app/models/check.rb

@@ -34,11 +34,11 @@ class Check < ApplicationRecord
   belongs_to :user
   has_many :logs, class_name: "CheckLog", dependent: :destroy
   has_many :check_notifications, dependent: :destroy
-  has_many :notifications, through: :check_notifications, validate: true
+  has_many :notifications, -> { order(checks_count: :desc) },
+                          through: :check_notifications, validate: true
 
   accepts_nested_attributes_for :notifications,
-    allow_destroy: true,
-    reject_if: lambda { |at| at["recipient"].blank? && at["interval"].blank? }
+    reject_if: lambda { |att| att["label"].blank? && att["interval"].blank? }
 
   enum kind: [:domain, :ssl]
 

app/services/notifier/channels/email.rb

@@ -17,11 +17,15 @@ module Notifier
 
       # Expiration notifications
       def domain_notify_expires_soon(check_notification)
-        NotificationsMailer.with(check_notification: check_notification).domain_expires_soon.deliver_now
+        NotificationsMailer.with(check_notification: check_notification)
+                           .domain_expires_soon
+                           .deliver_now
       end
 
-      def ssl_notify_expires_soon(notification)
-        NotificationsMailer.with(check_notification: check_notification).ssl_expires_soon.deliver_now
+      def ssl_notify_expires_soon(_notification)
+        NotificationsMailer.with(check_notification: check_notification)
+                           .ssl_expires_soon
+                           .deliver_now
       end
     end
   end

app/views/checks/_form.html.erb

@@ -21,16 +21,13 @@
     <%= f.input :active %>
   <% end %>
 
-  <h2 class="mt-5"><%= t(".notifications") %></h2>
-  <p class="alert alert-light"><%= t(".notifications_hint") %></p>
+  <p class="mt-5 bd-callout bd-callout-info"><%= t(".notifications_hint") %></p>
 
-  <%- check.notifications.each_with_index do |notification, index| %>
-    <div data-notification-id="<%= notification.id %>">
-      <%= f.fields_for :notifications, notification do |nf| %>
-        <%= render "notifications/nested_form_headers", f: nf if index.zero? %>
-        <%= render "notifications/nested_form", f: nf, check: check %>
-      <% end %>
-    </div>
+  <%= f.association :notifications, as: :check_boxes, label_method: :label, label_text: false %>
+
+  <%= f.fields_for :notifications, new_notification do |nf| %>
+    <%= render "notifications/nested_form_headers", f: nf %>
+    <%= render "notifications/nested_form", f: nf, notification: new_notification %>
   <% end %>
 
   <%= f.button :submit, class: "btn-primary mt-5" %>

app/views/checks/edit.html.erb

@@ -3,16 +3,16 @@
 <div class="container">
   <div class="row justify-content-center">
     <div class="col-12 col-lg-10">
-      <h1>Edit your check</h1>
+      <h1><%= t(".title") %></h1>
 
-      <%= render "form", check: @check %>
+      <%= render "form", check: @check, new_notification: @new_notification %>
     </div>
   </div>
 
   <div class="row mt-5 justify-content-center">
     <div class="col-12 col-lg-10">
-      <%= button_to("Delete", check_path(@check), class: "btn btn-danger", method: :delete,
-            data: { confirm: "Are you sure ?" }) %>
+      <%= button_to(t("helpers.submit.check.delete"), check_path(@check), class: "btn btn-danger", method: :delete,
+            data: { confirm: t(".destroy_confirmation") }) %>
     </div>
   </div>
 </div>

app/views/checks/new.html.erb

@@ -5,7 +5,7 @@
     <div class="col-12 col-lg-10">
       <h1><%= t(".#{@check.kind}.title") %></h1>
 
-      <%= render "form", check: @check %>
+      <%= render "form", check: @check, new_notification: @new_notification %>
     </div>
   </div>
 </div>

app/views/notifications/_nested_form.html.erb

@@ -12,20 +12,16 @@
       </div>
     <% end %>
 
-    <div class="form-group <%= recipient_col_class %>">
+    <div class="form-group <%= notification_variable_col_class %>">
+      <%= f.input :label, label: false %>
+    </div>
+
+    <div class="form-group <%= notification_variable_col_class %>">
       <%= f.input :recipient, as: :email, label: false %>
     </div>
 
     <div class="form-group col-md-2">
-      <%= f.input :interval, as: :integer, label: false %>
-    </div>
-
-    <div class="form-group col-md-1">
-      <% if f.object.persisted? %>
-        <%= link_to check_notification_path(check, f.object), method: :delete, remote: true, class: "btn btn-danger" do %>
-          <%== Octicons::Octicon.new("x", width: 15, height: 20).to_svg %>
-        <% end %>
-      <% end %>
+      <%= f.input :interval, as: :integer, label: false, required: true %>
     </div>
   </div>
 </fieldset>

app/views/notifications/_nested_form_headers.html.erb

@@ -7,7 +7,11 @@
     </div>
   <% end %>
 
-  <div class="<%= recipient_col_class %>">
+  <div class="<%= notification_variable_col_class %>">
+    <%= f.label :label %>
+  </div>
+
+  <div class="<%= notification_variable_col_class %>">
     <%= f.label :recipient %>
   </div>
 

config/locales/en.yml

@@ -36,12 +36,13 @@ en:
   helpers:
     submit:
       check:
-        create: "Create"
-        update: "Update"
+        create: "Create Check"
+        update: "Update Check"
+        delete: "Destroy Check"
       notification:
-        create: "Create"
-        update: "Update"
-        delete: "Delete"
+        create: "Create Notification"
+        update: "Update Notification"
+        delete: "Destroy Notification"
 
   flashes:
     user_not_authorized: "You are not authorized to access to this resource."
@@ -125,7 +126,9 @@ en:
         title: New domain check
       ssl:
         title: New SSL check
-
+    edit:
+      title: Check edition
+      destroy_confirmation: Are you sure to destroy this check ?
     create:
       saved: "Your check has been saved."
       invalid: "Please check the form."
@@ -144,7 +147,7 @@ en:
         domain: Hostname
       notifications_hint: |
         Receive notifications to warn you when our system detects that the
-        expiration date is coming. The time is set in number of days.
+        expiration date is coming.
 
     table:
       th:

config/locales/fr.yml

@@ -26,11 +26,12 @@ fr:
   helpers:
     submit:
       check:
-        create: "Créer"
-        update: "Valider"
+        create: "Créer la vérification"
+        update: "Modifier la vérification"
+        delete: "Supprimer la vérification"
       notification:
-        create: "Créer"
-        update: "Modifier"
+        create: "Créer la notification"
+        update: "Modifier la notification"
         delete: "Supprimer la notification"
     page_entries_info:
       one_page:
@@ -153,6 +154,9 @@ fr:
         title: Nouvelle vérification d'un nom de domaine
       ssl:
         title: Nouvelle vérification d'un certificat SSL
+    edit:
+      title: Modification de la vérification
+      destroy_confirmation: Confirmez-vous la suppression de cette vérification ?
 
     filters:
       kind_domain: Domaine
@@ -168,7 +172,7 @@ fr:
         domain: Nom d'hôte
       notifications_hint: |
         Recevez des notifications pour vous avertir lorsque notre système détecte
-        que la date d'expiration approche. Le délai est indiqué ennombre de jours.
+        que la date d'expiration approche.
 
     table:
       th:

test/controllers/notifications_controller_test.rb

@@ -1,4 +1,4 @@
-require 'test_helper'
+require "test_helper"
 
 class NotificationsControllerTest < ActionDispatch::IntegrationTest
   # test "the truth" do

test/models/check_notification_test.rb

@@ -21,7 +21,7 @@
 #  fk_rails_...  (notification_id => notifications.id)
 #
 
-require 'test_helper'
+require "test_helper"
 
 class CheckNotificationTest < ActiveSupport::TestCase
   # test "the truth" do

test/services/notifier/channels/base_test.rb

@@ -50,7 +50,8 @@ module Notifier
       end
 
       test "#notify does nothing when channel doesn't support a notification whatever the reason" do
-        check_notification = create(:check_notification, notification: build(:notification, interval: 10_000))
+        check_notification = create(:check_notification,
+                                    notification: build(:notification, interval: 10_000))
 
         @channel.notify(check_notification)
 

test/services/notifier/processor_test.rb

@@ -5,6 +5,7 @@ require "test_helper"
 
 module Notifier
   class ProcessorTest < ActiveSupport::TestCase
+    # rubocop:disable Metrics/LineLength
     test "#process_expires_soon sends an email for checks expiring soon" do
       create_list(:check_notification, 3, notification: email_notification, check: build(:check, :expires_next_week))
       create(:check_notification, notification: email_notification, check: build(:check, :nil_dates))
@@ -30,6 +31,7 @@ module Notifier
         configuration.expect(:consecutive_failures, 4.2)
       end
     end
+    # rubocop:enable Metrics/LineLength
 
     private
 

test/services/notifier/resolver_test.rb

@@ -3,6 +3,7 @@
 
 require "test_helper"
 
+# rubocop:disable Metrics/LineLength
 module Notifier
   class ResolverTest < ActiveSupport::TestCase
     setup do
@@ -116,3 +117,4 @@ module Notifier
     end
   end
 end
+# rubocop:enable Metrics/LineLength

test/system/checks_test.rb

@@ -9,7 +9,7 @@ class ChecksTest < ApplicationSystemTestCase
     login_as(@user)
   end
 
-  test "create a check and a notification without kind" do
+  test "create a check and a new notification without kind" do
     visit new_check_path
 
     choose "domain"
@@ -33,20 +33,34 @@ class ChecksTest < ApplicationSystemTestCase
     fill_and_valid_new_check
   end
 
-  test "remove a notification" do
-    check = create(:check, :with_notifications, domain: "dom-with-notif.net", user: @user)
+  test "dissociate a notification" do
+    check = create(:check, :with_notifications, user: @user)
+    notification = create(:notification, label: "label-notification", user: @user)
+    check.notifications << notification
+
     visit edit_check_path(check)
-    notification = check.notifications.first
 
-    selector = "[data-notification-id=\"#{notification.id}\"]"
+    uncheck notification.label
 
-    assert_difference "Notification.where(check_id: #{check.id}).count", -1 do
-      within selector do
-        find(".btn-danger").click
-      end
+    click_button "Update Check"
 
-      page.has_no_content?(selector)
-    end
+    notification.reload
+    assert_equal 0, notification.checks_count
+    assert_equal 2, check.check_notifications.count
+  end
+
+  test "associate a notification" do
+    check = create(:check, user: @user)
+    notification = create(:notification, label: "label-notification", user: @user)
+    visit edit_check_path(check)
+
+    check notification.label
+    click_button "Update Check"
+
+    notification.reload
+
+    assert_equal 1, notification.checks_count
+    assert_equal 1, check.check_notifications.count
   end
 
   test "update a check" do
@@ -64,29 +78,6 @@ class ChecksTest < ApplicationSystemTestCase
     assert_equal "My comment", check.comment
   end
 
-  test "add a notification" do
-    check = create(:check, :with_notifications, domain: "dom-with-notif.net", user: @user)
-    visit edit_check_path(check)
-
-    recipient = "recipient2@example.org"
-    fill_in("check[notifications_attributes][2][recipient]", with: recipient)
-    fill_in("check[notifications_attributes][2][interval]", with: 55)
-
-    assert_difference "Notification.where(check_id: #{check.id}).count", +1 do
-      click_button "Update Check"
-
-      assert_equal checks_path, page.current_path
-    end
-
-    assert page.has_css?(".alert-success")
-
-    notification = Notification.last
-    assert_equal recipient, notification.recipient
-    assert_equal 55, notification.interval
-    assert notification.email?
-    assert notification.pending?
-  end
-
   test "list my checks" do
     create(:check, :domain, domain: "dom.com", domain_expires_at: Time.new(2018, 7, 5, 12), user: @user) # rubocop:disable Metrics/LineLength
     create(:check, :ssl, domain: "ssldom.com", user: @user)
@@ -266,6 +257,8 @@ class ChecksTest < ApplicationSystemTestCase
     fill_in("check[domain]", with: domain)
 
     recipient = "recipient@example.org"
+    label = "my new notificatiion"
+    fill_in("check[notifications_attributes][0][label]", with: label)
     fill_in("check[notifications_attributes][0][recipient]", with: recipient)
     fill_in("check[notifications_attributes][0][interval]", with: 30)
 
@@ -277,10 +270,14 @@ class ChecksTest < ApplicationSystemTestCase
     assert page.has_content?(domain)
 
     notification = Notification.last
+    assert_equal label, notification.label
     assert_equal recipient, notification.recipient
     assert_equal 30, notification.interval
     assert notification.email?
-    assert notification.pending?
+
+    check_notification = CheckNotification.last
+    assert check_notification.pending?
+    assert_nil check_notification.sent_at
   end
   # rubocop:enable Metrics/AbcSize
   # rubocop:enable Metrics/MethodLength