diff --git a/app/models/integrations/base_chat_notification.rb b/app/models/integrations/base_chat_notification.rb index fa5ce098379c48a939ac42260c24b77cf37ed019..963ba918089024c7a63c2594274c203aa44e94af 100644 --- a/app/models/integrations/base_chat_notification.rb +++ b/app/models/integrations/base_chat_notification.rb @@ -23,6 +23,7 @@ class BaseChatNotification < Integration ].freeze SECRET_MASK = '************' + CHANNEL_LIMIT_PER_EVENT = 10 attribute :category, default: 'chat' @@ -37,7 +38,8 @@ class BaseChatNotification < Integration presence: true, public_url: true, if: -> (integration) { integration.activated? && integration.requires_webhook? } - validates :labels_to_be_notified_behavior, inclusion: { in: LABEL_NOTIFICATION_BEHAVIOURS }, allow_blank: true + validates :labels_to_be_notified_behavior, inclusion: { in: LABEL_NOTIFICATION_BEHAVIOURS }, allow_blank: true, if: :activated? + validate :validate_channel_limit, if: :activated? def initialize_properties super @@ -300,7 +302,7 @@ def channels_for_event(event) channel_names = event_channel_value(event).presence || channel.presence return [] unless channel_names - channel_names.split(',').map(&:strip) + channel_names.split(',').map(&:strip).uniq end def unique_channels @@ -308,6 +310,21 @@ def unique_channels channels_for_event(event) end.uniq end + + def validate_channel_limit + supported_events.each do |event| + count = channels_for_event(event).count + next unless count > CHANNEL_LIMIT_PER_EVENT + + errors.add( + event_channel_name(event).to_sym, + format( + s_('SlackIntegration|cannot have more than %{limit} channels'), + limit: CHANNEL_LIMIT_PER_EVENT + ) + ) + end + end end end diff --git a/doc/user/project/integrations/mattermost.md b/doc/user/project/integrations/mattermost.md index 39b89cd87a9946763f42284978490a9cfac69dc3..3782d3acd0cb35f7300d40280452d6630c9d45ce 100644 --- a/doc/user/project/integrations/mattermost.md +++ b/doc/user/project/integrations/mattermost.md @@ -36,6 +36,8 @@ Display name override is not enabled by default, you need to ask your administra ## Configure GitLab to send notifications to Mattermost +> [Changed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106760) in GitLab 15.9 to limit Mattermost channels to 10 per event. + After the Mattermost instance has an incoming webhook set up, you can set up GitLab to send the notifications: diff --git a/doc/user/project/integrations/slack.md b/doc/user/project/integrations/slack.md index 9932e782ff41b5937ef51da0e1d228d1bfadb5d6..d14401a5c9dedcd923501fd9608e168fede15cbb 100644 --- a/doc/user/project/integrations/slack.md +++ b/doc/user/project/integrations/slack.md @@ -28,6 +28,8 @@ to control GitLab from Slack. Slash commands are configured separately. ## Configure GitLab +> [Changed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106760) in GitLab 15.9 to limit Slack channels to 10 per event. + 1. On the top bar, select **Main menu > Projects** and find your project. 1. On the left sidebar, select **Settings > Integrations**. 1. Select **Slack notifications**. diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 13ae7fc419eeaf701f132be16acd3564277e22c7..570e19285621ee35b5da2028c82e71fe2a0d96ef 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -40071,6 +40071,9 @@ msgstr "" msgid "SlackIntegration|You may need to reinstall the GitLab for Slack app when we %{linkStart}make updates or change permissions%{linkEnd}." msgstr "" +msgid "SlackIntegration|cannot have more than %{limit} channels" +msgstr "" + msgid "SlackModal|Are you sure you want to change the project?" msgstr "" diff --git a/spec/models/integrations/base_chat_notification_spec.rb b/spec/models/integrations/base_chat_notification_spec.rb index 1527ffd727839587977472e7f441dc420d074178..13dd9e03ab118238ee4535e48b7fa8e5d54e597d 100644 --- a/spec/models/integrations/base_chat_notification_spec.rb +++ b/spec/models/integrations/base_chat_notification_spec.rb @@ -9,13 +9,33 @@ describe 'validations' do before do - allow(subject).to receive(:activated?).and_return(true) + subject.active = active + allow(subject).to receive(:default_channel_placeholder).and_return('placeholder') allow(subject).to receive(:webhook_help).and_return('help') end - it { is_expected.to validate_presence_of :webhook } - it { is_expected.to validate_inclusion_of(:labels_to_be_notified_behavior).in_array(%w[match_any match_all]).allow_blank } + def build_channel_list(count) + (1..count).map { |i| "##{i}" }.join(',') + end + + context 'when active' do + let(:active) { true } + + it { is_expected.to validate_presence_of :webhook } + it { is_expected.to validate_inclusion_of(:labels_to_be_notified_behavior).in_array(%w[match_any match_all]).allow_blank } + it { is_expected.to allow_value(build_channel_list(10)).for(:push_channel) } + it { is_expected.not_to allow_value(build_channel_list(11)).for(:push_channel) } + end + + context 'when inactive' do + let(:active) { false } + + it { is_expected.not_to validate_presence_of :webhook } + it { is_expected.not_to validate_inclusion_of(:labels_to_be_notified_behavior).in_array(%w[match_any match_all]).allow_blank } + it { is_expected.to allow_value(build_channel_list(10)).for(:push_channel) } + it { is_expected.to allow_value(build_channel_list(11)).for(:push_channel) } + end end describe '#execute' do @@ -309,6 +329,10 @@ context 'with multiple channel names with spaces specified' do it_behaves_like 'with channel specified', 'slack-integration, #slack-test, @UDLP91W0A', ['slack-integration', '#slack-test', '@UDLP91W0A'] end + + context 'with duplicate channel names' do + it_behaves_like 'with channel specified', '#slack-test,#slack-test,#slack-test-2', ['#slack-test', '#slack-test-2'] + end end describe '#default_channel_placeholder' do