Merge branch '212566-foss-design-management-models-remainder' into 'master'

Move Design Management-related model code to FOSS

See merge request gitlab-org/gitlab!31165

app/models/concerns/noteable.rb

@@ -17,7 +17,7 @@ module Noteable
 
     # `Noteable` class names that support resolvable notes.
     def resolvable_types
-      %w(MergeRequest)
+      %w(MergeRequest DesignManagement::Design)
     end
   end
 
@@ -138,6 +138,8 @@ module Noteable
   end
 
   def note_etag_key
+    return Gitlab::Routing.url_helpers.designs_project_issue_path(project, issue, { vueroute: filename }) if self.is_a?(DesignManagement::Design)
+
     Gitlab::Routing.url_helpers.project_noteable_notes_path(
       project,
       target_type: self.class.name.underscore,

app/models/event.rb

@@ -313,6 +313,10 @@ class Event < ApplicationRecord
     note? && target && target.for_personal_snippet?
   end
 
+  def design_note?
+    note? && note.for_design?
+  end
+
   def note_target
     target.noteable
   end
@@ -380,6 +384,11 @@ class Event < ApplicationRecord
 
   protected
 
+  # rubocop:disable Metrics/CyclomaticComplexity
+  # rubocop:disable Metrics/PerceivedComplexity
+  #
+  # TODO Refactor this method so we no longer need to disable the above cops
+  # https://gitlab.com/gitlab-org/gitlab/-/issues/216879.
   def capability
     @capability ||= begin
                       if push_action? || commit_note?
@@ -396,9 +405,13 @@ class Event < ApplicationRecord
                         :read_milestone
                       elsif wiki_page?
                         :read_wiki
+                      elsif design_note?
+                        :read_design
                       end
                     end
   end
+  # rubocop:enable Metrics/CyclomaticComplexity
+  # rubocop:enable Metrics/PerceivedComplexity
 
   private
 

app/models/project.rb

@@ -1438,6 +1438,7 @@ class Project < ApplicationRecord
   def expire_caches_before_rename(old_path)
     repo = Repository.new(old_path, self, shard: repository_storage)
     wiki = Repository.new("#{old_path}.wiki", self, shard: repository_storage, repo_type: Gitlab::GlRepository::WIKI)
+    design = Repository.new("#{old_path}#{Gitlab::GlRepository::DESIGN.path_suffix}", self, shard: repository_storage, repo_type: Gitlab::GlRepository::DESIGN)
 
     if repo.exists?
       repo.before_delete
@@ -1446,6 +1447,10 @@ class Project < ApplicationRecord
     if wiki.exists?
       wiki.before_delete
     end
+
+    if design.exists?
+      design.before_delete
+    end
   end
 
   # Check if repository already exists on disk

app/models/system_note_metadata.rb

@@ -15,6 +15,7 @@ class SystemNoteMetadata < ApplicationRecord
 
   ICON_TYPES = %w[
     commit description merge confidential visible label assignee cross_reference
+    designs_added designs_modified designs_removed designs_discussion_added
     title time_tracking branch milestone discussion task moved
     opened closed merged duplicate locked unlocked outdated
     tag due_date pinned_embed cherry_pick health_status

app/models/todo.rb

@@ -183,6 +183,10 @@ class Todo < ApplicationRecord
     target_type == "Commit"
   end
 
+  def for_design?
+    target_type == DesignManagement::Design.name
+  end
+
   # override to return commits, which are not active record
   def target
     if for_commit?

ee/app/models/concerns/ee/noteable.rb

@@ -11,10 +11,6 @@ module EE
       def replyable_types
         super + %w[Epic Vulnerability]
       end
-
-      def resolvable_types
-        super + %w(DesignManagement::Design)
-      end
     end
 
     override :note_etag_key
@@ -24,8 +20,6 @@ module EE
         ::Gitlab::Routing.url_helpers.group_epic_notes_path(group, self)
       when Vulnerability
         ::Gitlab::Routing.url_helpers.project_security_vulnerability_notes_path(project, self)
-      when DesignManagement::Design
-        ::Gitlab::Routing.url_helpers.designs_project_issue_path(project, issue, { vueroute: filename })
       else
         super
       end

ee/app/models/ee/event.rb

@@ -23,18 +23,12 @@ module EE
       @capability ||= begin
                         if epic? || epic_note?
                           :read_epic
-                        elsif design_note?
-                          :read_design
                         else
                           super
                         end
                       end
     end
 
-    def design_note?
-      note? && note.for_design?
-    end
-
     def epic_note?
       note? && note_target.is_a?(::Epic)
     end

ee/app/models/ee/project.rb

@@ -656,17 +656,6 @@ module EE
       super.presence || build_feature_usage
     end
 
-    override(:expire_caches_before_rename)
-    def expire_caches_before_rename(old_path)
-      super
-
-      design = ::Repository.new("#{old_path}#{::Gitlab::GlRepository::DESIGN.path_suffix}", self, shard: repository_storage, repo_type: ::Gitlab::GlRepository::DESIGN)
-
-      if design.exists?
-        design.before_delete
-      end
-    end
-
     def package_already_taken?(package_name)
       namespace.root_ancestor.all_projects
         .joins(:packages)

ee/app/models/ee/system_note_metadata.rb

@@ -8,7 +8,6 @@ module EE
       weight approved unapproved relate unrelate published
       epic_issue_added issue_added_to_epic epic_issue_removed issue_removed_from_epic
       epic_issue_moved issue_changed_epic epic_date_changed relate_epic unrelate_epic
-      designs_added designs_modified designs_removed designs_discussion_added
       vulnerability_confirmed vulnerability_dismissed vulnerability_resolved
     ].freeze
 

ee/app/models/ee/todo.rb

@@ -13,9 +13,5 @@ module EE
     def resource_parent
       project || group
     end
-
-    def for_design?
-      target_type == DesignManagement::Design.name
-    end
   end
 end

ee/spec/models/concerns/ee/noteable_spec.rb

@@ -14,10 +14,4 @@ describe EE::Noteable do
       expect(klazz.replyable_types).to include("Vulnerability")
     end
   end
-
-  describe '.resolvable_types' do
-    it 'includes design management' do
-      expect(klazz.resolvable_types).to include('DesignManagement::Design')
-    end
-  end
 end

ee/spec/models/ee/event_spec.rb

@@ -76,41 +76,6 @@ describe Event do
       end
     end
 
-    context 'design event' do
-      include DesignManagementTestHelpers
-
-      before do
-        enable_design_management
-      end
-
-      it_behaves_like 'visible to group members only' do
-        let(:event) { create(:event, :for_design, project: project) }
-      end
-
-      context 'the event refers to a design on a confidential issue' do
-        let(:project) { create(:project, :public) }
-        let(:issue) { create(:issue, :confidential, project: project) }
-        let(:note) { create(:note, :on_design, issue: issue) }
-        let(:event) { create(:event, project: project, target: note) }
-
-        let(:assignees) do
-          create_list(:user, 3).each { |user| issue.assignees << user }
-        end
-
-        it 'visible to group reporters, the issue author, and assignees', :aggregate_failures do
-          expect(event).not_to be_visible_to(non_member)
-          expect(event).not_to be_visible_to(guest)
-
-          expect(event).to be_visible_to(reporter)
-          expect(event).to be_visible_to(member)
-          expect(event).to be_visible_to(admin)
-          expect(event).to be_visible_to(issue.author)
-
-          expect(assignees).to all(have_access_to(event))
-        end
-      end
-    end
-
     context 'epic event' do
       let(:target) { epic }
 

ee/spec/models/ee/system_note_metadata_spec.rb

@@ -4,7 +4,6 @@ require 'spec_helper'
 
 describe EE::SystemNoteMetadata do
   %i[
-    designs_added designs_modified designs_removed
     vulnerability_confirmed vulnerability_dismissed vulnerability_resolved
   ].each do |action|
     context 'when action type is valid' do

ee/spec/models/ee/todo_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe Todo do
-  describe '#for_design?' do
-    it 'returns true when target is a Design' do
-      todo = build(:todo, target: build(:design))
-
-      expect(todo.for_design?).to eq(true)
-    end
-
-    it 'returns false when target is not a Design' do
-      todo = build(:todo)
-
-      expect(todo.for_design?).to eq(false)
-    end
-  end
-end

ee/spec/models/project_spec.rb

@@ -2537,31 +2537,6 @@ describe Project do
     it { expect(subject.license_compliance).to be_instance_of(::SCA::LicenseCompliance) }
   end
 
-  describe '#expire_caches_before_rename' do
-    let(:project) { create(:project, :repository) }
-    let(:repo)    { double(:repo, exists?: true, before_delete: true) }
-    let(:wiki)    { double(:wiki, exists?: true, before_delete: true) }
-    let(:design)  { double(:design, exists?: true) }
-
-    it 'expires the caches of the design repository' do
-      allow(Repository).to receive(:new)
-        .with('foo', project, shard: project.repository_storage)
-        .and_return(repo)
-
-      allow(Repository).to receive(:new)
-        .with('foo.wiki', project, shard: project.repository_storage, repo_type: Gitlab::GlRepository::WIKI)
-        .and_return(wiki)
-
-      allow(Repository).to receive(:new)
-        .with('foo.design', project, shard: project.repository_storage, repo_type: ::Gitlab::GlRepository::DESIGN)
-        .and_return(design)
-
-      expect(design).to receive(:before_delete)
-
-      project.expire_caches_before_rename('foo')
-    end
-  end
-
   describe '#template_source?' do
     let_it_be(:group) { create(:group, :private) }
     let_it_be(:subgroup) { create(:group, :private, parent: group) }

spec/factories/notes.rb

@@ -16,6 +16,7 @@ FactoryBot.define do
     factory :note_on_merge_request,      traits: [:on_merge_request]
     factory :note_on_project_snippet,    traits: [:on_project_snippet]
     factory :note_on_personal_snippet,   traits: [:on_personal_snippet]
+    factory :note_on_design,             traits: [:on_design]
     factory :system_note,                traits: [:system]
 
     factory :discussion_note, class: 'DiscussionNote'

spec/models/concerns/noteable_spec.rb

@@ -241,7 +241,7 @@ describe Noteable do
 
   describe '.resolvable_types' do
     it 'exposes the replyable types' do
-      expect(described_class.resolvable_types).to include('MergeRequest')
+      expect(described_class.resolvable_types).to include('MergeRequest', 'DesignManagement::Design')
     end
   end
 

spec/models/event_spec.rb

@@ -195,11 +195,13 @@ describe Event do
     let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignees: [assignee]) }
     let(:project_snippet) { create(:project_snippet, :public, project: project, author: author) }
     let(:personal_snippet) { create(:personal_snippet, :public, author: author) }
