Commit 5e4b4c71 authored by Luke Duncalfe's avatar Luke Duncalfe

Rename `DesignVersion` to `Action`

This change makes the join model between `Design` and `Version` easier
to reason about when using:

```ruby
design.actions
version.actions
action.design
action.version
```

It also avoids an awkward similarity in relation naming now that
`Issue.has_many :design_versions, class_name: "...::Version"`.

https://gitlab.com/gitlab-org/gitlab/issues/13825
parent 7c0c77c6
......@@ -43,9 +43,9 @@ module Types
def event(parent:)
version = cached_stateful_version(parent)
design_version = cached_design_versions_for_version(version)[design.id]
action = cached_actions_for_version(version)[design.id]
design_version&.event || Types::DesignManagement::DesignVersionEventEnum::NONE
action&.event || Types::DesignManagement::DesignVersionEventEnum::NONE
end
# Returns a `DesignManagement::Version` for this query based on the
......@@ -65,9 +65,9 @@ module Types
end
end
def cached_design_versions_for_version(version)
Gitlab::SafeRequestStore.fetch([request_cache_base_key, 'design_versions_for_version', version.id]) do
version.design_versions.to_h { |dv| [dv.design_id, dv] }
def cached_actions_for_version(version)
Gitlab::SafeRequestStore.fetch([request_cache_base_key, 'actions_for_version', version.id]) do
version.actions.to_h { |dv| [dv.design_id, dv] }
end
end
......
......@@ -10,7 +10,7 @@ module Types
value NONE, 'No change'
::DesignManagement::DesignVersion.events.keys.each do |event_name|
::DesignManagement::Action.events.keys.each do |event_name|
value event_name.upcase, value: event_name, description: "A #{event_name} event"
end
end
......
# frozen_string_literal: true
module DesignManagement
class DesignVersion < ApplicationRecord
class Action < ApplicationRecord
self.table_name = "#{DesignManagement.table_name_prefix}designs_versions"
belongs_to :design, class_name: "DesignManagement::Design", inverse_of: :design_versions
belongs_to :version, class_name: "DesignManagement::Version", inverse_of: :design_versions
belongs_to :design, class_name: "DesignManagement::Design", inverse_of: :actions
belongs_to :version, class_name: "DesignManagement::Version", inverse_of: :actions
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
# For each design, only select the most recent action
scope :most_recent, -> do
selection = Arel.sql("DISTINCT ON (#{table_name}.design_id) #{table_name}.*")
......
......@@ -10,8 +10,8 @@ module DesignManagement
belongs_to :project, inverse_of: :designs
belongs_to :issue
has_many :design_versions
has_many :versions, through: :design_versions, class_name: 'DesignManagement::Version', inverse_of: :designs
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
......@@ -35,16 +35,16 @@ module DesignManagement
# 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::DesignVersion.events[:deletion]
deletion = ::DesignManagement::Action.events[:deletion]
designs = arel_table
design_versions = ::DesignManagement::DesignVersion
actions = ::DesignManagement::Action
.most_recent.up_to_version(version)
.arel.as('most_recent_design_versions')
.arel.as('most_recent_actions')
join = designs.join(design_versions)
.on(design_versions[:design_id].eq(designs[:id]))
join = designs.join(actions)
.on(actions[:design_id].eq(designs[:id]))
joins(join.join_sources).where(design_versions[:event].not_eq(deletion))
joins(join.join_sources).where(actions[:event].not_eq(deletion))
end
# A design is current if the most recent event is not a deletion
......@@ -61,11 +61,11 @@ module DesignManagement
end
def deleted?
most_recent_design_version&.deletion?
most_recent_action&.deletion?
end
def most_recent_design_version
strong_memoize(:most_recent_design_version) { design_versions.ordered.last }
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
......@@ -90,7 +90,7 @@ module DesignManagement
end
def new_design?
strong_memoize(:new_design) { design_versions.none? }
strong_memoize(:new_design) { actions.none? }
end
def full_path
......@@ -104,8 +104,8 @@ module DesignManagement
end
def clear_version_cache
[versions, design_versions].each(&:reset)
[:new_design, :diff_refs, :head_sha, :most_recent_design_version].each do |key|
[versions, actions].each(&:reset)
[:new_design, :diff_refs, :head_sha, :most_recent_action].each do |key|
clear_memoization(key)
end
end
......
......@@ -8,9 +8,9 @@ module DesignManagement
include ActiveModel::Validations
EVENT_FOR_GITALY_ACTION = {
create: DesignManagement::DesignVersion.events[:creation],
update: DesignManagement::DesignVersion.events[:modification],
delete: DesignManagement::DesignVersion.events[:deletion]
create: DesignManagement::Action.events[:creation],
update: DesignManagement::Action.events[:modification],
delete: DesignManagement::Action.events[:deletion]
}.freeze
attr_reader :design, :action, :content
......
......@@ -27,9 +27,9 @@ module DesignManagement
end
belongs_to :issue
has_many :design_versions
has_many :actions
has_many :designs,
through: :design_versions,
through: :actions,
class_name: "DesignManagement::Design",
source: :design,
inverse_of: :versions
......@@ -45,7 +45,7 @@ module DesignManagement
sha_attribute :sha
scope :for_designs, -> (designs) do
where(id: DesignVersion.where(design_id: designs).select(:version_id)).distinct
where(id: Action.where(design_id: designs).select(:version_id)).distinct
end
scope :earlier_or_equal_to, -> (version) { where('id <= ?', version) }
scope :ordered, -> { order(id: :desc) }
......@@ -73,7 +73,7 @@ module DesignManagement
rows = design_actions.map { |action| action.row_attrs(version) }
Gitlab::Database.bulk_insert(DesignVersion.table_name, rows)
Gitlab::Database.bulk_insert(Action.table_name, rows)
version.designs.reset
version.validate!
design_actions.each(&:performed)
......
......@@ -11,7 +11,7 @@ FactoryBot.define do
project = design.project
repository = project.design_repository
repository.create_if_not_exists
dv_table_name = DesignManagement::DesignVersion.table_name
dv_table_name = DesignManagement::Action.table_name
updates = [0, evaluator.versions_count - (evaluator.deleted ? 2 : 1)].max
run_action = ->(action) do
......
......@@ -111,7 +111,7 @@ describe Mutations::DesignManagement::Delete do
# 22. leave transaction 2
# 23. create version with sha and issue
# 24. create design-version links
# 25. validate version.design_versions.present?
# 25. validate version.actions.present?
# 26. validate version.sha is unique
# 27. leave transaction 1
#
......
# frozen_string_literal: true
require 'spec_helper'
describe DesignManagement::DesignVersion do
describe DesignManagement::Action do
describe 'relations' do
it { is_expected.to belong_to(:design) }
it { is_expected.to belong_to(:version) }
......
......@@ -14,7 +14,7 @@ describe DesignManagement::Design do
describe 'relations' do
it { is_expected.to belong_to(:project) }
it { is_expected.to belong_to(:issue) }
it { is_expected.to have_many(:design_versions) }
it { is_expected.to have_many(:actions) }
it { is_expected.to have_many(:versions) }
it { is_expected.to have_many(:notes).dependent(:delete_all) }
end
......@@ -205,8 +205,8 @@ describe DesignManagement::Design do
expect(deleted_design).not_to be_new_design
end
it "does not cause extra queries when versions are loaded" do
design.design_versions.map(&:id)
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
......
......@@ -3,8 +3,8 @@ require 'spec_helper'
describe DesignManagement::Version do
describe 'relations' do
it { is_expected.to have_many(:design_versions) }
it { is_expected.to have_many(:designs).through(:design_versions) }
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)
......@@ -139,7 +139,7 @@ describe DesignManagement::Version do
actions = as_actions(designs)
expect do
described_class.create_for_designs(actions, '') rescue nil
end.not_to change { DesignManagement::DesignVersion.count }
end.not_to change { DesignManagement::Action.count }
end
it "creates a version and links it to multiple designs" do
......@@ -156,7 +156,7 @@ describe DesignManagement::Version do
described_class.create_for_designs(actions, "abc")
expect(designs.map(&:most_recent_design_version)).to all(be_creation)
expect(designs.map(&:most_recent_action)).to all(be_creation)
end
it 'correctly associates the version with the issue' do
......@@ -172,7 +172,7 @@ describe DesignManagement::Version do
described_class.create_for_designs(actions, "abc")
expect(designs.map(&:most_recent_design_version)).to all(be_modification)
expect(designs.map(&:most_recent_action)).to all(be_modification)
end
it 'deletes designs when the git action was delete' do
......@@ -191,7 +191,7 @@ describe DesignManagement::Version do
described_class.create_for_designs(as_actions(designs, :create), "ghi")
expect(designs.map(&:most_recent_design_version)).to all(be_creation)
expect(designs.map(&:most_recent_action)).to all(be_creation)
expect(designs).not_to include(be_deleted)
end
......
......@@ -231,7 +231,7 @@ describe "Getting designs related to an issue" do
let(:version) { all_versions.last }
before do
second_design.design_versions.create!(version: version, event: "modification")
second_design.actions.create!(version: version, event: "modification")
post_graphql(query, current_user: current_user)
end
......
......@@ -142,7 +142,7 @@ describe DesignManagement::DeleteDesignsService do
end
it 'associates the new version with all the designs' do
current_versions = deleted_designs.map { |d| d.most_recent_design_version.version }
current_versions = deleted_designs.map { |d| d.most_recent_action.version }
expect(current_versions).to all(eq version)
end
......@@ -151,7 +151,7 @@ describe DesignManagement::DeleteDesignsService do
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)
expect(deleted_designs.map { |d| d.most_recent_action.version_id }.uniq)
.to have_attributes(size: 1)
end
end
......
......@@ -7,15 +7,15 @@ module DesignManagementTestHelpers
end
def delete_designs(*designs)
act_on_designs(designs) { ::DesignManagement::DesignVersion.deletion }
act_on_designs(designs) { ::DesignManagement::Action.deletion }
end
def restore_designs(*designs)
act_on_designs(designs) { ::DesignManagement::DesignVersion.creation }
act_on_designs(designs) { ::DesignManagement::Action.creation }
end
def modify_designs(*designs)
act_on_designs(designs) { ::DesignManagement::DesignVersion.modification }
act_on_designs(designs) { ::DesignManagement::Action.modification }
end
private
......
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