From 565e06cc19cff88499650186a8e7e8752af8fe69 Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Thu, 30 Aug 2018 11:07:17 +0200 Subject: [PATCH 01/10] [MIG] Adds a CheckNotification model, Notification becomes a template --- app/models/check_notification.rb | 46 +++++++++++++++++++ app/models/notification.rb | 22 +++++---- ...2038_add_consecutive_failures_to_checks.rb | 3 ++ ...180829073920_create_check_notifications.rb | 15 ++++++ ...80829164227_add_fields_to_notifications.rb | 41 +++++++++++++++++ ...remove_obsolete_fields_to_notifications.rb | 9 ++++ db/schema.rb | 22 +++++++-- test/factories/check_notifications.rb | 44 ++++++++++++++++++ test/factories/notifications.rb | 22 +++++---- test/models/check_notification_test.rb | 30 ++++++++++++ test/models/notification_test.rb | 22 +++++---- 11 files changed, 243 insertions(+), 33 deletions(-) create mode 100644 app/models/check_notification.rb create mode 100644 db/migrate/20180829073920_create_check_notifications.rb create mode 100644 db/migrate/20180829164227_add_fields_to_notifications.rb create mode 100644 db/migrate/20180830083927_remove_obsolete_fields_to_notifications.rb create mode 100644 test/factories/check_notifications.rb create mode 100644 test/models/check_notification_test.rb diff --git a/app/models/check_notification.rb b/app/models/check_notification.rb new file mode 100644 index 0000000..789280b --- /dev/null +++ b/app/models/check_notification.rb @@ -0,0 +1,46 @@ +# == Schema Information +# +# Table name: check_notifications +# +# id :bigint(8) not null, primary key +# sent_at :datetime +# status :integer default("pending"), not null +# created_at :datetime not null +# updated_at :datetime not null +# check_id :bigint(8) +# notification_id :bigint(8) +# +# Indexes +# +# index_check_notifications_on_check_id (check_id) +# index_check_notifications_on_notification_id (notification_id) +# +# Foreign Keys +# +# fk_rails_... (check_id => checks.id) +# fk_rails_... (notification_id => notifications.id) +# + +# Copyright (C) 2018 Colin Darie , 2018 Evolix +# License: GNU AGPL-3+ (see full text in LICENSE file) + +class CheckNotification < ApplicationRecord + belongs_to :check + belongs_to :notification, counter_cache: :checks_count + + enum status: [:pending, :ongoing, :succeed, :failed] + + scope :active_check, -> { Check.active } + scope :check_last_run_failed, -> { Check.last_run_failed } + + def pending! + self.sent_at = nil + super + end + alias reset! pending! + + def ongoing! + self.sent_at = Time.now + super + end +end diff --git a/app/models/notification.rb b/app/models/notification.rb index 860f67e..cec38ce 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -1,27 +1,29 @@ # Copyright (C) 2018 Colin Darie , 2018 Jeremy Lecour , 2018 Evolix # License: GNU AGPL-3+ (see full text in LICENSE file) - # == Schema Information # # Table name: notifications # -# id :bigint(8) not null, primary key -# channel :integer default("email"), not null -# interval :integer not null -# recipient :string(255) not null -# sent_at :datetime -# status :integer default("pending"), not null -# created_at :datetime not null -# updated_at :datetime not null -# check_id :bigint(8) +# id :bigint(8) not null, primary key +# channel :integer default("email"), not null +# checks_count :integer default(0), not null +# interval :integer not null +# label :string(255) +# recipient :string(255) not null +# created_at :datetime not null +# updated_at :datetime not null +# check_id :bigint(8) +# user_id :bigint(8) # # Indexes # # index_notifications_on_check_id (check_id) +# index_notifications_on_user_id (user_id) # # Foreign Keys # # fk_rails_... (check_id => checks.id) +# fk_rails_... (user_id => users.id) # class Notification < ApplicationRecord diff --git a/db/migrate/20180801072038_add_consecutive_failures_to_checks.rb b/db/migrate/20180801072038_add_consecutive_failures_to_checks.rb index dbf038b..02c6faa 100644 --- a/db/migrate/20180801072038_add_consecutive_failures_to_checks.rb +++ b/db/migrate/20180801072038_add_consecutive_failures_to_checks.rb @@ -1,3 +1,6 @@ +# Copyright (C) 2018 Colin Darie , 2018 Evolix +# License: GNU AGPL-3+ (see full text in LICENSE file) + class AddConsecutiveFailuresToChecks < ActiveRecord::Migration[5.2] def change add_column :checks, :consecutive_failures, :integer, default: 0, null: false diff --git a/db/migrate/20180829073920_create_check_notifications.rb b/db/migrate/20180829073920_create_check_notifications.rb new file mode 100644 index 0000000..0522c80 --- /dev/null +++ b/db/migrate/20180829073920_create_check_notifications.rb @@ -0,0 +1,15 @@ +# Copyright (C) 2018 Colin Darie , 2018 Evolix +# License: GNU AGPL-3+ (see full text in LICENSE file) + +class CreateCheckNotifications < ActiveRecord::Migration[5.2] + def change + create_table :check_notifications do |t| + t.references :check, foreign_key: true + t.references :notification, foreign_key: true + t.integer :status, null: false, default: 0, limit: 1 + t.datetime :sent_at + + t.timestamps + end + end +end diff --git a/db/migrate/20180829164227_add_fields_to_notifications.rb b/db/migrate/20180829164227_add_fields_to_notifications.rb new file mode 100644 index 0000000..52fdbc3 --- /dev/null +++ b/db/migrate/20180829164227_add_fields_to_notifications.rb @@ -0,0 +1,41 @@ +# Copyright (C) 2018 Colin Darie , 2018 Evolix +# License: GNU AGPL-3+ (see full text in LICENSE file) + +class AddFieldsToNotifications < ActiveRecord::Migration[5.2] + def change + add_reference :notifications, :user, foreign_key: true + add_column :notifications, :label, :string + add_column :notifications, :checks_count, :integer, default: 0, null: false + + reversible do |dir| + dir.up do + # first set user & label for *all* notifications + Notification.find_each do |notification| + check = Check.find(notification.check_id) # check relation does not exist anymore + + notification.user_id = check.user_id + notification.save! + end + + # then build the equivalent check notification + Notification.find_each do |notification| + assoc_notification = Notification.where( + user_id: notification.user_id, + recipient: notification.recipient, + interval: notification.interval, + ).order(checks_count: :desc).limit(1).first + + CheckNotification.create!( + check_id: notification.check_id, + notification: assoc_notification, + status: notification.status, + sent_at: notification.sent_at + ) + end + + # last delete duplicate notification templates not used + Notification.where(checks_count: 0).destroy_all + end + end + end +end diff --git a/db/migrate/20180830083927_remove_obsolete_fields_to_notifications.rb b/db/migrate/20180830083927_remove_obsolete_fields_to_notifications.rb new file mode 100644 index 0000000..f4454b3 --- /dev/null +++ b/db/migrate/20180830083927_remove_obsolete_fields_to_notifications.rb @@ -0,0 +1,9 @@ +# Copyright (C) 2018 Colin Darie , 2018 Evolix +# License: GNU AGPL-3+ (see full text in LICENSE file) + +class RemoveObsoleteFieldsToNotifications < ActiveRecord::Migration[5.2] + def change + remove_column :notifications, :status, :integer, null: false, limit: 1, default: 0 + remove_column :notifications, :sent_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index 6957dbb..7da2aa0 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2018_08_29_134404) do +ActiveRecord::Schema.define(version: 2018_08_30_083927) do create_table "check_logs", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t| t.bigint "check_id" @@ -24,6 +24,17 @@ ActiveRecord::Schema.define(version: 2018_08_29_134404) do t.index ["check_id"], name: "index_check_logs_on_check_id" end + create_table "check_notifications", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t| + t.bigint "check_id" + t.bigint "notification_id" + t.integer "status", limit: 1, default: 0, null: false + t.datetime "sent_at" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["check_id"], name: "index_check_notifications_on_check_id" + t.index ["notification_id"], name: "index_check_notifications_on_notification_id" + end + create_table "checks", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t| t.bigint "user_id" t.integer "kind", null: false @@ -49,11 +60,13 @@ ActiveRecord::Schema.define(version: 2018_08_29_134404) do t.integer "channel", default: 0, null: false t.string "recipient", null: false t.integer "interval", null: false - t.integer "status", default: 0, null: false - t.datetime "sent_at" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.bigint "user_id" + t.string "label" + t.integer "checks_count", default: 0, null: false t.index ["check_id"], name: "index_notifications_on_check_id" + t.index ["user_id"], name: "index_notifications_on_user_id" end create_table "users", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t| @@ -82,6 +95,9 @@ ActiveRecord::Schema.define(version: 2018_08_29_134404) do end add_foreign_key "check_logs", "checks" + add_foreign_key "check_notifications", "checks" + add_foreign_key "check_notifications", "notifications" add_foreign_key "checks", "users" add_foreign_key "notifications", "checks" + add_foreign_key "notifications", "users" end diff --git a/test/factories/check_notifications.rb b/test/factories/check_notifications.rb new file mode 100644 index 0000000..3167933 --- /dev/null +++ b/test/factories/check_notifications.rb @@ -0,0 +1,44 @@ +# == Schema Information +# +# Table name: check_notifications +# +# id :bigint(8) not null, primary key +# sent_at :datetime +# status :integer default("pending"), not null +# created_at :datetime not null +# updated_at :datetime not null +# check_id :bigint(8) +# notification_id :bigint(8) +# +# Indexes +# +# index_check_notifications_on_check_id (check_id) +# index_check_notifications_on_notification_id (notification_id) +# +# Foreign Keys +# +# fk_rails_... (check_id => checks.id) +# fk_rails_... (notification_id => notifications.id) +# + +FactoryBot.define do + factory :check_notification do + check + notification + status :pending + sent_at nil + + trait :ongoing do + status :ongoing + end + + trait :succeed do + status :succeed + sent_at { 1.day.ago } + end + + trait :failed do + status :failed + end + end +end diff --git a/test/factories/notifications.rb b/test/factories/notifications.rb index b851774..de8052c 100644 --- a/test/factories/notifications.rb +++ b/test/factories/notifications.rb @@ -1,27 +1,29 @@ # Copyright (C) 2018 Colin Darie , 2018 Jeremy Lecour , 2018 Evolix # License: GNU AGPL-3+ (see full text in LICENSE file) - # == Schema Information # # Table name: notifications # -# id :bigint(8) not null, primary key -# channel :integer default("email"), not null -# interval :integer not null -# recipient :string(255) not null -# sent_at :datetime -# status :integer default("pending"), not null -# created_at :datetime not null -# updated_at :datetime not null -# check_id :bigint(8) +# id :bigint(8) not null, primary key +# channel :integer default("email"), not null +# checks_count :integer default(0), not null +# interval :integer not null +# label :string(255) +# recipient :string(255) not null +# created_at :datetime not null +# updated_at :datetime not null +# check_id :bigint(8) +# user_id :bigint(8) # # Indexes # # index_notifications_on_check_id (check_id) +# index_notifications_on_user_id (user_id) # # Foreign Keys # # fk_rails_... (check_id => checks.id) +# fk_rails_... (user_id => users.id) # FactoryBot.define do diff --git a/test/models/check_notification_test.rb b/test/models/check_notification_test.rb new file mode 100644 index 0000000..dca5c8f --- /dev/null +++ b/test/models/check_notification_test.rb @@ -0,0 +1,30 @@ +# == Schema Information +# +# Table name: check_notifications +# +# id :bigint(8) not null, primary key +# sent_at :datetime +# status :integer default("pending"), not null +# created_at :datetime not null +# updated_at :datetime not null +# check_id :bigint(8) +# notification_id :bigint(8) +# +# Indexes +# +# index_check_notifications_on_check_id (check_id) +# index_check_notifications_on_notification_id (notification_id) +# +# Foreign Keys +# +# fk_rails_... (check_id => checks.id) +# fk_rails_... (notification_id => notifications.id) +# + +require 'test_helper' + +class CheckNotificationTest < ActiveSupport::TestCase + # test "the truth" do + # assert true + # end +end diff --git a/test/models/notification_test.rb b/test/models/notification_test.rb index 05e9c46..a542799 100644 --- a/test/models/notification_test.rb +++ b/test/models/notification_test.rb @@ -1,27 +1,29 @@ # Copyright (C) 2018 Colin Darie , 2018 Jeremy Lecour , 2018 Evolix # License: GNU AGPL-3+ (see full text in LICENSE file) - # == Schema Information # # Table name: notifications # -# id :bigint(8) not null, primary key -# channel :integer default("email"), not null -# interval :integer not null -# recipient :string(255) not null -# sent_at :datetime -# status :integer default("pending"), not null -# created_at :datetime not null -# updated_at :datetime not null -# check_id :bigint(8) +# id :bigint(8) not null, primary key +# channel :integer default("email"), not null +# checks_count :integer default(0), not null +# interval :integer not null +# label :string(255) +# recipient :string(255) not null +# created_at :datetime not null +# updated_at :datetime not null +# check_id :bigint(8) +# user_id :bigint(8) # # Indexes # # index_notifications_on_check_id (check_id) +# index_notifications_on_user_id (user_id) # # Foreign Keys # # fk_rails_... (check_id => checks.id) +# fk_rails_... (user_id => users.id) # require "test_helper" From 0052b5496773c724f07aebc1147c004a91abb5a5 Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Thu, 30 Aug 2018 11:08:37 +0200 Subject: [PATCH 02/10] Update models & tests with new notification template --- app/mailers/notifications_mailer.rb | 5 +- app/models/check.rb | 10 +-- app/models/notification.rb | 20 ++---- app/models/user.rb | 3 +- app/services/notifier/channels/base.rb | 16 ++--- app/services/notifier/channels/email.rb | 8 +-- app/services/notifier/processor.rb | 4 +- app/services/notifier/resolver.rb | 4 +- test/factories/notifications.rb | 18 +---- test/mailers/notifications_mailer_test.rb | 28 ++++---- test/models/check_test.rb | 14 ++-- test/services/notifier/channels/base_test.rb | 38 +++++----- test/services/notifier/processor_test.rb | 12 ++-- test/services/notifier/resolver_test.rb | 76 ++++++++++---------- 14 files changed, 121 insertions(+), 135 deletions(-) diff --git a/app/mailers/notifications_mailer.rb b/app/mailers/notifications_mailer.rb index b8cdc01..b57de6c 100644 --- a/app/mailers/notifications_mailer.rb +++ b/app/mailers/notifications_mailer.rb @@ -5,8 +5,9 @@ class NotificationsMailer < ApplicationMailer helper :application before_action except: :recurrent_failures do - @notification = params.fetch(:notification) - @check = @notification.check + @check_notification = params.fetch(:check_notification) + @check = @check_notification.check + @notification = @check_notification.notification end def domain_expires_soon diff --git a/app/models/check.rb b/app/models/check.rb index 292283f..6334e17 100644 --- a/app/models/check.rb +++ b/app/models/check.rb @@ -34,7 +34,9 @@ class Check < ApplicationRecord belongs_to :user has_many :logs, class_name: "CheckLog", dependent: :destroy - has_many :notifications, validate: true, dependent: :destroy + has_many :check_notifications, dependent: :destroy + has_many :notifications, through: :check_notifications, validate: true + accepts_nested_attributes_for :notifications, allow_destroy: true, reject_if: lambda { |at| at["recipient"].blank? && at["interval"].blank? } @@ -58,7 +60,7 @@ class Check < ApplicationRecord before_save :reset_consecutive_failures before_save :set_mode - after_update :reset_notifications + after_update :reset_check_notifications after_save :enqueue_sync scope :active, -> { where(active: true) } @@ -119,10 +121,10 @@ class Check < ApplicationRecord ResyncJob.perform_later(id) end - def reset_notifications + def reset_check_notifications return unless (saved_changes.keys & %w[domain domain_expires_at]).present? - notifications.each(&:reset!) + check_notifications.each(&:reset!) end def reset_consecutive_failures diff --git a/app/models/notification.rb b/app/models/notification.rb index cec38ce..2c39b0e 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -27,26 +27,14 @@ # class Notification < ApplicationRecord - belongs_to :check + belongs_to :user + has_many :check_notifications + has_many :checks, through: :notifications enum channel: [:email] - enum status: [:pending, :ongoing, :succeed, :failed] + validates :label, presence: true validates :channel, presence: true validates :interval, numericality: { only_integer: true, greater_than_or_equal_to: 1 } validates :recipient, presence: true - - scope :active_check, -> { Check.active } - scope :check_last_run_failed, -> { Check.last_run_failed } - - def pending! - self.sent_at = nil - super - end - alias reset! pending! - - def ongoing! - self.sent_at = Time.now - super - end end diff --git a/app/models/user.rb b/app/models/user.rb index 8ec2a96..ab649ed 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -39,7 +39,8 @@ class User < ApplicationRecord devise :database_authenticatable, :registerable, :recoverable, :rememberable, :validatable, :confirmable - has_many :checks + has_many :checks, dependent: :destroy + has_many :notifications, dependent: :destroy validates :tos_accepted, acceptance: true validates :locale, inclusion: { in: I18n.available_locales.map(&:to_s) } diff --git a/app/services/notifier/channels/base.rb b/app/services/notifier/channels/base.rb index 264a318..40a5b49 100644 --- a/app/services/notifier/channels/base.rb +++ b/app/services/notifier/channels/base.rb @@ -4,26 +4,26 @@ module Notifier module Channels class Base - def notify(notification) # rubocop:disable Metrics/MethodLength - return unless supports?(notification) + def notify(check_notification) # rubocop:disable Metrics/MethodLength + return unless supports?(check_notification) - notification.ongoing! + check_notification.ongoing! - case notification.check.kind.to_sym + case check_notification.check.kind.to_sym when :domain - domain_notify_expires_soon(notification) + domain_notify_expires_soon(check_notification) when :ssl - ssl_notify_expires_soon(notification) + ssl_notify_expires_soon(check_notification) else fail ArgumentError, - "Invalid notification for check kind `#{notification.check.kind}`." + "Invalid notification for check kind `#{check_notification.check.kind}`." end end private # :nocov: - def supports?(_notification) + def supports?(_check_notification) fail NotImplementedError, "#{self.class.name} channel did not implemented method #{__callee__}" end diff --git a/app/services/notifier/channels/email.rb b/app/services/notifier/channels/email.rb index 743aa5a..e745804 100644 --- a/app/services/notifier/channels/email.rb +++ b/app/services/notifier/channels/email.rb @@ -11,17 +11,17 @@ module Notifier protected - def supports?(_notification) + def supports?(_check_notification) true end # Expiration notifications - def domain_notify_expires_soon(notification) - NotificationsMailer.with(notification: notification).domain_expires_soon.deliver_now + def domain_notify_expires_soon(check_notification) + NotificationsMailer.with(check_notification: check_notification).domain_expires_soon.deliver_now end def ssl_notify_expires_soon(notification) - NotificationsMailer.with(notification: notification).ssl_expires_soon.deliver_now + NotificationsMailer.with(check_notification: check_notification).ssl_expires_soon.deliver_now end end end diff --git a/app/services/notifier/processor.rb b/app/services/notifier/processor.rb index 502c867..de60c28 100644 --- a/app/services/notifier/processor.rb +++ b/app/services/notifier/processor.rb @@ -19,8 +19,8 @@ module Notifier end def process_expires_soon - resolver.notifications_expiring_soon.find_each do |notification| - notifier_channel_for(notification).notify(notification) + resolver.notifications_expiring_soon.find_each do |check_notification| + notifier_channel_for(check_notification.notification).notify(check_notification) sleep configuration.interval end diff --git a/app/services/notifier/resolver.rb b/app/services/notifier/resolver.rb index bc1120c..d99577f 100644 --- a/app/services/notifier/resolver.rb +++ b/app/services/notifier/resolver.rb @@ -21,8 +21,8 @@ module Notifier private def scope - Notification - .includes(:check) + CheckNotification + .includes(:check, :notification) .where(status: [:pending, :failed]) .merge(Check.active) .where.not(checks: { user: ignore_users }) diff --git a/test/factories/notifications.rb b/test/factories/notifications.rb index de8052c..434e386 100644 --- a/test/factories/notifications.rb +++ b/test/factories/notifications.rb @@ -28,28 +28,14 @@ FactoryBot.define do factory :notification do - check + user interval 30 channel :email + label { "#{recipient} (#{interval})" } recipient "recipient@domain.fr" - status :pending - sent_at nil trait :email do channel :email end - - trait :ongoing do - status :ongoing - end - - trait :succeed do - status :succeed - sent_at { 1.day.ago } - end - - trait :failed do - status :failed - end end end diff --git a/test/mailers/notifications_mailer_test.rb b/test/mailers/notifications_mailer_test.rb index dc84db2..29543bf 100644 --- a/test/mailers/notifications_mailer_test.rb +++ b/test/mailers/notifications_mailer_test.rb @@ -6,10 +6,11 @@ require "test_helper" class NotificationsMailerTest < ActionMailer::TestCase # rubocop:disable Metrics/ClassLength test "domain_expires_soon" do check = create(:check, domain_expires_at: Time.new(2018, 6, 10, 12, 0, 5, "+02:00")) - notification = build(:notification, interval: 10, check: check, recipient: "colin@example.org") + notification = build(:notification, interval: 10, recipient: "colin@example.org") + check_notification = build(:check_notification, check: check, notification: notification) Date.stub :today, Date.new(2018, 6, 2) do - mail = NotificationsMailer.with(notification: notification).domain_expires_soon + mail = NotificationsMailer.with(check_notification: check_notification).domain_expires_soon assert_emails 1 do mail.deliver_now @@ -37,10 +38,11 @@ class NotificationsMailerTest < ActionMailer::TestCase # rubocop:disable Metrics check = create(:check, domain_expires_at: Time.new(2018, 6, 10, 12, 0, 5, "+02:00"), user: build(:user, :fr)) - notification = build(:notification, interval: 10, check: check, recipient: "colin@example.org") + notification = build(:notification, interval: 10, recipient: "colin@example.org") + check_notification = build(:check_notification, check: check, notification: notification) Date.stub :today, Date.new(2018, 6, 2) do - mail = NotificationsMailer.with(notification: notification).domain_expires_soon + mail = NotificationsMailer.with(check_notification: check_notification).domain_expires_soon assert_emails 1 do mail.deliver_now @@ -69,9 +71,9 @@ class NotificationsMailerTest < ActionMailer::TestCase # rubocop:disable Metrics domain_expires_at: 1.week.from_now, comment: "My comment", vendor: "The vendor") - notification = build(:notification, check: check) + check_notification = build(:check_notification, check: check) - mail = NotificationsMailer.with(notification: notification).domain_expires_soon + mail = NotificationsMailer.with(check_notification: check_notification).domain_expires_soon parts = [mail.text_part.decode_body, mail.html_part.decode_body] @@ -87,9 +89,9 @@ class NotificationsMailerTest < ActionMailer::TestCase # rubocop:disable Metrics comment: "My comment", vendor: "The vendor", user: build(:user, :fr)) - notification = build(:notification, check: check) + check_notification = build(:check_notification, check: check) - mail = NotificationsMailer.with(notification: notification).domain_expires_soon + mail = NotificationsMailer.with(check_notification: check_notification).domain_expires_soon parts = [mail.text_part.decode_body, mail.html_part.decode_body] @@ -158,10 +160,11 @@ class NotificationsMailerTest < ActionMailer::TestCase # rubocop:disable Metrics test "ssl_expires_soon" do check = create(:check, :ssl, domain_expires_at: Time.new(2018, 6, 10, 12, 0, 5, "+02:00")) - notification = build(:notification, interval: 10, check: check, recipient: "colin@example.org") + notification = build(:notification, interval: 10, recipient: "colin@example.org") + check_notification = build(:check_notification, check: check, notification: notification) Date.stub :today, Date.new(2018, 6, 2) do - mail = NotificationsMailer.with(notification: notification).ssl_expires_soon + mail = NotificationsMailer.with(check_notification: check_notification).ssl_expires_soon assert_emails 1 do mail.deliver_now @@ -190,10 +193,11 @@ class NotificationsMailerTest < ActionMailer::TestCase # rubocop:disable Metrics check = create(:check, :ssl, domain_expires_at: Time.new(2018, 6, 10, 12, 0, 5, "+02:00"), user: build(:user, :fr)) - notification = build(:notification, interval: 10, check: check, recipient: "colin@example.org") + notification = build(:notification, interval: 10, recipient: "colin@example.org") + check_notification = build(:check_notification, check: check, notification: notification) Date.stub :today, Date.new(2018, 6, 2) do - mail = NotificationsMailer.with(notification: notification).ssl_expires_soon + mail = NotificationsMailer.with(check_notification: check_notification).ssl_expires_soon assert_emails 1 do mail.deliver_now diff --git a/test/models/check_test.rb b/test/models/check_test.rb index 8064463..96b6946 100644 --- a/test/models/check_test.rb +++ b/test/models/check_test.rb @@ -35,23 +35,23 @@ require "test_helper" class CheckTest < ActiveSupport::TestCase test "notifications are resetted when domain expiration date has changed" do check = create(:check) - notification = create(:notification, :succeed, check: check) + check_notification = create(:check_notification, :succeed, check: check) check.comment = "Will not reset because of this attribute" check.save! - notification.reload + check_notification.reload - assert notification.succeed? - assert_not_nil notification.sent_at + assert check_notification.succeed? + assert_not_nil check_notification.sent_at check.domain_expires_at = 1.year.from_now check.save! - notification.reload + check_notification.reload - assert notification.pending? - assert_nil notification.sent_at + assert check_notification.pending? + assert_nil check_notification.sent_at end test "days_from_last_success without any success" do diff --git a/test/services/notifier/channels/base_test.rb b/test/services/notifier/channels/base_test.rb index d696745..f20779f 100644 --- a/test/services/notifier/channels/base_test.rb +++ b/test/services/notifier/channels/base_test.rb @@ -8,8 +8,8 @@ module Notifier class BaseTest < ActiveSupport::TestCase setup do class FakeChannel < Base - def supports?(notification) - notification.interval < 1_000 + def supports?(check_notification) + check_notification.notification.interval < 1_000 end def domain_notify_expires_soon(*); end @@ -19,45 +19,45 @@ module Notifier end test "#notify change the status of the notification" do - notification = create(:notification) + check_notification = create(:check_notification) - @channel.notify(notification) + @channel.notify(check_notification) - notification.reload + check_notification.reload - assert notification.ongoing? - assert_just_now notification.sent_at + assert check_notification.ongoing? + assert_just_now check_notification.sent_at end test "#notify raises an exception for a non supported check kind" do - notification = Minitest::Mock.new - notification.expect :ongoing!, true - notification.expect :interval, 10 + check_notification = Minitest::Mock.new + check_notification.expect :ongoing!, true + check_notification.expect :notification, OpenStruct.new(interval: 10) check = Minitest::Mock.new check.expect(:kind, :invalid_kind) check.expect(:kind, :invalid_kind) # twice (second call for exception message) - notification.expect :check, check - notification.expect :check, check + check_notification.expect :check, check + check_notification.expect :check, check assert_raises ArgumentError do - @channel.notify(notification) + @channel.notify(check_notification) end check.verify - notification.verify + check_notification.verify end test "#notify does nothing when channel doesn't support a notification whatever the reason" do - notification = create(:notification, interval: 10_000) + check_notification = create(:check_notification, notification: build(:notification, interval: 10_000)) - @channel.notify(notification) + @channel.notify(check_notification) - notification.reload + check_notification.reload - assert notification.pending? - assert_nil notification.sent_at + assert check_notification.pending? + assert_nil check_notification.sent_at end end end diff --git a/test/services/notifier/processor_test.rb b/test/services/notifier/processor_test.rb index 2b2693f..14121ee 100644 --- a/test/services/notifier/processor_test.rb +++ b/test/services/notifier/processor_test.rb @@ -6,9 +6,9 @@ require "test_helper" module Notifier class ProcessorTest < ActiveSupport::TestCase test "#process_expires_soon sends an email for checks expiring soon" do - create_list(:notification, 3, :email, check: build(:check, :expires_next_week)) - create(:notification, :email, check: build(:check, :nil_dates)) - create(:notification, :email, check: build(:check, :inactive)) + 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)) + create(:check_notification, notification: email_notification, check: build(:check, :inactive)) processor = Processor.new assert_difference "ActionMailer::Base.deliveries.size", +3 do @@ -20,7 +20,7 @@ module Notifier end test "#process_expires_soon respects the interval configuration between sends" do - create_list(:notification, 3, :email, check: build(:check, :expires_next_week)) + create_list(:check_notification, 3, notification: email_notification, check: build(:check, :expires_next_week)) test_interval_respected(:process_expires_soon, 3) end @@ -33,6 +33,10 @@ module Notifier private + def email_notification + build(:notification, :email) + end + # rubocop:disable Metrics/MethodLength def test_interval_respected(process_method, count_expected) configuration = Minitest::Mock.new diff --git a/test/services/notifier/resolver_test.rb b/test/services/notifier/resolver_test.rb index 2ed9c84..419adfe 100644 --- a/test/services/notifier/resolver_test.rb +++ b/test/services/notifier/resolver_test.rb @@ -10,74 +10,74 @@ module Notifier end test "#notifications_expiring_soon ignores user having notification disabled" do - n1 = create(:notification, check: build(:check, :expires_next_week)) + n1 = create(:check_notification, check: build(:check, :expires_next_week)) n1.check.user.update_attribute(:notifications_enabled, false) - n2 = create(:notification, check: build(:check, :expires_next_week)) + n2 = create(:check_notification, check: build(:check, :expires_next_week)) - notifications = @resolver.notifications_expiring_soon + check_notifications = @resolver.notifications_expiring_soon - assert_not_includes notifications, n1 - assert_includes notifications, n2 + assert_not_includes check_notifications, n1 + assert_includes check_notifications, n2 end test "#notifications_expiring_soon ignores inactive checks" do - n1 = create(:notification, check: build(:check, :expires_next_week, :inactive)) - n2 = create(:notification, check: build(:check, :expires_next_week)) + n1 = create(:check_notification, check: build(:check, :expires_next_week, :inactive)) + n2 = create(:check_notification, check: build(:check, :expires_next_week)) - notifications = @resolver.notifications_expiring_soon + check_notifications = @resolver.notifications_expiring_soon - assert_not_includes notifications, n1 - assert_includes notifications, n2 + assert_not_includes check_notifications, n1 + assert_includes check_notifications, n2 end test "#notifications_expiring_soon gets only checks inside interval" do - n1 = create(:notification, check: build(:check, :expires_next_week), interval: 6) - n2 = create(:notification, check: build(:check, :expires_next_week), interval: 7) + n1 = create(:check_notification, check: build(:check, :expires_next_week), notification: build(:notification, interval: 6)) + n2 = create(:check_notification, check: build(:check, :expires_next_week), notification: build(:notification, interval: 7)) - notifications = @resolver.notifications_expiring_soon + check_notifications = @resolver.notifications_expiring_soon - assert_not_includes notifications, n1 - assert_includes notifications, n2 + assert_not_includes check_notifications, n1 + assert_includes check_notifications, n2 end test "#notifications_expiring_soon can gets several notifications for a same check" do check = create(:check, :expires_next_week) - n1 = create(:notification, check: check, interval: 3) - n2 = create(:notification, check: check, interval: 10) - n3 = create(:notification, check: check, interval: 30) + n1 = create(:check_notification, check: check, notification: build(:notification, interval: 3)) + n2 = create(:check_notification, check: check, notification: build(:notification, interval: 10)) + n3 = create(:check_notification, check: check, notification: build(:notification, interval: 30)) - notifications = @resolver.notifications_expiring_soon + check_notifications = @resolver.notifications_expiring_soon - assert_not_includes notifications, n1 - assert_includes notifications, n2 - assert_includes notifications, n3 + assert_not_includes check_notifications, n1 + assert_includes check_notifications, n2 + assert_includes check_notifications, n3 end test "#notifications_expiring_soon takes care of the status" do check = create(:check, :expires_next_week) - n1 = create(:notification, check: check) - n2 = create(:notification, :failed, check: check) - n3 = create(:notification, :ongoing, check: check) - n4 = create(:notification, :succeed, check: check) + n1 = create(:check_notification, check: check) + n2 = create(:check_notification, :failed, check: check) + n3 = create(:check_notification, :ongoing, check: check) + n4 = create(:check_notification, :succeed, check: check) - notifications = @resolver.notifications_expiring_soon + check_notifications = @resolver.notifications_expiring_soon - assert_includes notifications, n1 - assert_includes notifications, n2 - assert_not_includes notifications, n3 - assert_not_includes notifications, n4 + assert_includes check_notifications, n1 + assert_includes check_notifications, n2 + assert_not_includes check_notifications, n3 + assert_not_includes check_notifications, n4 end test "#notifications_expiring_soon ignores checks expired and without date" do - n1 = create(:notification, check: build(:check, :expires_next_week)) - n2 = create(:notification, check: build(:check, domain_expires_at: 1.week.ago)) - n3 = create(:notification, check: build(:check, :nil_dates)) + n1 = create(:check_notification, check: build(:check, :expires_next_week)) + n2 = create(:check_notification, check: build(:check, domain_expires_at: 1.week.ago)) + n3 = create(:check_notification, check: build(:check, :nil_dates)) - notifications = @resolver.notifications_expiring_soon + check_notifications = @resolver.notifications_expiring_soon - assert_includes notifications, n1 - assert_not_includes notifications, n2 - assert_not_includes notifications, n3 + assert_includes check_notifications, n1 + assert_not_includes check_notifications, n2 + assert_not_includes check_notifications, n3 end test "#checks_recurrent_failures ignores inactive checks" do From 9c35dbc7a6e88e75941f086545b3c6308d11abcd Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Fri, 31 Aug 2018 10:06:16 +0200 Subject: [PATCH 03/10] Notifications template CRUD --- .../check_notifications_controller.rb | 47 ++++++++++++++++ app/controllers/checks_controller.rb | 10 ++-- app/controllers/notifications_controller.rb | 55 +++++++++++------- app/frontend/scss/components/callouts.scss | 51 +++++++++++++++++ .../scss/components/notifications.scss | 8 +++ app/frontend/scss/index.scss | 2 + app/helpers/check_notifications_helper.rb | 8 +++ app/helpers/notifications_helper.rb | 7 --- app/models/notification.rb | 4 +- app/policies/check_notification_policy.rb | 24 ++++++++ app/policies/notification_policy.rb | 18 +++--- .../_nested_form.html.erb | 0 .../_nested_form_headers.html.erb | 0 .../destroy.js.erb | 0 app/views/checks/_form.html.erb | 1 - app/views/checks/_table.html.erb | 18 +++--- app/views/notifications/_form.html.erb | 13 +++++ app/views/notifications/_table.html.erb | 46 +++++++++++++++ app/views/notifications/edit.html.erb | 27 +++++++++ app/views/notifications/index.html.erb | 22 ++++++++ app/views/notifications/new.html.erb | 11 ++++ app/views/shared/_navbar.html.erb | 3 + config/locales/en.yml | 54 ++++++++++++++++++ config/locales/fr.yml | 56 +++++++++++++++++-- config/routes.rb | 18 ++++-- .../check_notifications_controller_test.rb | 10 ++++ .../notifications_controller_test.rb | 5 +- 27 files changed, 456 insertions(+), 62 deletions(-) create mode 100644 app/controllers/check_notifications_controller.rb create mode 100644 app/frontend/scss/components/callouts.scss create mode 100644 app/frontend/scss/components/notifications.scss create mode 100644 app/helpers/check_notifications_helper.rb create mode 100644 app/policies/check_notification_policy.rb rename app/views/{notifications => check_notifications}/_nested_form.html.erb (100%) rename app/views/{notifications => check_notifications}/_nested_form_headers.html.erb (100%) rename app/views/{notifications => check_notifications}/destroy.js.erb (100%) create mode 100644 app/views/notifications/_form.html.erb create mode 100644 app/views/notifications/_table.html.erb create mode 100644 app/views/notifications/edit.html.erb create mode 100644 app/views/notifications/index.html.erb create mode 100644 app/views/notifications/new.html.erb create mode 100644 test/controllers/check_notifications_controller_test.rb diff --git a/app/controllers/check_notifications_controller.rb b/app/controllers/check_notifications_controller.rb new file mode 100644 index 0000000..b1e47b1 --- /dev/null +++ b/app/controllers/check_notifications_controller.rb @@ -0,0 +1,47 @@ +# Copyright (C) 2018 Colin Darie , 2018 Evolix +# License: GNU AGPL-3+ (see full text in LICENSE file) + +class CheckNotificationsController < ApplicationController + before_action :authenticate_user! + before_action :set_notification, except: [:create] + + def create + check = Check.find(params[:check_id]) + @notification = check.notifications.build(notification_params) + authorize @notification + + if @notification.save + flash[:notice] = "Your notification has been saved." + redirect_to check_path + else + flash.now[:alert] = "An error occured." + render "checks/edit" + end + end + + def destroy + @notification.destroy! + + respond_to do |format| + format.js + end + end + + private + + def set_notification + # joins the check because policy use the check relation + @notification = Notification + .joins(:check) + .find_by!(id: params[:id], check_id: params[:check_id]) + authorize @notification + end + + def notification_params + params.require(:notification).permit(:channel, :recipient, :interval) + end + + def check_path + edit_check_path(check_id: params[:check_id]) + end +end diff --git a/app/controllers/checks_controller.rb b/app/controllers/checks_controller.rb index 3913cd7..143bf37 100644 --- a/app/controllers/checks_controller.rb +++ b/app/controllers/checks_controller.rb @@ -35,10 +35,10 @@ class ChecksController < ApplicationController authorize @check if @check.save - flash[:notice] = t(".saved") + flash[:notice] = t("checks.created", scope: :flashes) redirect_to checks_path else - flash.now[:alert] = t(".invalid") + flash.now[:alert] = t("checks.invalid", scope: :flashes) render :new end end @@ -49,10 +49,10 @@ class ChecksController < ApplicationController def update if @check.update(update_check_params) - flash[:notice] = "Your check has been updated." + flash[:notice] = t("checks.updated", scope: :flashes) redirect_to checks_path else - flash.now[:alert] = "An error occured." + flash.now[:alert] = t("checks.invalid", scope: :flashes) build_empty_notification render :edit end @@ -61,7 +61,7 @@ class ChecksController < ApplicationController def destroy @check.destroy! - flash[:notice] = "Your check has been destroyed." + flash[:notice] = t("checks.destroyed", scope: :flashes) redirect_to checks_path end diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 8fc3103..e159174 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -3,45 +3,62 @@ class NotificationsController < ApplicationController before_action :authenticate_user! - before_action :set_notification, except: [:create] + before_action :set_notification, except: [:index, :new, :create] + after_action :verify_authorized, except: :index + after_action :verify_policy_scoped, only: :index + + def index + @notifications = policy_scope(Notification).order(checks_count: :desc) + end + + def new; + @notification = Notification.new + authorize @notification + @notification.recipient = current_user.email + end def create - check = Check.find(params[:check_id]) - @notification = check.notifications.build(notification_params) + @notification = Notification.new(notification_params) + @notification.user = current_user authorize @notification if @notification.save - flash[:notice] = "Your notification has been saved." - redirect_to check_path + flash[:notice] = t("notifications.created", scope: :flashes) + redirect_to notifications_path else - flash.now[:alert] = "An error occured." - render "checks/edit" + flash.now[:alert] = t("notifications.invalid", scope: :flashes) + render :new + end + end + + + def edit; end + + def update + if @notification.update(notification_params) + flash[:notice] = t("notifications.updated", scope: :flashes) + redirect_to notifications_path + else + flash.now[:alert] = t("notifications.error", scope: :flashes) + render :edit end end def destroy @notification.destroy! - respond_to do |format| - format.js - end + flash[:notice] = t("notifications.destroyed", scope: :flashes) + redirect_to notifications_path end private def set_notification - # joins the check because policy use the check relation - @notification = Notification - .joins(:check) - .find_by!(id: params[:id], check_id: params[:check_id]) + @notification = Notification.find(params[:id]) authorize @notification end def notification_params - params.require(:notification).permit(:channel, :recipient, :interval) - end - - def check_path - edit_check_path(check_id: params[:check_id]) + params.require(:notification).permit(:label, :recipient, :interval) end end diff --git a/app/frontend/scss/components/callouts.scss b/app/frontend/scss/components/callouts.scss new file mode 100644 index 0000000..75f4bb4 --- /dev/null +++ b/app/frontend/scss/components/callouts.scss @@ -0,0 +1,51 @@ +// Taken from Bootstrap 4 documentation + +.bd-callout { + padding: 1.25rem; + margin-top: 1.25rem; + margin-bottom: 1.25rem; + border: 1px solid #eee; + border-left-width: .25rem; + border-radius: .25rem +} + +.bd-callout h4 { + margin-top: 0; + margin-bottom: .25rem +} + +.bd-callout p:last-child { + margin-bottom: 0 +} + +.bd-callout code { + border-radius: .25rem +} + +.bd-callout+.bd-callout { + margin-top: -.25rem +} + +.bd-callout-info { + border-left-color: #5bc0de +} + +.bd-callout-info h4 { + color: #5bc0de +} + +.bd-callout-warning { + border-left-color: #f0ad4e +} + +.bd-callout-warning h4 { + color: #f0ad4e +} + +.bd-callout-danger { + border-left-color: #d9534f +} + +.bd-callout-danger h4 { + color: #d9534f +} diff --git a/app/frontend/scss/components/notifications.scss b/app/frontend/scss/components/notifications.scss new file mode 100644 index 0000000..aef4306 --- /dev/null +++ b/app/frontend/scss/components/notifications.scss @@ -0,0 +1,8 @@ +// Copyright (C) 2018 Colin Darie , 2018 Evolix +// License: GNU AGPL-3+ (see full text in LICENSE file) + +.notifications-table { + .action a { + color: black; + } +} diff --git a/app/frontend/scss/index.scss b/app/frontend/scss/index.scss index 2b38b06..20ea0ae 100644 --- a/app/frontend/scss/index.scss +++ b/app/frontend/scss/index.scss @@ -5,5 +5,7 @@ @import '~bootstrap/scss/bootstrap'; @import 'layout'; @import 'icons'; +@import 'components/callouts'; @import 'components/users'; @import 'components/checks'; +@import 'components/notifications'; diff --git a/app/helpers/check_notifications_helper.rb b/app/helpers/check_notifications_helper.rb new file mode 100644 index 0000000..a6dbd59 --- /dev/null +++ b/app/helpers/check_notifications_helper.rb @@ -0,0 +1,8 @@ +# Copyright (C) 2018 Colin Darie , 2018 Evolix +# 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 diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 0e9b90d..706cbe4 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -1,12 +1,5 @@ -# Copyright (C) 2018 Colin Darie , 2018 Evolix -# License: GNU AGPL-3+ (see full text in LICENSE file) - module NotificationsHelper def many_channels_available? Notification.channels.many? end - - def recipient_col_class - many_channels_available? ? "col-md-7" : "col-md-9" - end end diff --git a/app/models/notification.rb b/app/models/notification.rb index 2c39b0e..b49f6d6 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -28,8 +28,8 @@ class Notification < ApplicationRecord belongs_to :user - has_many :check_notifications - has_many :checks, through: :notifications + has_many :check_notifications, dependent: :destroy + has_many :checks, -> { order(domain_expires_at: :asc) }, through: :check_notifications enum channel: [:email] diff --git a/app/policies/check_notification_policy.rb b/app/policies/check_notification_policy.rb new file mode 100644 index 0000000..ff9837a --- /dev/null +++ b/app/policies/check_notification_policy.rb @@ -0,0 +1,24 @@ +# Copyright (C) 2018 Colin Darie , 2018 Evolix +# License: GNU AGPL-3+ (see full text in LICENSE file) + +class CheckNotificationPolicy < ApplicationPolicy + class Scope < Scope + def resolve + scope.joins(:check).where(checks: { user: user }) + end + end + + def destroy? + check_owner? + end + + def show? + false + end + + private + + def check_owner? + record.check.user == user + end +end diff --git a/app/policies/notification_policy.rb b/app/policies/notification_policy.rb index ba2a514..c541694 100644 --- a/app/policies/notification_policy.rb +++ b/app/policies/notification_policy.rb @@ -4,21 +4,25 @@ class NotificationPolicy < ApplicationPolicy class Scope < Scope def resolve - scope.joins(:check).where(checks: { user: user }) + scope.where(user: user) end end - def destroy? - check_owner? + def create? + true end - def show? - false + def update? + owner? + end + + def destroy? + owner? end private - def check_owner? - record.check.user == user + def owner? + record.user == user end end diff --git a/app/views/notifications/_nested_form.html.erb b/app/views/check_notifications/_nested_form.html.erb similarity index 100% rename from app/views/notifications/_nested_form.html.erb rename to app/views/check_notifications/_nested_form.html.erb diff --git a/app/views/notifications/_nested_form_headers.html.erb b/app/views/check_notifications/_nested_form_headers.html.erb similarity index 100% rename from app/views/notifications/_nested_form_headers.html.erb rename to app/views/check_notifications/_nested_form_headers.html.erb diff --git a/app/views/notifications/destroy.js.erb b/app/views/check_notifications/destroy.js.erb similarity index 100% rename from app/views/notifications/destroy.js.erb rename to app/views/check_notifications/destroy.js.erb diff --git a/app/views/checks/_form.html.erb b/app/views/checks/_form.html.erb index 991494f..c01f873 100644 --- a/app/views/checks/_form.html.erb +++ b/app/views/checks/_form.html.erb @@ -53,6 +53,5 @@ <% end %> - <%= f.button :submit, class: "btn-primary mt-5" %> <% end %> diff --git a/app/views/checks/_table.html.erb b/app/views/checks/_table.html.erb index cba39e9..8cb7244 100644 --- a/app/views/checks/_table.html.erb +++ b/app/views/checks/_table.html.erb @@ -7,16 +7,20 @@ <%= t(".th.domain") %> - - <%== checks_sort_links(:domain) %> - + <% unless defined?(skip_sort) %> + + <%== checks_sort_links(:domain) %> + + <% end %> <%= t(".th.expiry_date") %> <%= t(".th.expiry_date_short") %> - - <%== checks_sort_links(:domain_expires_at) %> - + <% unless defined?(skip_sort) %> + + <%== checks_sort_links(:domain_expires_at) %> + + <% end %> <%= t(".th.edit") %> @@ -48,4 +52,4 @@ -<%= paginate @checks %> +<%= paginate checks unless defined?(skip_pagination)%> diff --git a/app/views/notifications/_form.html.erb b/app/views/notifications/_form.html.erb new file mode 100644 index 0000000..54ec17a --- /dev/null +++ b/app/views/notifications/_form.html.erb @@ -0,0 +1,13 @@ +<% # Copyright (C) 2018 Colin Darie , 2018 Evolix %> +<% # License: GNU AGPL-3+ (see full text in LICENSE file) %> +<%= simple_form_for(notification) do |f| %> + <%= f.input :label, hint: t(".label_hint")%> + + <%= f.input :recipient, as: :email, + input_html: { autocapitalize: :none, autocorrect: :off } + %> + + <%= f.input :interval, as: :integer, required: true %> + + <%= f.button :submit, class: "btn-primary mt-3" %> +<% end %> diff --git a/app/views/notifications/_table.html.erb b/app/views/notifications/_table.html.erb new file mode 100644 index 0000000..f54721d --- /dev/null +++ b/app/views/notifications/_table.html.erb @@ -0,0 +1,46 @@ +<% # Copyright (C) 2018 Colin Darie , 2018 Evolix %> +<% # License: GNU AGPL-3+ (see full text in LICENSE file) %> +
+ + + + + + + + + + + + <% notifications.each do |notification| %> + + + + + + + + <% end %> + +
+ <%= Notification.human_attribute_name("label") %> + + <%= Notification.human_attribute_name("recipient") %> + + <%= Notification.human_attribute_name("interval") %> + + <%= Notification.human_attribute_name("checks_count") %> + <%= t(".th.edit") %>
+ <%= notification.label %> + + <%= notification.recipient %> + + <%= t(".interval_in_days", count: notification.interval) %> + + <%= notification.checks_count %> + + <%= link_to edit_notification_path(notification), "data-turbolinks" => false do %> + <%== Octicons::Octicon.new("pencil").to_svg %> + <% end %> +
+
diff --git a/app/views/notifications/edit.html.erb b/app/views/notifications/edit.html.erb new file mode 100644 index 0000000..9c996c5 --- /dev/null +++ b/app/views/notifications/edit.html.erb @@ -0,0 +1,27 @@ +<% # Copyright (C) 2018 Colin Darie , 2018 Evolix %> +<% # License: GNU AGPL-3+ (see full text in LICENSE file) %> +
+
+
+

