From 70b65011351f25e8d10e4479e27f180729a5bb7e Mon Sep 17 00:00:00 2001 From: Aleksey Date: Fri, 20 Sep 2019 18:24:05 +0300 Subject: [PATCH] Refactoring & Feature: edit/delete controls of tasks (#11357) * 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 --- .../specs/store/getters/tasks/canDelete.js | 4 +- .../unit/specs/store/getters/tasks/canEdit.js | 19 ++++++ .../components/challenges/challengeDetail.vue | 5 +- .../group-plans/taskInformation.vue | 4 -- website/client/components/tasks/column.vue | 7 +-- website/client/components/tasks/task.vue | 31 +++++++-- website/client/store/getters/tasks.js | 63 +++++++++++++++++-- 7 files changed, 109 insertions(+), 24 deletions(-) create mode 100644 test/client/unit/specs/store/getters/tasks/canEdit.js diff --git a/test/client/unit/specs/store/getters/tasks/canDelete.js b/test/client/unit/specs/store/getters/tasks/canDelete.js index d40036697b..e07d8714ad 100644 --- a/test/client/unit/specs/store/getters/tasks/canDelete.js +++ b/test/client/unit/specs/store/getters/tasks/canDelete.js @@ -6,7 +6,7 @@ describe('canDelete getter', () => { const task = {userId: 1, challenge: {id: 2}}; - expect(store.getters['tasks:canDelete'](task)).to.equal(false); + expect(store.getters['tasks:canDelete'](task, 'challenge', true, null, null)).to.equal(false); }); it('can delete broken challenge task', () => { @@ -14,6 +14,6 @@ describe('canDelete getter', () => { const task = {userId: 1, challenge: {id: 2, broken: true}}; - expect(store.getters['tasks:canDelete'](task)).to.equal(true); + expect(store.getters['tasks:canDelete'](task, 'challenge', true, null, null)).to.equal(true); }); }); diff --git a/test/client/unit/specs/store/getters/tasks/canEdit.js b/test/client/unit/specs/store/getters/tasks/canEdit.js new file mode 100644 index 0000000000..55ba314913 --- /dev/null +++ b/test/client/unit/specs/store/getters/tasks/canEdit.js @@ -0,0 +1,19 @@ +import generateStore from 'client/store'; + +describe('canEdit getter', () => { + it('cannot Edit active challenge task', () => { + const store = generateStore(); + + + const task = {userId: 1, challenge: {id: 2}}; + expect(store.getters['tasks:canEdit'](task, 'challenge', true, null, null)).to.equal(false); + }); + + it('can Edit broken challenge task', () => { + const store = generateStore(); + + + const task = {userId: 1, challenge: {id: 2, broken: true}}; + expect(store.getters['tasks:canEdit'](task, 'challenge', true, null, null)).to.equal(true); + }); +}); diff --git a/website/client/components/challenges/challengeDetail.vue b/website/client/components/challenges/challengeDetail.vue index d243af8053..4e9477bb66 100644 --- a/website/client/components/challenges/challengeDetail.vue +++ b/website/client/components/challenges/challengeDetail.vue @@ -57,7 +57,7 @@ :type="column", :key="column", :taskListOverride='tasksByType[column]', - :showOptions="showOptions", + :challenge="challenge" @editTask="editTask", @taskDestroyed="taskDestroyed", v-if='tasksByType[column].length > 0') @@ -252,9 +252,6 @@ export default { canJoin () { return !this.isMember; }, - showOptions () { - return this.isLeader; - }, }, mounted () { if (!this.searchId) this.searchId = this.challengeId; diff --git a/website/client/components/group-plans/taskInformation.vue b/website/client/components/group-plans/taskInformation.vue index 201d67d71b..fbe4516d4f 100644 --- a/website/client/components/group-plans/taskInformation.vue +++ b/website/client/components/group-plans/taskInformation.vue @@ -41,7 +41,6 @@ :type="column", :key="column", :taskListOverride='tasksByType[column]', - :showOptions="showOptions" v-on:editTask="editTask", v-on:loadGroupCompletedTodos="loadGroupCompletedTodos", v-on:taskDestroyed="taskDestroyed", @@ -177,9 +176,6 @@ export default { if (!this.group) return false; 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; - }, }, methods: { async load () { diff --git a/website/client/components/tasks/column.vue b/website/client/components/tasks/column.vue index a25d4567ad..f5a69506bb 100644 --- a/website/client/components/tasks/column.vue +++ b/website/client/components/tasks/column.vue @@ -45,10 +45,10 @@ v-for="task in taskList", :key="task.id", :task="task", :isUser="isUser", - :showOptions="showOptions" @editTask="editTask", @moveTo="moveTo", :group='group', + :challenge="challenge" v-on:taskDestroyed='taskDestroyed' ) template(v-if="hasRewardsList") @@ -312,10 +312,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({ diff --git a/website/client/components/tasks/task.vue b/website/client/components/tasks/task.vue index 3d57573948..70e3d04619 100644 --- a/website/client/components/tasks/task.vue +++ b/website/client/components/tasks/task.vue @@ -27,7 +27,7 @@ div(slot="dropdown-toggle", draggable=false) .svg-icon.dropdown-icon(v-html="icons.menu") div(slot="dropdown-content", draggable=false) - .dropdown-item.edit-task-item(ref="editTaskItem") + .dropdown-item.edit-task-item(ref="editTaskItem" v-if="showEdit") span.dropdown-icon-item span.svg-icon.inline.edit-icon(v-html="icons.edit") span.text {{ $t('edit') }} @@ -39,7 +39,7 @@ span.dropdown-icon-item span.svg-icon.inline.push-to-bottom(v-html="icons.bottom") span.text {{ $t('taskToBottom') }} - .dropdown-item(@click="destroy", v-if="canDelete(task)") + .dropdown-item(@click="destroy", v-if="showDelete") span.dropdown-icon-item.delete-task-item span.svg-icon.inline.delete(v-html="icons.delete") span.text {{ $t('delete') }} @@ -521,6 +521,7 @@ import axios from 'axios'; import scoreTask from 'common/script/ops/scoreTask'; import Vue from 'vue'; import * as Analytics from 'client/libs/analytics'; +import isEmpty from 'lodash/isEmpty'; import positiveIcon from 'assets/svg/positive.svg'; import negativeIcon from 'assets/svg/negative.svg'; @@ -555,7 +556,7 @@ export default { directives: { markdown: markdownDirective, }, - 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 @@ -589,6 +590,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; @@ -665,6 +667,27 @@ export default { return this.task.challenge.shortName ? this.task.challenge.shortName.toString() : ''; }, + isChallangeTask () { + return !isEmpty(this.task.challenge); + }, + isGroupTask () { + return !isEmpty(this.task.group) && (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({ @@ -678,7 +701,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; diff --git a/website/client/store/getters/tasks.js b/website/client/store/getters/tasks.js index 5fb2e49604..2c453a0423 100644 --- a/website/client/store/getters/tasks.js +++ b/website/client/store/getters/tasks.js @@ -37,11 +37,64 @@ function getTaskColor (task) { } } -export function canDelete () { - return (task) => { - let isUserChallenge = Boolean(task.userId); - let activeChallenge = isUserChallenge && task.challenge && task.challenge.id && !task.challenge.broken; - return !activeChallenge; +export function canDelete (store) { + return (task, taskCategory, isTaskFromUserDashboard, group, challenge) => { + let isUserCanDeleteTask = isTaskFromUserDashboard; + const user = store.state.user.data; + + switch (taskCategory) { + case 'challenge': + if (challenge && !isTaskFromUserDashboard) { + let isUserChallenge = user.id === challenge.leader.id; + isUserCanDeleteTask = isUserChallenge || task.challenge.broken; + } else { + isUserCanDeleteTask = task.challenge.broken; + } + break; + case 'group': + if (group && !isTaskFromUserDashboard) { + isUserCanDeleteTask = + group.leader && + group.leader._id === user._id || group.managers && + Boolean(group.managers[user._id]); + } else { + isUserCanDeleteTask = false; + } + break; + default: + break; + } + return Boolean(isUserCanDeleteTask); + }; +} + +export function canEdit (store) { + return (task, taskCategory, isTaskFromUserDashboard, group, challenge) => { + let isUserCanEditTask = isTaskFromUserDashboard; + const user = store.state.user.data; + switch (taskCategory) { + case 'challenge': + if (challenge && !isTaskFromUserDashboard) { + let isUserChallenge = user.id === challenge.leader.id; + isUserCanEditTask = isUserChallenge || task.challenge.broken; + } else { + isUserCanEditTask = task.challenge.broken; + } + break; + case 'group': + if (group && !isTaskFromUserDashboard) { + isUserCanEditTask = + group.leader && + group.leader._id === user._id || group.managers && + Boolean(group.managers[user._id]); + } else { + isUserCanEditTask = false; + } + break; + default: + break; + } + return Boolean(isUserCanEditTask); }; }