Skip to content

Commit

Permalink
Sabrecat/groups quick wins (#11146)
Browse files Browse the repository at this point in the history 
* WIP(groups): quickish wins

* WIP(groups): two quick wins
1. Don't show task creation button if user is not leader or manager
2. Don't require JS confirm() for approving tasks

* fix(group-plans): allow delete from options button

* fix(group-plans): update tasksOrder when task deleted

* fix(group-tasks): dismiss notification when user takes action

* refactor(tasks): DRY out create button styling

* fix(group-tasks): sync after claiming/unclaiming

* fix(claiming): better sync and notif handling

* fix(tasks): force sync instead of explicitly clearing notif

* fix(tasks): reposition task creation button

* fix(group-tasks): default to single completion

* fix(group-tasks): move completion condition field above approval switch

* fix(group-tasks): todo validation error and approval notif dismissal

* fix(group-tasks): default single completion on client

* fix(group-tasks): move completion condition up more

* fix(group-tasks): maintain client-side user assignment list

* fix(group-tasks): remove approval notifications when task deleted

* fix(group-tasks): send assigned task to top of task list

* fix(group-tasks): remove useless tag filter dropdown

* feat(group-tasks): notify user of assigned task

* fix(group-tasks): don't allow approval of tasks w/ no approval request

* fix(tests): adjust expectations

* fix(group-tasks): more sensible action on assignment notif click
  • Loading branch information
@SabreCat
SabreCat committed Apr 29, 2019
1 parent 5a15c73 commit 1135ab9
Show file tree
Hide file tree
Showing 16 changed files with 262 additions and 214 deletions.
32 changes: 32 additions & 0 deletions test/api/v3/integration/tasks/groups/DELETE-group_tasks_id.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,38 @@ describe('Groups DELETE /tasks/:id', () => {
});
});

it('removes deleted taskʾs approval pending notifications from managers', async () => {
await user.post(`/groups/${guild._id}/add-manager`, {
managerId: member2._id,
});
await user.put(`/tasks/${task._id}/`, {
requiresApproval: true,
});
let memberTasks = await member.get('/tasks/user');
let syncedTask = find(memberTasks, findAssignedTask);
await expect(member.post(`/tasks/${syncedTask._id}/score/up`))
.to.eventually.be.rejected.and.to.eql({
code: 401,
error: 'NotAuthorized',
message: t('taskApprovalHasBeenRequested'),
});

await user.sync();
await member2.sync();
expect(user.notifications.length).to.equal(2);
expect(user.notifications[1].type).to.equal('GROUP_TASK_APPROVAL');
expect(member2.notifications.length).to.equal(2);
expect(member2.notifications[1].type).to.equal('GROUP_TASK_APPROVAL');

await member2.del(`/tasks/${task._id}`);

await user.sync();
await member2.sync();

expect(user.notifications.length).to.equal(1);
expect(member2.notifications.length).to.equal(1);
});

