Merge branch 'pl-status-page-mvc-changes-issues' into 'master'

Publish status page on issue changes

See merge request gitlab-org/gitlab!27575

ee/app/serializers/status_page/incident_entity.rb

 # frozen_string_literal: true
 
 module StatusPage
+  # Note: Any new fields exposures should also be added to
+  # +StatusPage::TriggerPublishService::PUBLISH_WHEN_ISSUE_CHANGED+.
   class IncidentEntity < Grape::Entity
     expose :iid, as: :id
     expose :state, as: :status

ee/app/services/ee/issues/create_service.rb

@@ -12,6 +12,13 @@ module EE
         super
       end
 
+      override :after_create
+      def after_create(issue)
+        super
+
+        StatusPage.trigger_publish(project, current_user, issue)
+      end
+
       def handle_issue_epic_link(issue)
         return unless params.key?(:epic)
 

ee/app/services/ee/issues/update_service.rb

@@ -16,6 +16,8 @@ module EE
           Epics::UpdateDatesService.new([issue.epic]).execute
         end
 
+        StatusPage.trigger_publish(project, current_user, issue) if issue.valid?
+
         result
       end
 

ee/app/services/status_page/trigger_publish_service.rb

@@ -3,24 +3,37 @@
 module StatusPage
   # Triggers a background job to publish of incidents to the status page.
   #
-  # Use this service when issues/notes/emoji have changed to kickoff the
-  # publish process.
+  # This service determines whether the passed +triggered_by+ (issue, note,
+  # or emoji) is eligible to kick-off the publish process.
   class TriggerPublishService
-    def initialize(user:, project:)
-      @user = user
+    include Gitlab::Utils::StrongMemoize
+
+    # Publish status page only if the following issue attributes have changed.
+    # If we expose new fields in +StatusPage::IncidentEntity+ add them to
+    # this list too.
+    #
+    # Note: `closed_by_id` is needed because we cannot rely on `state_id` in
+    # Issues::CloseService
+    PUBLISH_WHEN_ISSUE_CHANGED =
+      %w[title description confidential state_id closed_by_id].freeze
+
+    def initialize(project, user, triggered_by)
       @project = project
+      @user = user
+      @triggered_by = triggered_by
     end
 
-    def execute(issue_id)
+    def execute
       return unless can_publish?
       return unless status_page_enabled?
+      return unless issue_id
 
       StatusPage::PublishWorker.perform_async(user.id, project.id, issue_id)
     end
 
     private
 
-    attr_reader :user, :project
+    attr_reader :user, :project, :triggered_by
 
     def can_publish?
       user&.can?(:publish_status_page, project)
@@ -29,5 +42,28 @@ module StatusPage
     def status_page_enabled?
       project.status_page_setting&.enabled?
     end
+
+    def issue_id
+      strong_memoize(:issue_id) { eligable_issue_id }
+    end
+
+    def eligable_issue_id
+      case triggered_by
+      when Issue then eligable_issue_id_from_issue
+      else
+        raise ArgumentError, "unsupported trigger type #{triggered_by.class}"
+      end
+    end
+
+    def eligable_issue_id_from_issue
+      changes = triggered_by.previous_changes.keys & PUBLISH_WHEN_ISSUE_CHANGED
+
+      return if changes.none?
+      # Ignore updates for already confidential issues
+      # Note: Issues becoming confidential _will_ be unpublished.
+      return if triggered_by.confidential? && changes.exclude?('confidential')
+
+      triggered_by.id
+    end
   end
 end

ee/lib/status_page.rb

+# frozen_string_literal: true
+
+module StatusPage
+  # Convenient method to trigger a status page update.
+  def self.trigger_publish(project, user, triggered_by)
+    TriggerPublishService.new(project, user, triggered_by).execute
+  end
+end

ee/spec/lib/status_page_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe StatusPage do
+  describe '.trigger_publish' do
+    let(:project) { instance_double(Project) }
+    let(:user) { instance_double(User) }
+    let(:triggered_by) { instance_double(Issue) }
+
+    subject { described_class.trigger_publish(project, user, triggered_by) }
+
+    it 'delegates to TriggerPublishService' do
+      expect_next_instance_of(StatusPage::TriggerPublishService,
+                              project, user, triggered_by) do |service|
+        expect(service).to receive(:execute)
+      end
+
+      subject
+    end
+  end
+end

