Merge branch '9491-graphql-view-design-board-at-version' into 'master'

Show design boards at previous versions in GraphQL

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

ee/app/finders/design_management/designs_finder.rb

+# frozen_string_literal: true
+
+module DesignManagement
+  class DesignsFinder
+    attr_reader :issue, :current_user, :params
+
+    def initialize(issue, current_user, params = {})
+      @issue = issue
+      @current_user = current_user
+      @params = params
+    end
+
+    def execute
+      unless Ability.allowed?(current_user, :read_design, issue)
+        return ::DesignManagement::Design.none
+      end
+
+      items = issue.designs
+      items = by_visible_at_version(items)
+      items
+    end
+
+    private
+
+    # Returns all designs that existed at a particular design version
+    def by_visible_at_version(items)
+      return items unless params[:visible_at_version]
+
+      items.visible_at_version(params[:visible_at_version])
+    end
+  end
+end

ee/app/finders/design_management/versions_finder.rb

+# frozen_string_literal: true
+
+module DesignManagement
+  class VersionsFinder
+    attr_reader :design_or_collection, :current_user, :params
+
+    # The `design_or_collection` argument should be either a:
+    #
+    # - DesignManagement::Design, or
+    # - DesignManagement::DesignCollection
+    #
+    # The object will have `#versions` called on it to set up the
+    # initial scope of the versions.
+    def initialize(design_or_collection, current_user, params = {})
+      @design_or_collection = design_or_collection
+      @current_user = current_user
+      @params = params
+    end
+
+    def execute
+      unless Ability.allowed?(current_user, :read_design, design_or_collection)
+        return ::DesignManagement::Version.none
+      end
+
+      items = design_or_collection.versions
+      items = by_earlier_or_equal_to(items)
+      items.ordered
+    end
+
+    private
+
+    def by_earlier_or_equal_to(items)
+      return items unless params[:earlier_or_equal_to]
+
+      items.earlier_or_equal_to(params[:earlier_or_equal_to])
+    end
+  end
+end

ee/app/graphql/resolvers/design_management/design_resolver.rb

+# frozen_string_literal: true
+
+module Resolvers
+  module DesignManagement
+    class DesignResolver < BaseResolver
+      argument :at_version,
+               GraphQL::ID_TYPE,
+               required: false,
+               description: 'Filters designs to only those that existed at the version. ' \
+                            'If argument is omitted or nil then all designs will reflect the latest version.'
+
+      def resolve(at_version: nil)
+        version = at_version ? GitlabSchema.object_from_id(at_version) : nil
+
+        ::DesignManagement::DesignsFinder.new(
+          object.issue,
+          context[:current_user],
+          visible_at_version: version
+        ).execute
+      end
+    end
+  end
+end

ee/app/graphql/resolvers/design_management/version_resolver.rb

@@ -7,12 +7,21 @@ module Resolvers
 
       alias_method :design_or_collection, :object
 
-      def resolve(*_args)
-        unless Ability.allowed?(context[:current_user], :read_design, design_or_collection)
-          return ::DesignManagement::Version.none
-        end
+      def resolve(parent: nil)
+        # Find an `at_version` argument passed to a parent node.
+        #
+        # If one is found, then a design collection further up the AST
+        # has been filtered to reflect designs at that version, and so
+        # for consistency we should only present versions up to the given
+        # version here.
+        at_version = Gitlab::Graphql::FindArgumentInParent.find(parent, :at_version, limit_depth: 4)
+        version = at_version ? GitlabSchema.object_from_id(at_version) : nil
 
-        design_or_collection.versions.ordered
+        ::DesignManagement::VersionsFinder.new(
+          design_or_collection,
+          context[:current_user],
+          earlier_or_equal_to: version
+        ).execute
       end
     end
   end

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

@@ -12,6 +12,7 @@ module Types
       field :designs,
             Types::DesignManagement::DesignType.connection_type,
             null: false,
