Merge branch '13353-GraphQL-DesignType-event' into 'master'

Expose new GraphQL `event` field for `DesignType`

See merge request gitlab-org/gitlab-ee!15561

ee/app/graphql/types/design_management/design_type.rb

@@ -21,26 +21,58 @@ module Types
             description: 'The total count of user-created notes for this design'
       field :filename, GraphQL::STRING_TYPE, null: false
       field :full_path, GraphQL::STRING_TYPE, null: false
+      field :event,
+            Types::DesignManagement::DesignVersionEventEnum,
+            null: false,
+            description: 'The change that happened to the design at this version',
+            extras: [:parent]
       field :image, GraphQL::STRING_TYPE, null: false, extras: [:parent]
       field :diff_refs, Types::DiffRefsType, null: false, calls_gitaly: true
       field :versions,
             Types::DesignManagement::VersionType.connection_type,
             resolver: Resolvers::DesignManagement::VersionResolver,
-            description: "All versions related to this design ordered newest first",
+            description: 'All versions related to this design ordered newest first',
             extras: [:parent]
 
       def image(parent:)
-        # Find an `at_version` argument passed to a parent node.
-        #
-        # If no argument is found then a nil value for sha is fine
-        # and the image displayed will be the latest version
-        version_id = Gitlab::Graphql::FindArgumentInParent.find(parent, :at_version, limit_depth: 4)
-        sha = version_id ? GitlabSchema.object_from_id(version_id)&.sync&.sha : nil
+        sha = cached_stateful_version(parent).sha
+
+        Gitlab::Routing.url_helpers.project_design_url(design.project, design, sha)
+      end
+
+      def event(parent:)
+        version = cached_stateful_version(parent)
+
+        design_version = cached_design_versions_for_version(version)[design.id]
 
-        project = Gitlab::Graphql::Loaders::BatchModelLoader.new(Project, design.project_id).find
-        project = project&.sync
+        design_version&.event || Types::DesignManagement::DesignVersionEventEnum::NONE
+      end
+
+      # Returns a `DesignManagement::Version` for this query based on the
+      # `atVersion` argument passed to a parent node if present, or otherwise
+      # the most recent `Version` for the issue.
+      def cached_stateful_version(parent_node)
+        version_gid = Gitlab::Graphql::FindArgumentInParent.find(parent_node, :at_version)
+
+        # Caching is scoped to an `issue_id` to allow us to cache the
+        # most recent `Version` for an issue
+        Gitlab::SafeRequestStore.fetch([request_cache_base_key, 'stateful_version', object.issue_id, version_gid]) do
+          if version_gid
+            GitlabSchema.object_from_id(version_gid)&.sync
+          else
+            object.issue.design_versions.most_recent
+          end
+        end
+      end
+
+      def cached_design_versions_for_version(version)
+        Gitlab::SafeRequestStore.fetch([request_cache_base_key, 'design_versions_for_version', version.id]) do
+          version.design_versions.to_h { |dv| [dv.design_id, dv] }
+        end
+      end
 
-        Gitlab::Routing.url_helpers.project_design_url(project, design, sha)
+      def request_cache_base_key
+        self.class.name
       end
     end
   end

ee/app/graphql/types/design_management/design_version_event_enum.rb

+# frozen_string_literal: true
+
+module Types
+  module DesignManagement
+    class DesignVersionEventEnum < BaseEnum
+      graphql_name 'DesignVersionEvent'
+      description 'Mutation event of a Design within a Version'
+
+      NONE = 'NONE'
+
+      value NONE, 'No change'
+
+      ::DesignManagement::DesignVersion.events.keys.each do |event_name|
+        value event_name.upcase, value: event_name, description: "A #{event_name} event"
+      end
+    end
+  end
+end

ee/app/models/ee/issue.rb

@@ -19,6 +19,11 @@ module EE
       has_one :epic_issue
       has_one :epic, through: :epic_issue
       has_many :designs, class_name: "DesignManagement::Design", inverse_of: :issue
+      has_many :design_versions, class_name: "DesignManagement::Version", inverse_of: :issue do
+        def most_recent
+          ordered.first
+        end
+      end
 
       validates :weight, allow_nil: true, numericality: { greater_than_or_equal_to: 0 }
     end

ee/changelogs/unreleased/13353-GraphQL-DesignType-event.yml