ee/spec/services/ee/issues/create_service_spec.rb

@@ -43,5 +43,22 @@ describe Issues::CreateService do
     it_behaves_like 'issue with epic_id parameter' do
       let(:execute) { service.execute }
     end
+
+    describe 'publish to status page' do
+      let(:execute) { service.execute }
+      let(:issue_id) { execute&.id }
+
+      context 'when creation succeeds' do
+        let(:params) { { title: 'New title' } }
+
+        include_examples 'trigger status page publish'
+      end
+
+      context 'when creation fails' do
+        let(:params) { { title: nil } }
+
+        include_examples 'no trigger status page publish'
+      end
+    end
   end
 end

ee/spec/services/ee/issues/update_service_spec.rb

@@ -185,5 +185,35 @@ describe Issues::UpdateService do
         end
       end
     end
+
+    describe 'publish to status page' do
+      let(:execute) { update_issue(params) }
+      let(:issue_id) { execute&.id }
+
+      context 'when update succeeds' do
+        let(:params) { { title: 'New title' } }
+
+        include_examples 'trigger status page publish'
+      end
+
+      context 'when closing' do
+        let(:params) { { state_event: 'close' } }
+
+        include_examples 'trigger status page publish'
+      end
+
+      context 'when reopening' do
+        let(:issue) { create(:issue, :closed, project: project) }
+        let(:params) { { state_event: 'reopen' } }
+
+        include_examples 'trigger status page publish'
+      end
+
+      context 'when update fails' do
+        let(:params) { { title: nil } }
+
+        include_examples 'no trigger status page publish'
+      end
+    end
   end
 end

ee/spec/services/status_page/trigger_publish_service_spec.rb

@@ -5,79 +5,135 @@ require 'spec_helper'
 describe StatusPage::TriggerPublishService do
   let_it_be(:user) { create(:user) }
   let_it_be(:project, refind: true) { create(:project) }
-  let_it_be(:issue) { create(:issue, project: project) }
-  let(:service) { described_class.new(user: user, project: project) }
-  let(:worker) { StatusPage::PublishWorker }
 
-  let_it_be(:status_page_setting) do
-    create(:status_page_setting, :enabled, project: project)
-  end
-
-  subject { service.execute(issue.id) }
+  let(:service) { described_class.new(project, user, triggered_by) }
 
-  shared_examples 'no job scheduled' do
-    it 'does not schedule a job' do
-      expect(worker).not_to receive(:perform_async)
+  describe '#execute' do
+    # Variables used by shared examples
+    let(:execute) { subject }
+    let(:issue_id) { triggered_by.id }
 
-      subject
+    let_it_be(:status_page_setting, reload: true) do
+      create(:status_page_setting, :enabled, project: project)
     end
-  end
 
-  describe '#execute' do
-    before do
-      project.add_maintainer(user)
-      stub_feature_flags(status_page: true)
-      stub_licensed_features(status_page: true)
+    subject { service.execute }
 
-      allow(worker).to receive(:perform_async)
-        .with(user.id, project.id, issue.id)
-    end
+    describe 'triggered by issue' do
+      let_it_be(:triggered_by, reload: true) { create(:issue, project: project) }
 
-    it 'schedules a job' do
-      expect(worker).to receive(:perform_async)
-        .with(user.id, project.id, issue.id)
+      using RSpec::Parameterized::TableSyntax
 
-      subject
-    end
+      where(:changes, :shared_example_name) do
+        { weight: 23 }              | 'no trigger status page publish'
+        { title: 'changed' }        | 'trigger status page publish'
+        { description: 'changed' }  | 'trigger status page publish'
+        { confidential: true }      | 'trigger status page publish'
+      end
 
-    context 'when status page is missing' do
-      before do
-        status_page_setting.destroy
+      with_them do
+        include_examples params[:shared_example_name] do
+          before do
+            triggered_by.update!(changes)
+          end
+        end
       end
 
-      include_examples 'no job scheduled'
-    end
+      context 'without changes' do
+        include_examples 'no trigger status page publish'
+      end
 
-    context 'when status page is not enabled' do
-      before do
-        status_page_setting.update!(enabled: false)
+      context 'when a confidential issue changes' do
+        let(:triggered_by) { create(:issue, :confidential, project: project) }
+
+        include_examples 'no trigger status page publish' do
+          before do
+            triggered_by.update!(title: 'changed')
+          end
+        end
       end
 
-      include_examples 'no job scheduled'
-    end
+      context 'when closing an issue' do
+        include_examples 'trigger status page publish' do
+          before do
+            # Mimic Issues::CloseService#close_issue
+            triggered_by.close!
+            triggered_by.update!(closed_by: user)
+          end
+        end
+      end
 
-    context 'when license is not available' do
-      before do
-        stub_licensed_features(status_page: false)
+      context 'when reopening an issue' do
+        include_examples 'trigger status page publish' do
+          let_it_be(:triggered_by) { create(:issue, :closed, project: project) }
+
+          before do
+            triggered_by.reopen!
+          end
+        end
       end
+    end
+
+    describe 'triggered by unsupported type' do
+      context 'for some abitary type' do
+        let(:triggered_by) { Object.new }
+
+        include_context 'status page enabled'
 
-      include_examples 'no job scheduled'
+        it 'raises ArgumentError' do
+          expect { subject }
+            .to raise_error(ArgumentError, 'unsupported trigger type Object')
+        end
+      end
     end
 
-    context 'when feature is disabled' do
-      before do
-        stub_feature_flags(status_page: false)
+    context 'with eligable triggered_by' do
+      let_it_be(:triggered_by) { create(:issue, project: project) }
+
+      context 'when eligable' do
+        include_examples 'trigger status page publish'
       end
 
-      include_examples 'no job scheduled'
-    end
+      context 'when status page is missing' do
+        include_examples 'no trigger status page publish' do
+          before do
+            project.status_page_setting.destroy
+            project.reload
+          end
+        end
+      end
+
+      context 'when status page is not enabled' do
+        include_examples 'no trigger status page publish' do
+          before do
+            project.status_page_setting.update!(enabled: false)
+          end
+        end
+      end
 
-    context 'when user cannot publish status page' do
-      before do
-        project.add_reporter(user)
+      context 'when license is not available' do
+        include_examples 'no trigger status page publish' do
+          before do
+            stub_licensed_features(status_page: false)
+          end
+        end
       end
 
-      include_examples 'no job scheduled'
+      context 'when feature is disabled' do
+        include_examples 'no trigger status page publish' do
+          before do
+            stub_feature_flags(status_page: false)
+          end
+        end
+      end
+
+      context 'when user cannot publish status page' do
+        include_examples 'no trigger status page publish' do
+          before do
+            project.add_reporter(user)
+          end
+        end
+      end
     end
   end
 end

ee/spec/support/shared_contexts/status_page/status_page_enabled_context.rb

+# frozen_string_literal: true
+
+RSpec.shared_context 'status page enabled' do
+  before do
+    project.add_maintainer(user)
+
+    stub_feature_flags(status_page: true)
+    stub_licensed_features(status_page: true)
+
+    unless project.status_page_setting
+      create(:status_page_setting, :enabled, project: project)
+    end
+  end
+end

ee/spec/support/shared_examples/status_page/trigger_publish_shared_examples.rb

+# frozen_string_literal: true
+
+# This shared_example requires the following variables:
+# - execute: Executes the service
+# - issue_id: The issue id to be published
+# - project: The project related to published issue
+# - user: The user who triggers the publish
+#
+# Usage:
+#
+#   include_examples 'trigger status page publish' do
+#     let(:execute) { service.execute }
+#     let(:issue_id) { execute.id }
+#   end
+RSpec.shared_examples 'trigger status page publish' do
+  include_context 'status page enabled'
+
+  it 'triggers status page publish' do
+    allow(StatusPage::PublishWorker)
+      .to receive(:perform_async)
+      .with(user.id, project.id, kind_of(Integer))
+
+    execute
+
+    expect(StatusPage::PublishWorker)
+      .to have_received(:perform_async)
+      .with(user.id, project.id, issue_id)
+  end
+end
+
+# This shared_example requires the following variables:
+# - execute: Executes the service
+# - project: The project related to published issue
+# - user: The user who triggers the publish
+#
+# Usage:
+#
+#   include_examples 'no trigger status page publish' do
+#     let(:execute) { service.execute }
+#   end
+RSpec.shared_examples 'no trigger status page publish' do
+  include_context 'status page enabled'
+
+  it 'does not trigger status page publish service' do
+    expect(StatusPage::PublishWorker).not_to receive(:perform_async)
+
+    execute
+  end
+end