From b7807f3d84983ab46c3345d076576ffa8433dab4 Mon Sep 17 00:00:00 2001
From: Matt Jankowski <matt@jankowski.online>
Date: Mon, 13 Nov 2023 17:47:44 -0500
Subject: [PATCH] Use `normalizes` to prepare `Webhook#events` value (#27605)

---
 app/models/webhook.rb       | 16 ++++++++--------
 spec/models/webhook_spec.rb | 31 +++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/app/models/webhook.rb b/app/models/webhook.rb
index 044097921e..304b2b1f18 100644
--- a/app/models/webhook.rb
+++ b/app/models/webhook.rb
@@ -33,11 +33,11 @@ class Webhook < ApplicationRecord
   validates :secret, presence: true, length: { minimum: 12 }
   validates :events, presence: true
 
-  validate :validate_events
+  validate :events_validation_error, if: :invalid_events?
   validate :validate_permissions
   validate :validate_template
 
-  before_validation :strip_events
+  normalizes :events, with: ->(events) { events.filter_map { |event| event.strip.presence } }
   before_validation :generate_secret
 
   def rotate_secret!
@@ -69,8 +69,12 @@ class Webhook < ApplicationRecord
 
   private
 
-  def validate_events
-    errors.add(:events, :invalid) if events.any? { |e| EVENTS.exclude?(e) }
+  def events_validation_error
+    errors.add(:events, :invalid)
+  end
+
+  def invalid_events?
+    events.blank? || events.difference(EVENTS).any?
   end
 
   def validate_permissions
@@ -88,10 +92,6 @@ class Webhook < ApplicationRecord
     end
   end
 
-  def strip_events
-    self.events = events.filter_map { |str| str.strip.presence } if events.present?
-  end
-
   def generate_secret
     self.secret = SecureRandom.hex(20) if secret.blank?
   end
diff --git a/spec/models/webhook_spec.rb b/spec/models/webhook_spec.rb
index 715dd7574f..effaf92e9c 100644
--- a/spec/models/webhook_spec.rb
+++ b/spec/models/webhook_spec.rb
@@ -5,6 +5,37 @@ require 'rails_helper'
 RSpec.describe Webhook do
   let(:webhook) { Fabricate(:webhook) }
 
+  describe 'Validations' do
+    it 'requires presence of events' do
+      record = described_class.new(events: nil)
+      record.valid?
+
+      expect(record).to model_have_error_on_field(:events)
+    end
+
+    it 'requires non-empty events value' do
+      record = described_class.new(events: [])
+      record.valid?
+
+      expect(record).to model_have_error_on_field(:events)
+    end
+
+    it 'requires valid events value from EVENTS' do
+      record = described_class.new(events: ['account.invalid'])
+      record.valid?
+
+      expect(record).to model_have_error_on_field(:events)
+    end
+  end
+
+  describe 'Normalizations' do
+    it 'cleans up events values' do
+      record = described_class.new(events: ['account.approved', 'account.created     ', ''])
+
+      expect(record.events).to eq(%w(account.approved account.created))
+    end
+  end
+
   describe '#rotate_secret!' do
     it 'changes the secret' do
       previous_value = webhook.secret