vignesh/bottleneckcrm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Refactor URL patterns to use <uid:pk> for unique identifiers across leads, macros, opportunities, and tasks

Browse source 
- Updated URL patterns in leads, macros, opportunity, and tasks to replace <str:pk> with <uid:pk> for better clarity and consistency.
- Adjusted related views to ensure compatibility with the new URL structure.
- Enhanced frontend templates and server-side logic to accommodate changes in URL patterns.
- Improved documentation in tags and templates to clarify usage and functionality.
This commit is contained in:
Ashwin 2026-08-05 08:25:02 +05:30
parent 0e6d880187
commit 446ac0bdab
31 changed files with 1625 additions and 205 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
backend/accounts/urls.py
View file

@ -6,8 +6,8 @@ app_name = "api_accounts"
urlpatterns = [
path("", views.AccountsListView.as_view()),
path("<str:pk>/", views.AccountDetailView.as_view()),
path("<str:pk>/create_mail/", views.AccountCreateMailView.as_view()),
path("comment/<str:pk>/", views.AccountCommentView.as_view()),
path("attachment/<str:pk>/", views.AccountAttachmentView.as_view()),
path("<uid:pk>/", views.AccountDetailView.as_view()),
path("<uid:pk>/create_mail/", views.AccountCreateMailView.as_view()),
path("comment/<uid:pk>/", views.AccountCommentView.as_view()),
path("attachment/<uid:pk>/", views.AccountAttachmentView.as_view()),
]

6
backend/business_hours/urls.py
View file

