Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,11 @@ instance/
# Sphinx documentation
docs/_build/

# Planning artefacts (Claude Code plan-mode files; local-only)
docs/plans/
# Planning artefacts (Claude Code plan-mode files; local-only).
# The pattern must match the contents, not the directory: git never descends
# into an excluded directory, which would leave the negation unreachable.
docs/plans/*
!docs/plans/_template.md

# PyBuilder
target/
Expand Down Expand Up @@ -275,3 +278,6 @@ session-replays/
docs/.codesight/
frontend/.codesight/
*.tsv

# Local state for scripts/write_google_doc.py (per-instance runtime data)
scripts/google_doc_manifest.json
27 changes: 0 additions & 27 deletions .vscode/tasks.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,32 +27,6 @@
"panel": "dedicated"
}
},
{
"label": "Frontend Manual Dev Server",
"type": "process",
"command": "${env:HOME}/.nvm/nvm-exec",
"args": ["npm", "run", "manual:dev"],
"hide": true,
"options": {
"cwd": "${workspaceFolder}/frontend"
},
"isBackground": true,
"problemMatcher": {
"owner": "vitepress",
"pattern": {
"regexp": "^$"
},
"background": {
"activeOnStart": true,
"beginsPattern": "vitepress",
"endsPattern": "http://localhost"
}
},
"presentation": {
"reveal": "always",
"panel": "dedicated"
}
},
{
"label": "Ngrok Tunnels",
"type": "shell",
Expand Down Expand Up @@ -123,7 +97,6 @@
"label": "Start Dev Environment",
"dependsOn": [
"Frontend Dev Server",
"Frontend Manual Dev Server",
"Ngrok Tunnels",
"Celery Worker",
"Celery Beat"
Expand Down
13 changes: 9 additions & 4 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,15 @@ ADJUSTMENT entries (kind='adjust'):
fixes when that is the pragmatic path, but direct-to-main commits are banned.
- Before committing, check the current branch. If it is `main`, create or switch
to a branch first.
- Do not leave uncommitted changes behind at the end of a task. If the change is
complete and scoped, commit it on the current branch. If the scope is unclear,
mixed with unrelated work, or the user may not want it committed, ask before
committing.
- Commits are all-or-nothing for the worktree: either do not commit at all, or
commit every tracked change together. Never use a path-limited commit.
- Do not leave uncommitted changes behind at the end of a task. Finish and
verify incomplete work before committing the whole worktree.
- `docs/plans/` is ephemeral scratch — one plan per piece of work, gitignored
(except `_template.md`). Delete a plan when its PR is opened, having first
migrated anything durable to its real home: open work → the Jira ticket, tools
Comment thread
coderabbitai[bot] marked this conversation as resolved.
→ `scripts/`, decisions → an ADR. Never leave a non-plan artifact (script, data
file) sitting in `docs/plans/` — it goes through that same migrate-or-delete gate.
- Run focused tests for touched code when useful. Do not manually run expensive hook commands like `bash scripts/check_mypy.sh`, `npm run test:unit`, `npm run lint`, `npm run type-check`, or frontend builds unless diagnosing a hook failure; they run automatically during `git commit`/`git push`.
- Tests must protect enduring behaviour, invariants, or algorithms. Never assert the implementation's own text — `assertIn` on source code, a CLI flag or log string, or source line ordering — which mirrors the code, breaks on every refactor, and catches no bug. Execute the code path and assert the observable outcome: return value, exit code, output, or resulting state.

Expand Down
28 changes: 28 additions & 0 deletions apps/accounts/tests/test_staff_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,31 @@ def test_staff_list_prefetches_groups_for_serializer(self):
and "accounts_staff_groups" in query["sql"].lower()
]
self.assertEqual(len(group_queries), 1)


class StaffDetailAPIViewTests(BaseTestCase):
def test_staff_cannot_be_deleted_via_api(self) -> None:
"""Staff are offboarded by setting date_left, never deleted. The detail
endpoint must reject DELETE so a hard delete (which would orphan or be
blocked by protected time entries) can't be reintroduced."""
office_user = Staff.objects.create_user(
email="office@example.test",
password="testpass",
first_name="Office",
last_name="User",
is_office_staff=True,
)
target = Staff.objects.create_user(
email="leaver@example.test",
password="testpass",
first_name="Depa",
last_name="Rting",
)

client = APIClient()
client.force_authenticate(user=office_user)

response = client.delete(f"/api/accounts/staff/{target.id}/")

self.assertEqual(response.status_code, 405)
self.assertTrue(Staff.objects.filter(pk=target.id).exists())
4 changes: 2 additions & 2 deletions apps/accounts/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from apps.accounts.views.password_views import SecurityPasswordChangeView
from apps.accounts.views.staff_api import (
StaffListCreateAPIView,
StaffRetrieveUpdateDestroyAPIView,
StaffRetrieveUpdateAPIView,
)
from apps.accounts.views.staff_views import (
StaffListAPIView,
Expand Down Expand Up @@ -39,7 +39,7 @@
path("staff/", StaffListCreateAPIView.as_view(), name="api_staff_list_create"),
path(
"staff/<uuid:pk>/",
StaffRetrieveUpdateDestroyAPIView.as_view(),
StaffRetrieveUpdateAPIView.as_view(),
name="api_staff_detail",
),
]
4 changes: 2 additions & 2 deletions apps/accounts/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from django.apps import apps

if apps.ready:
from .staff_api import StaffListCreateAPIView, StaffRetrieveUpdateDestroyAPIView
from .staff_api import StaffListCreateAPIView, StaffRetrieveUpdateAPIView
except (ImportError, RuntimeError):
# Django not ready or circular import, skip conditional imports
pass
Expand All @@ -23,6 +23,6 @@
"SecurityPasswordChangeView",
"StaffListAPIView",
"StaffListCreateAPIView",
"StaffRetrieveUpdateDestroyAPIView",
"StaffRetrieveUpdateAPIView",
"get_staff_rates",
]
22 changes: 13 additions & 9 deletions apps/accounts/views/staff_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,11 @@ def post(self, request, *args, **kwargs):


@extend_schema(
summary="Retrieve, update, or delete staff member",
description="API endpoint for retrieving, updating, and deleting individual staff members. "
"Supports GET (retrieve), PUT/PATCH (update), and DELETE operations. "
"Includes comprehensive logging for update operations and handles multipart/form data for file uploads.",
summary="Retrieve or update staff member",
description="API endpoint for retrieving and updating individual staff members. "
"Supports GET (retrieve) and PUT/PATCH (update). "
"Includes comprehensive logging for update operations and handles multipart/form data for file uploads. "
"Staff are not deleted; offboarding is done by setting date_left.",
tags=["Staff Management"],
examples=[
OpenApiExample(
Expand All @@ -110,12 +111,15 @@ def post(self, request, *args, **kwargs):
),
],
)
class StaffRetrieveUpdateDestroyAPIView(generics.RetrieveUpdateDestroyAPIView):
"""API endpoint for retrieving, updating, and deleting individual staff members.
class StaffRetrieveUpdateAPIView(generics.RetrieveUpdateAPIView[Staff]):
"""API endpoint for retrieving and updating individual staff members.

Supports GET (retrieve), PUT/PATCH (update), and DELETE operations on
specific staff members. Includes comprehensive logging for update operations
and handles multipart/form data for file uploads.
Supports GET (retrieve) and PUT/PATCH (update) on specific staff members.
Includes comprehensive logging for update operations and handles
multipart/form data for file uploads.

Staff are never deleted (their time entries are protected); offboarding is
done by setting date_left.
"""

queryset = Staff.objects.all()
Expand Down
12 changes: 12 additions & 0 deletions apps/job/tests/_pdf_golden_fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

from apps.accounts.models import Staff
from apps.company.models import Company, Person
from apps.crm.models import PhoneEndpoint
from apps.job.models import CostLine, Job, JobEvent, JobFile, LabourSubtype
from apps.workflow.models import CompanyDefaults, XeroPayItem

Expand Down Expand Up @@ -73,6 +74,17 @@ def build_golden_job(test_staff: Staff) -> Job:
company.starting_job_number = STARTING_JOB_NUMBER
company.save()

# The letterhead prints the shop's main-line number
# (workshop_pdf_service._primary_company_endpoint_number). Create it here
# so PDF output stays byte-identical regardless of which seed fixtures a
# caller happens to load — this builder is the single source of truth for
# every field that influences the rendered PDF.
PhoneEndpoint.objects.create(
number="+6496365131",
label="Main line",
endpoint_type=PhoneEndpoint.EndpointType.MAIN_LINE,
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.

company = Company.objects.create(
name="ACME Engineering Ltd",
xero_last_modified=FROZEN_NOW,
Expand Down
41 changes: 14 additions & 27 deletions apps/quoting/services/ai_price_extraction.py
Original file line number Diff line number Diff line change
@@ -1,51 +1,38 @@
import abc
import logging
from typing import Any, Dict, Optional, Tuple

from apps.workflow.enums import AIProviderTypes

from .providers.gemini_provider import GeminiPriceExtractionProvider
from .providers.base import PriceExtractionProvider
from .providers.gemini_provider import (
GEMINI_FLASH_MODEL,
GeminiPriceExtractionProvider,
)

# from .providers.claude_provider import ClaudePriceExtractionProvider
from .providers.mistral_provider import MistralPriceExtractionProvider
from .providers.mistral_provider import (
MISTRAL_OCR_MODEL,
MistralPriceExtractionProvider,
)

logger = logging.getLogger(__name__)


class PriceExtractionProvider(abc.ABC):
"""Abstract base class for AI price extraction providers."""

provider_name: str

@abc.abstractmethod
def extract_price_data(
self, file_path: str, content_type: Optional[str] = None
) -> Tuple[Optional[Dict[str, Any]], Optional[str]]:
"""
Extract price data from a supplier price list file.

Args:
file_path: Path to the price list file
content_type: MIME type of the file

Returns:
Tuple containing extracted data dict and error message if any
"""


class PriceExtractionFactory:
"""Factory for creating AI price extraction providers."""

@staticmethod
def create_provider(
provider_type: str, api_key: str, model_name: str = None
provider_type: str, api_key: str, model_name: str | None = None
) -> PriceExtractionProvider:
"""Create a provider instance based on type."""
if provider_type == AIProviderTypes.MISTRAL:
return MistralPriceExtractionProvider(api_key)
return MistralPriceExtractionProvider(
api_key, model_name or MISTRAL_OCR_MODEL
)
elif provider_type == AIProviderTypes.GOOGLE:
return GeminiPriceExtractionProvider(
api_key, model_name or "gemini-2.0-flash-exp"
api_key, model_name or GEMINI_FLASH_MODEL
)
# elif provider_type == AIProviderTypes.ANTHROPIC:
# return ClaudePriceExtractionProvider(api_key)
Expand Down
6 changes: 3 additions & 3 deletions apps/quoting/services/pdf_data_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ class PDFDataValidationService:
and duplicate detection for supplier products.
"""

def __init__(self):
self.validation_errors = []
self.warnings = []
def __init__(self) -> None:
self.validation_errors: List[str] = []
self.warnings: List[str] = []

def validate_extracted_data(
self, data: Dict[str, Any]
Expand Down
24 changes: 24 additions & 0 deletions apps/quoting/services/providers/base.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import abc
from typing import Any, Dict, Optional, Tuple
Comment thread
coderabbitai[bot] marked this conversation as resolved.


class PriceExtractionProvider(abc.ABC):
"""Abstract base class for AI price extraction providers."""

provider_name: str
model_name: str

@abc.abstractmethod
def extract_price_data(
self, file_path: str, content_type: Optional[str] = None
) -> Tuple[Optional[Dict[str, Any]], Optional[str]]:
"""
Extract price data from a supplier price list file.

Args:
file_path: Path to the price list file
content_type: MIME type of the file

Returns:
Tuple containing extracted data dict and error message if any
"""
Comment thread
coderabbitai[bot] marked this conversation as resolved.
15 changes: 10 additions & 5 deletions apps/quoting/services/providers/gemini_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,25 +12,28 @@

from apps.workflow.models import CompanyDefaults

from .base import PriceExtractionProvider
from .common import clean_json_response, create_extraction_prompt, log_token_usage

logger = logging.getLogger(__name__)

GEMINI_FLASH_MODEL = "gemini-flash-latest"

class GeminiPriceExtractionProvider:

class GeminiPriceExtractionProvider(PriceExtractionProvider):
"""Gemini AI provider for price extraction from PDF documents."""

provider_name = "Gemini"

def __init__(self, api_key: str, model_name: str = "gemini-2.5-flash"):
def __init__(self, api_key: str, model_name: str = GEMINI_FLASH_MODEL):
self.api_key = api_key
self.model_name = model_name

def extract_price_data(
self, file_path: str, content_type: Optional[str] = None
) -> Tuple[Optional[Dict[str, Any]], Optional[str]]:
"""
Extract price data from a supplier price list PDF using Gemini 2.5 Flash.
Extract price data from a supplier price list PDF using Gemini Flash.

Args:
file_path: Path to the PDF file
Expand Down Expand Up @@ -140,7 +143,7 @@ def _process_extracted_data(
"total_lines": len(str(raw_data).split("\n")),
"items_found": len(processed_items),
"pages_processed": 1, # Gemini processes the entire PDF at once
"extraction_method": "Gemini 2.5 Flash",
"extraction_method": f"Gemini ({self.model_name})",
},
}

Expand Down Expand Up @@ -420,7 +423,9 @@ def _extract_from_multiple_pages(
"total_lines": len(str(all_items).split("\n")),
"items_found": len(all_items),
"pages_processed": num_pages,
"extraction_method": "Gemini 2.5 Flash (Page-by-page)",
"extraction_method": (
f"Gemini ({self.model_name}, page-by-page)"
),
},
}
logger.info(
Expand Down
Loading
Loading