feat: soft delete feature for news and articles content - #3681
feat: soft delete feature for news and articles content#3681ahtesham-quraish wants to merge 4 commits into
Conversation
OpenAPI ChangesNo changes detected Unexpected changes? Ensure your branch is up-to-date with |
There was a problem hiding this comment.
Pull request overview
Adds a soft-delete mechanism for draft website content (news + articles) and exposes “Delete” actions in the editor and draft listing UI, allowing editors/admins to remove drafts without Django admin access while keeping rows for audit/recovery.
Changes:
- Backend: introduce
deleted_onsoft-delete field and exclude soft-deleted items from all WebsiteContent API querysets. - Backend: change DELETE behavior to soft-delete drafts and reject deletion of published content; add/adjust API tests accordingly.
- Frontend: add a reusable delete-confirmation dialog and wire delete actions into the editor toolbar and draft listing, with new/updated UI tests.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| website_content/views.py | Filters out soft-deleted content; soft-deletes drafts on destroy and rejects published deletions. |
| website_content/views_test.py | Updates destroy tests for soft-delete + visibility + permission cases. |
| website_content/models.py | Adds deleted_on soft-delete timestamp field to WebsiteContent. |
| website_content/migrations/0006_add_deleted_on_to_websitecontent.py | Migration adding the deleted_on column. |
| frontends/main/src/page-components/WebsiteContentDialogs/DeleteWebsiteContentDialog.tsx | New confirmation dialog that calls the destroy API hook. |
| frontends/main/src/page-components/TiptapEditor/core/WebsiteContentEditor.tsx | Adds “Delete” action to the draft editor toolbar and redirects after delete. |
| frontends/main/src/page-components/TiptapEditor/contentTypes/news/NewsEditor.happydom.test.tsx | Adds UI tests for deleting a draft and ensuring no delete button for published. |
| frontends/main/src/app-pages/WebsiteContent/WebsiteContentDraftListingPage.tsx | Adds delete action for each draft card in the drafts listing. |
| frontends/main/src/app-pages/WebsiteContent/WebsiteContentDraftListingPage.test.tsx | Adds UI test coverage for draft deletion flow from the listing page. |
| <Card.Actions> | ||
| <StyledDeleteButton | ||
| variant="text" | ||
| size="small" | ||
| aria-label={`Delete ${contentItem.title}`} | ||
| startIcon={<RiDeleteBinLine />} | ||
| onClick={() => showDeleteWebsiteContentDialog(contentItem)} | ||
| > | ||
| Delete | ||
| </StyledDeleteButton> | ||
| </Card.Actions> |
| const modal = NiceModal.useModal() | ||
| const hideModal = modal.hide | ||
| const destroy = useWebsiteContentDestroy() | ||
| const label = CONTENT_TYPE_LABELS[contentItem.content_type ?? ""] ?? "Item" | ||
|
|
||
| const handleConfirm = useCallback(async () => { | ||
| await destroy.mutateAsync(contentItem.id) | ||
| hideModal() | ||
| onDestroy?.() | ||
| }, [destroy, hideModal, contentItem, onDestroy]) |
ChristopherChudzicki
left a comment
There was a problem hiding this comment.
This is working well, and for the model change a DateTime deleted_on field is definitely better than a boolean — django-safedelete uses the same shape (it calls the field deleted), so the change I'm suggesting below doesn't lose anything.
I do have some requested changes. The first is a fairly big change, but it simplifies the PR, and I think it's worth it.
I apologize for not suggesting django-safedelete when you first wrote the issue. (TBH, I didn't realize that's how we do it in Studio until now.)
Let's use django-safedelete
Safe delete is well-trodden ground and Django has some standard libraries for this. The one we use in OCW Studio is django-safedelete, on a model with the same name as ours — see websites/models.py and websites/admin.py.
Using django-safedelete buys us fewer decisions of our own and a pattern to follow. That's a big win. But also:
SafeDeleteModelthat makesdelete()method soft by default (but optionally a hard delete)SafeDeleteQuerySetso our views etc naturally filter out deleted stuff.SafeDeleteAdminthat shows the relevant stuff in Django Admin. Normally this would take a bit of extra effort with the custom queryset.- Cascade effects we'd need to handle ourselves. (May not need these yet.)
(1) + (2) removes essentially all the custom soft delete logic that is in the viewsets. The drafts-only rule stays in the view, either as perform_destroy logic or as a CanEditWebsiteContent object-level permission check.
Minor Frontend Issues
CONTENT_TYPE_LABELSis now duplicated two places.-
Let's define it once, in
@/common/website_content. -
Don't use
Record<string, string>. If we ever add a third content type, we want to update this mapping. And withRecord<string,string>, nothing enforces that. Key it on the generated enum instead:1import { WebsiteContentContentTypeEnum } from "api/v1" export const CONTENT_TYPE_LABELS: Record<WebsiteContentContentTypeEnum, string> = { [WebsiteContentContentTypeEnum.News]: "News", [WebsiteContentContentTypeEnum.Article]: "Article", }
-
- Opinion / Suggestion: On the draft listing view, the card's delete button should be an icon-only
ActionButton.- That's how we do card actions everywhere else
- It looks kind of buggy on hover as-is... the background highlight touches bottom of card.
- Since you added the confirmation dialog (Nice!) that's explicit enough. I don't think we need the "Delete" word also. Plus, trash can is very standard delete symbolism
WebsiteContentDraftListingPage.test.tsxmocksnext/navigationexports unnecessarily. The test file passes if you remove these lines, since nothing triggers or asserts on router methods.- Delete Button Placement: You could put
<Spacer />between "Delete" and "Save As" to reduce misclicks. Misclicks aren't a big deal because of the dialog anyway, but Claude suggested this and I thought it was a reasonable addition.
On Soft-deleting Published Content via Django Admin
@pdpinch commented on the issue:
Can I suggest that ... and that we can use the django admin for (soft) deleting published articles when that becomes an issue?
With django-safedelete, the admin panel will allow soft-deleting. If we use that, we should make sure the FeedItem gets deleted, too. Your original acceptance criteria included that:
Deleting a CMS article/news WebsiteContent also soft-deletes (or suppresses) its derived FeedItem on /news.
I think that's probably pretty simple to add, so we might as well. Two notes:
- It can be a hard delete, no need for soft. The ETL can always reconstruct the FeedItem, and ETL hard-deletes non-existant items anyway at its next scheduled run.
- Should purge the listing page, too... Probably
purge_content_on_saveis what we want here.
Footnotes
-
There's a generated
WebsiteContentContentTypeEnumDescriptionsholding these exact strings already, but its values come from theWebsiteContentTypelabels inwebsite_content/constants.py, so a backend rename would quietly change UI copy. Let's take the type from the generated code and keep the strings on the frontend. Note this isn't a pure find-and-replace:WebsiteContentDraftListingPage'scontentTypeprop is a plainstring, soCONTENT_TYPE_LABELS[type]won't typecheck until that's narrowed toWebsiteContentContentTypeEnumas well. ↩
What are the relevant tickets?
https://github.com/mitodl/hq/issues/12513
Description (What does it do?)
As an article editor / super admin, I want to delete articles and news items directly from the UI, so I can remove outdated, incorrect, or unwanted content without needing Django admin access.
Screenshots (if appropriate):
How can this be tested?
For testing you need to go to following url
/news/draft, '/article/draft' andwebsite_content/news/<draft_id>/editand see the delete button is functionalAdditional Context