Commit 6702edce authored by Jan Provaznik's avatar Jan Provaznik

Disable epic quick actions for not persisted epics

Adding/removal of epic links expects that both parent and child epics
are created. If parent_epic is applied on a new not-persisted epic,
then EpicLink then this epic link is created before target epic is saved
which has couple of side-effects:
* CreateService's set_child_epic fails to update the new epic
* no system notes are created
* internal_id for epics is out-of-sync because EpicLink attempts to
  store new epic (and fails and rolls-back thsi save) -> then
  internal_id is already set which target epic is saved (as part of
  regular save operation) but not bumped in DB

Because epic quick actions don't work properly with unpersisted epics,
it will be disabled until the code is updated to work properly with new
epics.

Also DB migration to reset internal_id is renamed to re-enqueue (and
re-run) it again.
parent 296275d8
# frozen_string_literal: true # frozen_string_literal: true
class UpdateInternalIdsLastValueForEpics < ActiveRecord::Migration[6.0] class UpdateInternalIdsLastValueForEpicsRenamed < ActiveRecord::Migration[6.0]
DOWNTIME = false DOWNTIME = false
def up def up
......
cbc6bfa122167e9a46edaa14351a73eeb10586fa0eb82f231c792384c9d7986c
\ No newline at end of file
5c429e8090fd779ba29a8bd78d69e78d1d5d143a6fd3097a715e178fb150d877
\ No newline at end of file
---
title: Disable epic quick actions when creating new epics
merge_request: 49470
author:
type: fixed
...@@ -90,7 +90,8 @@ module EE ...@@ -90,7 +90,8 @@ module EE
end end
def action_allowed? def action_allowed?
quick_action_target.group&.feature_available?(:subepics) && quick_action_target.persisted? &&
quick_action_target.group&.feature_available?(:subepics) &&
current_user.can?(:"admin_#{quick_action_target.to_ability_name}", quick_action_target) current_user.can?(:"admin_#{quick_action_target.to_ability_name}", quick_action_target)
end end
......
...@@ -503,6 +503,12 @@ RSpec.describe QuickActions::InterpretService do ...@@ -503,6 +503,12 @@ RSpec.describe QuickActions::InterpretService do
let(:target) { merge_request } let(:target) { merge_request }
end end
context 'when target epic is not persisted yet' do
let(:target) { build(:epic, group: group) }
it_behaves_like 'quick action is unavailable', :child_epic
end
context 'when passed child epic is nil' do context 'when passed child epic is nil' do
let(:child_epic) { nil } let(:child_epic) { nil }
...@@ -647,6 +653,12 @@ RSpec.describe QuickActions::InterpretService do ...@@ -647,6 +653,12 @@ RSpec.describe QuickActions::InterpretService do
let(:target) { merge_request } let(:target) { merge_request }
end end
context 'when target epic is not persisted yet' do
let(:target) { build(:epic, group: group) }
it_behaves_like 'quick action is unavailable', :remove_child_epic
end
it_behaves_like 'epic relation is removed' it_behaves_like 'epic relation is removed'
context 'when trying to remove child epic from a different epic' do context 'when trying to remove child epic from a different epic' do
...@@ -1176,6 +1188,12 @@ RSpec.describe QuickActions::InterpretService do ...@@ -1176,6 +1188,12 @@ RSpec.describe QuickActions::InterpretService do
it_behaves_like 'target epic does not exist', :parent it_behaves_like 'target epic does not exist', :parent
end end
context 'when target epic is not persisted yet' do
let(:target) { build(:epic, group: group) }
it_behaves_like 'quick action is unavailable', :parent_epic
end
context 'when user has no permission to read epic' do context 'when user has no permission to read epic' do
let(:content) { "/parent_epic #{epic2&.to_reference(epic)}" } let(:content) { "/parent_epic #{epic2&.to_reference(epic)}" }
...@@ -1209,6 +1227,12 @@ RSpec.describe QuickActions::InterpretService do ...@@ -1209,6 +1227,12 @@ RSpec.describe QuickActions::InterpretService do
end end
end end
context 'when target epic is not persisted yet' do
let(:target) { build(:epic, group: group) }
it_behaves_like 'quick action is unavailable', :remove_parent_epic
end
context 'when parent is not present' do context 'when parent is not present' do
before do before do
epic.parent = nil epic.parent = nil
......
# frozen_string_literal: true # frozen_string_literal: true
require 'spec_helper' require 'spec_helper'
require Rails.root.join('db', 'migrate', '20201202081429_update_internal_ids_last_value_for_epics.rb') require Rails.root.join('db', 'migrate', '20201208081429_update_internal_ids_last_value_for_epics_renamed.rb')
RSpec.describe UpdateInternalIdsLastValueForEpics, :migration, schema: 20201124185639 do RSpec.describe UpdateInternalIdsLastValueForEpicsRenamed, :migration, schema: 20201124185639 do
let(:namespaces) { table(:namespaces) } let(:namespaces) { table(:namespaces) }
let(:users) { table(:users) } let(:users) { table(:users) }
let(:epics) { table(:epics) } let(:epics) { table(:epics) }
......
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