Enhance account deletion handling and date filtering across models

- Added handling for ProtectedError when deleting accounts with linked invoices, estimates, or recurring invoices.
- Updated account deletion response message to include all related billing records.
- Modified date filtering in various views to ensure inclusivity of the end day by using `created_at__date`.
- Introduced tests for date filtering edge cases and account deletion scenarios with billing records.
- Updated package version to 1.4.0 in uv.lock.
This commit is contained in:
Ashwin 2026-08-16 04:50:18 +05:30
parent d9082aed75
commit ff02a12d2b
16 changed files with 336 additions and 35 deletions

View file

@ -8,6 +8,7 @@ from django.core.files.uploadedfile import SimpleUploadedFile
from accounts.models import Account, AccountEmail, AccountEmailLog
from common.models import Attachments, Comment, Tags, Teams
from contacts.models import Contact
from invoices.models import Estimate, Invoice, RecurringInvoice
@pytest.mark.django_db
@ -223,6 +224,38 @@ class TestAccountDetailView:
assert response.status_code == 200
assert not Account.objects.filter(id=account.id).exists()
@pytest.mark.parametrize(
("model", "extra"),
[
(Invoice, {"invoice_number": "INV-1"}),
(Estimate, {"estimate_number": "EST-1"}),
(RecurringInvoice, {"title": "Monthly retainer"}),
],
ids=["invoice", "estimate", "recurring-invoice"],
)
def test_delete_account_with_billing_records_is_refused(
self, admin_client, org_a, model, extra
):
"""Each PROTECT relation answers 409, and the message covers all three.
Unguarded, the ProtectedError left the view as a 500. The message is
asserted on because it is the only thing telling an admin which list to
go and empty, and it named two of the three relations for a while.
"""
account = Account.objects.create(name="Has Billing", org=org_a)
model.objects.create(org=org_a, account=account, **extra)
response = admin_client.delete(f"/api/accounts/{account.id}/")
assert response.status_code == 409
body = response.json()
assert body["error"] is True
assert body["errors"] == (
"This account can't be deleted while it still has invoices, "
"estimates or recurring invoices linked to it."
)
assert Account.objects.filter(id=account.id).exists()
def test_get_account_cross_org(self, org_b_client, org_a):
account = Account.objects.create(name="Org A Account", org=org_a)
response = org_b_client.get(f"/api/accounts/{account.id}/")

View file

@ -571,11 +571,15 @@ class AccountDetailView(APIView):
try:
self.object.delete()
except ProtectedError:
# All three of Invoice, Estimate and RecurringInvoice point here with
# on_delete=PROTECT, so all three must be named. An account blocked
# only by a recurring invoice used to be told to go and look at two
# lists that were already empty.
return Response(
{
"error": True,
"errors": "This account can't be deleted while it still has "
"invoices or estimates linked to it.",
"invoices, estimates or recurring invoices linked to it.",
},
status=status.HTTP_409_CONFLICT,
)

View file

@ -132,10 +132,10 @@ class CaseKanbanView(APIView):
queryset = queryset.filter(tags__id=tags)
created_at_gte = date_param(params, "created_at__gte")
if created_at_gte:
queryset = queryset.filter(created_at__gte=created_at_gte)
queryset = queryset.filter(created_at__date__gte=created_at_gte)
created_at_lte = date_param(params, "created_at__lte")
if created_at_lte:
queryset = queryset.filter(created_at__lte=created_at_lte)
queryset = queryset.filter(created_at__date__lte=created_at_lte)
return queryset.distinct()
def _get_status_kanban(self, queryset):

View file

@ -128,10 +128,10 @@ def apply_case_list_filters(queryset, params):
)
created_at_gte = date_param(params, "created_at__gte")
if created_at_gte:
queryset = queryset.filter(created_at__gte=created_at_gte)
queryset = queryset.filter(created_at__date__gte=created_at_gte)
created_at_lte = date_param(params, "created_at__lte")
if created_at_lte:
queryset = queryset.filter(created_at__lte=created_at_lte)
queryset = queryset.filter(created_at__date__lte=created_at_lte)
if params.get("sla_breached") == "true":
# Wall-clock approximation matching the mobile card's
# `isFirstResponseSlaBreached` getter; `Case.is_sla_*_breached` uses

View file

