Merge branch 'epics-conf-db' into 'master'

Add epic confidentiality attribute and model validations

See merge request gitlab-org/gitlab!28428

db/migrate/20200330121000_add_confidential_attribute_to_epics.rb

+# frozen_string_literal: true
+
+class AddConfidentialAttributeToEpics < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column_with_default(:epics, :confidential, :boolean, default: false)
+  end
+
+  def down
+    remove_column(:epics, :confidential)
+  end
+end

db/structure.sql

@@ -2271,6 +2271,7 @@ CREATE TABLE public.epics (
     state_id smallint DEFAULT 1 NOT NULL,
     start_date_sourcing_epic_id integer,
     due_date_sourcing_epic_id integer,
+    confidential boolean DEFAULT false NOT NULL,
     external_key character varying(255)
 );
 
@@ -12925,6 +12926,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200326144443
 20200326145443
 20200330074719
+20200330121000
 20200330132913
 20200331220930
 \.

ee/app/models/ee/epic.rb

@@ -55,6 +55,8 @@ module EE
 
       validates :group, presence: true
       validate :validate_parent, on: :create
+      validate :validate_confidential_issues_and_subepics
+      validate :validate_confidential_parent
 
       alias_attribute :parent_ids, :parent_id
       alias_method :issuing_parent, :group
@@ -67,6 +69,7 @@ module EE
       scope :in_issues, -> (issues) { joins(:epic_issues).where(epic_issues: { issue_id: issues }).distinct }
       scope :has_parent, -> { where.not(parent_id: nil) }
       scope :iid_starts_with, -> (query) { where("CAST(iid AS VARCHAR) LIKE ?", "#{sanitize_sql_like(query)}%") }
+      scope :public_only, -> { where(confidential: false) }
 
       scope :within_timeframe, -> (start_date, end_date) do
         where('start_date is not NULL or end_date is not NULL')
@@ -385,5 +388,25 @@ module EE
 
       hierarchy.base_and_ancestors(hierarchy_order: :asc)
     end
+
+    def validate_confidential_issues_and_subepics
+      return unless confidential?
+
+      if issues.public_only.any?
+        errors.add :confidential, _('Cannot make epic confidential if it contains not-confidential issues')
+      end
+
+      if children.public_only.any?
+        errors.add :confidential, _('Cannot make epic confidential if it contains not-confidential sub-epics')
+      end
+    end
+
+    def validate_confidential_parent
+      return unless parent
+
+      if !confidential? && parent.confidential?
+        errors.add :confidential, _('Not-confidential epic cannot be assigned to a confidential parent epic')
+      end
+    end
   end
 end

ee/app/models/ee/issue.rb

@@ -45,6 +45,7 @@ module EE
       has_many :related_vulnerabilities, through: :vulnerability_links, source: :vulnerability
 
       validates :weight, allow_nil: true, numericality: { greater_than_or_equal_to: 0 }
+      validate :validate_confidential_epic
 
       after_create :update_generic_alert_title, if: :generic_alert_with_default_title?
     end
@@ -223,5 +224,13 @@ module EE
         project.alerts_service_activated? &&
         author == ::User.alert_bot
     end
+
+    def validate_confidential_epic
+      return unless epic
+
+      if !confidential? && epic.confidential?
+        errors.add :issue, _('Cannot set confidential epic for not-confidential issue')
+      end
+    end
   end
 end

ee/app/models/epic_issue.rb

@@ -15,4 +15,16 @@ class EpicIssue < ApplicationRecord
 
   scope :in_epic, ->(epic_id) { where(epic_id: epic_id) }
   scope :related_issues_for_batches, ->(epic_ids) { select(:id, :relative_position).where(epic_id: epic_ids) }
+
+  validate :validate_confidential_epic
+
+  private
+
+  def validate_confidential_epic
+    return unless epic && issue
+
+    if epic.confidential? && !issue.confidential?
+      errors.add :issue, _('Cannot set confidential epic for not-confidential issue')
+    end
+  end
 end

ee/changelogs/unreleased/epics-conf-db.yml

+---
+title: Added confidential column to epics table
+merge_request: 28428
+author:
+type: added

ee/spec/factories/epics.rb

@@ -15,6 +15,10 @@ FactoryBot.define do
       due_date_is_fixed { true }
     end
 
+    trait :confidential do
+      confidential { true }
+    end
+
     trait :opened do
       state { :opened }
     end

ee/spec/models/epic_issue_spec.rb

@@ -3,6 +3,29 @@
 require 'spec_helper'
 
 describe EpicIssue do
+  describe 'validations' do
+    let(:epic) { build(:epic) }
+    let(:confidential_epic) { build(:epic, :confidential) }
+    let(:issue) { build(:issue) }
+    let(:confidential_issue) { build(:issue, :confidential) }
+
+    it 'is valid to add not-confidential issue to not-confidential epic' do
+      expect(build(:epic_issue, epic: epic, issue: issue)).to be_valid
+    end
+
+    it 'is valid to add confidential issue to confidential epic' do
+      expect(build(:epic_issue, epic: confidential_epic, issue: confidential_issue)).to be_valid
+    end
+
+    it 'is valid to add confidential issue to not-confidential epic' do
+      expect(build(:epic_issue, epic: epic, issue: confidential_issue)).to be_valid
+    end
+
+    it 'is not valid to add not-confidential issue to confidential epic' do
+      expect(build(:epic_issue, epic: confidential_epic, issue: issue)).not_to be_valid
+    end
+  end
+
   context "relative positioning" do
     it_behaves_like "a class that supports relative positioning" do
       let(:epic) { create(:epic) }