it('unlinks assigned user', async () => {
await user.del(`/tasks/${task._id}`);

Expand Down
111 changes: 101 additions & 10 deletions test/api/v3/integration/tasks/groups/POST-group_tasks_id_approve_userId.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,18 +53,29 @@ describe('POST /tasks/:id/approve/:userId', () => {

it('approves an assigned user', async () => {
await user.post(`/tasks/${task._id}/assign/${member._id}`);
await user.post(`/tasks/${task._id}/approve/${member._id}`);

let memberTasks = await member.get('/tasks/user');
let syncedTask = find(memberTasks, findAssignedTask);

await expect(member.post(`/tasks/${syncedTask._id}/score/up`))
.to.eventually.be.rejected.and.to.eql({
code: 401,
error: 'NotAuthorized',
message: t('taskApprovalHasBeenRequested'),
});

await user.post(`/tasks/${task._id}/approve/${member._id}`);

await member.sync();

expect(member.notifications.length).to.equal(2);
expect(member.notifications[0].type).to.equal('GROUP_TASK_APPROVED');
expect(member.notifications[0].data.message).to.equal(t('yourTaskHasBeenApproved', {taskText: task.text}));
expect(member.notifications[1].type).to.equal('SCORED_TASK');
expect(member.notifications.length).to.equal(3);
expect(member.notifications[1].type).to.equal('GROUP_TASK_APPROVED');
expect(member.notifications[1].data.message).to.equal(t('yourTaskHasBeenApproved', {taskText: task.text}));
expect(member.notifications[2].type).to.equal('SCORED_TASK');
expect(member.notifications[2].data.message).to.equal(t('yourTaskHasBeenApproved', {taskText: task.text}));

memberTasks = await member.get('/tasks/user');
syncedTask = find(memberTasks, findAssignedTask);

expect(syncedTask.group.approval.approved).to.be.true;
expect(syncedTask.group.approval.approvingUser).to.equal(user._id);
Expand All @@ -77,18 +88,28 @@ describe('POST /tasks/:id/approve/:userId', () => {
});

await member2.post(`/tasks/${task._id}/assign/${member._id}`);
await member2.post(`/tasks/${task._id}/approve/${member._id}`);

let memberTasks = await member.get('/tasks/user');
let syncedTask = find(memberTasks, findAssignedTask);

await expect(member.post(`/tasks/${syncedTask._id}/score/up`))
.to.eventually.be.rejected.and.to.eql({
code: 401,
error: 'NotAuthorized',
message: t('taskApprovalHasBeenRequested'),
});

await member2.post(`/tasks/${task._id}/approve/${member._id}`);
await member.sync();

expect(member.notifications.length).to.equal(2);
expect(member.notifications[0].type).to.equal('GROUP_TASK_APPROVED');
expect(member.notifications[0].data.message).to.equal(t('yourTaskHasBeenApproved', {taskText: task.text}));
expect(member.notifications[1].type).to.equal('SCORED_TASK');
expect(member.notifications.length).to.equal(3);
expect(member.notifications[1].type).to.equal('GROUP_TASK_APPROVED');
expect(member.notifications[1].data.message).to.equal(t('yourTaskHasBeenApproved', {taskText: task.text}));
expect(member.notifications[2].type).to.equal('SCORED_TASK');
expect(member.notifications[2].data.message).to.equal(t('yourTaskHasBeenApproved', {taskText: task.text}));

memberTasks = await member.get('/tasks/user');
syncedTask = find(memberTasks, findAssignedTask);

expect(syncedTask.group.approval.approved).to.be.true;
expect(syncedTask.group.approval.approvingUser).to.equal(member2._id);
Expand Down Expand Up @@ -132,6 +153,16 @@ describe('POST /tasks/:id/approve/:userId', () => {
});

await member2.post(`/tasks/${task._id}/assign/${member._id}`);

let memberTasks = await member.get('/tasks/user');
let syncedTask = find(memberTasks, findAssignedTask);
await expect(member.post(`/tasks/${syncedTask._id}/score/up`))
.to.eventually.be.rejected.and.to.eql({
code: 401,
error: 'NotAuthorized',
message: t('taskApprovalHasBeenRequested'),
});

await member2.post(`/tasks/${task._id}/approve/${member._id}`);
await expect(user.post(`/tasks/${task._id}/approve/${member._id}`))
.to.eventually.be.rejected.and.to.eql({
Expand All @@ -141,6 +172,17 @@ describe('POST /tasks/:id/approve/:userId', () => {
});
});

it('prevents approving a task if it is not waiting for approval', async () => {
await user.post(`/tasks/${task._id}/assign/${member._id}`);

await expect(user.post(`/tasks/${task._id}/approve/${member._id}`))
.to.eventually.be.rejected.and.to.eql({
code: 401,
error: 'NotAuthorized',
message: t('taskApprovalWasNotRequested'),
});
});

it('completes master task when single-completion task is approved', async () => {
let sharedCompletionTask = await user.post(`/tasks/group/${guild._id}`, {
text: 'shared completion todo',
Expand All @@ -151,6 +193,16 @@ describe('POST /tasks/:id/approve/:userId', () => {

await user.post(`/tasks/${sharedCompletionTask._id}/assign/${member._id}`);
await user.post(`/tasks/${sharedCompletionTask._id}/assign/${member2._id}`);

let memberTasks = await member.get('/tasks/user');
let syncedTask = find(memberTasks, findAssignedTask);
await expect(member.post(`/tasks/${syncedTask._id}/score/up`))
.to.eventually.be.rejected.and.to.eql({
code: 401,
error: 'NotAuthorized',
message: t('taskApprovalHasBeenRequested'),
});

await user.post(`/tasks/${sharedCompletionTask._id}/approve/${member._id}`);

let groupTasks = await user.get(`/tasks/group/${guild._id}?type=completedTodos`);
Expand All @@ -172,6 +224,16 @@ describe('POST /tasks/:id/approve/:userId', () => {

await user.post(`/tasks/${sharedCompletionTask._id}/assign/${member._id}`);
await user.post(`/tasks/${sharedCompletionTask._id}/assign/${member2._id}`);

let memberTasks = await member.get('/tasks/user');
let syncedTask = find(memberTasks, findAssignedTask);
await expect(member.post(`/tasks/${syncedTask._id}/score/up`))
.to.eventually.be.rejected.and.to.eql({
code: 401,
error: 'NotAuthorized',
message: t('taskApprovalHasBeenRequested'),
});

await user.post(`/tasks/${sharedCompletionTask._id}/approve/${member._id}`);

let member2Tasks = await member2.get('/tasks/user');
Expand All @@ -193,6 +255,16 @@ describe('POST /tasks/:id/approve/:userId', () => {

await user.post(`/tasks/${sharedCompletionTask._id}/assign/${member._id}`);
await user.post(`/tasks/${sharedCompletionTask._id}/assign/${member2._id}`);

let memberTasks = await member.get('/tasks/user');
let syncedTask = find(memberTasks, findAssignedTask);
await expect(member.post(`/tasks/${syncedTask._id}/score/up`))
.to.eventually.be.rejected.and.to.eql({
code: 401,
error: 'NotAuthorized',
message: t('taskApprovalHasBeenRequested'),
});

await user.post(`/tasks/${sharedCompletionTask._id}/approve/${member._id}`);

let groupTasks = await user.get(`/tasks/group/${guild._id}`);
Expand All @@ -214,6 +286,25 @@ describe('POST /tasks/:id/approve/:userId', () => {

await user.post(`/tasks/${sharedCompletionTask._id}/assign/${member._id}`);
await user.post(`/tasks/${sharedCompletionTask._id}/assign/${member2._id}`);

let memberTasks = await member.get('/tasks/user');
let syncedTask = find(memberTasks, findAssignedTask);
await expect(member.post(`/tasks/${syncedTask._id}/score/up`))
.to.eventually.be.rejected.and.to.eql({
code: 401,
error: 'NotAuthorized',
message: t('taskApprovalHasBeenRequested'),
});

let member2Tasks = await member2.get('/tasks/user');
let member2SyncedTask = find(member2Tasks, findAssignedTask);
await expect(member2.post(`/tasks/${member2SyncedTask._id}/score/up`))
.to.eventually.be.rejected.and.to.eql({
code: 401,
error: 'NotAuthorized',
message: t('taskApprovalHasBeenRequested'),
});

await user.post(`/tasks/${sharedCompletionTask._id}/approve/${member._id}`);
await user.post(`/tasks/${sharedCompletionTask._id}/approve/${member2._id}`);

Expand Down
17 changes: 14 additions & 3 deletions test/api/v3/integration/tasks/groups/POST-group_tasks_id_needs-work_userId.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe('POST /tasks/:id/needs-work/:userId', () => {
});
});

it('marks as task as needing more work', async () => {
it('marks a task as needing more work', async () => {
const initialNotifications = member.notifications.length;

await user.post(`/tasks/${task._id}/assign/${member._id}`);
Expand All @@ -77,7 +77,7 @@ describe('POST /tasks/:id/needs-work/:userId', () => {
expect(syncedTask.group.approval.requestedDate).to.equal(undefined);

// Check that the notification is correct
expect(member.notifications.length).to.equal(initialNotifications + 1);
expect(member.notifications.length).to.equal(initialNotifications + 2);
const notification = member.notifications[member.notifications.length - 1];
expect(notification.type).to.equal('GROUP_TASK_NEEDS_WORK');

Expand Down Expand Up @@ -131,7 +131,7 @@ describe('POST /tasks/:id/needs-work/:userId', () => {
expect(syncedTask.group.approval.requested).to.equal(false);
expect(syncedTask.group.approval.requestedDate).to.equal(undefined);

expect(member.notifications.length).to.equal(initialNotifications + 1);
expect(member.notifications.length).to.equal(initialNotifications + 2);
const notification = member.notifications[member.notifications.length - 1];
expect(notification.type).to.equal('GROUP_TASK_NEEDS_WORK');

Expand Down Expand Up @@ -167,6 +167,17 @@ describe('POST /tasks/:id/needs-work/:userId', () => {
});

await member2.post(`/tasks/${task._id}/assign/${member._id}`);

let memberTasks = await member.get('/tasks/user');
let syncedTask = find(memberTasks, findAssignedTask);

await expect(member.post(`/tasks/${syncedTask._id}/score/up`))
.to.eventually.be.rejected.and.to.eql({
code: 401,
error: 'NotAuthorized',
message: t('taskApprovalHasBeenRequested'),
});

await member2.post(`/tasks/${task._id}/approve/${member._id}`);
await expect(user.post(`/tasks/${task._id}/needs-work/${member._id}`))
.to.eventually.be.rejected.and.to.eql({
Expand Down
7 changes: 7 additions & 0 deletions test/api/v3/integration/tasks/groups/POST-group_tasks_id_score_direction.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,13 @@ describe('POST /tasks/:id/score/:direction', () => {
let memberTasks = await member.get('/tasks/user');
let syncedTask = find(memberTasks, findAssignedTask);

await expect(member.post(`/tasks/${syncedTask._id}/score/up`))
.to.eventually.be.rejected.and.to.eql({
code: 401,
error: 'NotAuthorized',
message: t('taskApprovalHasBeenRequested'),
});

await user.post(`/tasks/${task._id}/approve/${member._id}`);

await member.post(`/tasks/${syncedTask._id}/score/up`);
Expand Down
11 changes: 11 additions & 0 deletions test/api/v3/integration/tasks/groups/POST-tasks_group_id_assign_user_id.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,17 @@ describe('POST /tasks/:taskId/assign/:memberId', () => {
expect(syncedTask).to.exist;
});

it('sends a notification to assigned user', async () => {
await user.post(`/tasks/${task._id}/assign/${member._id}`);
await member.sync();

let groupTask = await user.get(`/tasks/group/${guild._id}`);

expect(member.notifications.length).to.equal(1);
expect(member.notifications[0].type).to.equal('GROUP_TASK_ASSIGNED');
expect(member.notifications[0].taskId).to.equal(groupTask._id);
});

it('assigns a task to multiple users', async () => {
await user.post(`/tasks/${task._id}/assign/${member._id}`);
await user.post(`/tasks/${task._id}/assign/${member2._id}`);
Expand Down
7 changes: 7 additions & 0 deletions test/api/v3/integration/tasks/groups/POST-tasks_task_id_unassign.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,13 @@ describe('POST /tasks/:taskId/unassign/:memberId', () => {
expect(syncedTask).to.not.exist;
});

it('removes task assignment notification from unassigned user', async () => {
await user.post(`/tasks/${task._id}/unassign/${member._id}`);

await member.sync();
expect(member.notifications.length).to.equal(0);
});

it('unassigns a user and only that user from a task', async () => {
await user.post(`/tasks/${task._id}/assign/${member2._id}`);

Expand Down
Loading

0 comments on commit 1135ab9

Please sign in to comment.