+            resolver: Resolvers::DesignManagement::DesignResolver,
             description: "All visible designs for this collection"
       # TODO: allow getting a single design by filename
       # TODO: when we allow hiding designs, we will also expose a relation

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

@@ -9,17 +9,31 @@ module Types
 
       implements(Types::Notes::NoteableType)
 
+      alias_method :design, :object
+
       field :id, GraphQL::ID_TYPE, null: false
       field :project, Types::ProjectType, null: false
       field :issue, Types::IssueType, null: false
       field :filename, GraphQL::STRING_TYPE, null: false
-      field :image, GraphQL::STRING_TYPE, null: false, resolve: -> (design, _args, _ctx) do
-        Gitlab::Routing.url_helpers.project_design_url(design.project, design)
-      end
+      field :image, GraphQL::STRING_TYPE, null: false, extras: [:parent]
       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).sha : nil
+
+        project = Gitlab::Graphql::Loaders::BatchModelLoader.new(Project, design.project_id).find
+
+        Gitlab::Routing.url_helpers.project_design_url(project, design, sha)
+      end
     end
   end
 end

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

@@ -9,6 +9,7 @@ module Types
 
       authorize :read_design
 
+      field :id, GraphQL::ID_TYPE, null: false
       field :sha, GraphQL::ID_TYPE, null: false
       field :designs,
             Types::DesignManagement::DesignType.connection_type,

ee/app/models/design_management/design.rb

@@ -18,6 +18,14 @@ module DesignManagement
     validates :filename, uniqueness: { scope: :issue_id }
     validate :validate_file_is_image
 
+    scope :visible_at_version, -> (version) do
+      created_before_version = DesignManagement::DesignVersion.select(1)
+                               .where("#{table_name}.id = #{DesignManagement::DesignVersion.table_name}.design_id")
+                               .where("#{DesignManagement::DesignVersion.table_name}.version_id <= ?", version)
+
+      where('EXISTS(?)', created_before_version)
+    end
+
     def new_design?
       versions.none?
     end

ee/app/models/design_management/version.rb

@@ -19,7 +19,7 @@ module DesignManagement
     scope :for_designs, -> (designs) do
       where(id: DesignVersion.where(design_id: designs).select(:version_id)).distinct
     end
-
+    scope :earlier_or_equal_to, -> (version) { where('id <= ?', version) }
     scope :ordered, -> { order(id: :desc) }
 
     def self.create_for_designs(designs, sha)

ee/changelogs/unreleased/9491-graphql-view-design-board-at-version.yml

+---
+title: Show design boards at previous versions
+merge_request: 14292
+author:
+type: added

