From 72a0df8c24d58688cd6b796e76de8a0cf27acfbb Mon Sep 17 00:00:00 2001 From: Abhishek Date: Sat, 22 Feb 2025 02:07:07 +0530 Subject: [PATCH 1/4] Fixed Deletion of View Only Channel --- .../channelList/views/Channel/ChannelItem.vue | 18 +++++++++---- .../frontend/shared/data/resources.js | 9 ++++++- .../contentcuration/viewsets/user.py | 26 ++++++++++++++++++- 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelList/views/Channel/ChannelItem.vue b/contentcuration/contentcuration/frontend/channelList/views/Channel/ChannelItem.vue index ae440d54ab..3102c07f34 100644 --- a/contentcuration/contentcuration/frontend/channelList/views/Channel/ChannelItem.vue +++ b/contentcuration/contentcuration/frontend/channelList/views/Channel/ChannelItem.vue @@ -343,13 +343,21 @@ } }, methods: { - ...mapActions('channel', ['deleteChannel']), + ...mapActions('channel', ['deleteChannel', 'removeViewer']), ...mapMutations('channel', { updateChannel: 'UPDATE_CHANNEL' }), handleDelete() { - this.deleteChannel(this.channelId).then(() => { - this.deleteDialog = false; - this.$store.dispatch('showSnackbarSimple', this.$tr('channelDeletedSnackbar')); - }); + if (!this.canEdit) { + const currentUserId = this.$store.state.session.currentUser.id; + this.removeViewer({ channelId: this.channelId, userId: currentUserId }).then(() => { + this.deleteDialog = false; + this.$store.dispatch('showSnackbarSimple', this.$tr('channelDeletedSnackbar')); + }); + } else { + this.deleteChannel(this.channelId).then(() => { + this.deleteDialog = false; + this.$store.dispatch('showSnackbarSimple', this.$tr('channelDeletedSnackbar')); + }); + } }, goToChannelRoute() { this.linkToChannelTree diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js index 6f180dc08b..ad09f33fee 100644 --- a/contentcuration/contentcuration/frontend/shared/data/resources.js +++ b/contentcuration/contentcuration/frontend/shared/data/resources.js @@ -2058,7 +2058,14 @@ export const ChannelUser = new APIResource({ }); }, removeViewer(channel, user) { - return ViewerM2M.delete([user, channel]); + const modelUrl = urls.channeluser_remove_self(user); + const params = { channel_id: channel }; + return ViewerM2M.delete([user, channel]) + .then(() => client.delete(modelUrl, { params })) + .then(() => Channel.table.delete(channel)) + .catch(err => { + throw err; + }); }, fetchCollection(params) { return client.get(this.collectionUrl(), { params }).then(response => { diff --git a/contentcuration/contentcuration/viewsets/user.py b/contentcuration/contentcuration/viewsets/user.py index 766e3097a0..852708436d 100644 --- a/contentcuration/contentcuration/viewsets/user.py +++ b/contentcuration/contentcuration/viewsets/user.py @@ -19,6 +19,7 @@ from rest_framework.permissions import BasePermission from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response +from rest_framework import status from contentcuration.constants import feature_flags from contentcuration.models import boolean_val @@ -266,7 +267,30 @@ def create_from_changes(self, changes): def delete_from_changes(self, changes): return self._handle_relationship_changes(changes) - + @action(detail=True, methods=['delete']) + def remove_self(self, request, pk=None): + """ + Allows a user to remove themselves from a channel as a viewer. + """ + user = self.get_object() + channel_id = request.query_params.get('channel_id', None) + + if not channel_id: + return Response({'detail': 'channel_id is required.'}, status=status.HTTP_400_BAD_REQUEST) + + + channel = Channel.objects.get(id=channel_id) + if not channel: + return Response({'detail': 'Channel not found.'}, status=status.HTTP_404_NOT_FOUND) + + if request.user != user and not request.user.can_edit(channel_id): + return Response({'detail': 'You do not have permission to remove this user.'}, status=status.HTTP_403_FORBIDDEN) + + if channel.viewers.filter(id=user.id).exists(): + channel.viewers.remove(user) + return Response(status=status.HTTP_204_NO_CONTENT) + else: + return Response({'detail': 'User is not a viewer on this channel.'}, status=status.HTTP_400_BAD_REQUEST) class AdminUserFilter(FilterSet): keywords = CharFilter(method="filter_keywords") From 5f95c69067ab06a56940e2dfb2f5ee040eb3a4ce Mon Sep 17 00:00:00 2001 From: Abhishek Date: Thu, 13 Mar 2025 23:39:24 +0530 Subject: [PATCH 2/4] Added Unit Tests for Deletion of View Only Channels --- .../Channel/__tests__/channelItem.spec.js | 32 ++++++++++-- .../shared/data/__tests__/resources.spec.js | 52 ++++++++++++++++++- 2 files changed, 80 insertions(+), 4 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelList/views/Channel/__tests__/channelItem.spec.js b/contentcuration/contentcuration/frontend/channelList/views/Channel/__tests__/channelItem.spec.js index 79bcea297e..1e8b9d63d4 100644 --- a/contentcuration/contentcuration/frontend/channelList/views/Channel/__tests__/channelItem.spec.js +++ b/contentcuration/contentcuration/frontend/channelList/views/Channel/__tests__/channelItem.spec.js @@ -76,11 +76,37 @@ describe('channelItem', () => { wrapper.find('[data-test="token-listitem"]').trigger('click'); expect(wrapper.vm.tokenDialog).toBe(true); }); - it('clicking delete button in dialog should delete the channel', () => { + it('when user can edit, clicking delete button in dialog should call deleteChannel', async () => { + const deleteChannelSpy = jest.fn().mockResolvedValue(); + const removeViewerSpy = jest.fn().mockResolvedValue(); + wrapper = makeWrapper(true, deleteStub); + wrapper.setMethods({ + deleteChannel: deleteChannelSpy, + removeViewer: removeViewerSpy, + }); + + wrapper.setData({ deleteDialog: true }); + wrapper.find('[data-test="delete-modal"]').trigger('submit'); + await wrapper.vm.$nextTick(() => { + expect(deleteChannelSpy).toHaveBeenCalledWith(channelId); + expect(removeViewerSpy).not.toHaveBeenCalled(); + }); + }); + + it('when user cannot edit, clicking delete button in dialog should call removeViewer', async () => { + const deleteChannelSpy = jest.fn().mockResolvedValue(); + const removeViewerSpy = jest.fn().mockResolvedValue(); + wrapper = makeWrapper(false, deleteStub); + wrapper.setMethods({ + deleteChannel: deleteChannelSpy, + removeViewer: removeViewerSpy, + }); + wrapper.setData({ deleteDialog: true }); wrapper.find('[data-test="delete-modal"]').trigger('submit'); - wrapper.vm.$nextTick(() => { - expect(deleteStub).toHaveBeenCalled(); + await wrapper.vm.$nextTick(() => { + expect(removeViewerSpy).toHaveBeenCalledWith({ channelId, userId: 0 }); + expect(deleteChannelSpy).not.toHaveBeenCalled(); }); }); diff --git a/contentcuration/contentcuration/frontend/shared/data/__tests__/resources.spec.js b/contentcuration/contentcuration/frontend/shared/data/__tests__/resources.spec.js index 34b285e48e..4139dccb3f 100644 --- a/contentcuration/contentcuration/frontend/shared/data/__tests__/resources.spec.js +++ b/contentcuration/contentcuration/frontend/shared/data/__tests__/resources.spec.js @@ -1,9 +1,11 @@ import { UpdatedDescendantsChange } from '../changes'; +import { ViewerM2M, ChannelUser, Channel, ContentNode } from '../resources'; import db from 'shared/data/db'; import { CHANGE_TYPES, TABLE_NAMES } from 'shared/data/constants'; import { ContentKindsNames } from 'shared/leUtils/ContentKinds'; -import { ContentNode } from 'shared/data/resources'; import { mockChannelScope, resetMockChannelScope } from 'shared/utils/testing'; +import client from 'shared/client'; +import urls from 'shared/urls'; const CLIENTID = 'test-client-id'; @@ -170,5 +172,53 @@ describe('Resources', () => { expect(change.mods).toEqual(changes); }); }); + describe('ChannelUser resource', () => { + const testChannelId = 'test-channel-id'; + const testUserId = 'test-user-id'; + + beforeEach(async () => { + await db[TABLE_NAMES.VIEWER_M2M].clear(); + await db[TABLE_NAMES.CHANNEL].clear(); + jest.spyOn(client, 'delete').mockResolvedValue({}); + jest.spyOn(Channel.table, 'delete').mockResolvedValue(true); + jest.spyOn(urls, 'channeluser_remove_self').mockReturnValue(`fake_url_for_${testUserId}`); + }); + + afterEach(() => { + client.delete.mockRestore(); + Channel.table.delete.mockRestore(); + urls.channeluser_remove_self.mockRestore(); + }); + + it('should remove the user from the ViewerM2M table when removeViewer is called', async () => { + await ViewerM2M.add({ user: testUserId, channel: testChannelId }); + let viewer = await ViewerM2M.get([testUserId, testChannelId]); + expect(viewer).toBeTruthy(); + + await ChannelUser.removeViewer(testChannelId, testUserId); + + viewer = await ViewerM2M.get([testUserId, testChannelId]); + expect(viewer).toBeUndefined(); + expect(client.delete).toHaveBeenCalledWith(urls.channeluser_remove_self(testUserId), { + params: { channel_id: testChannelId }, + }); + }); + + it('should call Channel.table.delete(channel) when removeViewer is called', async () => { + await ViewerM2M.add({ user: testUserId, channel: testChannelId }); + const viewer = await ViewerM2M.get([testUserId, testChannelId]); + expect(viewer).toBeTruthy(); + await ChannelUser.removeViewer(testChannelId, testUserId); + expect(Channel.table.delete).toHaveBeenCalledWith(testChannelId); + }); + + it('should handle error from client.delete when removeViewer is called', async () => { + jest.spyOn(client, 'delete').mockRejectedValue(new Error('error deleting')); + await ViewerM2M.add({ user: testUserId, channel: testChannelId }); + await expect(ChannelUser.removeViewer(testChannelId, testUserId)).rejects.toThrow( + 'error deleting' + ); + }); + }); }); }); From 9790c4e7ab6e13ab760fb980de07ecd676321bd9 Mon Sep 17 00:00:00 2001 From: Abhishek Date: Thu, 3 Apr 2025 01:37:51 +0530 Subject: [PATCH 3/4] Added Suggested changes --- .../contentcuration/viewsets/user.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/contentcuration/contentcuration/viewsets/user.py b/contentcuration/contentcuration/viewsets/user.py index 852708436d..fa9d98d85f 100644 --- a/contentcuration/contentcuration/viewsets/user.py +++ b/contentcuration/contentcuration/viewsets/user.py @@ -11,6 +11,9 @@ from django.db.models import Value from django.db.models.functions import Cast from django.db.models.functions import Concat +from django.http import HttpResponseBadRequest +from django.http.response import HttpResponseForbidden +from django.http.response import HttpResponseNotFound from django_filters.rest_framework import BooleanFilter from django_filters.rest_framework import CharFilter from django_filters.rest_framework import FilterSet @@ -19,7 +22,7 @@ from rest_framework.permissions import BasePermission from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response -from rest_framework import status +from rest_framework.status import HTTP_204_NO_CONTENT from contentcuration.constants import feature_flags from contentcuration.models import boolean_val @@ -267,6 +270,7 @@ def create_from_changes(self, changes): def delete_from_changes(self, changes): return self._handle_relationship_changes(changes) + @action(detail=True, methods=['delete']) def remove_self(self, request, pk=None): """ @@ -276,21 +280,21 @@ def remove_self(self, request, pk=None): channel_id = request.query_params.get('channel_id', None) if not channel_id: - return Response({'detail': 'channel_id is required.'}, status=status.HTTP_400_BAD_REQUEST) - + return HttpResponseBadRequest('Channel ID is required.') channel = Channel.objects.get(id=channel_id) if not channel: - return Response({'detail': 'Channel not found.'}, status=status.HTTP_404_NOT_FOUND) + return HttpResponseNotFound("Channel not found {}".format(channel_id)) if request.user != user and not request.user.can_edit(channel_id): - return Response({'detail': 'You do not have permission to remove this user.'}, status=status.HTTP_403_FORBIDDEN) + return HttpResponseForbidden("You do not have permission to remove this user {}".format(user.id)) if channel.viewers.filter(id=user.id).exists(): channel.viewers.remove(user) - return Response(status=status.HTTP_204_NO_CONTENT) + return Response(status=HTTP_204_NO_CONTENT) else: - return Response({'detail': 'User is not a viewer on this channel.'}, status=status.HTTP_400_BAD_REQUEST) + return HttpResponseBadRequest('User is not a viewer of this channel.') + class AdminUserFilter(FilterSet): keywords = CharFilter(method="filter_keywords") From 1e4b9249a5d103c5a0ae7cd5014814315a76a237 Mon Sep 17 00:00:00 2001 From: Abhishek Date: Tue, 15 Apr 2025 20:29:37 +0530 Subject: [PATCH 4/4] Modified Dialog box and context menu strs --- .../channelList/views/Channel/ChannelItem.vue | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelList/views/Channel/ChannelItem.vue b/contentcuration/contentcuration/frontend/channelList/views/Channel/ChannelItem.vue index 3102c07f34..1b94439e23 100644 --- a/contentcuration/contentcuration/frontend/channelList/views/Channel/ChannelItem.vue +++ b/contentcuration/contentcuration/frontend/channelList/views/Channel/ChannelItem.vue @@ -190,7 +190,9 @@ icon="trash" /> - {{ $tr('deleteChannel') }} + + {{ canEdit ? $tr('deleteChannel') : $tr('removeChannel') }} + @@ -202,14 +204,14 @@ - {{ $tr('deletePrompt') }} + {{ canEdit ? $tr('deletePrompt') : $tr('removePrompt') }} { this.deleteDialog = false; - this.$store.dispatch('showSnackbarSimple', this.$tr('channelDeletedSnackbar')); + this.$store.dispatch('showSnackbarSimple', this.$tr('channelRemovedSnackbar')); }); } else { this.deleteChannel(this.channelId).then(() => { @@ -382,8 +384,14 @@ copyToken: 'Copy channel token', deleteChannel: 'Delete channel', deleteTitle: 'Delete this channel', + removeChannel: 'Remove from channel list', + removeBtn: 'Remove', + removeTitle: 'Remove from channel list', deletePrompt: 'This channel will be permanently deleted. This cannot be undone.', + removePrompt: + 'You have view-only access to this channel. Confirm that you want to remove it from your list of channels.', channelDeletedSnackbar: 'Channel deleted', + channelRemovedSnackbar: 'Channel removed', channelLanguageNotSetIndicator: 'No language set', cancel: 'Cancel', },