KAN-321 Fix NULL-only text failures in UAT provisioning - #505
Conversation
Nothing in DocketWorks ever writes Staff.last_login. Authentication is JWT-only: SIMPLE_JWT leaves UPDATE_LAST_LOGIN at its False default, and CustomTokenObtainPairSerializer authenticates via authenticate(), which never emits the user_logged_in signal that Django's update_last_login receiver listens for. django.contrib.admin is not installed either, so there is no session-login path. The inherited AbstractBaseUser column has been NULL by construction. Admin > Staff nonetheless rendered a permanently blank "Last Login" column, and StaffFormModal round-tripped the value back to the server with no input bound to it. Worse, StaffCreateSerializer listed only wage_rate as read-only, so POST /api/accounts/staff/ accepted a client-supplied last_login -- an API client could forge a login time. Remove last_login from STAFF_API_FIELDS and from the admin staff table (fixing the empty-state colspan, which was already off by one), and drop the dead last_login/date_joined round-trip from StaffFormModal. The DB column stays; it belongs to AbstractBaseUser. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018CvGn5ojk7NtQzr5WmzAEi
|
Warning Review limit reached
Next review available in: 32 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/workflow/api/xero/transforms.py`:
- Line 1012: Remove the missing-attribute fallback in the account description
mappings: use the required description attribute directly in
apps/workflow/api/xero/transforms.py lines 1012-1012 and
apps/workflow/management/commands/seed_xero_from_database.py lines 214-214,
replacing getattr(account, "description", None) with direct account.description
access.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a0ca138b-5c74-4546-91ec-c993001ed0d7
⛔ Files ignored due to path filters (1)
frontend/src/api/generated/api.tsis excluded by!**/generated/**
📒 Files selected for processing (13)
apps/accounts/models.pyapps/accounts/serializers.pyapps/accounts/tests/test_staff_api.pyapps/workflow/api/xero/transforms.pyapps/workflow/management/commands/seed_xero_from_database.pyapps/workflow/tests/test_seed_xero_from_database.pyapps/workflow/tests/test_xero_instance_templates.pyfrontend/schema.ymlfrontend/src/components/StaffFormModal.vuefrontend/src/views/AdminStaffView.vuemypy-baseline.txtscripts/server/instance.shscripts/server/templates/phone-provider-settings.json.template
💤 Files with no reviewable changes (4)
- apps/accounts/serializers.py
- frontend/src/components/StaffFormModal.vue
- frontend/schema.yml
- mypy-baseline.txt
`getattr(account, "description", None)` and `getattr(account, "enable_payments_to_account", False)` both imply the attribute might be absent. Neither ever is: both are declared in `Account.openapi_types` and initialised in the xero-python constructor, so the defaults are unreachable. Verified against all 166 XeroAccount rows — both keys present in every raw payload, and `enable_payments_to_account` is never null (148 False / 18 True). Pure dead-code removal, no behaviour change. The sibling fields in the same dict already use direct access (`account.type or None`), so this also removes an internal inconsistency. `or None` is retained on `description` — 57 of 166 accounts come back from Xero with `""`, and that clause is what turns them into NULL. It is deliberately not added to `enable_payments`: coercing a null to False would mask a genuinely surprising value, where the NOT NULL constraint fails early instead. Raised by CodeRabbit on PR #505. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016TZsTtMB6opqWJFwzrUteK
1 participant
What changed
nullininstance.sh, rather than an empty string.Nonein both regular account sync andseed_xero_from_database.Root cause
The new database checks make
NULLthe only valid representation of unset nullable text, but two provisioning writers bypassing serializers still emitted empty strings:instance.sh reconfigurequoted absent phone credentials in its JSON fixture.Impact
A scrubbed UAT restore can now be reconfigured with a disabled/unconfigured phone provider, and demo Xero seeding can ingest accounts whose optional fields are empty. Configured non-empty values are unchanged.
Validation
bash -n scripts/server/instance.shJira: https://docketworks.atlassian.net/browse/KAN-321
Summary by CodeRabbit
Bug Fixes
null, including when clearing existing descriptions.Tests