Allow a version to belong_to many desings

At first we were going to store one commit per design.

But now we commit several files at once, which means a version could
apply to multiple designs.

This updates the datamodel to reflect that

ee/app/models/design_management/design.rb

@@ -4,10 +4,9 @@ module DesignManagement
   class Design < ApplicationRecord
     belongs_to :project
     belongs_to :issue
-    has_many :versions, class_name: 'DesignManagement::Version', inverse_of: :design
+    has_and_belongs_to_many :versions, class_name: 'DesignManagement::Version', inverse_of: :designs
 
     validates :project, :issue, :filename, presence: true
-    validates :issue, uniqueness: true
     validates :filename, uniqueness: { scope: :issue_id }
   end
 end

ee/app/models/design_management/version.rb

@@ -4,15 +4,15 @@ module DesignManagement
   class Version < ApplicationRecord
     include ShaAttribute
 
-    belongs_to :design, class_name: "DesignManagement::Design", foreign_key: 'design_management_design_id'
-    has_one :project, through: :design
-    has_one :issue, through: :design
+    has_and_belongs_to_many :designs,
+                            class_name: "DesignManagement::Design",
+                            inverse_of: :versions
 
     # 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
 
-    validates :sha, :design, presence: true
+    validates :sha, presence: true
     validates :sha, uniqueness: { case_sensitive: false }
 
     sha_attribute :sha

ee/db/migrate/20190402112450_add_design_management_designs_versions.rb

 # frozen_string_literal: true
 
-# See http://doc.gitlab.com/ce/development/migration_style_guide.html
-# for more information on how to write migrations for GitLab.
-
 class AddDesignManagementDesignsVersions < ActiveRecord::Migration[5.0]
-  include Gitlab::Database::MigrationHelpers
-
-  # Set this constant to true if this migration requires downtime.
   DOWNTIME = false
 
-  # When a migration requires downtime you **must** uncomment the following
-  # constant and define a short and easy to understand explanation as to why the
-  # migration requires downtime.
-  # DOWNTIME_REASON = ''
-
-  # When using the methods "add_concurrent_index", "remove_concurrent_index" or
-  # "add_column_with_default" you must disable the use of transactions
-  # as these methods can not run in an existing transaction.
-  # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure
-  # that either of them is the _only_ method called in the migration,
-  # any other changes should go in a separate migration.
-  # This ensures that upon failure _only_ the index creation or removing fails
-  # and can be retried or reverted easily.
-  #
-  # To disable transactions uncomment the following line and remove these
-  # comments:
-  # disable_ddl_transaction!
-
   def change
     create_table(:design_management_designs_versions, id: false) do |t|
       t.references :design,

ee/spec/factories/design_management/versions.rb

@@ -2,7 +2,6 @@
 
 FactoryBot.define do
   factory :design_version, class: DesignManagement::Version do
-    design
     sequence(:sha) { |n| Digest::SHA1.hexdigest("commit-like-#{n}") }
   end
 end

ee/spec/models/design_management/design_spec.rb

@@ -6,7 +6,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(:versions) }
+    it { is_expected.to have_and_belong_to_many(:versions) }
   end
 
   describe 'validations' do
@@ -16,7 +16,6 @@ describe DesignManagement::Design do
     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(:issue) }
     it { is_expected.to validate_uniqueness_of(:filename).scoped_to(:issue_id) }
   end
 end

ee/spec/models/design_management/version_spec.rb

@@ -3,9 +3,25 @@ require 'rails_helper'
 
 describe DesignManagement::Version do
   describe 'relations' do
-    it { is_expected.to belong_to(:design) }
-    it { is_expected.to have_one(:issue) }
-    it { is_expected.to have_one(:project) }
+    it { is_expected.to have_and_belong_to_many(:designs) }
+
+    it 'constrains the designs relation correctly' do
+      design = create(:design)
+      version = create(:design_version)
+
+      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
+
     it { is_expected.to have_many(:notes).dependent(:delete_all) }
   end
 
@@ -13,7 +29,6 @@ describe DesignManagement::Version do
     subject(:design_version) { build(:design_version) }
 
     it { is_expected.to be_valid }
-    it { is_expected.to validate_presence_of(:design) }
     it { is_expected.to validate_presence_of(:sha) }
     it { is_expected.to validate_uniqueness_of(:sha).case_insensitive }
   end