@ -7,17 +7,17 @@ app_name = "api_business_hours"
urlpatterns = [
path("calendar/", views.BusinessCalendarView.as_view(), name="calendar_default"),
path(
"calendar/<str:pk>/",
"calendar/<uid:pk>/",
views.BusinessCalendarView.as_view(),
name="calendar_detail",
),
path(
"calendar/<str:pk>/holidays/",
"calendar/<uid:pk>/holidays/",
views.BusinessHolidayListView.as_view(),
name="holiday_list",
),
path(
"calendar/<str:pk>/holidays/<str:hid>/",
"calendar/<uid:pk>/holidays/<uid:hid>/",
views.BusinessHolidayDetailView.as_view(),
name="holiday_detail",
),

6
backend/cases/time_entry_urls.py
View file

@ -10,7 +10,7 @@ from cases import time_views
app_name = "api_time_entries"
urlpatterns = [
# Timesheet must come before <str:pk>/ to avoid the catchall.
# Timesheet must come before <uid:pk>/ to avoid the catchall.
path(
"timesheet/",
time_views.TimesheetView.as_view(),
@ -22,12 +22,12 @@ urlpatterns = [
name="time_entries_unbilled",
),
path(
"<str:pk>/stop/",
"<uid:pk>/stop/",
time_views.TimeEntryStopView.as_view(),
name="time_entry_stop",
),
path(
"<str:pk>/",
"<uid:pk>/",
time_views.TimeEntryDetailView.as_view(),
name="time_entry_detail",
),

108
backend/cases/urls.py
View file

@ -25,7 +25,7 @@ app_name = "api_cases"
urlpatterns = [
# Cases list endpoint
path("", views.CaseListView.as_view()),
# Kanban endpoints (must be before <str:pk>/ to avoid conflicts)
# Kanban endpoints (must be before <uid:pk>/ to avoid conflicts)
path("kanban/", kanban_views.CaseKanbanView.as_view(), name="case_kanban"),
# Pipeline management
path(
@ -34,81 +34,81 @@ urlpatterns = [
name="pipeline_list_create",
),
path(
"pipelines/<str:pk>/",
"pipelines/<uid:pk>/",
kanban_views.CasePipelineDetailView.as_view(),
name="pipeline_detail",
),
path(
"pipelines/<str:pipeline_pk>/stages/",
"pipelines/<uid:pipeline_pk>/stages/",
kanban_views.CaseStageCreateView.as_view(),
name="stage_create",
),
path(
"pipelines/<str:pipeline_pk>/stages/reorder/",
"pipelines/<uid:pipeline_pk>/stages/reorder/",
kanban_views.CaseStageReorderView.as_view(),
name="stage_reorder",
),
# Stage management
path(
"stages/<str:pk>/",
"stages/<uid:pk>/",
kanban_views.CaseStageDetailView.as_view(),
name="stage_detail",
),
# Solutions (Knowledge Base) endpoints (must be before <str:pk>/ to avoid conflicts)
# Solutions (Knowledge Base) endpoints (must be before <uid:pk>/ to avoid conflicts)
path(
"solutions/", solution_views.SolutionListView.as_view(), name="solutions_list"
),
path(
"solutions/<str:pk>/",
"solutions/<uid:pk>/",
solution_views.SolutionDetailView.as_view(),
name="solution_detail",
),
path(
"solutions/<str:pk>/publish/",
"solutions/<uid:pk>/publish/",
solution_views.SolutionPublishView.as_view(),
name="solution_publish",
),
path(
"solutions/<str:pk>/unpublish/",
"solutions/<uid:pk>/unpublish/",
solution_views.SolutionUnpublishView.as_view(),
name="solution_unpublish",
),
# Reopen policy (admin singleton, must be before <str:pk>/ patterns)
# Reopen policy (admin singleton, must be before <uid:pk>/ patterns)
path(
"reopen-policy/",
views.ReopenPolicyView.as_view(),
name="reopen_policy",
),
# Escalation policies (admin CRUD, must be before <str:pk>/ patterns)
# Escalation policies (admin CRUD, must be before <uid:pk>/ patterns)
path(
"escalation-policies/",
escalation_views.EscalationPolicyListCreateView.as_view(),
name="escalation_policy_list_create",
),
path(
"escalation-policies/<str:pk>/",
"escalation-policies/<uid:pk>/",
escalation_views.EscalationPolicyDetailView.as_view(),
name="escalation_policy_detail",
),
# Routing rules (admin CRUD + dry-run test endpoint, must be before <str:pk>/ patterns)
# Routing rules (admin CRUD + dry-run test endpoint, must be before <uid:pk>/ patterns)
path(
"routing-rules/",
routing_views.RoutingRuleListCreateView.as_view(),
name="routing_rule_list_create",
),
path(
"routing-rules/<str:pk>/",
"routing-rules/<uid:pk>/",
routing_views.RoutingRuleDetailView.as_view(),
name="routing_rule_detail",
),
path(
"routing-rules/<str:pk>/test/",
"routing-rules/<uid:pk>/test/",
routing_views.RoutingRuleTestView.as_view(),
name="routing_rule_test",
),
# Inbound email: public webhook (SNS-signed) + admin mailbox CRUD.
path(
"inbound/<str:mailbox_id>/",
"inbound/<uid:mailbox_id>/",
inbound_views.InboundMailboxWebhookView.as_view(),
name="inbound_webhook",
),
@ -118,17 +118,17 @@ urlpatterns = [
name="inbound_mailbox_list_create",
),
path(
"mailboxes/<str:pk>/",
"mailboxes/<uid:pk>/",
inbound_views.InboundMailboxDetailView.as_view(),
name="inbound_mailbox_detail",
),
# Watching list (must be before <str:pk>/ patterns)
# Watching list (must be before <uid:pk>/ patterns)
path(
"watching/",
watcher_views.WatchingListView.as_view(),
name="cases_watching",
),
# Analytics dashboards (must be before <str:pk>/ patterns)
# Analytics dashboards (must be before <uid:pk>/ patterns)
path(
"analytics/frt/",
analytics_views.AnalyticsFrtView.as_view(),
@ -169,13 +169,13 @@ urlpatterns = [
analytics_views.AnalyticsExportView.as_view(),
name="analytics_export",
),
# CSAT aggregate (must be before <str:pk>/ patterns)
# CSAT aggregate (must be before <uid:pk>/ patterns)
path(
"csat/aggregate/",
csat_views.CsatAggregateView.as_view(),
name="csat_aggregate",
),
# Bulk endpoints (must be before <str:pk>/ patterns)
# Bulk endpoints (must be before <uid:pk>/ patterns)
path(
"bulk/update/",
bulk_views.BulkUpdateCasesView.as_view(),
@ -186,7 +186,7 @@ urlpatterns = [
bulk_views.BulkDeleteCasesView.as_view(),
name="cases_bulk_delete",
),
# CSV import (must be before <str:pk>/ patterns)
# CSV import (must be before <uid:pk>/ patterns)
path(
"import/preview/",
import_views.CaseImportPreviewView.as_view(),
@ -197,91 +197,91 @@ urlpatterns = [
import_views.CaseImportCommitView.as_view(),
name="cases_import_commit",
),
# Case ↔ Solution linking (M2M endpoints, must be before <str:pk>/ patterns)
# Case ↔ Solution linking (M2M endpoints, must be before <uid:pk>/ patterns)
path(
"<str:pk>/solutions/",
"<uid:pk>/solutions/",
views.CaseSolutionLinkView.as_view(),
name="case_solution_link",
),
path(
"<str:pk>/solutions/<str:solution_pk>/",
"<uid:pk>/solutions/<uid:solution_pk>/",
views.CaseSolutionLinkView.as_view(),
name="case_solution_unlink",
),
# Audit-log feed (must be before <str:pk>/ pattern)
# Audit-log feed (must be before <uid:pk>/ pattern)
path(
"<str:pk>/activities/",
"<uid:pk>/activities/",
views.CaseActivityListView.as_view(),
name="case_activities",
),
# KB suggester typeahead (must be before <str:pk>/ pattern)
# KB suggester typeahead (must be before <uid:pk>/ pattern)
path(
"<str:pk>/solution-suggestions/",
"<uid:pk>/solution-suggestions/",
kb_views.SolutionSuggestionsView.as_view(),
name="case_solution_suggestions",
),
# Watch toggle + watchers list (must be before <str:pk>/ pattern)
# Watch toggle + watchers list (must be before <uid:pk>/ pattern)
path(
"<str:pk>/watch/",
"<uid:pk>/watch/",
watcher_views.WatchView.as_view(),
name="case_watch",
),
path(
"<str:pk>/watchers/",
"<uid:pk>/watchers/",
watcher_views.WatchersListView.as_view(),
name="case_watchers",
),
# Merge endpoint (must be before <str:pk>/ pattern)
# Merge endpoint (must be before <uid:pk>/ pattern)
path(
"<str:pk>/merge/<str:into_id>/",
"<uid:pk>/merge/<uid:into_id>/",
merge_views.CaseMergeView.as_view(),
name="case_merge",
),
path(
"<str:pk>/unmerge/",
"<uid:pk>/unmerge/",
unmerge_views.CaseUnmergeView.as_view(),
name="case_unmerge",
),
# Parent/child tree endpoints (must be before <str:pk>/ pattern)
# Parent/child tree endpoints (must be before <uid:pk>/ pattern)
path(
"<str:pk>/tree/",
"<uid:pk>/tree/",
parent_views.CaseTreeView.as_view(),
name="case_tree",
),
path(
"<str:pk>/link/",
"<uid:pk>/link/",
parent_views.CaseLinkParentView.as_view(),
name="case_link_parent",
),
path(
"<str:pk>/close-with-children/",
"<uid:pk>/close-with-children/",
parent_views.CaseCloseWithChildrenView.as_view(),
name="case_close_with_children",
),
# Time-tracking endpoints (must be before <str:pk>/ catchall)
# Time-tracking endpoints (must be before <uid:pk>/ catchall)
path(
"<str:pk>/time-entries/",
"<uid:pk>/time-entries/",
time_views.TimeEntryListCreateView.as_view(),
name="case_time_entries",
),
path(
"<str:pk>/time-entries/start/",
"<uid:pk>/time-entries/start/",
time_views.TimeEntryStartView.as_view(),
name="case_time_entry_start",
),
path(
"<str:pk>/time-summary/",
"<uid:pk>/time-summary/",
time_views.TimeSummaryView.as_view(),
name="case_time_summary",
),
# Approval workflows (must be before <str:pk>/ catchall), Tier 3 approvals.
# Approval workflows (must be before <uid:pk>/ catchall), Tier 3 approvals.
path(
"approval-rules/",
approval_views.ApprovalRuleListCreateView.as_view(),
name="approval_rule_list_create",
),
path(
"approval-rules/<str:pk>/",
"approval-rules/<uid:pk>/",
approval_views.ApprovalRuleDetailView.as_view(),
name="approval_rule_detail",
),
@ -291,28 +291,28 @@ urlpatterns = [
name="approval_inbox",
),
path(
"approvals/<str:pk>/approve/",
"approvals/<uid:pk>/approve/",
approval_views.ApprovalApproveView.as_view(),
name="approval_approve",
),
path(
"approvals/<str:pk>/reject/",
"approvals/<uid:pk>/reject/",
approval_views.ApprovalRejectView.as_view(),
name="approval_reject",
),
path(
"approvals/<str:pk>/cancel/",
"approvals/<uid:pk>/cancel/",
approval_views.ApprovalCancelView.as_view(),
name="approval_cancel",
),
path(
"<str:pk>/request-approval/",
"<uid:pk>/request-approval/",
approval_views.CaseRequestApprovalView.as_view(),
name="case_request_approval",
),
# Case detail routes (must be after all named routes due to <str:pk> pattern)
path("<str:pk>/", views.CaseDetailView.as_view()),
path("<str:pk>/move/", kanban_views.CaseMoveView.as_view(), name="case_move"),
path("comment/<str:pk>/", views.CaseCommentView.as_view()),
path("attachment/<str:pk>/", views.CaseAttachmentView.as_view()),
# Case detail routes (must be after all named routes due to <uid:pk> pattern)
path("<uid:pk>/", views.CaseDetailView.as_view()),
path("<uid:pk>/move/", kanban_views.CaseMoveView.as_view(), name="case_move"),
path("comment/<uid:pk>/", views.CaseCommentView.as_view()),
path("attachment/<uid:pk>/", views.CaseAttachmentView.as_view()),
]

17
backend/cases/views.py
View file

@ -1110,7 +1110,16 @@ class CaseSolutionLinkView(APIView):
fields={"solution_id": serializers.CharField()},
),
)
def post(self, request, pk):
def post(self, request, pk, solution_pk=None):
# This class is routed twice: `<pk>/solutions/` for POST and
# `<pk>/solutions/<solution_pk>/` for DELETE. Django hands every route's
# captured kwargs to whichever method it dispatches, so POST to the
# second route arrived with an argument it had no parameter for and
# DELETE to the first arrived missing one. Both were an uncaught
# TypeError, which is a 500 on a route the client is simply using the
# wrong verb on. 405 is the answer to a wrong verb.
if solution_pk is not None:
self.http_method_not_allowed(request)
case = get_case_or_404(request.profile, pk)
# Attaching an article to a ticket changes the ticket, so it takes the
# same permission as any other change to it. Org alone was the whole
@ -1144,7 +1153,11 @@ class CaseSolutionLinkView(APIView):
)
@extend_schema(tags=["Cases"])
def delete(self, request, pk, solution_pk):
def delete(self, request, pk, solution_pk=None):
# See `post` above: reached without a solution id, this is the
# collection route being deleted, which is a wrong verb, not a crash.
if solution_pk is None:
self.http_method_not_allowed(request)
case = get_case_or_404(request.profile, pk)
assert_case_write_access(request.profile, case)
sol = self._get_solution(solution_pk, request.profile.org)

46
backend/common/converters.py Normal file
View file

@ -0,0 +1,46 @@
"""A URL path converter for id segments, so a malformed id 404s at the router.
Every model here has a UUID primary key, but the routes captured those ids with
``<str:pk>``. The string reached the view unexamined and went straight into an
``id`` lookup, where Django parses it while *building* the query and raises
``django.core.exceptions.ValidationError``. DRF's exception handler does not
translate that class, so the request answered 500. A stale bookmark, a
hand-edited URL, or a crawler hitting ``/api/leads/undefined/`` took the
endpoint down, and it did so on 114 route/method combinations across roughly 50
views: every detail, move, comment, attachment, watch, merge and approval route
in the product.
Fixing it per view means auditing 50 view bodies and remembering the guard in
every future one. Fixing it at the router is one line per route and cannot be
forgotten, because a view can no longer be reached with an unparseable id.
Django ships a ``uuid`` converter already, and it is deliberately not used here:
its regex accepts only the canonical lowercase hyphenated form, while the ORM
accepts bare hex, braced and URN forms and any case. Swapping in ``<uuid:pk>``
would turn requests that work today into 404s. ``uuid.UUID`` is what
``UUIDField.to_python`` itself calls, so parsing with it accepts exactly what
the ORM accepts and nothing that used to work can break. ``common.validators``
made the same choice for query parameters and says so for the same reason.
Raising ``ValueError`` from ``to_python`` is the documented way to tell the
resolver "this pattern does not match": resolution continues to the next
pattern and ends in a normal 404, rather than the pattern matching and the view
failing later.
"""
import uuid as uuid_module
class UUIDLikeConverter:
"""Match a path segment that ``uuid.UUID`` can parse, in any of its forms."""
regex = "[^/]+"
def to_python(self, value):
try:
return str(uuid_module.UUID(value))
except (AttributeError, TypeError, ValueError):
raise ValueError(f"{value!r} is not a usable id.") from None
def to_url(self, value):
return str(value)

163
backend/common/tests/test_malformed_id_routes.py Normal file
View file

@ -0,0 +1,163 @@
"""No route may answer 500 because an id in the path was not a UUID.
Every model here has a UUID primary key, and the routes used to capture those
ids with ``<str:pk>``. The raw string reached the view and went into an ``id``
lookup, where Django parses it while building the query and raises
``django.core.exceptions.ValidationError``. DRF does not translate that class,
so the request answered 500: 114 route/method combinations across roughly 50
views, reachable by anything as ordinary as a stale bookmark or a crawler
following ``/api/leads/undefined/``.
The fix is the ``uid`` path converter (``common/converters.py``), so this is a
sweep rather than a list: it walks the live URLconf, so a route added later
with ``<str:pk>`` fails here instead of shipping the old bug back. That is the
point of driving it from the resolver and not from a fixed list of paths.
``pack_id`` and the public ``token`` are deliberately still ``<str:...>``: one
is a registry key and the other a ``secrets.token_urlsafe`` string. Neither is
a UUID, and neither reaches a UUID column.
"""
import re
import pytest
from django.urls import get_resolver
MALFORMED = "not-a-uuid"
# Path params that are legitimately not UUIDs. Anything else must be `<uid:>`.
NON_UUID_PARAMS = {"token", "pack_id"}
def _routes():
"""Every leaf route in the live URLconf, with its full pattern and view."""
found = []
def walk(patterns, prefix=""):
for entry in patterns:
if hasattr(entry, "url_patterns"):
walk(entry.url_patterns, prefix + str(entry.pattern))
else:
found.append((prefix + str(entry.pattern), entry.callback))
walk(get_resolver().url_patterns)
return found
def _id_routes():
"""Routes carrying at least one id segment, excluding the public portal."""
out = []
for route, callback in _routes():
if "api/public/" in route:
continue
if "<uid:" not in route and "<str:" not in route:
continue
out.append((route, callback))
return out
def test_no_id_param_is_still_captured_as_a_bare_string():
"""The converter is only a fix while every id route actually uses it."""
offenders = [
(route, name)
for route, _ in _id_routes()
for name in re.findall(r"<str:(\w+)>", route)
if name not in NON_UUID_PARAMS
]
assert offenders == [], (
"these routes capture an id as a bare string, so a malformed id "
f"reaches the ORM and answers 500: {offenders}"
)
@pytest.mark.django_db
def test_no_route_answers_500_for_a_malformed_id(admin_client):
"""Drive every id-bearing route with an unparseable id."""
failures = []
for route, callback in _id_routes():
url = "/" + re.sub(r"<\w+:(\w+)>", MALFORMED, route)
view = getattr(callback, "cls", None)
if view is None:
continue
for method in ("get", "post", "put", "patch", "delete"):
if not hasattr(view, method):
continue
try:
code = getattr(admin_client, method)(url, {}, format="json").status_code
except Exception as exc: # noqa: BLE001
failures.append(f"{method.upper()} {url} raised {type(exc).__name__}: {exc}")
continue
if code >= 500:
failures.append(f"{method.upper()} {url} answered {code}")
assert failures == [], "routes that fail on a malformed id:\n" + "\n".join(failures)
@pytest.mark.django_db
class TestTheConverterAcceptsEveryFormTheORMAccepts:
"""A guard that rejects too much is the same defect facing the other way.
``uuid.UUID`` accepts bare hex, braced and URN forms and any case, and so
does the ORM. Django's built-in ``uuid`` converter accepts only canonical
lowercase, which is why it is not used here. A 404 in these cases would
mean the fix broke requests that used to work.
"""
@pytest.fixture
def account(self, org_a):
from accounts.models import Account
return Account.objects.create(name="Converter Account", org=org_a)
@pytest.mark.parametrize(
"form",
[
"canonical",
"uppercase",
"bare_hex",
"braced",
"urn",
],
)
def test_form_resolves(self, admin_client, account, form):
raw = account.id
rendered = {
"canonical": str(raw),
"uppercase": str(raw).upper(),
"bare_hex": raw.hex,
"braced": f"{{{raw}}}",
"urn": raw.urn,
}[form]
response = admin_client.get(f"/api/accounts/{rendered}/")
assert response.status_code != 404, (
f"the {form} form of a real id no longer resolves; the converter "
"is stricter than the ORM"
)
@pytest.mark.django_db
class TestWrongVerbOnTheCaseSolutionRoutes:
"""One view class, two routes, different captured kwargs.
Django passes a route's kwargs to whichever method it dispatches, so
``DELETE`` on the collection route arrived missing ``solution_pk`` and
``POST`` on the member route arrived with one. Both were an uncaught
``TypeError``, so a client using the wrong verb got a 500 instead of a 405.
A malformed id now 404s before reaching either, which would have hidden
this: these use a well-formed id on purpose.
"""
@pytest.fixture
def case(self, org_a):
from cases.models import Case
return Case.objects.create(name="Converter Case", status="New", org=org_a)
def test_delete_on_the_collection_route(self, admin_client, case):
response = admin_client.delete(f"/api/cases/{case.id}/solutions/")
assert response.status_code == 405, response.status_code
def test_post_on_the_member_route(self, admin_client, case):
response = admin_client.post(
f"/api/cases/{case.id}/solutions/{case.id}/", {}, format="json"
)
assert response.status_code == 405, response.status_code

77
backend/common/tests/test_tags.py
View file

@ -9,15 +9,22 @@ from rest_framework import status
from accounts.models import Account
from cases.models import Case
from common.models import Tags
from common.models import APISettings, Tags
from contacts.models import Contact
from leads.models import Lead
from opportunity.models import Opportunity
from tasks.models import Task
def _tag_row(response, tag_id):
return next(r for r in response.data["tags"] if r["id"] == str(tag_id))
def _tag_row_by_id(body, tag_id):
"""Same lookup against an already-unwrapped response body."""
return next(r for r in body["tags"] if r["id"] == str(tag_id))
@pytest.mark.django_db
class TestTagsListView:
"""Tests for GET/POST /api/tags/"""
@ -168,6 +175,12 @@ class TestTagsUsageAndTotals:
Case.objects.create(
org=org_a, name="Ticket", status="New", priority="Normal"
).tags.add(tag)
Contact.objects.create(
org=org_a, first_name="Dana", last_name="Client"
).tags.add(tag)
Task.objects.create(
org=org_a, title="Follow up", status="New", priority="Medium"
).tags.add(tag)
response = admin_client.get(self.url)
assert response.status_code == status.HTTP_200_OK
@ -177,6 +190,9 @@ class TestTagsUsageAndTotals:
"leads": 2,
"opportunities": 1,
"cases": 1,
"contacts": 1,
"tasks": 1,
"api_settings": 0,
}
def test_unused_tag_reports_zero_usage(self, admin_client, org_a):
@ -187,8 +203,67 @@ class TestTagsUsageAndTotals:
"leads": 0,
"opportunities": 0,
"cases": 0,
"contacts": 0,
"tasks": 0,
"api_settings": 0,
}
@pytest.mark.parametrize("kind", ["contacts", "tasks", "api_settings"])
def test_a_tag_used_only_off_the_headline_models_is_not_unused(
self, admin_client, org_a, kind
):
"""The bug: `_TAGGABLE` held four of the seven taggable models.
A tag applied only to a contact, a task or an API setting reported zero
usage everywhere and was counted in the "unused" stat, telling an admin
it was safe to archive while it was in active use.
"""
tag = Tags.objects.create(name=f"Only {kind}", slug=f"only-{kind}", org=org_a)
if kind == "contacts":
Contact.objects.create(
org=org_a, first_name="Dana", last_name="Client"
).tags.add(tag)
elif kind == "tasks":
Task.objects.create(
org=org_a, title="Follow up", status="New", priority="Medium"
).tags.add(tag)
else:
APISettings.objects.create(
org=org_a, title="Site", website="https://example.com"
).tags.add(tag)
body = admin_client.get(self.url).data
assert _tag_row_by_id(body, tag.id)["usage"][kind] == 1
assert body["totals"]["unused"] == 0, (
f"a tag in use on {kind} was reported as unused"
)
def test_taggable_covers_every_model_with_a_tags_m2m(self):
"""Guard the hand-written constant with the live model registry.
`_TAGGABLE` is written out by hand so a change shows up in a diff, which
is only safe if something notices when a new taggable model appears.
Without this, the next model to grow a `tags` M2M silently repeats the
bug above.
"""
from django.apps import apps
from common.views.tags_views import _TAGGABLE
declared = {
model
for model in apps.get_models()
for field in model._meta.get_fields()
if field.many_to_many
and not field.auto_created
and getattr(field, "related_model", None) is Tags
}
counted = {model for _, model in _TAGGABLE}
assert declared == counted, (
"these models carry a tags M2M but their usage is never counted, so "
f"a tag used only on them reads as unused: {declared - counted}"
)
def test_totals_count_active_unused(self, admin_client, org_a):
used = Tags.objects.create(name="Used", slug="used", org=org_a)
Account.objects.create(org=org_a, name="Acme").tags.add(used)

24
backend/common/urls.py
View file

@ -85,7 +85,7 @@ urlpatterns = [
# Organization and profile management
path("org/", OrgProfileCreateView.as_view()),
path("org/settings/", OrgSettingsView.as_view(), name="org_settings"),
# These literal org/… paths must precede org/<str:pk>/ so they are not
# These literal org/… paths must precede org/<uid:pk>/ so they are not
# captured as a pk (org/tokens/ would otherwise resolve to OrgUpdateView
# with pk="tokens"). org/tokens/ is ADMIN-only token oversight, separate
# from profile/tokens/ (self-scoped) so the self guard is never widened; an
@ -98,7 +98,7 @@ urlpatterns = [
OrgAccessTokenDetailView.as_view(),
name="org_pat_detail",
),
path("org/<str:pk>/", OrgUpdateView.as_view()),
path("org/<uid:pk>/", OrgUpdateView.as_view()),
path("profile/", ProfileView.as_view()),
# Personal Access Tokens (REST API), a user manages ONLY their own
path(
@ -114,23 +114,23 @@ urlpatterns = [
# User management
path("users/get-teams-and-users/", GetTeamsAndUsersView.as_view()),
path("users/", UsersListView.as_view()),
path("user/<str:pk>/", UserDetailView.as_view()),
path("user/<str:pk>/status/", UserStatusView.as_view()),
path("user/<uid:pk>/", UserDetailView.as_view()),
path("user/<uid:pk>/status/", UserStatusView.as_view()),
# Documents
path("documents/", DocumentListView.as_view()),
path("documents/<str:pk>/", DocumentDetailView.as_view()),
path("documents/<uid:pk>/", DocumentDetailView.as_view()),
# API Settings
path("api-settings/", DomainList.as_view()),
path("api-settings/<str:pk>/", DomainDetailView.as_view()),
path("api-settings/<uid:pk>/", DomainDetailView.as_view()),
# Activities (for dashboard recent activities)
path("activities/", ActivityListView.as_view(), name="activities"),
# Teams (merged from teams app)
path("teams/", TeamsListView.as_view()),
path("teams/<str:pk>/", TeamsDetailView.as_view()),
path("teams/<uid:pk>/", TeamsDetailView.as_view()),
# Tags
path("tags/", TagsListView.as_view()),
path("tags/<str:pk>/", TagsDetailView.as_view()),
path("tags/<str:pk>/restore/", TagsRestoreView.as_view()),
path("tags/<uid:pk>/", TagsDetailView.as_view()),
path("tags/<uid:pk>/restore/", TagsRestoreView.as_view()),
# Custom fields (per-org schema extension; cross-entity)
path(
"custom-fields/",
@ -138,7 +138,7 @@ urlpatterns = [
name="custom_fields_list_create",
),
path(
"custom-fields/<str:pk>/",
"custom-fields/<uid:pk>/",
CustomFieldDefinitionDetailView.as_view(),
name="custom_field_detail",
),
@ -150,12 +150,12 @@ urlpatterns = [
name="notifications_read_all",
),
path(
"notifications/<str:pk>/read/",
"notifications/<uid:pk>/read/",
NotificationReadView.as_view(),
name="notifications_read",
),
path(
"notifications/<str:pk>/",
"notifications/<uid:pk>/",
NotificationDetailView.as_view(),
name="notifications_detail",
),

19
backend/common/views/tags_views.py
View file

@ -12,19 +12,36 @@ from accounts.models import Account
from cases.models import Case
from common import swagger_params
from common.lookups import get_scoped_or_404
from common.models import Tags
from common.models import APISettings, Tags
from common.permissions import HasOrgContext
from common.serializer import TagsSerializer
from contacts.models import Contact
from leads.models import Lead
from opportunity.models import Opportunity
from tasks.models import Task
# The models a tag can be applied to, paired with the usage key the settings
# page shows. Each is org-scoped and carries a `tags` M2M back to Tags.
#
# This must list EVERY model with that M2M, not just the prominent ones. It
# held four of the seven, and the three it left out (Contact, Task,
# APISettings) all accept tag writes through the API, contacts through the CSV
# importer too. A tag applied only to contacts therefore reported zero usage
# and was counted as "unused" on the settings page, which is the one number an
# admin uses to decide whether a tag is safe to archive.
#
# Hand-written on purpose, so a change shows up in a diff, and guarded by
# `test_taggable_covers_every_model_with_a_tags_m2m`, which walks the live
# model registry: a new taggable model fails that test instead of silently
# under-reporting usage forever.
_TAGGABLE = (
("accounts", Account),
("leads", Lead),
("opportunities", Opportunity),
("cases", Case),
("contacts", Contact),
("tasks", Task),
("api_settings", APISettings),
)

8
backend/contacts/urls.py
View file

@ -6,7 +6,7 @@ app_name = "api_contacts"
urlpatterns = [
path("", views.ContactsListView.as_view()),
# CSV import (must be before <str:pk>/ to avoid being captured as an ID)
# CSV import (must be before <uid:pk>/ to avoid being captured as an ID)
path(
"import/preview/",
import_views.ContactImportPreviewView.as_view(),
@ -17,7 +17,7 @@ urlpatterns = [
import_views.ContactImportCommitView.as_view(),
name="contacts_import_commit",
),
path("<str:pk>/", views.ContactDetailView.as_view()),
path("comment/<str:pk>/", views.ContactCommentView.as_view()),
path("attachment/<str:pk>/", views.ContactAttachmentView.as_view()),
path("<uid:pk>/", views.ContactDetailView.as_view()),
path("comment/<uid:pk>/", views.ContactCommentView.as_view()),
path("attachment/<uid:pk>/", views.ContactAttachmentView.as_view()),
]

16
backend/crm/urls.py
View file

@ -1,15 +1,29 @@
from django.conf import settings
from django.contrib import admin
from django.contrib.auth import views
from django.urls import include, path
from django.urls import include, path, register_converter
from django.urls import re_path as url
from django.views.generic import TemplateView
from drf_spectacular.plumbing import DJANGO_PATH_CONVERTER_MAPPING
from drf_spectacular.types import OpenApiTypes
from drf_spectacular.views import (
SpectacularAPIView,
SpectacularRedocView,
SpectacularSwaggerView,
)
from common.converters import UUIDLikeConverter
# Registered on the root URLconf so every app's `<uid:...>` segment resolves.
# See common/converters.py for why this exists and why it is not `<uuid:...>`.
register_converter(UUIDLikeConverter, "uid")
# drf-spectacular maps a converter name to an OpenAPI type; an unknown one
# falls back to a bare string. Without this the published schema would describe
# 165 id path parameters as untyped strings, which is what `<str:pk>` gave us
# and is less than the route now guarantees.
DJANGO_PATH_CONVERTER_MAPPING["uid"] = OpenApiTypes.UUID
app_name = "crm"
urlpatterns = [

5
backend/invoices/api_urls.py
View file

@ -139,6 +139,11 @@ urlpatterns = [
api_views.InvoiceTemplateDetailView.as_view(),
name="template_detail",
),
path(
"templates/<uuid:pk>/editor/",
api_views.InvoiceTemplateEditorView.as_view(),
name="template_editor",
),
# ==========================================================================
# REPORTS
# ==========================================================================

72
backend/invoices/api_views.py
View file

@ -59,6 +59,7 @@ from invoices.serializer import (
InvoiceSerializer,
InvoiceTemplateCreateSerializer,
InvoiceTemplateListSerializer,
InvoiceTemplateSerializer,
PaymentCreateSerializer,
PaymentSerializer,
ProductCreateSerializer,
@ -653,7 +654,9 @@ class InvoiceLineItemListView(APIView):
if error:
return error
serializer = InvoiceLineItemCreateSerializer(data=request.data)
serializer = InvoiceLineItemCreateSerializer(
data=request.data, request_obj=request
)
if serializer.is_valid():
line_item = serializer.save(invoice=invoice, org=request.profile.org)
@ -700,7 +703,7 @@ class InvoiceLineItemDetailView(APIView):
)
serializer = InvoiceLineItemCreateSerializer(
line_item, data=request.data, partial=True
line_item, data=request.data, partial=True, request_obj=request
)
if serializer.is_valid():
line_item = serializer.save()
@ -1528,22 +1531,26 @@ class RecurringInvoicePauseView(APIView):
# =============================================================================
def _forbid_non_admin_template_write(request):
def _forbid_non_admin_template(
request, message="Only an administrator can change invoice templates."
):
"""Return a 403 Response for non-admins, or None to allow.
Invoice templates are org-wide shared config: every member reads the
catalogue (to see how their invoices look), but changing a template: its
colours, its default flag, its raw HTML/CSS, affects everyone in the org
and drives the server-side PDF render. So writes (create/update/delete) are
admin-only, while reads stay open. Role is derived from ``request.profile``,
never the request body.
admin-only, while the catalogue read stays open. Role is derived from
``request.profile``, never the request body.
``message`` is a parameter because one read is admin-only too: fetching a
template's raw markup for the editor. That is the same rule for the same
reason, so it is the same gate rather than a second copy of the role check
that could drift away from this one.
"""
if request.profile.role != "ADMIN" and not request.user.is_superuser:
return Response(
{
"error": True,
"message": "Only an administrator can change invoice templates.",
},
{"error": True, "message": message},
status=status.HTTP_403_FORBIDDEN,
)
return None
@ -1574,7 +1581,7 @@ class InvoiceTemplateListView(APIView, LimitOffsetPagination):
@extend_schema(tags=["Invoice Templates"], operation_id="templates_create")
def post(self, request):
denied = _forbid_non_admin_template_write(request)
denied = _forbid_non_admin_template(request)
if denied:
return denied
@ -1621,7 +1628,7 @@ class InvoiceTemplateDetailView(APIView):
@extend_schema(tags=["Invoice Templates"], operation_id="templates_update")
def put(self, request, pk):
denied = _forbid_non_admin_template_write(request)
denied = _forbid_non_admin_template(request)
if denied:
return denied
@ -1652,7 +1659,7 @@ class InvoiceTemplateDetailView(APIView):
@extend_schema(tags=["Invoice Templates"], operation_id="templates_destroy")
def delete(self, request, pk):
denied = _forbid_non_admin_template_write(request)
denied = _forbid_non_admin_template(request)
if denied:
return denied
@ -1670,6 +1677,47 @@ class InvoiceTemplateDetailView(APIView):
)
class InvoiceTemplateEditorView(APIView):
"""The only endpoint that returns a template's raw ``template_html``/``css``.
Every other read path uses ``InvoiceTemplateListSerializer``, which reports
booleans and a byte count in place of the markup, because that markup is
org-authored HTML/CSS that WeasyPrint renders into a PDF server-side: the
moment either field lands in a browser DOM, a PDF setting becomes stored
XSS.
An editor still has to load what it is editing, and it could not: the edit
form had no way to pre-fill the two fields it exists to change, so opening
it and saving silently blanked them. Serving the markup from its own
admin-only route keeps that fix out of the detail response every member
reads. A client must bind these two fields to a ``<textarea>`` value and
never to ``{@html}``.
"""
permission_classes = (IsAuthenticated, HasOrgContext)
@extend_schema(tags=["Invoice Templates"], operation_id="templates_editor_retrieve")
def get(self, request, pk):
# Role first, so the 403 does not depend on whether the id exists.
denied = _forbid_non_admin_template(
request,
"Only an administrator can view invoice template markup.",
)
if denied:
return denied
template = InvoiceTemplate.objects.filter(
id=pk, org=request.profile.org
).first()
if not template:
return Response(
{"error": True, "message": "Template not found"},
status=status.HTTP_404_NOT_FOUND,
)
return Response(InvoiceTemplateSerializer(template).data)
# =============================================================================
# COMMENTS AND ATTACHMENTS
# =============================================================================

72
backend/invoices/serializer.py
View file

@ -302,6 +302,28 @@ class InvoiceLineItemCreateSerializer(serializers.ModelSerializer):
"order",
)
def __init__(self, *args, **kwargs):
request_obj = kwargs.pop("request_obj", None)
super().__init__(*args, **kwargs)
self.org = None
if request_obj and hasattr(request_obj, "profile"):
self.org = request_obj.profile.org
def validate_product(self, value):
"""Reject a product belonging to another organization.
Only the standalone routes (`InvoiceLineItemListView.post` and
`InvoiceLineItemDetailView.put`) pass `request_obj`, and they are the
only callers that can: nested under `InvoiceCreateSerializer` this runs
as a child with no org context, which is why the nested path is guarded
by `validate_line_item_products` on the parent instead.
"""
if value is not None and self.org and value.org_id != self.org.id:
raise serializers.ValidationError(
"Product not found or does not belong to your organization"
)
return value
class EstimateLineItemCreateSerializer(serializers.ModelSerializer):
"""Serializer for creating/updating Estimate Line Items"""
@ -339,6 +361,35 @@ class RecurringInvoiceLineItemCreateSerializer(serializers.ModelSerializer):
)
def validate_line_item_products(line_items, org):
"""Reject line items that reference a product from another organization.
The three line-item create serializers above expose `product` as an
auto-generated PrimaryKeyRelatedField over an unscoped queryset, so a
product UUID belonging to any org resolves. Their parent serializer is the
first place with org context, which is why this is called from each
parent's `validate()` instead of living on the line-item serializer.
One function rather than three copies: invoices had the check and
estimates and recurring invoices did not, which is exactly how a guard
goes missing when it is written per call site. Guards create and update,
since `validate()` runs on both.
"""
if org is None:
return
for item in line_items or []:
product = item.get("product")
if product is not None and product.org_id != org.id:
raise serializers.ValidationError(
{
"line_items": (
"A line item references a product from another "
"organization."
)
}
)
# =============================================================================
# PAYMENT SERIALIZERS
# =============================================================================
@ -686,22 +737,7 @@ class InvoiceCreateSerializer(serializers.ModelSerializer):
{"contact_id": "Contact does not belong to the selected account"}
)
# Line-item products must belong to this org. The nested line-item
# serializer's `product` is an auto PrimaryKeyRelatedField over an
# unscoped queryset (all orgs), so a cross-org product UUID resolves;
# this is where we have org context to reject it. Guards create + update.
if self.org:
for item in attrs.get("line_items") or []:
product = item.get("product")
if product is not None and product.org_id != self.org.id:
raise serializers.ValidationError(
{
"line_items": (
"A line item references a product from another "
"organization."
)
}
)
validate_line_item_products(attrs.get("line_items"), self.org)
return attrs
def create(self, validated_data):
@ -992,6 +1028,8 @@ class EstimateCreateSerializer(serializers.ModelSerializer):
raise serializers.ValidationError(
{"contact_id": "Contact does not belong to the selected account"}
)
validate_line_item_products(attrs.get("line_items"), self.org)
return attrs
def create(self, validated_data):
@ -1205,6 +1243,8 @@ class RecurringInvoiceCreateSerializer(serializers.ModelSerializer):
raise serializers.ValidationError(
{"contact_id": "Contact does not belong to the selected account"}
)
validate_line_item_products(attrs.get("line_items"), self.org)
return attrs
def _recalculate_totals(self, instance):

2
backend/invoices/tests/test_invoices_api.py
View file

@ -2187,7 +2187,7 @@ class TestInvoiceTemplateWriteAuthorization:
"""Invoice templates are org-wide shared config: how every invoice reaches a
customer. Any member reads the catalogue, but creating/editing/deleting a
template, or flipping the org default, is admin-only (see
_forbid_non_admin_template_write). Both directions proven: a non-admin is
_forbid_non_admin_template). Both directions proven: a non-admin is
refused, an admin is allowed.
"""

285
backend/invoices/tests/test_line_item_product_org.py Normal file
View file

@ -0,0 +1,285 @@
"""A line item may not reference another organization's product.
``product`` is an auto-generated ``PrimaryKeyRelatedField`` on the three
line-item create serializers, so its queryset is ``Product.objects.all()``:
every org's catalogue. A member who knows a product UUID from another tenant
could name it on a line item and have it resolve, pulling that product's row
onto their own document.
``InvoiceCreateSerializer`` guarded this. ``EstimateCreateSerializer`` and
``RecurringInvoiceCreateSerializer`` are the same function without the check,
and the two standalone line-item routes built the serializer with no org
context at all, so neither could have checked. Five write paths, one guard
between them.
Both directions are pinned on every path: the cross-org product is refused,
and the org's own product still saves. A guard that rejects everything passes
the first half of that pair and breaks the feature.
"""
import datetime
import pytest
from rest_framework import status
from accounts.models import Account
from contacts.models import Contact
from invoices.models import Invoice, InvoiceLineItem, Product
INVOICES_URL = "/api/invoices/"
ESTIMATES_URL = "/api/invoices/estimates/"
RECURRING_URL = "/api/invoices/recurring/"
TODAY = datetime.date.today()
@pytest.fixture
def account_a(org_a):
return Account.objects.create(name="Line Item Account", org=org_a)
@pytest.fixture
def contact_a(org_a):
return Contact.objects.create(
first_name="Line",
last_name="Item",
email="lineitem@example.com",
org=org_a,
)
@pytest.fixture
def product_a(org_a):
return Product.objects.create(name="Own Widget", price=10, org=org_a)
@pytest.fixture
def product_b(org_b):
"""The other tenant's catalogue entry. Never reachable from org A."""
return Product.objects.create(name="Foreign Widget", price=999, org=org_b)
def _line_item(product):
return {
"product": str(product.id),
"name": "A line",
"quantity": 1,
"unit_price": "10.00",
}
def _invoice_payload(account, contact, product):
return {
"invoice_title": "Invoice with a line item",
"account_id": str(account.id),
"contact_id": str(contact.id),
"currency": "USD",
"line_items": [_line_item(product)],
}
def _estimate_payload(account, contact, product):
return {
"title": "Estimate with a line item",
"account_id": str(account.id),
"contact_id": str(contact.id),
"currency": "USD",
"issue_date": str(TODAY),
"expiry_date": str(TODAY + datetime.timedelta(days=30)),
"line_items": [_line_item(product)],
}
def _recurring_payload(account, contact, product):
return {
"title": "Recurring with a line item",
"account_id": str(account.id),
"contact_id": str(contact.id),
"frequency": "MONTHLY",
"start_date": str(TODAY),
"next_generation_date": str(TODAY),
"payment_terms": "NET_30",
"currency": "USD",
"is_active": True,
"line_items": [_line_item(product)],
}
@pytest.mark.django_db
class TestNestedCreateRefusesAForeignProduct:
def test_invoice_create(self, admin_client, account_a, contact_a, product_b):
response = admin_client.post(
INVOICES_URL,
_invoice_payload(account_a, contact_a, product_b),
format="json",
)
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert "line_items" in response.json()["errors"]
def test_estimate_create(self, admin_client, account_a, contact_a, product_b):
response = admin_client.post(
ESTIMATES_URL,
_estimate_payload(account_a, contact_a, product_b),
format="json",
)
assert response.status_code == status.HTTP_400_BAD_REQUEST, (
"an estimate accepted a line item pointing at another org's product"
)
assert "line_items" in response.json()["errors"]
def test_recurring_create(self, admin_client, account_a, contact_a, product_b):
response = admin_client.post(
RECURRING_URL,
_recurring_payload(account_a, contact_a, product_b),
format="json",
)
assert response.status_code == status.HTTP_400_BAD_REQUEST, (
"a recurring invoice accepted a line item pointing at another "
"org's product"
)
assert "line_items" in response.json()["errors"]
@pytest.mark.django_db
class TestNestedCreateStillAcceptsOwnProduct:
"""The False direction. Without these the guard could be `return False`."""
def test_invoice_create(self, admin_client, account_a, contact_a, product_a):
response = admin_client.post(
INVOICES_URL,
_invoice_payload(account_a, contact_a, product_a),
format="json",
)
assert response.status_code == status.HTTP_201_CREATED, response.content
def test_estimate_create(self, admin_client, account_a, contact_a, product_a):
response = admin_client.post(
ESTIMATES_URL,
_estimate_payload(account_a, contact_a, product_a),
format="json",
)
assert response.status_code == status.HTTP_201_CREATED, response.content
def test_recurring_create(self, admin_client, account_a, contact_a, product_a):
response = admin_client.post(
RECURRING_URL,
_recurring_payload(account_a, contact_a, product_a),
format="json",
)
assert response.status_code == status.HTTP_201_CREATED, response.content
def test_a_line_item_with_no_product_is_still_allowed(
self, admin_client, account_a, contact_a
):
"""`product` is optional. The guard must not have made it required."""
response = admin_client.post(
INVOICES_URL,
{
"invoice_title": "Ad hoc invoice",
"account_id": str(account_a.id),
"contact_id": str(contact_a.id),
"currency": "USD",
"line_items": [
{"name": "Ad hoc work", "quantity": 2, "unit_price": "50.00"}
],
},
format="json",
)
assert response.status_code == status.HTTP_201_CREATED, response.content
@pytest.mark.django_db
class TestNestedUpdateRefusesAForeignProduct:
"""`validate()` runs on update too, which is the path a PUT takes."""
def test_invoice_put(self, admin_client, org_a, account_a, contact_a, product_b):
invoice = Invoice.objects.create(
invoice_title="Existing", account=account_a, currency="USD", org=org_a
)
response = admin_client.put(
f"{INVOICES_URL}{invoice.id}/",
{
"account_id": str(account_a.id),
"contact_id": str(contact_a.id),
"line_items": [_line_item(product_b)],
},
format="json",
)
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert not InvoiceLineItem.objects.filter(invoice=invoice).exists()
@pytest.mark.django_db
class TestStandaloneLineItemRoutes:
"""These build the serializer directly, so they need their own org context."""
@pytest.fixture
def invoice(self, org_a, account_a):
return Invoice.objects.create(
invoice_title="Standalone target",
account=account_a,
currency="USD",
org=org_a,
)
def test_post_refuses_a_foreign_product(self, admin_client, invoice, product_b):
response = admin_client.post(
f"{INVOICES_URL}{invoice.id}/line-items/",
_line_item(product_b),
format="json",
)
assert response.status_code == status.HTTP_400_BAD_REQUEST, (
"the standalone line-item route attached another org's product"
)
assert "product" in response.json()["errors"]
assert not InvoiceLineItem.objects.filter(invoice=invoice).exists()
def test_post_accepts_own_product(self, admin_client, invoice, product_a):
response = admin_client.post(
f"{INVOICES_URL}{invoice.id}/line-items/",
_line_item(product_a),
format="json",
)
assert response.status_code == status.HTTP_201_CREATED, response.content
assert InvoiceLineItem.objects.filter(
invoice=invoice, product=product_a
).exists()
def test_put_refuses_a_foreign_product(
self, admin_client, org_a, invoice, product_a, product_b
):
line_item = InvoiceLineItem.objects.create(
invoice=invoice,
org=org_a,
product=product_a,
name="A line",
quantity=1,
unit_price=10,
)
response = admin_client.put(
f"{INVOICES_URL}{invoice.id}/line-items/{line_item.id}/",
{"product": str(product_b.id)},
format="json",
)
assert response.status_code == status.HTTP_400_BAD_REQUEST, (
"an existing line item was repointed at another org's product"
)
line_item.refresh_from_db()
assert line_item.product_id == product_a.id
def test_put_accepts_own_product(self, admin_client, org_a, invoice, product_a):
line_item = InvoiceLineItem.objects.create(
invoice=invoice,
org=org_a,
name="A line",
quantity=1,
unit_price=10,
)
response = admin_client.put(
f"{INVOICES_URL}{invoice.id}/line-items/{line_item.id}/",
{"product": str(product_a.id)},
format="json",
)
assert response.status_code == status.HTTP_200_OK, response.content
line_item.refresh_from_db()
assert line_item.product_id == product_a.id

158
backend/invoices/tests/test_template_editor.py Normal file
View file

@ -0,0 +1,158 @@
"""The template editor fetch: the one read that returns the raw markup.
``template_html`` and ``template_css`` are org-authored HTML/CSS rendered into
a PDF server-side, so every list and detail response replaces them with
booleans and a byte count. That is right, and it left the edit form unable to
pre-fill the two fields it exists to change.
This route serves them, admin-only, on its own path so the detail response that
every member reads stays narrow. The tests pin both halves: the markup comes
back here for an admin, and it still does not come back anywhere else.
"""
import pytest
from rest_framework import status
from accounts.models import Account
from invoices.models import InvoiceTemplate
MARKUP = "<h1>{{ invoice.number }}</h1><script>alert(1)</script>"
CSS = "h1 { color: red; }"
@pytest.fixture
def template(org_a):
return InvoiceTemplate.objects.create(
name="House style",
template_html=MARKUP,
template_css=CSS,
org=org_a,
)
@pytest.fixture
def template_org_b(org_b):
return InvoiceTemplate.objects.create(
name="Other tenant style",
template_html="<h1>not yours</h1>",
org=org_b,
)
def _editor_url(template):
return f"/api/invoices/templates/{template.id}/editor/"
@pytest.mark.django_db
class TestAdminCanLoadTheMarkup:
def test_returns_html_and_css(self, admin_client, template):
response = admin_client.get(_editor_url(template))
assert response.status_code == status.HTTP_200_OK, response.content
body = response.json()
assert body["template_html"] == MARKUP
assert body["template_css"] == CSS
def test_returns_the_rest_of_the_editable_fields(self, admin_client, template):
"""A prefill that omits a field makes the next save blank it."""
body = admin_client.get(_editor_url(template)).json()
for field in (
"name",
"primary_color",
"secondary_color",
"default_notes",
"default_terms",
"footer_text",
"is_default",
):
assert field in body, f"the editor cannot pre-fill {field}"
@pytest.mark.django_db
class TestTheMarkupStaysOffEveryOtherRead:
"""The False direction for the widening: nothing else grew the fields."""
def test_detail_still_hides_it(self, admin_client, template):
body = admin_client.get(f"/api/invoices/templates/{template.id}/").json()
assert "template_html" not in body
assert "template_css" not in body
assert body["has_custom_html"] is True
def test_list_still_hides_it(self, admin_client, template):
body = admin_client.get("/api/invoices/templates/").json()
rows = body["results"] if isinstance(body, dict) else body
assert rows, "no templates came back, so this proves nothing"
for row in rows:
assert "template_html" not in row
assert "template_css" not in row
@pytest.mark.django_db
class TestOnlyAdminsAndOnlyThisOrg:
def test_a_member_is_refused(self, user_client, template):
response = user_client.get(_editor_url(template))
assert response.status_code == status.HTTP_403_FORBIDDEN
assert "template_html" not in response.content.decode()
def test_a_member_is_refused_before_the_id_is_looked_up(
self, user_client, org_a
):
"""A 404 here would tell a member which template ids exist."""
missing = InvoiceTemplate(id="00000000-0000-0000-0000-000000000000")
response = user_client.get(_editor_url(missing))
assert response.status_code == status.HTTP_403_FORBIDDEN
def test_another_org_is_a_404(self, admin_client, template_org_b):
response = admin_client.get(_editor_url(template_org_b))
assert response.status_code == status.HTTP_404_NOT_FOUND
assert "not yours" not in response.content.decode()
def test_unauthenticated_is_refused(self, unauthenticated_client, template):
response = unauthenticated_client.get(_editor_url(template))
assert response.status_code in (
status.HTTP_401_UNAUTHORIZED,
status.HTTP_403_FORBIDDEN,
)
@pytest.mark.django_db
class TestTheEditRoundTrip:
"""What the endpoint exists for: load, change one field, save the rest."""
def test_prefill_then_save_keeps_the_markup(self, admin_client, template):
loaded = admin_client.get(_editor_url(template)).json()
loaded["name"] = "House style v2"
response = admin_client.put(
f"/api/invoices/templates/{template.id}/",
{k: v for k, v in loaded.items() if k not in ("id", "created_at", "updated_at")},
format="json",
)
assert response.status_code == status.HTTP_200_OK, response.content
template.refresh_from_db()
assert template.name == "House style v2"
assert template.template_html == MARKUP, (
"the round trip blanked the markup, which is the bug this endpoint "
"exists to prevent"
)
assert template.template_css == CSS
@pytest.mark.django_db
def test_the_pdf_still_renders_from_the_stored_markup(admin_client, org_a, template):
"""The markup is only meaningful because the server renders it.
Asserting the field survives a round trip is not enough on its own if the
render path stops reading it, so this pins that they are the same field.
"""
account = Account.objects.create(name="Template Account", org=org_a)
from invoices.models import Invoice
invoice = Invoice.objects.create(
invoice_title="Rendered",
account=account,
currency="USD",
org=org_a,
template=template,
)
assert invoice.template.template_html == MARKUP

16
backend/leads/urls.py
View file

@ -36,23 +36,23 @@ urlpatterns = [
"pipelines/", LeadPipelineListCreateView.as_view(), name="pipeline_list_create"
),
path(
"pipelines/<str:pk>/", LeadPipelineDetailView.as_view(), name="pipeline_detail"
"pipelines/<uid:pk>/", LeadPipelineDetailView.as_view(), name="pipeline_detail"
),
path(
"pipelines/<str:pipeline_pk>/stages/",
"pipelines/<uid:pipeline_pk>/stages/",
LeadStageCreateView.as_view(),
name="stage_create",
),
path(
"pipelines/<str:pipeline_pk>/stages/reorder/",
"pipelines/<uid:pipeline_pk>/stages/reorder/",
LeadStageReorderView.as_view(),
name="stage_reorder",
),
# Stage management
path("stages/<str:pk>/", LeadStageDetailView.as_view(), name="stage_detail"),
path("stages/<uid:pk>/", LeadStageDetailView.as_view(), name="stage_detail"),
# Lead detail routes (must be after specific routes due to pk pattern)
path("<str:pk>/", LeadDetailView.as_view()),
path("<str:pk>/move/", LeadMoveView.as_view(), name="lead_move"),
path("comment/<str:pk>/", LeadCommentView.as_view()),
path("attachment/<str:pk>/", LeadAttachmentView.as_view()),
path("<uid:pk>/", LeadDetailView.as_view()),
path("<uid:pk>/move/", LeadMoveView.as_view(), name="lead_move"),
path("comment/<uid:pk>/", LeadCommentView.as_view()),
path("attachment/<uid:pk>/", LeadAttachmentView.as_view()),
]

4
backend/macros/urls.py
View file

@ -6,6 +6,6 @@ app_name = "api_macros"
urlpatterns = [
path("", views.MacroListCreateView.as_view(), name="list_create"),
path("<str:pk>/", views.MacroDetailView.as_view(), name="detail"),
path("<str:pk>/render/", views.MacroRenderView.as_view(), name="render"),
path("<uid:pk>/", views.MacroDetailView.as_view(), name="detail"),
path("<uid:pk>/render/", views.MacroRenderView.as_view(), name="render"),
]

14
backend/opportunity/urls.py
View file

@ -31,19 +31,19 @@ urlpatterns = [
path("aging-config/", StageAgingConfigView.as_view()),
path("goals/", SalesGoalListView.as_view()),
path("goals/leaderboard/", SalesGoalLeaderboardView.as_view()),
path("goals/<str:pk>/", SalesGoalDetailView.as_view()),
path("<str:pk>/", OpportunityDetailView.as_view()),
path("<str:pk>/move/", OpportunityMoveView.as_view()),
path("comment/<str:pk>/", OpportunityCommentView.as_view()),
path("attachment/<str:pk>/", OpportunityAttachmentView.as_view()),
path("goals/<uid:pk>/", SalesGoalDetailView.as_view()),
path("<uid:pk>/", OpportunityDetailView.as_view()),
path("<uid:pk>/move/", OpportunityMoveView.as_view()),
path("comment/<uid:pk>/", OpportunityCommentView.as_view()),
path("attachment/<uid:pk>/", OpportunityAttachmentView.as_view()),
# Line items
path(
"<str:opportunity_id>/line-items/",
"<uid:opportunity_id>/line-items/",
OpportunityLineItemListView.as_view(),
name="opportunity-line-items-list",
),
path(
"<str:opportunity_id>/line-items/<str:line_item_id>/",
"<uid:opportunity_id>/line-items/<uid:line_item_id>/",
OpportunityLineItemDetailView.as_view(),
name="opportunity-line-items-detail",
),

26
backend/tasks/urls.py
View file

@ -28,48 +28,48 @@ app_name = "api_tasks"
urlpatterns = [
# Task endpoints
path("", TaskListView.as_view()),
# Kanban endpoints (must be before <str:pk>/ to avoid conflicts)
# Kanban endpoints (must be before <uid:pk>/ to avoid conflicts)
path("kanban/", TaskKanbanView.as_view(), name="task_kanban"),
path(
"pipelines/", TaskPipelineListCreateView.as_view(), name="pipeline_list_create"
),
path(
"pipelines/<str:pk>/", TaskPipelineDetailView.as_view(), name="pipeline_detail"
"pipelines/<uid:pk>/", TaskPipelineDetailView.as_view(), name="pipeline_detail"
),
path(
"pipelines/<str:pipeline_pk>/stages/",
"pipelines/<uid:pipeline_pk>/stages/",
TaskStageCreateView.as_view(),
name="stage_create",
),
path(
"pipelines/<str:pipeline_pk>/stages/reorder/",
"pipelines/<uid:pipeline_pk>/stages/reorder/",
TaskStageReorderView.as_view(),
name="stage_reorder",
),
path("stages/<str:pk>/", TaskStageDetailView.as_view(), name="stage_detail"),
path("stages/<uid:pk>/", TaskStageDetailView.as_view(), name="stage_detail"),
# Task detail and move endpoints
path("<str:pk>/", TaskDetailView.as_view()),
path("<str:pk>/move/", TaskMoveView.as_view(), name="task_move"),
path("comment/<str:pk>/", TaskCommentView.as_view()),
path("attachment/<str:pk>/", TaskAttachmentView.as_view()),
path("<uid:pk>/", TaskDetailView.as_view()),
path("<uid:pk>/move/", TaskMoveView.as_view(), name="task_move"),
path("comment/<uid:pk>/", TaskCommentView.as_view()),
path("attachment/<uid:pk>/", TaskAttachmentView.as_view()),
]
# Board URLs (kept separate for namespace compatibility with frontend)
board_urlpatterns = [
# Boards
path("", BoardListCreateView.as_view(), name="board_list_create"),
path("<str:pk>/", BoardDetailView.as_view(), name="board_detail"),
path("<uid:pk>/", BoardDetailView.as_view(), name="board_detail"),
# Columns
path(
"<str:board_pk>/columns/",
"<uid:board_pk>/columns/",
BoardColumnListCreateView.as_view(),
name="column_list_create",
),
# Tasks
path(
"columns/<str:column_pk>/tasks/",
"columns/<uid:column_pk>/tasks/",
BoardTaskListCreateView.as_view(),
name="task_list_create",
),
path("tasks/<str:pk>/", BoardTaskDetailView.as_view(), name="task_detail"),
path("tasks/<uid:pk>/", BoardTaskDetailView.as_view(), name="task_detail"),
]

8
frontend/src/lib/server/v2/tags.js
View file

@ -3,11 +3,15 @@
*
* Server-only. Reads `GET /tags/?include_archived=true`, which returns every
* tag in the org (active and archived. The page shows the "Off" ones so an
* admin can see what's been retired), each carrying a `usage` block
* `{ accounts, leads, opportunities, cases }` and a `totals` summary
* admin can see what's been retired), each carrying a `usage` block, one key
* per model a tag can be applied to, and a `totals` summary
* `{ count, active, unused }` for the stat cards. Both are computed server-side
* (the usage counts are org-scoped subqueries, not a client tally over rows).
*
* The page sums whatever keys `usage` carries rather than naming them, because
* that block used to cover four of the seven taggable models and a tag in real
* use on contacts or tasks reported as unused.
*
* "New tag" is wired: `createTag` below posts the name and the page's action
* calls it. Turning a tag off and back on are wired too, through `archiveTag`
* and `restoreTag`. The duplicate-merge banner is still deferred; its "Merge"

164
frontend/src/lib/server/v2/templates.js
View file

@ -21,16 +21,23 @@
* that stops the WeasyPrint SSRF/local-file-read); this page is the client that
* draws the one write a worklist should own.
*
* THE ONE WRITE: SET DEFAULT, THEN A SECOND: CREATE
* THE WRITES: SET DEFAULT, CREATE, EDIT
* `is_default` is a singleton, the model clears it on every other row inside a
* transaction, so making a template the default is a swap, not a toggle.
* Creating a template is the second write, `createInvoiceTemplate` below.
* Editing one stays out of scope: the detail GET uses the same stripping
* serializer as the list, so an edit form could never pre-fill the raw markup,
* and that gap is left as an honest, disabled control rather than a silent
* dead button (see the list page). `template_html` / `template_css` are also
* left out of the create form on purpose, for the same reason: see
* `CREATE_FIELDS` below.
* transaction, so making a template the default is a swap, not a toggle. It is
* owned by the list page's "Use instead of" control and deliberately absent
* from the edit form, so that saving an unrelated change can never demote the
* org's default by submitting an unchecked box.
*
* Editing used to be impossible rather than merely unwired: the detail GET
* answers with the same stripping serializer as the list, so a form could not
* pre-fill the two fields it exists to change, and saving would have blanked
* them. `GET /invoices/templates/<id>/editor/` now serves the raw markup on its
* own admin-only route, which is what `getInvoiceTemplateForEdit` reads. The
* detail and list responses are unchanged and still strip both fields.
*
* The markup is bound to `<textarea>` values, never to `{@html}`. That rule is
* what makes serving it safe, and it is the one thing a future edit to these
* pages must not break.
*
* TOTALS ARE COMPUTED HERE
* The list endpoint has no `totals` envelope. `count` comes from the response;
@ -39,6 +46,7 @@
* templates, and that ceiling is called out where it is set.
*/
import { apiRequest } from '$lib/api-helpers.js';
import { viewerRole } from '$lib/server/v2/organization.js';
const num = (/** @type {any} */ value) => {
const n = Number(value);
@ -121,16 +129,17 @@ export async function setDefaultTemplate({ cookies }, id) {
const HEX_COLOR = /^#[0-9a-fA-F]{6}$/;
/**
* Every field this form is allowed to write.
* Every field the create form is allowed to write.
*
* `template_html` and `template_css` are deliberately absent even though
* `InvoiceTemplateCreateSerializer` accepts them. Every read serializer strips
* them on purpose and substitutes `has_custom_html` / `has_custom_css` /
* `custom_html_bytes`, and the detail GET uses that same stripping serializer,
* so a value written here could never be read back or edited afterwards. It
* also feeds the WeasyPrint PDF renderer. A write-once, never-readable markup
* field is a trap, so this form does not offer one. Never render either value
* with `{@html}`.
* `template_html` and `template_css` are absent because a new template starts
* from the built-in layout: replacing the whole document, line-item table and
* totals included, is a deliberate follow-up on an existing template, not part
* of naming a new one. The edit form owns those two fields (`UPDATE_FIELDS`),
* where the saved markup can be shown alongside the change.
*
* That reasoning used to be different and is worth not re-deriving: they were
* excluded because every read serializer stripped them, so a value written
* here could never be read back. The editor route fixed that.
*/
export const CREATE_FIELDS = [
'name',
@ -142,10 +151,30 @@ export const CREATE_FIELDS = [
'is_default'
];
/**
* Every field the edit form is allowed to write.
*
* `is_default` is deliberately not here. It is a singleton the model swaps
* inside a transaction, and it has its own control on the list page. Reading it
* from an edit form means an unchecked box rides along with every unrelated
* save, which on the org's current default would quietly demote it and leave
* the org with none.
*/
export const UPDATE_FIELDS = [
'name',
'primary_color',
'secondary_color',
'default_notes',
'default_terms',
'footer_text',
'template_html',
'template_css'
];
/**
* Create an invoice template.
*
* Admin-only server-side: `_forbid_non_admin_template_write` 403s anyone whose
* Admin-only server-side: `_forbid_non_admin_template` 403s anyone whose
* `profile.role` is not ADMIN and who is not a superuser. The page hides the
* control using `can_manage`, but that is a display hint decoded from the JWT;
* the backend re-derives the role and is the check that matters.
@ -217,3 +246,100 @@ export async function createInvoiceTemplate({ cookies }, values, logoFile = null
);
return resp.template ?? resp;
}
/**
* Load one template for the edit form, including the raw markup.
*
* Refuses a non-admin up front with `can_edit: false`, the same shape
* `getProductForEdit` uses, so the form is never drawn for someone the PUT
* would refuse and the markup is not even requested. That is a display
* decision; the editor route re-derives the role server-side and 403s anyway.
*
* This is the only call in the app that receives `template_html` /
* `template_css`, and it reads the dedicated editor route rather than the
* detail route, so nothing here widens what an ordinary member's page fetches.
*
* The two markup fields come back as plain strings for a `<textarea>` value.
* Binding either into `{@html}` would turn a PDF setting into stored XSS, which
* is the whole reason every other read strips them.
*
* @param {{ cookies: import('@sveltejs/kit').Cookies }} event
* @param {string} id
*/
export async function getInvoiceTemplateForEdit({ cookies }, id) {
if (viewerRole(cookies) !== 'ADMIN') return { can_edit: false };
const t = await apiRequest(`/invoices/templates/${id}/editor/`, {}, { cookies });
return {
can_edit: true,
template: {
id: t.id,
name: t.name ?? '',
primary_color: t.primary_color || '#3B82F6',
secondary_color: t.secondary_color || '#1E40AF',
default_notes: t.default_notes ?? '',
default_terms: t.default_terms ?? '',
footer_text: t.footer_text ?? '',
template_html: t.template_html ?? '',
template_css: t.template_css ?? '',
is_default: Boolean(t.is_default),
has_logo: Boolean(t.logo)
}
};
}
/**
* Save an edit. `PUT /invoices/templates/<id>/` is already `partial=True` and
* admin-gated on the server; this sends only the fields the form owns.
*
* Clearing the markup is a real edit, so an empty string is sent rather than
* dropped: omitting it under `partial=True` would mean "leave it alone", and an
* admin who selects the markup and deletes it expects it gone. The empty-colour
* skip from `createInvoiceTemplate` is kept for the opposite reason: neither
* colour field is `blank=True`, so `''` is a 400 rather than a clear.
*
* @param {{ cookies: import('@sveltejs/kit').Cookies }} event
* @param {string} id
* @param {Record<string, any>} values
* @param {File | null} [logoFile]
*/
export async function updateInvoiceTemplate({ cookies }, id, values, logoFile = null) {
const name = (values.name ?? '').toString().trim();
if (!name) throw new Error('Give the template a name.');
for (const key of ['primary_color', 'secondary_color']) {
const colour = values[key];
if (colour !== undefined && colour !== '' && !HEX_COLOR.test(String(colour))) {
throw new Error('Colours must be a six digit hex value, for example #3B82F6.');
}
}
/** @type {Record<string, any>} */
const fields = {};
for (const key of UPDATE_FIELDS) {
if (values[key] === undefined) continue;
if ((key === 'primary_color' || key === 'secondary_color') && values[key] === '') continue;
fields[key] = key === 'name' ? name : values[key];
}
if (logoFile) {
const body = new FormData();
for (const [key, value] of Object.entries(fields)) {
body.append(key, typeof value === 'boolean' ? String(value) : value);
}
body.append('logo', logoFile);
const resp = await apiRequest(
`/invoices/templates/${id}/`,
{ method: 'PUT', body },
{ cookies }
);
return resp.template ?? resp;
}
const resp = await apiRequest(
`/invoices/templates/${id}/`,
{ method: 'PUT', body: fields },
{ cookies }
);
return resp.template ?? resp;
}

157
frontend/src/lib/server/v2/templates.test.js
View file

@ -5,7 +5,13 @@ vi.mock('$lib/api-helpers.js', () => ({
apiRequest: (/** @type {any[]} */ ...args) => apiRequest(...args)
}));
const { createInvoiceTemplate, CREATE_FIELDS } = await import('./templates.js');
const {
createInvoiceTemplate,
getInvoiceTemplateForEdit,
updateInvoiceTemplate,
CREATE_FIELDS,
UPDATE_FIELDS
} = await import('./templates.js');
const cookies = /** @type {any} */ ({});
@ -113,3 +119,152 @@ describe('createInvoiceTemplate', () => {
expect(result).toEqual({ id: 't1', name: 'Clean' });
});
});
/**
* `getInvoiceTemplateForEdit`'s admin gate comes from `viewerRole`, which
* decodes the `role` claim straight out of the `jwt_access` cookie with no
* network call. A `{ get: () => 'token' }` stub cannot drive it: `'token'` has
* no `.` to split on, so `viewerRole` catches that and returns null and every
* test would see a non-admin. This builds a JWT-shaped string with a real
* base64url payload so the decode path actually runs.
*
* @param {string} role
*/
function eventWithRole(role) {
const payload = Buffer.from(JSON.stringify({ role }), 'utf-8').toString('base64url');
const token = `h.${payload}.s`;
return /** @type {any} */ ({
cookies: { get: (/** @type {string} */ name) => (name === 'jwt_access' ? token : null) }
});
}
const SAVED = {
id: 't1',
name: 'House style',
primary_color: '#111111',
secondary_color: '#222222',
default_notes: 'notes',
default_terms: 'terms',
footer_text: 'footer',
template_html: '<h1>{{ invoice.number }}</h1>',
template_css: 'h1{color:red}',
is_default: true,
logo: '/media/logo.png'
};
describe('getInvoiceTemplateForEdit', () => {
beforeEach(() => {
apiRequest.mockReset();
apiRequest.mockResolvedValue(SAVED);
});
it('reads the dedicated editor route, not the detail route', async () => {
await getInvoiceTemplateForEdit(eventWithRole('ADMIN'), 't1');
const [url] = apiRequest.mock.calls[0];
expect(url).toBe('/invoices/templates/t1/editor/');
});
it('returns the raw markup, which is the whole point of the route', async () => {
const data = await getInvoiceTemplateForEdit(eventWithRole('ADMIN'), 't1');
expect(data.can_edit).toBe(true);
expect(data.template.template_html).toBe(SAVED.template_html);
expect(data.template.template_css).toBe(SAVED.template_css);
});
it('refuses a non-admin without fetching anything', async () => {
const data = await getInvoiceTemplateForEdit(eventWithRole('USER'), 't1');
expect(data).toEqual({ can_edit: false });
expect(apiRequest).not.toHaveBeenCalled();
});
it('refuses a caller with no readable role', async () => {
const data = await getInvoiceTemplateForEdit(
/** @type {any} */ ({ cookies: { get: () => null } }),
't1'
);
expect(data).toEqual({ can_edit: false });
expect(apiRequest).not.toHaveBeenCalled();
});
it('carries every field the form has an input for', async () => {
const { template } = await getInvoiceTemplateForEdit(eventWithRole('ADMIN'), 't1');
for (const key of UPDATE_FIELDS) {
expect(template[key]).toBeDefined();
}
});
});
describe('updateInvoiceTemplate', () => {
beforeEach(() => {
apiRequest.mockReset();
apiRequest.mockResolvedValue({ id: 't1' });
});
it('PUTs to the template detail route', async () => {
await updateInvoiceTemplate({ cookies }, 't1', { name: 'Clean' });
const [url, options] = apiRequest.mock.calls[0];
expect(url).toBe('/invoices/templates/t1/');
expect(options.method).toBe('PUT');
});
it('does send the markup, unlike create', async () => {
await updateInvoiceTemplate(
{ cookies },
't1',
{ name: 'Clean', template_html: '<h1>x</h1>', template_css: 'h1{}' }
);
const [, options] = apiRequest.mock.calls[0];
expect(options.body.template_html).toBe('<h1>x</h1>');
expect(options.body.template_css).toBe('h1{}');
});
it('sends empty markup rather than dropping it, so clearing works', async () => {
await updateInvoiceTemplate({ cookies }, 't1', { name: 'Clean', template_html: '' });
const [, options] = apiRequest.mock.calls[0];
expect(options.body.template_html).toBe('');
});
it('never sends is_default, which would demote the org default', async () => {
expect(UPDATE_FIELDS).not.toContain('is_default');
await updateInvoiceTemplate({ cookies }, 't1', { name: 'Clean', is_default: false });
const [, options] = apiRequest.mock.calls[0];
expect(options.body.is_default).toBeUndefined();
});
it('drops server-derived keys', async () => {
await updateInvoiceTemplate({ cookies }, 't1', {
name: 'Clean',
org: 'attacker-org',
id: 'forged'
});
const [, options] = apiRequest.mock.calls[0];
expect(options.body.org).toBeUndefined();
expect(options.body.id).toBeUndefined();
});
it('requires a name', async () => {
await expect(updateInvoiceTemplate({ cookies }, 't1', { name: ' ' })).rejects.toThrow();
expect(apiRequest).not.toHaveBeenCalled();
});
it('rejects a colour that is not a hex value', async () => {
await expect(
updateInvoiceTemplate({ cookies }, 't1', { name: 'Clean', primary_color: 'purple' })
).rejects.toThrow();
expect(apiRequest).not.toHaveBeenCalled();
});
it('omits an empty colour, which the model would 400 on', async () => {
await updateInvoiceTemplate({ cookies }, 't1', { name: 'Clean', primary_color: '' });
const [, options] = apiRequest.mock.calls[0];
expect(options.body.primary_color).toBeUndefined();
});
it('sends multipart when a logo is supplied', async () => {
const file = new File(['x'], 'logo.png', { type: 'image/png' });
await updateInvoiceTemplate({ cookies }, 't1', { name: 'Clean' }, file);
const [, options] = apiRequest.mock.calls[0];
expect(options.body).toBeInstanceOf(FormData);
expect(options.body.get('logo')).toBe(file);
});
});

34
frontend/src/routes/(app)/invoices/templates/+page.svelte
View file

@ -121,33 +121,15 @@
{#if canManage}
<div style="display:flex;gap:7px;margin-top:14px;flex-wrap:wrap;align-items:center">
<!-- Disabled on purpose, not hidden: a control that quietly
does nothing is the defect this phase fixes elsewhere.
The REAL reason edit is out of scope: the detail GET
(`GET /invoices/templates/<id>/`) answers with
<!-- Live since `GET /invoices/templates/<id>/editor/` landed.
This button was disabled for a real reason, not an
unfinished one: the detail GET answers with
`InvoiceTemplateListSerializer`, same as the list, which
never returns `template_html` or `template_css` (see
templates.js). An edit form here could not pre-fill the
custom markup, so it could not show what is actually
saved. This is a data-shape gap, not a missing endpoint:
`PUT /invoices/templates/<id>/` exists, is admin-gated,
and is already `partial=True`
(backend/invoices/api_views.py:1639), so editing every
OTHER field, name, colours, logo, notes, terms, footer,
is_default, would need no backend change at all.
The reason is also rendered as visible text next to the
button, not only in `title`: a disabled control is out of
the tab order and its title is not announced by
assistive tech. -->
<button
class="v2-btn v2-btn-sm"
disabled
title="Editing isn't wired yet. The detail view never returns the custom HTML/CSS, so a form here couldn't show what's already saved. Every other field could be edited without a backend change."
>
Edit
</button>
<span class="v2-sub" style="font-size:11px">Editing isn't wired yet</span>
never returns `template_html` or `template_css`, so a form
could not show what was saved and saving would have
blanked it. The editor route serves those two fields on an
admin-only path, leaving this page's own fetch unchanged. -->
<a class="v2-btn v2-btn-sm" href="/invoices/templates/{t.id}/edit">Edit</a>
{#if !t.is_default}
<!-- Named as the swap it is: one default exists at a time.
`is_default` is a singleton the model enforces, so this

81
frontend/src/routes/(app)/invoices/templates/[id]/edit/+page.server.js Normal file
View file

@ -0,0 +1,81 @@
import { fail, redirect, error } from '@sveltejs/kit';
import {
getInvoiceTemplateForEdit,
updateInvoiceTemplate
} from '$lib/server/v2/templates.js';
import { readableError } from '$lib/server/v2/form-errors.js';
/**
* Editing an invoice template.
*
* Admin-only, enforced server-side twice over: `getInvoiceTemplateForEdit`
* returns `can_edit: false` for a non-admin without fetching anything, and the
* editor route it would have called is admin-gated in the API. The fetch is
* org-scoped, so a foreign or unknown id 404s.
*
* This page is the reason `GET /invoices/templates/<id>/editor/` exists: every
* other read strips `template_html` / `template_css`, so before it a form here
* could not show what was saved, and saving would have blanked it.
*
* @type {import('./$types').PageServerLoad}
*/
export async function load(event) {
try {
return await getInvoiceTemplateForEdit(event, event.params.id);
} catch (/** @type {any} */ err) {
if (err?.status === 404) error(404, 'Template not found');
throw err;
}
}
/**
* Read only the fields this form owns.
*
* `is_default` is not among them on purpose. It is a singleton the model swaps
* inside a transaction and it has its own control on the list page; reading it
* here would send an unchecked box along with every unrelated save and demote
* the org's current default.
*
* @param {FormData} form
*/
function readValues(form) {
return {
name: form.get('name')?.toString() ?? '',
primary_color: form.get('primary_color')?.toString() ?? '',
secondary_color: form.get('secondary_color')?.toString() ?? '',
default_notes: form.get('default_notes')?.toString() ?? '',
default_terms: form.get('default_terms')?.toString() ?? '',
footer_text: form.get('footer_text')?.toString() ?? '',
template_html: form.get('template_html')?.toString() ?? '',
template_css: form.get('template_css')?.toString() ?? ''
};
}
/** @type {import('./$types').Actions} */
export const actions = {
save: async (event) => {
const form = await event.request.formData();
const logo = form.get('logo');
const logoFile = logo instanceof File && logo.size > 0 ? logo : null;
const values = readValues(form);
if (!values.name.trim()) {
return fail(400, { values, error: 'Give the template a name.' });
}
try {
await updateInvoiceTemplate(event, event.params.id, values, logoFile);
} catch (/** @type {any} */ err) {
if (err?.status === 403) {
return fail(403, {
values,
error: 'Only an administrator can change invoice templates.'
});
}
if (err?.status === 404) error(404, 'Template not found');
return fail(400, { values, error: readableError(err, 'Could not save this template.') });
}
redirect(303, '/invoices/templates');
}
};

203
frontend/src/routes/(app)/invoices/templates/[id]/edit/+page.svelte Normal file
View file

@ -0,0 +1,203 @@
<script>
/**
* Editing an invoice template: the brand fields, the boilerplate text, and
* the two markup fields no other page in this app is allowed to receive.
*
* THE MARKUP RULE. `template_html` and `template_css` are bound to
* `<textarea>` values and nothing else. A textarea's value is text, never
* parsed as markup, which is what makes showing them safe here. `{@html}`
* appears nowhere in this app and must never appear on this page: these two
* fields are org-authored HTML that WeasyPrint renders into a PDF
* server-side, so putting either into the DOM turns a PDF setting into
* stored XSS. Every other read path strips them for exactly that reason;
* this page reads the dedicated admin-only editor route instead.
*
* `is_default` is absent on purpose. One template holds it at a time and the
* list page owns the swap ("Use instead of ..."), so an unchecked box here
* cannot ride along with an unrelated save and leave the org with no default.
*
* VALIDATION HERE IS A UX HINT. `PUT /api/invoices/templates/<id>/` is
* admin-gated server-side and enforces that regardless of what this page
* draws; curl and the mobile client never pass through here.
*/
import { enhance } from '$app/forms';
import PageHeader from '$lib/v2/components/PageHeader.svelte';
import { ChevronRight, Lock, FileCode } from '@lucide/svelte';
/** @type {{ data: any, form: any }} */
let { data, form } = $props();
// A failed save re-renders with what was typed; a fresh load uses the saved
// record. Without the fallback an admin loses their edits on any 400.
let values = $derived(form?.values ?? data.template ?? {});
</script>
<PageHeader title={data.template?.name ?? 'Edit template'} record center width="72ch">
{#snippet crumb()}
<a href="/invoices/templates">Templates</a>
<ChevronRight size={12} />
<span>Edit</span>
{/snippet}
</PageHeader>
<div class="v2-scroll">
{#if !data.can_edit}
<div class="v2-pad" style="padding-top:24px;max-width:56ch;margin-left:auto;margin-right:auto">
<div class="v2-next" role="note">
<Lock size={17} style="flex:none" />
<div class="v2-next-body">
<div style="font-weight:600">Admins only</div>
<div class="v2-sub" style="margin-top:2px">
A template decides how every invoice in the org prints, so only an administrator can
change one. You can still see the catalogue on the templates page.
</div>
</div>
</div>
<a class="v2-btn" href="/invoices/templates" style="margin-top:16px">Back to templates</a>
</div>
{:else}
<form
method="POST"
action="?/save"
enctype="multipart/form-data"
use:enhance
class="v2-pad"
style="padding-top:18px;padding-bottom:36px;max-width:72ch;margin-left:auto;margin-right:auto"
>
{#if form?.error}
<p style="color:var(--v2-rust);font-size:12.5px;margin:0 0 14px" role="alert">
{form.error}
</p>
{/if}
<label class="v2-field">
<span class="v2-label">Name</span>
<input class="v2-input" name="name" required maxlength="100" value={values.name ?? ''} />
</label>
<div class="color-row">
<label class="color-field">
<span class="v2-label">Primary colour</span>
<input
class="color-swatch"
type="color"
name="primary_color"
value={values.primary_color || '#3B82F6'}
/>
</label>
<label class="color-field">
<span class="v2-label">Secondary colour</span>
<input
class="color-swatch"
type="color"
name="secondary_color"
value={values.secondary_color || '#1E40AF'}
/>
</label>
</div>
<label class="v2-field">
<span class="v2-label">
Logo <span class="opt">{data.template?.has_logo ? '(replaces the current one)' : '(optional)'}</span>
</span>
<input class="v2-input" type="file" name="logo" accept="image/*" />
</label>
<label class="v2-field">
<span class="v2-label">Default notes <span class="opt">(optional)</span></span>
<textarea class="v2-input" name="default_notes" rows="3">{values.default_notes ?? ''}</textarea>
</label>
<label class="v2-field">
<span class="v2-label">Default terms <span class="opt">(optional)</span></span>
<textarea class="v2-input" name="default_terms" rows="3">{values.default_terms ?? ''}</textarea>
</label>
<label class="v2-field">
<span class="v2-label">Footer text <span class="opt">(optional)</span></span>
<textarea class="v2-input" name="footer_text" rows="2">{values.footer_text ?? ''}</textarea>
</label>
<div class="markup-note" role="note">
<FileCode size={14} style="flex:none;margin-top:2px" />
<div>
<strong>Custom layout</strong>
<div class="v2-sub" style="margin-top:2px">
Filling these in replaces the whole printed document, the line-item table and the totals
included. Nothing checks that what you write still prints an amount due. Leave both empty
to use the built-in layout. Emptying a field that has markup in it now clears it.
</div>
</div>
</div>
<label class="v2-field">
<span class="v2-label">Template HTML <span class="opt">(optional)</span></span>
<textarea class="v2-input mono" name="template_html" rows="10" spellcheck="false"
>{values.template_html ?? ''}</textarea
>
</label>
<label class="v2-field">
<span class="v2-label">Template CSS <span class="opt">(optional)</span></span>
<textarea class="v2-input mono" name="template_css" rows="6" spellcheck="false"
>{values.template_css ?? ''}</textarea
>
</label>
{#if data.template?.is_default}
<p class="v2-sub" style="font-size:11.5px;margin:0 0 16px">
This is the org default, so these changes apply to every new invoice. Which template is the
default is changed from the templates page, not here.
</p>
{/if}
<div style="display:flex;gap:9px;margin-top:6px">
<button class="v2-btn v2-btn-primary" type="submit">Save changes</button>
<a class="v2-btn" href="/invoices/templates">Cancel</a>
</div>
</form>
{/if}
</div>
<style>
.opt {
text-transform: none;
font-weight: 500;
letter-spacing: 0;
color: var(--v2-slate);
}
.color-row {
display: flex;
gap: 16px;
}
.color-field {
display: flex;
flex-direction: column;
gap: 6px;
}
.color-swatch {
width: 56px;
height: 38px;
padding: 3px;
border: 1px solid var(--v2-line);
border-radius: 8px;
background: var(--v2-card);
cursor: pointer;
}
.markup-note {
display: flex;
gap: 9px;
align-items: flex-start;
font-size: 12.5px;
padding: 11px 12px;
margin: 4px 0 16px;
border: 1px solid var(--v2-line);
border-radius: 9px;
background: var(--v2-card);
}
.mono {
font-family: ui-monospace, SFMono-Regular, Menlo, monospace;
font-size: 12px;
line-height: 1.55;
}
</style>

2
frontend/src/routes/(app)/invoices/templates/new/+page.server.js
View file

@ -6,7 +6,7 @@ import { readableError } from '$lib/server/v2/form-errors.js';
* Creating a template.
*
* Admin-only: `POST /api/invoices/templates/` refuses a non-admin via
* `_forbid_non_admin_template_write`, so this mirrors the products/new and
* `_forbid_non_admin_template`, so this mirrors the products/new and
* goals/new pattern: `load` computes `can_manage` from `locals.profile.role`
* (same source the list page's `load` already uses) so a non-admin who
* navigates here directly sees an "admins only" state instead of a form the

2
frontend/src/routes/(app)/invoices/templates/new/+page.svelte
View file

@ -12,7 +12,7 @@
* full reasoning.
*
* VALIDATION HERE IS A UX HINT, NOT A RULE. `POST /api/invoices/templates/`
* is admin-gated (`_forbid_non_admin_template_write`) and enforces that
* is admin-gated (`_forbid_non_admin_template`) and enforces that
* regardless of what this page shows; curl and the mobile client reach the
* API without passing through here. The two colour inputs use
* `type="color"` so the browser can only ever submit a valid six-digit hex

27
frontend/src/routes/(app)/settings/tags/+page.svelte
View file

@ -38,16 +38,17 @@
let totals = $derived(data.totals);
/**
* Sums the four models the backend's `_TAGGABLE` list
* (`backend/common/views/tags_views.py:22-27`) counts: accounts, leads,
* opportunities (deals) and cases (tickets). `Contact` and `Task` also
* declare `tags = ManyToManyField(Tags, ...)` and accept tag writes through
* their own APIs, plus the contacts CSV importer, but `_TAGGABLE` does not
* include them, so this total, and the "unused" totals card below, are
* partial: a tag can carry real usage on contacts or tasks and still show
* as unused here.
* Total usage across every model a tag can be applied to.
*
* Summed over the keys the backend sends rather than a list written out
* here, so it cannot drift from `_TAGGABLE`
* (`backend/common/views/tags_views.py`) the way the old hard-coded four
* did: contacts, tasks and API settings were all missing, so a tag in real
* use on contacts read as unused both here and on the totals card. A test
* on the backend now walks the model registry to keep that list complete;
* this sums whatever it sends.
*/
const used = (t) => t.usage.accounts + t.usage.leads + t.usage.opportunities + t.usage.cases;
const used = (t) => Object.values(t.usage ?? {}).reduce((sum, n) => sum + (n || 0), 0);
let tags = $derived(
[...data.tags].sort((a, b) => used(b) - used(a) || a.name.localeCompare(b.name))
@ -178,9 +179,11 @@
<tr>
<th>Tag</th>
<th style="text-align:right">Accounts</th>
<th style="text-align:right">Contacts</th>
<th style="text-align:right">Leads</th>
<th style="text-align:right">Deals</th>
<th style="text-align:right">Tickets</th>
<th style="text-align:right">Tasks</th>
<th style="text-align:right">Total</th>
<th></th>
{#if data.can_edit}<th></th>{/if}
@ -198,9 +201,11 @@
<span class="v2-sub" style="font-size:11px;margin-left:7px">{t.color}</span>
</td>
<td class="v2-num" style="text-align:right">{t.usage.accounts || '—'}</td>
<td class="v2-num" style="text-align:right">{t.usage.contacts || '—'}</td>
<td class="v2-num" style="text-align:right">{t.usage.leads || '—'}</td>
<td class="v2-num" style="text-align:right">{t.usage.opportunities || '—'}</td>
<td class="v2-num" style="text-align:right">{t.usage.cases || '—'}</td>
<td class="v2-num" style="text-align:right">{t.usage.tasks || '—'}</td>
<td class="v2-num" style="text-align:right;font-weight:600">{used(t) || '—'}</td>
<td style="text-align:right">
{#if !t.is_active}
@ -223,8 +228,8 @@
label="Turn off"
confirmLabel="Turn off"
explain={used(t) > 0
? `${used(t)} ${used(t) === 1 ? 'account, lead, deal or ticket keeps' : 'accounts, leads, deals and tickets keep'} this tag (contacts and tasks can carry it too, but are not counted here). Turning it off stops it being offered on new records. You can turn it back on.`
: 'No account, lead, deal or ticket carries this tag. Contacts and tasks can also carry tags but are not counted here, so this is not the full picture. Turning it off stops it being offered on new records. You can turn it back on.'}
? `${used(t)} ${used(t) === 1 ? 'record keeps' : 'records keep'} this tag. Turning it off stops it being offered on new records, and keeps it on the ones that have it. You can turn it back on.`
: 'Nothing carries this tag. Turning it off stops it being offered on new records. You can turn it back on.'}
hidden={{ id: t.id }}
/>
{:else}