@ -0,0 +1,125 @@
"""A `created_at` range includes the day at each end of it.
`created_at` is a DateTimeField and `?created_at__lte=` carries a date, so
comparing the two directly put the upper bound at midnight and dropped
everything recorded during the end day itself. A user filtering "up to today"
saw none of today. Filtering on the date part instead makes the range inclusive
by calendar day, and resolves it in the org's timezone rather than the server's,
since `__date` answers in whichever timezone `GetProfileAndOrg` activated.
Every endpoint accepting the pair is covered here rather than in each app's own
suite, because the defect was one shape repeated eight times. Seven of the eight
were still live after the first was fixed in isolation, which is the case for
pinning them together: a new list view that copies the old two lines fails here
immediately.
Each assertion is paired with its mirror, so a filter that quietly stopped being
applied at all fails the pair rather than passing the half that only asks
whether the record is present.
"""
import datetime
import pytest
from accounts.models import Account
from cases.models import Case
from contacts.models import Contact
from leads.models import Lead
from opportunity.models import Opportunity
from tasks.models import Task
# Midday, so the record sits inside its day rather than on either edge. The org
# fixtures keep the default UTC timezone, so this is the 10th for the filter too.
CREATED_AT = datetime.datetime(2026, 3, 10, 12, 0, tzinfo=datetime.timezone.utc)
THE_DAY = "2026-03-10"
DAY_BEFORE = "2026-03-09"
DAY_AFTER = "2026-03-11"
def _account(org, user):
return Account.objects.create(name="Filter Me", org=org, created_by=user)
def _lead(org, user):
# `status` is nullable on the model, and the kanban builds one column per
# LEAD_STATUS value, so a lead without one is counted by the board and shown
# in none of its columns. Set it, or the kanban case would assert nothing.
return Lead.objects.create(
first_name="Filter", last_name="Me", status="assigned", org=org, created_by=user
)
def _contact(org, user):
return Contact.objects.create(
first_name="Filter", last_name="Me", org=org, created_by=user
)
def _case(org, user):
return Case.objects.create(
name="Filter Me", status="New", priority="Normal", org=org, created_by=user
)
def _opportunity(org, user):
return Opportunity.objects.create(
name="Filter Me", stage="QUALIFICATION", org=org, created_by=user
)
def _task(org, user):
return Task.objects.create(
title="Filter Me", status="New", priority="Medium", org=org, created_by=user
)
ENDPOINTS = [
("/api/accounts/", _account),
("/api/leads/", _lead),
("/api/leads/kanban/", _lead),
("/api/contacts/", _contact),
("/api/cases/", _case),
("/api/cases/kanban/", _case),
("/api/opportunities/", _opportunity),
("/api/tasks/", _task),
]
IDS = [url for url, _factory in ENDPOINTS]
def _record(factory, org, user):
"""One record whose `created_at` is a known instant.
`created_at` is `auto_now_add`, so it can only be moved by an UPDATE that
goes around the field's default.
"""
obj = factory(org, user)
type(obj).objects.filter(pk=obj.pk).update(created_at=CREATED_AT)
return obj
def _shows(response, obj):
"""Whether the record came back.
Matched on the raw body rather than a results key: the list and kanban
endpoints answer with different shapes, and the question here is only
whether the filter kept the row.
"""
assert response.status_code == 200
return str(obj.id) in response.content.decode()
@pytest.mark.django_db
@pytest.mark.parametrize(("url", "factory"), ENDPOINTS, ids=IDS)
def test_lte_includes_the_end_day(admin_client, org_a, admin_user, url, factory):
obj = _record(factory, org_a, admin_user)
assert _shows(admin_client.get(f"{url}?created_at__lte={THE_DAY}"), obj)
assert not _shows(admin_client.get(f"{url}?created_at__lte={DAY_BEFORE}"), obj)
@pytest.mark.django_db
@pytest.mark.parametrize(("url", "factory"), ENDPOINTS, ids=IDS)
def test_gte_includes_the_start_day(admin_client, org_a, admin_user, url, factory):
obj = _record(factory, org_a, admin_user)
assert _shows(admin_client.get(f"{url}?created_at__gte={THE_DAY}"), obj)
assert not _shows(admin_client.get(f"{url}?created_at__gte={DAY_AFTER}"), obj)

View file

