Merge branch '205570-sprint_relationships' into 'master'

Add Sprint relationships and constraints

See merge request gitlab-org/gitlab!30127

app/models/internal_id_enums.rb

@@ -3,7 +3,18 @@
 module InternalIdEnums
   def self.usage_resources
     # when adding new resource, make sure it doesn't conflict with EE usage_resources
-    { issues: 0, merge_requests: 1, deployments: 2, milestones: 3, epics: 4, ci_pipelines: 5, operations_feature_flags: 6, operations_user_lists: 7, alert_management_alerts: 8 }
+    {
+        issues: 0,
+        merge_requests: 1,
+        deployments: 2,
+        milestones: 3,
+        epics: 4,
+        ci_pipelines: 5,
+        operations_feature_flags: 6,
+        operations_user_lists: 7,
+        alert_management_alerts: 8,
+        sprints: 9
+    }
   end
 end
 

app/models/issue.rb

@@ -31,6 +31,7 @@ class Issue < ApplicationRecord
   belongs_to :project
   belongs_to :duplicated_to, class_name: 'Issue'
   belongs_to :closed_by, class_name: 'User'
+  belongs_to :sprint
 
   belongs_to :moved_to, class_name: 'Issue'
   has_one :moved_from, class_name: 'Issue', foreign_key: :moved_to_id

app/models/merge_request.rb

@@ -32,6 +32,7 @@ class MergeRequest < ApplicationRecord
   belongs_to :target_project, class_name: "Project"
   belongs_to :source_project, class_name: "Project"
   belongs_to :merge_user, class_name: "User"
+  belongs_to :sprint
 
   has_internal_id :iid, scope: :target_project, track_if: -> { !importing? }, init: ->(s) { s&.target_project&.merge_requests&.maximum(:iid) }
 

app/models/sprint.rb

 # frozen_string_literal: true
 
 class Sprint < ApplicationRecord
+  STATE_ID_MAP = {
+      active: 1,
+      closed: 2
+  }.with_indifferent_access.freeze
+
+  include AtomicInternalId
+
+  has_many :issues
+  has_many :merge_requests
+
   belongs_to :project
   belongs_to :group
+
+  has_internal_id :iid, scope: :project, init: ->(s) { s&.project&.sprints&.maximum(:iid) }
+  has_internal_id :iid, scope: :group, init: ->(s) { s&.group&.sprints&.maximum(:iid) }
 end

changelogs/unreleased/205570-sprint_relationships.yml

+---
+title: Flesh out Sprints relationships and constraints
+merge_request: 30127
+author:
+type: added

db/migrate/20200304023245_add_sprint_to_issues.rb

+# frozen_string_literal: true
+
+class AddSprintToIssues < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      # index will be added in another migration with `add_concurrent_index`
+      add_column :issues, :sprint_id, :bigint
+    end
+  end
+
+  def down
+    with_lock_retries do
+      remove_column :issues, :sprint_id
+    end
+  end
+end

db/migrate/20200304023851_add_sprint_to_merge_requests.rb

+# frozen_string_literal: true
+
+class AddSprintToMergeRequests < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      # index will be added in another migration with `add_concurrent_index`
+      add_column :merge_requests, :sprint_id, :bigint
+    end
+  end
+
+  def down
+    with_lock_retries do
+      remove_column :merge_requests, :sprint_id
+    end
+  end
+end

db/migrate/20200304024025_add_sprint_id_index_to_issues.rb

+# frozen_string_literal: true
+
+class AddSprintIdIndexToIssues < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :issues, :sprint_id
+    add_concurrent_foreign_key :issues, :sprints, column: :sprint_id
+  end
+
+  def down
+    with_lock_retries do # rubocop:disable Migration/WithLockRetriesWithoutDdlTransaction
+      remove_foreign_key :issues, column: :sprint_id
+    end
+    remove_concurrent_index :issues, :sprint_id
+  end
+end

