fix(workload): work settings form shows defaults after reload - #110
Merged
Merged
Conversation
The Work settings page copied `workSettings` into its draft when `!isLoading && !hasHydrated`. `isLoading` starts false and the effect runs in the same commit pass as the hook's fetch effect, so it latched DEFAULT_WORK_SETTINGS before the GET resolved and never re-armed — after a reload the form showed defaults although the PUT had persisted. - `useWorkSettings` exposes `hasLoaded` (true only after a successful GET for the current slug); the page hydrates on that instead. - Reset the hydration latch when the workspace slug changes. - Lock inputs and Save until hydrated, so a save can no longer overwrite real settings with defaults during load or after a GET error. PLANE-195 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LuJp4jZbzT6GT5u1Ykeyw4
tuha263
added a commit
that referenced
this pull request
Sep 11, 2026
…rm fix (#110) (#111) * feat(workspace_ext): public API to create a workspace (POST /api/v1/workspaces/) (#108) * feat(workspace_ext): public API to create a workspace (POST /api/v1/workspaces/) The public API could list the workspaces a key could reach (#29) but never mint one, so an API-key client (plane-mcp-server, a t1k bootstrap, a CI job) had to stop and have a human create the slug through god-mode. This adds the write-side counterpart. Home is workspace_ext, not project_ext as the issue proposed: project_ext is scoped to projects, and workspace_ext already exists on master, is already mounted before core plane.api.urls, and is already in forkApps — so this is append-only with no new app, no registry edit and no migration. Authorization is InstanceAdminPermission (option (a) in the issue). A self-hosted single-tenant instance has no tenant boundary, so "any authenticated user may create a workspace" would let a key for a workspace member mint sibling workspaces; this matches what the UI enforces. Behaviour mirrors WorkSpaceViewSet.create so API- and UI-created workspaces are indistinguishable. Two deliberate divergences, both because a machine consumer cannot act on what core returns: a duplicate slug is a real 409 (core's serializer UniqueValidator short-circuits it to a 400 first), and the 403 bodies carry an error_code instead of DRF's opaque {"detail": ...}. Closes #107 Claude-Session: https://claude.ai/code/session_01LEq97wvWgXTHKhayQkWXbM * fix(workspace_ext): whitelist create-workspace body, atomic create, tighten tests Apply the review findings on PR #108. B1 (blocker) — mass assignment. The view handed request.data straight to WorkSpaceSerializer, which is core and declares `fields = "__all__"` with a read_only_fields list that omits deleted_at, logo, logo_asset, timezone and background_color. A POSTed `deleted_at` committed a 201 row that SoftDeletionManager hides from every default-manager query — including the view's own 409 pre-check and the serializer's UniqueValidator — while the database's unique index on slug still held, so the slug was permanently unusable with no API path to recover it. logo_asset was a second hole (an unscoped FK to any FileAsset on the instance). The serializer is now fed from an explicit whitelist {name, slug, organization_size} and any other key is a 400 error_code UNEXPECTED_FIELDS. Core serializer untouched. S1 — transaction boundary. serializer.save() and WorkspaceMember.objects.create() now run inside transaction.atomic(), and both .delay() dispatches moved to transaction.on_commit(..., robust=True) so a broker outage cannot 500 a committed create. The generic 409 WORKSPACE_CREATE_CONFLICT catch-all is deleted: a non-slug IntegrityError now propagates instead of being reported as a client-side conflict the caller cannot act on. S2 — test_url_in_name_returns_400 was vacuous: the serializer rejects the same input, so the view's own contains_url guard could be deleted with the test still green. It now asserts error_code WORKSPACE_NAME_CONTAINS_URL. S3 — test_is_active_false_api_key_never_authenticates asserted a 403 that is indistinguishable from INSTANCE_ADMIN_REQUIRED. It now asserts the DRF AuthenticationFailed body, which has no error_code. The docstring's mechanism was also inverted: APIKeyAuthentication does not override authenticate_header, so the 403 exists because no header is produced; the 401 on the anonymous path comes from the fork's own exception handler. S4 — the docs/FORK.md contract fence had no gate asserting it matches CREATE_WORKSPACE_CONTRACT. Added tests that extract the fence from the real docs/FORK.md, dedent it and compare byte-for-byte, plus a derived check that every error_code the view can emit is documented. UNEXPECTED_FIELDS added to both copies. S5 — the role__gte=15 boundary was untested (every test used role 20). Added role 15 -> 201 and roles 14/10/5 -> 403 INSTANCE_ADMIN_REQUIRED. Negative assertions across the file now read through Workspace.all_objects, the unfiltered manager — the default manager is exactly what hides the B1 bug. Disclosed deviation, not requested by the review: the `WorkspaceMember.objects .create(...)` call no longer passes `company_role=request.data.get("company_role", "")`. That key is not one of the three the contract allows, so the whitelist now rejects any request carrying it and the read was unreachable. Dropping the argument leaves the column at its model default (None) rather than ""; the field is TextField(null=True, blank=True) and both render as "no job title". Claude-Session: https://claude.ai/code/session_01LEq97wvWgXTHKhayQkWXbM * fix(workload): hydrate work settings form after GET resolves (#110) The Work settings page copied `workSettings` into its draft when `!isLoading && !hasHydrated`. `isLoading` starts false and the effect runs in the same commit pass as the hook's fetch effect, so it latched DEFAULT_WORK_SETTINGS before the GET resolved and never re-armed — after a reload the form showed defaults although the PUT had persisted. - `useWorkSettings` exposes `hasLoaded` (true only after a successful GET for the current slug); the page hydrates on that instead. - Reset the hydration latch when the workspace slug changes. - Lock inputs and Save until hydrated, so a save can no longer overwrite real settings with defaults during load or after a GET error. PLANE-195 Claude-Session: https://claude.ai/code/session_01LuJp4jZbzT6GT5u1Ykeyw4 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Nguyễn Mạnh <75465287+frostbun@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Symptom
After editing Work settings (daily hour cap, workdays, week start) and reloading the page, the form showed
DEFAULT_WORK_SETTINGS(8h, Mon–Fri, Monday) even though the PUT had persisted the real values — the saved settings were gone from the form until a hard refetch, and a save in that state would have written the defaults back over the workspace's real settings.Root cause
The Work settings page (
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/(workspace)/workload/page.tsx) copiedworkSettingsinto its local draft inside an effect gated on!isLoading && !hasHydrated. ButisLoadinginuseWorkSettingsstartsfalseand only flipstrueinside the hook's async fetch — so the page's effect ran in the same commit pass as the hook's fetch effect, sawisLoading === false, latchedDEFAULT_WORK_SETTINGSinto the draft, sethasHydrated = true, and never re-armed when the GET actually resolved.Fix
useWorkSettingsexposeshasLoaded(true only after a successful GET for the current slug); the page hydrates on that instead.Verification
pnpm check:types— exit 0.-D react-hooks/exhaustive-depson both files — 0 errors.apps/web.PLANE-195
🤖 Generated with Claude Code
https://claude.ai/code/session_01LuJp4jZbzT6GT5u1Ykeyw4