From 22aeabd3dbb99c5fe45b453b8d06972b0b11245a Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Mon, 23 Sep 2024 22:17:17 +0300 Subject: [PATCH 1/9] Fixes decline invitation --- .../vuex/channelList/__tests__/module.spec.js | 2 +- .../channelList/vuex/channelList/actions.js | 4 ++-- .../frontend/shared/data/resources.js | 3 +-- .../contentcuration/viewsets/invitation.py | 15 ++++++++++++--- 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelList/vuex/channelList/__tests__/module.spec.js b/contentcuration/contentcuration/frontend/channelList/vuex/channelList/__tests__/module.spec.js index 2998520efd..235791f11a 100644 --- a/contentcuration/contentcuration/frontend/channelList/vuex/channelList/__tests__/module.spec.js +++ b/contentcuration/contentcuration/frontend/channelList/vuex/channelList/__tests__/module.spec.js @@ -93,7 +93,7 @@ describe('invitation actions', () => { store.commit('channelList/SET_INVITATION_LIST', [{ id, ...invitation }]); }); it('should call client.delete', () => { - const updateSpy = jest.spyOn(Invitation, 'update'); + const updateSpy = jest.spyOn(Invitation, 'accept'); return store.dispatch('channelList/declineInvitation', id).then(() => { expect(updateSpy).toHaveBeenCalled(); expect(updateSpy.mock.calls[0][0]).toBe(id); diff --git a/contentcuration/contentcuration/frontend/channelList/vuex/channelList/actions.js b/contentcuration/contentcuration/frontend/channelList/vuex/channelList/actions.js index 98178b39f4..0419f42e1a 100644 --- a/contentcuration/contentcuration/frontend/channelList/vuex/channelList/actions.js +++ b/contentcuration/contentcuration/frontend/channelList/vuex/channelList/actions.js @@ -56,7 +56,7 @@ export function loadInvitationList(context) { export function acceptInvitation(context, invitationId) { const invitation = context.getters.getInvitation(invitationId); - return Invitation.accept(invitationId) + return Invitation.accept(invitationId, { accepted: true }) .then(() => { return context .dispatch('channel/loadChannel', invitation.channel, { root: true }) @@ -76,7 +76,7 @@ export function acceptInvitation(context, invitationId) { } export function declineInvitation(context, invitationId) { - return Invitation.update(invitationId, { declined: true }).then(() => { + return Invitation.accept(invitationId, { declined: true }).then(() => { return context.commit('REMOVE_INVITATION', invitationId); }); } diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js index 5925bd3c8e..87ad204d2a 100644 --- a/contentcuration/contentcuration/frontend/shared/data/resources.js +++ b/contentcuration/contentcuration/frontend/shared/data/resources.js @@ -1930,8 +1930,7 @@ export const Invitation = new Resource({ urlName: 'invitation', indexFields: ['channel'], - accept(id) { - const changes = { accepted: true }; + accept(id, changes) { return client.post(window.Urls.invitationAccept(id), changes).then(() => { return this.transaction({ mode: 'rw' }, () => { return this.table.update(id, changes); diff --git a/contentcuration/contentcuration/viewsets/invitation.py b/contentcuration/contentcuration/viewsets/invitation.py index 55507297ef..274cc21a00 100644 --- a/contentcuration/contentcuration/viewsets/invitation.py +++ b/contentcuration/contentcuration/viewsets/invitation.py @@ -140,12 +140,21 @@ def perform_update(self, serializer): @action(detail=True, methods=["post"]) def accept(self, request, pk=None): invitation = self.get_object() - invitation.accept() - invitation.accepted = True + changes = request.data + + accepted = changes.get("accepted", False) + if accepted: + invitation.accept() + invitation.accepted = accepted + + declined = changes.get("declined", False) + if declined: + invitation.declined = declined + invitation.save() Change.create_change( generate_update_event( - invitation.id, INVITATION, {"accepted": True}, channel_id=invitation.channel_id + invitation.id, INVITATION, changes, channel_id=invitation.channel_id ), applied=True, created_by_id=request.user.id ) return Response({"status": "success"}) From 0730fbe72ebd606ec4baca9ca91defa99f07e3ac Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Mon, 23 Sep 2024 23:05:19 +0300 Subject: [PATCH 2/9] Fixes failing tests --- .../contentcuration/tests/viewsets/test_invitation.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/contentcuration/contentcuration/tests/viewsets/test_invitation.py b/contentcuration/contentcuration/tests/viewsets/test_invitation.py index a14aae8ccf..bfe3a16b9f 100644 --- a/contentcuration/contentcuration/tests/viewsets/test_invitation.py +++ b/contentcuration/contentcuration/tests/viewsets/test_invitation.py @@ -283,7 +283,10 @@ def test_update_invitation_accept(self): invitation = models.Invitation.objects.create(**self.invitation_db_metadata) self.client.force_authenticate(user=self.invited_user) - response = self.client.post(reverse("invitation-accept", kwargs={"pk": invitation.id})) + response = self.client.post( + reverse("invitation-accept", kwargs={"pk": invitation.id}), + {"accepted": True}, + ) self.assertEqual(response.status_code, 200, response.content) try: invitation = models.Invitation.objects.get(id=invitation.id) From 3c8d98ebad43603cd1d399532ade18c4c596e603 Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Mon, 23 Sep 2024 23:13:09 +0300 Subject: [PATCH 3/9] Adds test for decline invitation --- .../tests/viewsets/test_invitation.py | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/contentcuration/contentcuration/tests/viewsets/test_invitation.py b/contentcuration/contentcuration/tests/viewsets/test_invitation.py index bfe3a16b9f..adf5a84f59 100644 --- a/contentcuration/contentcuration/tests/viewsets/test_invitation.py +++ b/contentcuration/contentcuration/tests/viewsets/test_invitation.py @@ -322,3 +322,26 @@ def test_delete_invitation(self): reverse("invitation-detail", kwargs={"pk": invitation.id}) ) self.assertEqual(response.status_code, 405, response.content) + + def test_update_invitation_decline(self): + invitation = models.Invitation.objects.create(**self.invitation_db_metadata) + + self.client.force_authenticate(user=self.invited_user) + response = self.client.post( + reverse("invitation-accept", kwargs={"pk": invitation.id}), + {"declined": True}, + ) + self.assertEqual(response.status_code, 200, response.content) + try: + invitation = models.Invitation.objects.get(id=invitation.id) + except models.Invitation.DoesNotExist: + self.fail("Invitation was deleted") + + self.assertTrue(invitation.declined) + self.assertFalse(self.channel.editors.filter(pk=self.invited_user.id).exists()) + self.assertTrue( + models.Invitation.objects.filter( + email=self.invited_user.email, channel=self.channel + ).exists() + ) + self.assertTrue(models.Change.objects.filter(channel=self.channel).exists()) From a6f171945f4332bc0f23f9046e53735e75bca8fc Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Tue, 24 Sep 2024 01:28:51 +0300 Subject: [PATCH 4/9] Splts accept endpoint for clarity --- .../vuex/channelList/__tests__/module.spec.js | 3 +- .../channelList/vuex/channelList/actions.js | 4 +-- .../frontend/shared/data/resources.js | 12 +++++-- .../tests/viewsets/test_invitation.py | 10 ++---- .../contentcuration/viewsets/invitation.py | 32 +++++++++++++------ 5 files changed, 37 insertions(+), 24 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelList/vuex/channelList/__tests__/module.spec.js b/contentcuration/contentcuration/frontend/channelList/vuex/channelList/__tests__/module.spec.js index 235791f11a..35d9124ddc 100644 --- a/contentcuration/contentcuration/frontend/channelList/vuex/channelList/__tests__/module.spec.js +++ b/contentcuration/contentcuration/frontend/channelList/vuex/channelList/__tests__/module.spec.js @@ -93,11 +93,10 @@ describe('invitation actions', () => { store.commit('channelList/SET_INVITATION_LIST', [{ id, ...invitation }]); }); it('should call client.delete', () => { - const updateSpy = jest.spyOn(Invitation, 'accept'); + const updateSpy = jest.spyOn(Invitation, 'decline'); return store.dispatch('channelList/declineInvitation', id).then(() => { expect(updateSpy).toHaveBeenCalled(); expect(updateSpy.mock.calls[0][0]).toBe(id); - expect(updateSpy.mock.calls[0][1]).toEqual({ declined: true }); updateSpy.mockRestore(); }); }); diff --git a/contentcuration/contentcuration/frontend/channelList/vuex/channelList/actions.js b/contentcuration/contentcuration/frontend/channelList/vuex/channelList/actions.js index 0419f42e1a..93cf0405c4 100644 --- a/contentcuration/contentcuration/frontend/channelList/vuex/channelList/actions.js +++ b/contentcuration/contentcuration/frontend/channelList/vuex/channelList/actions.js @@ -56,7 +56,7 @@ export function loadInvitationList(context) { export function acceptInvitation(context, invitationId) { const invitation = context.getters.getInvitation(invitationId); - return Invitation.accept(invitationId, { accepted: true }) + return Invitation.accept(invitationId) .then(() => { return context .dispatch('channel/loadChannel', invitation.channel, { root: true }) @@ -76,7 +76,7 @@ export function acceptInvitation(context, invitationId) { } export function declineInvitation(context, invitationId) { - return Invitation.accept(invitationId, { declined: true }).then(() => { + return Invitation.decline(invitationId).then(() => { return context.commit('REMOVE_INVITATION', invitationId); }); } diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js index 87ad204d2a..25cd88fadf 100644 --- a/contentcuration/contentcuration/frontend/shared/data/resources.js +++ b/contentcuration/contentcuration/frontend/shared/data/resources.js @@ -1930,8 +1930,16 @@ export const Invitation = new Resource({ urlName: 'invitation', indexFields: ['channel'], - accept(id, changes) { - return client.post(window.Urls.invitationAccept(id), changes).then(() => { + accept(id) { + const changes = { accepted: true }; + return this._handleInvitation(id, window.Urls.invitationAccept(id), changes); + }, + decline(id) { + const changes = { declined: true }; + return this._handleInvitation(id, window.Urls.invitationDecline(id), changes); + }, + _handleInvitation(id, url, changes) { + return client.post(url).then(() => { return this.transaction({ mode: 'rw' }, () => { return this.table.update(id, changes); }); diff --git a/contentcuration/contentcuration/tests/viewsets/test_invitation.py b/contentcuration/contentcuration/tests/viewsets/test_invitation.py index adf5a84f59..fad9b52be4 100644 --- a/contentcuration/contentcuration/tests/viewsets/test_invitation.py +++ b/contentcuration/contentcuration/tests/viewsets/test_invitation.py @@ -283,10 +283,7 @@ def test_update_invitation_accept(self): invitation = models.Invitation.objects.create(**self.invitation_db_metadata) self.client.force_authenticate(user=self.invited_user) - response = self.client.post( - reverse("invitation-accept", kwargs={"pk": invitation.id}), - {"accepted": True}, - ) + response = self.client.post(reverse("invitation-accept", kwargs={"pk": invitation.id})) self.assertEqual(response.status_code, 200, response.content) try: invitation = models.Invitation.objects.get(id=invitation.id) @@ -327,10 +324,7 @@ def test_update_invitation_decline(self): invitation = models.Invitation.objects.create(**self.invitation_db_metadata) self.client.force_authenticate(user=self.invited_user) - response = self.client.post( - reverse("invitation-accept", kwargs={"pk": invitation.id}), - {"declined": True}, - ) + response = self.client.post(reverse("invitation-decline", kwargs={"pk": invitation.id})) self.assertEqual(response.status_code, 200, response.content) try: invitation = models.Invitation.objects.get(id=invitation.id) diff --git a/contentcuration/contentcuration/viewsets/invitation.py b/contentcuration/contentcuration/viewsets/invitation.py index 274cc21a00..808b1b2227 100644 --- a/contentcuration/contentcuration/viewsets/invitation.py +++ b/contentcuration/contentcuration/viewsets/invitation.py @@ -137,24 +137,36 @@ def perform_update(self, serializer): instance = serializer.save() instance.save() + def create_invitation_change(self, invitation, changes, user_id): + Change.create_change( + generate_update_event( + invitation.id, INVITATION, changes, channel_id=invitation.channel_id + ), applied=True, created_by_id=user_id + ) + @action(detail=True, methods=["post"]) def accept(self, request, pk=None): invitation = self.get_object() - changes = request.data - - accepted = changes.get("accepted", False) - if accepted: - invitation.accept() - invitation.accepted = accepted + invitation.accept() + invitation.accepted = True + invitation.save() - declined = changes.get("declined", False) - if declined: - invitation.declined = declined + Change.create_change( + generate_update_event( + invitation.id, INVITATION, {"accepted": True}, channel_id=invitation.channel_id + ), applied=True, created_by_id=request.user.id + ) + return Response({"status": "success"}) + @action(detail=True, methods=["post"]) + def decline(self, request, pk=None): + invitation = self.get_object() + invitation.declined = True invitation.save() + Change.create_change( generate_update_event( - invitation.id, INVITATION, changes, channel_id=invitation.channel_id + invitation.id, INVITATION, {"declined": True}, channel_id=invitation.channel_id ), applied=True, created_by_id=request.user.id ) return Response({"status": "success"}) From 0284d8cd9f77581d9fb0de36e8422eb9683fea8b Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Tue, 24 Sep 2024 01:31:44 +0300 Subject: [PATCH 5/9] removes redundant method --- contentcuration/contentcuration/viewsets/invitation.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/contentcuration/contentcuration/viewsets/invitation.py b/contentcuration/contentcuration/viewsets/invitation.py index 808b1b2227..2602a28cb6 100644 --- a/contentcuration/contentcuration/viewsets/invitation.py +++ b/contentcuration/contentcuration/viewsets/invitation.py @@ -137,13 +137,6 @@ def perform_update(self, serializer): instance = serializer.save() instance.save() - def create_invitation_change(self, invitation, changes, user_id): - Change.create_change( - generate_update_event( - invitation.id, INVITATION, changes, channel_id=invitation.channel_id - ), applied=True, created_by_id=user_id - ) - @action(detail=True, methods=["post"]) def accept(self, request, pk=None): invitation = self.get_object() From 0b25c35daa256a8645676a034102be2b02935fba Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Tue, 24 Sep 2024 01:41:24 +0300 Subject: [PATCH 6/9] small tweak --- .../contentcuration/frontend/shared/data/resources.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js index 25cd88fadf..ef595d6e6a 100644 --- a/contentcuration/contentcuration/frontend/shared/data/resources.js +++ b/contentcuration/contentcuration/frontend/shared/data/resources.js @@ -1939,7 +1939,7 @@ export const Invitation = new Resource({ return this._handleInvitation(id, window.Urls.invitationDecline(id), changes); }, _handleInvitation(id, url, changes) { - return client.post(url).then(() => { + return client.post(url, changes).then(() => { return this.transaction({ mode: 'rw' }, () => { return this.table.update(id, changes); }); From 0fa0e31aac220c65f1586bdbf53ad2e4ea340913 Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Tue, 24 Sep 2024 01:50:27 +0300 Subject: [PATCH 7/9] renames test variable --- .../channelList/vuex/channelList/__tests__/module.spec.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelList/vuex/channelList/__tests__/module.spec.js b/contentcuration/contentcuration/frontend/channelList/vuex/channelList/__tests__/module.spec.js index 35d9124ddc..ce169ef867 100644 --- a/contentcuration/contentcuration/frontend/channelList/vuex/channelList/__tests__/module.spec.js +++ b/contentcuration/contentcuration/frontend/channelList/vuex/channelList/__tests__/module.spec.js @@ -93,11 +93,11 @@ describe('invitation actions', () => { store.commit('channelList/SET_INVITATION_LIST', [{ id, ...invitation }]); }); it('should call client.delete', () => { - const updateSpy = jest.spyOn(Invitation, 'decline'); + const declineSpy = jest.spyOn(Invitation, 'decline'); return store.dispatch('channelList/declineInvitation', id).then(() => { - expect(updateSpy).toHaveBeenCalled(); - expect(updateSpy.mock.calls[0][0]).toBe(id); - updateSpy.mockRestore(); + expect(declineSpy).toHaveBeenCalled(); + expect(declineSpy.mock.calls[0][0]).toBe(id); + declineSpy.mockRestore(); }); }); it('should not load and set the invited channel', () => { From e9e603c85f32dfeb51f0669dafb74a37b24bc89b Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Tue, 24 Sep 2024 07:22:29 +0300 Subject: [PATCH 8/9] Minor code improvements --- contentcuration/contentcuration/viewsets/invitation.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/contentcuration/contentcuration/viewsets/invitation.py b/contentcuration/contentcuration/viewsets/invitation.py index 2602a28cb6..81b1e5c680 100644 --- a/contentcuration/contentcuration/viewsets/invitation.py +++ b/contentcuration/contentcuration/viewsets/invitation.py @@ -143,7 +143,6 @@ def accept(self, request, pk=None): invitation.accept() invitation.accepted = True invitation.save() - Change.create_change( generate_update_event( invitation.id, INVITATION, {"accepted": True}, channel_id=invitation.channel_id @@ -156,7 +155,6 @@ def decline(self, request, pk=None): invitation = self.get_object() invitation.declined = True invitation.save() - Change.create_change( generate_update_event( invitation.id, INVITATION, {"declined": True}, channel_id=invitation.channel_id From 7e1af0c8c1d3f2c18363225ece89ccc5f1862b11 Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Tue, 24 Sep 2024 07:28:25 +0300 Subject: [PATCH 9/9] Uncomments accept invitation tests --- .../vuex/channelList/__tests__/module.spec.js | 69 +++++++++---------- .../frontend/shared/data/resources.js | 2 +- 2 files changed, 35 insertions(+), 36 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelList/vuex/channelList/__tests__/module.spec.js b/contentcuration/contentcuration/frontend/channelList/vuex/channelList/__tests__/module.spec.js index ce169ef867..e6df06501d 100644 --- a/contentcuration/contentcuration/frontend/channelList/vuex/channelList/__tests__/module.spec.js +++ b/contentcuration/contentcuration/frontend/channelList/vuex/channelList/__tests__/module.spec.js @@ -53,41 +53,40 @@ describe('invitation actions', () => { }); }); }); - // TODO: Figure out why client isn't getting mocked then uncomment this - // describe('acceptInvitation action', () => { - // const channel = { id: channel_id, name: 'test', deleted: false, edit: true }; - // beforeEach(() => { - // store.commit('channelList/SET_INVITATION_LIST', [{ id, ...invitation }]); - // return Channel.add(channel); - // }); - // afterEach(() => { - // return Channel.table.toCollection().delete(); - // }); - // it('should call accept', () => { - // const updateSpy = jest.spyOn(Invitation, 'accept'); - // return store.dispatch('channelList/acceptInvitation', id).then(() => { - // expect(updateSpy).toHaveBeenCalled(); - // expect(updateSpy.mock.calls[0][0]).toBe(id); - // updateSpy.mockRestore(); - // }); - // }); - // it('should load and set the invited channel', () => { - // return store.dispatch('channelList/acceptInvitation', id).then(() => { - // expect(store.getters['channel/getChannel'](channel_id).id).toBeTruthy(); - // }); - // }); - // it('should remove the invitation from the list', () => { - // return store.dispatch('channelList/acceptInvitation', id).then(() => { - // expect(store.getters['channelList/getInvitation'](id)).toBeFalsy(); - // }); - // }); - // it('should set the correct permission on the accepted invite', () => { - // return store.dispatch('channelList/acceptInvitation', id).then(() => { - // expect(store.getters['channel/getChannel'](channel_id).view).toBe(true); - // expect(store.getters['channel/getChannel'](channel_id).edit).toBe(false); - // }); - // }); - // }); + describe('acceptInvitation action', () => { + const channel = { id: channel_id, name: 'test', deleted: false, edit: true }; + beforeEach(() => { + store.commit('channelList/SET_INVITATION_LIST', [{ id, ...invitation }]); + return Channel.add(channel); + }); + afterEach(() => { + return Channel.table.toCollection().delete(); + }); + it('should call accept', () => { + const acceptSpy = jest.spyOn(Invitation, 'accept'); + return store.dispatch('channelList/acceptInvitation', id).then(() => { + expect(acceptSpy).toHaveBeenCalled(); + expect(acceptSpy.mock.calls[0][0]).toBe(id); + acceptSpy.mockRestore(); + }); + }); + it('should load and set the invited channel', () => { + return store.dispatch('channelList/acceptInvitation', id).then(() => { + expect(store.getters['channel/getChannel'](channel_id).id).toBeTruthy(); + }); + }); + it('should remove the invitation from the list', () => { + return store.dispatch('channelList/acceptInvitation', id).then(() => { + expect(store.getters['channelList/getInvitation'](id)).toBeFalsy(); + }); + }); + it('should set the correct permission on the accepted invite', () => { + return store.dispatch('channelList/acceptInvitation', id).then(() => { + expect(store.getters['channel/getChannel'](channel_id).view).toBe(true); + expect(store.getters['channel/getChannel'](channel_id).edit).toBe(false); + }); + }); + }); describe('declineInvitation action', () => { beforeEach(() => { store.commit('channelList/SET_INVITATION_LIST', [{ id, ...invitation }]); diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js index ef595d6e6a..25cd88fadf 100644 --- a/contentcuration/contentcuration/frontend/shared/data/resources.js +++ b/contentcuration/contentcuration/frontend/shared/data/resources.js @@ -1939,7 +1939,7 @@ export const Invitation = new Resource({ return this._handleInvitation(id, window.Urls.invitationDecline(id), changes); }, _handleInvitation(id, url, changes) { - return client.post(url, changes).then(() => { + return client.post(url).then(() => { return this.transaction({ mode: 'rw' }, () => { return this.table.update(id, changes); });