From e216c58448cab065150b9b700071ee6dd8dae045 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 30 Oct 2025 14:11:38 -0500 Subject: [PATCH 1/2] Delete project and organization objects asynchronously Closes https://github.com/readthedocs/readthedocs.org/issues/10040 --- readthedocs/core/history.py | 12 +++++-- readthedocs/core/mixins.py | 18 ++++++++++ readthedocs/core/tasks.py | 39 ++++++++++++++++++++++ readthedocs/organizations/views/private.py | 5 +-- readthedocs/projects/views/private.py | 5 +-- 5 files changed, 73 insertions(+), 6 deletions(-) diff --git a/readthedocs/core/history.py b/readthedocs/core/history.py index db6789a8298..eceeaec74c8 100644 --- a/readthedocs/core/history.py +++ b/readthedocs/core/history.py @@ -12,7 +12,7 @@ log = structlog.get_logger(__name__) -def set_change_reason(instance, reason): +def set_change_reason(instance, reason, user=None): """ Set the change reason for the historical record created from the instance. @@ -20,9 +20,17 @@ def set_change_reason(instance, reason): It sets `reason` to the `_change_reason` attribute of the instance, that's used to create the historical record on the save/delete signals. - https://django-simple-history.readthedocs.io/en/latest/historical_model.html#change-reason # noqa + `user` is useful to track who made the change, this is only needed + if this method is called outside of a request context, + as the middleware already sets the user from the request. + + See: + - https://django-simple-history.readthedocs.io/en/latest/historical_model.html#change-reason + - https://django-simple-history.readthedocs.io/en/latest/user_tracking.html """ instance._change_reason = reason + if user: + instance._history_user = user def safe_update_change_reason(instance, reason): diff --git a/readthedocs/core/mixins.py b/readthedocs/core/mixins.py index 6f751b699f4..600b732f85c 100644 --- a/readthedocs/core/mixins.py +++ b/readthedocs/core/mixins.py @@ -2,9 +2,11 @@ from django.contrib import messages from django.contrib.auth.mixins import LoginRequiredMixin +from django.http import HttpResponseRedirect from vanilla import DeleteView from vanilla import ListView +from readthedocs.core.tasks import delete_object from readthedocs.proxito.cache import cache_response from readthedocs.proxito.cache import private_response @@ -82,3 +84,19 @@ def post(self, request, *args, **kwargs): if resp.status_code == 302 and self.success_message: messages.success(self.request, self.success_message) return resp + + +class AsyncDeleteViewWithMessage(DeleteView): + """Delete view that shows a message after queuing an object for deletion.""" + + success_message = None + + def post(self, request, *args, **kwargs): + self.object = self.get_object() + delete_object.delay( + model_name=self.object._meta.label, + pk=self.object.pk, + user_id=request.user.pk, + ) + messages.success(request, self.success_message) + return HttpResponseRedirect(self.get_success_url()) diff --git a/readthedocs/core/tasks.py b/readthedocs/core/tasks.py index ae7c3167c27..8949dc72237 100644 --- a/readthedocs/core/tasks.py +++ b/readthedocs/core/tasks.py @@ -4,9 +4,13 @@ import redis import structlog +from django.apps import apps from django.conf import settings +from django.contrib.auth.models import User from django.core.mail import EmailMultiAlternatives +from readthedocs.builds.utils import memcache_lock +from readthedocs.core.history import set_change_reason from readthedocs.worker import app @@ -71,3 +75,38 @@ def cleanup_pidbox_keys(): client.delete(key) log.info("Redis pidbox objects.", memory=total_memory, keys=len(keys)) + + +@app.task(queue="web", bind=True) +def delete_object(self, model_name: str, pk: int, user_id: int | None = None): + """ + Delete an object from the database asynchronously. + + This is useful for deleting large objects that may take time + to delete, without timing out the request. + + :param model_name: The model name in the format 'app_label.ModelName'. + :param object_id: The primary key of the object to delete. + :param user_id: The ID of the user performing the deletion. + Just for logging purposes. + """ + task_log = log.bind(model_name=model_name, object_pk=pk, user_id=user_id) + lock_id = f"{self.name}-{model_name}-{pk}-lock" + lock_expire = 60 * 60 * 2 # 2 hours + with memcache_lock( + lock_id=lock_id, lock_expire=lock_expire, app_identifier=self.app.oid + ) as acquired: + if not acquired: + task_log.info("Object is already being deleted.") + return + + user = User.objects.filter(pk=user_id).first() if user_id else None + Model = apps.get_model(model_name) + obj = Model.objects.filter(pk=pk).first() + if obj: + task_log.info("Deleting object.") + set_change_reason(obj, reason="Object deleted asynchronously", user=user) + obj.delete() + task_log.info("Object deleted.") + else: + task_log.info("Object does not exist.") diff --git a/readthedocs/organizations/views/private.py b/readthedocs/organizations/views/private.py index daf1255bce7..be6eb493009 100644 --- a/readthedocs/organizations/views/private.py +++ b/readthedocs/organizations/views/private.py @@ -19,6 +19,7 @@ from readthedocs.audit.models import AuditLog from readthedocs.core.filters import FilterContextMixin from readthedocs.core.history import UpdateChangeReasonPostView +from readthedocs.core.mixins import AsyncDeleteViewWithMessage from readthedocs.core.mixins import DeleteViewWithMessage from readthedocs.core.mixins import PrivateViewMixin from readthedocs.invitations.models import Invitation @@ -119,10 +120,10 @@ class DeleteOrganization( PrivateViewMixin, UpdateChangeReasonPostView, OrganizationView, - DeleteViewWithMessage, + AsyncDeleteViewWithMessage, ): http_method_names = ["post"] - success_message = _("Organization deleted") + success_message = _("Organization queued for deletion") def get_success_url(self): return reverse_lazy("organization_list") diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index 6400910f527..dea94bacfcd 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -37,6 +37,7 @@ from readthedocs.builds.models import VersionAutomationRule from readthedocs.core.filters import FilterContextMixin from readthedocs.core.history import UpdateChangeReasonPostView +from readthedocs.core.mixins import AsyncDeleteViewWithMessage from readthedocs.core.mixins import DeleteViewWithMessage from readthedocs.core.mixins import ListViewWithForm from readthedocs.core.mixins import PrivateViewMixin @@ -191,8 +192,8 @@ def get_form(self, data=None, files=None, **kwargs): return super().get_form(data, files, **kwargs) -class ProjectDelete(UpdateChangeReasonPostView, ProjectMixin, DeleteViewWithMessage): - success_message = _("Project deleted") +class ProjectDelete(UpdateChangeReasonPostView, ProjectMixin, AsyncDeleteViewWithMessage): + success_message = _("Project queued for deletion") template_name = "projects/project_delete.html" def get_context_data(self, **kwargs): From 8a4b8a703f5c2658ea1f2ee917dda4a6a709f30e Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 30 Oct 2025 18:26:25 -0500 Subject: [PATCH 2/2] Update readthedocs/core/tasks.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- readthedocs/core/tasks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/core/tasks.py b/readthedocs/core/tasks.py index 8949dc72237..2ab5b23531f 100644 --- a/readthedocs/core/tasks.py +++ b/readthedocs/core/tasks.py @@ -86,7 +86,7 @@ def delete_object(self, model_name: str, pk: int, user_id: int | None = None): to delete, without timing out the request. :param model_name: The model name in the format 'app_label.ModelName'. - :param object_id: The primary key of the object to delete. + :param pk: The primary key of the object to delete. :param user_id: The ID of the user performing the deletion. Just for logging purposes. """