Commit b8f65d1f authored by Alex Kalderimis's avatar Alex Kalderimis Committed by Thong Kuah

Add backend support for deleting designs

This adds a new GraphQL mutation to deleted designs, which does not
permanently destroy them, but instead creates a new version in which
they are not visible. They can be queried for at various points in the
history and their deletion status will be respected.
parent 4f50a5d6
......@@ -15,17 +15,13 @@ module DesignManagement
return ::DesignManagement::Design.none
end
items = issue.designs
items = by_visible_at_version(items)
items
by_visible_at_version(issue.designs)
end
private
# Returns all designs that existed at a particular design version
def by_visible_at_version(items)
return items unless params[:visible_at_version]
items.visible_at_version(params[:visible_at_version])
end
end
......
......@@ -11,7 +11,10 @@ module EE
resolve: -> (obj, _args, _ctx) { obj.supports_weight? ? obj.weight : nil }
field :designs, ::Types::DesignManagement::DesignCollectionType,
null: true, method: :design_collection
null: true, method: :design_collection,
deprecation_reason: 'use design_collection'
field :design_collection, ::Types::DesignManagement::DesignCollectionType, null: true
end
end
end
......
......@@ -7,6 +7,7 @@ module EE
prepended do
mount_mutation ::Mutations::DesignManagement::Upload, calls_gitaly: true
mount_mutation ::Mutations::DesignManagement::Delete, calls_gitaly: true
end
end
end
......
......@@ -13,10 +13,6 @@ module Mutations
required: true,
description: "The iid of the issue to modify designs for"
field :designs, [Types::DesignManagement::DesignType],
null: false,
description: "The designs that were updated by the mutation"
private
def find_object(project_path:, iid:)
......
# frozen_string_literal: true
module Mutations
module DesignManagement
class Delete < Base
Errors = ::Gitlab::Graphql::Errors
graphql_name "DesignManagementDelete"
argument :filenames, [GraphQL::STRING_TYPE],
required: true,
description: "The filenames of the designs to delete",
prepare: ->(names, _ctx) do
names.presence || (raise Errors::ArgumentError, 'no filenames')
end
field :version, Types::DesignManagement::VersionType,
null: true, # null on error
description: 'The new version in which the designs are deleted'
authorize :destroy_design
def resolve(project_path:, iid:, filenames:)
issue = authorized_find!(project_path: project_path, iid: iid)
project = issue.project
designs = resolve_designs(issue, filenames)
result = ::DesignManagement::DeleteDesignsService
.new(project, current_user, issue: issue, designs: designs)
.execute
{
version: result[:version],
errors: Array.wrap(result[:message])
}
end
private
# Here we check that:
# * we find exactly as many designs as filenames
def resolve_designs(issue, filenames)
designs = issue.design_collection.designs_by_filename(filenames)
validate_all_were_found!(designs, filenames)
designs
end
def validate_all_were_found!(designs, filenames)
found_filenames = designs.map(&:filename)
missing = filenames.difference(found_filenames)
if missing.present?
raise Errors::ArgumentError, <<~MSG
Not all the designs you named currently exist.
The following filenames were not found:
#{missing.join(', ')}
They may have already been deleted.
MSG
end
end
end
end
end
......@@ -11,6 +11,10 @@ module Mutations
authorize :create_design
field :designs, [Types::DesignManagement::DesignType],
null: false,
description: "The designs that were uploaded by the mutation"
def resolve(project_path:, iid:, files:)
issue = authorized_find!(project_path: project_path, iid: iid)
project = issue.project
......
......@@ -13,9 +13,8 @@ module Types
Types::DesignManagement::DesignType.connection_type,
null: false,
resolver: Resolvers::DesignManagement::DesignResolver,
description: "All visible designs for this collection"
description: "All designs for this collection"
# TODO: allow getting a single design by filename
# TODO: when we allow hiding designs, we will also expose a relation
# exposing all designs
field :versions,
Types::DesignManagement::VersionType.connection_type,
......
......@@ -4,6 +4,7 @@ module DesignManagement
class Design < ApplicationRecord
include Noteable
include Gitlab::FileTypeDetection
include Gitlab::Utils::StrongMemoize
belongs_to :project, inverse_of: :designs
belongs_to :issue
......@@ -20,12 +21,50 @@ module DesignManagement
alias_attribute :title, :filename
# 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
created_before_version = DesignManagement::DesignVersion.select(1)
.where("#{table_name}.id = #{DesignManagement::DesignVersion.table_name}.design_id")
.where("#{DesignManagement::DesignVersion.table_name}.version_id <= ?", version)
deletion = ::DesignManagement::DesignVersion.events[:deletion]
designs = arel_table
design_versions = ::DesignManagement::DesignVersion
.most_recent.up_to_version(version)
.arel.as('most_recent_design_versions')
where('EXISTS(?)', created_before_version)
join = designs.join(design_versions)
.on(design_versions[:design_id].eq(designs[:id]))
joins(join.join_sources).where(design_versions[:event].not_eq(deletion))
end
# 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_design_version&.deletion?
end
def most_recent_design_version
strong_memoize(:most_recent_design_version) { design_versions.ordered.last }
end
def to_reference(_opts)
......@@ -37,7 +76,7 @@ module DesignManagement
end
def new_design?
versions.none?
strong_memoize(:new_design) { design_versions.none? }
end
def full_path
......@@ -45,9 +84,16 @@ module DesignManagement
end
def diff_refs
return if new_design?
strong_memoize(:diff_refs) do
head_version.presence && repository.commit(head_version.sha).diff_refs
end
end
@diff_refs ||= repository.commit(head_version.sha).diff_refs
def clear_version_cache
[versions, design_versions].each(&:reset)
[:new_design, :diff_refs, :head_sha, :most_recent_design_version].each do |key|
clear_memoization(key)
end
end
def repository
......@@ -57,7 +103,7 @@ module DesignManagement
private
def head_version
@head_sha ||= versions.ordered.first
strong_memoize(:head_sha) { versions.ordered.first }
end
def validate_file_is_image
......
# 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::DesignVersion.events[:creation],
update: DesignManagement::DesignVersion.events[:modification],
delete: DesignManagement::DesignVersion.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
# - gitaly_action [Symbol]: the action that gitlay 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
......@@ -22,5 +22,9 @@ module DesignManagement
def repository
project.design_repository
end
def designs_by_filename(filenames)
designs.current.where(filename: filenames)
end
end
end
......@@ -6,5 +6,29 @@ module DesignManagement
belongs_to :design, class_name: "DesignManagement::Design", inverse_of: :design_versions
belongs_to :version, class_name: "DesignManagement::Version", inverse_of: :design_versions
enum event: [:creation, :modification, :deletion]
# we assume sequential ordering.
scope :ordered, -> { order(version_id: :asc) }
# For each design, only select the most recent design_version
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))
else
raise "Expected a DesignManagement::Version, got #{version}"
end
end
end
end
......@@ -4,6 +4,28 @@ module DesignManagement
class Version < ApplicationRecord
include ShaAttribute
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
has_many :design_versions
has_many :designs,
......@@ -12,6 +34,7 @@ module DesignManagement
source: :design,
inverse_of: :versions
validates :designs, presence: true
validates :sha, presence: true
validates :sha, uniqueness: { case_sensitive: false, scope: :issue_id }
......@@ -22,19 +45,39 @@ module DesignManagement
end
scope :earlier_or_equal_to, -> (version) { where('id <= ?', version) }
scope :ordered, -> { order(id: :desc) }
scope :for_issue, -> (issue) { where(issue: issue) }
def self.create_for_designs(designs, sha)
issue_id = designs.first.issue_id
# 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.
#
# Parameters:
# - designs [DesignManagement::DesignAction]:
# the actions that have been performed in the repository.
# - sha [String]:
# the SHA of the commit that performed them
# returns [DesignManagement::Version]
def self.create_for_designs(design_actions, sha)
issue_id, not_uniq = design_actions.map(&:issue_id).compact.uniq
raise NotSameIssue, 'All designs must belong to the same issue!' if not_uniq
version = safe_find_or_create_by!(sha: sha, issue_id: issue_id)
transaction do
version = safe_find_or_create_by(sha: sha, issue_id: issue_id)
version.save(validate: false) # We need it to have an ID, validate later
rows = designs.map do |design|
{ design_id: design.id, version_id: version.id }
end
rows = design_actions.map { |action| action.row_attrs(version) }
Gitlab::Database.bulk_insert(DesignVersion.table_name, rows)
Gitlab::Database.bulk_insert(DesignVersion.table_name, rows)
version.designs.reset
version.validate!
design_actions.each(&:performed)
version
version
end
rescue
raise CouldNotCreateVersion.new(sha, issue_id, design_actions)
end
end
end
# frozen_string_literal: true
module DesignManagement
class DeleteDesignsService < DesignService
include RunsDesignActions
include OnSuccessCallbacks
def initialize(project, user, params = {})
super
@designs = params.fetch(:designs)
end
def execute
return error('Forbidden!') unless can_delete_designs?
actions = build_actions
version = run_actions(actions)
success(version: version)
end
def commit_message
n = designs.size
<<~MSG
Removed #{n} #{'designs'.pluralize(n)}
#{formatted_file_list}
MSG
end
private
attr_reader :designs
def can_delete_designs?
Ability.allowed?(current_user, :destroy_design, issue)
end
def build_actions
designs.map { |d| design_action(d) }
end
def design_action(design)
on_success { counter.count(:delete) }
DesignManagement::DesignAction.new(design, :delete)
end
def counter
::Gitlab::UsageCounters::DesignsCounter
end
def formatted_file_list
designs.map { |design| "- #{design.full_path}" }.join("\n")
end
end
end
# frozen_string_literal: true
module DesignManagement
class DesignService < ::BaseService
def initialize(project, user, params = {})
super
@issue = params.fetch(:issue)
end
# Accessors common to all subclasses:
attr_reader :issue
def target_branch
repository.root_ref || "master"
end
def collection
issue.design_collection
end
def repository
collection.repository
end
def project
issue.project
end
end
end
# frozen_string_literal: true
module DesignManagement
module OnSuccessCallbacks
def on_success(&block)
success_callbacks.push(block)
end
def success(*_)
while cb = success_callbacks.pop
cb.call
end
super
end
private
def success_callbacks
@success_callbacks ||= []
end
end
end
# frozen_string_literal: true
module DesignManagement
module RunsDesignActions
# this concern requires the following methods to be implemented:
# current_user, target_branch, repository, commit_message
def run_actions(actions)
repository.create_if_not_exists
sha = repository.multi_action(current_user,
branch_name: target_branch,
message: commit_message,
actions: actions.map(&:gitaly_action))
::DesignManagement::Version.create_for_designs(actions, sha)
end
end
end
# frozen_string_literal: true
module DesignManagement
class SaveDesignsService < ::BaseService
class SaveDesignsService < DesignService
include RunsDesignActions
include OnSuccessCallbacks
MAX_FILES = 10
def initialize(project, user, params = {})
super
@issue = params.fetch(:issue)
@files = params.fetch(:files)
@success_callbacks = []
end
def execute
return error("Not allowed!") unless can_create_designs?
return error("Only #{MAX_FILES} files are allowed simultaneously") if files.size > MAX_FILES
save_designs!
actions = build_actions
run_actions(actions)
success({ designs: updated_designs })
rescue ::Gitlab::Git::BaseError, ::ActiveRecord::RecordInvalid => e
success({ designs: actions.map(&:design) })
rescue ::ActiveRecord::RecordInvalid => e
error(e.message)
end
private
attr_reader :files, :issue
def success(*_)
while cb = @success_callbacks.pop
cb.call
end
super
end
def on_success(&block)
@success_callbacks.push(block)
end
def save_designs!
commit_sha = create_and_commit_designs!
::DesignManagement::Version.create_for_designs(updated_designs, commit_sha)
end
attr_reader :files
attr_accessor :paths_in_repo
def create_and_commit_designs!
def build_actions
repository.create_if_not_exists
# Do not inline `build_repository_action` here!
# We have to do this as two *separate* calls to #map so that the call
# to `new_file?` does not accidentally cache the wrong data half-way
# through the operation.
corresponding_designs = files.map do |file|
designs = files.map do |file|
collection.find_or_create_design!(filename: file.original_filename)
end
actions = files.zip(corresponding_designs).map do |(file, design)|
build_repository_action(file, design)
end
# Needs to be called before any call to build_design_action
cache_existence(designs)
repository.multi_action(current_user,
branch_name: target_branch,
message: commit_message,
actions: actions)
files.zip(designs).map do |(file, design)|
build_design_action(file, design)
end
end
def build_repository_action(file, design)
def build_design_action(file, design)
action = new_file?(design) ? :create : :update
content = file_content(file, design.full_path)
on_success { ::Gitlab::UsageCounters::DesignsCounter.count(action) }
{
action: action,
file_path: design.full_path,
content: file_content(file, design.full_path)
}
end
def collection
issue.design_collection
end
def repository
project.design_repository
end
def project
issue.project
end
def target_branch
repository.root_ref || "master"
DesignManagement::DesignAction.new(design, action, content)
end
def commit_message
......@@ -108,16 +69,16 @@ module DesignManagement
@filenames ||= files.map(&:original_filename)
end
def updated_designs
@updated_designs ||= collection.designs.select { |design| filenames.include?(design.filename) }
end
def can_create_designs?
Ability.allowed?(current_user, :create_design, issue)
end
def new_file?(design)
design.new_design? && existing_metadata.none? { |blob| blob.path == design.full_path }
design.new_design? && !on_disk?(design)
end
def on_disk?(design)
paths_in_repo === design.full_path
end
def file_content(file, full_path)
......@@ -125,11 +86,9 @@ module DesignManagement
transformer.new_file(full_path, file.to_io).content
end
def existing_metadata
@existing_metadata ||= begin
paths = updated_designs.map(&:full_path)
repository.blobs_metadata(paths)
end
def cache_existence(designs)
paths = designs.map(&:full_path)
self.paths_in_repo = repository.blobs_metadata(paths).map(&:path).to_set
end
end
end
---
title: Add deletion support for designs
merge_request: 14656
type: added
# frozen_string_literal: true
class SeedDesigns
SETTINGS = %i[
n_issues
max_designs_per_issue
max_versions_per_issue
max_designs_per_version
].freeze
attr_reader(*SETTINGS)
def initialize(settings)
SETTINGS.each do |k|
instance_variable_set("@#{k}".to_sym, settings.fetch(k))
end
end
def uploads
@uploads ||= ["dk.png", "rails_sample.jpg"]
.map { |fn| upload(fn) }
.cycle(max_designs_per_issue)
.map { |upload| upload.rename(random_file_name) }
end
def random_file_name
"#{FFaker::Product.product_name}-#{FFaker::Product.unique.model}"
end
Upload = Struct.new(:original_filename, :to_io) do
def rename(name)
Upload.new("#{name}.#{File.extname(original_filename)}", to_io)
end
end
def upload(filename)
content = File.open("spec/fixtures/#{filename}", 'r') do |f|
StringIO.new(f.read)
end
Upload.new(filename, content)
end
def as_action(design)
next_action = case design.status
when :deleted, :new
:create
when :current
[:update, :delete].sample
end
DesignManagement::DesignAction.new(
design,
next_action,
next_action == :delete ? nil : uploads.sample.to_io
)
end
def create_version(repo, devs, to_change, version_number)
user = devs.sample
actions = to_change.map { |design| as_action(design) }
sha = repo.multi_action(user, branch_name: 'master',
message: "version #{version_number}",
actions: actions.map(&:gitaly_action))
version = DesignManagement::Version.create_for_designs(actions, sha)
if version.valid?
print('.' * to_change.size)
else
print('F' * to_change.size)
version.errors.each { |e| warn(e) }
end
end
def create_designs(project, issue, repo, devs)
files = uploads.sample(Random.rand(2..max_designs_per_issue))
files.in_groups_of(10).map(&:compact).select(&:present?).flat_map do |fs|
user = devs.sample
service = DesignManagement::SaveDesignsService.new(project, user,
issue: issue,
files: fs)
message, designs = service.execute.values_at(:message, :designs)
if message
print('F' * fs.size)
warn(message)
else
print('.' * designs.size)
end
designs || []
end
end
def run
Issue.all.sample(n_issues).each do |issue|
project = issue.project
repo = project.design_repository
devs = project.team.developers.all
repo.create_if_not_exists
# All designs get created at least once
designs = create_designs(project, issue, repo, devs)
Random.rand(max_versions_per_issue).times do |i|
to_change = designs.sample(Random.rand(1..max_designs_per_version))
create_version(repo, devs, to_change, i)
end
end
end
def warn(msg)
Rails.logger.warn(msg) # rubocop: disable Gitlab/RailsLogger
end
end
Gitlab::Seeder.quiet do
clear = ENV.fetch('DESIGN_MANAGEMENT_SEED_CLEAR', false)
n_issues = ENV.fetch('DESIGN_MANAGEMENT_SEED_N_ISSUES', 3).to_i
max_designs_per_issue = ENV.fetch('DESIGN_MANAGEMENT_SEED_DESIGNS_PER_ISSUE', 5).to_i
max_versions_per_issue = ENV.fetch('DESIGN_MANAGEMENT_SEED_VERSIONS_PER_ISSUE', 5).to_i
max_designs_per_version = ENV.fetch('DESIGN_MANAGEMENT_SEED_DESIGNS_PER_VERSION', 5).to_i
max_designs_per_issue = [2, max_designs_per_issue].max
flags = %i[design_management design_management_flag].map do |flag|
old = Feature.enabled?(flag)
Feature.enable(flag)
[flag, old]
end.to_h
DesignManagement::Design.delete_all if clear
seed = SeedDesigns.new(n_issues: n_issues,
max_designs_per_issue: max_designs_per_issue,
max_versions_per_issue: max_versions_per_issue,
max_designs_per_version: max_designs_per_version)
seed.run
ensure
flags.each do |(flag, old_value)|
old_value ? Feature.enable(flag) : Feature.disable(flag)
end
end
......@@ -2,10 +2,38 @@
FactoryBot.define do
factory :design, class: DesignManagement::Design do
issue
issue { create(:issue) }
project { issue.project }
sequence(:filename) { |n| "homescreen-#{n}.jpg" }
create_versions = ->(design, evaluator, commit_version) do
unless evaluator.versions_count.zero?
project = design.project
repository = project.design_repository
repository.create_if_not_exists
dv_table_name = DesignManagement::DesignVersion.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: design.issue)
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
end
trait :with_lfs_file do
with_file
......@@ -14,35 +42,50 @@ FactoryBot.define do
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 if you want your designs to be as true-to-life as possible,
# with correctly made 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|
unless evaluator.versions_count.zero?
project = design.project
repository = project.design_repository
repository.create_if_not_exists
evaluator.versions_count.times do |i|
actions = [{
action: i.zero? ? :create : :update, # First version is :create, successive versions are :update
file_path: design.full_path,
content: evaluator.file
}]
sha = repository.multi_action(
project.creator,
branch_name: 'master',
message: "Automatically created file #{design.filename}",
actions: actions
)
FactoryBot.create(:design_version, designs: [design], sha: sha)
end
project = design.project
repository = project.design_repository
commit_version = ->(action) do
repository.multi_action(
project.creator,
branch_name: 'master',
message: "#{action.action} for #{design.filename}",
actions: [action.gitaly_action]
)
end
create_versions[design, evaluator, commit_version]
end
end
end
......
......@@ -4,5 +4,18 @@ 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) }
# Warning: this will intentionally result in an invalid version!
trait :empty do
transient do
no_designs true
end
end
after(:build) do |version, evaluator|
unless evaluator.try(:no_designs) || version.designs.present?
version.designs << create(:design, issue: version.issue)
end
end
end
end
......@@ -47,13 +47,13 @@ describe DesignManagement::DesignsFinder do
let(:second_version) { all_versions.first }
context 'when argument is the first version' do
let(:params) { { visible_at_version: first_version.id } }
let(:params) { { visible_at_version: first_version } }
it { is_expected.to eq([design1]) }
end
context 'when argument is the second version' do
let(:params) { { visible_at_version: second_version.id } }
let(:params) { { visible_at_version: second_version } }
it { is_expected.to contain_exactly(design2, design1) }
end
......
# frozen_string_literal: true
require 'spec_helper'
describe Mutations::DesignManagement::Delete do
include DesignManagementTestHelpers
Errors = Gitlab::Graphql::Errors
let(:issue) { create(:issue) }
let(:current_designs) { issue.designs.current }
let(:user) { issue.author }
let(:project) { issue.project }
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) }
let(:filenames) { [design_a, design_b, design_c].map(&:filename) }
let(:mutation) { described_class.new(object: nil, context: { current_user: user }) }
def run_mutation
mutation = described_class.new(object: nil, context: { current_user: user })
mutation.resolve(project_path: project.full_path, iid: issue.iid, filenames: filenames)
end
describe '#resolve' do
let(:expected_response) do
{ errors: [], version: DesignManagement::Version.for_issue(issue).ordered.first }
end
shared_examples "failures" do |error: Errors::ResourceNotAvailable|
it "raises #{error.name}" do
expect { run_mutation }.to raise_error(error)
end
end
shared_examples "resource not available" do
it_behaves_like "failures"
end
context "when the feature is not available" do
before do
enable_design_management(false)
end
it_behaves_like "resource not available"
end
context "when the feature is available" do
before do
enable_design_management(true)
end
context "when the user is not allowed to delete designs" do
let(:user) { create(:user) }
it_behaves_like "resource not available"
end
context 'deleting an already deleted file' do
before do
run_mutation
end
it 'fails with an argument error' do
expect { run_mutation }.to raise_error(Errors::ArgumentError)
end
end
context "when deleting all the designs" do
let(:response) { run_mutation }
it "returns a new version, and no errors" do
expect(response).to include(expected_response)
end
describe 'the current designs' do
before do
run_mutation
end
it 'is empty' do
expect(current_designs).to be_empty
end
end
it 'runs no more than 27 queries' do
filenames.each(&:present?) # ignore setup
# Queries: as of 2019-08-08
# -------------
# 01. routing query
# 02. find project by id
# 03. project.project_features
# 04. find namespace by id and type
# 05,06. project.authorizations for user (same query twice)
# 07. find issue by iid
# 08. find project by id
# 09. find namespace by id
# 10. find group namespace by id
# 11. project.authorizations for user (same query as 5)
# 12. project.project_features (same query as 3)
# 13. project.authorizations for user (same query as 5)
# 14. current designs by filename and issue
# 15, 16 project.authorizations for user (same query as 5)
# 17. find route by id and source_type
# ------------- our queries are below:
# 18. start transaction 1
# 19. start transaction 2
# 20. find version by sha and issue
# 21. exists version with sha and issue?
# 22. leave transaction 2
# 23. create version with sha and issue
# 24. create design-version links
# 25. validate version.design_versions.present?
# 26. validate version.sha is unique
# 27. leave transaction 1
#
expect { run_mutation }.not_to exceed_query_limit(27)
end
end
context "when deleting a design" do
let(:filenames) { [design_a.filename] }
let(:response) { run_mutation }
it "returns the expected response" do
expect(response).to include(expected_response)
end
describe 'the current designs' do
before do
run_mutation
end
it 'does contain designs b and c' do
expect(current_designs).to contain_exactly(design_b, design_c)
end
end
end
end
end
end
......@@ -8,7 +8,18 @@ describe GitlabSchema.types['Design'] do
it { expect(described_class.interfaces).to include(Types::Notes::NoteableType.to_graphql) }
it 'exposes the expected fields' do
expected_fields = [:id, :project, :issue, :filename, :image, :versions, :discussions, :notes, :diff_refs, :full_path]
expected_fields = %i[
id
project
issue
filename
image
versions
discussions
notes
diff_refs
full_path
]
is_expected.to have_graphql_fields(*expected_fields)
end
......
# 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
......@@ -2,7 +2,10 @@
require 'spec_helper'
describe DesignManagement::DesignCollection do
let(:issue) { create(:issue) }
include DesignManagementTestHelpers
set(:issue) { create(:issue) }
subject(:collection) { described_class.new(issue) }
describe ".find_or_create_design!" do
......@@ -51,4 +54,29 @@ describe DesignManagement::DesignCollection 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
......@@ -3,6 +3,14 @@
require 'rails_helper'
describe DesignManagement::Design do
include DesignManagementTestHelpers
set(:issue) { create(:issue) }
set(:design1) { create(:design, :with_versions, issue: issue, versions_count: 1) }
set(:design2) { create(:design, :with_versions, issue: issue, versions_count: 1) }
set(:design3) { create(:design, :with_versions, issue: issue, versions_count: 1) }
set(: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) }
......@@ -25,48 +33,195 @@ describe DesignManagement::Design do
expect(design).not_to be_valid
expect(design.errors[:filename].first)
.to match /Only these extensions are supported/
.to match %r/Only these extensions are supported/
end
end
describe 'scopes' do
describe '.visible_at_version' do
let!(:design1) { create(:design, :with_file, versions_count: 1) }
let!(:design2) { create(:design, :with_file, versions_count: 1) }
let(:first_version) { DesignManagement::Version.ordered.last }
let(:second_version) { DesignManagement::Version.ordered.first }
it 'returns just designs that existed at that version' do
expect(described_class.visible_at_version(first_version)).to eq([design1])
expect(described_class.visible_at_version(second_version)).to contain_exactly(design1, design2)
let(:versions) { DesignManagement::Version.where(issue: issue).ordered }
context 'at oldest version' do
let(:version) { versions.last }
it 'finds the first design only' do
expect(described_class.visible_at_version(version)).to contain_exactly(design1)
end
end
context 'at version 2' do
let(:version) { versions.second }
it 'finds the first and second designs' do
expect(described_class.visible_at_version(version)).to contain_exactly(design1, design2)
end
end
context 'at latest version' do
let(:version) { versions.first }
it 'finds designs' do
expect(described_class.visible_at_version(version)).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(described_class.visible_at_version(version)).to contain_exactly(design1, design2, design3)
end
end
it 'can be passed either a DesignManagement::Version or an ID' do
[first_version, first_version.id].each do |arg|
expect(described_class.visible_at_version(arg)).to eq([design1])
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 '.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 '#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
set(:versions) { create(:design_version) }
set(:design) { create(:design, versions: [versions]) }
let(:design) { design1 }
it "is false when there are versions" do
expect(design.new_design?).to be_falsy
expect(design1).not_to be_new_design
end
it "is true when there are no versions" do
expect(build(:design).new_design?).to be_truthy
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 versions are loaded" do
design.versions.map(&:id)
design.design_versions.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
......@@ -84,18 +239,32 @@ describe DesignManagement::Design do
end
describe '#diff_refs' do
it "builds diff refs based on the first commit and it's for the design" do
design = create(:design, :with_file, versions_count: 3)
let(:design) { create(:design, :with_file, versions_count: versions_count) }
context 'there are several versions' do
let(:versions_count) { 3 }
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)
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
it 'builds diff refs based on the empty tree if there was only one version' do
design = create(:design, :with_file, versions_count: 1)
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.base_sha).to eq(Gitlab::Git::BLANK_SHA)
expect(design.diff_refs.head_sha).to eq(design.diff_refs.head_sha)
expect(design.diff_refs).to be_nil
end
end
......
......@@ -6,4 +6,72 @@ describe DesignManagement::DesignVersion do
it { is_expected.to belong_to(:design) }
it { is_expected.to belong_to(:version) }
end
describe 'scopes' do
describe '.most_recent' do
set(:design_a) { create(:design) }
set(:design_b) { create(:design) }
set(: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
set(:issue) { create(:issue) }
set(:design_a) { create(:design, issue: issue) }
set(: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.
set(:oldest) { create(:design_version, designs: [design_a, design_b]) }
set(:middle) { create(:design_version, designs: [design_a, design_b]) }
set(: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 '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
end
end
......@@ -8,9 +8,7 @@ describe DesignManagement::Version do
it 'constrains the designs relation correctly' do
design = create(:design)
version = create(:design_version)
version.designs << design
version = create(:design_version, designs: [design])
expect { version.designs << design }.to raise_error(ActiveRecord::RecordNotUnique)
end
......@@ -29,6 +27,7 @@ describe DesignManagement::Version do
it { is_expected.to be_valid }
it { is_expected.to validate_presence_of(:sha) }
it { is_expected.to validate_presence_of(:designs) }
it { is_expected.to validate_uniqueness_of(:sha).scoped_to(:issue_id).case_insensitive }
end
......@@ -61,13 +60,147 @@ describe DesignManagement::Version do
end
end
describe ".bulk_create" do
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
set(:issue) { create(:issue) }
set(:design_a) { create(:design, issue: issue) }
set(:design_b) { create(:design, issue: issue) }
let(:designs) { [design_a, design_b] }
describe 'the error raised when there are no actions' do
let(:sha) { 'f00' }
def call_with_empty_actions
described_class.create_for_designs([], sha)
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 'then error raised when the designs come from different issues' do
let(:sha) { 'f00' }
let(:designs) { create_list(:design, 2) }
let(:actions) { as_actions(designs) }
def call_with_mismatched_designs
described_class.create_for_designs(actions, sha)
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') 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, '') rescue nil
end.not_to change { DesignManagement::DesignVersion.count }
end
it "creates a version and links it to multiple designs" do
designs = create_list(:design, 2)
actions = as_actions(designs, :create)
version = described_class.create_for_designs(designs, "abc")
version = described_class.create_for_designs(actions, "abc")
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")
expect(designs.map(&:most_recent_design_version)).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")
expect(version.issue).to eq(issue)
end
it 'modifies designs if git updated them' do
actions = as_actions(designs, :update)
described_class.create_for_designs(actions, "abc")
expect(designs.map(&:most_recent_design_version)).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")
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")
described_class.create_for_designs(as_actions(designs, :delete), "def")
expect(designs).to all(be_deleted)
described_class.create_for_designs(as_actions(designs, :create), "ghi")
expect(designs.map(&:most_recent_design_version)).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")
expect do
described_class.create_for_designs(actions, "after")
end.to change { current_version_id(design_a) }
end
end
end
......@@ -365,4 +365,37 @@ describe Issue do
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
# frozen_string_literal: true
require "spec_helper"
describe "deleting designs" do
include GraphqlHelpers
include DesignManagementTestHelpers
let(:developer) { create(:user) }
let(:current_user) { developer }
let(:issue) { create(:issue) }
let(:project) { issue.project }
let(:designs) { create_designs }
let(:variables) { {} }
let(:mutation) do
input = {
project_path: project.full_path,
iid: issue.iid,
filenames: designs.map(&:filename)
}.merge(variables)
graphql_mutation(:design_management_delete, input)
end
let(:mutation_response) { graphql_mutation_response(:design_management_delete) }
def mutate!
post_graphql_mutation(mutation, current_user: current_user)
end
before do
enable_design_management
project.add_developer(developer)
end
shared_examples 'a failed request' do
let(:the_error) { be_present }
it 'reports an error' do
mutate!
expect(graphql_errors).to include(a_hash_including('message' => the_error))
end
end
context 'the designs list is empty' do
it_behaves_like 'a failed request' do
let(:designs) { [] }
let(:the_error) { eq 'no filenames' }
end
end
context 'the designs list contains filenames we cannot find' do
it_behaves_like 'a failed request' do
let(:designs) { %w/foo bar baz/.map { |fn| OpenStruct.new(filename: fn) } }
let(:the_error) { a_string_matching %r/filenames were not found/ }
end
end
context 'the current user does not have developer access' do
it_behaves_like 'a failed request' do
let(:current_user) { create(:user) }
let(:the_error) { a_string_matching %r/you don't have permission/ }
end
end
context "when the issue does not exist" do
it_behaves_like 'a failed request' do
let(:variables) { { iid: "1234567890" } }
let(:the_error) { a_string_matching %r/does not exist/ }
end
end
context "when saving the designs raises an error" do
let(:designs) { create_designs(1) }
it "responds with errors" do
expect_next_instance_of(::DesignManagement::DeleteDesignsService) do |service|
expect(service)
.to receive(:execute)
.and_return({ status: :error, message: "Something went wrong" })
end
mutate!
expect(mutation_response).to include('errors' => include(eq "Something went wrong"))
end
end
context 'one of the designs is already deleted' do
let(:designs) do
create_designs(2).push(create(:design, :with_file, deleted: true, issue: issue))
end
it 'reports an error' do
mutate!
expect(graphql_errors).to be_present
end
end
context 'when the user names designs to delete' do
before do
create_designs(1)
end
let!(:designs) { create_designs(2) }
it 'deletes the designs' do
expect { mutate! }
.to change { issue.reset.designs.current.count }.from(3).to(1)
end
it 'has no errors' do
mutate!
expect(mutation_response).to include('errors' => be_empty)
end
end
private
def create_designs(how_many = 2)
create_list(:design, how_many, :with_file, issue: issue)
end
end
......@@ -31,16 +31,23 @@ describe "uploading designs" do
it "returns an error if the user is not allowed to upload designs" do
post_graphql_mutation(mutation, current_user: create(:user))
expect(graphql_errors).not_to be_empty
expect(graphql_errors).to be_present
end
it "responds with the created designs" do
it 'succeeds' do
post_graphql_mutation(mutation, current_user: current_user)
designs = mutation_response["designs"]
expect(graphql_errors).not_to be_present
end
it "responds with the created designs" do
post_graphql_mutation(mutation, current_user: current_user)
expect(designs.size).to eq(1)
expect(designs.first["filename"]).to eq("dk.png")
expect(mutation_response).to include(
'designs' => a_collection_containing_exactly(
a_hash_including('filename' => 'dk.png')
)
)
end
context "when the issue does not exist" do
......
# frozen_string_literal: true
require 'spec_helper'
describe DesignManagement::DeleteDesignsService do
include DesignManagementTestHelpers
set(:project) { create(:project) }
set(:issue) { create(:issue, project: project) }
set(:user) { create(:user) }
let(:designs) { create_designs }
subject(:service) { described_class.new(project, user, issue: issue, designs: designs) }
# Defined as a method so that the reponse is not cached. We also construct
# a new service executor each time to avoid the intermediate cached values
# it constructs during its execution.
def run_service(delenda = nil)
service = described_class.new(project, user, issue: issue, designs: delenda || designs)
service.execute
end
let(:response) { run_service }
shared_examples 'a service error' do
it 'returns an error', :aggregate_failures do
expect(response).to include(status: :error)
end
end
shared_examples 'a top-level error' do
let(:expected_error) { StandardError }
it 'raises an en expected error', :aggregate_failures do
expect { run_service }.to raise_error(expected_error)
end
end
shared_examples 'a success' do
it 'returns successfully', :aggregate_failures do
expect(response).to include(status: :success)
end
end
before do
enable_design_management(enabled)
project.add_developer(user)
end
describe "#execute" do
context "when the feature is not available" do
let(:enabled) { false }
it_behaves_like "a service error"
end
context "when the feature is available" do
let(:enabled) { true }
it 'is able to delete designs' do
expect(service.send(:can_delete_designs?)).to be true
end
context 'no designs were passed' do
let(:designs) { [] }
it_behaves_like "a top-level error"
it 'does not log any events' do
counter = ::Gitlab::UsageCounters::DesignsCounter
expect { run_service rescue nil }.not_to change { counter.totals }
end
end
context 'one design is passed' do
before do
create_designs(2)
end
let!(:designs) { create_designs(1) }
it 'removes that design' do
expect { run_service }.to change { issue.designs.current.count }.from(3).to(2)
end
it 'logs a deletion event' do
counter = ::Gitlab::UsageCounters::DesignsCounter
expect { run_service }.to change { counter.read(:delete) }.by(1)
end
it 'creates a new verison' do
expect { run_service }.to change { DesignManagement::Version.where(issue: issue).count }.by(1)
end
it 'returns the new version' do
version = response[:version]
expect(version).to eq(DesignManagement::Version.for_issue(issue).ordered.first)
end
it_behaves_like "a success"
it 'removes the design from the current design list' do
run_service
expect(issue.designs.current).not_to include(designs.first)
end
it 'marks the design as deleted' do
expect { run_service }
.to change { designs.first.deleted? }.from(false).to(true)
end
end
context 'more than one design is passed' do
before do
create_designs(1)
end
let!(:designs) { create_designs(2) }
it 'removes those designs' do
expect { run_service }
.to change { issue.designs.current.count }.from(3).to(1)
end
it 'logs the correct number of deletion events' do
counter = ::Gitlab::UsageCounters::DesignsCounter
expect { run_service }.to change { counter.read(:delete) }.by(2)
end
it_behaves_like "a success"
context 'after executing the service' do
let(:deleted_designs) { designs.map(&:reset) }
let!(:version) { run_service[:version] }
it 'removes the removed designs from the current design list' do
expect(issue.designs.current).not_to include(*deleted_designs)
end
it 'does not make the designs impossible to find' do
expect(issue.designs).to include(*deleted_designs)
end
it 'associates the new version with all the designs' do
current_versions = deleted_designs.map { |d| d.most_recent_design_version.version }
expect(current_versions).to all(eq version)
end
it 'marks all deleted designs as deleted' do
expect(deleted_designs).to all(be_deleted)
end
it 'marks all deleted designs with the same deletion version' do
expect(deleted_designs.map { |d| d.most_recent_design_version.version_id }.uniq)
.to have_attributes(size: 1)
end
end
end
describe 'scalability' do
before do
run_service(create_designs(1)) # ensure project, issue, etc are created
end
it 'makes the same number of DB requests for one design as for several' do
one = create_designs(1)
many = create_designs(5)
baseline = ActiveRecord::QueryRecorder.new { run_service(one) }
expect { run_service(many) }.not_to exceed_query_limit(baseline)
end
end
end
end
private
def create_designs(how_many = 2)
create_list(:design, how_many, :with_lfs_file, issue: issue)
end
end
# frozen_string_literal: true
module DesignManagementTestHelpers
def enable_design_management
stub_licensed_features(design_management: true)
stub_lfs_setting(enabled: true)
def enable_design_management(enabled = true)
stub_licensed_features(design_management: enabled)
stub_lfs_setting(enabled: enabled)
end
def delete_designs(*designs)
act_on_designs(designs) { ::DesignManagement::DesignVersion.deletion }
end
def restore_designs(*designs)
act_on_designs(designs) { ::DesignManagement::DesignVersion.creation }
end
def modify_designs(*designs)
act_on_designs(designs) { ::DesignManagement::DesignVersion.modification }
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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment