Polish settings, profile, and credentials UX#4641
Draft
Conversation
Replaces the global unique index on collections.name with a per-project (project_id, name) index, allowing sandboxes to share collection names with their parent. Updates the schema constraint to match and adds conflict detection to get_collection/1, plus a new get_collection/2 that takes a project_id for unambiguous lookup.
Adds a :conflict clause to FallbackController so that when get_collection/1 finds the same collection name in multiple projects, all actions return a 409 with a message directing clients to use the v2 API.
Fixes the create_collection test to match the new constraint name (collections_project_id_name_index). Adds tests for get_collection/1 returning :conflict when the same name exists in multiple projects, and for get_collection/2 resolving unambiguously by project_id. Adds controller tests asserting all v1 endpoints return 409 when a name conflict exists.
Replaces the explicit collections routes with a single wildcard that dispatches to v1 or v2 logic based on the X-API-VERSION header. V2 routes include project_id in the path, resolving collections unambiguously without name-collision risk. Also migrates the download endpoint to v2 (/:project_id/:name) and updates the LiveView download link accordingly.
Covers all v2 endpoints (GET item, stream, PUT, POST, DELETE item, DELETE all), including access control with personal and run tokens, and the key scenario where v2 resolves correctly by project_id when the same collection name exists in multiple projects.
Adds clone_collections_from_parent/2 to the sandbox provisioning pipeline. When a sandbox is forked, empty collection records matching each parent collection name are created in the new project. Items are not copied. Uses on_conflict: :nothing so re-provisioning is safe.
Adds sync_collections/2, called after a successful merge. Collections present in the sandbox but not the parent are created (empty) in the parent. Collections present in the parent but not the sandbox are deleted from the parent along with all their items. Collections in both are left untouched. Data is never merged.
Extends the merge confirmation warning to explain that collection names will be synchronized: new sandbox collections are created empty in the target, and collections deleted in the sandbox are permanently removed from the target including all their data.
The single dispatch function had a cyclomatic complexity of 14 (max 9). Splitting by version brings each function to 7 branches.
Address review feedback on the sandbox collections work: - Add a plug that validates the x-api-version header early and returns 400 for unsupported values instead of silently falling back to v1 - Consolidate v1/v2 controller actions through a shared resolve/1 helper, dropping ~100 lines of duplication - Return 422 with a clear error on missing value/items in request body (previously raised FunctionClauseError) - Move sync_collections into the Sandboxes context, wrap creates and deletes in a single transaction, and replace the N delete_collection calls with a single batch delete_all - Scope the collection unique constraint error to :name instead of :project_id so the user-facing field reports the conflict - Tone down the merge warning copy while keeping the essential info Adds tests for unsupported/garbage x-api-version values, malformed request bodies, and transaction rollback behavior for sync_collections.
- Log a warning when collection sync fails after merge instead of silently ignoring the error - Remove on_conflict: :nothing from insert_empty_collections since both callers guarantee no duplicates can exist - Rename misleading test to match what it actually verifies
- Use project-scoped get_collection/2 in CollectionsComponent to prevent MatchError when collection names collide across projects - Guard migration rollback with duplicate name check so it fails with a clear message instead of a cryptic Postgres error - Make sync_collections failure fail the merge instead of silently logging, and skip the GitHub commit when collections fail to sync - Bind MapSet.difference outside the comprehension in sync_collections - Add v2 write-path test with run token to cover the primary worker path
Introduces LightningWeb.Plugs.VersionedRouter, a reusable behaviour for header-based API versioning. Instead of `match :*, "/*path"` in the Phoenix router, the collections scope now uses `forward` to a router plug that resolves the version and delegates to V1Routes or V2Routes modules with explicit pattern matching per version. The controller keeps only business logic; routing and param assembly live in the version-specific route modules. The VersionedRouter plug handles fallback controller integration so actions can still return error tuples.
Replaces the 4-file VersionedRouter macro abstraction with a single plug at plugs/collections_router.ex. Inlines the ApiVersion logic so version resolution, routing, and fallback handling all live in one place. The router uses forward instead of match :*.
The controller actions return error tuples from with chains (handled by the fallback controller), but their specs claimed Plug.Conn.t() only. This caused a pattern_match_cov warning in the CollectionsRouter plug. Broaden the return types to Plug.Conn.t() | term().
The full merge sequence (workflow import + collection sync) now lives in the Sandboxes context instead of the LiveView. Any caller gets collection sync automatically, not just the UI.
Introduces a small reusable banner component with three variants (Local, Editable, Inherited) that explains how settings on a tab will or won't flow on merge. Settings LiveView preloads the parent project and exposes both sandbox? and parent_project as assigns so the template can render the sandbox-specific UI.
Per Joe's per-page proposal in #3398: * project: Sandbox Identity panel with parent project link, hide Delete button, show Local banner * credentials, collections: Editable banner * webhook_security: replace auth methods table with explanatory message in sandboxes (V1 doesn't support webhook security in sandboxes) * collaboration: Local banner * security (MFA): Inherited banner, MFA toggle disabled * vcs, data-storage, history-exports: Local banner
Adds Sandboxes.parent_admin?/2 which walks the parent chain and returns true when the user is admin or owner on any ancestor project. Projects.delete_project_user!/1 now raises when removing such a user from a sandbox, so the protection holds even if the LiveView UI guard is bypassed.
LiveView tests cover the per-tab banners, Sandbox Identity panel, disabled MFA toggle, webhook security message, and the parent admin floor rule (UI guard + Projects.delete_project_user! enforcement). Regression tests in sandboxes_test.exs document that the merge pipeline does not propagate Local/Inherited project-level fields (requires_mfa, concurrency, retention, name, description, collaborators) from a sandbox to its parent. These guard against future MergeProjects or Provisioner changes that could accidentally start syncing these fields.
The ancestors/1 helper handled a parent project lookup returning nil, but the parent_id FK uses on_delete: :nilify_all so a stale parent_id can't exist. The LiveView's parent_admin?/2 had a second clause fetching a user by user_id when the user wasn't preloaded, but get_project_users!/1 always preloads :user. Both were defensive code for scenarios prevented by FK constraints. Removed per CLAUDE.md guidance not to handle scenarios that can't happen.
Two bugs from the previous "remove dead code" commit: 1. Sandboxes.ancestors/1 needs the nil clause back. The parent_id FK uses :nilify_all but that fires at delete-time in the database. An in-memory project struct loaded before the parent was deleted will still have the old parent_id. The nil clause handles this race gracefully instead of crashing the LiveView. Added a regression test that exercises this branch. 2. The handle_event remove path loaded the project_user without preloading :user, so removing the second parent_admin?/2 clause would have crashed it with FunctionClauseError. Preloading :user at the call site is more targeted than restoring the dead clause.
The view shouldn't reach into Repo. Adds an include opt to get_project_user!/2 so callers can request preloaded associations through the context (mirrors Ecto's :preload but named for the caller's intent). Drops the now-unused Repo alias in the settings LiveView.
Copy and styling (Joe's inline review):
- Drop "won't sync" / "can't be changed here" in favour of "do not sync"
/ "cannot be changed here" in the sandbox settings banner.
- Add a coloured border to each banner variant so they don't look ghosted.
- Reword the Sandbox Identity parent line to "Identifies this sandbox
within its parent: <PARENT>".
- Drop "for now" from the Webhook Security copy.
Banner placement:
- Move the banner under the tab's section_header across all sandbox
panels so the reading order is page -> section -> context -> content.
- For Collections, the section_header lives inside CollectionsComponent,
so pass `sandbox?` into the component and render the banner there.
Fix a misleading "Role based permissions" message:
- The Webhook Security and Security (MFA) sandbox branches were passing
`can_perform_action={false}` to section_header, which forced the role
message even for admins. The block isn't role-based in either case:
webhook security is categorically disabled, MFA is inherited.
- Make `permissions_message` on section_header optional (default nil).
When nil, the component does not render the role message even if
`can_perform_action` is false. Existing callers are unaffected.
- Webhook Security sandbox branch: render a regular content card
(matching other settings cards on the page) with accurate copy that
explains webhook auth methods are shared with and enforced on the
sandbox but only manageable from the parent. Avoids the misleading
"disabled" framing.
- Security sandbox branch: `can_perform_action={@can_edit_project}` with
`permissions_message` nilified in sandbox mode. Role message still
fires for viewers outside sandbox; in sandbox, the Inherited banner
and the disabled MFA toggle carry the meaning without double-messaging.
Tests:
- New regression: the sandbox settings page never renders the role
permissions line on the webhook_security or security tabs.
Unify the card pattern across panels, tighten labels, and wire the MFA
toggle through the shared <.input type="toggle"> component so every
on/off control on the page uses the same primitive.
- Shared toggle component now accepts %JS{} commands on on_click.
- Version switcher in workflow editor uses the shared toggle too.
- Sentence-case tab labels, panel titles, and sub-section headings.
- Security panel: MFA + webhook auth presented as sibling cards.
- Setup panel: identity/concurrency/export/danger zone cards.
- Data storage: each sub-setting in its own card, shared save footer.
- Danger zone: red-bordered card with "What gets deleted" detail.
- Exports tab title aligned with its tab label.
- Layout: reserve scrollbar gutter to stop horizontal jitter.
- Shared table container gets overflow-hidden to clip white tbody
corners inside the rounded outer ring.
Collapse profile sub-pages into a single-column card layout, delete the stale Components module, and rebuild the MFA enrolment flow around a GitHub-style 6-box OTP input with inline validation, error state, and explicit "Enable MFA" commit. - New OtpInput Phoenix hook: auto-advance, paste distribution, arrow navigation, backspace-to-previous, focus-select, autofocus, client-side clear on server-pushed otp:clear event. - MfaComponent: validate_totp pushed directly from the hook on each keystroke; save_totp failures clear the boxes and surface an inline "That code didn't match" message with red ring state. - update/2 now uses assign_new so editing_totp/totp_changeset/qrcode_uri survive parent re-renders; previously the secret could be wiped between showing the QR code and validating the user's input. - Profile page: single max-w-3xl column, unified card style across FormComponent (basic/email/password sections), MFA, GitHub, and Experimental features, with a red-bordered Danger zone at the bottom. - Backup codes page rewritten as three cards (MFA-disabled warning, codes + copy/print, regenerate) to match the new profile aesthetic.
Restructure the credentials index around self-contained sections with their own heading, subtitle, and primary action button; drop the shared "New" dropdown at the top of the panel in favour of per-section CTAs that match the GitHub/Stripe settings pattern. - Order: Credentials → Keychain credentials → OAuth clients. - Credentials and Keychain each get a "New credential" / "New keychain" button sized to the section; OAuth clients gets its own "New OAuth client" button. - Reused credentials table gains a display_table_title toggle so the inline table title can be suppressed when the section already has a heading above it. - Shared table container clips white tbody corners with overflow-hidden. - Column casing aligned: "Projects With Access" → "Projects with access". - Environments column rendered as a subtle chip with a globe icon + count so its meaning is legible without hovering. - Project access badges truncate at max-w-[12rem] with a native title tooltip so long project names no longer overflow. - Menu button label "Add new" → "New"; empty-state sentence case; typos in button ids (big-buttton → big-button) fixed.
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
2 participants
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.
Summary
Broad UX/styling polish across the Project Settings, Profile, and Credentials pages. Scoped structural + styling changes only — no behaviour changes, no table-system refactor (deferred to a later PR).
What's in here
Project settings (
/projects/:id/settings)bg-white p-4 rounded-mdcard pattern across Setup, Security, Data storage, and Danger zone sub-sections.<.input type=\"toggle\">used for the MFA and workflow version-switcher toggles (on_click now accepts%JS{}commands).<.table>getsoverflow-hiddenso white tbody corners are clipped inside the rounded outer ring.Profile (
/profile,/profile/auth/backup_codes)max-w-3xlcard layout; the orphan Components module is deleted.OtpInputPhoenix hook: auto-advance, paste distribution, arrow keys, backspace-to-previous, focus-select, autofocus.validate_totppushed from the hook on each keystroke;save_totpfailure clears the boxes and surfaces an inline "That code didn't match" message with a red ring.update/2usesassign_newso the secret survives parent re-renders (previously the QR-code secret could be wiped between showing and validating).Credentials (
/credentials, project settings → Credentials tab)New credential,New keychain,New OAuth client).max-w-[12rem]with a nativetitletooltip.Deferred
Test plan
mix test test/lightning_web/live/project_live_test.exsmix test test/lightning_web/live/project_live/sandbox_settings_test.exsmix test test/lightning_web/live/profile_live_test.exsmix test test/lightning_web/live/backup_codes_live_test.exsmix test test/lightning_web/live/credential_live_test.exs