ee/spec/models/epic_spec.rb

@@ -19,6 +19,17 @@ describe Epic do
     it { is_expected.to have_many(:user_mentions).class_name("EpicUserMention") }
   end
 
+  describe 'scopes' do
+    describe '.public_only' do
+      it 'only returns public epics' do
+        public_epic = create(:epic)
+        create(:epic, confidential: true)
+
+        expect(described_class.public_only).to eq([public_epic])
+      end
+    end
+  end
+
   describe 'validations' do
     subject { build(:epic) }
 
@@ -37,6 +48,41 @@ describe Epic do
 
       expect(epic).not_to be_valid
     end
+
+    it 'is valid if epic is confidential and has only confidential issues' do
+      issue = create(:issue, :confidential)
+      epic = create(:epic_issue, issue: issue).epic
+
+      epic.confidential = true
+
+      expect(epic).to be_valid
+    end
+
+    it 'is not valid if epic is confidential and has not-confidential issues' do
+      epic = create(:epic_issue).epic
+
+      epic.confidential = true
+
+      expect(epic).not_to be_valid
+    end
+
+    it 'is valid if epic is confidential and has only confidential subepics' do
+      epic = create(:epic, group: group)
+      create(:epic, :confidential, parent: epic, group: group)
+
+      epic.confidential = true
+
+      expect(epic).to be_valid
+    end
+
+    it 'is not valid if epic is confidential and has not-confidential subepics' do
+      epic = create(:epic, group: group)
+      create(:epic, parent: epic, group: group)
+
+      epic.confidential = true
+
+      expect(epic).not_to be_valid
+    end
   end
 
   describe 'modules' do

ee/spec/models/issue_spec.rb

@@ -105,9 +105,9 @@ describe Issue do
   end
 
   describe 'validations' do
-    subject { build(:issue) }
-
     describe 'weight' do
+      subject { build(:issue) }
+
       it 'is not valid when negative number' do
         subject.weight = -1
 
@@ -125,6 +125,26 @@ describe Issue do
         expect(subject).to be_valid
       end
     end
+
+    describe 'confidential' do
+      subject { build(:issue, :confidential) }
+
+      it 'is valid when changing to not-confidential and is associated with not-confidential epic' do
+        subject.epic = build(:epic)
+
+        subject.confidential = false
+
+        expect(subject).to be_valid
+      end
+
+      it 'is not valid when changing to not-confidential and is associated with confidential epic' do
+        subject.epic = build(:epic, :confidential)
+
+        subject.confidential = false
+
+        expect(subject).not_to be_valid
+      end
+    end
   end
 
   describe 'relations' do

ee/spec/requests/api/graphql/mutations/epics/add_issue_spec.rb

@@ -50,7 +50,7 @@ describe 'Add an issue to an Epic' do
         expect(response['errors']).to be_empty
         expect(response['epicIssue']['iid']).to eq(issue.iid.to_s)
         expect(response['epicIssue']['epic']['iid']).to eq(epic.iid.to_s)
-        expect(issue.epic).to eq(epic)
+        expect(issue.reload.epic).to eq(epic)
       end
     end
 

ee/spec/services/epic_issues/create_service_spec.rb

@@ -239,6 +239,7 @@ describe EpicIssues::CreateService do
         before do
           group.add_developer(user)
           create(:epic_issue, epic: epic, issue: issue)
+          issue.reload
         end
 
         let(:another_epic) { create(:epic, group: group) }

locale/gitlab.pot

@@ -3353,6 +3353,12 @@ msgstr ""
 msgid "Cannot create the abuse report. This user has been blocked."
 msgstr ""
 
+msgid "Cannot make epic confidential if it contains not-confidential issues"
+msgstr ""
+
+msgid "Cannot make epic confidential if it contains not-confidential sub-epics"
+msgstr ""
+
 msgid "Cannot merge"
 msgstr ""
 
@@ -3371,6 +3377,9 @@ msgstr ""
 msgid "Cannot refer to a group milestone by an internal id!"
 msgstr ""
 
+msgid "Cannot set confidential epic for not-confidential issue"
+msgstr ""
+
 msgid "Cannot show preview. For previews on sketch files, they must have the file format introduced by Sketch version 43 and above."
 msgstr ""
 
@@ -13604,6 +13613,9 @@ msgstr ""
 msgid "Not started"
 msgstr ""
 
+msgid "Not-confidential epic cannot be assigned to a confidential parent epic"
+msgstr ""
+
 msgid "Note"
 msgstr ""
 

spec/lib/gitlab/import_export/safe_model_attributes.yml

@@ -845,6 +845,7 @@ Epic:
   - due_date_sourcing_epic_id
   - health_status
   - external_key
+  - confidential
 EpicIssue:
   - id
   - relative_position