db/migrate/20200304024042_add_sprint_id_index_to_merge_requests.rb

+# frozen_string_literal: true
+
+class AddSprintIdIndexToMergeRequests < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :merge_requests, :sprint_id
+    add_concurrent_foreign_key :merge_requests, :sprints, column: :sprint_id
+  end
+
+  def down
+    with_lock_retries do # rubocop:disable Migration/WithLockRetriesWithoutDdlTransaction
+      remove_foreign_key :merge_requests, column: :sprint_id
+    end
+    remove_concurrent_index :merge_requests, :sprint_id
+  end
+end

db/migrate/20200420201933_add_check_constraint_to_sprint_must_belong_to_project_or_group.rb

+# frozen_string_literal: true
+
+class AddCheckConstraintToSprintMustBelongToProjectOrGroup < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  CONSTRAINT_NAME = 'sprints_must_belong_to_project_or_group'
+
+  def up
+    add_check_constraint :sprints, '(project_id != NULL AND group_id IS NULL) OR (group_id != NULL AND project_id IS NULL)', CONSTRAINT_NAME
+  end
+
+  def down
+    remove_check_constraint :sprints, CONSTRAINT_NAME
+  end
+end

db/structure.sql

@@ -3357,7 +3357,8 @@ CREATE TABLE public.issues (
     duplicated_to_id integer,
     promoted_to_epic_id integer,
     health_status smallint,
-    external_key character varying(255)
+    external_key character varying(255),
+    sprint_id bigint
 );
 
 CREATE SEQUENCE public.issues_id_seq