+    let(:design) { create(:design, issue: issue, project: project) }
     let(:note_on_commit) { create(:note_on_commit, project: project) }
     let(:note_on_issue) { create(:note_on_issue, noteable: issue, project: project) }
     let(:note_on_confidential_issue) { create(:note_on_issue, noteable: confidential_issue, project: project) }
     let(:note_on_project_snippet) { create(:note_on_project_snippet, author: author, noteable: project_snippet, project: project) }
     let(:note_on_personal_snippet) { create(:note_on_personal_snippet, author: author, noteable: personal_snippet, project: nil) }
+    let(:note_on_design) { create(:note_on_design, author: author, noteable: design, project: project) }
     let(:milestone_on_project) { create(:milestone, project: project) }
     let(:event) do
       described_class.new(project: project,
@@ -452,6 +454,32 @@ describe Event do
         include_examples 'visible to author', true
       end
     end
+
+    context 'design event' do
+      include DesignManagementTestHelpers
+
+      let(:target) { note_on_design }
+
+      before do
+        enable_design_management
+      end
+
+      include_examples 'visibility examples' do
+        let(:visibility) { visible_to_all }
+      end
+
+      include_examples 'visible to assignee and author', true
+
+      context 'the event refers to a design on a confidential issue' do
+        let(:design) { create(:design, issue: confidential_issue, project: project) }
+
+        include_examples 'visibility examples' do
+          let(:visibility) { visible_to_none_except(:member, :admin) }
+        end
+
+        include_examples 'visible to assignee and author', true
+      end
+    end
   end
 
   describe 'wiki_page predicate scopes' do

spec/models/project_spec.rb

@@ -1780,6 +1780,7 @@ describe Project do
     let(:project) { create(:project, :repository) }
     let(:repo)    { double(:repo, exists?: true) }
     let(:wiki)    { double(:wiki, exists?: true) }
+    let(:design)  { double(:design, exists?: true) }
 
     it 'expires the caches of the repository and wiki' do
       # In EE, there are design repositories as well
@@ -1793,8 +1794,13 @@ describe Project do
         .with('foo.wiki', project, shard: project.repository_storage, repo_type: Gitlab::GlRepository::WIKI)
         .and_return(wiki)
 
+      allow(Repository).to receive(:new)
+        .with('foo.design', project, shard: project.repository_storage, repo_type: Gitlab::GlRepository::DESIGN)
+        .and_return(design)
+
       expect(repo).to receive(:before_delete)
       expect(wiki).to receive(:before_delete)
+      expect(design).to receive(:before_delete)
 
       project.expire_caches_before_rename('foo')
     end

spec/models/todo_spec.rb

@@ -61,11 +61,13 @@ describe Todo do
   describe '#done' do
     it 'changes state to done' do
       todo = create(:todo, state: :pending)
+
       expect { todo.done }.to change(todo, :state).from('pending').to('done')
     end
 
     it 'does not raise error when is already done' do
       todo = create(:todo, state: :done)
+
       expect { todo.done }.not_to raise_error
     end
   end
@@ -73,15 +75,31 @@ describe Todo do
   describe '#for_commit?' do
     it 'returns true when target is a commit' do
       subject.target_type = 'Commit'
+
       expect(subject.for_commit?).to eq true
     end
 
     it 'returns false when target is an issuable' do
       subject.target_type = 'Issue'
+
       expect(subject.for_commit?).to eq false
     end
   end
 
+  describe '#for_design?' do
+    it 'returns true when target is a Design' do
+      subject.target_type = 'DesignManagement::Design'
+
+      expect(subject.for_design?).to eq(true)
+    end
+
+    it 'returns false when target is not a Design' do
+      subject.target_type = 'Issue'
+
+      expect(subject.for_design?).to eq(false)
+    end
+  end
+
   describe '#target' do
     context 'for commits' do
       let(:project) { create(:project, :repository) }
@@ -108,6 +126,7 @@ describe Todo do
     it 'returns the issuable for issuables' do
       subject.target_id = issue.id
       subject.target_type = issue.class.name
+
       expect(subject.target).to eq issue
     end
   end
@@ -126,6 +145,7 @@ describe Todo do
 
     it 'returns full reference for issuables' do
       subject.target = issue
+
       expect(subject.target_reference).to eq issue.to_reference(full: false)
     end
   end