ee/spec/finders/design_management/designs_finder_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe DesignManagement::DesignsFinder do
+  include DesignManagementTestHelpers
+
+  set(:user) { create(:user) }
+  set(:project) { create(:project, :private) }
+  set(:issue) { create(:issue, project: project) }
+  set(:design1) { create(:design, :with_file, issue: issue, versions_count: 1) }
+  set(:design2) { create(:design, :with_file, issue: issue, versions_count: 1) }
+  let(:params) { {} }
+
+  subject(:designs) { described_class.new(issue, user, params).execute }
+
+  describe '#execute' do
+    context 'when user can not read designs of an issue' do
+      it 'returns no results' do
+        is_expected.to be_empty
+      end
+    end
+
+    context 'when user can read designs of an issue' do
+      before do
+        project.add_developer(user)
+      end
+
+      context 'when design management feature is disabled' do
+        it 'returns no results' do
+          is_expected.to be_empty
+        end
+      end
+
+      context 'when design management feature is enabled' do
+        before do
+          enable_design_management
+        end
+
+        it 'returns the designs' do
+          is_expected.to contain_exactly(design2, design1)
+        end
+
+        describe 'returning designs that existed at a particular given version' do
+          let(:all_versions) { issue.design_collection.versions.ordered }
+          let(:first_version) { all_versions.last }
+          let(:second_version) { all_versions.first }
+
+          context 'when argument is the first version' do
+            let(:params) { { visible_at_version: first_version.id } }
+
+            it { is_expected.to eq([design1]) }
+          end
+
+          context 'when argument is the second version' do
+            let(:params) { { visible_at_version: second_version.id } }
+
+            it { is_expected.to contain_exactly(design2, design1) }
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/finders/design_management/versions_finder_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe DesignManagement::VersionsFinder do
+  include DesignManagementTestHelpers
+
+  set(:user) { create(:user) }
+  set(:project) { create(:project, :private) }
+  set(:issue) { create(:issue, project: project) }
+  set(:design_1) { create(:design, :with_file, issue: issue, versions_count: 1) }
+  set(:design_2) { create(:design, :with_file, issue: issue, versions_count: 1) }
+  let(:version_1) { design_1.versions.first }
+  let(:version_2) { design_2.versions.first }
+  let(:design_or_collection) { issue.design_collection }
+  let(:params) { {} }
+
+  subject(:versions) { described_class.new(design_or_collection, user, params).execute }
+
+  describe '#execute' do
+    shared_examples 'returns no results' do
+      it 'returns no results when passed a DesignCollection' do
+        expect(design_or_collection).is_a?(DesignManagement::DesignCollection)
+        is_expected.to be_empty
+      end
+
+      context 'when passed a Design' do
+        let(:design_or_collection) { design_1 }
+
+        it 'returns no results when passed a Design' do
+          is_expected.to be_empty
+        end
+      end
+    end
+
+    context 'when user cannot read designs of an issue' do
+      include_examples 'returns no results'
+    end
+
+    context 'when user can read designs of an issue' do
+      before do
+        project.add_developer(user)
+      end
+
+      context 'when design management feature is disabled' do
+        include_examples 'returns no results'
+      end
+
+      context 'when design management feature is enabled' do
+        before do
+          enable_design_management
+        end
+
+        describe 'passing a DesignCollection or a Design for the initial scoping' do
+          it 'returns the versions scoped to the DesignCollection' do
+            expect(design_or_collection).is_a?(DesignManagement::DesignCollection)
+            is_expected.to eq(issue.design_collection.versions.ordered)
+          end
+
+          context 'when passed a Design' do
+            let(:design_or_collection) { design_1 }
+
+            it 'returns the versions scoped to the Design' do
+              is_expected.to eq(design_1.versions)
+            end
+          end
+        end
+
+        describe 'returning versions earlier or equal to a version' do
+          context 'when argument is the first version' do
+            let(:params) { { earlier_or_equal_to: version_1 }}
+
+            it { is_expected.to eq([version_1]) }
+          end
+
+          context 'when argument is the second version' do
+            let(:params) { { earlier_or_equal_to: version_2 }}
+
+            it { is_expected.to contain_exactly(version_1, version_2) }
+          end
+        end
+      end
+    end
+  end
+end

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

@@ -5,5 +5,5 @@ require 'spec_helper'
 describe GitlabSchema.types['DesignVersion'] do
   it { expect(described_class).to require_graphql_authorizations(:read_design) }
 
-  it { expect(described_class).to have_graphql_fields(:sha, :designs) }
+  it { expect(described_class).to have_graphql_fields(:id, :sha, :designs) }
 end

ee/spec/models/design_management/design_spec.rb

@@ -29,6 +29,26 @@ describe DesignManagement::Design do
     end
   end
 
+  describe 'scopes' do
+    describe '.visible_at_version' do
+      let!(:design1) { create(:design, :with_file, versions_count: 1) }
+      let!(:design2) { create(:design, :with_file, versions_count: 1) }
+      let(:first_version) { DesignManagement::Version.ordered.last }
+      let(:second_version) { DesignManagement::Version.ordered.first }
+
+      it 'returns just designs that existed at that version' do
+        expect(described_class.visible_at_version(first_version)).to eq([design1])
+        expect(described_class.visible_at_version(second_version)).to contain_exactly(design1, design2)
+      end
+
+      it 'can be passed either a DesignManagement::Version or an ID' do
+        [first_version, first_version.id].each do |arg|
+          expect(described_class.visible_at_version(arg)).to eq([design1])
+        end
+      end
+    end
+  end
+
   describe "#new_design?" do
     set(:versions) { create(:design_version) }
     set(:design) { create(:design, versions: [versions]) }

