Refactoring & Fix: editing group and challenges from user dashboard (#11418)

* Refactoring & Feature: edit/delete group and challenge tasks

- Remove showOption from tasks props
- Pass all needed data to task for understand in mapGetter function which controls we should show
- Improve current solution with edit and delete logic

* Fix: this in template

* Fix & Test: extend tests, fix can Edit/Delete functions

* Fix: allow user edit challenge tasks on dashboard

* Fix: test case after code change

* fix import path

* Fix:

- Extend canEdit and canDelete functions with admin role
- Clarify canEdit and canDelete conditions
- Extend test cases

website/client/src/components/challenges/challengeDetail.vue

@@ -123,7 +123,7 @@
           class="col-12 col-sm-6"
           :type="column"
           :task-list-override="tasksByType[column]"
-          :show-options="showOptions"
+          :challenge="challenge"
           @editTask="editTask"
           @taskDestroyed="taskDestroyed"
         />
@@ -386,9 +386,6 @@ export default {
     canJoin () {
       return !this.isMember;
     },
-    showOptions () {
-      return this.isLeader;
-    },
   },
   mounted () {
     if (!this.searchId) this.searchId = this.challengeId;

website/client/src/components/group-plans/taskInformation.vue

@@ -75,7 +75,6 @@
         class="col-12 col-md-3"
         :type="column"
         :task-list-override="tasksByType[column]"
-        :show-options="showOptions"
         :group="group"
         :search-text="searchText"
         @editTask="editTask"
@@ -199,9 +198,6 @@ export default {
       return (this.group.leader && this.group.leader._id === this.user._id)
         || (this.group.managers && Boolean(this.group.managers[this.user._id]));
     },
-    showOptions () {
-      return this.canCreateTasks;
-    },
   },
   watch: {
     // call again the method if the route changes (when this route is already active)

website/client/src/components/tasks/column.vue

@@ -90,8 +90,8 @@
           :key="task.id"
           :task="task"
           :is-user="isUser"
-          :show-options="showOptions"
           :group="group"
+          :challenge="challenge"
           @editTask="editTask"
           @moveTo="moveTo"
           @taskDestroyed="taskDestroyed"
@@ -372,10 +372,7 @@ export default {
     selectedTags: {},
     taskListOverride: {},
     group: {},
-    showOptions: {
-      type: Boolean,
-      default: true,
-    },
+    challenge: {},
   }, // @TODO: maybe we should store the group on state?
   data () {
     const icons = Object.freeze({

website/client/src/components/tasks/task.vue

@@ -99,6 +99,7 @@
                 </div>
                 <div slot="dropdown-content">
                   <div
+                    v-if="showEdit"
                     ref="editTaskItem"
                     class="dropdown-item edit-task-item"
                   >
@@ -137,7 +138,7 @@
                     </span>
                   </div>
                   <div
-                    v-if="canDelete(task)"
+                    v-if="showDelete"
                     class="dropdown-item"
                     @click="destroy"
                   >
@@ -789,6 +790,7 @@ import moment from 'moment';
 import axios from 'axios';
 import Vue from 'vue';
 import uuid from 'uuid';
+import isEmpty from 'lodash/isEmpty';
 import { mapState, mapGetters, mapActions } from '@/libs/store';
 import scoreTask from '@/../../common/script/ops/scoreTask';
 import * as Analytics from '@/libs/analytics';
@@ -825,7 +827,7 @@ export default {
     markdown: markdownDirective,
   },
   mixins: [notifications],
-  props: ['task', 'isUser', 'group', 'dueDate', 'showOptions'], // @TODO: maybe we should store the group on state?
+  props: ['task', 'isUser', 'group', 'challenge', 'dueDate'], // @TODO: maybe we should store the group on state?
   data () {
     return {
       random: uuid.v4(), // used to avoid conflicts between checkboxes ids
@@ -859,6 +861,7 @@ export default {
       getTagsFor: 'tasks:getTagsFor',
       getTaskClasses: 'tasks:getTaskClasses',
       canDelete: 'tasks:canDelete',
+      canEdit: 'tasks:canEdit',
     }),
     hasChecklist () {
       return this.task.checklist && this.task.checklist.length > 0;
@@ -937,6 +940,27 @@ export default {
 
       return this.task.challenge.shortName ? this.task.challenge.shortName.toString() : '';
     },
+    isChallangeTask () {
+      return !isEmpty(this.task.challenge);
+    },
+    isGroupTask () {
+      return this.task.group.taskId || this.task.group.id;
+    },
+    taskCategory () {
+      let taskCategory = 'default';
+      if (this.isGroupTask) taskCategory = 'group';
+      else if (this.isChallangeTask) taskCategory = 'challenge';
+      return taskCategory;
+    },
+    showDelete () {
+      return this.canDelete(this.task, this.taskCategory, this.isUser, this.group, this.challenge);
+    },
+    showEdit () {
+      return this.canEdit(this.task, this.taskCategory, this.isUser, this.group, this.challenge);
+    },
+    showOptions () {
+      return this.showEdit || this.showDelete || this.isUser;
+    },
   },
   methods: {
     ...mapActions({
@@ -950,7 +974,7 @@ export default {
       this.scoreChecklistItem({ taskId: this.task._id, itemId: item.id });
     },
     edit (e, task) {
-      if (this.isRunningYesterdailies) return;
+      if (this.isRunningYesterdailies || !this.showEdit) return;
 
       // Prevent clicking on a link from opening the edit modal
       const target = e.target || e.srcElement;

website/client/src/store/getters/tasks.js

@@ -34,12 +34,75 @@ function getTaskColor (task) {
   return 'best';
 }
 
-export function canDelete () {
-  return task => {
-    const isUserChallenge = Boolean(task.userId);
-    const activeChallenge = isUserChallenge
-      && task.challenge && task.challenge.id && !task.challenge.broken;
-    return !activeChallenge;
+export function canDelete (store) {
+  return (task, taskCategory, onUserDashboard, group, challenge) => {
+    const user = store.state.user.data;
+    const userId = user.id || user._id;
+
+    const isUserAdmin = user.contributor && !!user.contributor.admin;
+    const isUserGroupLeader = group && (group.leader
+      && group.leader._id === userId);
+    const isUserGroupManager = group && (group.managers
+        && Boolean(group.managers[userId]));
+    const isUserChallenge = userId === (challenge && challenge.leader.id);
+
+    let isUserCanDeleteTask = onUserDashboard;
+
+    switch (taskCategory) {
+      case 'challenge':
+        if (!onUserDashboard) {
+          isUserCanDeleteTask = isUserChallenge || isUserAdmin;
+        } else {
+          isUserCanDeleteTask = isUserAdmin;
+        }
+        break;
+      case 'group':
+        if (!onUserDashboard) {
+          isUserCanDeleteTask = isUserGroupLeader || isUserGroupManager || isUserAdmin;
+        } else {
+          isUserCanDeleteTask = isUserAdmin;
+        }
+        break;
+      default:
+        break;
+    }
+    return Boolean(isUserCanDeleteTask);
+  };
+}
+
+export function canEdit (store) {
+  return (task, taskCategory, onUserDashboard, group, challenge) => {
+    let isUserCanEditTask = onUserDashboard;
+    const user = store.state.user.data;
+    const userId = user.id || user._id;
+
+    const isUserAdmin = user.contributor && !!user.contributor.admin;
+    const isUserGroupLeader = group && (group.leader
+      && group.leader._id === userId);
+    const isUserGroupManager = group && (group.managers
+        && Boolean(group.managers[userId]));
+    const isUserChallenge = userId === (challenge && challenge.leader.id);
+
+
+    switch (taskCategory) {
+      case 'challenge':
+        if (!onUserDashboard) {
+          isUserCanEditTask = isUserChallenge || isUserAdmin;
+        } else {
+          isUserCanEditTask = true;
+        }
+        break;
+      case 'group':
+        if (!onUserDashboard) {
+          isUserCanEditTask = isUserGroupLeader || isUserGroupManager || isUserAdmin;
+        } else {
+          isUserCanEditTask = true;
+        }
+        break;
+      default:
+        break;
+    }
+    return Boolean(isUserCanEditTask);
   };
 }
 

website/client/tests/unit/store/getters/tasks/canDelete.spec.js

@@ -1,19 +1,73 @@
 import generateStore from '@/store';
 
+
 describe('canDelete getter', () => {
-  it('cannot delete active challenge task', () => {
-    const store = generateStore();
+  let store;
+  let group;
+  let challenge;
+  let task;
 
+  beforeEach(() => {
+    store = generateStore();
 
-    const task = { userId: 1, challenge: { id: 2 } };
-    expect(store.getters['tasks:canDelete'](task)).to.equal(false);
+    store.state.user.data = {
+      id: 10,
+      contributor: {
+        admin: false,
+      },
+    };
+
+    group = {
+      leader: {
+        _id: 123,
+      },
+      managers: {
+        123984: {},
+      },
+    };
+
+    challenge = {
+      leader: {
+        id: 123,
+      },
+    };
+
+    task = { userId: 1, challenge: { id: 2 } };
+  });
+  it('cannot Delete challenge or group task in own dashboard', () => {
+    expect(store.getters['tasks:canDelete'](task, 'challenge', true, null, challenge)).to.equal(false);
+    expect(store.getters['tasks:canDelete'](task, 'group', true, group, null)).to.equal(false);
   });
 
-  it('can delete broken challenge task', () => {
-    const store = generateStore();
+  it('can Delete any challenge task as admin', () => {
+    store.state.user.data.contributor.admin = true;
 
+    expect(store.getters['tasks:canDelete'](task, 'challenge', true, null, challenge)).to.equal(true);
+  });
 
-    const task = { userId: 1, challenge: { id: 2, broken: true } };
-    expect(store.getters['tasks:canDelete'](task)).to.equal(true);
+  it('can Delete own challenge task if leader', () => {
+    store.state.user.data.id = 123;
+
+    expect(store.getters['tasks:canDelete'](task, 'challenge', false, null, challenge)).to.equal(true);
+  });
+
+  it('cannot Delete challenge task if non leader on challenge page', () => {
+    expect(store.getters['tasks:canDelete'](task, 'challenge', false, null, challenge)).to.equal(false);
+  });
+
+  it('can Delete group task as leader on group page', () => {
+    store.state.user.data.id = 123;
+
+    expect(store.getters['tasks:canDelete'](task, 'group', false, group)).to.equal(true);
+  });
+
+  it('can Delete group task if manager on group page', () => {
+    store.state.user.data.id = 123984;
+
+    expect(store.getters['tasks:canDelete'](task, 'group', false, group)).to.equal(true);
+  });
+
+  it('cannot Delete group task if not a leader on group page', () => {
+    expect(store.getters['tasks:canDelete'](task, 'group', false, group)).to.equal(false);
   });
 });

website/client/tests/unit/store/getters/tasks/canEdit.spec.js

@@ -0,0 +1,75 @@
+import generateStore from '@/store';
+
+
+describe('canEdit getter', () => {
+  let store;
+  let group;
+  let challenge;
+  let task;
+
+  beforeEach(() => {
+    store = generateStore();
+
+    store.state.user.data = {
+      id: 10,
+      contributor: {
+        admin: false,
+      },
+    };
+
+    group = {
+      leader: {
+        _id: 123,
+      },
+      managers: {
+        123984: {},
+      },
+    };
+
+    challenge = {
+      leader: {
+        id: 123,
+      },
+    };
+
+    task = { userId: 1, challenge: { id: 2 } };
+  });
+  it('can Edit task in own dashboard', () => {
+    expect(store.getters['tasks:canEdit'](task, 'challenge', true, null, challenge)).to.equal(true);
+    expect(store.getters['tasks:canEdit'](task, 'group', true, group, null)).to.equal(true);
+  });
+
+  it('can Edit any challenge task if admin', () => {
+    store.state.user.data.contributor.admin = true;
+
+    expect(store.getters['tasks:canEdit'](task, 'challenge', true, null, challenge)).to.equal(true);
+    expect(store.getters['tasks:canEdit'](task, 'challenge', false, null, challenge)).to.equal(true);
+  });
+
+  it('can Edit own challenge task if leader', () => {
+    store.state.user.data.id = 123;
+
+    expect(store.getters['tasks:canEdit'](task, 'challenge', true, null, challenge)).to.equal(true);
+    expect(store.getters['tasks:canEdit'](task, 'challenge', false, null, challenge)).to.equal(true);
+  });
+
+  it('cannot Edit challenge task if not leader on challenge page', () => {
+    expect(store.getters['tasks:canEdit'](task, 'challenge', false, null, challenge)).to.equal(false);
+  });
+
+  it('can Edit group task as leader on group page', () => {
+    store.state.user.data.id = 123;
+
+    expect(store.getters['tasks:canEdit'](task, 'group', false, group)).to.equal(true);
+  });
+
+  it('can Edit group task if manager on group page', () => {
+    store.state.user.data.id = 123984;
+
+    expect(store.getters['tasks:canEdit'](task, 'group', false, group)).to.equal(true);
+  });
+
+  it('cannot Edit group task if not leader on group page', () => {
+    expect(store.getters['tasks:canEdit'](task, 'group', false, group)).to.equal(false);
+  });
+});