@ -0,0 +1,24 @@
"""The liveness probe answers without a tenant.
`RequireOrgContext` 403s any resolvable path with no org context, and
`/healthz/` has no org to carry: it is fetched by load balancers, uptime
monitors and container orchestrators, none of which hold a JWT. While it was
gated, every health check read the service as down.
Paired with a request that must still be refused, so the exemption is pinned as
narrow rather than as "the middleware stopped enforcing".
"""
import pytest
@pytest.mark.django_db
class TestHealthzNoOrgContext:
def test_probe_is_reachable_unauthenticated(self, unauthenticated_client):
response = unauthenticated_client.get("/healthz/")
assert response.status_code == 200
def test_other_paths_still_require_org_context(self, unauthenticated_client):
# Same client, same absence of a token. Only /healthz/ is exempt.
response = unauthenticated_client.get("/api/accounts/")
assert response.status_code == 403

View file

@ -88,6 +88,35 @@ class TestAMalformedRangeFilterIsRefusedNotCrashed:
assert resp.status_code == http.HTTP_200_OK
@pytest.mark.parametrize("url,param", DATE_PARAMS)
def test_a_day_that_does_not_exist_is_a_400(self, admin_client, org_a, url, param):
"""A date-shaped value for a day that never happened.
`parse_date` raises `ValueError` on it rather than answering None, so it
went straight past the check that was only looking for None and took the
endpoint down the same way "banana" used to.
"""
resp = admin_client.get(url, {param: "2026-02-30"})
assert resp.status_code == http.HTTP_400_BAD_REQUEST
assert param in resp.data
@pytest.mark.parametrize("url,param", DATE_PARAMS)
def test_a_full_timestamp_still_filters(self, admin_client, org_a, url, param):
"""A date-valued filter also has to take a timestamp.
`mobile/lib/providers/tickets_provider.dart` sends
`createdAfter.toUtc().toIso8601String()`, so this is the live shape of
the ticket list's date filter, not a hypothetical one. Neither lookup
will parse it out of the query string: a DateField raises on anything
that is not YYYY-MM-DD, and so does the `__date` lookup the datetime
columns are filtered through. `date_param` resolves the calendar day
before either sees it.
"""
resp = admin_client.get(url, {param: "2026-08-01T12:00:00.000Z"})
assert resp.status_code == http.HTTP_200_OK
@pytest.mark.parametrize("url,param", NUMBER_PARAMS)
def test_a_word_where_a_number_belongs_is_a_400(
self, admin_client, org_a, url, param

View file

@ -4,9 +4,14 @@ Tests for user management views: list users, user detail, update user, status.
Run with: pytest common/tests/test_users.py -v
"""
from unittest.mock import patch
import pytest
from django.utils import timezone
from rest_framework import status
from cases.approvals import Approval, ApprovalRule
from cases.models import Case, TimeEntry
from common.models import Address, Profile
from common.permissions import is_org_admin
@ -431,6 +436,53 @@ class TestUserDetailView:
response = user_client.delete(self._url(admin_user.id))
assert response.status_code == status.HTTP_403_FORBIDDEN
@patch("common.views.user_views.send_email_user_delete")
def test_delete_user_tells_them_they_were_removed(
self, send_email, admin_client, org_a, regular_user, user_profile
):
"""The notice goes out, and only once the removal has actually happened."""
response = admin_client.delete(self._url(regular_user.id))
assert response.status_code == status.HTTP_200_OK
send_email.delay.assert_called_once_with(
regular_user.email, deleted_by="admin@test.com"
)
@pytest.mark.parametrize("relation", ["time-entry", "approval"])
@patch("common.views.user_views.send_email_user_delete")
def test_delete_user_with_protected_history_is_refused(
self, send_email, admin_client, org_a, regular_user, user_profile, relation
):
"""A member who logged time or raised an approval cannot be deleted.
Both relations are on_delete=PROTECT, so the delete raises and the view
used to answer 500. The email assertion is the other half of it: the
notice was sent before the delete was attempted, so a user who was still
in the org was told they had been removed from it.
"""
case = Case.objects.create(
name="Blocking Case", status="New", priority="Normal", org=org_a
)
if relation == "time-entry":
TimeEntry.objects.create(
org=org_a, case=case, profile=user_profile, started_at=timezone.now()
)
else:
rule = ApprovalRule.objects.create(name="Discount sign-off", org=org_a)
Approval.objects.create(
org=org_a, case=case, rule=rule, requested_by=user_profile
)
response = admin_client.delete(self._url(regular_user.id))
assert response.status_code == status.HTTP_409_CONFLICT
assert response.data["errors"] == (
"This user can't be deleted while they still have time entries or "
"approvals linked to them."
)
assert Profile.objects.filter(id=user_profile.id).exists()
send_email.delay.assert_not_called()
@pytest.mark.django_db
class TestUserStatusView:

View file

@ -2,6 +2,7 @@
Common validators for CRM models.
"""
import datetime
import json
import re
import uuid as uuid_module
@ -10,6 +11,7 @@ from zoneinfo import available_timezones
from django.core.exceptions import ValidationError as DjangoValidationError
from django.core.validators import RegexValidator
from django.utils import timezone
from django.utils.dateparse import parse_date, parse_datetime
from django.utils.translation import gettext_lazy as _
from rest_framework.exceptions import ValidationError as DRFValidationError
@ -129,30 +131,49 @@ def uuid_list_param(params, field: str) -> list:
return validate_uuid_list(params.get(field), field)
def validate_date(value, field: str) -> str:
"""Return ``value`` unchanged if the ORM will be able to parse it as a date.
def validate_date(value, field: str) -> datetime.date:
"""The calendar day a date-valued query parameter names.
The same defect ``validate_uuid`` closes, wearing a different type. Range
filters (``due_date__gte``, ``created_at__lte``, ``close_date__gte``, ...)
take raw query-string text and hand it to a date or datetime lookup.
Django parses it while building the query and raises
take raw query-string text and hand it to a date lookup. Django parses it
while building the query and raises
``django.core.exceptions.ValidationError`` on anything malformed, which
DRF's handler does not translate, so ``?due_date__gte=banana`` answered 500
rather than 400.
rather than 400. ``2026-02-30`` is the same 500 by a different route:
``parse_date`` raises ``ValueError`` on a day that does not exist rather
than answering ``None``, so a check looking only for ``None`` walks past it.
Accepts a plain date and a full timestamp alike, because the datetime
columns here (``created_at``) are filtered with both. The value is returned
as text rather than as a parsed object so the lookup keeps behaving exactly
as it did: passing a ``date`` into a ``DateTimeField`` lookup changes how
Django interprets midnight, and this is a 500-to-400 fix, not a semantics
change.
A full timestamp is accepted as well as a plain date, because callers send
both: ``mobile/lib/providers/tickets_provider.dart`` sends
``toUtc().toIso8601String()``. Neither column type will take that text.
A DateField parses only ``YYYY-MM-DD``, and the datetime columns are
filtered through ``__date`` so their range covers the end day, which parses
only ``YYYY-MM-DD`` too. Resolving the day here rather than returning the
text is what lets one parameter serve both.
An aware timestamp resolves in the active timezone, the org's, which is the
timezone ``__date`` resolves the rows against.
"""
text = str(value).strip()
if parse_date(text) is None and parse_datetime(text) is None:
try:
day = parse_date(text)
if day is None:
moment = parse_datetime(text)
if moment is not None:
day = (
timezone.localtime(moment).date()
if timezone.is_aware(moment)
else moment.date()
)
except ValueError:
# Well formed, no such moment: "2026-02-30", "2026-02-30T10:00:00".
day = None
if day is None:
raise DRFValidationError(
{field: [f"'{value}' is not a valid date. Use YYYY-MM-DD."]}
) from None
return text
return day
def date_param(params, field: str):

View file

@ -1,6 +1,6 @@
from django.conf import settings
from django.db import IntegrityError, transaction
from django.db.models import Count, Q
from django.db.models import Count, ProtectedError, Q
from django.shortcuts import get_object_or_404
from django.utils import timezone
from drf_spectacular.utils import extend_schema, inline_serializer
@ -520,11 +520,24 @@ class UserDetailView(APIView):
status=status.HTTP_403_FORBIDDEN,
)
deleted_by = self.request.profile.user.email
send_email_user_delete.delay(
self.object.user.email,
deleted_by=deleted_by,
)
self.object.delete()
recipient = self.object.user.email
try:
self.object.delete()
except ProtectedError:
# TimeEntry.profile, Approval.requested_by and Approval.approver all
# point here with on_delete=PROTECT, so a member who has logged time
# or touched an approval cannot be removed. Unguarded this was a 500,
# and the notice below had already gone out by then: the user was
# told they had been removed from an org they were still in.
return Response(
{
"error": True,
"errors": "This user can't be deleted while they still have "
"time entries or approvals linked to them.",
},
status=status.HTTP_409_CONFLICT,
)
send_email_user_delete.delay(recipient, deleted_by=deleted_by)
return Response({"status": "success"}, status=status.HTTP_200_OK)

View file

@ -101,10 +101,10 @@ class ContactsListView(APIView, LimitOffsetPagination):
)
created_at_gte = date_param(params, "created_at__gte")
if created_at_gte:
queryset = queryset.filter(created_at__gte=created_at_gte)
queryset = queryset.filter(created_at__date__gte=created_at_gte)
created_at_lte = date_param(params, "created_at__lte")
if created_at_lte:
queryset = queryset.filter(created_at__lte=created_at_lte)
queryset = queryset.filter(created_at__date__lte=created_at_lte)
# Custom-field filters: ?cf_<key>=<value> -> custom_fields contains pair.
for raw_key, raw_value in params.items():
if raw_key.startswith("cf_") and raw_value:

View file

@ -116,10 +116,10 @@ class LeadKanbanView(APIView):
queryset = queryset.filter(source=params.get("source"))
created_at_gte = date_param(params, "created_at__gte")
if created_at_gte:
queryset = queryset.filter(created_at__gte=created_at_gte)
queryset = queryset.filter(created_at__date__gte=created_at_gte)
created_at_lte = date_param(params, "created_at__lte")
if created_at_lte:
queryset = queryset.filter(created_at__lte=created_at_lte)
queryset = queryset.filter(created_at__date__lte=created_at_lte)
return queryset
def _get_status_kanban(self, queryset):

View file

@ -159,10 +159,10 @@ class LeadListView(APIView, LimitOffsetPagination):
)
created_at_gte = date_param(params, "created_at__gte")
if created_at_gte:
queryset = queryset.filter(created_at__gte=created_at_gte)
queryset = queryset.filter(created_at__date__gte=created_at_gte)
created_at_lte = date_param(params, "created_at__lte")
if created_at_lte:
queryset = queryset.filter(created_at__lte=created_at_lte)
queryset = queryset.filter(created_at__date__lte=created_at_lte)
close_date_gte = date_param(params, "close_date__gte")
if close_date_gte:
queryset = queryset.filter(close_date__gte=close_date_gte)

View file

@ -172,10 +172,10 @@ class OpportunityListView(APIView, LimitOffsetPagination):
queryset = queryset.filter(name__icontains=params.get("search"))
created_at_gte = date_param(params, "created_at__gte")
if created_at_gte:
queryset = queryset.filter(created_at__gte=created_at_gte)
queryset = queryset.filter(created_at__date__gte=created_at_gte)
created_at_lte = date_param(params, "created_at__lte")
if created_at_lte:
queryset = queryset.filter(created_at__lte=created_at_lte)
queryset = queryset.filter(created_at__date__lte=created_at_lte)
closed_on_gte = date_param(params, "closed_on__gte")
if closed_on_gte:
queryset = queryset.filter(closed_on__gte=closed_on_gte)

View file

@ -121,10 +121,10 @@ class TaskListView(APIView, LimitOffsetPagination):
queryset = queryset.filter(due_date__lte=due_date_lte)
created_at_gte = date_param(params, "created_at__gte")
if created_at_gte:
queryset = queryset.filter(created_at__gte=created_at_gte)
queryset = queryset.filter(created_at__date__gte=created_at_gte)
created_at_lte = date_param(params, "created_at__lte")
if created_at_lte:
queryset = queryset.filter(created_at__lte=created_at_lte)
queryset = queryset.filter(created_at__date__lte=created_at_lte)
for related in ("account", "opportunity", "case", "lead"):
related_id = uuid_param(params, related)
if related_id:

View file

@ -583,7 +583,7 @@ wheels = [
[[package]]
name = "django-crm"
version = "1.3.1"
version = "1.4.0"
source = { editable = "." }
dependencies = [
{ name = "cairocffi" },