<%= t(".title") %>

+ + <%= render "form", notification: @notification %> +
+
+ +
+
+

<%= t(".checks", count: @notification.checks_count) %>

+ <% if @notification.checks_count.positive? %> + <%= render "checks/table", checks: @notification.checks, skip_sort: true, skip_pagination: true %> + <% end %> +
+
+ +
+
+ <%= button_to(t("helpers.submit.notification.delete"), notification_path(@notification), class: "btn btn-danger", method: :delete, + data: { confirm: t(".destroy_confirmation", count: @notification.checks_count) }) %> +
+
+
diff --git a/app/views/notifications/index.html.erb b/app/views/notifications/index.html.erb new file mode 100644 index 0000000..af5f65b --- /dev/null +++ b/app/views/notifications/index.html.erb @@ -0,0 +1,22 @@ +<% # Copyright (C) 2018 Colin Darie , 2018 Evolix %> +<% # License: GNU AGPL-3+ (see full text in LICENSE file) %> +
+
+
+ <% if @notifications.empty? %> +
+ <%= t(".no_notification_yet_html", new_path: new_notification_path) %> +
+ <% else %> +
+ <%= link_to("Ajouter une notification", new_notification_path, class: "btn btn-primary") %> +
+

<%= t(".title") %>

+ <%= render "table", notifications: @notifications %> + <% end %> +
+

