diff --git a/apps/accounts/models.py b/apps/accounts/models.py index fbe85628f..0a96d2ad3 100644 --- a/apps/accounts/models.py +++ b/apps/accounts/models.py @@ -103,7 +103,6 @@ class Staff(AbstractBaseUser, PermissionsMixin): "date_joined", "created_at", "updated_at", - "last_login", "groups", "user_permissions", ] @@ -111,6 +110,10 @@ class Staff(AbstractBaseUser, PermissionsMixin): # Internal fields not exposed via API (write-only or internal use). # `icon` is deliberately absent: it is written only by the dedicated icon # upload endpoint and read via the icon_url property. + # `last_login` is deliberately absent: authentication is JWT-only + # (`SIMPLE_JWT` leaves `UPDATE_LAST_LOGIN` at its `False` default and + # `authenticate()` never emits `user_logged_in`), so the inherited + # `AbstractBaseUser` column is never written and has no meaning to expose. STAFF_INTERNAL_FIELDS = [ "password", ] diff --git a/apps/accounts/serializers.py b/apps/accounts/serializers.py index efb545edf..a2869732a 100644 --- a/apps/accounts/serializers.py +++ b/apps/accounts/serializers.py @@ -92,7 +92,6 @@ class Meta: read_only_fields = [ "id", "wage_rate", - "last_login", "date_joined", "created_at", "updated_at", diff --git a/apps/accounts/tests/test_staff_api.py b/apps/accounts/tests/test_staff_api.py index b9fe6d174..0fd6e1c33 100644 --- a/apps/accounts/tests/test_staff_api.py +++ b/apps/accounts/tests/test_staff_api.py @@ -137,6 +137,33 @@ def test_create_leaves_a_new_staff_member_active(self) -> None: self.assertIsNone(created.date_left) self.assertTrue(created.is_currently_active) + def test_create_ignores_a_client_supplied_last_login(self) -> None: + """`last_login` is not part of the write contract. + + Authentication is JWT-only and nothing ever stamps the inherited + `AbstractBaseUser` column, so an API client must not be able to forge a + login time. The create serializer once listed only `wage_rate` as + read-only, which let this value through. + """ + response = self.client_api.post( + "/api/accounts/staff/", + { + "email": "forger@example.test", + "first_name": "For", + "last_name": "Ger", + "password": "TestPassword123!", + "base_wage_rate": 32.5, + "date_left": None, + "last_login": "2026-07-01T09:00:00Z", + }, + format="json", + ) + + self.assertEqual(response.status_code, 201, response.content) + self.assertNotIn("last_login", response.json()) + created = Staff.objects.get(email="forger@example.test") + self.assertIsNone(created.last_login) + def test_setting_date_left_offboards_a_staff_member(self) -> None: target = Staff.objects.create_user( email="leaving@example.test", diff --git a/apps/workflow/api/xero/transforms.py b/apps/workflow/api/xero/transforms.py index 4d12e43cb..e9cc92769 100644 --- a/apps/workflow/api/xero/transforms.py +++ b/apps/workflow/api/xero/transforms.py @@ -1,5 +1,6 @@ import logging import time +from collections.abc import Iterable from datetime import date, datetime from datetime import timezone as dt_timezone from decimal import Decimal @@ -7,6 +8,7 @@ from django.utils import timezone from xero_python.accounting import AccountingApi +from xero_python.accounting.models import Account from apps.accounting.models import Bill, CreditNote, Invoice, Quote from apps.company.models import Company @@ -999,20 +1001,18 @@ def sync_companies(xero_contacts): return companies -def sync_accounts(xero_accounts): +def sync_accounts(xero_accounts: Iterable[Account]) -> None: """Sync Xero accounts""" for account in xero_accounts: XeroAccount.objects.update_or_create( xero_id=account.account_id, defaults={ - "account_code": account.code, + "account_code": account.code or None, "account_name": account.name, - "description": getattr(account, "description", None), - "account_type": account.type, - "tax_type": account.tax_type, - "enable_payments": getattr( - account, "enable_payments_to_account", False - ), + "description": account.description or None, + "account_type": account.type or None, + "tax_type": account.tax_type or None, + "enable_payments": account.enable_payments_to_account, "xero_last_modified": account._updated_date_utc, "xero_last_synced": timezone.now(), "raw_json": process_xero_data(account), diff --git a/apps/workflow/management/commands/seed_xero_from_database.py b/apps/workflow/management/commands/seed_xero_from_database.py index a21b39ac5..9debe6573 100644 --- a/apps/workflow/management/commands/seed_xero_from_database.py +++ b/apps/workflow/management/commands/seed_xero_from_database.py @@ -178,7 +178,7 @@ def handle(self, *args, **options): CompanyDefaults.set_xero_sync_enabled(enabled=True) self.stdout.write("Xero seeding complete! enable_xero_sync is now True.") - def process_accounts(self, dry_run): + def process_accounts(self, dry_run: bool) -> int: """Phase 0: Update XeroAccount xero_ids from prod to dev Xero tenant. The backup includes XeroAccount records with prod xero_id values. @@ -210,13 +210,11 @@ def process_accounts(self, dry_run): account_name=account.name, defaults={ "xero_id": account.account_id, - "account_code": account.code, - "description": getattr(account, "description", None), - "account_type": account.type, - "tax_type": account.tax_type, - "enable_payments": getattr( - account, "enable_payments_to_account", False - ), + "account_code": account.code or None, + "description": account.description or None, + "account_type": account.type or None, + "tax_type": account.tax_type or None, + "enable_payments": account.enable_payments_to_account, "xero_last_modified": account._updated_date_utc, "xero_last_synced": None, "raw_json": process_xero_data(account), diff --git a/apps/workflow/tests/test_seed_xero_from_database.py b/apps/workflow/tests/test_seed_xero_from_database.py index 19d38e247..931a2ce39 100644 --- a/apps/workflow/tests/test_seed_xero_from_database.py +++ b/apps/workflow/tests/test_seed_xero_from_database.py @@ -10,14 +10,18 @@ """ from io import StringIO -from unittest.mock import patch +from types import SimpleNamespace +from unittest.mock import Mock, patch +from uuid import uuid4 from django.conf import settings from django.test import TestCase from django.utils import timezone from apps.company.models import Company +from apps.workflow.api.xero.transforms import sync_accounts from apps.workflow.management.commands.seed_xero_from_database import Command +from apps.workflow.models import XeroAccount SENTINEL_XERO_CONTACT_ID = "11111111-1111-1111-1111-111111111111" @@ -44,3 +48,81 @@ def test_refuses_when_db_name_ends_with_prod(self) -> None: company.refresh_from_db() self.assertEqual(company.xero_contact_id, SENTINEL_XERO_CONTACT_ID) + + +class XeroAccountSeedTests(TestCase): + @patch( + "apps.workflow.api.xero.transforms.process_xero_data", + return_value={}, + ) + def test_sync_normalises_empty_optional_text( + self, _process_xero_data_mock: Mock + ) -> None: + sync_accounts( + [ + SimpleNamespace( + account_id=uuid4(), + code="", + name="Uncoded account", + description="", + type="", + tax_type="", + enable_payments_to_account=False, + _updated_date_utc=timezone.now(), + ) + ] + ) + + account = XeroAccount.objects.get(account_name="Uncoded account") + self.assertIsNone(account.account_code) + self.assertIsNone(account.description) + self.assertIsNone(account.account_type) + self.assertIsNone(account.tax_type) + + @patch( + "apps.workflow.management.commands.seed_xero_from_database.process_xero_data", + return_value={}, + ) + @patch( + "apps.workflow.management.commands.seed_xero_from_database.get_tenant_id", + return_value="demo-tenant", + ) + @patch("apps.workflow.management.commands.seed_xero_from_database.AccountingApi") + def test_empty_xero_description_is_stored_as_null( + self, + accounting_api_mock: Mock, + _tenant_id_mock: Mock, + _process_xero_data_mock: Mock, + ) -> None: + prod_xero_id = uuid4() + demo_xero_id = uuid4() + modified_at = timezone.now() + XeroAccount.objects.create( + xero_id=prod_xero_id, + account_code="805", + account_name="Accrued Liabilities", + description="Production description", + account_type="CURRLIAB", + tax_type="NONE", + xero_last_modified=modified_at, + raw_json={}, + ) + accounting_api_mock.return_value.get_accounts.return_value.accounts = [ + SimpleNamespace( + account_id=demo_xero_id, + code="805", + name="Accrued Liabilities", + description="", + type="CURRLIAB", + tax_type="NONE", + enable_payments_to_account=False, + _updated_date_utc=modified_at, + ) + ] + + command = Command() + command.process_accounts(dry_run=False) + + account = XeroAccount.objects.get(account_name="Accrued Liabilities") + self.assertEqual(account.xero_id, demo_xero_id) + self.assertIsNone(account.description) diff --git a/apps/workflow/tests/test_xero_instance_templates.py b/apps/workflow/tests/test_xero_instance_templates.py index 24e14c20c..d5a5f1e3c 100644 --- a/apps/workflow/tests/test_xero_instance_templates.py +++ b/apps/workflow/tests/test_xero_instance_templates.py @@ -1,4 +1,5 @@ import json +import os import subprocess import tempfile from decimal import Decimal @@ -10,11 +11,12 @@ from django.test import SimpleTestCase, TestCase from apps.accounts.models import Staff -from apps.crm.models import PhoneEndpoint +from apps.crm.models import PhoneEndpoint, PhoneProviderSettings from apps.workflow.models import CompanyDefaults, XeroApp REPO_ROOT = Path(__file__).resolve().parents[3] COMMON_SCRIPT = REPO_ROOT / "scripts" / "server" / "common.sh" +INSTANCE_SCRIPT = REPO_ROOT / "scripts" / "server" / "instance.sh" XERO_APPS_TEMPLATE = ( REPO_ROOT / "scripts" / "server" / "templates" / "xero-apps.json.template" ) @@ -135,9 +137,9 @@ def test_phone_provider_settings_template_renders_to_valid_json(self) -> None: "__PHONE_PROVIDER_BASE_URL_JSON__", '"http://phone-provider.lan"', ) - .replace("__PHONE_PROVIDER_USERNAME__", "phone-user") - .replace("__PHONE_PROVIDER_PASSWORD__", "phone-secret") - .replace("__PHONE_PROVIDER_ACCOUNT_CODE__", "15539090") + .replace("__PHONE_PROVIDER_USERNAME_JSON__", '"phone-user"') + .replace("__PHONE_PROVIDER_PASSWORD_JSON__", '"phone-secret"') + .replace("__PHONE_PROVIDER_ACCOUNT_CODE_JSON__", '"15539090"') ) payload = json.loads(rendered) @@ -237,3 +239,53 @@ def run(creds_file: str) -> "subprocess.CompletedProcess[str]": link = base / "linkdir" link.symlink_to(base) self.assertNotEqual(run(str(link / "inst.credentials.env")).returncode, 0) + + +class PhoneProviderReconfigureFixtureTests(TestCase): + def test_unconfigured_fixture_loads_with_null_credentials(self) -> None: + """Reconfigure must seed a scrubbed instance without violating constraints.""" + PhoneProviderSettings.objects.all().delete() + PhoneProviderSettings.get_solo() + + with tempfile.TemporaryDirectory() as instance_dir: + environment = os.environ.copy() + for variable in ( + "PHONE_PROVIDER_BASE_URL", + "PHONE_PROVIDER_USERNAME", + "PHONE_PROVIDER_PASSWORD", + "PHONE_PROVIDER_ACCOUNT_CODE", + "PHONE_PROVIDER_DOWNLOADS_ENABLED", + "PHONE_PROVIDER_RECORDING_DELETION_ENABLED", + ): + environment.pop(variable, None) + + subprocess.run( + [ + "bash", + "-c", + ( + 'source "$1"; ' + "log() { :; }; " + "chown() { :; }; " + 'render_phone_provider_settings_fixture "$2" ignored' + ), + str(INSTANCE_SCRIPT.parent / "test-harness"), + str(INSTANCE_SCRIPT), + instance_dir, + ], + check=True, + capture_output=True, + text=True, + env=environment, + ) + + fixture = Path(instance_dir) / ".fixtures" / "phone_provider_settings.json" + call_command("loaddata", str(fixture), verbosity=0) + + settings = PhoneProviderSettings.objects.get(pk=1) + self.assertFalse(settings.downloads_enabled) + self.assertFalse(settings.recording_deletion_enabled) + self.assertIsNone(settings.base_url) + self.assertIsNone(settings.username) + self.assertIsNone(settings.password) + self.assertIsNone(settings.account_code) diff --git a/frontend/schema.yml b/frontend/schema.yml index e372b6b50..94c814cf9 100644 --- a/frontend/schema.yml +++ b/frontend/schema.yml @@ -21719,11 +21719,6 @@ components: type: string format: date-time readOnly: true - last_login: - type: string - format: date-time - readOnly: true - nullable: true groups: type: array items: @@ -21746,7 +21741,6 @@ components: - first_name - icon_url - id - - last_login - last_name - updated_at - wage_rate @@ -21864,10 +21858,6 @@ components: created_at: type: string format: date-time - last_login: - type: string - format: date-time - nullable: true groups: type: array items: diff --git a/frontend/src/api/generated/api.ts b/frontend/src/api/generated/api.ts index 011dc2658..2f53ab219 100644 --- a/frontend/src/api/generated/api.ts +++ b/frontend/src/api/generated/api.ts @@ -466,7 +466,6 @@ const Staff = z.object({ date_joined: z.string().datetime({ offset: true }), created_at: z.string().datetime({ offset: true }), updated_at: z.string().datetime({ offset: true }), - last_login: z.string().datetime({ offset: true }).nullable(), groups: z.array(z.number().int()).optional(), user_permissions: z.array(z.number().int()).optional(), icon_url: z.string().nullable(), @@ -492,7 +491,6 @@ const StaffCreateRequest = z.object({ hours_sun: z.number().gt(-100).lt(100).optional(), date_joined: z.string().datetime({ offset: true }).optional(), created_at: z.string().datetime({ offset: true }).optional(), - last_login: z.string().datetime({ offset: true }).nullish(), groups: z.array(z.number().int()).optional(), user_permissions: z.array(z.number().int()).optional(), password: z.string().min(1).max(128), diff --git a/frontend/src/components/StaffFormModal.vue b/frontend/src/components/StaffFormModal.vue index 89fa11562..25c6e54f0 100644 --- a/frontend/src/components/StaffFormModal.vue +++ b/frontend/src/components/StaffFormModal.vue @@ -406,8 +406,6 @@ const form = ref({ is_superuser: false, groups: '', user_permissions: '', - last_login: '', - date_joined: '', date_left: '', }) const error = ref('') @@ -465,8 +463,6 @@ watch( Array.isArray(staff.user_permissions) && staff.user_permissions.length > 0 ? staff.user_permissions.join(', ') : '', - last_login: staff.last_login || '', - date_joined: staff.date_joined || '', date_left: staff.date_left || '', } } else { @@ -491,8 +487,6 @@ watch( is_superuser: false, groups: '', user_permissions: '', - last_login: '', - date_joined: '', date_left: '', } } @@ -518,8 +512,6 @@ async function submitForm() { // password and its confirmation, which must never reach the browser console. // Prepare base data - shared between validation and API call - const lastLogin = normalizeOptionalString(form.value.last_login) - const dateJoined = normalizeOptionalString(form.value.date_joined) const preferredName = normalizeOptionalString(form.value.preferred_name) const xeroUserId = normalizeOptionalString(form.value.xero_user_id) // date_left is always sent (null when blank) so an offboarded staff member @@ -561,8 +553,6 @@ async function submitForm() { // Handle optional fields - only include if they have a value ...(preferredName && { preferred_name: preferredName }), ...(xeroUserId && { xero_user_id: xeroUserId }), - ...(lastLogin && { last_login: lastLogin }), - ...(dateJoined && { date_joined: dateJoined }), } // Add password if provided diff --git a/frontend/src/views/AdminStaffView.vue b/frontend/src/views/AdminStaffView.vue index bf4393e74..4276f5d57 100644 --- a/frontend/src/views/AdminStaffView.vue +++ b/frontend/src/views/AdminStaffView.vue @@ -43,7 +43,6 @@ Last Name Is Office Staff Is SuperUser - Last Login Date Joined Actions @@ -78,7 +77,6 @@ ✔️ - {{ formatDateTime(staff.last_login) }} {{ formatDateTime(staff.date_joined) }}