Handle deletion of a Channel with a related Community Library Submission #5551
Conversation
AlexVelezLl
left a comment
AlexVelezLl
left a comment
There was a problem hiding this comment.
Just a general concern regarding computing the has_community_library_submission for every channel load.
| def filter_deleted(self, queryset, name, value): | ||
| has_cl_filter = self.request.query_params.get( | ||
| "has_community_library_submission" | ||
| ) | ||
| if has_cl_filter and has_cl_filter.lower() == "true": | ||
| return queryset | ||
| return queryset.filter(deleted=value) |
There was a problem hiding this comment.
Oh, I think we can leave this responsibility to the frontend, so if we ever want to filter by deleted channels that have been submitted to the Community Library, we can just adjust the frontend code!
| # check if channel has any Community Library submissions | ||
| queryset = queryset.annotate( | ||
| has_community_library_submission=Exists( | ||
| CommunityLibrarySubmission.objects.filter(channel_id=OuterRef("id")) | ||
| ) | ||
| ) |
There was a problem hiding this comment.
I am a bit skeptical about computing the has_community_library_submission here for every channel for a feature that may only appear in very few cases, such as removing a channel.
I think an alternative may be to create a RemoveChannelModal and load one Community Library submission for a given channel within that modal. With this, we will only load this data when needed, rather than on every single channel load. What do you think? @taoerman
…hub.com/taoerman/studio into issue-5459-Handle-deletion-of-a-Channel merge
| }; | ||
|
|
||
| if (params.has_community_library_submission === undefined) { | ||
| extendedParams.deleted = Boolean(params.deleted) && params.deleted.toString() === 'true'; |
There was a problem hiding this comment.
-
When filtering by Community Library:
- params.has_community_library_submission is present
- The if condition is false, so deleted is not set
- Backend doesn't filter by deleted → shows both deleted and non-deleted channels
-
When filtering by other types:
- params.has_community_library_submission is undefined
- The if condition is true, so deleted is set based on params.deleted
- Backend filters by deleted status as expected
AlexVelezLl
left a comment
AlexVelezLl
left a comment
There was a problem hiding this comment.
Just a couple of notes before merging this PR! 👐
| <VProgressCircular | ||
| indeterminate | ||
| size="24" | ||
| /> |
There was a problem hiding this comment.
Could we use the KCircularLoader component instead? As we are getting rid of the Vuetify components, its important to stop using them!
| import { defineComponent, ref, computed, watch, getCurrentInstance } from 'vue'; | ||
| import client from 'shared/client'; | ||
| export default defineComponent({ |
There was a problem hiding this comment.
Oh, we don't need here the defineComponent function, since we are working with component files, Vue automatically interprets that the default export of this file is the definition of the component.
| watch(dialog, newValue => { | ||
| if (newValue && props.canEdit) { | ||
| fetchCommunityLibrarySubmissionStatus(); | ||
| } else { | ||
| hasCommunityLibrarySubmission.value = false; | ||
| loading.value = false; | ||
| } | ||
| }); |
There was a problem hiding this comment.
I think that to prevent having to reset this data when the prop.value changes, a more common pattern we have across our products is to use the v-if directive rather than the v-model to mount and unmount these components. And expose a @close event to set the rendering variable to false.
So, if we call this component like this instead:
<RemoveChannelModal
v-if="deleteDialog"
:channel-id="channelId"
:can-edit="canEdit"
data-test="delete-modal"
@delete="handleDelete"
@close="deleteDialog = false"
/>Then we may only need to rely on the onMounted hook to fetch the data, and we wouldn't need to care about resetting the ref values, as they will always be defined when the modal is opened.
| const detailUrl = window.Urls.channel_detail(props.channelId); | ||
| const url = `${detailUrl.replace(/\/$/, '')}/has_community_library_submission`; |
There was a problem hiding this comment.
Here, we don't really need to invoke the channel_detail first and then concatenate the has_community_library_submission endpoint. What we can do instead is to abstract this call inside the Channel Resource and use the getUrlFunction instead, and pass the URL name that we defined in the viewset. Here is an example of how we do this for the language_exists action https://github.com/taoerman/studio/blob/ce0416e0eee2b01548811d40b514eb41d2905779/contentcuration/contentcuration/frontend/shared/data/resources.js#L1397
| const $tr = messageId => { | ||
| return proxy.$tr(messageId); | ||
| }; |
There was a problem hiding this comment.
Here we don't need to define this $tr here! We already have a $tr function globally available in the template 👐
| > | ||
| {{ $tr('deleteChannelWithCLWarning') }} | ||
| </div> | ||
| {{ canEdit ? $tr('deletePrompt') : $tr('removePrompt') }} |
There was a problem hiding this comment.
So, if canEdit is false, we are not deleting the channel itself, just removing the user from the list of readers. In that case, I think it wouldn't make much sense to show the deleteChannelWithCLWarning warning either, because the channel won't be removed.
| }; | ||
|
|
||
| if (params.has_community_library_submission === undefined) { | ||
| extendedParams.deleted = Boolean(params.deleted) && params.deleted.toString() === 'true'; |
2 participants
Summary
Shows a warning in delete confirmation modals when deleting a channel that has Community Library submissions, explaining that deletion in Studio does not remove it from the Community Library.
The backend exposes a has_community_library_submission boolean field for the frontend to detect these channels.
When admins filter by "Community Library" in the admin channels list, soft-deleted channels with Community Library submissions are now included, allowing admins to audit and manage them.
Includes backend tests for both features.
References
Fixed #5459
Reviewer guidance
Run unit tests Or test manually!