<%= t(".explanation") %>

+
+
+
+
diff --git a/app/views/notifications/new.html.erb b/app/views/notifications/new.html.erb new file mode 100644 index 0000000..6c777a9 --- /dev/null +++ b/app/views/notifications/new.html.erb @@ -0,0 +1,11 @@ +<% # Copyright (C) 2018 Colin Darie , 2018 Evolix %> +<% # License: GNU AGPL-3+ (see full text in LICENSE file) %> +
+
+
+

<%= t(".title") %>

+ + <%= render "form", notification: @notification %> +
+
+
diff --git a/app/views/shared/_navbar.html.erb b/app/views/shared/_navbar.html.erb index 00b87e5..4da000b 100644 --- a/app/views/shared/_navbar.html.erb +++ b/app/views/shared/_navbar.html.erb @@ -24,6 +24,9 @@
  • <%= link_to(t(".GitHub"), "https://github.com/Evolix/chexpire", class: "nav-link") %>
    • +
  • + <%= link_to(t(".my_notifications"), notifications_path, class: "nav-link") %> +
    • <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 58c4495..5eb7edf 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -33,8 +33,28 @@ en: notifications: recipient: john@example.com + helpers: + submit: + check: + create: "Create" + update: "Update" + notification: + create: "Create" + update: "Update" + delete: "Delete" + flashes: user_not_authorized: "You are not authorized to access to this resource." + checks: + create: The check has been saved. + updated: The check has been updated. + invalid: Please check the form. + destroyed: The check has been destroyed. + notifications: + created: The notification has been created. + updated: The notification has been updated." + destroyed: The notification has been destroyed. + invalid: Please check the form. notifications_mailer: domain_expires_soon: @@ -68,6 +88,7 @@ en: my_checks: "My checks" new_domain_check: "New domain check" new_ssl_check: "New SSL check" + my_notifications: "My notifications" sign_up: "Sign up" sign_in: "Log in" sign_out: "Log out" @@ -143,3 +164,36 @@ en: zero: "Last check successful: today" one: "Last check successful: yesterday" other: "Last check successful %{count} days ago" + + notifications: + index: + title: "List of your notifications" + no_notification_yet_html: | + You have not set up a notification yet. + Create your first notification. + explanation: | + For each of your checks, + you can associate one or more notifications that you will receive + by email at the interval of your choice before the expiration date. + table: + th: + edit: Edit + interval_in_days: + one: 1 day + other: "%{count} days" + edit: + title: "Edit the notification" + checks: + zero: No check associated check. + one: Check associated + other: Checks associated + destroy_confirmation: + zero: "Are you sure ?" + one: | + Are you sure ? + You won't receive this notification for the associated check. + other: | + Are you sure ? + You won't receive this notification for the associated checks. + form: + label_hint: This label allows you to identify this notification and associate it with a check. diff --git a/config/locales/fr.yml b/config/locales/fr.yml index e48ca32..499ac99 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -13,8 +13,10 @@ fr: domain_updated_at: "Date de modification" domain_expires_at: "Date d'expiration" notification: + label: Étiquette interval: Délai recipient: Destinataire + checks_count: Vérifications user: tos_accepted: "Conditions d'utilisation" notifications_enabled: "Notifications activées" @@ -27,6 +29,10 @@ fr: check: create: "Créer" update: "Valider" + notification: + create: "Créer" + update: "Modifier" + delete: "Supprimer la notification" page_entries_info: one_page: display_entries: @@ -71,6 +77,16 @@ fr: flashes: user_not_authorized: "Vous n'êtes pas autorisé•e à accéder à cette ressouce." + checks: + create: La vérification est enregistrée. + updated: La vérification est mise à jour. + invalid: Veuillez vérifier le formulaire. + destroyed: La vérification est supprimée. + notifications: + created: La notification est créée. + updated: "La notification est mise à jour." + destroyed: La notification est supprimée. + invalid: Veuillez vérifier le formulaire. notifications_mailer: domain_expires_soon: @@ -101,6 +117,7 @@ fr: my_checks: "Mes vérifications" new_domain_check: "Nouveau nom de domaine" new_ssl_check: "Nouveau certificat SSL" + my_notifications: "Mes notifications" sign_up: "Enregistrement" sign_in: "Connexion" sign_out: "Déconnexion" @@ -138,10 +155,6 @@ fr: ssl: title: Nouvelle vérification d'un certificat SSL - create: - saved: La vérification est enregistrée. - invalid: Veuillez vérifier le formulaire. - filters: kind_domain: Domaine kind_ssl: SSL @@ -176,3 +189,38 @@ fr: zero: "Dernière vérification réussie : aujourd'hui" one: "Dernière vérification réussie : hier" other: "Dernière vérification réussie il y a %{count} jours" + + notifications: + index: + title: "Liste de vos notifications" + no_notification_yet_html: | + Vous n'avez pas encore créé de notification. + Créez votre première notification. + explanation: | + Pour chacune de vos vérifications, + vous pouvez associer une ou plusieurs notifications que vous recevrez + par email un certain nombre de jours avant la date d'expiration. + table: + th: + edit: Modifier + interval_in_days: + one: 1 jour + other: "%{count} jours" + new: + title: Nouvelle notification + edit: + title: "Modification d'une notification" + checks: + zero: Aucune vérification associée. + one: Vérification associée + other: Vérifications associées + destroy_confirmation: + zero: "Confirmez-vous la suppression de cette notification ?" + one: | + Confirmez-vous la suppression ? + Vous ne recevrez plus la notification associée à cette vérification. + other: | + Confirmez-vous la suppression ? + Vous ne recevrez plus les notifications associées à ces vérifications. + form: + label_hint: Ce nom vous permet d'identifier cette notification pour l'associer à une vérification. diff --git a/config/routes.rb b/config/routes.rb index 6dc56fd..ee3120f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -7,12 +7,11 @@ Rails.application.routes.draw do # For details on the DSL available within this file, see http://guides.rubyonrails.org/routing.html resources :checks, except: [:show] do - resources :notifications, only: [:destroy] - collection do - post :supports, format: :json - end + resources :check_notifications, only: [:destroy] end + resources :notifications, except: [:show] + devise_for :users root to: "pages#home" @@ -22,7 +21,7 @@ end # == Route Map # # Prefix Verb URI Pattern Controller#Action -# check_notification DELETE /checks/:check_id/notifications/:id(.:format) notifications#destroy +# check_check_notification DELETE /checks/:check_id/check_notifications/:id(.:format) check_notifications#destroy # supports_checks POST /checks/supports(.:format) checks#supports # checks GET /checks(.:format) checks#index # POST /checks(.:format) checks#create @@ -31,6 +30,13 @@ end # check PATCH /checks/:id(.:format) checks#update # PUT /checks/:id(.:format) checks#update # DELETE /checks/:id(.:format) checks#destroy +# notifications GET /notifications(.:format) notifications#index +# POST /notifications(.:format) notifications#create +# new_notification GET /notifications/new(.:format) notifications#new +# edit_notification GET /notifications/:id/edit(.:format) notifications#edit +# notification PATCH /notifications/:id(.:format) notifications#update +# PUT /notifications/:id(.:format) notifications#update +# DELETE /notifications/:id(.:format) notifications#destroy # new_user_session GET /users/sign_in(.:format) devise/sessions#new # user_session POST /users/sign_in(.:format) devise/sessions#create # destroy_user_session DELETE /users/sign_out(.:format) devise/sessions#destroy @@ -56,7 +62,7 @@ end # rails_disk_service GET /rails/active_storage/disk/:encoded_key/*filename(.:format) active_storage/disk#show # update_rails_disk_service PUT /rails/active_storage/disk/:encoded_token(.:format) active_storage/disk#update # rails_direct_uploads POST /rails/active_storage/direct_uploads(.:format) active_storage/direct_uploads#create -# +# # Routes for LetterOpenerWeb::Engine: # clear_letters DELETE /clear(.:format) letter_opener_web/letters#clear # delete_letter DELETE /:id(.:format) letter_opener_web/letters#destroy diff --git a/test/controllers/check_notifications_controller_test.rb b/test/controllers/check_notifications_controller_test.rb new file mode 100644 index 0000000..57b93cf --- /dev/null +++ b/test/controllers/check_notifications_controller_test.rb @@ -0,0 +1,10 @@ +# Copyright (C) 2018 Colin Darie , 2018 Evolix +# License: GNU AGPL-3+ (see full text in LICENSE file) + +require "test_helper" + +class CheckNotificationsControllerTest < ActionDispatch::IntegrationTest + # test "the truth" do + # assert true + # end +end diff --git a/test/controllers/notifications_controller_test.rb b/test/controllers/notifications_controller_test.rb index c3ba49f..d608893 100644 --- a/test/controllers/notifications_controller_test.rb +++ b/test/controllers/notifications_controller_test.rb @@ -1,7 +1,4 @@ -# Copyright (C) 2018 Colin Darie , 2018 Evolix -# License: GNU AGPL-3+ (see full text in LICENSE file) - -require "test_helper" +require 'test_helper' class NotificationsControllerTest < ActionDispatch::IntegrationTest # test "the truth" do From 54d3dbad12408f409037a9aac5a2a9e337207bf7 Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Thu, 30 Aug 2018 14:53:39 +0200 Subject: [PATCH 04/10] Updated factory & policies for new notification template --- test/factories/checks.rb | 2 +- .../check_notification_policy_test.rb | 35 +++++++++++++++++++ test/policies/notification_policy_test.rb | 28 ++++++++------- 3 files changed, 51 insertions(+), 14 deletions(-) create mode 100644 test/policies/check_notification_policy_test.rb diff --git a/test/factories/checks.rb b/test/factories/checks.rb index 70055d4..c7e5243 100644 --- a/test/factories/checks.rb +++ b/test/factories/checks.rb @@ -86,7 +86,7 @@ FactoryBot.define do trait :with_notifications do after :create do |check| - create_list :notification, 2, check: check + create_list :check_notification, 2, check: check end end end diff --git a/test/policies/check_notification_policy_test.rb b/test/policies/check_notification_policy_test.rb new file mode 100644 index 0000000..318a2fb --- /dev/null +++ b/test/policies/check_notification_policy_test.rb @@ -0,0 +1,35 @@ +# Copyright (C) 2018 Colin Darie , 2018 Evolix +# License: GNU AGPL-3+ (see full text in LICENSE file) + +require "test_helper" + +class CheckNotificationPolicyTest < ActiveSupport::TestCase + setup do + @owner, @other = create_list(:user, 2) + @check_notification = create(:check_notification, check: build(:check, user: @owner)) + end + + test "permit to check user" do + assert_permit @owner, @check_notification, :destroy + end + + test "disallow to anonymous and other user" do + refute_permit @other, @check_notification, :destroy + refute_permit nil, @check_notification, :destroy + end + + test "scope only to user checks" do + other_notifications = create_list(:check_notification, 2, check: build(:check, user: @other)) + + assert_empty Pundit.policy_scope!(nil, CheckNotification) + assert_equal [@check_notification], Pundit.policy_scope!(@owner, CheckNotification) + assert_equal other_notifications, Pundit.policy_scope!(@other, CheckNotification) + end + + test "disabled actions" do + refute_permit @owner, @check_notification, :update + refute_permit @owner, @check_notification, :edit + refute_permit @owner, @check_notification, :create + refute_permit @owner, @check_notification, :index + end +end diff --git a/test/policies/notification_policy_test.rb b/test/policies/notification_policy_test.rb index 688149e..cf9254c 100644 --- a/test/policies/notification_policy_test.rb +++ b/test/policies/notification_policy_test.rb @@ -6,30 +6,32 @@ require "test_helper" class NotificationPolicyTest < ActiveSupport::TestCase setup do @owner, @other = create_list(:user, 2) - @notification = create(:notification, check: build(:check, user: @owner)) + @notification = create(:notification, user: @owner) end - test "permit to check user" do + test "create" do + assert_permit @other, Notification, :create + assert_permit @other, Notification, :new + end + + test "permit to owner" do + assert_permit @owner, @notification, :edit + assert_permit @owner, @notification, :update assert_permit @owner, @notification, :destroy end test "disallow to anonymous and other user" do - refute_permit @other, @notification, :destroy - refute_permit nil, @notification, :destroy + %i[update edit destroy].each do |action| + refute_permit @other, @notification, action + refute_permit nil, @notification, action + end end - test "scope only to user checks" do - other_notifications = create_list(:notification, 2, check: build(:check, user: @other)) + test "scope only to owners" do + other_notifications = create_list(:notification, 2, user: @other) assert_empty Pundit.policy_scope!(nil, Notification) assert_equal [@notification], Pundit.policy_scope!(@owner, Notification) assert_equal other_notifications, Pundit.policy_scope!(@other, Notification) end - - test "disabled actions" do - refute_permit @owner, @notification, :update - refute_permit @owner, @notification, :edit - refute_permit @owner, @notification, :create - refute_permit @owner, @notification, :index - end end From bd7784bf8d803ae31f8c722bdbe0181b2f66aff5 Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Thu, 30 Aug 2018 16:25:50 +0200 Subject: [PATCH 05/10] Redirect home for signed up users to their checks --- app/controllers/application_controller.rb | 12 ++++++++++++ app/controllers/pages_controller.rb | 4 +++- app/views/shared/_navbar.html.erb | 2 +- test/system/users_test.rb | 2 +- 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 3fa3d5e..1d70ea6 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -15,6 +15,18 @@ class ApplicationController < ActionController::Base devise_parameter_sanitizer.permit(:account_update, keys: [:notifications_enabled, :locale]) end + def after_sign_in_path_for(_resource) + checks_path + end + + def after_sign_up_path_for(_resource) + checks_path + end + + def after_sign_out_path_for(_resource) + root_path + end + def user_not_authorized flash[:alert] = I18n.t("user_not_authorized", scope: :flashes) redirect_to(request.referrer || root_path) diff --git a/app/controllers/pages_controller.rb b/app/controllers/pages_controller.rb index d0c1067..8c5d170 100644 --- a/app/controllers/pages_controller.rb +++ b/app/controllers/pages_controller.rb @@ -2,5 +2,7 @@ # License: GNU AGPL-3+ (see full text in LICENSE file) class PagesController < ApplicationController - def home; end + def home + redirect_to checks_path if user_signed_in? + end end diff --git a/app/views/shared/_navbar.html.erb b/app/views/shared/_navbar.html.erb index 4da000b..f4ba67b 100644 --- a/app/views/shared/_navbar.html.erb +++ b/app/views/shared/_navbar.html.erb @@ -2,7 +2,7 @@ <% # License: GNU AGPL-3+ (see full text in LICENSE file) %>