feat(auth): collect the workspace logo on the sign-up creation step#21723
feat(auth): collect the workspace logo on the sign-up creation step#21723FelixMalfait wants to merge 10 commits into
Conversation
There was a problem hiding this comment.
4 issues found across 11 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Visual Regression Report (twenty-front)
Changed stories
|
778ae69 to
cf45463
Compare
|
🚀 Preview Environment Ready! Your preview environment is available at: https://athletes-prospects-northwest-betty.trycloudflare.com This environment will automatically shut down after 5 hours. |
Unify workspace creation through one form on the root domain (name + logo, plus subdomain in multi-workspace), removing the duplicate subdomain-side prompt and the on-the-fly nameless creation that could strand users mid-onboarding. - New scoped uploadNewWorkspaceLogo(workspaceId, file) mutation: the creator sets a logo on their PENDING_CREATION workspace via the workspace-agnostic token (membership enforced); upload size capped via settings.storage.maxFileSize (also applied to the existing logo / profile-picture uploads). - The creation form is shared by multi- and single-workspace self-host; the in-app "Create Workspace" entry now redirects to the root-domain creation form (?action=create-new-workspace) instead of creating a nameless workspace. - The onboarding step is a pure activation loader (Retry on failure); the name is always set by then, so the displayName guard is removed. Follow-up to #21641. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xw37hR5seiCyWnppG9z4op
cf45463 to
1149e99
Compare
🔍 Automated Pre-Review✅ No issues detected - This PR is ready for human review. Automated pre-review — human approval still required. |
There was a problem hiding this comment.
3 issues found across 22 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- Authorize the new-workspace logo upload before buffering the stream, so a rejected request never reads the untrusted file into memory (authorization split out of the upload service and reused on the resolver) - Gate workspace activation on the workspace being loaded, since the backend requires a non-empty display name to activate - Skip subdomain availability lookups in single-workspace mode where the address field is hidden Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xw37hR5seiCyWnppG9z4op
| 'create-new-workspace'; | ||
|
|
||
| if (availableWorkspacesCount === 0 || wantsToCreateNewWorkspace) { | ||
| // Both multi-workspace and single-workspace self-host now go through the |
There was a problem hiding this comment.
Remove this comment
| width: 100%; | ||
| `; | ||
|
|
||
| // The workspace name (and optional logo) are already collected in the shared |
There was a problem hiding this comment.
Remove comment. Should this step/file be renamed then?
…InUp Per review feedback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xw37hR5seiCyWnppG9z4op
Per review feedback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xw37hR5seiCyWnppG9z4op
| setNextOnboardingStatus, | ||
| ]); | ||
|
|
||
| // Activation sends the workspace name to the backend, so we wait for the |
There was a problem hiding this comment.
Remove comment. Also I don't understand why we still need to send the display name?
In single-workspace mode isOnAWorkspace is always true, so sign-up went to
signUpInWorkspace and created a workspace with no name, which then failed
activation ("'displayName' not provided"). Route no-existing-workspace sign-up
(including single-workspace) through the workspace-agnostic path so the user
lands on the creation form and the workspace is named before it is created.
Single-workspace has no subdomain to redirect to after creation, so authenticate
in place via the login token and let onboarding take over on the same domain.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xw37hR5seiCyWnppG9z4op
…visioning Enforce the invariant that a PENDING workspace always has a name: - signUpOnNewWorkspace now requires a non-empty displayName (only the user can supply a name; the subdomain is still auto-generated when omitted), so a nameless workspace can no longer be created. - SSO sign-ins with no target workspace/invitation now route through the same workspace-agnostic create-or-select flow as credentials, instead of auto-creating a workspace. - activateWorkspace no longer requires or rewrites displayName (the name is set at creation); the onboarding loader stops sending it. Renaming at activation remains available as an optional override. Integration util updated to name the workspace at creation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xw37hR5seiCyWnppG9z4op
|
Design notes for reviewers (keeping these here rather than as inline code comments): Naming — required at creation, not at activation. Invariant: a PENDING workspace always has a name. With naming enforced at creation, no path can create a nameless workspace, so the activation step can't be reached with an empty name (the original "Workspace creation failed" bug). Single-workspace sign-up routing. Logo upload. The picked file is held locally (object URL, revoked on unmount to avoid a blob leak) and uploaded only after the workspace exists; a logo failure is non-fatal. Generated by Claude Code |
Move the design rationale to the PR thread rather than inline code comments (per review preference), including a now-stale activation comment. Also collapse the duplicated isContinueDisabled ternary into a single expression and drop a redundant clearStepTimeouts() call (the finally block already clears). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xw37hR5seiCyWnppG9z4op
| return this.fileCorePictureService.uploadWorkspacePicture({ | ||
| file: buffer, | ||
| filename, | ||
| workspace, | ||
| }); |
There was a problem hiding this comment.
Bug: The uploadNewWorkspaceLogo resolver attempts to write file metadata to the workspace database before the corresponding schema has been created, leading to a database error.
Severity: HIGH
Suggested Fix
Defer the logo upload until after the workspace has been successfully activated. Alternatively, ensure the workspace database schema is created before any file operations are attempted, possibly by creating it during the initial workspace creation step rather than during activation.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/twenty-server/src/engine/core-modules/auth/auth.resolver.ts#L588-L592
Potential issue: When a user creates a new workspace and immediately uploads a logo, the
`uploadNewWorkspaceLogo` resolver is triggered. This calls
`fileStorageService.writeFile`, which attempts to save file metadata using
`fileRepository.upsert()`. However, the workspace's database schema is only created
during the `activateWorkspace` step, which occurs after the logo upload attempt. This
race condition results in an attempt to write to a non-existent database schema, causing
a database error and preventing the logo from being saved correctly.
Also affects:
packages/twenty-server/src/engine/core-modules/file/file-core-picture/services/file-core-picture.service.ts:160~169
…peForm SignInUp renders the creation form at the top level for the WorkspaceCreation step before this form is reached, so the WorkspaceCreation branch and its guard here were unreachable. Removed them and the now-unused import. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xw37hR5seiCyWnppG9z4op
createWorkspace had a single caller that always passed newTab: false, so the new-tab branch was unreachable. Removed the param and the '_blank' branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xw37hR5seiCyWnppG9z4op
What & why
A single, consistent workspace-creation step for both multi-workspace and single-workspace self-host — collecting name + logo (and the subdomain in multi-workspace) — which removes the duplicate name/logo prompt that previously reappeared on the workspace subdomain (reported after #21641).
Changes
One creation form for both modes
SignInUpWorkspaceCreationForm;SignInUprenders it for theWorkspaceCreationstep regardless of domain/scope.Logo on the creation step
uploadNewWorkspaceLogo(workspaceId, file)mutation: the creator sets a logo on their just-createdPENDING_CREATIONworkspace via the workspace-agnostic token (membership enforced — only the creator is a member at that point), reusinguploadWorkspacePicture. Upload size is capped viasettings.storage.maxFileSize(also applied to the existing logo / profile-picture uploads).Onboarding step → pure activation loader
Testing
auth.resolver.spec,useWorkspaceSubdomainField,SignInUpWorkspaceCreationForm(multi + single-workspace),useAuth✅twenty-client-sdkschema regenerated.Follow-up to #21641.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Xw37hR5seiCyWnppG9z4op