Commit 2a8f6990 authored by Luke Duncalfe's avatar Luke Duncalfe

Move Design Management models to FOSS

This change is part of
https://gitlab.com/gitlab-org/gitlab/-/issues/212566 to move all Design
Management code to FOSS.

This MR moves:

- Models
- Factories
- Design repository-related code

The repository-related code has been included because the models and
model specs rely on design repositories to be working.
parent 9b1270af
...@@ -11,7 +11,7 @@ module DesignManagement ...@@ -11,7 +11,7 @@ module DesignManagement
belongs_to :design, class_name: "DesignManagement::Design", inverse_of: :actions belongs_to :design, class_name: "DesignManagement::Design", inverse_of: :actions
belongs_to :version, class_name: "DesignManagement::Version", inverse_of: :actions belongs_to :version, class_name: "DesignManagement::Version", inverse_of: :actions
enum event: [:creation, :modification, :deletion] enum event: { creation: 0, modification: 1, deletion: 2 }
# we assume sequential ordering. # we assume sequential ordering.
scope :ordered, -> { order(version_id: :asc) } scope :ordered, -> { order(version_id: :asc) }
......
...@@ -18,7 +18,7 @@ module DesignManagement ...@@ -18,7 +18,7 @@ module DesignManagement
# This is a polymorphic association, so we can't count on FK's to delete the # This is a polymorphic association, so we can't count on FK's to delete the
# data # data
has_many :notes, as: :noteable, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent has_many :notes, as: :noteable, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
has_many :user_mentions, class_name: "DesignUserMention", dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent has_many :user_mentions, class_name: 'DesignUserMention', dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
validates :project, :filename, presence: true validates :project, :filename, presence: true
validates :issue, presence: true, unless: :importing? validates :issue, presence: true, unless: :importing?
......
...@@ -11,8 +11,8 @@ module DesignManagement ...@@ -11,8 +11,8 @@ module DesignManagement
attr_reader :version attr_reader :version
attr_reader :design attr_reader :design
validates_presence_of :version validates :version, presence: true
validates_presence_of :design validates :design, presence: true
validate :design_and_version_belong_to_the_same_issue validate :design_and_version_belong_to_the_same_issue
validate :design_and_version_have_issue_id validate :design_and_version_have_issue_id
......
...@@ -13,10 +13,10 @@ module DesignManagement ...@@ -13,10 +13,10 @@ module DesignManagement
GA GA
def initialize(project) def initialize(project)
full_path = project.full_path + EE::Gitlab::GlRepository::DESIGN.path_suffix full_path = project.full_path + Gitlab::GlRepository::DESIGN.path_suffix
disk_path = project.disk_path + EE::Gitlab::GlRepository::DESIGN.path_suffix disk_path = project.disk_path + Gitlab::GlRepository::DESIGN.path_suffix
super(full_path, project, shard: project.repository_storage, disk_path: disk_path, repo_type: EE::Gitlab::GlRepository::DESIGN) super(full_path, project, shard: project.repository_storage, disk_path: disk_path, repo_type: Gitlab::GlRepository::DESIGN)
end end
# Override of a method called on Repository instances but sent via # Override of a method called on Repository instances but sent via
......
...@@ -9,7 +9,7 @@ class DiffNote < Note ...@@ -9,7 +9,7 @@ class DiffNote < Note
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
def self.noteable_types def self.noteable_types
%w(MergeRequest Commit) %w(MergeRequest Commit DesignManagement::Design)
end end
validates :original_position, presence: true validates :original_position, presence: true
...@@ -60,6 +60,8 @@ class DiffNote < Note ...@@ -60,6 +60,8 @@ class DiffNote < Note
# Returns the diff file from `position` # Returns the diff file from `position`
def latest_diff_file def latest_diff_file
strong_memoize(:latest_diff_file) do strong_memoize(:latest_diff_file) do
next if for_design?
position.diff_file(repository) position.diff_file(repository)
end end
end end
...@@ -67,6 +69,8 @@ class DiffNote < Note ...@@ -67,6 +69,8 @@ class DiffNote < Note
# Returns the diff file from `original_position` # Returns the diff file from `original_position`
def diff_file def diff_file
strong_memoize(:diff_file) do strong_memoize(:diff_file) do
next if for_design?
enqueue_diff_file_creation_job if should_create_diff_file? enqueue_diff_file_creation_job if should_create_diff_file?
fetch_diff_file fetch_diff_file
...@@ -145,7 +149,7 @@ class DiffNote < Note ...@@ -145,7 +149,7 @@ class DiffNote < Note
end end
def supported? def supported?
for_commit? || self.noteable.has_complete_diff_refs? for_commit? || for_design? || self.noteable.has_complete_diff_refs?
end end
def set_line_code def set_line_code
...@@ -184,5 +188,3 @@ class DiffNote < Note ...@@ -184,5 +188,3 @@ class DiffNote < Note
noteable.respond_to?(:repository) ? noteable.repository : project.repository noteable.respond_to?(:repository) ? noteable.repository : project.repository
end end
end end
DiffNote.prepend_if_ee('::EE::DiffNote')
...@@ -49,6 +49,12 @@ class Issue < ApplicationRecord ...@@ -49,6 +49,12 @@ class Issue < ApplicationRecord
has_many :zoom_meetings has_many :zoom_meetings
has_many :user_mentions, class_name: "IssueUserMention", dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent has_many :user_mentions, class_name: "IssueUserMention", dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
has_many :sent_notifications, as: :noteable has_many :sent_notifications, as: :noteable
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
has_one :sentry_issue has_one :sentry_issue
has_one :alert_management_alert, class_name: 'AlertManagement::Alert' has_one :alert_management_alert, class_name: 'AlertManagement::Alert'
...@@ -334,6 +340,10 @@ class Issue < ApplicationRecord ...@@ -334,6 +340,10 @@ class Issue < ApplicationRecord
previous_changes['updated_at']&.first || updated_at previous_changes['updated_at']&.first || updated_at
end end
def design_collection
@design_collection ||= ::DesignManagement::DesignCollection.new(self)
end
private private
def ensure_metrics def ensure_metrics
......
...@@ -279,6 +279,10 @@ class Note < ApplicationRecord ...@@ -279,6 +279,10 @@ class Note < ApplicationRecord
!for_personal_snippet? !for_personal_snippet?
end end
def for_design?
noteable_type == DesignManagement::Design.name
end
def for_issuable? def for_issuable?
for_issue? || for_merge_request? for_issue? || for_merge_request?
end end
......
...@@ -215,6 +215,7 @@ class Project < ApplicationRecord ...@@ -215,6 +215,7 @@ class Project < ApplicationRecord
has_many :protected_branches has_many :protected_branches
has_many :protected_tags has_many :protected_tags
has_many :repository_languages, -> { order "share DESC" } has_many :repository_languages, -> { order "share DESC" }
has_many :designs, inverse_of: :project, class_name: 'DesignManagement::Design'
has_many :project_authorizations has_many :project_authorizations
has_many :authorized_users, through: :project_authorizations, source: :user, class_name: 'User' has_many :authorized_users, through: :project_authorizations, source: :user, class_name: 'User'
...@@ -791,6 +792,11 @@ class Project < ApplicationRecord ...@@ -791,6 +792,11 @@ class Project < ApplicationRecord
Feature.enabled?(:jira_issue_import, self, default_enabled: true) Feature.enabled?(:jira_issue_import, self, default_enabled: true)
end end
# LFS and hashed repository storage are required for using Design Management.
def design_management_enabled?
lfs_enabled? && hashed_storage?(:repository)
end
def team def team
@team ||= ProjectTeam.new(self) @team ||= ProjectTeam.new(self)
end end
...@@ -799,6 +805,12 @@ class Project < ApplicationRecord ...@@ -799,6 +805,12 @@ class Project < ApplicationRecord
@repository ||= Repository.new(full_path, self, shard: repository_storage, disk_path: disk_path) @repository ||= Repository.new(full_path, self, shard: repository_storage, disk_path: disk_path)
end end
def design_repository
strong_memoize(:design_repository) do
DesignManagement::Repository.new(self)
end
end
def cleanup def cleanup
@repository = nil @repository = nil
end end
......
# frozen_string_literal: true
module EE
module DiffNote
extend ::Gitlab::Utils::Override
extend ActiveSupport::Concern
class_methods do
def noteable_types
super + %w(DesignManagement::Design)
end
end
override :supported?
def supported?
for_design? || super
end
# diffs are currently not suported for designs
# this needs to be decoupled from the `Project#repository`
override :latest_diff_file
def latest_diff_file
return super unless for_design?
end
override :diff_file
def diff_file
return super unless for_design?
end
end
end
...@@ -32,12 +32,6 @@ module EE ...@@ -32,12 +32,6 @@ module EE
has_one :epic_issue has_one :epic_issue
has_one :epic, through: :epic_issue has_one :epic, through: :epic_issue
belongs_to :promoted_to_epic, class_name: 'Epic' belongs_to :promoted_to_epic, class_name: 'Epic'
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
has_one :status_page_published_incident, class_name: 'StatusPage::PublishedIncident', inverse_of: :issue has_one :status_page_published_incident, class_name: 'StatusPage::PublishedIncident', inverse_of: :issue
...@@ -163,10 +157,6 @@ module EE ...@@ -163,10 +157,6 @@ module EE
@group ||= project.group @group ||= project.group
end end
def design_collection
@design_collection ||= ::DesignManagement::DesignCollection.new(self)
end
def promoted? def promoted?
!!promoted_to_epic_id !!promoted_to_epic_id
end end
......
...@@ -57,10 +57,6 @@ module EE ...@@ -57,10 +57,6 @@ module EE
for_epic? || super for_epic? || super
end end
def for_design?
noteable_type == DesignManagement::Design.name
end
override :resource_parent override :resource_parent
def resource_parent def resource_parent
for_epic? ? noteable.group : super for_epic? ? noteable.group : super
......
...@@ -57,7 +57,6 @@ module EE ...@@ -57,7 +57,6 @@ module EE
has_many :approval_rules, class_name: 'ApprovalProjectRule' has_many :approval_rules, class_name: 'ApprovalProjectRule'
has_many :approval_merge_request_rules, through: :merge_requests, source: :approval_rules has_many :approval_merge_request_rules, through: :merge_requests, source: :approval_rules
has_many :audit_events, as: :entity has_many :audit_events, as: :entity
has_many :designs, inverse_of: :project, class_name: 'DesignManagement::Design'
has_many :path_locks has_many :path_locks
has_many :requirements, inverse_of: :project, class_name: 'RequirementsManagement::Requirement' has_many :requirements, inverse_of: :project, class_name: 'RequirementsManagement::Requirement'
...@@ -656,22 +655,11 @@ module EE ...@@ -656,22 +655,11 @@ module EE
super.presence || build_feature_usage super.presence || build_feature_usage
end end
# LFS and hashed repository storage are required for using Design Management.
def design_management_enabled?
lfs_enabled? && hashed_storage?(:repository)
end
def design_repository
strong_memoize(:design_repository) do
DesignManagement::Repository.new(self)
end
end
override(:expire_caches_before_rename) override(:expire_caches_before_rename)
def expire_caches_before_rename(old_path) def expire_caches_before_rename(old_path)
super super
design = ::Repository.new("#{old_path}#{::EE::Gitlab::GlRepository::DESIGN.path_suffix}", self, shard: repository_storage, repo_type: ::EE::Gitlab::GlRepository::DESIGN) design = ::Repository.new("#{old_path}#{::Gitlab::GlRepository::DESIGN.path_suffix}", self, shard: repository_storage, repo_type: ::Gitlab::GlRepository::DESIGN)
if design.exists? if design.exists?
design.before_delete design.before_delete
......
...@@ -42,7 +42,7 @@ module EE ...@@ -42,7 +42,7 @@ module EE
end end
def design_path_suffix def design_path_suffix
@design_path_suffix ||= EE::Gitlab::GlRepository::DESIGN.path_suffix @design_path_suffix ||= ::Gitlab::GlRepository::DESIGN.path_suffix
end end
def old_design_disk_path def old_design_disk_path
......
...@@ -52,11 +52,11 @@ module EE ...@@ -52,11 +52,11 @@ module EE
end end
def old_design_repo_path def old_design_repo_path
"#{old_path}#{EE::Gitlab::GlRepository::DESIGN.path_suffix}" "#{old_path}#{::Gitlab::GlRepository::DESIGN.path_suffix}"
end end
def new_design_repo_path def new_design_repo_path
"#{new_path}#{EE::Gitlab::GlRepository::DESIGN.path_suffix}" "#{new_path}#{::Gitlab::GlRepository::DESIGN.path_suffix}"
end end
end end
end end
......
...@@ -10,7 +10,7 @@ module EE ...@@ -10,7 +10,7 @@ module EE
super super
if project.design_repository.exists? if project.design_repository.exists?
mirror_repository(new_repository_storage_key, type: Gitlab::GlRepository::DESIGN) mirror_repository(new_repository_storage_key, type: ::Gitlab::GlRepository::DESIGN)
end end
end end
......
...@@ -81,7 +81,7 @@ module Geo ...@@ -81,7 +81,7 @@ module Geo
end end
def design_path_suffix def design_path_suffix
EE::Gitlab::GlRepository::DESIGN.path_suffix ::Gitlab::GlRepository::DESIGN.path_suffix
end end
def old_design_disk_path def old_design_disk_path
......
# frozen_string_literal: true
module EE
module Gitlab
module GitAccessDesign
extend ::Gitlab::Utils::Override
private
override :check_protocol!
def check_protocol!
return if geo?
super
end
override :check_can_create_design!
def check_can_create_design!
return if geo?
super
end
end
end
end
# frozen_string_literal: true
module EE
module Gitlab
module GlRepository
extend ::Gitlab::Utils::Override
extend ActiveSupport::Concern
DESIGN = ::Gitlab::GlRepository::RepoType.new(
name: :design,
access_checker_class: ::Gitlab::GitAccessDesign,
repository_resolver: -> (project) { ::DesignManagement::Repository.new(project) },
suffix: :design
)
EE_TYPES = {
DESIGN.name.to_s => DESIGN
}.freeze
override :types
def types
super.merge(EE_TYPES)
end
end
end
end
# frozen_string_literal: true
module EE
module Gitlab
module GlRepository
module RepoType
def design?
self == DESIGN
end
end
end
end
end
...@@ -11,20 +11,6 @@ FactoryBot.modify do ...@@ -11,20 +11,6 @@ FactoryBot.modify do
noteable { association(:vulnerability, project: project) } noteable { association(:vulnerability, project: project) }
end end
trait :on_design do
transient do
issue { association(:issue, project: project) }
end
noteable { association(:design, :with_file, issue: issue) }
after(:build) do |note|
next if note.project == note.noteable.project
# note validations require consistency between these two objects
note.project = note.noteable.project
end
end
trait :with_review do trait :with_review do
review review
end end
...@@ -36,8 +22,4 @@ FactoryBot.define do ...@@ -36,8 +22,4 @@ FactoryBot.define do
factory :note_on_vulnerability, parent: :note, traits: [:on_vulnerability] factory :note_on_vulnerability, parent: :note, traits: [:on_vulnerability]
factory :discussion_note_on_vulnerability, parent: :note, traits: [:on_vulnerability], class: 'DiscussionNote' factory :discussion_note_on_vulnerability, parent: :note, traits: [:on_vulnerability], class: 'DiscussionNote'
factory :diff_note_on_design, parent: :note, traits: [:on_design], class: 'DiffNote' do
position { build(:image_diff_position, file: noteable.full_path, diff_refs: noteable.diff_refs) }
end
end end
...@@ -30,12 +30,6 @@ FactoryBot.modify do ...@@ -30,12 +30,6 @@ FactoryBot.modify do
end end
end end
trait :design_repo do
after(:create) do |project|
raise 'Failed to create design repository!' unless project.design_repository.create_if_not_exists
end
end
trait :import_none do trait :import_none do
import_status { :none } import_status { :none }
end end
......
# frozen_string_literal: true
FactoryBot.modify do
factory :upload do
trait :design_action_image_v432x230_upload do
mount_point { :image_v432x230 }
model { create(:design_action) }
uploader { ::DesignManagement::DesignV432x230Uploader.name }
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::GitAccessDesign do
include DesignManagementTestHelpers
using RSpec::Parameterized::TableSyntax
let_it_be(:project) { create(:project) }
let_it_be(:user) { project.owner }
let(:actor) { :geo }
subject(:access) do
described_class.new(actor, project, protocol, authentication_abilities: [:read_project, :download_code, :push_code])
end
describe '#check' do
subject { access.check('git-receive-pack', ::Gitlab::GitAccess::ANY) }
before do
enable_design_management
end
where(:protocol_name) do
%w(ssh web http https)
end
with_them do
let(:protocol) { protocol_name }
it { is_expected.to be_a(::Gitlab::GitAccessResult::Success) }
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::GlRepository do
describe "DESIGN" do
it "uses the design access checker" do
expect(described_class::DESIGN.access_checker_class).to eq(::Gitlab::GitAccessDesign)
end
it "builds a design repository" do
expect(described_class::DESIGN.repository_resolver.call(create(:project)))
.to be_a(::DesignManagement::Repository)
end
end
describe '.parse' do
let_it_be(:project) { create(:project, :repository) }
it 'parses a design gl_repository' do
expect(described_class.parse("design-#{project.id}")).to eq([project, project, EE::Gitlab::GlRepository::DESIGN])
end
end
describe '.types' do
it 'contains both the EE and CE repository types' do
expected_types = Gitlab::GlRepository::TYPES.merge(EE::Gitlab::GlRepository::EE_TYPES)
expect(described_class.types).to eq(expected_types)
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe DiffNote do
describe 'Validations' do
it 'allows diffnotes on designs' do
diff_note = build(:diff_note_on_design)
expect(diff_note).to be_valid
end
end
context 'diff files' do
let(:design) { create(:design, :with_file, versions_count: 2) }
let(:diff_note) { create(:diff_note_on_design, noteable: design) }
describe '#latest_diff_file' do
it 'does not return a diff file' do
expect(diff_note.latest_diff_file).to be_nil
end
end
describe '#diff_file' do
it 'does not return a diff file' do
expect(diff_note.diff_file).to be_nil
end
end
end
end
...@@ -169,8 +169,6 @@ describe Issue do ...@@ -169,8 +169,6 @@ 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) }
it { is_expected.to have_and_belong_to_many(:prometheus_alert_events) } it { is_expected.to have_and_belong_to_many(:prometheus_alert_events) }
it { is_expected.to have_and_belong_to_many(:self_managed_prometheus_alert_events) } it { is_expected.to have_and_belong_to_many(:self_managed_prometheus_alert_events) }
it { is_expected.to have_many(:prometheus_alerts) } it { is_expected.to have_many(:prometheus_alerts) }
...@@ -593,50 +591,6 @@ describe Issue do ...@@ -593,50 +591,6 @@ describe Issue do
end end
end end
describe "#design_collection" do
it "returns a design collection" do
issue = build(:issue)
collection = issue.design_collection
expect(collection).to be_a(DesignManagement::DesignCollection)
expect(collection.issue).to eq(issue)
end
end
describe 'current designs' do
let(:issue) { create(:issue) }
subject { issue.designs.current }
context 'an issue has no designs' do
it { is_expected.to be_empty }
end
context 'an issue only has current designs' do
let!(:design_a) { create(:design, :with_file, issue: issue) }
let!(:design_b) { create(:design, :with_file, issue: issue) }
let!(:design_c) { create(:design, :with_file, issue: issue) }
it { is_expected.to include(design_a, design_b, design_c) }
end
context 'an issue only has deleted designs' do
let!(:design_a) { create(:design, :with_file, issue: issue, deleted: true) }
let!(:design_b) { create(:design, :with_file, issue: issue, deleted: true) }
let!(:design_c) { create(:design, :with_file, issue: issue, deleted: true) }
it { is_expected.to be_empty }
end
context 'an issue has a mixture of current and deleted designs' do
let!(:design_a) { create(:design, :with_file, issue: issue) }
let!(:design_b) { create(:design, :with_file, issue: issue, deleted: true) }
let!(:design_c) { create(:design, :with_file, issue: issue) }
it { is_expected.to contain_exactly(design_a, design_c) }
end
end
describe "#issue_link_type" do describe "#issue_link_type" do
let(:issue) { build(:issue) } let(:issue) { build(:issue) }
......
...@@ -171,14 +171,6 @@ describe Note do ...@@ -171,14 +171,6 @@ describe Note do
end end
end end
describe '#for_design' do
it 'is true when the noteable is a design' do
note = build(:note, noteable: build(:design))
expect(note).to be_for_design
end
end
describe '.by_humans' do describe '.by_humans' do
it 'excludes notes by bots and service users' do it 'excludes notes by bots and service users' do
user_note = create(:note) user_note = create(:note)
......
...@@ -2076,28 +2076,6 @@ describe Project do ...@@ -2076,28 +2076,6 @@ describe Project do
end end
end end
describe '#design_management_enabled?' do
let(:project) { build(:project) }
where(:lfs_enabled, :hashed_storage_enabled, :expectation) do
false | false | false
true | false | false
false | true | false
true | true | true
end
with_them do
before do
expect(project).to receive(:lfs_enabled?).and_return(lfs_enabled)
allow(project).to receive(:hashed_storage?).with(:repository).and_return(hashed_storage_enabled)
end
it do
expect(project.design_management_enabled?).to be(expectation)
end
end
end
describe "#kerberos_url_to_repo" do describe "#kerberos_url_to_repo" do
let(:project) { create(:project, path: "somewhere") } let(:project) { create(:project, path: "somewhere") }
...@@ -2562,7 +2540,7 @@ describe Project do ...@@ -2562,7 +2540,7 @@ describe Project do
.and_return(wiki) .and_return(wiki)
allow(Repository).to receive(:new) allow(Repository).to receive(:new)
.with('foo.design', project, shard: project.repository_storage, repo_type: ::EE::Gitlab::GlRepository::DESIGN) .with('foo.design', project, shard: project.repository_storage, repo_type: ::Gitlab::GlRepository::DESIGN)
.and_return(design) .and_return(design)
expect(design).to receive(:before_delete) expect(design).to receive(:before_delete)
......
...@@ -78,7 +78,7 @@ describe API::Internal::Base do ...@@ -78,7 +78,7 @@ describe API::Internal::Base do
context "for design repositories" do context "for design repositories" do
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project) }
let(:gl_repository) { EE::Gitlab::GlRepository::DESIGN.identifier_for_container(project) } let(:gl_repository) { ::Gitlab::GlRepository::DESIGN.identifier_for_container(project) }
it "does not allow access" do it "does not allow access" do
post(api("/internal/allowed"), post(api("/internal/allowed"),
......
...@@ -102,7 +102,7 @@ describe DesignManagement::DeleteDesignsService do ...@@ -102,7 +102,7 @@ describe DesignManagement::DeleteDesignsService do
end end
it 'calls repository#log_geo_updated_event' do it 'calls repository#log_geo_updated_event' do
design_repository = EE::Gitlab::GlRepository::DESIGN.repository_resolver.call(project) design_repository = ::Gitlab::GlRepository::DESIGN.repository_resolver.call(project)
allow_next_instance_of(described_class) do |instance| allow_next_instance_of(described_class) do |instance|
allow(instance).to receive(:repository).and_return(design_repository) allow(instance).to receive(:repository).and_return(design_repository)
end end
......
...@@ -10,7 +10,7 @@ describe DesignManagement::SaveDesignsService do ...@@ -10,7 +10,7 @@ describe DesignManagement::SaveDesignsService do
let(:issue) { create(:issue) } let(:issue) { create(:issue) }
let(:user) { developer } let(:user) { developer }
let(:files) { [rails_sample] } let(:files) { [rails_sample] }
let(:design_repository) { EE::Gitlab::GlRepository::DESIGN.repository_resolver.call(project) } let(:design_repository) { ::Gitlab::GlRepository::DESIGN.repository_resolver.call(project) }
let(:rails_sample_name) { 'rails_sample.jpg' } let(:rails_sample_name) { 'rails_sample.jpg' }
let(:rails_sample) { sample_image(rails_sample_name) } let(:rails_sample) { sample_image(rails_sample_name) }
let(:dk_png) { sample_image('dk.png') } let(:dk_png) { sample_image('dk.png') }
......
...@@ -98,7 +98,7 @@ describe Projects::TransferService do ...@@ -98,7 +98,7 @@ describe Projects::TransferService do
end end
describe 'when the project has a design repository' do describe 'when the project has a design repository' do
let(:project_repo_path) { "#{project.path}#{EE::Gitlab::GlRepository::DESIGN.path_suffix}" } let(:project_repo_path) { "#{project.path}#{::Gitlab::GlRepository::DESIGN.path_suffix}" }
let(:old_full_path) { "#{user.namespace.full_path}/#{project_repo_path}" } let(:old_full_path) { "#{user.namespace.full_path}/#{project_repo_path}" }
let(:new_full_path) { "#{group.full_path}/#{project_repo_path}" } let(:new_full_path) { "#{group.full_path}/#{project_repo_path}" }
......
...@@ -2,11 +2,9 @@ ...@@ -2,11 +2,9 @@
module Gitlab module Gitlab
class GitAccessDesign < GitAccess class GitAccessDesign < GitAccess
def check(cmd, _changes) def check(_cmd, _changes)
unless geo?
check_protocol! check_protocol!
check_can_create_design! check_can_create_design!
end
success_result success_result
end end
...@@ -26,3 +24,5 @@ module Gitlab ...@@ -26,3 +24,5 @@ module Gitlab
end end
end end
end end
Gitlab::GitAccessDesign.prepend_if_ee('EE::Gitlab::GitAccessDesign')
...@@ -23,11 +23,18 @@ module Gitlab ...@@ -23,11 +23,18 @@ module Gitlab
project_resolver: -> (snippet) { snippet&.project }, project_resolver: -> (snippet) { snippet&.project },
guest_read_ability: :read_snippet guest_read_ability: :read_snippet
).freeze ).freeze
DESIGN = ::Gitlab::GlRepository::RepoType.new(
name: :design,
access_checker_class: ::Gitlab::GitAccessDesign,
repository_resolver: -> (project) { ::DesignManagement::Repository.new(project) },
suffix: :design
).freeze
TYPES = { TYPES = {
PROJECT.name.to_s => PROJECT, PROJECT.name.to_s => PROJECT,
WIKI.name.to_s => WIKI, WIKI.name.to_s => WIKI,
SNIPPET.name.to_s => SNIPPET SNIPPET.name.to_s => SNIPPET,
DESIGN.name.to_s => DESIGN
}.freeze }.freeze
def self.types def self.types
...@@ -58,5 +65,3 @@ module Gitlab ...@@ -58,5 +65,3 @@ module Gitlab
private_class_method :instance private_class_method :instance
end end
end end
Gitlab::GlRepository.prepend_if_ee('::EE::Gitlab::GlRepository')
...@@ -57,6 +57,10 @@ module Gitlab ...@@ -57,6 +57,10 @@ module Gitlab
self == SNIPPET self == SNIPPET
end end
def design?
self == DESIGN
end
def path_suffix def path_suffix
suffix ? ".#{suffix}" : '' suffix ? ".#{suffix}" : ''
end end
...@@ -87,5 +91,3 @@ module Gitlab ...@@ -87,5 +91,3 @@ module Gitlab
end end
end end
end end
Gitlab::GlRepository::RepoType.prepend_if_ee('EE::Gitlab::GlRepository::RepoType')
...@@ -107,6 +107,10 @@ FactoryBot.define do ...@@ -107,6 +107,10 @@ FactoryBot.define do
end end
end end
factory :diff_note_on_design, parent: :note, traits: [:on_design], class: 'DiffNote' do
position { build(:image_diff_position, file: noteable.full_path, diff_refs: noteable.diff_refs) }
end
trait :on_commit do trait :on_commit do
association :project, :repository association :project, :repository
noteable { nil } noteable { nil }
...@@ -136,6 +140,20 @@ FactoryBot.define do ...@@ -136,6 +140,20 @@ FactoryBot.define do
project { nil } project { nil }
end end
trait :on_design do
transient do
issue { association(:issue, project: project) }
end
noteable { association(:design, :with_file, issue: issue) }
after(:build) do |note|
next if note.project == note.noteable.project
# note validations require consistency between these two objects
note.project = note.noteable.project
end
end
trait :system do trait :system do
system { true } system { true }
end end
......
...@@ -215,6 +215,12 @@ FactoryBot.define do ...@@ -215,6 +215,12 @@ FactoryBot.define do
end end
end end
trait :design_repo do
after(:create) do |project|
raise 'Failed to create design repository!' unless project.design_repository.create_if_not_exists
end
end
trait :remote_mirror do trait :remote_mirror do
transient do transient do
remote_name { "remote_mirror_#{SecureRandom.hex}" } remote_name { "remote_mirror_#{SecureRandom.hex}" }
......
...@@ -65,5 +65,11 @@ FactoryBot.define do ...@@ -65,5 +65,11 @@ FactoryBot.define do
model { create(:note) } model { create(:note) }
uploader { "AttachmentUploader" } uploader { "AttachmentUploader" }
end end
trait :design_action_image_v432x230_upload do
mount_point { :image_v432x230 }
model { create(:design_action) }
uploader { ::DesignManagement::DesignV432x230Uploader.name }
end
end end
end end
...@@ -13,41 +13,42 @@ describe Gitlab::GitAccessDesign do ...@@ -13,41 +13,42 @@ describe Gitlab::GitAccessDesign do
described_class.new(actor, project, protocol, authentication_abilities: [:read_project, :download_code, :push_code]) described_class.new(actor, project, protocol, authentication_abilities: [:read_project, :download_code, :push_code])
end end
describe "#check!" do describe '#check' do
subject { access.check('git-receive-pack', ::Gitlab::GitAccess::ANY) } subject { access.check('git-receive-pack', ::Gitlab::GitAccess::ANY) }
before do before do
enable_design_management enable_design_management
end end
context "when the user is allowed to manage designs" do context 'when the user is allowed to manage designs' do
it { is_expected.to be_a(::Gitlab::GitAccessResult::Success) } # TODO This test is being temporarily skipped unless run in EE,
# as we are in the process of moving Design Management to FOSS in 13.0
# in steps. In the current step the policies have not yet been moved
# which means that although the `GitAccessDesign` class has moved, the
# user will always be denied access in FOSS.
#
# See https://gitlab.com/gitlab-org/gitlab/-/issues/212566#note_327724283.
it do
skip 'See https://gitlab.com/gitlab-org/gitlab/-/issues/212566#note_327724283' unless Gitlab.ee?
is_expected.to be_a(::Gitlab::GitAccessResult::Success)
end
end end
context "when the user is not allowed to manage designs" do context 'when the user is not allowed to manage designs' do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
it "raises an error " do it 'raises an error' do
expect { subject }.to raise_error(::Gitlab::GitAccess::ForbiddenError) expect { subject }.to raise_error(::Gitlab::GitAccess::ForbiddenError)
end end
end end
context "when the protocol is not web" do context 'when the protocol is not web' do
let(:protocol) { 'https' } let(:protocol) { 'https' }
it "raises an error " do it 'raises an error' do
expect { subject }.to raise_error(::Gitlab::GitAccess::ForbiddenError) expect { subject }.to raise_error(::Gitlab::GitAccess::ForbiddenError)
end end
end end
context 'Geo' do
let(:actor) { :geo }
context 'http protocol' do
let(:protocol) { 'http' }
it { is_expected.to be_a(::Gitlab::GitAccessResult::Success) }
end
end
end end
end end
...@@ -7,6 +7,7 @@ describe Gitlab::GlRepository::RepoType do ...@@ -7,6 +7,7 @@ describe Gitlab::GlRepository::RepoType do
let_it_be(:project_snippet) { create(:project_snippet, project: project, author: project.owner) } let_it_be(:project_snippet) { create(:project_snippet, project: project, author: project.owner) }
let(:project_path) { project.repository.full_path } let(:project_path) { project.repository.full_path }
let(:wiki_path) { project.wiki.repository.full_path } let(:wiki_path) { project.wiki.repository.full_path }
let(:design_path) { project.design_repository.full_path }
let(:personal_snippet_path) { "snippets/#{personal_snippet.id}" } let(:personal_snippet_path) { "snippets/#{personal_snippet.id}" }
let(:project_snippet_path) { "#{project.full_path}/snippets/#{project_snippet.id}" } let(:project_snippet_path) { "#{project.full_path}/snippets/#{project_snippet.id}" }
...@@ -24,6 +25,7 @@ describe Gitlab::GlRepository::RepoType do ...@@ -24,6 +25,7 @@ describe Gitlab::GlRepository::RepoType do
expect(described_class).not_to be_wiki expect(described_class).not_to be_wiki
expect(described_class).to be_project expect(described_class).to be_project
expect(described_class).not_to be_snippet expect(described_class).not_to be_snippet
expect(described_class).not_to be_design
end end
end end
...@@ -33,6 +35,7 @@ describe Gitlab::GlRepository::RepoType do ...@@ -33,6 +35,7 @@ describe Gitlab::GlRepository::RepoType do
expect(described_class.valid?(wiki_path)).to be_truthy expect(described_class.valid?(wiki_path)).to be_truthy
expect(described_class.valid?(personal_snippet_path)).to be_truthy expect(described_class.valid?(personal_snippet_path)).to be_truthy
expect(described_class.valid?(project_snippet_path)).to be_truthy expect(described_class.valid?(project_snippet_path)).to be_truthy
expect(described_class.valid?(design_path)).to be_truthy
end end
end end
end end
...@@ -51,6 +54,7 @@ describe Gitlab::GlRepository::RepoType do ...@@ -51,6 +54,7 @@ describe Gitlab::GlRepository::RepoType do
expect(described_class).to be_wiki expect(described_class).to be_wiki
expect(described_class).not_to be_project expect(described_class).not_to be_project
expect(described_class).not_to be_snippet expect(described_class).not_to be_snippet
expect(described_class).not_to be_design
end end
end end
...@@ -60,6 +64,7 @@ describe Gitlab::GlRepository::RepoType do ...@@ -60,6 +64,7 @@ describe Gitlab::GlRepository::RepoType do
expect(described_class.valid?(wiki_path)).to be_truthy expect(described_class.valid?(wiki_path)).to be_truthy
expect(described_class.valid?(personal_snippet_path)).to be_falsey expect(described_class.valid?(personal_snippet_path)).to be_falsey
expect(described_class.valid?(project_snippet_path)).to be_falsey expect(described_class.valid?(project_snippet_path)).to be_falsey
expect(described_class.valid?(design_path)).to be_falsey
end end
end end
end end
...@@ -79,6 +84,7 @@ describe Gitlab::GlRepository::RepoType do ...@@ -79,6 +84,7 @@ describe Gitlab::GlRepository::RepoType do
expect(described_class).to be_snippet expect(described_class).to be_snippet
expect(described_class).not_to be_wiki expect(described_class).not_to be_wiki
expect(described_class).not_to be_project expect(described_class).not_to be_project
expect(described_class).not_to be_design
end end
end end
...@@ -88,6 +94,7 @@ describe Gitlab::GlRepository::RepoType do ...@@ -88,6 +94,7 @@ describe Gitlab::GlRepository::RepoType do
expect(described_class.valid?(wiki_path)).to be_falsey expect(described_class.valid?(wiki_path)).to be_falsey
expect(described_class.valid?(personal_snippet_path)).to be_truthy expect(described_class.valid?(personal_snippet_path)).to be_truthy
expect(described_class.valid?(project_snippet_path)).to be_truthy expect(described_class.valid?(project_snippet_path)).to be_truthy
expect(described_class.valid?(design_path)).to be_falsey
end end
end end
end end
...@@ -115,8 +122,38 @@ describe Gitlab::GlRepository::RepoType do ...@@ -115,8 +122,38 @@ describe Gitlab::GlRepository::RepoType do
expect(described_class.valid?(wiki_path)).to be_falsey expect(described_class.valid?(wiki_path)).to be_falsey
expect(described_class.valid?(personal_snippet_path)).to be_truthy expect(described_class.valid?(personal_snippet_path)).to be_truthy
expect(described_class.valid?(project_snippet_path)).to be_truthy expect(described_class.valid?(project_snippet_path)).to be_truthy
expect(described_class.valid?(design_path)).to be_falsey
end end
end end
end end
end end
describe Gitlab::GlRepository::DESIGN do
it_behaves_like 'a repo type' do
let(:expected_identifier) { "design-#{project.id}" }
let(:expected_id) { project.id.to_s }
let(:expected_suffix) { '.design' }
let(:expected_repository) { project.design_repository }
let(:expected_container) { project }
end
it 'knows its type' do
aggregate_failures do
expect(described_class).to be_design
expect(described_class).not_to be_project
expect(described_class).not_to be_wiki
expect(described_class).not_to be_snippet
end
end
it 'checks if repository path is valid' do
aggregate_failures do
expect(described_class.valid?(design_path)).to be_truthy
expect(described_class.valid?(project_path)).to be_falsey
expect(described_class.valid?(wiki_path)).to be_falsey
expect(described_class.valid?(personal_snippet_path)).to be_falsey
expect(described_class.valid?(project_snippet_path)).to be_falsey
end
end
end
end end
...@@ -19,6 +19,10 @@ describe ::Gitlab::GlRepository do ...@@ -19,6 +19,10 @@ describe ::Gitlab::GlRepository do
expect(described_class.parse("snippet-#{snippet.id}")).to eq([snippet, nil, Gitlab::GlRepository::SNIPPET]) expect(described_class.parse("snippet-#{snippet.id}")).to eq([snippet, nil, Gitlab::GlRepository::SNIPPET])
end end
it 'parses a design gl_repository' do
expect(described_class.parse("design-#{project.id}")).to eq([project, project, Gitlab::GlRepository::DESIGN])
end
it 'throws an argument error on an invalid gl_repository type' do it 'throws an argument error on an invalid gl_repository type' do
expect { described_class.parse("badformat-#{project.id}") }.to raise_error(ArgumentError) expect { described_class.parse("badformat-#{project.id}") }.to raise_error(ArgumentError)
end end
...@@ -27,4 +31,15 @@ describe ::Gitlab::GlRepository do ...@@ -27,4 +31,15 @@ describe ::Gitlab::GlRepository do
expect { described_class.parse("project-foo") }.to raise_error(ArgumentError) expect { described_class.parse("project-foo") }.to raise_error(ArgumentError)
end end
end end
describe 'DESIGN' do
it 'uses the design access checker' do
expect(described_class::DESIGN.access_checker_class).to eq(::Gitlab::GitAccessDesign)
end
it 'builds a design repository' do
expect(described_class::DESIGN.repository_resolver.call(create(:project)))
.to be_a(::DesignManagement::Repository)
end
end
end end
...@@ -380,8 +380,15 @@ describe DesignManagement::Design do ...@@ -380,8 +380,15 @@ describe DesignManagement::Design do
end end
end end
# TODO these tests are being temporarily skipped unless run in EE,
# as we are in the process of moving Design Management to FOSS in 13.0
# in steps. In the current step the routes have not yet been moved.
#
# See https://gitlab.com/gitlab-org/gitlab/-/issues/212566#note_327724283.
describe '#note_etag_key' do describe '#note_etag_key' do
it 'returns a correct etag key' do it 'returns a correct etag key' do
skip 'See https://gitlab.com/gitlab-org/gitlab/-/issues/212566#note_327724283' unless Gitlab.ee?
design = create(:design) design = create(:design)
expect(design.note_etag_key).to eq( expect(design.note_etag_key).to eq(
......
...@@ -287,6 +287,24 @@ describe DiffNote do ...@@ -287,6 +287,24 @@ describe DiffNote do
reply_diff_note.reload.diff_file reply_diff_note.reload.diff_file
end end
end end
context 'when noteable is a Design' do
it 'does not return a diff file' do
diff_note = create(:diff_note_on_design)
expect(diff_note.diff_file).to be_nil
end
end
end
describe '#latest_diff_file' do
context 'when noteable is a Design' do
it 'does not return a diff file' do
diff_note = create(:diff_note_on_design)
expect(diff_note.latest_diff_file).to be_nil
end
end
end end
describe "#diff_line" do describe "#diff_line" do
......
...@@ -15,8 +15,20 @@ describe Issue do ...@@ -15,8 +15,20 @@ describe Issue do
it { is_expected.to belong_to(:closed_by).class_name('User') } it { is_expected.to belong_to(:closed_by).class_name('User') }
it { is_expected.to have_many(:assignees) } it { is_expected.to have_many(:assignees) }
it { is_expected.to have_many(:user_mentions).class_name("IssueUserMention") } it { is_expected.to have_many(:user_mentions).class_name("IssueUserMention") }
it { is_expected.to have_many(:designs) }
it { is_expected.to have_many(:design_versions) }
it { is_expected.to have_one(:sentry_issue) } it { is_expected.to have_one(:sentry_issue) }
it { is_expected.to have_one(:alert_management_alert) } it { is_expected.to have_one(:alert_management_alert) }
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 end
describe 'modules' do describe 'modules' do
...@@ -970,4 +982,48 @@ describe Issue do ...@@ -970,4 +982,48 @@ describe Issue do
expect(issue.previous_updated_at).to eq(Time.new(2013, 02, 06)) expect(issue.previous_updated_at).to eq(Time.new(2013, 02, 06))
end end
end end
describe '#design_collection' do
it 'returns a design collection' do
issue = build(:issue)
collection = issue.design_collection
expect(collection).to be_a(DesignManagement::DesignCollection)
expect(collection.issue).to eq(issue)
end
end
describe 'current designs' do
let(:issue) { create(:issue) }
subject { issue.designs.current }
context 'an issue has no designs' do
it { is_expected.to be_empty }
end
context 'an issue only has current designs' do
let!(:design_a) { create(:design, :with_file, issue: issue) }
let!(:design_b) { create(:design, :with_file, issue: issue) }
let!(:design_c) { create(:design, :with_file, issue: issue) }
it { is_expected.to include(design_a, design_b, design_c) }
end
context 'an issue only has deleted designs' do
let!(:design_a) { create(:design, :with_file, issue: issue, deleted: true) }
let!(:design_b) { create(:design, :with_file, issue: issue, deleted: true) }
let!(:design_c) { create(:design, :with_file, issue: issue, deleted: true) }
it { is_expected.to be_empty }
end
context 'an issue has a mixture of current and deleted designs' do
let!(:design_a) { create(:design, :with_file, issue: issue) }
let!(:design_b) { create(:design, :with_file, issue: issue, deleted: true) }
let!(:design_c) { create(:design, :with_file, issue: issue) }
it { is_expected.to contain_exactly(design_a, design_c) }
end
end
end end
...@@ -751,6 +751,14 @@ describe Note do ...@@ -751,6 +751,14 @@ describe Note do
end end
end end
describe '#for_design' do
it 'is true when the noteable is a design' do
note = build(:note, noteable: build(:design))
expect(note).to be_for_design
end
end
describe '#to_ability_name' do describe '#to_ability_name' do
it 'returns note' do it 'returns note' do
expect(build(:note).to_ability_name).to eq('note') expect(build(:note).to_ability_name).to eq('note')
......
...@@ -6,6 +6,7 @@ describe Project do ...@@ -6,6 +6,7 @@ describe Project do
include ProjectForksHelper include ProjectForksHelper
include GitHelpers include GitHelpers
include ExternalAuthorizationServiceHelpers include ExternalAuthorizationServiceHelpers
using RSpec::Parameterized::TableSyntax
it_behaves_like 'having unique enum values' it_behaves_like 'having unique enum values'
...@@ -6058,6 +6059,28 @@ describe Project do ...@@ -6058,6 +6059,28 @@ describe Project do
end end
end end
describe '#design_management_enabled?' do
let(:project) { build(:project) }
where(:lfs_enabled, :hashed_storage_enabled, :expectation) do
false | false | false
true | false | false
false | true | false
true | true | true
end
with_them do
before do
expect(project).to receive(:lfs_enabled?).and_return(lfs_enabled)
allow(project).to receive(:hashed_storage?).with(:repository).and_return(hashed_storage_enabled)
end
it do
expect(project.design_management_enabled?).to be(expectation)
end
end
end
def finish_job(export_job) def finish_job(export_job)
export_job.start export_job.start
export_job.finish export_job.finish
......
...@@ -23,8 +23,15 @@ describe DesignManagement::DesignUserNotesCountService, :use_clean_rails_memory_ ...@@ -23,8 +23,15 @@ describe DesignManagement::DesignUserNotesCountService, :use_clean_rails_memory_
end end
end end
# TODO These tests are being temporarily skipped unless run in EE,
# as we are in the process of moving Design Management to FOSS in 13.0
# in steps. In the current step the services have not yet been moved.
#
# See https://gitlab.com/gitlab-org/gitlab/-/issues/212566#note_327724283.
describe 'cache invalidation' do describe 'cache invalidation' do
it 'changes when a new note is created' do it 'changes when a new note is created' do
skip 'See https://gitlab.com/gitlab-org/gitlab/-/issues/212566#note_327724283' unless Gitlab.ee?
new_note_attrs = attributes_for(:diff_note_on_design, noteable: design) new_note_attrs = attributes_for(:diff_note_on_design, noteable: design)
expect do expect do
...@@ -33,6 +40,8 @@ describe DesignManagement::DesignUserNotesCountService, :use_clean_rails_memory_ ...@@ -33,6 +40,8 @@ describe DesignManagement::DesignUserNotesCountService, :use_clean_rails_memory_
end end
it 'changes when a note is destroyed' do it 'changes when a note is destroyed' do
skip 'See https://gitlab.com/gitlab-org/gitlab/-/issues/212566#note_327724283' unless Gitlab.ee?
note = create(:diff_note_on_design, noteable: design) note = create(:diff_note_on_design, noteable: design)
expect do expect 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