@@ -3932,7 +3933,8 @@ CREATE TABLE public.merge_requests (
     allow_maintainer_to_push boolean,
     state_id smallint DEFAULT 1 NOT NULL,
     rebase_jid character varying,
-    squash_commit_sha bytea
+    squash_commit_sha bytea,
+    sprint_id bigint
 );
 
 CREATE TABLE public.merge_requests_closing_issues (
@@ -6106,6 +6108,7 @@ CREATE TABLE public.sprints (
     title_html text,
     description text,
     description_html text,
+    CONSTRAINT sprints_must_belong_to_project_or_group CHECK ((((project_id <> NULL::bigint) AND (group_id IS NULL)) OR ((group_id <> NULL::bigint) AND (project_id IS NULL)))),
     CONSTRAINT sprints_title CHECK ((char_length(title) <= 255))
 );
 
@@ -9598,6 +9601,8 @@ CREATE INDEX index_issues_on_promoted_to_epic_id ON public.issues USING btree (p
 
 CREATE INDEX index_issues_on_relative_position ON public.issues USING btree (relative_position);
 
+CREATE INDEX index_issues_on_sprint_id ON public.issues USING btree (sprint_id);
+
 CREATE INDEX index_issues_on_title_trigram ON public.issues USING gin (title public.gin_trgm_ops);
 
 CREATE INDEX index_issues_on_updated_at ON public.issues USING btree (updated_at);
@@ -9760,6 +9765,8 @@ CREATE INDEX index_merge_requests_on_source_branch ON public.merge_requests USIN
 
 CREATE INDEX index_merge_requests_on_source_project_id_and_source_branch ON public.merge_requests USING btree (source_project_id, source_branch);
 
+CREATE INDEX index_merge_requests_on_sprint_id ON public.merge_requests USING btree (sprint_id);
+
 CREATE INDEX index_merge_requests_on_target_branch ON public.merge_requests USING btree (target_branch);
 
 CREATE UNIQUE INDEX index_merge_requests_on_target_project_id_and_iid ON public.merge_requests USING btree (target_project_id, iid);
@@ -10849,6 +10856,9 @@ ALTER TABLE ONLY public.push_event_payloads
 ALTER TABLE ONLY public.ci_builds
     ADD CONSTRAINT fk_3a9eaa254d FOREIGN KEY (stage_id) REFERENCES public.ci_stages(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY public.issues
+    ADD CONSTRAINT fk_3b8c72ea56 FOREIGN KEY (sprint_id) REFERENCES public.sprints(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY public.epics
     ADD CONSTRAINT fk_3c1fd1cccc FOREIGN KEY (due_date_sourcing_milestone_id) REFERENCES public.milestones(id) ON DELETE SET NULL;
 
@@ -10966,6 +10976,9 @@ ALTER TABLE ONLY public.vulnerabilities
 ALTER TABLE ONLY public.labels
     ADD CONSTRAINT fk_7de4989a69 FOREIGN KEY (project_id) REFERENCES public.projects(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY public.merge_requests
+    ADD CONSTRAINT fk_7e85395a64 FOREIGN KEY (sprint_id) REFERENCES public.sprints(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY public.merge_request_metrics
     ADD CONSTRAINT fk_7f28d925f3 FOREIGN KEY (merged_by_id) REFERENCES public.users(id) ON DELETE SET NULL;
 
@@ -13285,6 +13298,10 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200303055348
 20200303074328
 20200303181648
+20200304023245
+20200304023851
+20200304024025
+20200304024042
 20200304085423
 20200304090155
 20200304121828
@@ -13457,6 +13474,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200420172113
 20200420172752
 20200420172927
+20200420201933
 20200421233150
 20200423075720
 20200423080334

lib/gitlab/import_export/project/import_export.yml

@@ -190,6 +190,7 @@ excluded_attributes:
     - :merge_request_diff_id
   issues:
     - :milestone_id
+    - :sprint_id
     - :moved_to_id
     - :sent_notifications
     - :state_id
@@ -197,6 +198,7 @@ excluded_attributes:
     - :promoted_to_epic_id
   merge_request:
     - :milestone_id
+    - :sprint_id
     - :ref_fetched
     - :merge_jid
     - :rebase_jid
@@ -205,6 +207,7 @@ excluded_attributes:
     - :state_id
   merge_requests:
     - :milestone_id
+    - :sprint_id
     - :ref_fetched
     - :merge_jid
     - :rebase_jid

spec/factories/sprints.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :sprint do
+    title
+
+    transient do
+      project { nil }
+      group { nil }
+      project_id { nil }
+      group_id { nil }
+      resource_parent { nil }
+    end
+
+    trait :active do
+      state { Sprint::STATE_ID_MAP[:active] }
+    end
+
+    trait :closed do
+      state { Sprint::STATE_ID_MAP[:closed] }
+    end
+
+    trait :with_dates do
+      start_date { Date.new(2000, 1, 1) }
+      due_date { Date.new(2000, 1, 30) }
+    end
+
+    after(:build, :stub) do |sprint, evaluator|
+      if evaluator.group
+        sprint.group = evaluator.group
+      elsif evaluator.group_id
+        sprint.group_id = evaluator.group_id
+      elsif evaluator.project
+        sprint.project = evaluator.project
+      elsif evaluator.project_id
+        sprint.project_id = evaluator.project_id
+      elsif evaluator.resource_parent
+        id = evaluator.resource_parent.id
+        evaluator.resource_parent.is_a?(Group) ? evaluator.group_id = id : evaluator.project_id = id
+      else
+        sprint.project = create(:project)
+      end
+    end
+
+    factory :active_sprint, traits: [:active]
+    factory :closed_sprint, traits: [:closed]
+  end
+end

spec/lib/gitlab/import_export/all_models.yml

@@ -6,6 +6,7 @@ issues:
 - assignees
 - updated_by
 - milestone
+- sprint
 - notes
 - resource_label_events
 - resource_weight_events
@@ -113,6 +114,7 @@ merge_requests:
 - assignee
 - updated_by
 - milestone
+- sprint
 - notes
 - resource_label_events
 - resource_milestone_events

spec/models/group_spec.rb

@@ -24,6 +24,8 @@ describe Group do
     it { is_expected.to have_many(:cluster_groups).class_name('Clusters::Group') }
     it { is_expected.to have_many(:clusters).class_name('Clusters::Cluster') }
     it { is_expected.to have_many(:container_repositories) }
+    it { is_expected.to have_many(:milestones) }
+    it { is_expected.to have_many(:sprints) }
 
     it_behaves_like 'model with wiki' do
       let(:container) { create(:group, :nested, :wiki_repo) }

spec/models/issue_spec.rb

@@ -7,6 +7,7 @@ describe Issue do
 
   describe "Associations" do
     it { is_expected.to belong_to(:milestone) }
+    it { is_expected.to belong_to(:sprint) }
     it { is_expected.to belong_to(:project) }
     it { is_expected.to belong_to(:moved_to).class_name('Issue') }
     it { is_expected.to have_one(:moved_from).class_name('Issue') }

spec/models/merge_request_spec.rb

@@ -18,6 +18,8 @@ describe MergeRequest do
     it { is_expected.to have_many(:assignees).through(:merge_request_assignees) }
     it { is_expected.to have_many(:merge_request_diffs) }
     it { is_expected.to have_many(:user_mentions).class_name("MergeRequestUserMention") }
+    it { is_expected.to belong_to(:milestone) }
+    it { is_expected.to belong_to(:sprint) }
 
     context 'for forks' do
       let!(:project) { create(:project) }

spec/models/project_spec.rb

@@ -20,6 +20,7 @@ describe Project do
     it { is_expected.to have_many(:merge_requests) }
     it { is_expected.to have_many(:issues) }
     it { is_expected.to have_many(:milestones) }
+    it { is_expected.to have_many(:sprints) }
     it { is_expected.to have_many(:project_members).dependent(:delete_all) }
     it { is_expected.to have_many(:users).through(:project_members) }
     it { is_expected.to have_many(:requesters).dependent(:delete_all) }

spec/models/sprint_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Sprint do
+  let!(:project) { create(:project) }
+  let!(:group) { create(:group) }
+
+  describe 'modules' do
+    context 'with a project' do
+      it_behaves_like 'AtomicInternalId' do
+        let(:internal_id_attribute) { :iid }
+        let(:instance) { build(:sprint, project: build(:project), group: nil) }
+        let(:scope) { :project }
+        let(:scope_attrs) { { project: instance.project } }
+        let(:usage) {:sprints }
+      end
+    end
+
+    context 'with a group' do
+      it_behaves_like 'AtomicInternalId' do
+        let(:internal_id_attribute) { :iid }
+        let(:instance) { build(:sprint, project: nil, group: build(:group)) }
+        let(:scope) { :group }
+        let(:scope_attrs) { { namespace: instance.group } }
+        let(:usage) {:sprints }
+      end
+    end
+  end
+
+  describe "Associations" do
+    it { is_expected.to belong_to(:project) }
+    it { is_expected.to belong_to(:group) }
+    it { is_expected.to have_many(:issues) }
+    it { is_expected.to have_many(:merge_requests) }
+  end
+
+  describe "#iid" do
+    it "is properly scoped on project and group" do
+      sprint1 = create(:sprint, project: project)
+      sprint2 = create(:sprint, project: project)
+      sprint3 = create(:sprint, group: group)
+      sprint4 = create(:sprint, group: group)
+      sprint5 = create(:sprint, project: project)
+
+      want = {
+          sprint1: 1,
+          sprint2: 2,
+          sprint3: 1,
+          sprint4: 2,
+          sprint5: 3
+      }
+      got = {
+          sprint1: sprint1.iid,
+          sprint2: sprint2.iid,
+          sprint3: sprint3.iid,
+          sprint4: sprint4.iid,
+          sprint5: sprint5.iid
+      }
+      expect(got).to eq(want)
+    end
+  end
+end