Commit 20a8936c authored by Peter Leitzen's avatar Peter Leitzen

Merge branch 'sarnold-219266-service-for-status-page-publish-quick-action' into 'master'

Refactor /publish to use a designated service entrypoint rather than queue a worker

Closes #219266

See merge request gitlab-org/gitlab!33925
parents ff387090 d4ec5c8c
...@@ -17,10 +17,13 @@ module StatusPage ...@@ -17,10 +17,13 @@ module StatusPage
PUBLISH_WHEN_ISSUE_CHANGED = PUBLISH_WHEN_ISSUE_CHANGED =
%w[title description confidential state_id closed_by_id].freeze %w[title description confidential state_id closed_by_id].freeze
def initialize(project, user, triggered_by) VALID_ACTIONS = %i[init update].freeze
def initialize(project, user, triggered_by, action:)
@project = project @project = project
@user = user @user = user
@triggered_by = triggered_by @triggered_by = triggered_by
@action = set_action(action)
end end
def execute def execute
...@@ -33,7 +36,21 @@ module StatusPage ...@@ -33,7 +36,21 @@ module StatusPage
private private
attr_reader :user, :project, :triggered_by attr_reader :user, :project, :triggered_by, :action
def set_action(action)
raise ArgumentError, 'Invalid action' unless VALID_ACTIONS.include?(action)
action
end
def update?
action == :update
end
def init?
action == :init
end
def can_publish? def can_publish?
user&.can?(:publish_status_page, project) user&.can?(:publish_status_page, project)
...@@ -59,13 +76,14 @@ module StatusPage ...@@ -59,13 +76,14 @@ module StatusPage
# Trigger publish for public (non-confidential) issues # Trigger publish for public (non-confidential) issues
# - which were changed # - which were changed
# - which were not changed, and the action is not update (i.e init action)
# - just become confidential to unpublish # - just become confidential to unpublish
def eligable_issue_id_from_issue def eligable_issue_id_from_issue
issue = triggered_by issue = triggered_by
changes = issue.previous_changes.keys & PUBLISH_WHEN_ISSUE_CHANGED changes = issue.previous_changes.keys & PUBLISH_WHEN_ISSUE_CHANGED
return if changes.none? return if update? && changes.none?
return if issue.confidential? && changes.exclude?('confidential') return if issue.confidential? && changes.exclude?('confidential')
return unless issue.status_page_published_incident return unless issue.status_page_published_incident
......
...@@ -143,10 +143,7 @@ module EE ...@@ -143,10 +143,7 @@ module EE
end end
command :publish do command :publish do
if StatusPage.mark_for_publication(project, current_user, quick_action_target).success? if StatusPage.mark_for_publication(project, current_user, quick_action_target).success?
# Ideally, we want to use `StatusPage.trigger_publish` instead of dispatching a worker. StatusPage.trigger_publish(project, current_user, quick_action_target, action: :init)
# See https://gitlab.com/gitlab-org/gitlab/-/issues/219266
StatusPage::PublishWorker.perform_async(current_user.id, project.id, quick_action_target.id)
@execution_message[:publish] = _('Issue published on status page.') @execution_message[:publish] = _('Issue published on status page.')
else else
@execution_message[:publish] = _('Failed to publish issue on status page.') @execution_message[:publish] = _('Failed to publish issue on status page.')
......
...@@ -7,8 +7,8 @@ module StatusPage ...@@ -7,8 +7,8 @@ module StatusPage
AWARD_EMOJI = 'microphone' AWARD_EMOJI = 'microphone'
# Convenient method to trigger a status page update. # Convenient method to trigger a status page update.
def self.trigger_publish(project, user, triggered_by) def self.trigger_publish(project, user, triggered_by, action: :update)
TriggerPublishService.new(project, user, triggered_by).execute TriggerPublishService.new(project, user, triggered_by, action: action).execute
end end
# Method to mark an issue as published and trigger update # Method to mark an issue as published and trigger update
......
...@@ -12,7 +12,7 @@ RSpec.describe StatusPage do ...@@ -12,7 +12,7 @@ RSpec.describe StatusPage do
it 'delegates to TriggerPublishService' do it 'delegates to TriggerPublishService' do
expect_next_instance_of(StatusPage::TriggerPublishService, expect_next_instance_of(StatusPage::TriggerPublishService,
project, user, triggered_by) do |service| project, user, triggered_by, action: :update) do |service|
expect(service).to receive(:execute) expect(service).to receive(:execute)
end end
......
...@@ -6,7 +6,7 @@ RSpec.describe StatusPage::TriggerPublishService do ...@@ -6,7 +6,7 @@ RSpec.describe StatusPage::TriggerPublishService do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:project, refind: true) { create(:project, :repository) } let_it_be(:project, refind: true) { create(:project, :repository) }
let(:service) { described_class.new(project, user, triggered_by) } let(:service) { described_class.new(project, user, triggered_by, action: :update) }
describe '#execute' do describe '#execute' do
# Variables used by shared examples # Variables used by shared examples
...@@ -18,6 +18,16 @@ RSpec.describe StatusPage::TriggerPublishService do ...@@ -18,6 +18,16 @@ RSpec.describe StatusPage::TriggerPublishService do
subject { service.execute } subject { service.execute }
describe 'invalid action' do
let(:service) { described_class.new(project, user, double(:issue), action: :something_invalid) }
it 'raises an argument error and does not process' do
expect(StatusPage::PublishWorker).not_to receive(:perform_async)
expect { subject }.to raise_error(ArgumentError)
end
end
describe 'triggered by issue' do describe 'triggered by issue' do
let_it_be(:triggered_by, reload: true) { create(:issue, :published, project: project) } let_it_be(:triggered_by, reload: true) { create(:issue, :published, project: project) }
let(:issue_id) { triggered_by.id } let(:issue_id) { triggered_by.id }
...@@ -41,6 +51,12 @@ RSpec.describe StatusPage::TriggerPublishService do ...@@ -41,6 +51,12 @@ RSpec.describe StatusPage::TriggerPublishService do
context 'without changes' do context 'without changes' do
include_examples 'no trigger status page publish' include_examples 'no trigger status page publish'
context 'with init action' do
let(:service) { described_class.new(project, user, triggered_by, action: :init) }
include_examples 'trigger status page publish'
end
end end
context 'when a confidential issue changes' do context 'when a confidential issue changes' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment