From 9a1c54a82d55b2eab3db1f1685cdaf56e0b42ee9 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Thu, 28 Mar 2024 13:14:16 +0530 Subject: [PATCH] feat: disable automation rules if condition fails multiple times (#9017) * feat: add email for disabling automation rule * feat: disable automation rules and notify admin * feat: reset error count after update * feat: trigger invalid_condition_error if rule is invalid * feat: setup error trackable concern * refactor: use ErrorTrackable in Reauthorizable * fix: optional argument * feat: separate reauthorization_required_key * test: update case to use ERROR_TRACKABLE_COUNT * Revert "test: update case to use ERROR_TRACKABLE_COUNT" This reverts commit f439847147556a02759a7597a7fcf1d66091cafc. * Revert "feat: separate reauthorization_required_key" This reverts commit f4514fce217b0a2f2c2bf701a15de0a8b47acbc4. * Revert "fix: optional argument" This reverts commit 93b4194ec3f10f67e2402388c966c071c4d3b4fd. * Revert "refactor: use ErrorTrackable in Reauthorizable" This reverts commit 513c2a522bc782e73ea4b0f5ae34ce01e70e042c. * Revert "feat: setup error trackable concern" This reverts commit 278683060cf422f60af5d5c77100aa5272141141. * feat: use reauthorizable for automation rule * feat: remove redis key * test: fix method names * chore: refactor --------- Co-authored-by: Vishnu Narayanan Co-authored-by: Sojan --- .../channel_notifications_mailer.rb | 12 +++++++ app/models/automation_rule.rb | 3 ++ app/models/concerns/reauthorizable.rb | 3 ++ .../conditions_filter_service.rb | 2 +- .../automation_rule_disabled.liquid | 8 +++++ spec/models/automation_rule_spec.rb | 36 +++++++++++++++++++ spec/models/concerns/reauthorizable_shared.rb | 9 +++++ 7 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 app/views/mailers/administrator_notifications/channel_notifications_mailer/automation_rule_disabled.liquid diff --git a/app/mailers/administrator_notifications/channel_notifications_mailer.rb b/app/mailers/administrator_notifications/channel_notifications_mailer.rb index 6c0f7cee2..dc4e6d7fe 100644 --- a/app/mailers/administrator_notifications/channel_notifications_mailer.rb +++ b/app/mailers/administrator_notifications/channel_notifications_mailer.rb @@ -69,6 +69,18 @@ class AdministratorNotifications::ChannelNotificationsMailer < ApplicationMailer send_mail_with_liquid(to: email_to, subject: subject) and return end + def automation_rule_disabled(rule) + return unless smtp_config_set_or_development? + + @action_url ||= "#{ENV.fetch('FRONTEND_URL', nil)}/app/accounts/#{Current.account.id}/settings/automation/list" + + subject = 'Automation rule disabled due to validation errors.'.freeze + @meta = {} + @meta['rule_name'] = rule.name + + send_mail_with_liquid(to: admin_emails, subject: subject) and return + end + private def admin_emails diff --git a/app/models/automation_rule.rb b/app/models/automation_rule.rb index 955df7497..9f7da28a0 100644 --- a/app/models/automation_rule.rb +++ b/app/models/automation_rule.rb @@ -19,6 +19,7 @@ # class AutomationRule < ApplicationRecord include Rails.application.routes.url_helpers + include Reauthorizable belongs_to :account has_many_attached :files @@ -28,6 +29,8 @@ class AutomationRule < ApplicationRecord validate :query_operator_presence validates :account_id, presence: true + after_update_commit :reauthorized!, if: -> { saved_change_to_conditions? } + scope :active, -> { where(active: true) } def conditions_attributes diff --git a/app/models/concerns/reauthorizable.rb b/app/models/concerns/reauthorizable.rb index fa46d0060..0fc234ada 100644 --- a/app/models/concerns/reauthorizable.rb +++ b/app/models/concerns/reauthorizable.rb @@ -50,6 +50,9 @@ module Reauthorizable mailer.whatsapp_disconnect(inbox).deliver_later when 'Channel::Email' mailer.email_disconnect(inbox).deliver_later + when 'AutomationRule' + update!(active: false) + mailer.automation_rule_disabled(self).deliver_later end end diff --git a/app/services/automation_rules/conditions_filter_service.rb b/app/services/automation_rules/conditions_filter_service.rb index daf9cb7e2..23873371d 100644 --- a/app/services/automation_rules/conditions_filter_service.rb +++ b/app/services/automation_rules/conditions_filter_service.rb @@ -45,8 +45,8 @@ class AutomationRules::ConditionsFilterService < FilterService def rule_valid? is_valid = AutomationRules::ConditionValidationService.new(@rule).perform - Rails.logger.info "Automation rule condition validation failed for rule id: #{@rule.id}" unless is_valid + @rule.authorization_error! unless is_valid is_valid end diff --git a/app/views/mailers/administrator_notifications/channel_notifications_mailer/automation_rule_disabled.liquid b/app/views/mailers/administrator_notifications/channel_notifications_mailer/automation_rule_disabled.liquid new file mode 100644 index 000000000..56aefd6b7 --- /dev/null +++ b/app/views/mailers/administrator_notifications/channel_notifications_mailer/automation_rule_disabled.liquid @@ -0,0 +1,8 @@ +

Hello there,

+ +

The automation rule {{meta['rule_name']}} has been disabled becuase it has invalid conditions.

+

This typically happens when you delete any custom attributes which are still being used in automation rules.

+ +

+Click here to update the conditions. +

diff --git a/spec/models/automation_rule_spec.rb b/spec/models/automation_rule_spec.rb index a20d3d71d..53ebfa0c7 100644 --- a/spec/models/automation_rule_spec.rb +++ b/spec/models/automation_rule_spec.rb @@ -1,6 +1,11 @@ require 'rails_helper' +require Rails.root.join 'spec/models/concerns/reauthorizable_shared.rb' RSpec.describe AutomationRule do + describe 'concerns' do + it_behaves_like 'reauthorizable' + end + describe 'associations' do let(:account) { create(:account) } let(:params) do @@ -56,4 +61,35 @@ RSpec.describe AutomationRule do expect(rule.errors.messages[:conditions]).to eq(['Automation conditions should have query operator.']) end end + + describe 'reauthorizable' do + context 'when prompt_reauthorization!' do + it 'marks the rule inactive' do + rule = create(:automation_rule) + expect(rule.active).to be true + rule.prompt_reauthorization! + expect(rule.active).to be false + end + end + + context 'when reauthorization_required?' do + it 'unsets the error count if conditions are updated' do + rule = create(:automation_rule) + rule.prompt_reauthorization! + expect(rule.reauthorization_required?).to be true + + rule.update!(conditions: [{ attribute_key: 'browser_language', filter_operator: 'equal_to', values: ['en'], query_operator: 'AND' }]) + expect(rule.reauthorization_required?).to be false + end + + it 'will not unset the error count if conditions are not updated' do + rule = create(:automation_rule) + rule.prompt_reauthorization! + expect(rule.reauthorization_required?).to be true + + rule.update!(name: 'Updated name') + expect(rule.reauthorization_required?).to be true + end + end + end end diff --git a/spec/models/concerns/reauthorizable_shared.rb b/spec/models/concerns/reauthorizable_shared.rb index 0bfa112c4..9efe232e8 100644 --- a/spec/models/concerns/reauthorizable_shared.rb +++ b/spec/models/concerns/reauthorizable_shared.rb @@ -25,10 +25,19 @@ shared_examples_for 'reauthorizable' do it 'prompt_reauthorization!' do obj = FactoryBot.create(model.to_s.underscore.tr('/', '_').to_sym) + mailer = double + mailer_method = double + allow(AdministratorNotifications::ChannelNotificationsMailer).to receive(:with).and_return(mailer) + # allow mailer to receive any methods and return mailer + allow(mailer).to receive(:method_missing).and_return(mailer_method) + allow(mailer_method).to receive(:deliver_later) + expect(obj.reauthorization_required?).to be false obj.prompt_reauthorization! expect(obj.reauthorization_required?).to be true + expect(AdministratorNotifications::ChannelNotificationsMailer).to have_received(:with).with(account: obj.account) + expect(mailer_method).to have_received(:deliver_later) end it 'reauthorized!' do