summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2020-05-04 06:10:10 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2020-05-04 06:10:10 +0000
commit2fa68d3a97fd31bf469050e130f0fc95e8944316 (patch)
tree5c00585c55c44917765c152426cb58c803b4f57f
parent21be9646a94e2c145897e25d9c521523d55e1614 (diff)
downloadgitlab-ce-2fa68d3a97fd31bf469050e130f0fc95e8944316.tar.gz
Add latest changes from gitlab-org/gitlab@master
-rw-r--r--app/assets/javascripts/alert_management/components/alert_management_list.vue24
-rw-r--r--app/controllers/projects/issues_controller.rb9
-rw-r--r--app/models/design_management.rb13
-rw-r--r--app/models/design_management/action.rb42
-rw-r--r--app/models/design_management/design.rb266
-rw-r--r--app/models/design_management/design_action.rb64
-rw-r--r--app/models/design_management/design_at_version.rb119
-rw-r--r--app/models/design_management/design_collection.rb30
-rw-r--r--app/models/design_management/repository.rb51
-rw-r--r--app/models/design_management/version.rb144
-rw-r--r--app/models/design_user_mention.rb6
-rw-r--r--app/models/diff_note.rb10
-rw-r--r--app/models/issue.rb10
-rw-r--r--app/models/note.rb4
-rw-r--r--app/models/project.rb12
-rw-r--r--app/services/design_management/design_user_notes_count_service.rb34
-rw-r--r--app/services/issues/related_branches_service.rb20
-rw-r--r--app/uploaders/design_management/design_v432x230_uploader.rb45
-rw-r--r--app/views/projects/graphs/charts.html.haml4
-rw-r--r--app/views/projects/issues/_related_branches.html.haml8
-rw-r--r--changelogs/unreleased/mw-ca-add-title.yml5
-rw-r--r--changelogs/unreleased/mw-ra-add-title.yml5
-rw-r--r--lib/gitlab/git_access_design.rb28
-rw-r--r--lib/gitlab/gl_repository.rb11
-rw-r--r--lib/gitlab/gl_repository/repo_type.rb6
-rw-r--r--locale/gitlab.pot9
-rw-r--r--spec/controllers/projects/issues_controller_spec.rb85
-rw-r--r--spec/factories/design_management/actions.rb13
-rw-r--r--spec/factories/design_management/design_at_version.rb23
-rw-r--r--spec/factories/design_management/designs.rb128
-rw-r--r--spec/factories/design_management/versions.rb142
-rw-r--r--spec/factories/notes.rb18
-rw-r--r--spec/factories/projects.rb6
-rw-r--r--spec/factories/uploads.rb6
-rw-r--r--spec/frontend/alert_management/components/alert_management_list_spec.js12
-rw-r--r--spec/lib/gitlab/git_access_design_spec.rb54
-rw-r--r--spec/lib/gitlab/gl_repository/repo_type_spec.rb37
-rw-r--r--spec/lib/gitlab/gl_repository_spec.rb15
-rw-r--r--spec/models/design_management/action_spec.rb105
-rw-r--r--spec/models/design_management/design_action_spec.rb98
-rw-r--r--spec/models/design_management/design_at_version_spec.rb426
-rw-r--r--spec/models/design_management/design_collection_spec.rb82
-rw-r--r--spec/models/design_management/design_spec.rb582
-rw-r--r--spec/models/design_management/repository_spec.rb58
-rw-r--r--spec/models/design_management/version_spec.rb342
-rw-r--r--spec/models/design_user_mention_spec.rb12
-rw-r--r--spec/models/diff_note_spec.rb18
-rw-r--r--spec/models/issue_spec.rb56
-rw-r--r--spec/models/note_spec.rb8
-rw-r--r--spec/models/project_spec.rb23
-rw-r--r--spec/services/design_management/design_user_notes_count_service_spec.rb52
-rw-r--r--spec/services/issues/related_branches_service_spec.rb102
-rw-r--r--spec/support/helpers/design_management_test_helpers.rb45
-rw-r--r--spec/uploaders/design_management/design_v432x230_uploader_spec.rb86
-rw-r--r--spec/views/projects/issues/_related_branches.html.haml_spec.rb24
55 files changed, 3589 insertions, 48 deletions
diff --git a/app/assets/javascripts/alert_management/components/alert_management_list.vue b/app/assets/javascripts/alert_management/components/alert_management_list.vue
index 5ea51bef496..1e0cbfbf125 100644
--- a/app/assets/javascripts/alert_management/components/alert_management_list.vue
+++ b/app/assets/javascripts/alert_management/components/alert_management_list.vue
@@ -1,5 +1,13 @@
<script>
-import { GlEmptyState, GlDeprecatedButton, GlLoadingIcon, GlTable, GlAlert } from '@gitlab/ui';
+import {
+ GlEmptyState,
+ GlDeprecatedButton,
+ GlLoadingIcon,
+ GlTable,
+ GlAlert,
+ GlNewDropdown,
+ GlNewDropdownItem,
+} from '@gitlab/ui';
import { s__ } from '~/locale';
import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue';
import getAlerts from '../graphql/queries/getAlerts.query.graphql';
@@ -42,6 +50,11 @@ export default {
label: s__('AlertManagement|Status'),
},
],
+ statuses: {
+ triggered: s__('AlertManagement|Triggered'),
+ acknowledged: s__('AlertManagement|Acknowledged'),
+ resolved: s__('AlertManagement|Resolved'),
+ },
components: {
GlEmptyState,
GlLoadingIcon,
@@ -49,6 +62,8 @@ export default {
GlAlert,
GlDeprecatedButton,
TimeAgo,
+ GlNewDropdown,
+ GlNewDropdownItem,
},
props: {
projectPath: {
@@ -140,6 +155,13 @@ export default {
<template #cell(title)="{ item }">
<div class="gl-max-w-full text-truncate">{{ item.title }}</div>
</template>
+ <template #cell(status)="{ item }">
+ <gl-new-dropdown class="w-100" :text="item.status">
+ <gl-new-dropdown-item v-for="(label, field) in $options.statuses" :key="field">
+ {{ label }}
+ </gl-new-dropdown-item>
+ </gl-new-dropdown>
+ </template>
<template #empty>
{{ s__('AlertManagement|No alerts to display.') }}
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 3d53ad1a29f..95b4d5e5658 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -154,7 +154,10 @@ class Projects::IssuesController < Projects::ApplicationController
end
def related_branches
- @related_branches = Issues::RelatedBranchesService.new(project, current_user).execute(issue)
+ @related_branches = Issues::RelatedBranchesService
+ .new(project, current_user)
+ .execute(issue)
+ .map { |branch| branch.merge(link: branch_link(branch)) }
respond_to do |format|
format.json do
@@ -306,6 +309,10 @@ class Projects::IssuesController < Projects::ApplicationController
private
+ def branch_link(branch)
+ project_compare_path(project, from: project.default_branch, to: branch[:name])
+ end
+
def create_rate_limit
key = :issues_create
diff --git a/app/models/design_management.rb b/app/models/design_management.rb
new file mode 100644
index 00000000000..81e170f7e59
--- /dev/null
+++ b/app/models/design_management.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+module DesignManagement
+ DESIGN_IMAGE_SIZES = %w(v432x230).freeze
+
+ def self.designs_directory
+ 'designs'
+ end
+
+ def self.table_name_prefix
+ 'design_management_'
+ end
+end
diff --git a/app/models/design_management/action.rb b/app/models/design_management/action.rb
new file mode 100644
index 00000000000..8fe7d7c577c
--- /dev/null
+++ b/app/models/design_management/action.rb
@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+module DesignManagement
+ class Action < ApplicationRecord
+ include WithUploads
+
+ self.table_name = "#{DesignManagement.table_name_prefix}designs_versions"
+
+ mount_uploader :image_v432x230, DesignManagement::DesignV432x230Uploader
+
+ belongs_to :design, class_name: "DesignManagement::Design", inverse_of: :actions
+ belongs_to :version, class_name: "DesignManagement::Version", inverse_of: :actions
+
+ enum event: { creation: 0, modification: 1, deletion: 2 }
+
+ # we assume sequential ordering.
+ scope :ordered, -> { order(version_id: :asc) }
+
+ # For each design, only select the most recent action
+ scope :most_recent, -> do
+ selection = Arel.sql("DISTINCT ON (#{table_name}.design_id) #{table_name}.*")
+
+ order(arel_table[:design_id].asc, arel_table[:version_id].desc).select(selection)
+ end
+
+ # Find all records created before or at the given version, or all if nil
+ scope :up_to_version, ->(version = nil) do
+ case version
+ when nil
+ all
+ when DesignManagement::Version
+ where(arel_table[:version_id].lteq(version.id))
+ when ::Gitlab::Git::COMMIT_ID
+ versions = DesignManagement::Version.arel_table
+ subquery = versions.project(versions[:id]).where(versions[:sha].eq(version))
+ where(arel_table[:version_id].lteq(subquery))
+ else
+ raise ArgumentError, "Expected a DesignManagement::Version, got #{version}"
+ end
+ end
+ end
+end
diff --git a/app/models/design_management/design.rb b/app/models/design_management/design.rb
new file mode 100644
index 00000000000..e9b69eab7a7
--- /dev/null
+++ b/app/models/design_management/design.rb
@@ -0,0 +1,266 @@
+# frozen_string_literal: true
+
+module DesignManagement
+ class Design < ApplicationRecord
+ include Importable
+ include Noteable
+ include Gitlab::FileTypeDetection
+ include Gitlab::Utils::StrongMemoize
+ include Referable
+ include Mentionable
+ include WhereComposite
+
+ belongs_to :project, inverse_of: :designs
+ belongs_to :issue
+
+ has_many :actions
+ has_many :versions, through: :actions, class_name: 'DesignManagement::Version', inverse_of: :designs
+ # This is a polymorphic association, so we can't count on FK's to delete the
+ # data
+ 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
+
+ validates :project, :filename, presence: true
+ validates :issue, presence: true, unless: :importing?
+ validates :filename, uniqueness: { scope: :issue_id }
+ validate :validate_file_is_image
+
+ alias_attribute :title, :filename
+
+ # Pre-fetching scope to include the data necessary to construct a
+ # reference using `to_reference`.
+ scope :for_reference, -> { includes(issue: [{ project: [:route, :namespace] }]) }
+
+ # A design can be uniquely identified by issue_id and filename
+ # Takes one or more sets of composite IDs of the form:
+ # `{issue_id: Integer, filename: String}`.
+ #
+ # @see WhereComposite::where_composite
+ #
+ # e.g:
+ #
+ # by_issue_id_and_filename(issue_id: 1, filename: 'homescreen.jpg')
+ # by_issue_id_and_filename([]) # returns ActiveRecord::NullRelation
+ # by_issue_id_and_filename([
+ # { issue_id: 1, filename: 'homescreen.jpg' },
+ # { issue_id: 2, filename: 'homescreen.jpg' },
+ # { issue_id: 1, filename: 'menu.png' }
+ # ])
+ #
+ scope :by_issue_id_and_filename, ->(composites) do
+ where_composite(%i[issue_id filename], composites)
+ end
+
+ # Find designs visible at the given version
+ #
+ # @param version [nil, DesignManagement::Version]:
+ # the version at which the designs must be visible
+ # Passing `nil` is the same as passing the most current version
+ #
+ # Restricts to designs
+ # - created at least *before* the given version
+ # - not deleted as of the given version.
+ #
+ # As a query, we ascertain this by finding the last event prior to
+ # (or equal to) the cut-off, and seeing whether that version was a deletion.
+ scope :visible_at_version, -> (version) do
+ deletion = ::DesignManagement::Action.events[:deletion]
+ designs = arel_table
+ actions = ::DesignManagement::Action
+ .most_recent.up_to_version(version)
+ .arel.as('most_recent_actions')
+
+ join = designs.join(actions)
+ .on(actions[:design_id].eq(designs[:id]))
+
+ joins(join.join_sources).where(actions[:event].not_eq(deletion)).order(:id)
+ end
+
+ scope :with_filename, -> (filenames) { where(filename: filenames) }
+ scope :on_issue, ->(issue) { where(issue_id: issue) }
+
+ # Scope called by our REST API to avoid N+1 problems
+ scope :with_api_entity_associations, -> { preload(:issue) }
+
+ # A design is current if the most recent event is not a deletion
+ scope :current, -> { visible_at_version(nil) }
+
+ def status
+ if new_design?
+ :new
+ elsif deleted?
+ :deleted
+ else
+ :current
+ end
+ end
+
+ def deleted?
+ most_recent_action&.deletion?
+ end
+
+ # A design is visible_in? a version if:
+ # * it was created before that version
+ # * the most recent action before the version was not a deletion
+ def visible_in?(version)
+ map = strong_memoize(:visible_in) do
+ Hash.new do |h, k|
+ h[k] = self.class.visible_at_version(k).where(id: id).exists?
+ end
+ end
+
+ map[version]
+ end
+
+ def most_recent_action
+ strong_memoize(:most_recent_action) { actions.ordered.last }
+ end
+
+ # A reference for a design is the issue reference, indexed by the filename
+ # with an optional infix when full.
+ #
+ # e.g.
+ # #123[homescreen.png]
+ # other-project#72[sidebar.jpg]
+ # #38/designs[transition.gif]
+ # #12["filename with [] in it.jpg"]
+ def to_reference(from = nil, full: false)
+ infix = full ? '/designs' : ''
+ totally_simple = %r{ \A #{self.class.simple_file_name} \z }x
+ safe_name = if totally_simple.match?(filename)
+ filename
+ elsif filename =~ /[<>]/
+ %Q{base64:#{Base64.strict_encode64(filename)}}
+ else
+ escaped = filename.gsub(%r{[\\"]}) { |x| "\\#{x}" }
+ %Q{"#{escaped}"}
+ end
+
+ "#{issue.to_reference(from, full: full)}#{infix}[#{safe_name}]"
+ end
+
+ def self.reference_pattern
+ @reference_pattern ||= begin
+ # Filenames can be escaped with double quotes to name filenames
+ # that include square brackets, or other special characters
+ %r{
+ #{Issue.reference_pattern}
+ (\/designs)?
+ \[
+ (?<design> #{simple_file_name} | #{quoted_file_name} | #{base_64_encoded_name})
+ \]
+ }x
+ end
+ end
+
+ def self.simple_file_name
+ %r{
+ (?<simple_file_name>
+ ( \w | [_:,'-] | \. | \s )+
+ \.
+ \w+
+ )
+ }x
+ end
+
+ def self.base_64_encoded_name
+ %r{
+ base64:
+ (?<base_64_encoded_name>
+ [A-Za-z0-9+\n]+
+ =?
+ )
+ }x
+ end
+
+ def self.quoted_file_name
+ %r{
+ "
+ (?<escaped_filename>
+ (\\ \\ | \\ " | [^"\\])+
+ )
+ "
+ }x
+ end
+
+ def self.link_reference_pattern
+ @link_reference_pattern ||= begin
+ exts = SAFE_IMAGE_EXT + DANGEROUS_IMAGE_EXT
+ path_segment = %r{issues/#{Gitlab::Regex.issue}/designs}
+ filename_pattern = %r{(?<simple_file_name>[a-z0-9_=-]+\.(#{exts.join('|')}))}i
+
+ super(path_segment, filename_pattern)
+ end
+ end
+
+ def to_ability_name
+ 'design'
+ end
+
+ def description
+ ''
+ end
+
+ def new_design?
+ strong_memoize(:new_design) { actions.none? }
+ end
+
+ def full_path
+ @full_path ||= File.join(DesignManagement.designs_directory, "issue-#{issue.iid}", filename)
+ end
+
+ def diff_refs
+ strong_memoize(:diff_refs) { head_version&.diff_refs }
+ end
+
+ def clear_version_cache
+ [versions, actions].each(&:reset)
+ %i[new_design diff_refs head_sha visible_in most_recent_action].each do |key|
+ clear_memoization(key)
+ end
+ end
+
+ def repository
+ project.design_repository
+ end
+
+ def user_notes_count
+ user_notes_count_service.count
+ end
+
+ def after_note_changed(note)
+ user_notes_count_service.delete_cache unless note.system?
+ end
+ alias_method :after_note_created, :after_note_changed
+ alias_method :after_note_destroyed, :after_note_changed
+
+ private
+
+ def head_version
+ strong_memoize(:head_sha) { versions.ordered.first }
+ end
+
+ def allow_dangerous_images?
+ Feature.enabled?(:design_management_allow_dangerous_images, project)
+ end
+
+ def valid_file_extensions
+ allow_dangerous_images? ? (SAFE_IMAGE_EXT + DANGEROUS_IMAGE_EXT) : SAFE_IMAGE_EXT
+ end
+
+ def validate_file_is_image
+ unless image? || (dangerous_image? && allow_dangerous_images?)
+ message = _('does not have a supported extension. Only %{extension_list} are supported') % {
+ extension_list: valid_file_extensions.to_sentence
+ }
+ errors.add(:filename, message)
+ end
+ end
+
+ def user_notes_count_service
+ strong_memoize(:user_notes_count_service) do
+ ::DesignManagement::DesignUserNotesCountService.new(self) # rubocop: disable CodeReuse/ServiceClass
+ end
+ end
+ end
+end
diff --git a/app/models/design_management/design_action.rb b/app/models/design_management/design_action.rb
new file mode 100644
index 00000000000..22baa916296
--- /dev/null
+++ b/app/models/design_management/design_action.rb
@@ -0,0 +1,64 @@
+# frozen_string_literal: true
+
+module DesignManagement
+ # Parameter object which is a tuple of the database record and the
+ # last gitaly call made to change it. This serves to perform the
+ # logical mapping from git action to database representation.
+ class DesignAction
+ include ActiveModel::Validations
+
+ EVENT_FOR_GITALY_ACTION = {
+ create: DesignManagement::Action.events[:creation],
+ update: DesignManagement::Action.events[:modification],
+ delete: DesignManagement::Action.events[:deletion]
+ }.freeze
+
+ attr_reader :design, :action, :content
+
+ delegate :issue_id, to: :design
+
+ validates :design, presence: true
+ validates :action, presence: true, inclusion: { in: EVENT_FOR_GITALY_ACTION.keys }
+ validates :content,
+ absence: { if: :forbids_content?,
+ message: 'this action forbids content' },
+ presence: { if: :needs_content?,
+ message: 'this action needs content' }
+
+ # Parameters:
+ # - design [DesignManagement::Design]: the design that was changed
+ # - action [Symbol]: the action that gitaly performed
+ def initialize(design, action, content = nil)
+ @design, @action, @content = design, action, content
+ validate!
+ end
+
+ def row_attrs(version)
+ { design_id: design.id, version_id: version.id, event: event }
+ end
+
+ def gitaly_action
+ { action: action, file_path: design.full_path, content: content }.compact
+ end
+
+ # This action has been performed - do any post-creation actions
+ # such as clearing method caches.
+ def performed
+ design.clear_version_cache
+ end
+
+ private
+
+ def needs_content?
+ action != :delete
+ end
+
+ def forbids_content?
+ action == :delete
+ end
+
+ def event
+ EVENT_FOR_GITALY_ACTION[action]
+ end
+ end
+end
diff --git a/app/models/design_management/design_at_version.rb b/app/models/design_management/design_at_version.rb
new file mode 100644
index 00000000000..b4cafb93c2c
--- /dev/null
+++ b/app/models/design_management/design_at_version.rb
@@ -0,0 +1,119 @@
+# frozen_string_literal: true
+
+# Tuple of design and version
+# * has a composite ID, with lazy_find
+module DesignManagement
+ class DesignAtVersion
+ include ActiveModel::Validations
+ include GlobalID::Identification
+ include Gitlab::Utils::StrongMemoize
+
+ attr_reader :version
+ attr_reader :design
+
+ validates :version, presence: true
+ validates :design, presence: true
+
+ validate :design_and_version_belong_to_the_same_issue
+ validate :design_and_version_have_issue_id
+
+ def initialize(design: nil, version: nil)
+ @design, @version = design, version
+ end
+
+ def self.instantiate(attrs)
+ new(attrs).tap { |obj| obj.validate! }
+ end
+
+ # The ID, needed by GraphQL types and as part of the Lazy-fetch
+ # protocol, includes information about both the design and the version.
+ #
+ # The particular format is not interesting, and should be treated as opaque
+ # by all callers.
+ def id
+ "#{design.id}.#{version.id}"
+ end
+
+ def ==(other)
+ return false unless other && self.class == other.class
+
+ other.id == id
+ end
+
+ alias_method :eql?, :==
+
+ def self.lazy_find(id)
+ BatchLoader.for(id).batch do |ids, callback|
+ find(ids).each do |record|
+ callback.call(record.id, record)
+ end
+ end
+ end
+
+ def self.find(ids)
+ pairs = ids.map { |id| id.split('.').map(&:to_i) }
+
+ design_ids = pairs.map(&:first).uniq
+ version_ids = pairs.map(&:second).uniq
+
+ designs = ::DesignManagement::Design
+ .where(id: design_ids)
+ .index_by(&:id)
+
+ versions = ::DesignManagement::Version
+ .where(id: version_ids)
+ .index_by(&:id)
+
+ pairs.map do |(design_id, version_id)|
+ design = designs[design_id]
+ version = versions[version_id]
+
+ obj = new(design: design, version: version)
+
+ obj if obj.valid?
+ end.compact
+ end
+
+ def status
+ if not_created_yet?
+ :not_created_yet
+ elsif deleted?
+ :deleted
+ else
+ :current
+ end
+ end
+
+ def deleted?
+ action&.deletion?
+ end
+
+ def not_created_yet?
+ action.nil?
+ end
+
+ private
+
+ def action
+ strong_memoize(:most_recent_action) do
+ ::DesignManagement::Action
+ .most_recent.up_to_version(version)
+ .find_by(design: design)
+ end
+ end
+
+ def design_and_version_belong_to_the_same_issue
+ id_a, id_b = [design, version].map { |obj| obj&.issue_id }
+
+ return if id_a == id_b
+
+ errors.add(:issue, 'must be the same on design and version')
+ end
+
+ def design_and_version_have_issue_id
+ return if [design, version].all? { |obj| obj.try(:issue_id).present? }
+
+ errors.add(:issue, 'must be present on both design and version')
+ end
+ end
+end
diff --git a/app/models/design_management/design_collection.rb b/app/models/design_management/design_collection.rb
new file mode 100644
index 00000000000..18d1541e9c7
--- /dev/null
+++ b/app/models/design_management/design_collection.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+module DesignManagement
+ class DesignCollection
+ attr_reader :issue
+
+ delegate :designs, :project, to: :issue
+
+ def initialize(issue)
+ @issue = issue
+ end
+
+ def find_or_create_design!(filename:)
+ designs.find { |design| design.filename == filename } ||
+ designs.safe_find_or_create_by!(project: project, filename: filename)
+ end
+
+ def versions
+ @versions ||= DesignManagement::Version.for_designs(designs)
+ end
+
+ def repository
+ project.design_repository
+ end
+
+ def designs_by_filename(filenames)
+ designs.current.where(filename: filenames)
+ end
+ end
+end
diff --git a/app/models/design_management/repository.rb b/app/models/design_management/repository.rb
new file mode 100644
index 00000000000..985d6317d5d
--- /dev/null
+++ b/app/models/design_management/repository.rb
@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+
+module DesignManagement
+ class Repository < ::Repository
+ extend ::Gitlab::Utils::Override
+
+ # We define static git attributes for the design repository as this
+ # repository is entirely GitLab-managed rather than user-facing.
+ #
+ # Enable all uploaded files to be stored in LFS.
+ MANAGED_GIT_ATTRIBUTES = <<~GA.freeze
+ /#{DesignManagement.designs_directory}/* filter=lfs diff=lfs merge=lfs -text
+ GA
+
+ def initialize(project)
+ full_path = project.full_path + 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: Gitlab::GlRepository::DESIGN)
+ end
+
+ # Override of a method called on Repository instances but sent via
+ # method_missing to Gitlab::Git::Repository where it is defined
+ def info_attributes
+ @info_attributes ||= Gitlab::Git::AttributesParser.new(MANAGED_GIT_ATTRIBUTES)
+ end
+
+ # Override of a method called on Repository instances but sent via
+ # method_missing to Gitlab::Git::Repository where it is defined
+ def attributes(path)
+ info_attributes.attributes(path)
+ end
+
+ # Override of a method called on Repository instances but sent via
+ # method_missing to Gitlab::Git::Repository where it is defined
+ def gitattribute(path, name)
+ attributes(path)[name]
+ end
+
+ # Override of a method called on Repository instances but sent via
+ # method_missing to Gitlab::Git::Repository where it is defined
+ def attributes_at(_ref = nil)
+ info_attributes
+ end
+
+ override :copy_gitattributes
+ def copy_gitattributes(_ref = nil)
+ true
+ end
+ end
+end
diff --git a/app/models/design_management/version.rb b/app/models/design_management/version.rb
new file mode 100644
index 00000000000..6be98fe3d44
--- /dev/null
+++ b/app/models/design_management/version.rb
@@ -0,0 +1,144 @@
+# frozen_string_literal: true
+
+module DesignManagement
+ class Version < ApplicationRecord
+ include Importable
+ include ShaAttribute
+ include AfterCommitQueue
+ include Gitlab::Utils::StrongMemoize
+ extend Gitlab::ExclusiveLeaseHelpers
+
+ NotSameIssue = Class.new(StandardError)
+
+ class CouldNotCreateVersion < StandardError
+ attr_reader :sha, :issue_id, :actions
+
+ def initialize(sha, issue_id, actions)
+ @sha, @issue_id, @actions = sha, issue_id, actions
+ end
+
+ def message
+ "could not create version from commit: #{sha}"
+ end
+
+ def sentry_extra_data
+ {
+ sha: sha,
+ issue_id: issue_id,
+ design_ids: actions.map { |a| a.design.id }
+ }
+ end
+ end
+
+ belongs_to :issue
+ belongs_to :author, class_name: 'User'
+ has_many :actions
+ has_many :designs,
+ through: :actions,
+ class_name: "DesignManagement::Design",
+ source: :design,
+ inverse_of: :versions
+
+ validates :designs, presence: true, unless: :importing?
+ validates :sha, presence: true
+ validates :sha, uniqueness: { case_sensitive: false, scope: :issue_id }
+ validates :author, presence: true
+ # We are not validating the issue object as it incurs an extra query to fetch
+ # the record from the DB. Instead, we rely on the foreign key constraint to
+ # ensure referential integrity.
+ validates :issue_id, presence: true, unless: :importing?
+
+ sha_attribute :sha
+
+ delegate :project, to: :issue
+
+ scope :for_designs, -> (designs) do
+ where(id: ::DesignManagement::Action.where(design_id: designs).select(:version_id)).distinct
+ end
+ scope :earlier_or_equal_to, -> (version) { where("(#{table_name}.id) <= ?", version) } # rubocop:disable GitlabSecurity/SqlInjection
+ scope :ordered, -> { order(id: :desc) }
+ scope :for_issue, -> (issue) { where(issue: issue) }
+ scope :by_sha, -> (sha) { where(sha: sha) }
+
+ # This is the one true way to create a Version.
+ #
+ # This method means you can avoid the paradox of versions being invalid without
+ # designs, and not being able to add designs without a saved version. Also this
+ # method inserts designs in bulk, rather than one by one.
+ #
+ # Before calling this method, callers must guard against concurrent
+ # modification by obtaining the lock on the design repository. See:
+ # `DesignManagement::Version.with_lock`.
+ #
+ # Parameters:
+ # - design_actions [DesignManagement::DesignAction]:
+ # the actions that have been performed in the repository.
+ # - sha [String]:
+ # the SHA of the commit that performed them
+ # - author [User]:
+ # the user who performed the commit
+ # returns [DesignManagement::Version]
+ def self.create_for_designs(design_actions, sha, author)
+ issue_id, not_uniq = design_actions.map(&:issue_id).compact.uniq
+ raise NotSameIssue, 'All designs must belong to the same issue!' if not_uniq
+
+ transaction do
+ version = new(sha: sha, issue_id: issue_id, author: author)
+ version.save(validate: false) # We need it to have an ID. Validate later when designs are present
+
+ rows = design_actions.map { |action| action.row_attrs(version) }
+
+ Gitlab::Database.bulk_insert(::DesignManagement::Action.table_name, rows)
+ version.designs.reset
+ version.validate!
+ design_actions.each(&:performed)
+
+ version
+ end
+ rescue
+ raise CouldNotCreateVersion.new(sha, issue_id, design_actions)
+ end
+
+ CREATION_TTL = 5.seconds
+ RETRY_DELAY = ->(num) { 0.2.seconds * num**2 }
+
+ def self.with_lock(project_id, repository, &block)
+ key = "with_lock:#{name}:{#{project_id}}"
+
+ in_lock(key, ttl: CREATION_TTL, retries: 5, sleep_sec: RETRY_DELAY) do |_retried|
+ repository.create_if_not_exists
+ yield
+ end
+ end
+
+ def designs_by_event
+ actions
+ .includes(:design)
+ .group_by(&:event)
+ .transform_values { |group| group.map(&:design) }
+ end
+
+ def author
+ super || (commit_author if persisted?)
+ end
+
+ def diff_refs
+ strong_memoize(:diff_refs) { commit&.diff_refs }
+ end
+
+ def reset
+ %i[diff_refs commit].each { |k| clear_memoization(k) }
+ super
+ end
+
+ private
+
+ def commit_author
+ commit&.author
+ end
+
+ def commit
+ strong_memoize(:commit) { issue.project.design_repository.commit(sha) }
+ end
+ end
+end
diff --git a/app/models/design_user_mention.rb b/app/models/design_user_mention.rb
new file mode 100644
index 00000000000..baf4db29a0f
--- /dev/null
+++ b/app/models/design_user_mention.rb
@@ -0,0 +1,6 @@
+# frozen_string_literal: true
+
+class DesignUserMention < UserMention
+ belongs_to :design, class_name: 'DesignManagement::Design'
+ belongs_to :note
+end
diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb
index e3df61dadae..ff39dbb59f3 100644
--- a/app/models/diff_note.rb
+++ b/app/models/diff_note.rb
@@ -9,7 +9,7 @@ class DiffNote < Note
include Gitlab::Utils::StrongMemoize
def self.noteable_types
- %w(MergeRequest Commit)
+ %w(MergeRequest Commit DesignManagement::Design)
end
validates :original_position, presence: true
@@ -60,6 +60,8 @@ class DiffNote < Note
# Returns the diff file from `position`
def latest_diff_file
strong_memoize(:latest_diff_file) do
+ next if for_design?
+
position.diff_file(repository)
end
end
@@ -67,6 +69,8 @@ class DiffNote < Note
# Returns the diff file from `original_position`
def diff_file
strong_memoize(:diff_file) do
+ next if for_design?
+
enqueue_diff_file_creation_job if should_create_diff_file?
fetch_diff_file
@@ -145,7 +149,7 @@ class DiffNote < Note
end
def supported?
- for_commit? || self.noteable.has_complete_diff_refs?
+ for_commit? || for_design? || self.noteable.has_complete_diff_refs?
end
def set_line_code
@@ -184,5 +188,3 @@ class DiffNote < Note
noteable.respond_to?(:repository) ? noteable.repository : project.repository
end
end
-
-DiffNote.prepend_if_ee('::EE::DiffNote')
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 7e303bc257a..82643d8f5d6 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -49,6 +49,12 @@ class Issue < ApplicationRecord
has_many :zoom_meetings
has_many :user_mentions, class_name: "IssueUserMention", dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
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 :alert_management_alert, class_name: 'AlertManagement::Alert'
@@ -334,6 +340,10 @@ class Issue < ApplicationRecord
previous_changes['updated_at']&.first || updated_at
end
+ def design_collection
+ @design_collection ||= ::DesignManagement::DesignCollection.new(self)
+ end
+
private
def ensure_metrics
diff --git a/app/models/note.rb b/app/models/note.rb
index a2a711c987f..9d0cd30f5dc 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -279,6 +279,10 @@ class Note < ApplicationRecord
!for_personal_snippet?
end
+ def for_design?
+ noteable_type == DesignManagement::Design.name
+ end
+
def for_issuable?
for_issue? || for_merge_request?
end
diff --git a/app/models/project.rb b/app/models/project.rb
index 502d3391d63..bd1785bc620 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -215,6 +215,7 @@ class Project < ApplicationRecord
has_many :protected_branches
has_many :protected_tags
has_many :repository_languages, -> { order "share DESC" }
+ has_many :designs, inverse_of: :project, class_name: 'DesignManagement::Design'
has_many :project_authorizations
has_many :authorized_users, through: :project_authorizations, source: :user, class_name: 'User'
@@ -791,6 +792,11 @@ class Project < ApplicationRecord
Feature.enabled?(:jira_issue_import, self, default_enabled: true)
end
+ # LFS and hashed repository storage are required for using Design Management.
+ def design_management_enabled?
+ lfs_enabled? && hashed_storage?(:repository)
+ end
+
def team
@team ||= ProjectTeam.new(self)
end
@@ -799,6 +805,12 @@ class Project < ApplicationRecord
@repository ||= Repository.new(full_path, self, shard: repository_storage, disk_path: disk_path)
end
+ def design_repository
+ strong_memoize(:design_repository) do
+ DesignManagement::Repository.new(self)
+ end
+ end
+
def cleanup
@repository = nil
end
diff --git a/app/services/design_management/design_user_notes_count_service.rb b/app/services/design_management/design_user_notes_count_service.rb
new file mode 100644
index 00000000000..e49914ea6d3
--- /dev/null
+++ b/app/services/design_management/design_user_notes_count_service.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+module DesignManagement
+ # Service class for counting and caching the number of unresolved
+ # notes of a Design
+ class DesignUserNotesCountService < ::BaseCountService
+ # The version of the cache format. This should be bumped whenever the
+ # underlying logic changes. This removes the need for explicitly flushing
+ # all caches.
+ VERSION = 1
+
+ def initialize(design)
+ @design = design
+ end
+
+ def relation_for_count
+ design.notes.user
+ end
+
+ def raw?
+ # Since we're storing simple integers we don't need all of the
+ # additional Marshal data Rails includes by default.
+ true
+ end
+
+ def cache_key
+ ['designs', 'notes_count', VERSION, design.id]
+ end
+
+ private
+
+ attr_reader :design
+ end
+end
diff --git a/app/services/issues/related_branches_service.rb b/app/services/issues/related_branches_service.rb
index 76af482b7ac..46076218857 100644
--- a/app/services/issues/related_branches_service.rb
+++ b/app/services/issues/related_branches_service.rb
@@ -5,11 +5,29 @@
module Issues
class RelatedBranchesService < Issues::BaseService
def execute(issue)
- branches_with_iid_of(issue) - branches_with_merge_request_for(issue)
+ branch_names = branches_with_iid_of(issue) - branches_with_merge_request_for(issue)
+ branch_names.map { |branch_name| branch_data(branch_name) }
end
private
+ def branch_data(branch_name)
+ {
+ name: branch_name,
+ pipeline_status: pipeline_status(branch_name)
+ }
+ end
+
+ def pipeline_status(branch_name)
+ branch = project.repository.find_branch(branch_name)
+ target = branch&.dereferenced_target
+
+ return unless target
+
+ pipeline = project.pipeline_for(branch_name, target.sha)
+ pipeline.detailed_status(current_user) if can?(current_user, :read_pipeline, pipeline)
+ end
+
def branches_with_merge_request_for(issue)
Issues::ReferencedMergeRequestsService
.new(project, current_user)
diff --git a/app/uploaders/design_management/design_v432x230_uploader.rb b/app/uploaders/design_management/design_v432x230_uploader.rb
new file mode 100644
index 00000000000..ba48f381bbd
--- /dev/null
+++ b/app/uploaders/design_management/design_v432x230_uploader.rb
@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+module DesignManagement
+ # This Uploader is used to generate and serve the smaller versions of
+ # the design files.
+ #
+ # The original (full-sized) design files are stored in Git LFS, and so
+ # have a different uploader, `LfsObjectUploader`.
+ class DesignV432x230Uploader < GitlabUploader
+ include CarrierWave::MiniMagick
+ include RecordsUploads::Concern
+ include ObjectStorage::Concern
+ prepend ObjectStorage::Extension::RecordsUploads
+
+ # We choose not to resize `image/ico` as we assume there will be no
+ # benefit in generating an 432x230 sized icon.
+ #
+ # We currently cannot resize `image/tiff`.
+ # See https://gitlab.com/gitlab-org/gitlab/issues/207740
+ #
+ # We currently choose not to resize `image/svg+xml` for security reasons.
+ # See https://gitlab.com/gitlab-org/gitlab/issues/207740#note_302766171
+ MIME_TYPE_WHITELIST = %w(image/png image/jpeg image/bmp image/gif).freeze
+
+ process resize_to_fit: [432, 230]
+
+ # Allow CarrierWave to reject files without correct mimetypes.
+ def content_type_whitelist
+ MIME_TYPE_WHITELIST
+ end
+
+ # Override `GitlabUploader` and always return false, otherwise local
+ # `LfsObject` files would be deleted.
+ # https://github.com/carrierwaveuploader/carrierwave/blob/f84672a/lib/carrierwave/uploader/cache.rb#L131-L135
+ def move_to_cache
+ false
+ end
+
+ private
+
+ def dynamic_segment
+ File.join(model.class.underscore, mounted_as.to_s, model.id.to_s)
+ end
+ end
+end
diff --git a/app/views/projects/graphs/charts.html.haml b/app/views/projects/graphs/charts.html.haml
index 19fe7ba4360..f820d3f43cb 100644
--- a/app/views/projects/graphs/charts.html.haml
+++ b/app/views/projects/graphs/charts.html.haml
@@ -1,5 +1,9 @@
- page_title _("Repository Analytics")
+.mb-3
+ %h3
+ = _("Repository Analytics")
+
.repo-charts
%h4.sub-header
= _("Programming languages used in this repository")
diff --git a/app/views/projects/issues/_related_branches.html.haml b/app/views/projects/issues/_related_branches.html.haml
index 69b030ed76a..0604e89be6e 100644
--- a/app/views/projects/issues/_related_branches.html.haml
+++ b/app/views/projects/issues/_related_branches.html.haml
@@ -4,11 +4,9 @@
%ul.unstyled-list.related-merge-requests
- @related_branches.each do |branch|
%li
- - target = @project.repository.find_branch(branch).dereferenced_target
- - pipeline = @project.pipeline_for(branch, target.sha) if target
- - if can?(current_user, :read_pipeline, pipeline)
+ - if branch[:pipeline_status].present?
%span.related-branch-ci-status
- = render 'ci/status/icon', status: pipeline.detailed_status(current_user)
+ = render 'ci/status/icon', status: branch[:pipeline_status]
%span.related-branch-info
%strong
- = link_to branch, project_compare_path(@project, from: @project.default_branch, to: branch), class: "ref-name"
+ = link_to branch[:name], branch[:link], class: "ref-name"
diff --git a/changelogs/unreleased/mw-ca-add-title.yml b/changelogs/unreleased/mw-ca-add-title.yml
new file mode 100644
index 00000000000..5282b17bc04
--- /dev/null
+++ b/changelogs/unreleased/mw-ca-add-title.yml
@@ -0,0 +1,5 @@
+---
+title: 'Contribution Analytics: Add title to page'
+merge_request: 30842
+author:
+type: added
diff --git a/changelogs/unreleased/mw-ra-add-title.yml b/changelogs/unreleased/mw-ra-add-title.yml
new file mode 100644
index 00000000000..129ef57a6b1
--- /dev/null
+++ b/changelogs/unreleased/mw-ra-add-title.yml
@@ -0,0 +1,5 @@
+---
+title: 'Repository Analytics: Add title to page'
+merge_request: 30855
+author:
+type: added
diff --git a/lib/gitlab/git_access_design.rb b/lib/gitlab/git_access_design.rb
new file mode 100644
index 00000000000..36604bd0b3b
--- /dev/null
+++ b/lib/gitlab/git_access_design.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+module Gitlab
+ class GitAccessDesign < GitAccess
+ def check(_cmd, _changes)
+ check_protocol!
+ check_can_create_design!
+
+ success_result
+ end
+
+ private
+
+ def check_protocol!
+ if protocol != 'web'
+ raise ::Gitlab::GitAccess::ForbiddenError, "Designs are only accessible using the web interface"
+ end
+ end
+
+ def check_can_create_design!
+ unless user&.can?(:create_design, project)
+ raise ::Gitlab::GitAccess::ForbiddenError, "You are not allowed to manage designs of this project"
+ end
+ end
+ end
+end
+
+Gitlab::GitAccessDesign.prepend_if_ee('EE::Gitlab::GitAccessDesign')
diff --git a/lib/gitlab/gl_repository.rb b/lib/gitlab/gl_repository.rb
index 26440e6f82d..8abefad1ef3 100644
--- a/lib/gitlab/gl_repository.rb
+++ b/lib/gitlab/gl_repository.rb
@@ -23,11 +23,18 @@ module Gitlab
project_resolver: -> (snippet) { snippet&.project },
guest_read_ability: :read_snippet
).freeze
+ DESIGN = ::Gitlab::GlRepository::RepoType.new(
+ name: :design,
+ access_checker_class: ::Gitlab::GitAccessDesign,
+ repository_resolver: -> (project) { ::DesignManagement::Repository.new(project) },
+ suffix: :design
+ ).freeze
TYPES = {
PROJECT.name.to_s => PROJECT,
WIKI.name.to_s => WIKI,
- SNIPPET.name.to_s => SNIPPET
+ SNIPPET.name.to_s => SNIPPET,
+ DESIGN.name.to_s => DESIGN
}.freeze
def self.types
@@ -58,5 +65,3 @@ module Gitlab
private_class_method :instance
end
end
-
-Gitlab::GlRepository.prepend_if_ee('::EE::Gitlab::GlRepository')
diff --git a/lib/gitlab/gl_repository/repo_type.rb b/lib/gitlab/gl_repository/repo_type.rb
index 052ce578881..64c329b15ae 100644
--- a/lib/gitlab/gl_repository/repo_type.rb
+++ b/lib/gitlab/gl_repository/repo_type.rb
@@ -57,6 +57,10 @@ module Gitlab
self == SNIPPET
end
+ def design?
+ self == DESIGN
+ end
+
def path_suffix
suffix ? ".#{suffix}" : ''
end
@@ -87,5 +91,3 @@ module Gitlab
end
end
end
-
-Gitlab::GlRepository::RepoType.prepend_if_ee('EE::Gitlab::GlRepository::RepoType')
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 262a7e227ab..900adb03a03 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -1695,6 +1695,9 @@ msgstr[1] ""
msgid "Alert Details"
msgstr ""
+msgid "AlertManagement|Acknowledged"
+msgstr ""
+
msgid "AlertManagement|Alert"
msgstr ""
@@ -1731,6 +1734,9 @@ msgstr ""
msgid "AlertManagement|Overview"
msgstr ""
+msgid "AlertManagement|Resolved"
+msgstr ""
+
msgid "AlertManagement|Severity"
msgstr ""
@@ -1749,6 +1755,9 @@ msgstr ""
msgid "AlertManagement|There was an error displaying the alerts. Confirm your endpoint's configuration details to ensure alerts appear."
msgstr ""
+msgid "AlertManagement|Triggered"
+msgstr ""
+
msgid "AlertService|%{linkStart}Learn more%{linkEnd} about configuring this endpoint to receive alerts."
msgstr ""
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 35b43afbcf3..a2df023fe4c 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -243,6 +243,91 @@ describe Projects::IssuesController do
end
end
+ describe '#related_branches' do
+ subject { get :related_branches, params: params, format: :json }
+
+ before do
+ sign_in(user)
+ project.add_developer(developer)
+ end
+
+ let(:developer) { user }
+ let(:params) do
+ {
+ namespace_id: project.namespace,
+ project_id: project,
+ id: issue.iid
+ }
+ end
+
+ context 'the current user cannot download code' do
+ it 'prevents access' do
+ allow(controller).to receive(:can?).with(any_args).and_return(true)
+ allow(controller).to receive(:can?).with(user, :download_code, project).and_return(false)
+
+ subject
+
+ expect(response).to have_gitlab_http_status(:not_found)
+ end
+ end
+
+ context 'there are no related branches' do
+ it 'assigns empty arrays', :aggregate_failures do
+ subject
+
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(assigns(:related_branches)).to be_empty
+ expect(response).to render_template('projects/issues/_related_branches')
+ expect(json_response).to eq('html' => '')
+ end
+ end
+
+ context 'there are related branches' do
+ let(:missing_branch) { "#{issue.to_branch_name}-missing" }
+ let(:unreadable_branch) { "#{issue.to_branch_name}-unreadable" }
+ let(:pipeline) { build(:ci_pipeline, :success, project: project) }
+ let(:master_branch) { 'master' }
+
+ let(:related_branches) do
+ [
+ branch_info(issue.to_branch_name, pipeline.detailed_status(user)),
+ branch_info(missing_branch, nil),
+ branch_info(unreadable_branch, nil)
+ ]
+ end
+
+ def branch_info(name, status)
+ {
+ name: name,
+ link: controller.project_compare_path(project, from: master_branch, to: name),
+ pipeline_status: status
+ }
+ end
+
+ before do
+ allow(controller).to receive(:find_routable!)
+ .with(Project, project.full_path, any_args).and_return(project)
+ allow(project).to receive(:default_branch).and_return(master_branch)
+ allow_next_instance_of(Issues::RelatedBranchesService) do |service|
+ allow(service).to receive(:execute).and_return(related_branches)
+ end
+ end
+
+ it 'finds and assigns the appropriate branch information', :aggregate_failures do
+ subject
+
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(assigns(:related_branches)).to contain_exactly(
+ branch_info(issue.to_branch_name, an_instance_of(Gitlab::Ci::Status::Success)),
+ branch_info(missing_branch, be_nil),
+ branch_info(unreadable_branch, be_nil)
+ )
+ expect(response).to render_template('projects/issues/_related_branches')
+ expect(json_response).to match('html' => String)
+ end
+ end
+ end
+
# This spec runs as a request-style spec in order to invoke the
# Rails router. A controller-style spec matches the wrong route, and
# session['user_return_to'] becomes incorrect.
diff --git a/spec/factories/design_management/actions.rb b/spec/factories/design_management/actions.rb
new file mode 100644
index 00000000000..e2561f98f52
--- /dev/null
+++ b/spec/factories/design_management/actions.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+ factory :design_action, class: 'DesignManagement::Action' do
+ design
+ association :version, factory: :design_version
+ event { :creation }
+
+ trait :with_image_v432x230 do
+ image_v432x230 { fixture_file_upload('spec/fixtures/dk.png') }
+ end
+ end
+end
diff --git a/spec/factories/design_management/design_at_version.rb b/spec/factories/design_management/design_at_version.rb
new file mode 100644
index 00000000000..b73df71595c
--- /dev/null
+++ b/spec/factories/design_management/design_at_version.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+ factory :design_at_version, class: 'DesignManagement::DesignAtVersion' do
+ skip_create # This is not an Active::Record model.
+
+ design { nil }
+
+ version { nil }
+
+ transient do
+ issue { design&.issue || version&.issue || create(:issue) }
+ end
+
+ initialize_with do
+ attrs = attributes.dup
+ attrs[:design] ||= create(:design, issue: issue)
+ attrs[:version] ||= create(:design_version, issue: issue)
+
+ new(attrs)
+ end
+ end
+end
diff --git a/spec/factories/design_management/designs.rb b/spec/factories/design_management/designs.rb
new file mode 100644
index 00000000000..59d4cc56f95
--- /dev/null
+++ b/spec/factories/design_management/designs.rb
@@ -0,0 +1,128 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+ factory :design, class: 'DesignManagement::Design' do
+ issue { create(:issue) }
+ project { issue&.project || create(:project) }
+ sequence(:filename) { |n| "homescreen-#{n}.jpg" }
+
+ transient do
+ author { issue.author }
+ end
+
+ trait :importing do
+ issue { nil }
+
+ importing { true }
+ imported { false }
+ end
+
+ trait :imported do
+ importing { false }
+ imported { true }
+ end
+
+ create_versions = ->(design, evaluator, commit_version) do
+ unless evaluator.versions_count.zero?
+ project = design.project
+ issue = design.issue
+ repository = project.design_repository
+ repository.create_if_not_exists
+ dv_table_name = DesignManagement::Action.table_name
+ updates = [0, evaluator.versions_count - (evaluator.deleted ? 2 : 1)].max
+
+ run_action = ->(action) do
+ sha = commit_version[action]
+ version = DesignManagement::Version.new(sha: sha, issue: issue, author: evaluator.author)
+ version.save(validate: false) # We need it to have an ID, validate later
+ Gitlab::Database.bulk_insert(dv_table_name, [action.row_attrs(version)])
+ end
+
+ # always a creation
+ run_action[DesignManagement::DesignAction.new(design, :create, evaluator.file)]
+
+ # 0 or more updates
+ updates.times do
+ run_action[DesignManagement::DesignAction.new(design, :update, evaluator.file)]
+ end
+
+ # and maybe a deletion
+ run_action[DesignManagement::DesignAction.new(design, :delete)] if evaluator.deleted
+ end
+
+ design.clear_version_cache
+ end
+
+ # Use this trait to build designs that are backed by Git LFS, committed
+ # to the repository, and with an LfsObject correctly created for it.
+ trait :with_lfs_file do
+ with_file
+
+ transient do
+ raw_file { fixture_file_upload('spec/fixtures/dk.png', 'image/png') }
+ lfs_pointer { Gitlab::Git::LfsPointerFile.new(SecureRandom.random_bytes) }
+ file { lfs_pointer.pointer }
+ end
+
+ after :create do |design, evaluator|
+ lfs_object = create(:lfs_object, file: evaluator.raw_file, oid: evaluator.lfs_pointer.sha256, size: evaluator.lfs_pointer.size)
+ create(:lfs_objects_project, project: design.project, lfs_object: lfs_object, repository_type: :design)
+ end
+ end
+
+ # Use this trait if you want versions in a particular history, but don't
+ # want to pay for gitlay calls.
+ trait :with_versions do
+ transient do
+ deleted { false }
+ versions_count { 1 }
+ sequence(:file) { |n| "some-file-content-#{n}" }
+ end
+
+ after :create do |design, evaluator|
+ counter = (1..).lazy
+
+ # Just produce a SHA by hashing the action and a monotonic counter
+ commit_version = ->(action) do
+ Digest::SHA1.hexdigest("#{action.gitaly_action}.#{counter.next}")
+ end
+
+ create_versions[design, evaluator, commit_version]
+ end
+ end
+
+ # Use this trait to build designs that have commits in the repository
+ # and files that can be retrieved.
+ trait :with_file do
+ transient do
+ deleted { false }
+ versions_count { 1 }
+ file { File.join(Rails.root, 'spec/fixtures/dk.png') }
+ end
+
+ after :create do |design, evaluator|
+ project = design.project
+ repository = project.design_repository
+
+ commit_version = ->(action) do
+ repository.multi_action(
+ evaluator.author,
+ branch_name: 'master',
+ message: "#{action.action} for #{design.filename}",
+ actions: [action.gitaly_action]
+ )
+ end
+
+ create_versions[design, evaluator, commit_version]
+ end
+ end
+
+ trait :with_smaller_image_versions do
+ with_lfs_file
+
+ after :create do |design|
+ design.versions.each { |v| DesignManagement::GenerateImageVersionsService.new(v).execute }
+ end
+ end
+ end
+end
diff --git a/spec/factories/design_management/versions.rb b/spec/factories/design_management/versions.rb
new file mode 100644
index 00000000000..878665e02e5
--- /dev/null
+++ b/spec/factories/design_management/versions.rb
@@ -0,0 +1,142 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+ factory :design_version, class: 'DesignManagement::Version' do
+ sequence(:sha) { |n| Digest::SHA1.hexdigest("commit-like-#{n}") }
+ issue { designs.first&.issue || create(:issue) }
+ author { issue&.author || create(:user) }
+
+ transient do
+ designs_count { 1 }
+ created_designs { [] }
+ modified_designs { [] }
+ deleted_designs { [] }
+ end
+
+ # Warning: this will intentionally result in an invalid version!
+ trait :empty do
+ designs_count { 0 }
+ end
+
+ trait :importing do
+ issue { nil }
+
+ designs_count { 0 }
+ importing { true }
+ imported { false }
+ end
+
+ trait :imported do
+ importing { false }
+ imported { true }
+ end
+
+ after(:build) do |version, evaluator|
+ # By default all designs are created_designs, so just add them.
+ specific_designs = [].concat(
+ evaluator.created_designs,
+ evaluator.modified_designs,
+ evaluator.deleted_designs
+ )
+ version.designs += specific_designs
+
+ unless evaluator.designs_count.zero? || version.designs.present?
+ version.designs << create(:design, issue: version.issue)
+ end
+ end
+
+ after :create do |version, evaluator|
+ # FactoryBot does not like methods, so we use lambdas instead
+ events = DesignManagement::Action.events
+
+ version.actions
+ .where(design_id: evaluator.modified_designs.map(&:id))
+ .update_all(event: events[:modification])
+
+ version.actions
+ .where(design_id: evaluator.deleted_designs.map(&:id))
+ .update_all(event: events[:deletion])
+
+ version.designs.reload
+ # Ensure version.issue == design.issue for all version.designs
+ version.designs.update_all(issue_id: version.issue_id)
+
+ needed = evaluator.designs_count
+ have = version.designs.size
+
+ create_list(:design, [0, needed - have].max, issue: version.issue).each do |d|
+ version.designs << d
+ end
+
+ version.actions.reset
+ end
+
+ # Use this trait to build versions with designs that are backed by Git LFS, committed
+ # to the repository, and with an LfsObject correctly created for it.
+ trait :with_lfs_file do
+ committed
+
+ transient do
+ raw_file { fixture_file_upload('spec/fixtures/dk.png', 'image/png') }
+ lfs_pointer { Gitlab::Git::LfsPointerFile.new(SecureRandom.random_bytes) }
+ file { lfs_pointer.pointer }
+ end
+
+ after :create do |version, evaluator|
+ lfs_object = create(:lfs_object, file: evaluator.raw_file, oid: evaluator.lfs_pointer.sha256, size: evaluator.lfs_pointer.size)
+ create(:lfs_objects_project, project: version.project, lfs_object: lfs_object, repository_type: :design)
+ end
+ end
+
+ # This trait is for versions that must be present in the git repository.
+ trait :committed do
+ transient do
+ file { File.join(Rails.root, 'spec/fixtures/dk.png') }
+ end
+
+ after :create do |version, evaluator|
+ project = version.issue.project
+ repository = project.design_repository
+ repository.create_if_not_exists
+
+ designs = version.designs_by_event
+ base_change = { content: evaluator.file }
+
+ actions = %w[modification deletion].flat_map { |k| designs.fetch(k, []) }.map do |design|
+ base_change.merge(action: :create, file_path: design.full_path)
+ end
+
+ if actions.present?
+ repository.multi_action(
+ evaluator.author,
+ branch_name: 'master',
+ message: "created #{actions.size} files",
+ actions: actions
+ )
+ end
+
+ mapping = {
+ 'creation' => :create,
+ 'modification' => :update,
+ 'deletion' => :delete
+ }
+
+ version_actions = designs.flat_map do |(event, designs)|
+ base = event == 'deletion' ? {} : base_change
+ designs.map do |design|
+ base.merge(action: mapping[event], file_path: design.full_path)
+ end
+ end
+
+ sha = repository.multi_action(
+ evaluator.author,
+ branch_name: 'master',
+ message: "edited #{version_actions.size} files",
+ actions: version_actions
+ )
+
+ version.update(sha: sha)
+ end
+ end
+ end
+end
diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb
index fdd1a9a18b2..0868e38f70e 100644
--- a/spec/factories/notes.rb
+++ b/spec/factories/notes.rb
@@ -107,6 +107,10 @@ FactoryBot.define do
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
association :project, :repository
noteable { nil }
@@ -136,6 +140,20 @@ FactoryBot.define do
project { nil }
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
system { true }
end
diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb
index 64321c9f319..45caa7a2b6a 100644
--- a/spec/factories/projects.rb
+++ b/spec/factories/projects.rb
@@ -215,6 +215,12 @@ FactoryBot.define do
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
transient do
remote_name { "remote_mirror_#{SecureRandom.hex}" }
diff --git a/spec/factories/uploads.rb b/spec/factories/uploads.rb
index a060cd7d6f8..b19af277cc3 100644
--- a/spec/factories/uploads.rb
+++ b/spec/factories/uploads.rb
@@ -65,5 +65,11 @@ FactoryBot.define do
model { create(:note) }
uploader { "AttachmentUploader" }
end
+
+ trait :design_action_image_v432x230_upload do
+ mount_point { :image_v432x230 }
+ model { create(:design_action) }
+ uploader { ::DesignManagement::DesignV432x230Uploader.name }
+ end
end
end
diff --git a/spec/frontend/alert_management/components/alert_management_list_spec.js b/spec/frontend/alert_management/components/alert_management_list_spec.js
index a47363f4dc7..e45a9042f71 100644
--- a/spec/frontend/alert_management/components/alert_management_list_spec.js
+++ b/spec/frontend/alert_management/components/alert_management_list_spec.js
@@ -1,5 +1,5 @@
import { mount } from '@vue/test-utils';
-import { GlEmptyState, GlTable, GlAlert, GlLoadingIcon } from '@gitlab/ui';
+import { GlEmptyState, GlTable, GlAlert, GlLoadingIcon, GlNewDropdown } from '@gitlab/ui';
import AlertManagementList from '~/alert_management/components/alert_management_list.vue';
import mockAlerts from '../mocks/alerts.json';
@@ -11,6 +11,7 @@ describe('AlertManagementList', () => {
const findAlerts = () => wrapper.findAll('table tbody tr');
const findAlert = () => wrapper.find(GlAlert);
const findLoader = () => wrapper.find(GlLoadingIcon);
+ const findStatusDropdown = () => wrapper.find(GlNewDropdown);
function mountComponent({
props = {
@@ -103,5 +104,14 @@ describe('AlertManagementList', () => {
expect(findAlertsTable().exists()).toBe(true);
expect(findAlerts()).toHaveLength(mockAlerts.length);
});
+
+ it('displays status dropdown', () => {
+ mountComponent({
+ props: { alertManagementEnabled: true, userCanEnableAlertManagement: true },
+ data: { alerts: mockAlerts, errored: false },
+ loading: false,
+ });
+ expect(findStatusDropdown().exists()).toBe(true);
+ });
});
});
diff --git a/spec/lib/gitlab/git_access_design_spec.rb b/spec/lib/gitlab/git_access_design_spec.rb
new file mode 100644
index 00000000000..b09afc67c90
--- /dev/null
+++ b/spec/lib/gitlab/git_access_design_spec.rb
@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Gitlab::GitAccessDesign do
+ include DesignManagementTestHelpers
+
+ let_it_be(:project) { create(:project) }
+ let_it_be(:user) { project.owner }
+ let(:protocol) { 'web' }
+ let(:actor) { user }
+
+ 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
+
+ context 'when the user is allowed to manage designs' do
+ # 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
+
+ context 'when the user is not allowed to manage designs' do
+ let_it_be(:user) { create(:user) }
+
+ it 'raises an error' do
+ expect { subject }.to raise_error(::Gitlab::GitAccess::ForbiddenError)
+ end
+ end
+
+ context 'when the protocol is not web' do
+ let(:protocol) { 'https' }
+
+ it 'raises an error' do
+ expect { subject }.to raise_error(::Gitlab::GitAccess::ForbiddenError)
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/gl_repository/repo_type_spec.rb b/spec/lib/gitlab/gl_repository/repo_type_spec.rb
index 6185b068d4c..bf6df55b71e 100644
--- a/spec/lib/gitlab/gl_repository/repo_type_spec.rb
+++ b/spec/lib/gitlab/gl_repository/repo_type_spec.rb
@@ -7,6 +7,7 @@ describe Gitlab::GlRepository::RepoType do
let_it_be(:project_snippet) { create(:project_snippet, project: project, author: project.owner) }
let(:project_path) { project.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(:project_snippet_path) { "#{project.full_path}/snippets/#{project_snippet.id}" }
@@ -24,6 +25,7 @@ describe Gitlab::GlRepository::RepoType do
expect(described_class).not_to be_wiki
expect(described_class).to be_project
expect(described_class).not_to be_snippet
+ expect(described_class).not_to be_design
end
end
@@ -33,6 +35,7 @@ describe Gitlab::GlRepository::RepoType do
expect(described_class.valid?(wiki_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?(design_path)).to be_truthy
end
end
end
@@ -51,6 +54,7 @@ describe Gitlab::GlRepository::RepoType do
expect(described_class).to be_wiki
expect(described_class).not_to be_project
expect(described_class).not_to be_snippet
+ expect(described_class).not_to be_design
end
end
@@ -60,6 +64,7 @@ describe Gitlab::GlRepository::RepoType do
expect(described_class.valid?(wiki_path)).to be_truthy
expect(described_class.valid?(personal_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
@@ -79,6 +84,7 @@ describe Gitlab::GlRepository::RepoType do
expect(described_class).to be_snippet
expect(described_class).not_to be_wiki
expect(described_class).not_to be_project
+ expect(described_class).not_to be_design
end
end
@@ -88,6 +94,7 @@ describe Gitlab::GlRepository::RepoType do
expect(described_class.valid?(wiki_path)).to be_falsey
expect(described_class.valid?(personal_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
@@ -115,8 +122,38 @@ describe Gitlab::GlRepository::RepoType do
expect(described_class.valid?(wiki_path)).to be_falsey
expect(described_class.valid?(personal_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
+
+ 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
diff --git a/spec/lib/gitlab/gl_repository_spec.rb b/spec/lib/gitlab/gl_repository_spec.rb
index 858f436047e..5f5244b7116 100644
--- a/spec/lib/gitlab/gl_repository_spec.rb
+++ b/spec/lib/gitlab/gl_repository_spec.rb
@@ -19,6 +19,10 @@ describe ::Gitlab::GlRepository do
expect(described_class.parse("snippet-#{snippet.id}")).to eq([snippet, nil, Gitlab::GlRepository::SNIPPET])
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
expect { described_class.parse("badformat-#{project.id}") }.to raise_error(ArgumentError)
end
@@ -27,4 +31,15 @@ describe ::Gitlab::GlRepository do
expect { described_class.parse("project-foo") }.to raise_error(ArgumentError)
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
diff --git a/spec/models/design_management/action_spec.rb b/spec/models/design_management/action_spec.rb
new file mode 100644
index 00000000000..753c31b1549
--- /dev/null
+++ b/spec/models/design_management/action_spec.rb
@@ -0,0 +1,105 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe DesignManagement::Action do
+ describe 'relations' do
+ it { is_expected.to belong_to(:design) }
+ it { is_expected.to belong_to(:version) }
+ end
+
+ describe 'scopes' do
+ describe '.most_recent' do
+ let_it_be(:design_a) { create(:design) }
+ let_it_be(:design_b) { create(:design) }
+ let_it_be(:design_c) { create(:design) }
+
+ let(:designs) { [design_a, design_b, design_c] }
+
+ before_all do
+ create(:design_version, designs: [design_a, design_b, design_c])
+ create(:design_version, designs: [design_a, design_b])
+ create(:design_version, designs: [design_a])
+ end
+
+ it 'finds the correct version for each design' do
+ dvs = described_class.where(design: designs)
+
+ expected = designs
+ .map(&:id)
+ .zip(dvs.order("version_id DESC").pluck(:version_id).uniq)
+
+ actual = dvs.most_recent.map { |dv| [dv.design_id, dv.version_id] }
+
+ expect(actual).to eq(expected)
+ end
+ end
+
+ describe '.up_to_version' do
+ let_it_be(:issue) { create(:issue) }
+ let_it_be(:design_a) { create(:design, issue: issue) }
+ let_it_be(:design_b) { create(:design, issue: issue) }
+
+ # let bindings are not available in before(:all) contexts,
+ # so we need to redefine the array on each construction.
+ let_it_be(:oldest) { create(:design_version, designs: [design_a, design_b]) }
+ let_it_be(:middle) { create(:design_version, designs: [design_a, design_b]) }
+ let_it_be(:newest) { create(:design_version, designs: [design_a, design_b]) }
+
+ subject { described_class.where(design: issue.designs).up_to_version(version) }
+
+ context 'the version is nil' do
+ let(:version) { nil }
+
+ it 'returns all design_versions' do
+ is_expected.to have_attributes(size: 6)
+ end
+ end
+
+ context 'when given a Version instance' do
+ context 'the version is the most current' do
+ let(:version) { newest }
+
+ it { is_expected.to have_attributes(size: 6) }
+ end
+
+ context 'the version is the oldest' do
+ let(:version) { oldest }
+
+ it { is_expected.to have_attributes(size: 2) }
+ end
+
+ context 'the version is the middle one' do
+ let(:version) { middle }
+
+ it { is_expected.to have_attributes(size: 4) }
+ end
+ end
+
+ context 'when given a commit SHA' do
+ context 'the version is the most current' do
+ let(:version) { newest.sha }
+
+ it { is_expected.to have_attributes(size: 6) }
+ end
+
+ context 'the version is the oldest' do
+ let(:version) { oldest.sha }
+
+ it { is_expected.to have_attributes(size: 2) }
+ end
+
+ context 'the version is the middle one' do
+ let(:version) { middle.sha }
+
+ it { is_expected.to have_attributes(size: 4) }
+ end
+ end
+
+ context 'when given a String that is not a commit SHA' do
+ let(:version) { 'foo' }
+
+ it { expect { subject }.to raise_error(ArgumentError) }
+ end
+ end
+ end
+end
diff --git a/spec/models/design_management/design_action_spec.rb b/spec/models/design_management/design_action_spec.rb
new file mode 100644
index 00000000000..da4ad41dfcb
--- /dev/null
+++ b/spec/models/design_management/design_action_spec.rb
@@ -0,0 +1,98 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe DesignManagement::DesignAction do
+ describe 'validations' do
+ describe 'the design' do
+ let(:fail_validation) { raise_error(/design/i) }
+
+ it 'must not be nil' do
+ expect { described_class.new(nil, :create, :foo) }.to fail_validation
+ end
+ end
+
+ describe 'the action' do
+ let(:fail_validation) { raise_error(/action/i) }
+
+ it 'must not be nil' do
+ expect { described_class.new(double, nil, :foo) }.to fail_validation
+ end
+
+ it 'must be a known action' do
+ expect { described_class.new(double, :wibble, :foo) }.to fail_validation
+ end
+ end
+
+ describe 'the content' do
+ context 'content is necesary' do
+ let(:fail_validation) { raise_error(/needs content/i) }
+
+ %i[create update].each do |action|
+ it "must not be nil if the action is #{action}" do
+ expect { described_class.new(double, action, nil) }.to fail_validation
+ end
+ end
+ end
+
+ context 'content is forbidden' do
+ let(:fail_validation) { raise_error(/forbids content/i) }
+
+ it "must not be nil if the action is delete" do
+ expect { described_class.new(double, :delete, :foo) }.to fail_validation
+ end
+ end
+ end
+ end
+
+ describe '#gitaly_action' do
+ let(:path) { 'some/path/somewhere' }
+ let(:design) { OpenStruct.new(full_path: path) }
+
+ subject { described_class.new(design, action, content) }
+
+ context 'the action needs content' do
+ let(:action) { :create }
+ let(:content) { :foo }
+
+ it 'produces a good gitaly action' do
+ expect(subject.gitaly_action).to eq(
+ action: action,
+ file_path: path,
+ content: content
+ )
+ end
+ end
+
+ context 'the action forbids content' do
+ let(:action) { :delete }
+ let(:content) { nil }
+
+ it 'produces a good gitaly action' do
+ expect(subject.gitaly_action).to eq(action: action, file_path: path)
+ end
+ end
+ end
+
+ describe '#issue_id' do
+ let(:issue_id) { :foo }
+ let(:design) { OpenStruct.new(issue_id: issue_id) }
+
+ subject { described_class.new(design, :delete) }
+
+ it 'delegates to the design' do
+ expect(subject.issue_id).to eq(issue_id)
+ end
+ end
+
+ describe '#performed' do
+ let(:design) { double }
+
+ subject { described_class.new(design, :delete) }
+
+ it 'calls design#clear_version_cache when the action has been performed' do
+ expect(design).to receive(:clear_version_cache)
+
+ subject.performed
+ end
+ end
+end
diff --git a/spec/models/design_management/design_at_version_spec.rb b/spec/models/design_management/design_at_version_spec.rb
new file mode 100644
index 00000000000..f6fa8df243c
--- /dev/null
+++ b/spec/models/design_management/design_at_version_spec.rb
@@ -0,0 +1,426 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe DesignManagement::DesignAtVersion do
+ include DesignManagementTestHelpers
+
+ let_it_be(:issue, reload: true) { create(:issue) }
+ let_it_be(:issue_b, reload: true) { create(:issue) }
+ let_it_be(:design, reload: true) { create(:design, issue: issue) }
+ let_it_be(:version) { create(:design_version, designs: [design]) }
+
+ describe '#id' do
+ subject { described_class.new(design: design, version: version) }
+
+ it 'combines design.id and version.id' do
+ expect(subject.id).to include(design.id.to_s, version.id.to_s)
+ end
+ end
+
+ describe '#==' do
+ it 'identifies objects created with the same parameters as equal' do
+ design = build_stubbed(:design, issue: issue)
+ version = build_stubbed(:design_version, designs: [design], issue: issue)
+
+ this = build_stubbed(:design_at_version, design: design, version: version)
+ other = build_stubbed(:design_at_version, design: design, version: version)
+
+ expect(this).to eq(other)
+ expect(other).to eq(this)
+ end
+
+ it 'identifies unequal objects as unequal, by virtue of their version' do
+ design = build_stubbed(:design, issue: issue)
+ version_a = build_stubbed(:design_version, designs: [design])
+ version_b = build_stubbed(:design_version, designs: [design])
+
+ this = build_stubbed(:design_at_version, design: design, version: version_a)
+ other = build_stubbed(:design_at_version, design: design, version: version_b)
+
+ expect(this).not_to eq(nil)
+ expect(this).not_to eq(design)
+ expect(this).not_to eq(other)
+ expect(other).not_to eq(this)
+ end
+
+ it 'identifies unequal objects as unequal, by virtue of their design' do
+ design_a = build_stubbed(:design, issue: issue)
+ design_b = build_stubbed(:design, issue: issue)
+ version = build_stubbed(:design_version, designs: [design_a, design_b])
+
+ this = build_stubbed(:design_at_version, design: design_a, version: version)
+ other = build_stubbed(:design_at_version, design: design_b, version: version)
+
+ expect(this).not_to eq(other)
+ expect(other).not_to eq(this)
+ end
+
+ it 'rejects objects with the same id and the wrong class' do
+ dav = build_stubbed(:design_at_version)
+
+ expect(dav).not_to eq(OpenStruct.new(id: dav.id))
+ end
+
+ it 'expects objects to be of the same type, not subtypes' do
+ subtype = Class.new(described_class)
+ dav = build_stubbed(:design_at_version)
+ other = subtype.new(design: dav.design, version: dav.version)
+
+ expect(dav).not_to eq(other)
+ end
+ end
+
+ describe 'status methods' do
+ let!(:design_a) { create(:design, issue: issue) }
+ let!(:design_b) { create(:design, issue: issue) }
+
+ let!(:version_a) do
+ create(:design_version, designs: [design_a])
+ end
+ let!(:version_b) do
+ create(:design_version, designs: [design_b])
+ end
+ let!(:version_mod) do
+ create(:design_version, modified_designs: [design_a, design_b])
+ end
+ let!(:version_c) do
+ create(:design_version, deleted_designs: [design_a])
+ end
+ let!(:version_d) do
+ create(:design_version, deleted_designs: [design_b])
+ end
+ let!(:version_e) do
+ create(:design_version, designs: [design_a])
+ end
+
+ describe 'a design before it has been created' do
+ subject { build(:design_at_version, design: design_b, version: version_a) }
+
+ it 'is not deleted' do
+ expect(subject).not_to be_deleted
+ end
+
+ it 'has the status :not_created_yet' do
+ expect(subject).to have_attributes(status: :not_created_yet)
+ end
+ end
+
+ describe 'a design as of its creation' do
+ subject { build(:design_at_version, design: design_a, version: version_a) }
+
+ it 'is not deleted' do
+ expect(subject).not_to be_deleted
+ end
+
+ it 'has the status :current' do
+ expect(subject).to have_attributes(status: :current)
+ end
+ end
+
+ describe 'a design after it has been created, but before deletion' do
+ subject { build(:design_at_version, design: design_b, version: version_c) }
+
+ it 'is not deleted' do
+ expect(subject).not_to be_deleted
+ end
+
+ it 'has the status :current' do
+ expect(subject).to have_attributes(status: :current)
+ end
+ end
+
+ describe 'a design as of its modification' do
+ subject { build(:design_at_version, design: design_a, version: version_mod) }
+
+ it 'is not deleted' do
+ expect(subject).not_to be_deleted
+ end
+
+ it 'has the status :current' do
+ expect(subject).to have_attributes(status: :current)
+ end
+ end
+
+ describe 'a design as of its deletion' do
+ subject { build(:design_at_version, design: design_a, version: version_c) }
+
+ it 'is deleted' do
+ expect(subject).to be_deleted
+ end
+
+ it 'has the status :deleted' do
+ expect(subject).to have_attributes(status: :deleted)
+ end
+ end
+
+ describe 'a design after its deletion' do
+ subject { build(:design_at_version, design: design_b, version: version_e) }
+
+ it 'is deleted' do
+ expect(subject).to be_deleted
+ end
+
+ it 'has the status :deleted' do
+ expect(subject).to have_attributes(status: :deleted)
+ end
+ end
+
+ describe 'a design on its recreation' do
+ subject { build(:design_at_version, design: design_a, version: version_e) }
+
+ it 'is not deleted' do
+ expect(subject).not_to be_deleted
+ end
+
+ it 'has the status :current' do
+ expect(subject).to have_attributes(status: :current)
+ end
+ end
+ end
+
+ describe 'validations' do
+ subject(:design_at_version) { build(:design_at_version) }
+
+ it { is_expected.to be_valid }
+
+ describe 'a design-at-version without a design' do
+ subject { described_class.new(design: nil, version: build(:design_version)) }
+
+ it { is_expected.to be_invalid }
+
+ it 'mentions the design in the errors' do
+ subject.valid?
+
+ expect(subject.errors[:design]).to be_present
+ end
+ end
+
+ describe 'a design-at-version without a version' do
+ subject { described_class.new(design: build(:design), version: nil) }
+
+ it { is_expected.to be_invalid }
+
+ it 'mentions the version in the errors' do
+ subject.valid?
+
+ expect(subject.errors[:version]).to be_present
+ end
+ end
+
+ describe 'design_and_version_belong_to_the_same_issue' do
+ context 'both design and version are supplied' do
+ subject(:design_at_version) { build(:design_at_version, design: design, version: version) }
+
+ context 'the design belongs to the same issue as the version' do
+ it { is_expected.to be_valid }
+ end
+
+ context 'the design does not belong to the same issue as the version' do
+ let(:design) { create(:design) }
+ let(:version) { create(:design_version) }
+
+ it { is_expected.to be_invalid }
+ end
+ end
+
+ context 'the factory is just supplied with a design' do
+ let(:design) { create(:design) }
+
+ subject(:design_at_version) { build(:design_at_version, design: design) }
+
+ it { is_expected.to be_valid }
+ end
+
+ context 'the factory is just supplied with a version' do
+ let(:version) { create(:design_version) }
+
+ subject(:design_at_version) { build(:design_at_version, version: version) }
+
+ it { is_expected.to be_valid }
+ end
+ end
+
+ describe 'design_and_version_have_issue_id' do
+ subject(:design_at_version) { build(:design_at_version, design: design, version: version) }
+
+ context 'the design has no issue_id, because it is being imported' do
+ let(:design) { create(:design, :importing) }
+
+ it { is_expected.to be_invalid }
+ end
+
+ context 'the version has no issue_id, because it is being imported' do
+ let(:version) { create(:design_version, :importing) }
+
+ it { is_expected.to be_invalid }
+ end
+
+ context 'both the design and the version are being imported' do
+ let(:version) { create(:design_version, :importing) }
+ let(:design) { create(:design, :importing) }
+
+ it { is_expected.to be_invalid }
+ end
+ end
+ end
+
+ def id_of(design, version)
+ build(:design_at_version, design: design, version: version).id
+ end
+
+ describe '.instantiate' do
+ context 'when attrs are valid' do
+ subject do
+ described_class.instantiate(design: design, version: version)
+ end
+
+ it { is_expected.to be_a(described_class).and(be_valid) }
+ end
+
+ context 'when attrs are invalid' do
+ subject do
+ described_class.instantiate(
+ design: create(:design),
+ version: create(:design_version)
+ )
+ end
+
+ it 'raises a validation error' do
+ expect { subject }.to raise_error(ActiveModel::ValidationError)
+ end
+ end
+ end
+
+ describe '.lazy_find' do
+ let!(:version_a) do
+ create(:design_version, designs: create_list(:design, 3, issue: issue))
+ end
+ let!(:version_b) do
+ create(:design_version, designs: create_list(:design, 1, issue: issue))
+ end
+ let!(:version_c) do
+ create(:design_version, designs: create_list(:design, 1, issue: issue_b))
+ end
+
+ let(:id_a) { id_of(version_a.designs.first, version_a) }
+ let(:id_b) { id_of(version_a.designs.second, version_a) }
+ let(:id_c) { id_of(version_a.designs.last, version_a) }
+ let(:id_d) { id_of(version_b.designs.first, version_b) }
+ let(:id_e) { id_of(version_c.designs.first, version_c) }
+ let(:bad_id) { id_of(version_c.designs.first, version_a) }
+
+ def find(the_id)
+ described_class.lazy_find(the_id)
+ end
+
+ let(:db_calls) { 2 }
+
+ it 'issues fewer queries than the naive approach would' do
+ expect do
+ dav_a = find(id_a)
+ dav_b = find(id_b)
+ dav_c = find(id_c)
+ dav_d = find(id_d)
+ dav_e = find(id_e)
+ should_not_exist = find(bad_id)
+
+ expect(dav_a.version).to eq(version_a)
+ expect(dav_b.version).to eq(version_a)
+ expect(dav_c.version).to eq(version_a)
+ expect(dav_d.version).to eq(version_b)
+ expect(dav_e.version).to eq(version_c)
+ expect(should_not_exist).not_to be_present
+
+ expect(version_a.designs).to include(dav_a.design, dav_b.design, dav_c.design)
+ expect(version_b.designs).to include(dav_d.design)
+ expect(version_c.designs).to include(dav_e.design)
+ end.not_to exceed_query_limit(db_calls)
+ end
+ end
+
+ describe '.find' do
+ let(:results) { described_class.find(ids) }
+
+ # 2 versions, with 5 total designs on issue A, so 2*5 = 10
+ let!(:version_a) do
+ create(:design_version, designs: create_list(:design, 3, issue: issue))
+ end
+ let!(:version_b) do
+ create(:design_version, designs: create_list(:design, 2, issue: issue))
+ end
+ # 1 version, with 3 designs on issue B, so 1*3 = 3
+ let!(:version_c) do
+ create(:design_version, designs: create_list(:design, 3, issue: issue_b))
+ end
+
+ context 'invalid ids' do
+ let(:ids) do
+ version_b.designs.map { |d| id_of(d, version_c) }
+ end
+
+ describe '#count' do
+ it 'counts 0 records' do
+ expect(results.count).to eq(0)
+ end
+ end
+
+ describe '#empty?' do
+ it 'is empty' do
+ expect(results).to be_empty
+ end
+ end
+
+ describe '#to_a' do
+ it 'finds no records' do
+ expect(results.to_a).to eq([])
+ end
+ end
+ end
+
+ context 'valid ids' do
+ let(:red_herrings) { issue_b.designs.sample(2).map { |d| id_of(d, version_a) } }
+
+ let(:ids) do
+ a_ids = issue.designs.sample(2).map { |d| id_of(d, version_a) }
+ b_ids = issue.designs.sample(2).map { |d| id_of(d, version_b) }
+ c_ids = issue_b.designs.sample(2).map { |d| id_of(d, version_c) }
+
+ a_ids + b_ids + c_ids + red_herrings
+ end
+
+ before do
+ ids.size # force IDs
+ end
+
+ describe '#count' do
+ it 'counts 2 records' do
+ expect(results.count).to eq(6)
+ end
+
+ it 'issues at most two queries' do
+ expect { results.count }.not_to exceed_query_limit(2)
+ end
+ end
+
+ describe '#to_a' do
+ it 'finds 6 records' do
+ expect(results.size).to eq(6)
+ expect(results).to all(be_a(described_class))
+ end
+
+ it 'only returns records with matching IDs' do
+ expect(results.map(&:id)).to match_array(ids - red_herrings)
+ end
+
+ it 'only returns valid records' do
+ expect(results).to all(be_valid)
+ end
+
+ it 'issues at most two queries' do
+ expect { results.to_a }.not_to exceed_query_limit(2)
+ end
+ end
+ end
+ end
+end
diff --git a/spec/models/design_management/design_collection_spec.rb b/spec/models/design_management/design_collection_spec.rb
new file mode 100644
index 00000000000..bd48f742042
--- /dev/null
+++ b/spec/models/design_management/design_collection_spec.rb
@@ -0,0 +1,82 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe DesignManagement::DesignCollection do
+ include DesignManagementTestHelpers
+
+ let_it_be(:issue, reload: true) { create(:issue) }
+
+ subject(:collection) { described_class.new(issue) }
+
+ describe ".find_or_create_design!" do
+ it "finds an existing design" do
+ design = create(:design, issue: issue, filename: 'world.png')
+
+ expect(collection.find_or_create_design!(filename: 'world.png')).to eq(design)
+ end
+
+ it "creates a new design if one didn't exist" do
+ expect(issue.designs.size).to eq(0)
+
+ new_design = collection.find_or_create_design!(filename: 'world.png')
+
+ expect(issue.designs.size).to eq(1)
+ expect(new_design.filename).to eq('world.png')
+ expect(new_design.issue).to eq(issue)
+ end
+
+ it "only queries the designs once" do
+ create(:design, issue: issue, filename: 'hello.png')
+ create(:design, issue: issue, filename: 'world.jpg')
+
+ expect do
+ collection.find_or_create_design!(filename: 'hello.png')
+ collection.find_or_create_design!(filename: 'world.jpg')
+ end.not_to exceed_query_limit(1)
+ end
+ end
+
+ describe "#versions" do
+ it "includes versions for all designs" do
+ version_1 = create(:design_version)
+ version_2 = create(:design_version)
+ other_version = create(:design_version)
+ create(:design, issue: issue, versions: [version_1])
+ create(:design, issue: issue, versions: [version_2])
+ create(:design, versions: [other_version])
+
+ expect(collection.versions).to contain_exactly(version_1, version_2)
+ end
+ end
+
+ describe "#repository" do
+ it "builds a design repository" do
+ expect(collection.repository).to be_a(DesignManagement::Repository)
+ end
+ end
+
+ describe '#designs_by_filename' do
+ let(:designs) { create_list(:design, 5, :with_file, issue: issue) }
+ let(:filenames) { designs.map(&:filename) }
+ let(:query) { subject.designs_by_filename(filenames) }
+
+ it 'finds all the designs with those filenames on this issue' do
+ expect(query).to have_attributes(size: 5)
+ end
+
+ it 'only makes a single query' do
+ designs.each(&:id)
+ expect { query }.not_to exceed_query_limit(1)
+ end
+
+ context 'some are deleted' do
+ before do
+ delete_designs(*designs.sample(2))
+ end
+
+ it 'takes deletion into account' do
+ expect(query).to have_attributes(size: 3)
+ end
+ end
+ end
+end
diff --git a/spec/models/design_management/design_spec.rb b/spec/models/design_management/design_spec.rb
new file mode 100644
index 00000000000..555b7f04f86
--- /dev/null
+++ b/spec/models/design_management/design_spec.rb
@@ -0,0 +1,582 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe DesignManagement::Design do
+ include DesignManagementTestHelpers
+
+ let_it_be(:issue) { create(:issue) }
+ let_it_be(:design1) { create(:design, :with_versions, issue: issue, versions_count: 1) }
+ let_it_be(:design2) { create(:design, :with_versions, issue: issue, versions_count: 1) }
+ let_it_be(:design3) { create(:design, :with_versions, issue: issue, versions_count: 1) }
+ let_it_be(:deleted_design) { create(:design, :with_versions, deleted: true) }
+
+ describe 'relations' do
+ it { is_expected.to belong_to(:project) }
+ it { is_expected.to belong_to(:issue) }
+ it { is_expected.to have_many(:actions) }
+ it { is_expected.to have_many(:versions) }
+ it { is_expected.to have_many(:notes).dependent(:delete_all) }
+ it { is_expected.to have_many(:user_mentions) }
+ end
+
+ describe 'validations' do
+ subject(:design) { build(:design) }
+
+ it { is_expected.to be_valid }
+ it { is_expected.to validate_presence_of(:project) }
+ it { is_expected.to validate_presence_of(:issue) }
+ it { is_expected.to validate_presence_of(:filename) }
+ it { is_expected.to validate_uniqueness_of(:filename).scoped_to(:issue_id) }
+
+ it "validates that the extension is an image" do
+ design.filename = "thing.txt"
+ extensions = described_class::SAFE_IMAGE_EXT + described_class::DANGEROUS_IMAGE_EXT
+
+ expect(design).not_to be_valid
+ expect(design.errors[:filename].first).to eq(
+ "does not have a supported extension. Only #{extensions.to_sentence} are supported"
+ )
+ end
+
+ describe 'validating files with .svg extension' do
+ before do
+ design.filename = "thing.svg"
+ end
+
+ it "allows .svg files when feature flag is enabled" do
+ stub_feature_flags(design_management_allow_dangerous_images: true)
+
+ expect(design).to be_valid
+ end
+
+ it "does not allow .svg files when feature flag is disabled" do
+ stub_feature_flags(design_management_allow_dangerous_images: false)
+
+ expect(design).not_to be_valid
+ expect(design.errors[:filename].first).to eq(
+ "does not have a supported extension. Only #{described_class::SAFE_IMAGE_EXT.to_sentence} are supported"
+ )
+ end
+ end
+ end
+
+ describe 'scopes' do
+ describe '.visible_at_version' do
+ let(:versions) { DesignManagement::Version.where(issue: issue).ordered }
+ let(:found) { described_class.visible_at_version(version) }
+
+ context 'at oldest version' do
+ let(:version) { versions.last }
+
+ it 'finds the first design only' do
+ expect(found).to contain_exactly(design1)
+ end
+ end
+
+ context 'at version 2' do
+ let(:version) { versions.second }
+
+ it 'finds the first and second designs' do
+ expect(found).to contain_exactly(design1, design2)
+ end
+ end
+
+ context 'at latest version' do
+ let(:version) { versions.first }
+
+ it 'finds designs' do
+ expect(found).to contain_exactly(design1, design2, design3)
+ end
+ end
+
+ context 'when the argument is nil' do
+ let(:version) { nil }
+
+ it 'finds all undeleted designs' do
+ expect(found).to contain_exactly(design1, design2, design3)
+ end
+ end
+
+ describe 'one of the designs was deleted before the given version' do
+ before do
+ delete_designs(design2)
+ end
+
+ it 'is not returned' do
+ current_version = versions.first
+
+ expect(described_class.visible_at_version(current_version)).to contain_exactly(design1, design3)
+ end
+ end
+
+ context 'a re-created history' do
+ before do
+ delete_designs(design1, design2)
+ restore_designs(design1)
+ end
+
+ it 'is returned, though other deleted events are not' do
+ expect(described_class.visible_at_version(nil)).to contain_exactly(design1, design3)
+ end
+ end
+
+ # test that a design that has been modified at various points
+ # can be queried for correctly at different points in its history
+ describe 'dead or alive' do
+ let(:versions) { DesignManagement::Version.where(issue: issue).map { |v| [v, :alive] } }
+
+ before do
+ versions << [delete_designs(design1), :dead]
+ versions << [modify_designs(design2), :dead]
+ versions << [restore_designs(design1), :alive]
+ versions << [modify_designs(design3), :alive]
+ versions << [delete_designs(design1), :dead]
+ versions << [modify_designs(design2, design3), :dead]
+ versions << [restore_designs(design1), :alive]
+ end
+
+ it 'can establish the history at any point' do
+ history = versions.map(&:first).map do |v|
+ described_class.visible_at_version(v).include?(design1) ? :alive : :dead
+ end
+
+ expect(history).to eq(versions.map(&:second))
+ end
+ end
+ end
+
+ describe '.with_filename' do
+ it 'returns correct design when passed a single filename' do
+ expect(described_class.with_filename(design1.filename)).to eq([design1])
+ end
+
+ it 'returns correct designs when passed an Array of filenames' do
+ expect(
+ described_class.with_filename([design1, design2].map(&:filename))
+ ).to contain_exactly(design1, design2)
+ end
+ end
+
+ describe '.on_issue' do
+ it 'returns correct designs when passed a single issue' do
+ expect(described_class.on_issue(issue)).to match_array(issue.designs)
+ end
+
+ it 'returns correct designs when passed an Array of issues' do
+ expect(
+ described_class.on_issue([issue, deleted_design.issue])
+ ).to contain_exactly(design1, design2, design3, deleted_design)
+ end
+ end
+
+ describe '.current' do
+ it 'returns just the undeleted designs' do
+ delete_designs(design3)
+
+ expect(described_class.current).to contain_exactly(design1, design2)
+ end
+ end
+ end
+
+ describe '#visible_in?' do
+ let_it_be(:issue) { create(:issue) }
+
+ # It is expensive to re-create complex histories, so we do it once, and then
+ # assert that we can establish visibility at any given version.
+ it 'tells us when a design is visible' do
+ expected = []
+
+ first_design = create(:design, :with_versions, issue: issue, versions_count: 1)
+ prior_to_creation = first_design.versions.first
+ expected << [prior_to_creation, :not_created_yet, false]
+
+ v = modify_designs(first_design)
+ expected << [v, :not_created_yet, false]
+
+ design = create(:design, :with_versions, issue: issue, versions_count: 1)
+ created_in = design.versions.first
+ expected << [created_in, :created, true]
+
+ # The future state should not affect the result for any state, so we
+ # ensure that most states have a long future as well as a rich past
+ 2.times do
+ v = modify_designs(first_design)
+ expected << [v, :unaffected_visible, true]
+
+ v = modify_designs(design)
+ expected << [v, :modified, true]
+
+ v = modify_designs(first_design)
+ expected << [v, :unaffected_visible, true]
+
+ v = delete_designs(design)
+ expected << [v, :deleted, false]
+
+ v = modify_designs(first_design)
+ expected << [v, :unaffected_nv, false]
+
+ v = restore_designs(design)
+ expected << [v, :restored, true]
+ end
+
+ delete_designs(design) # ensure visibility is not corelated with current state
+
+ got = expected.map do |(v, sym, _)|
+ [v, sym, design.visible_in?(v)]
+ end
+
+ expect(got).to eq(expected)
+ end
+ end
+
+ describe '#to_ability_name' do
+ it { expect(described_class.new.to_ability_name).to eq('design') }
+ end
+
+ describe '#status' do
+ context 'the design is new' do
+ subject { build(:design) }
+
+ it { is_expected.to have_attributes(status: :new) }
+ end
+
+ context 'the design is current' do
+ subject { design1 }
+
+ it { is_expected.to have_attributes(status: :current) }
+ end
+
+ context 'the design has been deleted' do
+ subject { deleted_design }
+
+ it { is_expected.to have_attributes(status: :deleted) }
+ end
+ end
+
+ describe '#deleted?' do
+ context 'the design is new' do
+ let(:design) { build(:design) }
+
+ it 'is falsy' do
+ expect(design).not_to be_deleted
+ end
+ end
+
+ context 'the design is current' do
+ let(:design) { design1 }
+
+ it 'is falsy' do
+ expect(design).not_to be_deleted
+ end
+ end
+
+ context 'the design has been deleted' do
+ let(:design) { deleted_design }
+
+ it 'is truthy' do
+ expect(design).to be_deleted
+ end
+ end
+
+ context 'the design has been deleted, but was then re-created' do
+ let(:design) { create(:design, :with_versions, versions_count: 1, deleted: true) }
+
+ it 'is falsy' do
+ restore_designs(design)
+
+ expect(design).not_to be_deleted
+ end
+ end
+ end
+
+ describe "#new_design?" do
+ let(:design) { design1 }
+
+ it "is false when there are versions" do
+ expect(design1).not_to be_new_design
+ end
+
+ it "is true when there are no versions" do
+ expect(build(:design)).to be_new_design
+ end
+
+ it 'is false for deleted designs' do
+ expect(deleted_design).not_to be_new_design
+ end
+
+ it "does not cause extra queries when actions are loaded" do
+ design.actions.map(&:id)
+
+ expect { design.new_design? }.not_to exceed_query_limit(0)
+ end
+
+ it "implicitly caches values" do
+ expect do
+ design.new_design?
+ design.new_design?
+ end.not_to exceed_query_limit(1)
+ end
+
+ it "queries again when the clear_version_cache trigger has been called" do
+ expect do
+ design.new_design?
+ design.clear_version_cache
+ design.new_design?
+ end.not_to exceed_query_limit(2)
+ end
+
+ it "causes a single query when there versions are not loaded" do
+ design.reload
+
+ expect { design.new_design? }.not_to exceed_query_limit(1)
+ end
+ end
+
+ describe "#full_path" do
+ it "builds the full path for a design" do
+ design = build(:design, filename: "hello.jpg")
+ expected_path = "#{DesignManagement.designs_directory}/issue-#{design.issue.iid}/hello.jpg"
+
+ expect(design.full_path).to eq(expected_path)
+ end
+ end
+
+ describe '#diff_refs' do
+ let(:design) { create(:design, :with_file, versions_count: versions_count) }
+
+ context 'there are several versions' do
+ let(:versions_count) { 3 }
+
+ it "builds diff refs based on the first commit and it's for the design" do
+ expect(design.diff_refs.base_sha).to eq(design.versions.ordered.second.sha)
+ expect(design.diff_refs.head_sha).to eq(design.versions.ordered.first.sha)
+ end
+ end
+
+ context 'there is just one version' do
+ let(:versions_count) { 1 }
+
+ it 'builds diff refs based on the empty tree if there was only one version' do
+ design = create(:design, :with_file, versions_count: 1)
+
+ expect(design.diff_refs.base_sha).to eq(Gitlab::Git::BLANK_SHA)
+ expect(design.diff_refs.head_sha).to eq(design.diff_refs.head_sha)
+ end
+ end
+
+ it 'has no diff ref if new' do
+ design = build(:design)
+
+ expect(design.diff_refs).to be_nil
+ end
+ end
+
+ describe '#repository' do
+ it 'is a design repository' do
+ design = build(:design)
+
+ expect(design.repository).to be_a(DesignManagement::Repository)
+ 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
+ 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)
+
+ expect(design.note_etag_key).to eq(
+ ::Gitlab::Routing.url_helpers.designs_project_issue_path(design.project, design.issue, { vueroute: design.filename })
+ )
+ end
+ end
+
+ describe '#user_notes_count', :use_clean_rails_memory_store_caching do
+ let_it_be(:design) { create(:design, :with_file) }
+
+ subject { design.user_notes_count }
+
+ # Note: Cache invalidation tests are in `design_user_notes_count_service_spec.rb`
+
+ it 'returns a count of user-generated notes' do
+ create(:diff_note_on_design, noteable: design)
+
+ is_expected.to eq(1)
+ end
+
+ it 'does not count notes on other designs' do
+ second_design = create(:design, :with_file)
+ create(:diff_note_on_design, noteable: second_design)
+
+ is_expected.to eq(0)
+ end
+
+ it 'does not count system notes' do
+ create(:diff_note_on_design, system: true, noteable: design)
+
+ is_expected.to eq(0)
+ end
+ end
+
+ describe '#after_note_changed' do
+ subject { build(:design) }
+
+ it 'calls #delete_cache on DesignUserNotesCountService' do
+ expect_next_instance_of(DesignManagement::DesignUserNotesCountService) do |service|
+ expect(service).to receive(:delete_cache)
+ end
+
+ subject.after_note_changed(build(:note))
+ end
+
+ it 'does not call #delete_cache on DesignUserNotesCountService when passed a system note' do
+ expect(DesignManagement::DesignUserNotesCountService).not_to receive(:new)
+
+ subject.after_note_changed(build(:note, :system))
+ end
+ end
+
+ describe '.for_reference' do
+ let_it_be(:design_a) { create(:design) }
+ let_it_be(:design_b) { create(:design) }
+
+ it 'avoids extra queries when calling to_reference' do
+ designs = described_class.for_reference.where(id: [design_a.id, design_b.id]).to_a
+
+ expect { designs.map(&:to_reference) }.not_to exceed_query_limit(0)
+ end
+ end
+
+ describe '#to_reference' do
+ let(:namespace) { build(:namespace, path: 'sample-namespace') }
+ let(:project) { build(:project, name: 'sample-project', namespace: namespace) }
+ let(:group) { create(:group, name: 'Group', path: 'sample-group') }
+ let(:issue) { build(:issue, iid: 1, project: project) }
+ let(:filename) { 'homescreen.jpg' }
+ let(:design) { build(:design, filename: filename, issue: issue, project: project) }
+
+ context 'when nil argument' do
+ let(:reference) { design.to_reference }
+
+ it 'uses the simple format' do
+ expect(reference).to eq "#1[homescreen.jpg]"
+ end
+
+ context 'when the filename contains spaces, hyphens, periods, single-quotes, underscores and colons' do
+ let(:filename) { %q{a complex filename: containing - _ : etc., but still 'simple'.gif} }
+
+ it 'uses the simple format' do
+ expect(reference).to eq "#1[#{filename}]"
+ end
+ end
+
+ context 'when the filename contains HTML angle brackets' do
+ let(:filename) { 'a <em>great</em> filename.jpg' }
+
+ it 'uses Base64 encoding' do
+ expect(reference).to eq "#1[base64:#{Base64.strict_encode64(filename)}]"
+ end
+ end
+
+ context 'when the filename contains quotation marks' do
+ let(:filename) { %q{a "great" filename.jpg} }
+
+ it 'uses enclosing quotes, with backslash encoding' do
+ expect(reference).to eq %q{#1["a \"great\" filename.jpg"]}
+ end
+ end
+
+ context 'when the filename contains square brackets' do
+ let(:filename) { %q{a [great] filename.jpg} }
+
+ it 'uses enclosing quotes' do
+ expect(reference).to eq %q{#1["a [great] filename.jpg"]}
+ end
+ end
+ end
+
+ context 'when full is true' do
+ it 'returns complete path to the issue' do
+ refs = [
+ design.to_reference(full: true),
+ design.to_reference(project, full: true),
+ design.to_reference(group, full: true)
+ ]
+
+ expect(refs).to all(eq 'sample-namespace/sample-project#1/designs[homescreen.jpg]')
+ end
+ end
+
+ context 'when full is false' do
+ it 'returns complete path to the issue' do
+ refs = [
+ design.to_reference(build(:project), full: false),
+ design.to_reference(group, full: false)
+ ]
+
+ expect(refs).to all(eq 'sample-namespace/sample-project#1[homescreen.jpg]')
+ end
+ end
+
+ context 'when same project argument' do
+ it 'returns bare reference' do
+ expect(design.to_reference(project)).to eq("#1[homescreen.jpg]")
+ end
+ end
+ end
+
+ describe 'reference_pattern' do
+ let(:match) { described_class.reference_pattern.match(ref) }
+ let(:ref) { design.to_reference }
+ let(:design) { build(:design, filename: filename) }
+
+ context 'simple_file_name' do
+ let(:filename) { 'simple-file-name.jpg' }
+
+ it 'matches :simple_file_name' do
+ expect(match[:simple_file_name]).to eq(filename)
+ end
+ end
+
+ context 'quoted_file_name' do
+ let(:filename) { 'simple "file" name.jpg' }
+
+ it 'matches :simple_file_name' do
+ expect(match[:escaped_filename].gsub(/\\"/, '"')).to eq(filename)
+ end
+ end
+
+ context 'Base64 name' do
+ let(:filename) { '<>.png' }
+
+ it 'matches base_64_encoded_name' do
+ expect(Base64.decode64(match[:base_64_encoded_name])).to eq(filename)
+ end
+ end
+ end
+
+ describe '.by_issue_id_and_filename' do
+ let_it_be(:issue_a) { create(:issue) }
+ let_it_be(:issue_b) { create(:issue) }
+
+ let_it_be(:design_a) { create(:design, issue: issue_a) }
+ let_it_be(:design_b) { create(:design, issue: issue_a) }
+ let_it_be(:design_c) { create(:design, issue: issue_b, filename: design_a.filename) }
+ let_it_be(:design_d) { create(:design, issue: issue_b, filename: design_b.filename) }
+
+ it_behaves_like 'a where_composite scope', :by_issue_id_and_filename do
+ let(:all_results) { [design_a, design_b, design_c, design_d] }
+ let(:first_result) { design_a }
+
+ let(:composite_ids) do
+ all_results.map { |design| { issue_id: design.issue_id, filename: design.filename } }
+ end
+ end
+ end
+end
diff --git a/spec/models/design_management/repository_spec.rb b/spec/models/design_management/repository_spec.rb
new file mode 100644
index 00000000000..996316eeec9
--- /dev/null
+++ b/spec/models/design_management/repository_spec.rb
@@ -0,0 +1,58 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe DesignManagement::Repository do
+ let(:project) { create(:project) }
+ let(:repository) { described_class.new(project) }
+
+ shared_examples 'returns parsed git attributes that enable LFS for all file types' do
+ it do
+ expect(subject.patterns).to be_a_kind_of(Hash)
+ expect(subject.patterns).to have_key('/designs/*')
+ expect(subject.patterns['/designs/*']).to eql(
+ { "filter" => "lfs", "diff" => "lfs", "merge" => "lfs", "text" => false }
+ )
+ end
+ end
+
+ describe "#info_attributes" do
+ subject { repository.info_attributes }
+
+ include_examples 'returns parsed git attributes that enable LFS for all file types'
+ end
+
+ describe '#attributes_at' do
+ subject { repository.attributes_at }
+
+ include_examples 'returns parsed git attributes that enable LFS for all file types'
+ end
+
+ describe '#gitattribute' do
+ it 'returns a gitattribute when path has gitattributes' do
+ expect(repository.gitattribute('/designs/file.txt', 'filter')).to eq('lfs')
+ end
+
+ it 'returns nil when path has no gitattributes' do
+ expect(repository.gitattribute('/invalid/file.txt', 'filter')).to be_nil
+ end
+ end
+
+ describe '#copy_gitattributes' do
+ it 'always returns regardless of whether given a valid or invalid ref' do
+ expect(repository.copy_gitattributes('master')).to be true
+ expect(repository.copy_gitattributes('invalid')).to be true
+ end
+ end
+
+ describe '#attributes' do
+ it 'confirms that all files are LFS enabled' do
+ %w(png zip anything).each do |filetype|
+ path = "/#{DesignManagement.designs_directory}/file.#{filetype}"
+ attributes = repository.attributes(path)
+
+ expect(attributes['filter']).to eq('lfs')
+ end
+ end
+ end
+end
diff --git a/spec/models/design_management/version_spec.rb b/spec/models/design_management/version_spec.rb
new file mode 100644
index 00000000000..ab6958ea94a
--- /dev/null
+++ b/spec/models/design_management/version_spec.rb
@@ -0,0 +1,342 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe DesignManagement::Version do
+ let_it_be(:issue) { create(:issue) }
+
+ describe 'relations' do
+ it { is_expected.to have_many(:actions) }
+ it { is_expected.to have_many(:designs).through(:actions) }
+
+ it 'constrains the designs relation correctly' do
+ design = create(:design)
+ version = create(:design_version, designs: [design])
+
+ expect { version.designs << design }.to raise_error(ActiveRecord::RecordNotUnique)
+ end
+
+ it 'allows adding multiple versions to a single design' do
+ design = create(:design)
+ versions = create_list(:design_version, 2)
+
+ expect { versions.each { |v| design.versions << v } }
+ .not_to raise_error
+ end
+ end
+
+ describe 'validations' do
+ subject(:design_version) { build(:design_version) }
+
+ it { is_expected.to be_valid }
+ it { is_expected.to validate_presence_of(:author) }
+ it { is_expected.to validate_presence_of(:sha) }
+ it { is_expected.to validate_presence_of(:designs) }
+ it { is_expected.to validate_presence_of(:issue_id) }
+ it { is_expected.to validate_uniqueness_of(:sha).scoped_to(:issue_id).case_insensitive }
+ end
+
+ describe "scopes" do
+ let_it_be(:version_1) { create(:design_version) }
+ let_it_be(:version_2) { create(:design_version) }
+
+ describe ".for_designs" do
+ it "only returns versions related to the specified designs" do
+ _other_version = create(:design_version)
+ designs = [create(:design, versions: [version_1]),
+ create(:design, versions: [version_2])]
+
+ expect(described_class.for_designs(designs))
+ .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::Version 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
+
+ describe '.by_sha' do
+ it 'can find versions by sha' do
+ [version_1, version_2].each do |version|
+ expect(described_class.by_sha(version.sha)).to contain_exactly(version)
+ end
+ end
+ end
+ end
+
+ describe ".create_for_designs" do
+ def current_version_id(design)
+ design.send(:head_version).try(:id)
+ end
+
+ def as_actions(designs, action = :create)
+ designs.map do |d|
+ DesignManagement::DesignAction.new(d, action, action == :delete ? nil : :content)
+ end
+ end
+
+ let_it_be(:author) { create(:user) }
+ let_it_be(:design_a) { create(:design, issue: issue) }
+ let_it_be(:design_b) { create(:design, issue: issue) }
+ let_it_be(:designs) { [design_a, design_b] }
+
+ describe 'the error raised when there are no actions' do
+ let_it_be(:sha) { 'f00' }
+
+ def call_with_empty_actions
+ described_class.create_for_designs([], sha, author)
+ end
+
+ it 'raises CouldNotCreateVersion' do
+ expect { call_with_empty_actions }
+ .to raise_error(described_class::CouldNotCreateVersion)
+ end
+
+ it 'has an appropriate cause' do
+ expect { call_with_empty_actions }
+ .to raise_error(have_attributes(cause: ActiveRecord::RecordInvalid))
+ end
+
+ it 'provides extra data sentry can consume' do
+ extra_info = a_hash_including(sha: sha)
+
+ expect { call_with_empty_actions }
+ .to raise_error(have_attributes(sentry_extra_data: extra_info))
+ end
+ end
+
+ describe 'the error raised when the designs come from different issues' do
+ let_it_be(:sha) { 'f00' }
+ let_it_be(:designs) { create_list(:design, 2) }
+ let_it_be(:actions) { as_actions(designs) }
+
+ def call_with_mismatched_designs
+ described_class.create_for_designs(actions, sha, author)
+ end
+
+ it 'raises CouldNotCreateVersion' do
+ expect { call_with_mismatched_designs }
+ .to raise_error(described_class::CouldNotCreateVersion)
+ end
+
+ it 'has an appropriate cause' do
+ expect { call_with_mismatched_designs }
+ .to raise_error(have_attributes(cause: described_class::NotSameIssue))
+ end
+
+ it 'provides extra data sentry can consume' do
+ extra_info = a_hash_including(design_ids: designs.map(&:id))
+
+ expect { call_with_mismatched_designs }
+ .to raise_error(have_attributes(sentry_extra_data: extra_info))
+ end
+ end
+
+ it 'does not leave invalid versions around if creation fails' do
+ expect do
+ described_class.create_for_designs([], 'abcdef', author) rescue nil
+ end.not_to change { described_class.count }
+ end
+
+ it 'does not leave orphaned design-versions around if creation fails' do
+ actions = as_actions(designs)
+ expect do
+ described_class.create_for_designs(actions, '', author) rescue nil
+ end.not_to change { DesignManagement::Action.count }
+ end
+
+ it 'creates a version and links it to multiple designs' do
+ actions = as_actions(designs, :create)
+
+ version = described_class.create_for_designs(actions, 'abc', author)
+
+ expect(version.designs).to contain_exactly(*designs)
+ expect(designs.map(&method(:current_version_id))).to all(eq version.id)
+ end
+
+ it 'creates designs if they are new to git' do
+ actions = as_actions(designs, :create)
+
+ described_class.create_for_designs(actions, 'abc', author)
+
+ expect(designs.map(&:most_recent_action)).to all(be_creation)
+ end
+
+ it 'correctly associates the version with the issue' do
+ actions = as_actions(designs)
+
+ version = described_class.create_for_designs(actions, 'abc', author)
+
+ expect(version.issue).to eq(issue)
+ end
+
+ it 'correctly associates the version with the author' do
+ actions = as_actions(designs)
+
+ version = described_class.create_for_designs(actions, 'abc', author)
+
+ expect(version.author).to eq(author)
+ end
+
+ it 'modifies designs if git updated them' do
+ actions = as_actions(designs, :update)
+
+ described_class.create_for_designs(actions, 'abc', author)
+
+ expect(designs.map(&:most_recent_action)).to all(be_modification)
+ end
+
+ it 'deletes designs when the git action was delete' do
+ actions = as_actions(designs, :delete)
+
+ described_class.create_for_designs(actions, 'def', author)
+
+ expect(designs).to all(be_deleted)
+ end
+
+ it 're-creates designs if they are deleted' do
+ described_class.create_for_designs(as_actions(designs, :create), 'abc', author)
+ described_class.create_for_designs(as_actions(designs, :delete), 'def', author)
+
+ expect(designs).to all(be_deleted)
+
+ described_class.create_for_designs(as_actions(designs, :create), 'ghi', author)
+
+ expect(designs.map(&:most_recent_action)).to all(be_creation)
+ expect(designs).not_to include(be_deleted)
+ end
+
+ it 'changes the version of the designs' do
+ actions = as_actions([design_a])
+ described_class.create_for_designs(actions, 'before', author)
+
+ expect do
+ described_class.create_for_designs(actions, 'after', author)
+ end.to change { current_version_id(design_a) }
+ end
+ end
+
+ describe '#designs_by_event' do
+ context 'there is a single design' do
+ let_it_be(:design) { create(:design) }
+
+ shared_examples :a_correctly_categorised_design do |kind, category|
+ let_it_be(:version) { create(:design_version, kind => [design]) }
+
+ it 'returns a hash with a single key and the single design in that bucket' do
+ expect(version.designs_by_event).to eq(category => [design])
+ end
+ end
+
+ it_behaves_like :a_correctly_categorised_design, :created_designs, 'creation'
+ it_behaves_like :a_correctly_categorised_design, :modified_designs, 'modification'
+ it_behaves_like :a_correctly_categorised_design, :deleted_designs, 'deletion'
+ end
+
+ context 'there are a bunch of different designs in a variety of states' do
+ let_it_be(:version) do
+ create(:design_version,
+ created_designs: create_list(:design, 3),
+ modified_designs: create_list(:design, 4),
+ deleted_designs: create_list(:design, 5))
+ end
+
+ it 'puts them in the right buckets' do
+ expect(version.designs_by_event).to match(
+ a_hash_including(
+ 'creation' => have_attributes(size: 3),
+ 'modification' => have_attributes(size: 4),
+ 'deletion' => have_attributes(size: 5)
+ )
+ )
+ end
+
+ it 'does not suffer from N+1 queries' do
+ version.designs.map(&:id) # we don't care about the set-up queries
+ expect { version.designs_by_event }.not_to exceed_query_limit(2)
+ end
+ end
+ end
+
+ describe '#author' do
+ it 'returns the author' do
+ author = build(:user)
+ version = build(:design_version, author: author)
+
+ expect(version.author).to eq(author)
+ end
+
+ it 'returns nil if author_id is nil and version is not persisted' do
+ version = build(:design_version, author: nil)
+
+ expect(version.author).to eq(nil)
+ end
+
+ it 'retrieves author from the Commit if author_id is nil and version has been persisted' do
+ author = create(:user)
+ version = create(:design_version, :committed, author: author)
+ author.destroy
+ version.reload
+ commit = version.issue.project.design_repository.commit(version.sha)
+ commit_user = create(:user, email: commit.author_email, name: commit.author_name)
+
+ expect(version.author_id).to eq(nil)
+ expect(version.author).to eq(commit_user)
+ end
+ end
+
+ describe '#diff_refs' do
+ let(:project) { issue.project }
+
+ before do
+ expect(project.design_repository).to receive(:commit)
+ .once
+ .with(sha)
+ .and_return(commit)
+ end
+
+ subject { create(:design_version, issue: issue, sha: sha) }
+
+ context 'there is a commit in the repo by the SHA' do
+ let(:commit) { build(:commit) }
+ let(:sha) { commit.id }
+
+ it { is_expected.to have_attributes(diff_refs: commit.diff_refs) }
+
+ it 'memoizes calls to #diff_refs' do
+ expect(subject.diff_refs).to eq(subject.diff_refs)
+ end
+ end
+
+ context 'there is no commit in the repo by the SHA' do
+ let(:commit) { nil }
+ let(:sha) { Digest::SHA1.hexdigest("points to nothing") }
+
+ it { is_expected.to have_attributes(diff_refs: be_nil) }
+ end
+ end
+
+ describe '#reset' do
+ subject { create(:design_version, issue: issue) }
+
+ it 'removes memoized values' do
+ expect(subject).to receive(:commit).twice.and_return(nil)
+
+ subject.diff_refs
+ subject.diff_refs
+
+ subject.reset
+
+ subject.diff_refs
+ subject.diff_refs
+ end
+ end
+end
diff --git a/spec/models/design_user_mention_spec.rb b/spec/models/design_user_mention_spec.rb
new file mode 100644
index 00000000000..03c77c73c8d
--- /dev/null
+++ b/spec/models/design_user_mention_spec.rb
@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe DesignUserMention do
+ describe 'associations' do
+ it { is_expected.to belong_to(:design) }
+ it { is_expected.to belong_to(:note) }
+ end
+
+ it_behaves_like 'has user mentions'
+end
diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb
index b802c8ba506..65f06a5b270 100644
--- a/spec/models/diff_note_spec.rb
+++ b/spec/models/diff_note_spec.rb
@@ -287,6 +287,24 @@ describe DiffNote do
reply_diff_note.reload.diff_file
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
describe "#diff_line" do
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index 375c8c1546f..cc7dffb93d2 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -15,8 +15,20 @@ describe Issue do
it { is_expected.to belong_to(:closed_by).class_name('User') }
it { is_expected.to have_many(:assignees) }
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(: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
describe 'modules' do
@@ -970,4 +982,48 @@ describe Issue do
expect(issue.previous_updated_at).to eq(Time.new(2013, 02, 06))
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
diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index 74ec74e0def..4900743ccb5 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -751,6 +751,14 @@ describe Note do
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
it 'returns note' do
expect(build(:note).to_ability_name).to eq('note')
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 78c0e8aef1a..bcd28538e2c 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -6,6 +6,7 @@ describe Project do
include ProjectForksHelper
include GitHelpers
include ExternalAuthorizationServiceHelpers
+ using RSpec::Parameterized::TableSyntax
it_behaves_like 'having unique enum values'
@@ -6058,6 +6059,28 @@ describe Project do
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)
export_job.start
export_job.finish
diff --git a/spec/services/design_management/design_user_notes_count_service_spec.rb b/spec/services/design_management/design_user_notes_count_service_spec.rb
new file mode 100644
index 00000000000..f4808542995
--- /dev/null
+++ b/spec/services/design_management/design_user_notes_count_service_spec.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe DesignManagement::DesignUserNotesCountService, :use_clean_rails_memory_store_caching do
+ let_it_be(:design) { create(:design, :with_file) }
+
+ subject { described_class.new(design) }
+
+ it_behaves_like 'a counter caching service'
+
+ describe '#count' do
+ it 'returns the count of notes' do
+ create_list(:diff_note_on_design, 3, noteable: design)
+
+ expect(subject.count).to eq(3)
+ end
+ end
+
+ describe '#cache_key' do
+ it 'contains the `VERSION` and `design.id`' do
+ expect(subject.cache_key).to eq(['designs', 'notes_count', DesignManagement::DesignUserNotesCountService::VERSION, design.id])
+ 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
+ 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)
+
+ expect do
+ Notes::CreateService.new(design.project, create(:user), new_note_attrs).execute
+ end.to change { subject.count }.by(1)
+ end
+
+ 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)
+
+ expect do
+ Notes::DestroyService.new(note.project, note.author).execute(note)
+ end.to change { subject.count }.by(-1)
+ end
+ end
+end
diff --git a/spec/services/issues/related_branches_service_spec.rb b/spec/services/issues/related_branches_service_spec.rb
index eae35f12560..9f72e499414 100644
--- a/spec/services/issues/related_branches_service_spec.rb
+++ b/spec/services/issues/related_branches_service_spec.rb
@@ -3,39 +3,103 @@
require 'spec_helper'
describe Issues::RelatedBranchesService do
- let(:user) { create(:admin) }
- let(:issue) { create(:issue) }
+ let_it_be(:developer) { create(:user) }
+ let_it_be(:issue) { create(:issue) }
+ let(:user) { developer }
subject { described_class.new(issue.project, user) }
+ before do
+ issue.project.add_developer(developer)
+ end
+
describe '#execute' do
+ let(:sha) { 'abcdef' }
+ let(:repo) { issue.project.repository }
+ let(:project) { issue.project }
+ let(:branch_info) { subject.execute(issue) }
+
+ def make_branch
+ double('Branch', dereferenced_target: double('Target', sha: sha))
+ end
+
before do
- allow(issue.project.repository).to receive(:branch_names).and_return(["mpempe", "#{issue.iid}mepmep", issue.to_branch_name, "#{issue.iid}-branch"])
+ allow(repo).to receive(:branch_names).and_return(branch_names)
end
- it "selects the right branches when there are no referenced merge requests" do
- expect(subject.execute(issue)).to eq([issue.to_branch_name, "#{issue.iid}-branch"])
+ context 'no branches are available' do
+ let(:branch_names) { [] }
+
+ it 'returns an empty array' do
+ expect(branch_info).to be_empty
+ end
end
- it "selects the right branches when there is a referenced merge request" do
- merge_request = create(:merge_request, { description: "Closes ##{issue.iid}",
- source_project: issue.project,
- source_branch: "#{issue.iid}-branch" })
- merge_request.create_cross_references!(user)
+ context 'branches are available' do
+ let(:missing_branch) { "#{issue.to_branch_name}-missing" }
+ let(:unreadable_branch_name) { "#{issue.to_branch_name}-unreadable" }
+ let(:pipeline) { build(:ci_pipeline, :success, project: project) }
+ let(:unreadable_pipeline) { build(:ci_pipeline, :running) }
+
+ let(:branch_names) do
+ [
+ generate(:branch),
+ "#{issue.iid}doesnt-match",
+ issue.to_branch_name,
+ missing_branch,
+ unreadable_branch_name
+ ]
+ end
+
+ before do
+ {
+ issue.to_branch_name => pipeline,
+ unreadable_branch_name => unreadable_pipeline
+ }.each do |name, pipeline|
+ allow(repo).to receive(:find_branch).with(name).and_return(make_branch)
+ allow(project).to receive(:pipeline_for).with(name, sha).and_return(pipeline)
+ end
+
+ allow(repo).to receive(:find_branch).with(missing_branch).and_return(nil)
+ end
+
+ it 'selects relevant branches, along with pipeline status where available' do
+ expect(branch_info).to contain_exactly(
+ { name: issue.to_branch_name, pipeline_status: an_instance_of(Gitlab::Ci::Status::Success) },
+ { name: missing_branch, pipeline_status: be_nil },
+ { name: unreadable_branch_name, pipeline_status: be_nil }
+ )
+ end
+
+ context 'the user has access to otherwise unreadable pipelines' do
+ let(:user) { create(:admin) }
+
+ it 'returns info a developer could not see' do
+ expect(branch_info.pluck(:pipeline_status)).to include(an_instance_of(Gitlab::Ci::Status::Running))
+ end
+ end
+
+ it 'excludes branches referenced in merge requests' do
+ merge_request = create(:merge_request, { description: "Closes #{issue.to_reference}",
+ source_project: issue.project,
+ source_branch: issue.to_branch_name })
+ merge_request.create_cross_references!(user)
- referenced_merge_requests = Issues::ReferencedMergeRequestsService
- .new(issue.project, user)
- .referenced_merge_requests(issue)
+ referenced_merge_requests = Issues::ReferencedMergeRequestsService
+ .new(issue.project, user)
+ .referenced_merge_requests(issue)
- expect(referenced_merge_requests).not_to be_empty
- expect(subject.execute(issue)).to eq([issue.to_branch_name])
+ expect(referenced_merge_requests).not_to be_empty
+ expect(branch_info.pluck(:name)).not_to include(merge_request.source_branch)
+ end
end
- it 'excludes stable branches from the related branches' do
- allow(issue.project.repository).to receive(:branch_names)
- .and_return(["#{issue.iid}-0-stable"])
+ context 'one of the branches is stable' do
+ let(:branch_names) { ["#{issue.iid}-0-stable"] }
- expect(subject.execute(issue)).to eq []
+ it 'is excluded' do
+ expect(branch_info).to be_empty
+ end
end
end
end
diff --git a/spec/support/helpers/design_management_test_helpers.rb b/spec/support/helpers/design_management_test_helpers.rb
new file mode 100644
index 00000000000..bf41e2f5079
--- /dev/null
+++ b/spec/support/helpers/design_management_test_helpers.rb
@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+module DesignManagementTestHelpers
+ def enable_design_management(enabled = true, ref_filter = true)
+ stub_lfs_setting(enabled: enabled)
+ stub_feature_flags(design_management_reference_filter_gfm_pipeline: ref_filter)
+ end
+
+ def delete_designs(*designs)
+ act_on_designs(designs) { ::DesignManagement::Action.deletion }
+ end
+
+ def restore_designs(*designs)
+ act_on_designs(designs) { ::DesignManagement::Action.creation }
+ end
+
+ def modify_designs(*designs)
+ act_on_designs(designs) { ::DesignManagement::Action.modification }
+ end
+
+ def path_for_design(design)
+ path_options = { vueroute: design.filename }
+ Gitlab::Routing.url_helpers.designs_project_issue_path(design.project, design.issue, path_options)
+ end
+
+ def url_for_design(design)
+ path_options = { vueroute: design.filename }
+ Gitlab::Routing.url_helpers.designs_project_issue_url(design.project, design.issue, path_options)
+ end
+
+ def url_for_designs(issue)
+ Gitlab::Routing.url_helpers.designs_project_issue_url(issue.project, issue)
+ end
+
+ private
+
+ def act_on_designs(designs, &block)
+ issue = designs.first.issue
+ version = build(:design_version, :empty, issue: issue).tap { |v| v.save(validate: false) }
+ designs.each do |d|
+ yield.create(design: d, version: version)
+ end
+ version
+ end
+end
diff --git a/spec/uploaders/design_management/design_v432x230_uploader_spec.rb b/spec/uploaders/design_management/design_v432x230_uploader_spec.rb
new file mode 100644
index 00000000000..8c62b6ad6a8
--- /dev/null
+++ b/spec/uploaders/design_management/design_v432x230_uploader_spec.rb
@@ -0,0 +1,86 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe DesignManagement::DesignV432x230Uploader do
+ include CarrierWave::Test::Matchers
+
+ let(:model) { create(:design_action, :with_image_v432x230) }
+ let(:upload) { create(:upload, :design_action_image_v432x230_upload, model: model) }
+
+ subject(:uploader) { described_class.new(model, :image_v432x230) }
+
+ it_behaves_like 'builds correct paths',
+ store_dir: %r[uploads/-/system/design_management/action/image_v432x230/],
+ upload_path: %r[uploads/-/system/design_management/action/image_v432x230/],
+ relative_path: %r[uploads/-/system/design_management/action/image_v432x230/],
+ absolute_path: %r[#{CarrierWave.root}/uploads/-/system/design_management/action/image_v432x230/]
+
+ context 'object_store is REMOTE' do
+ before do
+ stub_uploads_object_storage
+ end
+
+ include_context 'with storage', described_class::Store::REMOTE
+
+ it_behaves_like 'builds correct paths',
+ store_dir: %r[design_management/action/image_v432x230/],
+ upload_path: %r[design_management/action/image_v432x230/],
+ relative_path: %r[design_management/action/image_v432x230/]
+ end
+
+ describe "#migrate!" do
+ before do
+ uploader.store!(fixture_file_upload('spec/fixtures/dk.png'))
+ stub_uploads_object_storage
+ end
+
+ it_behaves_like 'migrates', to_store: described_class::Store::REMOTE
+ it_behaves_like 'migrates', from_store: described_class::Store::REMOTE, to_store: described_class::Store::LOCAL
+ end
+
+ it 'resizes images', :aggregate_failures do
+ image_loader = CarrierWave::Test::Matchers::ImageLoader
+ original_file = fixture_file_upload('spec/fixtures/dk.png')
+ uploader.store!(original_file)
+
+ expect(
+ image_loader.load_image(original_file.tempfile.path)
+ ).to have_attributes(
+ width: 460,
+ height: 322
+ )
+ expect(
+ image_loader.load_image(uploader.file.file)
+ ).to have_attributes(
+ width: 329,
+ height: 230
+ )
+ end
+
+ context 'accept whitelist file content type' do
+ # We need to feed through a valid path, but we force the parsed mime type
+ # in a stub below so we can set any path.
+ let_it_be(:path) { File.join('spec', 'fixtures', 'dk.png') }
+
+ where(:mime_type) { described_class::MIME_TYPE_WHITELIST }
+
+ with_them do
+ include_context 'force content type detection to mime_type'
+
+ it_behaves_like 'accepted carrierwave upload'
+ end
+ end
+
+ context 'upload non-whitelisted file content type' do
+ let_it_be(:path) { File.join('spec', 'fixtures', 'logo_sample.svg') }
+
+ it_behaves_like 'denied carrierwave upload'
+ end
+
+ context 'upload misnamed non-whitelisted file content type' do
+ let_it_be(:path) { File.join('spec', 'fixtures', 'not_a_png.png') }
+
+ it_behaves_like 'denied carrierwave upload'
+ end
+end
diff --git a/spec/views/projects/issues/_related_branches.html.haml_spec.rb b/spec/views/projects/issues/_related_branches.html.haml_spec.rb
index a6817e3fdbf..6c9bbaea38c 100644
--- a/spec/views/projects/issues/_related_branches.html.haml_spec.rb
+++ b/spec/views/projects/issues/_related_branches.html.haml_spec.rb
@@ -5,23 +5,25 @@ require 'spec_helper'
describe 'projects/issues/_related_branches' do
include Devise::Test::ControllerHelpers
- let(:user) { create(:user) }
- let(:project) { create(:project, :repository) }
- let(:branch) { project.repository.find_branch('feature') }
- let!(:pipeline) { create(:ci_pipeline, project: project, sha: branch.dereferenced_target.id, ref: 'feature') }
+ let(:pipeline) { build(:ci_pipeline, :success) }
+ let(:status) { pipeline.detailed_status(build(:user)) }
before do
- assign(:project, project)
- assign(:related_branches, ['feature'])
-
- project.add_developer(user)
- allow(view).to receive(:current_user).and_return(user)
+ assign(:related_branches, [
+ { name: 'other', link: 'link-to-other', pipeline_status: nil },
+ { name: 'feature', link: 'link-to-feature', pipeline_status: status }
+ ])
render
end
- it 'shows the related branches with their build status' do
- expect(rendered).to match('feature')
+ it 'shows the related branches with their build status', :aggregate_failures do
+ expect(rendered).to have_text('feature')
+ expect(rendered).to have_text('other')
+ expect(rendered).to have_link(href: 'link-to-feature')
+ expect(rendered).to have_link(href: 'link-to-other')
expect(rendered).to have_css('.related-branch-ci-status')
+ expect(rendered).to have_css('.ci-status-icon')
+ expect(rendered).to have_css('.related-branch-info')
end
end