ee/spec/models/design_management/version_spec.rb

@@ -33,10 +33,11 @@ describe DesignManagement::Version do
   end
 
   describe "scopes" do
+    let(:version_1) { create(:design_version) }
+    let(:version_2) { create(:design_version) }
+
     describe ".for_designs" do
       it "only returns versions related to the specified designs" do
-        version_1 = create(:design_version)
-        version_2 = create(:design_version)
         _other_version = create(:design_version)
         designs = [create(:design, versions: [version_1]),
                    create(:design, versions: [version_2])]
@@ -45,6 +46,19 @@ describe DesignManagement::Version do
           .to contain_exactly(version_1, version_2)
       end
     end
+
+    describe '.earlier_or_equal_to' do
+      it 'only returns versions created earlier or later than the given version' do
+        expect(described_class.earlier_or_equal_to(version_1)).to eq([version_1])
+        expect(described_class.earlier_or_equal_to(version_2)).to contain_exactly(version_1, version_2)
+      end
+
+      it 'can be passed either a DesignManagement::Design or an ID' do
+        [version_1, version_1.id].each do |arg|
+          expect(described_class.earlier_or_equal_to(arg)).to eq([version_1])
+        end
+      end
+    end
   end
 
   describe ".bulk_create" do

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

@@ -6,26 +6,20 @@ describe "Getting designs related to an issue" do
   include GraphqlHelpers
   include DesignManagementTestHelpers
 
-  set(:design) { create(:design) }
-  set(:current_user) { design.project.owner }
-
-  let(:query) do
-    design_node = <<~NODE
+  let(:design) { create(:design, :with_file, versions_count: 1) }
+  let(:current_user) { design.project.owner }
+  let(:design_query) do
+    <<~NODE
     designs {
       edges {
         node {
           filename
-          versions {
-            edges {
-              node {
-                sha
-              }
-            }
-          }
         }
       }
     }
     NODE
+  end
+  let(:query) do
     graphql_query_for(
       "project",
       { "fullPath" => design.project.full_path },
@@ -33,7 +27,7 @@ describe "Getting designs related to an issue" do
         "issue",
         { iid: design.issue.iid },
         query_graphql_field(
-          "designs", {}, design_node
+          "designs", {}, design_query
         )
       )
     )
@@ -76,13 +70,36 @@ describe "Getting designs related to an issue" do
     end
 
     context "with versions" do
-      let(:version) { create(:design_version) }
+      let(:version) { design.versions.take }
+      let(:design_query) do
+        <<~NODE
+        designs {
+          edges {
+            node {
+              filename
+              versions {
+                edges {
+                  node {
+                    id
+                    sha
+                  }
+                }
+              }
+            }
+          }
+        }
+        NODE
+      end
 
-      before do
-        design.versions << version
+      it "includes the version id" do
+        post_graphql(query, current_user: current_user)
+
+        version_id = design_response["versions"]["edges"].first["node"]["id"]
+
+        expect(version_id).to eq(version.to_global_id.to_s)
       end
 
-      it "includes the version" do
+      it "includes the version sha" do
         post_graphql(query, current_user: current_user)
 
         version_sha = design_response["versions"]["edges"].first["node"]["sha"]
@@ -90,5 +107,102 @@ describe "Getting designs related to an issue" do
         expect(version_sha).to eq(version.sha)
       end
     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) }