+---
+title: Expose a new events property of DesignType in GraphQL that represents the change that happened to a Design within a given version.
+merge_request: 15561
+author:
+type: added

ee/spec/graphql/types/design_management/design_type_spec.rb

@@ -11,6 +11,7 @@ describe GitlabSchema.types['Design'] do
     expected_fields = %i[
       diff_refs
       discussions
+      event
       filename
       full_path
       id

ee/spec/graphql/types/design_management/design_version_event_enum_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe GitlabSchema.types['DesignVersionEvent'] do
+  it { expect(described_class.graphql_name).to eq('DesignVersionEvent') }
+
+  it 'exposes the correct event states' do
+    expect(described_class.values.keys).to include(*%w[CREATION MODIFICATION DELETION NONE])
+  end
+end

ee/spec/models/issue_spec.rb

@@ -30,8 +30,19 @@ describe Issue do
     end
   end
 
-  describe "relations" do
+  describe 'relations' do
     it { is_expected.to have_many(:designs) }
+    it { is_expected.to have_many(:design_versions) }
+
+    describe 'versions.most_recent' do
+      it 'returns the most recent version' do
+        issue = create(:issue)
+        create_list(:design_version, 2, issue: issue)
+        last_version = create(:design_version, issue: issue)
+
+        expect(issue.design_versions.most_recent).to eq(last_version)
+      end
+    end
   end
 
   it_behaves_like 'an editable mentionable with EE-specific mentions' do

ee/spec/requests/api/graphql/project/issue/designs/designs_spec.rb

@@ -6,8 +6,8 @@ describe "Getting designs related to an issue" do
   include GraphqlHelpers
   include DesignManagementTestHelpers
 
-  let(:design) { create(:design, :with_file, versions_count: 1) }
-  let(:current_user) { design.project.owner }
+  set(:design) { create(:design, :with_file, versions_count: 1) }
+  set(:current_user) { design.project.owner }
   let(:design_query) do
     <<~NODE
     designs {
@@ -70,7 +70,7 @@ describe "Getting designs related to an issue" do
     end
 
     context "with versions" do
-      let(:version) { design.versions.take }
+      set(:version) { design.versions.take }
       let(:design_query) do
         <<~NODE
         designs {
@@ -109,15 +109,18 @@ describe "Getting designs related to an issue" do
     end
 
     describe "viewing a design board at a particular version" do
-      let(:issue) { design.issue }
-      let(:all_versions) { issue.design_collection.versions.ordered }
-      let!(:second_design) { create(:design, :with_file, issue: issue, versions_count: 1) }
+      set(:issue) { design.issue }
+      set(:second_design) { create(:design, :with_file, issue: issue, versions_count: 1) }
+      set(:deleted_design) { create(:design, :with_versions, issue: issue, deleted: true, versions_count: 1) }
+      let(:all_versions) { issue.design_versions.ordered.reverse }
       let(:design_query) do
         <<~NODE
         designs(atVersion: "#{version.to_global_id}") {
           edges {
             node {
+              id
               image
+              event
               versions {
                 edges {
                   node {
@@ -138,14 +141,14 @@ describe "Getting designs related to an issue" do
         Gitlab::Routing.url_helpers.project_design_url(design.project, design, sha)
       end
 
-      def version_global_id(version)
-        version.to_global_id.to_s
+      def global_id(object)
+        object.to_global_id.to_s
       end
 
       # Filters just design nodes from the larger `design_response`
       def design_nodes
-        design_response.each do |response|
-          response['node'].delete('versions')
+        design_response.map do |response|
+          response['node']
         end
       end
 
@@ -156,57 +159,124 @@ describe "Getting designs related to an issue" do
         end
       end
 
-      context "viewing the original version" do
-        let(:version) { all_versions.last }
+      context "viewing the original version, when one design was created" do
+        let(:version) { all_versions.first }
 
-        it "only returns the first design, with the correct version of the design image" do
+        before do
           post_graphql(query, current_user: current_user)
+        end
 
-          expect(design_nodes).to eql(
-            [{ "node" => { "image" => image_url(design, version.sha) } }]
+        it "only returns the first design" do
+          expect(design_nodes).to contain_exactly(
+            a_hash_including("id" => global_id(design))
           )
         end
 
-        it "only returns one version record for the design (the original version)" do
-          post_graphql(query, current_user: current_user)
+        it "returns the correct version of the design image" do
+          expect(design_nodes).to contain_exactly(
+            a_hash_including("image" => image_url(design, version.sha))
+          )
+        end
 
-          expect(version_nodes).to eq(
-            [
-              [{ "node" => { "id" => version_global_id(version) } }]
-            ]
+        it "returns the correct event for the design in this version" do
+          expect(design_nodes).to contain_exactly(
+            a_hash_including("event" => "CREATION")
           )
         end
+
+        it "only returns one version record for the design (the original version)" do
+          expect(version_nodes).to eq([
+            [{ "node" => { "id" => global_id(version) } }]
+          ])
+        end
       end
 
-      context "viewing the newest version" do
-        let(:version) { all_versions.first }
+      context "viewing the second version, when one design was created" do
+        let(:version) { all_versions.second }
 
-        it "returns both designs, with the correct version of the design images" do
+        before do
           post_graphql(query, current_user: current_user)
+        end
 
-          expect(design_nodes).to eq(
-            [
-              { "node" => { "image" => image_url(design, version.sha) } },
-              { "node" => { "image" => image_url(second_design, version.sha) } }
-            ]
+        it "only returns the first two designs" do
+          expect(design_nodes).to contain_exactly(
+            a_hash_including("id" => global_id(design)),
+            a_hash_including("id" => global_id(second_design))
+          )
+        end
+
+        it "returns the correct versions of the design images" do
+          expect(design_nodes).to contain_exactly(
+            a_hash_including("image" => image_url(design, version.sha)),
+            a_hash_including("image" => image_url(second_design, version.sha))
+          )
+        end
+
+        it "returns the correct events for the designs in this version" do
+          expect(design_nodes).to contain_exactly(
+            a_hash_including("event" => "NONE"),
+            a_hash_including("event" => "CREATION")
           )
         end
 
         it "returns the correct versions records for both designs" do
+          expect(version_nodes).to eq([
+            [{ "node" => { "id" => global_id(design.versions.first) } }],
+            [{ "node" => { "id" => global_id(second_design.versions.first) } }]
+          ])
+        end
+      end
+
+      context "viewing the last version, when one design was deleted and one was updated" do
+        let(:version) { all_versions.last }
+
+        before do
+          second_design.design_versions.create!(version: version, event: "modification")
+
           post_graphql(query, current_user: current_user)
+        end
 
-          expect(version_nodes).to eq(
+        it "does not include the deleted design" do
+          # The design does exist in the version
+          expect(version.designs).to include(deleted_design)
+
+          # But the GraphQL API does not include it in these results
+          expect(design_nodes).to contain_exactly(
+            a_hash_including("id" => global_id(design)),
+            a_hash_including("id" => global_id(second_design))
+          )
+        end
+
+        it "returns the correct versions of the design images" do
+          expect(design_nodes).to contain_exactly(
+            a_hash_including("image" => image_url(design, version.sha)),
+            a_hash_including("image" => image_url(second_design, version.sha))
+          )
+        end
+
+        it "returns the correct events for the designs in this version" do
+          expect(design_nodes).to contain_exactly(
+            a_hash_including("event" => "NONE"),
+            a_hash_including("event" => "MODIFICATION")
+          )
+        end
+
+        it "returns all versions records for the designs" do
+          expect(version_nodes).to eq([
             [
-              [{ "node" => { "id" => version_global_id(design.versions.first) } }],
-              [{ "node" => { "id" => version_global_id(second_design.versions.first) } }]
+              { "node" => { "id" => global_id(design.versions.first) } }
+            ],
+            [
+              { "node" => { "id" => global_id(second_design.versions.second) } },
+              { "node" => { "id" => global_id(second_design.versions.first) } }
             ]
-          )
+          ])
         end
       end
     end
 
     describe 'a design with note annotations' do
-      let!(:note) { create(:diff_note_on_design, noteable: design, project: design.project) }
+      set(:note) { create(:diff_note_on_design, noteable: design, project: design.project) }
 
       let(:design_query) do
         <<~NODE

spec/lib/gitlab/import_export/all_models.yml

@@ -23,6 +23,7 @@ issues:
 - epic_issue
 - epic
 - designs
+- design_versions
 events:
 - author
 - project