Commit 536d56b0 authored by Felipe Artur's avatar Felipe Artur

Allow to change issues/epics health status

Allow IssuableBaseService to save issues/epics health_status_id value.
parent be711a8d
...@@ -11,10 +11,14 @@ class IssuableBaseService < BaseService ...@@ -11,10 +11,14 @@ class IssuableBaseService < BaseService
@skip_milestone_email = @params.delete(:skip_milestone_email) @skip_milestone_email = @params.delete(:skip_milestone_email)
end end
def filter_params(issuable) def can_admin_issuable?(issuable)
ability_name = :"admin_#{issuable.to_ability_name}" ability_name = :"admin_#{issuable.to_ability_name}"
unless can?(current_user, ability_name, issuable) can?(current_user, ability_name, issuable)
end
def filter_params(issuable)
unless can_admin_issuable?(issuable)
params.delete(:milestone_id) params.delete(:milestone_id)
params.delete(:labels) params.delete(:labels)
params.delete(:add_label_ids) params.delete(:add_label_ids)
......
# frozen_string_literal: true
class AddHealthStatusToEpics < ActiveRecord::Migration[6.0]
DOWNTIME = false
def change
add_column :epics, :health_status, :integer, limit: 2
end
end
# frozen_string_literal: true
class AddHealthStatusToIssues < ActiveRecord::Migration[6.0]
DOWNTIME = false
def change
add_column :issues, :health_status, :integer, limit: 2
end
end
...@@ -1554,6 +1554,7 @@ ActiveRecord::Schema.define(version: 2020_02_07_151640) do ...@@ -1554,6 +1554,7 @@ ActiveRecord::Schema.define(version: 2020_02_07_151640) do
t.integer "state_id", limit: 2, default: 1, null: false t.integer "state_id", limit: 2, default: 1, null: false
t.integer "start_date_sourcing_epic_id" t.integer "start_date_sourcing_epic_id"
t.integer "due_date_sourcing_epic_id" t.integer "due_date_sourcing_epic_id"
t.integer "health_status", limit: 2
t.index ["assignee_id"], name: "index_epics_on_assignee_id" t.index ["assignee_id"], name: "index_epics_on_assignee_id"
t.index ["author_id"], name: "index_epics_on_author_id" t.index ["author_id"], name: "index_epics_on_author_id"
t.index ["closed_by_id"], name: "index_epics_on_closed_by_id" t.index ["closed_by_id"], name: "index_epics_on_closed_by_id"
...@@ -2181,6 +2182,7 @@ ActiveRecord::Schema.define(version: 2020_02_07_151640) do ...@@ -2181,6 +2182,7 @@ ActiveRecord::Schema.define(version: 2020_02_07_151640) do
t.integer "state_id", limit: 2, default: 1, null: false t.integer "state_id", limit: 2, default: 1, null: false
t.integer "duplicated_to_id" t.integer "duplicated_to_id"
t.integer "promoted_to_epic_id" t.integer "promoted_to_epic_id"
t.integer "health_status", limit: 2
t.index ["author_id"], name: "index_issues_on_author_id" t.index ["author_id"], name: "index_issues_on_author_id"
t.index ["closed_by_id"], name: "index_issues_on_closed_by_id" t.index ["closed_by_id"], name: "index_issues_on_closed_by_id"
t.index ["confidential"], name: "index_issues_on_confidential" t.index ["confidential"], name: "index_issues_on_confidential"
......
...@@ -21,5 +21,9 @@ module EE ...@@ -21,5 +21,9 @@ module EE
def supports_epic? def supports_epic?
is_a?(Issue) && project.group is_a?(Issue) && project.group
end end
def supports_health_status?
false
end
end end
end end
# frozen_string_literal: true
module HealthStatus
extend ActiveSupport::Concern
extend ::Gitlab::Utils::Override
included do
enum health_status: {
on_track: 1,
needs_attention: 2,
at_risk: 3
}
end
override :supports_health_status?
def supports_health_status?
resource_parent.feature_available?(:issuable_health_status) &&
::Feature.enabled?(:save_issuable_health_status, resource_parent)
end
end
...@@ -15,6 +15,7 @@ module EE ...@@ -15,6 +15,7 @@ module EE
include UsageStatistics include UsageStatistics
include FromUnion include FromUnion
include EpicTreeSorting include EpicTreeSorting
include HealthStatus
enum state_id: { enum state_id: {
opened: ::Epic.available_states[:opened], opened: ::Epic.available_states[:opened],
......
...@@ -14,6 +14,7 @@ module EE ...@@ -14,6 +14,7 @@ module EE
include Elastic::ApplicationVersionedSearch include Elastic::ApplicationVersionedSearch
include UsageStatistics include UsageStatistics
include WeightEventable include WeightEventable
include HealthStatus
scope :order_weight_desc, -> { reorder ::Gitlab::Database.nulls_last_order('weight', 'DESC') } scope :order_weight_desc, -> { reorder ::Gitlab::Database.nulls_last_order('weight', 'DESC') }
scope :order_weight_asc, -> { reorder ::Gitlab::Database.nulls_last_order('weight') } scope :order_weight_asc, -> { reorder ::Gitlab::Database.nulls_last_order('weight') }
......
...@@ -115,6 +115,7 @@ class License < ApplicationRecord ...@@ -115,6 +115,7 @@ class License < ApplicationRecord
group_level_compliance_dashboard group_level_compliance_dashboard
incident_management incident_management
insights insights
issuable_health_status
license_management license_management
personal_access_token_expiration_policy personal_access_token_expiration_policy
pod_logs pod_logs
......
...@@ -11,14 +11,16 @@ module EE ...@@ -11,14 +11,16 @@ module EE
override :filter_params override :filter_params
def filter_params(issuable) def filter_params(issuable)
# This security check is repeated here to avoid multiple backports, can_admin_issuable = can_admin_issuable?(issuable)
# this should be refactored to be reused from the base class.
ability_name = :"admin_#{issuable.to_ability_name}"
unless issuable.supports_weight? && can?(current_user, ability_name, issuable) unless can_admin_issuable && issuable.supports_weight?
params.delete(:weight) params.delete(:weight)
end end
unless can_admin_issuable && issuable.supports_health_status?
params.delete(:health_status)
end
super super
end end
......
---
title: Add health_status column to issues and epics tables
merge_request: 24202
author:
type: added
...@@ -565,4 +565,6 @@ describe Epic do ...@@ -565,4 +565,6 @@ describe Epic do
end end
it_behaves_like 'versioned description' it_behaves_like 'versioned description'
it_behaves_like 'having health status'
end end
...@@ -572,4 +572,6 @@ describe Issue do ...@@ -572,4 +572,6 @@ describe Issue do
it { is_expected.to eq(expected) } it { is_expected.to eq(expected) }
end end
end end
it_behaves_like 'having health status'
end end
...@@ -58,6 +58,11 @@ describe Issues::UpdateService do ...@@ -58,6 +58,11 @@ describe Issues::UpdateService do
end end
end end
it_behaves_like 'updating issuable health status' do
let(:issuable) { issue }
let(:parent) { project }
end
context 'updating other fields' do context 'updating other fields' do
it 'does not call UpdateDatesService' do it 'does not call UpdateDatesService' do
expect(Epics::UpdateDatesService).not_to receive(:new) expect(Epics::UpdateDatesService).not_to receive(:new)
......
...@@ -254,5 +254,10 @@ describe Epics::UpdateService do ...@@ -254,5 +254,10 @@ describe Epics::UpdateService do
let(:issuable) { epic } let(:issuable) { epic }
let(:parent) { group } let(:parent) { group }
end end
it_behaves_like 'updating issuable health status' do
let(:issuable) { epic }
let(:parent) { group }
end
end end
end end
# frozen_string_literal: true
RSpec.shared_examples 'having health status' do
context 'validations' do
it do
is_expected.to define_enum_for(:health_status)
.with_values(on_track: 1, needs_attention: 2, at_risk: 3)
end
it { is_expected.to allow_value(nil).for(:health_status) }
end
end
# frozen_string_literal: true
RSpec.shared_examples 'updating issuable health status' do
context 'updating health_status' do
let(:current_user) { create(:user) }
let(:opts) { { health_status: 1 } }
let(:service) { described_class.new(parent, current_user, opts) }
context 'when feature is not available' do
it 'does not update issue health status' do
expect { service.execute(issuable) }.not_to change { issuable.health_status }
end
end
context 'when feature is available' do
before do
stub_licensed_features(issuable_health_status: true, epics: true)
end
context 'when feature flag is disabled' do
it 'does not update issuable' do
stub_feature_flags(save_issuable_health_status: false)
expect { service.execute(issuable) }.not_to change { issuable.health_status }
end
end
context 'when user has reporter permissions' do
before do
issuable.resource_parent.add_reporter(current_user)
end
it 'updates issuable with given health_status' do
expect { service.execute(issuable) }.to change { issuable.health_status }.to('on_track')
end
end
context 'when user does not have permissions' do
it 'does not update issuable status' do
expect { service.execute(issuable) }.not_to change { issuable.health_status }
end
end
end
end
end
...@@ -30,6 +30,7 @@ Issue: ...@@ -30,6 +30,7 @@ Issue:
- last_edited_at - last_edited_at
- last_edited_by_id - last_edited_by_id
- discussion_locked - discussion_locked
- health_status
Event: Event:
- id - id
- target_type - target_type
...@@ -824,3 +825,4 @@ Epic: ...@@ -824,3 +825,4 @@ Epic:
- state_id - state_id
- start_date_sourcing_epic_id - start_date_sourcing_epic_id
- due_date_sourcing_epic_id - due_date_sourcing_epic_id
- health_status
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