+      let(:design_query) do
+        <<~NODE
+        designs(atVersion: "#{version.to_global_id}") {
+          edges {
+            node {
+              image
+              versions {
+                edges {
+                  node {
+                    id
+                  }
+                }
+              }
+            }
+          }
+        }
+        NODE
+      end
+      let(:design_response) do
+        design_collection["designs"]["edges"]
+      end
+
+      def image_url(design, sha = nil)
+        Gitlab::Routing.url_helpers.project_design_url(design.project, design, sha)
+      end
+
+      def version_global_id(version)
+        version.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')
+        end
+      end
+
+      # Filters just version nodes from the larger `design_response`
+      def version_nodes
+        design_response.map do |response|
+          response.dig('node', 'versions', 'edges')
+        end
+      end
+
+      context "viewing the original version" do
+        let(:version) { all_versions.last }
+
+        it "only returns the first design, with the correct version of the design image" do
+          post_graphql(query, current_user: current_user)
+
+          expect(design_nodes).to eql(
+            [{ "node" => { "image" => image_url(design, version.sha) } }]
+          )
+        end
+
+        it "only returns one version record for the design (the original version)" do
+          post_graphql(query, current_user: current_user)
+
+          expect(version_nodes).to eq(
+            [
+              [{ "node" => { "id" => version_global_id(version) } }]
+            ]
+          )
+        end
+      end
+
+      context "viewing the newest version" do
+        let(:version) { all_versions.first }
+
+        it "returns both designs, with the correct version of the design images" do
+          post_graphql(query, current_user: current_user)
+
+          expect(design_nodes).to eq(
+            [
+              { "node" => { "image" => image_url(design, version.sha) } },
+              { "node" => { "image" => image_url(second_design, version.sha) } }
+            ]
+          )
+        end
+
+        it "returns the correct versions records for both designs" do
+          post_graphql(query, current_user: current_user)
+
+          expect(version_nodes).to eq(
+            [
+              [{ "node" => { "id" => version_global_id(design.versions.first) } }],
+              [{ "node" => { "id" => version_global_id(second_design.versions.first) } }]
+            ]
+          )
+        end
+      end
+    end
   end
 end

lib/gitlab/graphql/find_argument_in_parent.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Graphql
+    module FindArgumentInParent
+      # Searches up the GraphQL AST and returns the first matching argument
+      # passed to a node
+      def self.find(parent, argument, limit_depth: nil)
+        argument = argument.to_s.camelize(:lower).to_sym
+        depth = 0
+
+        while parent.respond_to?(:parent)
+          args = node_args(parent)
+          return args[argument] if args.key?(argument)
+
+          depth += 1
+          return if limit_depth && depth >= limit_depth
+
+          parent = parent.parent
+        end
+      end
+
+      class << self
+        private
+
+        def node_args(node)
+          node.irep_node.arguments
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/graphql/find_argument_in_parent_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Graphql::FindArgumentInParent do
+  describe '#find' do
+    def build_node(parent = nil, args: {})
+      props = { irep_node: double(arguments: args) }
+      props[:parent] = parent if parent # The root node shouldn't respond to parent
+
+      double(props)
+    end
+
+    let(:parent) do
+      build_node(
+        build_node(
+          build_node(
+            build_node,
+            args: { myArg: 1 }
+          )
+        )
+      )
+    end
+    let(:arg_name) { :my_arg }
+
+    it 'searches parents and returns the argument' do
+      expect(described_class.find(parent, :my_arg)).to eq(1)
+    end
+
+    it 'can find argument when passed in as both Ruby and GraphQL-formatted symbols and strings' do
+      [:my_arg, :myArg, 'my_arg', 'myArg'].each do |arg|
+        expect(described_class.find(parent, arg)).to eq(1)
+      end
+    end
+
+    it 'returns nil if no arguments found in parents' do
+      expect(described_class.find(parent, :bar)).to eq(nil)
+    end
+
+    it 'can limit the depth it searches to' do
+      expect(described_class.find(parent, :my_arg, limit_depth: 1)).to eq(nil)
+    end
+  end
+end