From 74df47ad9cc5801041b4fc19d542bc79f000026c Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Mon, 25 Nov 2024 03:19:16 -0500 Subject: [PATCH] Add coverage for `Webhook` validations (#33026) --- app/models/webhook.rb | 2 +- spec/models/webhook_spec.rb | 116 +++++++++++++++++++++++++++++++++--- 2 files changed, 108 insertions(+), 10 deletions(-) diff --git a/app/models/webhook.rb b/app/models/webhook.rb index 304b2b1f18..f9d6564c92 100644 --- a/app/models/webhook.rb +++ b/app/models/webhook.rb @@ -53,7 +53,7 @@ class Webhook < ApplicationRecord end def required_permissions - events.map { |event| Webhook.permission_for_event(event) } + events.map { |event| Webhook.permission_for_event(event) }.uniq end def self.permission_for_event(event) diff --git a/spec/models/webhook_spec.rb b/spec/models/webhook_spec.rb index 03ef1dcc68..18a6047dd0 100644 --- a/spec/models/webhook_spec.rb +++ b/spec/models/webhook_spec.rb @@ -6,20 +6,28 @@ RSpec.describe Webhook do let(:webhook) { Fabricate(:webhook) } describe 'Validations' do + subject { Fabricate.build :webhook } + it { is_expected.to validate_presence_of(:events) } - it 'requires non-empty events value' do - record = described_class.new(events: []) - record.valid? + it { is_expected.to_not allow_values([], %w(account.invalid)).for(:events) } - expect(record).to model_have_error_on_field(:events) - end + it { is_expected.to_not allow_values('{{account }').for(:template) } - it 'requires valid events value from EVENTS' do - record = described_class.new(events: ['account.invalid']) - record.valid? + context 'when current_account is assigned' do + subject { Fabricate.build :webhook, current_account: account } - expect(record).to model_have_error_on_field(:events) + context 'with account that has permissions' do + let(:account) { Fabricate(:user, role: UserRole.find_by(name: 'Admin')).account } + + it { is_expected.to allow_values(%w(account.created)).for(:events) } + end + + context 'with account lacking permissions' do + let(:account) { Fabricate :account } + + it { is_expected.to_not allow_values(%w(account.created)).for(:events) } + end end end @@ -29,6 +37,96 @@ RSpec.describe Webhook do end end + describe 'Callbacks' do + describe 'Generating a secret' do + context 'when secret exists already' do + subject { described_class.new(secret: 'secret') } + + it 'does not override' do + expect { subject.valid? } + .to_not change(subject, :secret) + end + end + + context 'when secret does not exist' do + subject { described_class.new(secret: nil) } + + it 'does not override' do + expect { subject.valid? } + .to change(subject, :secret) + end + end + end + end + + describe '.permission_for_event' do + subject { described_class.permission_for_event(event) } + + context 'with a nil value' do + let(:event) { nil } + + it { is_expected.to be_nil } + end + + context 'with an account approved event' do + let(:event) { 'account.approved' } + + it { is_expected.to eq(:manage_users) } + end + + context 'with an account created event' do + let(:event) { 'account.created' } + + it { is_expected.to eq(:manage_users) } + end + + context 'with an account updated event' do + let(:event) { 'account.updated' } + + it { is_expected.to eq(:manage_users) } + end + + context 'with an report created event' do + let(:event) { 'report.created' } + + it { is_expected.to eq(:manage_reports) } + end + + context 'with an report updated event' do + let(:event) { 'report.updated' } + + it { is_expected.to eq(:manage_reports) } + end + + context 'with an status created event' do + let(:event) { 'status.created' } + + it { is_expected.to eq(:view_devops) } + end + + context 'with an status updated event' do + let(:event) { 'status.updated' } + + it { is_expected.to eq(:view_devops) } + end + end + + describe '#required_permissions' do + subject { described_class.new(events:).required_permissions } + + context 'with empty events' do + let(:events) { [] } + + it { is_expected.to eq([]) } + end + + context 'with multiple event types' do + let(:events) { %w(account.created account.updated status.created) } + + it { is_expected.to eq %i(manage_users view_devops) } + end + end + describe '#rotate_secret!' do it 'changes the secret' do expect { webhook.rotate_secret! }