Update controller permissions to set parent of an epic

Update permissions for UI button

ee/app/assets/javascripts/related_items_tree/components/epic_issue_actions_split_button.vue

@@ -18,8 +18,8 @@ export default {
   },
   computed: {
     ...mapState(['parentItem']),
-    canAdminRelation() {
-      return this.parentItem.userPermissions.canAdminRelation;
+    canReadRelation() {
+      return this.parentItem.userPermissions.canReadRelation;
     },
     epicActionItems() {
       const epicActionItems = [];
@@ -57,7 +57,7 @@ export default {
         },
       ];
 
-      if (this.allowSubEpics && this.canAdminRelation) {
+      if (this.allowSubEpics && this.canReadRelation) {
         actions.push(this.epicActionItems);
       }
 

ee/app/assets/javascripts/related_items_tree/components/tree_item_body.vue

@@ -142,6 +142,10 @@ export default {
       return this.itemWebPath.length ? this.itemWebPath : null;
     },
     canAdminRelation() {
+      if (this.isEpic) {
+        return this.parentItem.userPermissions.canReadRelation;
+      }
+
       return this.parentItem.userPermissions.canAdminRelation;
     },
     itemActionInProgress() {

ee/app/assets/javascripts/related_items_tree/related_items_tree_bundle.js

@@ -70,6 +70,7 @@ export default () => {
         userPermissions: {
           canAdmin: initialData.canAdmin,
           canAdminRelation: initialData.canAdminRelation,
+          canReadRelation: initialData.canReadRelation,
         },
       });
 

ee/app/controllers/groups/epics/epic_links_controller.rb

@@ -26,9 +26,9 @@ def destroy
       private
 
       def authorize_admin!
-        return super unless action_name == 'destroy'
+        ability_name = action_name == 'update' ? :admin_epic_relation : :read_epic_relation
 
-        render_403 unless can?(current_user, 'admin_epic_relation', epic)
+        render_403 unless can?(current_user, ability_name, epic)
       end
 
       def create_service
@@ -42,10 +42,6 @@ def list_service
       def child_epic
         @child_epic ||= Epic.find(params[:id])
       end
-
-      def authorized_object
-        'epic_tree_relation'
-      end
     end
   end
 end

ee/app/helpers/ee/issuables_helper.rb

@@ -29,6 +29,7 @@ def issuable_initial_data(issuable)
         data[:issueLinksEndpoint] = group_epic_issues_path(parent, issuable)
         data[:issuesWebUrl] = issues_group_path(parent)
         data[:projectsEndpoint] = expose_path(api_v4_groups_projects_path(id: parent.id))
+        data[:canReadRelation] = can?(current_user, :read_epic_relation, issuable)
       end
 
       data

ee/spec/features/epics/epic_issues_spec.rb

@@ -6,22 +6,23 @@
   include NestedEpicsHelper
   include DragTo
 
-  let(:user) { create(:user) }
-  let(:group) { create(:group, :public) }
-  let(:epic) { create(:epic, group: group) }
-  let(:public_project) { create(:project, :public, group: group) }
-  let(:private_project) { create(:project, :private, group: group) }
-  let(:public_issue) { create(:issue, project: public_project) }
-  let(:private_issue) { create(:issue, project: private_project) }
-
-  let!(:epic_issues) do
+  let_it_be(:non_member) { create(:user) }
+  let_it_be(:developer) { create(:user) }
+  let_it_be(:group) { create(:group, :public).tap { |g| g.add_developer(developer) } }
+  let_it_be(:epic) { create(:epic, group: group) }
+  let_it_be(:public_project) { create(:project, :public, group: group) }
+  let_it_be(:private_project) { create(:project, :private, group: group) }
+  let_it_be(:public_issue) { create(:issue, project: public_project) }
+  let_it_be(:private_issue) { create(:issue, project: private_project) }
+
+  let_it_be(:epic_issues) do
     [
       create(:epic_issue, epic: epic, issue: public_issue, relative_position: 1),
       create(:epic_issue, epic: epic, issue: private_issue, relative_position: 2)
     ]
   end
 
-  let!(:nested_epics) do
+  let_it_be(:nested_epics) do
     [
       create(:epic, group: group, parent_id: epic.id, relative_position: 1),
       create(:epic, group: group, parent_id: epic.id, relative_position: 2)
@@ -51,27 +52,66 @@ def visit_epic_no_subepic
   end
 
   context 'when user is not a group member of a public group' do
+    let(:user) { non_member }
+    let(:add_child_menu_selector) do
+      '.related-items-tree-container .js-add-epics-issues-button [data-testid="disclosure-dropdown-item"]'
+    end
+
     before do
       visit_epic
     end
 
-    it 'user can see issues from public project but cannot delete the associations' do
+    it 'display remove button for child epics but not for issues' do
       within('.related-items-tree-container ul.related-items-list') do
         expect(page).to have_selector('li', count: 3)
         expect(page).to have_content(public_issue.title)
-        expect(page).not_to have_selector('button.js-issue-item-remove-button')
+        expect(page).to have_selector('button.js-issue-item-remove-button', count: 2)
       end
     end
 
-    it 'user cannot add new issues to the epic' do
-      expect(page).not_to have_selector('.related-items-tree-container .js-issue-actions-split-button > button:first-child')
+    it 'displays option to add new and existing issue' do
+      find(".related-items-tree-container .js-add-epics-issues-button").click
+      expect(page).to have_selector(add_child_menu_selector, text: 'Add a new issue')
+      expect(page).to have_selector(add_child_menu_selector, text: 'Add an existing issue')
+    end
+
+    it 'displays option to add an existing epic' do
+      find(".related-items-tree-container .js-add-epics-issues-button").click
+      expect(page).to have_selector(add_child_menu_selector, text: 'Add an existing epic')
+    end
+
+    it 'does not display option to add new epic' do
+      find(".related-items-tree-container .js-add-epics-issues-button").click
+      expect(page).not_to have_selector(add_child_menu_selector, text: 'Add a new epic')
+    end
+
+    context 'when epic_relations_for_non_members feature flag is disabled' do
+      before do
+        stub_feature_flags(epic_relations_for_non_members: false)
+        visit_epic
+      end
+
+      it 'user cannot add existing epic' do
+        find(".related-items-tree-container .js-add-epics-issues-button").click
+        expect(page).not_to have_selector(add_child_menu_selector, text: 'Add an existing epic')
+        expect(page).not_to have_selector(add_child_menu_selector, text: 'Add a new epic')
+      end
+
+      it 'user cannot remove existing epic children' do
+        within('.related-items-tree-container ul.related-items-list') do
+          expect(page).to have_selector('li', count: 3)
+          expect(page).not_to have_selector('button.js-issue-item-remove-button')
+        end
+      end
     end
   end
 
   context 'when user is a group member' do
-    let(:issue_to_add) { create(:issue, project: private_project) }
-    let(:issue_invalid) { create(:issue) }
-    let(:epic_to_add) { create(:epic, group: group) }
+    let_it_be(:issue_to_add) { create(:issue, project: private_project) }
+    let_it_be(:issue_invalid) { create(:issue) }
+    let_it_be(:epic_to_add) { create(:epic, group: group) }
+
+    let(:user) { developer }
 
     def add_issues(references)
       find(".related-items-tree-container .js-add-epics-issues-button").click
@@ -96,7 +136,6 @@ def add_epics(references)
     end
 
     before do
-      group.add_developer(user)
       visit_epic
     end
 

ee/spec/frontend/related_items_tree/components/epic_actions_split_button_spec.js

@@ -19,7 +19,7 @@ const createComponent = ({ slots, state = {} } = {}) => {
     userPermissions: {
       ...mockParentItem.userPermissions,
       canAdmin: state.canAdmin,
-      canAdminRelation: state.canAdminRelation,
+      canReadRelation: state.canReadRelation,
     },
   });
 
@@ -41,9 +41,9 @@ describe('ee/related_items_tree/components/epic_issue_actions_split_button.vue',
   const findDropdownSections = () => findDisclosureDropdown().props('items');
   const findDropdownItems = () => findDropdownSections().flatMap((x) => x.items);
 
-  describe('default (canAdmin and canAdminRelation)', () => {
+  describe('default (canAdmin and canReadRelation)', () => {
     beforeEach(() => {
-      wrapper = createComponent({ state: { canAdmin: true, canAdminRelation: true } });
+      wrapper = createComponent({ state: { canAdmin: true, canReadRelation: true } });
     });
 
     it('renders section headers', () => {
@@ -82,9 +82,9 @@ describe('ee/related_items_tree/components/epic_issue_actions_split_button.vue',
     });
   });
 
-  describe('when cannot admin relation', () => {
+  describe('when cannot read relation', () => {
     beforeEach(() => {
-      wrapper = createComponent({ state: { canAdminRelation: false } });
+      wrapper = createComponent({ state: { canReadRelation: false } });
     });
 
     it('does not render entire "Epic"', () => {
@@ -96,7 +96,7 @@ describe('ee/related_items_tree/components/epic_issue_actions_split_button.vue',
 
   describe('when cannot admin', () => {
     beforeEach(() => {
-      wrapper = createComponent({ state: { canAdmin: false, canAdminRelation: true } });
+      wrapper = createComponent({ state: { canAdmin: false, canReadRelation: true } });
     });
 
     it('does not render "Add a new epic" action', () => {

ee/spec/frontend/related_items_tree/components/tree_item_body_spec.js

@@ -447,9 +447,22 @@ describe('RelatedItemsTree', () => {
         expect(findItemAssignees().isVisible()).toBe(true);
       });
 
-      it('renders item remove button when `item.userPermissions.canAdminRelation` is true', () => {
+      it('renders remove button when item is issue and user can admin relation', () => {
+        mockItem = createIssueItem();
+        mockParentItem.userPermissions.canAdminRelation = true;
+
+        createComponent();
+
+        expect(findRemoveButton().isVisible()).toBe(true);
+      });
+
+      it('renders remove button when item is epic and user can read relation', () => {
+        mockItem = createEpicItem();
+        mockParentItem.userPermissions.canReadRelation = true;
+
+        createComponent();
+
         expect(findRemoveButton().isVisible()).toBe(true);
-        expect(findRemoveButton().attributes('title')).toBe('Remove');
       });
 
       describe.each`

ee/spec/helpers/ee/issuables_helper_spec.rb

@@ -21,6 +21,7 @@
         {
           canAdmin: permission,
           canAdminRelation: permission,
+          canReadRelation: permission,
           canDestroy: permission,
           canUpdate: permission,
           confidential: epic.confidential,

ee/spec/requests/groups/epics/epic_links_controller_spec.rb

@@ -14,7 +14,7 @@
     sign_in(user)
   end
 
-  shared_examples 'unlicensed subepics action' do
+  shared_examples 'unlicensed action' do |status: :forbidden|
     before do
       stub_licensed_features(features_when_forbidden)
       group.add_developer(user)
@@ -22,8 +22,8 @@
       subject
     end
 
-    it 'returns 403 status' do
-      expect(response).to have_gitlab_http_status(:forbidden)
+    it 'returns error response' do
+      expect(response).to have_gitlab_http_status(status)
     end
   end
 
@@ -36,7 +36,7 @@
 
     subject { get group_epic_links_path(group_id: group, epic_id: parent_epic.to_param) }
 
-    it_behaves_like 'unlicensed subepics action'
+    it_behaves_like 'unlicensed action'
 
     context 'when epics are enabled' do
       before do
@@ -141,7 +141,7 @@ def get_epics
       post group_epic_links_path(group_id: group, epic_id: parent_epic.to_param, issuable_references: reference)
     end
 
-    it_behaves_like 'unlicensed subepics action'
+    it_behaves_like 'unlicensed action', status: :not_found
 
     context 'when subepics are enabled' do
       before do
@@ -193,15 +193,40 @@ def get_epics
         end
       end
 
-      context 'when user does not have permissions to create requested association' do
-        it 'returns 403 status' do
-          subject
+      context 'when user does not have access to epic' do
+        context 'when `epic_relations_for_non_members` feature flag is disabled' do
+          before do
+            stub_feature_flags(epic_relations_for_non_members: false)
+          end
 
-          expect(response).to have_gitlab_http_status(:forbidden)
+          it 'returns 403 status' do
+            subject
+
+            expect(response).to have_gitlab_http_status(:forbidden)
+          end
         end
 
-        it 'does not update parent attribute' do
-          expect { subject }.not_to change { epic1.reload.parent }.from(nil)
+        context 'when group is private' do
+          let(:group) { create(:group, :private) }
+
+          it 'returns 404 status' do
+            subject
+
+            expect(response).to have_gitlab_http_status(:not_found)
+          end
+        end
+
+        context 'when epic is confidential' do
+          let_it_be(:epic1) { create(:epic, group: group, confidential: true) }
+          let_it_be(:parent_epic) { create(:epic, group: group, confidential: true) }
+
+          it 'returns 403 status when user is a guest' do
+            group.add_guest(user)
+
+            subject
+
+            expect(response).to have_gitlab_http_status(:forbidden)
+          end
         end
       end
     end
@@ -221,7 +246,7 @@ def get_epics
                                                      move_before_epic.id })
     end
 
-    it_behaves_like 'unlicensed subepics action'
+    it_behaves_like 'unlicensed action', status: :not_found
 
     context 'when subepics are enabled' do
       before do
@@ -273,7 +298,7 @@ def get_epics
 
     subject { delete group_epic_link_path(group_id: group, epic_id: parent_epic.to_param, id: epic1.id) }
 
-    it_behaves_like 'unlicensed subepics action'
+    it_behaves_like 'unlicensed action'
 
     context 'when epics are enabled' do
       before do
@@ -296,15 +321,40 @@ def get_epics
         end
       end
 
-      context 'when user does not have permissions to update the parent epic' do
-        it 'returns status 404' do
-          subject
+      context 'when user does not have access to epic' do
+        context 'when `epic_relations_for_non_members` feature flag is disabled' do
+          before do
+            stub_feature_flags(epic_relations_for_non_members: false)
+          end
 
-          expect(response).to have_gitlab_http_status(:forbidden)
+          it 'returns 403 status' do
+            subject
+
+            expect(response).to have_gitlab_http_status(:forbidden)
+          end
+        end
+
+        context 'when group is private' do
+          let(:group) { create(:group, :private) }
+
+          it 'returns 404 status' do
+            subject
+
+            expect(response).to have_gitlab_http_status(:not_found)
+          end
         end
 
-        it 'does not destroy the link' do
-          expect { subject }.not_to change { epic1.reload.parent }.from(parent_epic)
+        context 'when epic is confidential' do
+          let_it_be(:epic1) { create(:epic, group: group, confidential: true) }
+          let_it_be(:parent_epic) { create(:epic, group: group, confidential: true) }
+
+          it 'returns 403 status when user is a guest' do
+            group.add_guest(user)
+
+            subject
+
+            expect(response).to have_gitlab_http_status(:forbidden)
+          end
         end
       end
 
@@ -312,7 +362,7 @@ def get_epics
         it 'returns status 404' do
           delete group_epic_link_path(group_id: group, epic_id: parent_epic.to_param, id: epic2.id)
 
-          expect(response).to have_gitlab_http_status(:forbidden)
+          expect(response).to have_gitlab_http_status(:not_found)
         end
       end
     end