-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat: Configurable initial user #5671
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…configurable-initial-user
Thank you for following the naming conventions for pull request titles! 🙏 |
WalkthroughThis change introduces support for automated initial setup of the application using environment variables. Four new environment variables— Assessment against linked issues
Suggested labels
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/database/src/scripts/initial-user-setup.ts (2)
22-40
: Good validation functions, but consider handling type errors more explicitly.The validation functions correctly use Zod schemas to validate inputs. However, the static analysis tools are flagging potential type errors with
.safeParse()
and.success
access. Consider adding explicit type guards.const isValidEmail = (email: string): boolean => { - const parseResult = ZUserEmail.safeParse(email); - return parseResult.success; + try { + const parseResult = ZUserEmail.safeParse(email); + return parseResult.success; + } catch (error) { + logger.error("Error validating email:", error); + return false; + } };Apply similar changes to the other validation functions.
🧰 Tools
🪛 GitHub Check: Run Linters / Linters
[failure] 23-23:
Unsafe assignment of an error typed value
[failure] 23-23:
Unsafe call of anerror
type typed value
[failure] 23-23:
Unsafe member access .safeParse on anerror
typed value
[failure] 24-24:
Unsafe return of an error typed value
[failure] 24-24:
Unsafe member access .success on anerror
typed value
[failure] 28-28:
Unsafe assignment of an error typed value
[failure] 28-28:
Unsafe call of anerror
type typed value
106-115
: Consider handling case where project name is provided without organization name.The current implementation silently ignores the project name if no organization name is provided. Consider adding logic to handle this case more explicitly, either by logging a warning or creating a default organization.
// At the beginning of createInitialUser function + if (projectName && !organizationName) { + logger.warn("Project name provided without organization name. Using default organization name."); + organizationName = "My Organization"; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
apps/web/Dockerfile
(1 hunks)apps/web/app/(app)/environments/[environmentId]/actions/loading.tsx
(1 hunks)apps/web/app/(app)/environments/[environmentId]/components/MainNavigation.tsx
(1 hunks)apps/web/lib/constants.ts
(1 hunks)apps/web/lib/env.ts
(3 hunks)apps/web/modules/survey/editor/components/animated-survey-bg.tsx
(1 hunks)apps/web/modules/survey/editor/components/edit-welcome-card.tsx
(1 hunks)apps/web/modules/survey/editor/components/survey-placement-card.tsx
(1 hunks)apps/web/modules/survey/editor/components/survey-variables-card.tsx
(2 hunks)apps/web/modules/survey/follow-ups/components/follow-up-action-multi-email-input.tsx
(1 hunks)apps/web/modules/survey/list/components/copy-survey-form.tsx
(2 hunks)apps/web/modules/survey/list/components/tests/copy-survey-form.test.tsx
(1 hunks)docker/docker-compose.yml
(1 hunks)docs/self-hosting/configuration/automated-setup.mdx
(1 hunks)docs/self-hosting/configuration/environment-variables.mdx
(1 hunks)packages/database/package.json
(2 hunks)packages/database/src/scripts/initial-user-setup.ts
(1 hunks)turbo.json
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/web/lib/env.ts (1)
packages/types/user.ts (2)
- 8000
ZUserEmail
(31-31)ZUserPassword
(35-39)
🪛 GitHub Check: Run Linters / Linters
packages/database/src/scripts/initial-user-setup.ts
[failure] 3-3:
Unable to resolve path to module '@formbricks/types/project'
[failure] 4-4:
Unable to resolve path to module '@formbricks/types/user'
[failure] 23-23:
Unsafe assignment of an error typed value
[failure] 23-23:
Unsafe call of an error
type typed value
[failure] 23-23:
Unsafe member access .safeParse on an error
typed value
[failure] 24-24:
Unsafe return of an error typed value
[failure] 24-24:
Unsafe member access .success on an error
typed value
[failure] 28-28:
Unsafe assignment of an error typed value
[failure] 28-28:
Unsafe call of an error
type typed value
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Build Formbricks-web / Build Formbricks-web
- GitHub Check: Run E2E Tests / Run E2E Tests
- GitHub Check: SonarQube
- GitHub Check: Validate Docker Build
🔇 Additional comments (28)
apps/web/modules/survey/editor/components/survey-placement-card.tsx (1)
94-94
: CSS class reordering looks goodThe change simply reorders padding classes from
"pr-5 pl-2"
to"pl-2 pr-5"
which maintains the same styling while standardizing class ordering across the codebase.apps/web/modules/survey/follow-ups/components/follow-up-action-multi-email-input.tsx (1)
78-78
: CSS class reordering looks goodThe change reorders text styling classes from
"text-lg leading-none font-medium"
to"text-lg font-medium leading-none"
which maintains the same styling while standardizing class ordering.apps/web/modules/survey/editor/components/animated-survey-bg.tsx (1)
91-91
: CSS class reordering looks goodThe change reorders positioning classes from
"top-2 right-2"
to"right-2 top-2"
which maintains the same styling while standardizing class ordering throughout the codebase.apps/web/modules/survey/editor/components/edit-welcome-card.tsx (1)
70-70
: CSS class reordering looks goodThe change reorders border classes from
"border-t border-b border-l"
to"border-b border-l border-t"
which maintains the same styling while standardizing class order.apps/web/modules/survey/editor/components/survey-variables-card.tsx (2)
44-44
: Consistent Tailwind CSS class reorder – no functional change
The border utility classes have been reordered (border-b border-l border-t
) purely for style consistency. This has no impact on component behavior or layout.
78-78
: Consistent Tailwind CSS class reorder – no functional change
The text utility classes (text-sm italic text-slate-500
) have been reordered for consistency with other components. No behavior or styling differences.apps/web/modules/survey/list/components/copy-survey-form.tsx (4)
101-101
: Consistent Tailwind CSS class reorder – no functional change
The checkbox’s utility classes have been reordered. This is purely cosmetic and preserves the same styles and behavior.
105-105
: Consistent Tailwind CSS class reorder – no functional change
Reordered text utility classes in the label paragraph (text-sm font-medium capitalize text-slate-900
) for uniformity. No effect on rendering.
124-124
: Consistent Tailwind CSS class reorder – no functional change
Reordered container utility classes (fixed bottom-0 left-0 right-0 z-10 flex w-full justify-end space-x-2 bg-white
) for readability. No layout change.
125-125
: Consistent Tailwind CSS class reorder – no functional change
Reordered utility classes in the inner footer div (flex w-full justify-end pb-4 pr-4
). This does not alter styling or behavior.apps/web/modules/survey/list/components/tests/copy-survey-form.test.tsx (1)
35-35
: Consistent Tailwind CSS class reorder in mock – no impact on test logic
The mocked Checkbox’s class string has been reordered for consistency; the test behavior remains unchanged.apps/web/app/(app)/environments/[environmentId]/actions/loading.tsx (1)
36-36
: Consistent Tailwind CSS class reorder – no functional change
Reordered utility classes (whitespace-nowrap text-center text-sm
) in the loading placeholder. Styling and behavior are unaffected.apps/web/app/(app)/environments/[environmentId]/components/MainNavigation.tsx (1)
267-267
: Consistent Tailwind CSS class reorder – no functional change
The focus-related utility classes (focus:outline-none focus:ring-0 focus:ring-transparent
) have been reordered for consistency. No change to functionality.turbo.json (1)
175-179
: Looks good: Environment variables correctly added to the build task.The new environment variables for the initial user setup feature are properly added to the
env
array in the build task. These match the variables mentioned in the PR objectives.apps/web/Dockerfile (1)
167-168
: Appropriate integration of initial user setup in container startup sequence.The new command to run the initial user setup script is correctly placed after the database migration and SAML database creation steps, ensuring proper sequence of operations during container startup.
packages/database/package.json (3)
15-16
: Good implementation of deployment and development scripts for initial user setup.The two new npm scripts are well defined for both deployment and development contexts, with appropriate environment variable handling.
18-18
: Correctly integrating initial user setup into the database setup process.The
db:setup
script now includes the initial user setup step, which ensures it runs as part of the standard database setup process.
31-31
: Essential dependency for password handling.The addition of
bcryptjs
is appropriate for secure password hashing in the initial user setup script.docker/docker-compose.yml (1)
32-39
: Well-documented environment variables for automated setup.The new section for automated setup environment variables is clearly labeled and includes helpful commented examples with appropriate placeholder values. The section is positioned logically in the file structure.
apps/web/lib/constants.ts (1)
288-292
: Constants correctly exported for application use.The new environment variables are properly exposed as constants with a clear comment section. This follows the established pattern in the file and makes the variables accessible throughout the application.
docs/self-hosting/configuration/automated-setup.mdx (1)
1-85
: LGTM! Clear and comprehensive documentation.This new documentation file is well-structured and provides valuable information about the automated setup feature, including required and optional variables, workflow explanation, example configuration, security considerations, and important limitations.
apps/web/lib/env.ts (3)
3-3
: Import added correctly.The import of user validation schemas is properly added to support the validation of the new environment variables.
110-113
: Validation implementation looks good.Appropriate validation is applied to the new environment variables:
INITIAL_USER_EMAIL
is validated using theZUserEmail
schemaINITIAL_USER_PASSWORD
is validated using theZUserPassword
schemaINITIAL_ORGANIZATION_NAME
andINITIAL_PROJECT_NAME
are validated as optional strings
210-213
: Environment variables properly exposed.The new environment variables are correctly mapped to their runtime equivalents, making them accessible throughout the application.
packages/database/src/scripts/initial-user-setup.ts (4)
11-20
: Good implementation of fresh instance check.The function correctly checks if the instance is fresh by verifying that no users or organizations exist. Error handling is also properly implemented.
84-122
: Well-structured organization and project creation.The code correctly handles the creation of a user with optional organization and project. The use of nested creation through Prisma's API is efficient and maintains data integrity.
124-164
: Good implementation of the main setup function.The main function has good logic flow:
- Validates environment variables
- Checks if credentials are provided
- Verifies if the instance is fresh
- Creates the initial user with optional organization and project
- Provides detailed success logging
1-7
:⚠️ Potential issueFix module imports to resolve static analysis warnings.
Static analysis tools are reporting errors with the module import paths. The imports should be relative or use the correct module resolution.
import { hash } from "bcryptjs"; import { logger } from "@formbricks/logger"; -import { ZProject } from "../../../types/project"; -import { ZUserEmail, ZUserPassword } from "../../../types/user"; +import { ZProject } from "@formbricks/types/project"; +import { ZUserEmail, ZUserPassword } from "@formbricks/types/user"; import { ZOrganization } from "../../zod/organizations"; import { prisma } from "../client";Likely an incorrect or invalid review comment.
🧰 Tools
🪛 GitHub Check: Run Linters / Linters
[failure] 3-3:
Unable to resolve path to module '@formbricks/types/project'
[failure] 4-4:
Unable to resolve path to module '@formbricks/types/user'
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/database/src/scripts/initial-user-setup.ts (1)
62-72
: Export helper functions for better testability
Currently onlyisFreshInstance
is exported. Consider exportingvalidateEnvironmentVariables
andcreateInitialUser
(andinitialUserSetup
if desired) so that you can write focused unit tests against each piece of logic:-export const createInitialUser = async ( +export const createInitialUser = async ( email: string, password: string, organizationName?: string, projectName?: string ) : Promise<void> => { ... } -export const initialUserSetup = async (): Promise<boolean> => { +export const initialUserSetup = async (): Promise<boolean> => { ... };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/labeler.yml
(1 hunks)docs/self-hosting/configuration/environment-variables.mdx
(1 hunks)packages/database/src/scripts/initial-user-setup.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/labeler.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/self-hosting/configuration/environment-variables.mdx
🧰 Additional context used
🪛 GitHub Check: Run Linters / Linters
packages/database/src/scripts/initial-user-setup.ts
[failure] 3-3:
Unable to resolve path to module '@formbricks/types/project'
[failure] 4-4:
Unable to resolve path to module '@formbricks/types/user'
[failure] 23-23:
Unsafe assignment of an error typed value
[failure] 23-23:
Unsafe call of an error
type typed value
[failure] 23-23:
Unsafe member access .safeParse on an error
typed value
[failure] 24-24:
Unsafe return of an error typed value
[failure] 24
8000
-24:
Unsafe member access .success on an error
typed value
[failure] 28-28:
Unsafe assignment of an error typed value
[failure] 28-28:
Unsafe call of an error
type typed value
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build Formbricks-web / Build Formbricks-web
- GitHub Check: Run E2E Tests / Run E2E Tests
- GitHub Check: Validate Docker Build
🔇 Additional comments (1)
packages/database/src/scripts/initial-user-setup.ts (1)
124-124
: AI summary discrepancy:initialUserSetup
is not exported
The AI-generated summary states thatinitialUserSetup
is exported for reuse/testing, but in the code it’s declared as a plainconst
and not exported. If you intend to import this function elsewhere (e.g., for unit tests), please add theexport
keyword.Likely an incorrect or invalid review comment.
initialUserSetup() | ||
.then(() => { | ||
process.exit(0); | ||
}) | ||
.catch((error: unknown) => { | ||
logger.error(error, "Error during initial user setup"); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure proper exit codes and consistent error logging
Currently, the script always calls process.exit(0)
on promise resolution and logs errors with reversed parameters in the final .catch
, causing failures to be masked and log formatting to be inconsistent. Update this block to exit with a non-zero code on failure and unify the logger.error
signature:
-initialUserSetup()
- .then(() => {
- process.exit(0);
- })
- .catch((error: unknown) => {
- logger.error(error, "Error during initial user setup");
- });
+initialUserSetup()
+ .then((success) => {
+ process.exit(success ? 0 : 1);
+ })
+ .catch((error: unknown) => {
+ logger.error("Error during initial user setup:", error);
+ process.exit(1);
+ });
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
initialUserSetup() | |
.then(() => { | |
process.exit(0); | |
}) | |
.catch((error: unknown) => { | |
logger.error(error, "Error during initial user setup"); | |
}); | |
initialUserSetup() | |
.then((success) => { | |
process.exit(success ? 0 : 1); | |
}) | |
.catch((error: unknown) => { | |
logger.error("Error during initial user setup:", error); | |
process.exit(1); | |
}); |
🤖 Prompt for AI Agents (early access)
In packages/database/src/scripts/initial-user-setup.ts around lines 166 to 172, the promise resolution always calls process.exit(0), masking failures, and the logger.error call in the catch block has reversed parameters causing inconsistent log formatting. Modify the catch block to call process.exit with a non-zero code (e.g., process.exit(1)) to indicate failure, and update the logger.error call to use a consistent signature with the error message first followed by the error object or context.
What does this PR do?
Fixes #5584
How should this be tested?
Test adding env variables
INITIAL_USER_EMAIL
INITIAL_USER_PASSWORD
INITIAL_ORGANIZATION_NAME
INITIAL_PROJECT_NAME
Checklist
Required
pnpm build
console.logs
git pull origin main
Appreciated
Summary by CodeRabbit
New Features
Documentation
Chores
Style