chore: add tests to environments path - part 3 #5680
Conversation
|
⏩ PR is too large (90fb9c6) View output ↗ View check history
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
WalkthroughThis update introduces a comprehensive suite of new test files across the application, primarily targeting the environments, projects, surveys, and settings modules. The tests cover a wide range of components, including layout, navigation, profile, notifications, organization settings, survey analysis, and sharing features. Many tests focus on verifying correct re-exporting of components, rendering behavior under various conditions, user interactions, error handling, and state management. Additionally, some utility and helper functions are now explicitly exported for testing purposes. The update also refines test and coverage exclusion patterns in configuration files and adds new exclusions to SonarQube analysis. Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (7)
✨ 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:
Need 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.
Actionable comments posted: 8
🧹 Nitpick comments (30)
apps/web/app/(app)/environments/[environmentId]/project/look/page.test.tsx (1)
5-27: Consider reducing the number of mocked constants if possibleThe test correctly verifies that the Page component re-exports ProjectLookSettingsPage, but it mocks many constants that might not be necessary for this specific test. Consider if a more targeted approach to mocking would be sufficient.
If these constants aren't directly used by the components under test, you could simplify the mock to:
vi.mock("@/lib/constants", () => ({ - IS_FORMBRICKS_CLOUD: false, - POSTHOG_API_KEY: "mock-posthog-api-key", - POSTHOG_HOST: "mock-posthog-host", - IS_POSTHOG_CONFIGURED: true, - ENCRYPTION_KEY: "mock-encryption-key", - ENTERPRISE_LICENSE_KEY: "mock-enterprise-license-key", - GITHUB_ID: "mock-github-id", - GITHUB_SECRET: "test-githubID", - GOOGLE_CLIENT_ID: "test-google-client-id", - GOOGLE_CLIENT_SECRET: "test-google-client-secret", - AZUREAD_CLIENT_ID: "test-azuread-client-id", - AZUREAD_CLIENT_SECRET: "test-azure", - AZUREAD_TENANT_ID: "test-azuread-tenant-id", - OIDC_DISPLAY_NAME: "test-oidc-display-name", - OIDC_CLIENT_ID: "test-oidc-client-id", - OIDC_ISSUER: "test-oidc-issuer", - OIDC_CLIENT_SECRET: "test-oidc-client-secret", - OIDC_SIGNING_ALGORITHM: "test-oidc-signing-algorithm", - WEBAPP_URL: "test-webapp-url", - IS_PRODUCTION: false, - SENTRY_DSN: "mock-sentry-dsn", + // Only mock the constants that are actually needed }));However, if these constants are required for the module to load properly, then the current approach is fine.
apps/web/app/(app)/environments/[environmentId]/project/tags/loading.test.tsx (1)
10-15: Fix describe block naming inconsistencyThere's a minor naming inconsistency in the describe block. It refers to "TagsLoadingPage" while the component being tested is called "TagsLoading".
-describe("TagsLoadingPage Re-export", () => { +describe("TagsLoading Re-export", () => { test("should re-export TagsLoading from the correct module", () => { // Check if the re-exported component is the same as the original (mocked) component expect(TagsLoading).toBe(OriginalTagsLoading); }); });apps/web/app/(app)/environments/[environmentId]/project/page.test.tsx (1)
5-27: Consider reducing mocked constants if they're not all necessarySimilar to the other test files, this test mocks a large number of constants that might not all be necessary for this specific test. If these constants aren't directly used by the components under test, consider a more targeted mocking approach.
If possible, only mock the constants that are actually needed for the test to run successfully.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/WebAppTab.test.tsx (1)
35-36: Unused test data variablesThe
surveyUrlandsurveyIdvariables are defined but never used in the test. If they are intended to be passed to the WebAppTab component, you should update the render call. If they're not needed, consider removing them to avoid confusion.-const surveyUrl = "https://app.formbricks.com/s/test-survey-id"; -const surveyId = "test-survey-id";Or if they're meant to be used:
-render(<WebAppTab />); +render(<WebAppTab surveyUrl={surveyUrl} surveyId={surveyId} />);apps/web/app/(app)/environments/[environmentId]/settings/(account)/notifications/loading.test.tsx (1)
33-37: Consider using data-testid for more robust DOM selectionThe tests currently use CSS class-based selection (
.closest("div[class*='rounded-xl']")) to find parent card elements. This approach is somewhat brittle as it depends on CSS classes that might change. Consider adding data-testid attributes to the cards in the Loading component for more robust testing.-const alertsCard = screen - .getByText("environments.settings.notifications.email_alerts_surveys") - .closest("div[class*='rounded-xl']"); // Find parent card +const alertsCard = screen.getByTestId("alerts-card"); -const weeklySummaryCard = screen - .getByText("environments.settings.notifications.weekly_summary_projects") - .closest("div[class*='rounded-xl']"); // Find parent card +const weeklySummaryCard = screen.getByTestId("weekly-summary-card");And update the Loading component to include these test IDs.
Also applies to: 45-48
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/PanelInfoView.test.tsx (1)
27-27: Consider removing the NOSONAR comment.The NOSONAR comment suggests suppressing a static analysis warning. If this is intentional, consider adding a brief explanation of why the warning is suppressed, or refactor the mock to avoid the warning altogether.
- if (asChild) { - return <div // NOSONAR + if (asChild) { + return <div >apps/web/app/(app)/environments/[environmentId]/settings/(account)/layout.test.tsx (1)
10-43: Consider simplifying the constants mock.The current mock for
@/lib/constantsincludes many values that might not be needed for this specific test. Consider mocking only the constants that are actually used by the component under test.vi.mock("@/lib/constants", () => ({ IS_FORMBRICKS_CLOUD: false, - POSTHOG_API_KEY: "mock-posthog-api-key", - POSTHOG_HOST: "mock-posthog-host", - IS_POSTHOG_CONFIGURED: true, - ENCRYPTION_KEY: "mock-encryption-key", - ENTERPRISE_LICENSE_KEY: "mock-enterprise-license-key", - GITHUB_ID: "mock-github-id", - GITHUB_SECRET: "test-githubID", - GOOGLE_CLIENT_ID: "test-google-client-id", - GOOGLE_CLIENT_SECRET: "test-google-client-secret", - AZUREAD_CLIENT_ID: "test-azuread-client-id", - AZUREAD_CLIENT_SECRET: "test-azure", - AZUREAD_TENANT_ID: "test-azuread-tenant-id", - OIDC_DISPLAY_NAME: "test-oidc-display-name", - OIDC_CLIENT_ID: "test-oidc-client-id", - OIDC_ISSUER: "test-oidc-issuer", - OIDC_CLIENT_SECRET: "test-oidc-client-secret", - OIDC_SIGNING_ALGORITHM: "test-oidc-signing-algorithm", WEBAPP_URL: "test-webapp-url", IS_PRODUCTION: false, - SENTRY_DSN: "mock-sentry-dsn", }));apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/components/EmptyInAppSurveys.test.tsx (2)
1-5: Component import name doesn't match the filenameThe import statement uses
EmptyAppSurveyswhile the file name isEmptyInAppSurveys.test.tsx. This naming inconsistency could lead to confusion.Consider renaming either:
- The imported component to match the file name:
-import { EmptyAppSurveys } from "./EmptyInAppSurveys"; +import { EmptyInAppSurveys } from "./Emp 8000 tyInAppSurveys";
- Or update any references in the test to use the correct component name.
15-17: Consider using a more complete mock environment objectThe current mock environment is very minimal, using a type assertion to satisfy the
TEnvironmenttype. This might be sufficient for this specific test, but using a more complete mock would make the test more robust.Consider using the pattern from the provided code snippet in
apps/web/lib/survey/tests/__mock__/survey.mock.ts:-const mockEnvironment = { - id: "test-env-id", -} as unknown as TEnvironment; +const mockEnvironment: TEnvironment = { + id: "test-env-id", + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + type: "production", + projectId: "test-project-id", + appSetupCompleted: false, +};apps/web/app/(app)/environments/[environmentId]/settings/(organization)/layout.test.tsx (2)
31-37: Inconsistent mock constant valuesThere are inconsistencies in the mocked constant values:
GITHUB_SECRETis set to "test-githubID" which doesn't follow the pattern of other secretsAZUREAD_CLIENT_SECRETis truncated to "test-azure" instead of something more descriptive like "test-azuread-client-secret"- GITHUB_SECRET: "test-githubID", + GITHUB_SECRET: "test-github-secret", - AZUREAD_CLIENT_SECRET: "test-azure", + AZUREAD_CLIENT_SECRET: "test-azuread-client-secret",
52-54: Consider using more detailed mock objectsThe mock objects are very minimal and cast using
as unknown as. While this works for the current tests, adding more properties that match the actual objects would make the tests more robust and easier to understand.-const mockOrganization = { id: "org_test_id" } as unknown as TOrganization; -const mockProject = { id: "project_test_id" } as unknown as TProject; -const mockSession = { user: { id: "user_test_id" } } as unknown as Session; +const mockOrganization: Partial<TOrganization> = { + id: "org_test_id", + name: "Test Organization", + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString() +}; + +const mockProject: Partial<TProject> = { + id: "project_test_id", + name: "Test Project", + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString() +}; + +const mockSession: Partial<Session> = { + user: { + id: "user_test_id", + email: "test@example.com", + name: "Test User" + } +};apps/web/app/(app)/environments/[environmentId]/settings/(organization)/enterprise/page.test.tsx (1)
181-193: Effective component rendering verificationThe test verifies both structural elements (PageContentWrapper, PageHeader) and content-specific elements (text for SSO and branding removal). It also ensures no unexpected navigation occurs.
Consider adding more test cases to cover additional scenarios, such as:
- Testing with different user roles (admin, member)
- Testing with Formbricks Cloud enabled
- Testing error states or loading states
apps/web/app/(app)/environments/[environmentId]/settings/(organization)/general/components/EditOrganizationNameForm.test.tsx (2)
39-46: Mock cleanup redundancyThe test both clears all mocks in
afterEachand explicitly resets theupdateOrganizationNameActionmock inbeforeEach. This is redundant and one approach should be sufficient.afterEach(() => { cleanup(); vi.clearAllMocks(); }); beforeEach(() => { - vi.mocked(updateOrganizationNameAction).mockReset(); });
96-98: Improve type safety in mock responseUsing
as anyreduces type safety. Consider creating a properly typed mock response.vi.mocked(updateOrganizationNameAction).mockResolvedValueOnce({ - data: null as any, + data: null, });apps/web/app/(app)/environments/[environmentId]/settings/(organization)/general/components/DeleteOrganization.test.tsx (3)
23-32: Redundant custom translation implementationThe mock translation function (
mockT) is defined but seems redundant since you're already mocking the translation service withgetTranslate.Consider removing this custom implementation and using the mocked translation function consistently throughout the tests.
45-46: Consolidate router mock setupThe router push mock is defined as a separate variable and then used in the
beforeEachhook. Consider consolidating this for better maintainability.-const mockRouterPush = vi.fn(); describe("DeleteOrganization", () => { beforeEach(() => { vi.clearAllMocks(); - vi.mocked(useRouter).mockReturnValue({ push: mockRouterPush } as any); + const mockRouterPush = vi.fn(); + vi.mocked(useRouter).mockReturnValue({ push: mockRouterPush } as any); localStorage.clear(); });Also applies to: 58-61
125-125: Improve type safety in mock responsesUsing
as anyreduces type safety. Consider creating a properly typed mock response.- vi.mocked(deleteOrganizationAction).mockResolvedValue({} as any); + vi.mocked(deleteOrganizationAction).mockResolvedValue({ success: true });apps/web/app/(app)/environments/[environmentId]/settings/(account)/profile/components/EditProfileDetailsForm.test.tsx (1)
67-67: Use consistent approach for text matchingThere's inconsistency between using hardcoded strings like "English (US)" and translation keys like "common.update". Choose one approach for better maintainability.
Be consistent with either:
- Using translation keys throughout, or
- Using the actual rendered text throughout
Also applies to: 78-78
apps/web/app/(app)/environments/[environmentId]/settings/(account)/profile/components/EditProfileAvatarForm.test.tsx (1)
1-104: Consider adding a test for avatar upload functionality.The tests cover rendering with and without an avatar and removal scenarios, but there's no test for uploading a new avatar via the file input.
You could add a test like this:
test("handles image upload successfully", async () => { vi.mocked(fileUploadHooks.handleFileUpload).mockResolvedValue({ url: "new-avatar.jpg", error: undefined, }); render( <EditProfileAvatarForm session={mockSession} environmentId={environmentId} imageUrl={null} /> ); const fileInput = screen.getByLabelText("environments.settings.profile.upload_image"); const file = new File(['dummy content'], 'avatar.png', { type: 'image/png' }); await userEvent.upload(fileInput, file); await waitFor(() => { expect(fileUploadHooks.handleFileUpload).toHaveBeenCalled(); expect(profileActions.updateAvatarAction).toHaveBeenCalledWith({ environmentId, imageUrl: "new-avatar.jpg" }); }); });apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/components/SurveyAnalysisNavigation.test.tsx (1)
21-48: Consider simplifying mock constants.There are many environment and configuration constants being mocked that may not be directly relevant to testing the navigation component's functionality.
Consider using a more focused mock for constants:
vi.mock("@/lib/constants", () => ({ IS_FORMBRICKS_CLOUD: false, WEBAPP_URL: "test-webapp-url", IS_PRODUCTION: false, // Add only the constants actually needed by the component }));apps/web/app/(app)/environments/[environmentId]/settings/(organization)/general/page.test.tsx (3)
63-67: Avoid “import‐then‐mock” – placevi.mockbefore any direct import of the target moduleAlthough Vitest/Jest hoist
vi.mockcalls, mixing the runtime import (import { OrganizationSettingsNavbar } …) with a manual mock of the same path is brittle – TypeScript’s emit order or a future refactor (e.g. turning this file into an ESM‐only transform) could cause the real implementation to be imported before the hoist occurs.Prefer one of the following:
// 1. Pure mock (recommended – you never need the real module here) vi.mock("@/app/(app)/environments/[environmentId]/settings/(organization)/components/OrganizationSettingsNavbar", () => ({ OrganizationSettingsNavbar: vi.fn(() => <div>OrganizationSettingsNavbar</div>) })); // 2. If type information is needed: import type { OrganizationSettingsNavbar } from "@/app/(app)/environments/[environmentId]/settings/(organization)/components/OrganizationSettingsNavbar"; // ^ note `type` import so it’s erased at runtimeThen drop the immediate runtime import at the top of the file.
100-104: Pass theparamsobject directly instead of wrapping it inPromise.resolve
Pagealready returns a Promise because it’s an async server component. Supplyingprops.paramsas aPromiseintroduces an unnecessary layer and hides type mismatches (TS will inferunknown). Replace:const props = { params: Promise.resolve(mockParams) };with
const props = { params: mockParams };(and drop the extra
awaitif TypeScript complains).
This keeps the test signature identical to the real framework call-site.
85-110:vi.resetAllMocks()resets factory implementations as well – considervi.clearAllMocks()Calling
vi.resetAllMocks()inbeforeEachwipes the custom mock implementations created above (e.g.OrganizationSettingsNavbar,EditOrganizationNameForm) and replaces them with emptyvi.fn()s. They are then re-assigned only where you explicitly override them in the samebeforeEach, but any module you don’t reconfigure loses its mock body.If the goal is only to clear invocation history, switch to:
beforeEach(() => { vi.clearAllMocks(); // keeps the factories intact, only zeroes call counts });This avoids accidental “undefined is not a function” regressions should new mocked modules be added later without a matching re-initialisation block.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/responses/components/ResponseTable.test.tsx (1)
450-457: Remove commented-out assertions or make them passDead code reduces clarity and can quickly become outdated.
Either:
- Implement a small
tablemock so thattoolbar-delete-selectedcan be exercised; or- Delete the commented lines.
Keeping them commented suggests the test is incomplete.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/responses/components/ResponseDataView.tsx (2)
27-36: Exporting test helpers from production modules leaks internals
formatAddressData,formatContactInfoData,extractResponseData, andmapResponsesToTableDataare now part of the public API, increasing bundle size and coupling. A safer pattern is:
- Move these utilities into a co-located
*.internal.ts(orutils.ts) file.- Re-export them only from a
__tests__/helpersbarrel, or- Use Vitest’s
import { xxx } from "./file?__private"trick.This keeps production surface minimal while retaining direct unit-test access.
80-105: Minor performance micro-nit: avoidObject.assignin tight loopsInside
mapResponsesToTableData:variables: survey.variables.reduce( (acc, curr) => Object.assign(acc, { [curr.id]: response.variables[curr.id] }), {} ),
Object.assignallocates a new object on every iteration. Using property assignment is ~6-8× faster and clearer:- (acc, curr) => Object.assign(acc, { [curr.id]: response.variables[curr.id] }), + (acc, curr) => { + acc[curr.id] = response.variables[curr.id];+ return acc; + }, Not critical for typical dataset sizes but essentially free to adopt.
apps/web/app/(app)/environments/[environmentId]/settings/(account)/profile/page.test.tsx (1)
56-60: Use conventional prop-discard pattern instead of a phantom_propThe mocked
EditProfileAvatarFormcomponent destructures a non-existing prop named_.
While harmless at runtime, it introduces an incorrect public API surface for the component and adds noise to IntelliSense.-EditProfileAvatarForm: ({ _, environmentId }) => ( +EditProfileAvatarForm: ({ environmentId }) => (A conventional way to ignore unused props is to accept
...restor simply omit them from the signature.apps/web/modules/survey/editor/components/when-to-send-card.test.tsx (1)
321-345: RedundantfireEvent.changeafteruserEvent.type
userEvent.typealready fires theinput&changeevents.
The extra manualfireEvent.changedoubles the callbacks and can inflate thesetLocalSurveycall count, making the test brittle.-await userEvent.type(delayInput, "15"); -fireEvent.change(delayInput, { target: { value: "15" } }); +await userEvent.type(delayInput, "15");Apply the same simplification to the auto-close and display-percentage tests for leaner, faster specs.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/EmailTab.test.tsx (2)
92-93: Minor inconsistency in text content assertion.The test asserts on "Hello World ?preview=true&foo=bar" while the sample data contains HTML tags and encoded ampersand: "
Hello World ?preview=true&foo=bar
". The assertion likely works because textContent strips HTML tags and decodes entities, but it might be clearer to match the exact format.- expect(screen.getByText("Hello World ?preview=true&foo=bar")).toBeInTheDocument(); // Raw HTML content + // Assert on text content which strips HTML tags and decodes entities + expect(screen.getByText(/Hello World \?preview=true&foo=bar/)).toBeInTheDocument();
1-233: Consider adding test for HTML fetch failure.The test suite thoroughly covers failures in
sendEmbedSurveyPreviewEmailActionbut doesn't test what happens ifgetEmailHtmlActionfails. This could be a gap in error handling coverage.You could add a test like:
test("handles email HTML fetch failure", async () => { vi.mocked(getEmailHtmlAction).mockRejectedValue(new Error("Failed to fetch HTML")); render(<EmailTab surveyId={surveyId} email={userEmail} />); await waitFor(() => { expect(toast.error).toHaveBeenCalledWith("common.something_went_wrong_please_try_again"); }); expect(screen.queryByTestId("loading-spinner")).not.toBeInTheDocument(); // Assert on fallback UI or error state });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (64)
apps/web/app/(app)/environments/[environmentId]/project/(setup)/app-connection/loading.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/project/(setup)/app-connection/page.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/project/general/loading.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/project/general/page.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/project/languages/loading.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/project/languages/page.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/project/layout.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/project/look/loading.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/project/look/page.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/project/page.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/project/tags/loading.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/project/tags/page.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/project/teams/page.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(account)/components/AccountSettingsNavbar.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(account)/layout.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(account)/notifications/components/EditAlerts.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(account)/notifications/components/EditWeeklySummary.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(account)/notifications/components/IntegrationsTip.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(account)/notifications/components/NotificationSwitch.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(account)/notifications/loading.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(account)/notifications/page.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(account)/profile/components/AccountSecurity.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(account)/profile/components/DeleteAccount.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(account)/profile/components/EditProfileAvatarForm.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(account)/profile/components/EditProfileDetailsForm.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(account)/profile/loading.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(account)/profile/page.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(organization)/api-keys/loading.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(organization)/api-keys/page.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(organization)/billing/loading.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(organization)/billing/page.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(organization)/components/OrganizationSettingsNavbar.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(organization)/enterprise/loading.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(organization)/enterprise/page.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(organization)/general/components/DeleteOrganization.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(organization)/general/components/EditOrganizationNameForm.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(organization)/general/loading.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(organization)/general/page.test.tsx(3 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(organization)/layout.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(organization)/teams/page.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/components/SettingsCard.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/components/SettingsTitle.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/page.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/components/EmptyInAppSurveys.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/components/SurveyAnalysisNavigation.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/responses/components/ResponseCardModal.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/responses/components/ResponseDataView.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/responses/components/ResponseDataView.tsx(4 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/responses/components/ResponsePage.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/responses/components/ResponseTable.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/responses/components/ResponseTableColumns.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/responses/page.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/ScrollToTop.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/SurveyAnalysisCTA.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/AppTab.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/EmailTab.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/EmbedView.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/LinkTab.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/MobileAppTab.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/PanelInfoView.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/WebAppTab.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/WebsiteTab.test.tsx(1 hunks)apps/web/modules/survey/editor/components/when-to-send-card.test.tsx(3 hunks)apps/web/vite.config.mts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (19)
apps/web/app/(app)/environments/[environmentId]/project/general/page.test.tsx (1)
apps/web/modules/projects/settings/general/page.tsx (1)
GeneralSettingsPage(15-60)
apps/web/app/(app)/environments/[environmentId]/project/tags/page.test.tsx (1)
apps/web/modules/projects/settings/tags/page.tsx (1)
TagsPage(11-39)
apps/web/app/(app)/environments/[environmentId]/settings/(organization)/teams/page.test.tsx (1)
apps/web/modules/organization/settings/teams/page.tsx (1)
TeamsPage(11-46)
apps/web/app/(app)/environments/[environmentId]/project/teams/page.test.tsx (1)
apps/web/modules/ee/teams/project-teams/page.tsx (1)
ProjectTeams(9-31)
apps/web/app/(app)/environments/[environmentId]/settings/(organization)/api-keys/loading.test.tsx (1)
apps/web/app/(app)/environments/[environmentId]/settings/(organization)/api-keys/loading.tsx (1)
LoadingPage(4-6)
apps/web/app/(app)/environments/[environmentId]/project/(setup)/app-connection/page.test.tsx (1)
apps/web/modules/projects/settings/(setup)/app-connection/page.tsx (1)
AppConnectionPage(13-45)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/PanelInfoView.test.tsx (1)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/PanelInfoView.tsx (1)
PanelInfoView(16-98)
apps/web/app/(app)/environments/[environmentId]/project/(setup)/app-connection/loading.test.tsx (1)
apps/web/modules/projects/settings/(setup)/app-connection/loading.tsx (1)
AppConnectionLoading(9-47)
apps/web/app/(app)/environments/[environmentId]/project/tags/loading.test.tsx (1)
apps/web/modules/projects/settings/tags/loading.tsx (1)
TagsLoading(9-44)
apps/web/app/(app)/environments/[environmentId]/project/languages/loading.test.tsx (1)
apps/web/modules/ee/languages/loading.tsx (1)
LanguagesLoading(10-33)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/AppTab.test.tsx (1)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/AppTab.tsx (1)
AppTab(9-27)
apps/web/app/(app)/environments/[environmentId]/settings/(organization)/components/OrganizationSettingsNavbar.test.tsx (2)
packages/types/memberships.ts (1)
TOrganizationRole(7-7)apps/web/app/(app)/environments/[environmentId]/settings/(organization)/components/OrganizationSettingsNavbar.tsx (1)
OrganizationSettingsNavbar(17-67)
apps/web/app/(app)/environments/[environmentId]/settings/(account)/profile/components/DeleteAccount.test.tsx (1)
apps/web/app/(app)/environments/[environmentId]/settings/(account)/profile/components/DeleteAccount.tsx (1)
DeleteAccount(12-58)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/MobileAppTab.test.tsx (1)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/MobileAppTab.tsx (1)
MobileAppTab(8-25)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/components/EmptyInAppSurveys.test.tsx (1)
apps/web/lib/survey/tests/__mock__/survey.mock.ts (1)
mockEnvironment(111-118)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/EmailTab.test.tsx (2)
apps/web/lib/utils/helper.ts (1)
getFormattedErrorMessage(22-38)packages/types/errors.ts (1)
AuthenticationError(117-117)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/responses/components/ResponseDataView.tsx (2)
packages/types/responses.ts (2)
TResponseDataValue(36-36)TResponse(291-291)packages/types/surveys/types.ts (1)
TSurvey(2435-2435)
apps/web/app/(app)/environments/[environmentId]/settings/(account)/notifications/page.test.tsx (2)
apps/web/app/(app)/environments/[environmentId]/settings/(account)/notifications/components/EditAlerts.tsx (1)
EditAlerts(19-117)apps/web/app/(app)/environments/[environmentId]/settings/(account)/notifications/components/EditWeeklySummary.tsx (1)
EditWeeklySummary(16-59)
apps/web/app/(app)/environments/[environmentId]/settings/(account)/notifications/components/EditWeeklySummary.test.tsx (1)
apps/web/app/(app)/environments/[environmentId]/settings/(account)/notifications/components/EditWeeklySummary.tsx (1)
EditWeeklySummary(16-59)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Run E2E Tests / Run E2E Tests
- GitHub Check: Build Formbricks-web / Build Formbricks-web
- GitHub Check: Run Unit Tests / Unit Tests
- GitHub Check: Validate Docker Build
- GitHub Check: SonarQube
- GitHub Check: Tusk Tester
- GitHub Check: Analyze (javascript)
🔇 Additional comments (136)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/SurveyAnalysisCTA.test.tsx (1)
8-8: Import path correction improves module resolution.The import path has been updated to reference the component from the same directory rather than a parent directory, which aligns with the component's actual location.
apps/web/vite.config.mts (1)
141-145: Appropriate test coverage exclusions added.These additional exclusion patterns for type definitions, action files, stories, and mocks are appropriate as they don't contain functional code that needs direct testing. This will provide more accurate code coverage metrics by excluding non-functional files.
apps/web/app/(app)/environments/[environmentId]/project/general/page.test.tsx (1)
1-33: Good test for component re-export.This test ensures that the page module correctly re-exports the GeneralSettingsPage component, maintaining the expected architecture. The comprehensive mocking of environment constants provides proper test isolation.
apps/web/app/(app)/environments/[environmentId]/project/tags/page.test.tsx (1)
1-33: Well-structured re-export verification test.This test follows the established pattern for verifying component re-exports, ensuring the TagsPage component is correctly exported from the page module. The consistent approach to mocking environment constants across tests demonstrates good testing practices.
apps/web/app/(app)/environments/[environmentId]/settings/components/SettingsTitle.test.tsx (1)
1-25: Test implementation looks good and follows best practices!The test suite thoroughly checks that the
SettingsTitlecomponent renders correctly with both populated and empty titles. It uses proper accessibility attributes when querying the DOM withgetByRole, checks for expected styling classes, and implements proper cleanup after each test.apps/web/app/(app)/environments/[environmentId]/project/(setup)/app-connection/loading.test.tsx (1)
1-15: Test implementation is well-structured for verifying component re-export.The test properly verifies that the local import matches the original component, which is a good pattern for ensuring re-exports work correctly.
apps/web/app/(app)/environments/[environmentId]/settings/(organization)/teams/page.test.tsx (1)
1-38: Test implementation effectively verifies component re-export with proper environment isolation.The test mocks all necessary environment constants and properly verifies that the page default export matches the expected imported component.
apps/web/app/(app)/environments/[environmentId]/project/(setup)/app-connection/page.test.tsx (1)
1-33: Well-structured test for component re-exportThis test verifies that the local AppConnectionPage correctly re-exports the original component. The mocking of constants provides a clean test environment with controlled values.
apps/web/app/(app)/environments/[environmentId]/project/languages/loading.test.tsx (1)
1-15: Effective mock implementation for component verificationGood approach mocking the original component to focus exclusively on testing the re-export mechanism. This ensures the test verifies the correct linkage between modules without depending on the actual component implementation.
apps/web/app/(app)/environments/[environmentId]/project/teams/page.test.tsx (1)
1-33: Clean implementation of re-export verificationThe test correctly verifies that the local Page module re-exports the ProjectTeams component without modification. The constants mocking provides isolation from environment variables.
apps/web/app/(app)/environments/[environmentId]/project/languages/page.test.tsx (1)
1-33: Consistent approach to re-export testingThis test follows the same pattern as the other re-export tests, properly verifying that the LanguagesPage component is correctly forwarded. This maintains consistency across the test suite.
apps/web/app/(app)/environments/[environmentId]/settings/page.test.tsx (1)
1-16: Clean, well-structured test for the redirect functionality!This test effectively verifies that the settings page correctly redirects to the profile settings subpage. The test follows good practices by:
- Properly mocking external dependencies (redirect)
- Using descriptive test names
- Following a clear describe-test pattern
- Making specific assertions about the expected behavior
apps/web/app/(app)/environments/[environmentId]/project/look/page.test.tsx (1)
29-33: Well-structured re-export verification testThe test correctly verifies that the Page component is a direct re-export of ProjectLookSettingsPage. This helps ensure consistency in the component export structure.
apps/web/app/(app)/environments/[environmentId]/project/tags/loading.test.tsx (1)
5-8: Good approach for testing the re-exportThe mock implementation provides a simple and clear way to test that the component is correctly re-exported. The use of a test ID in the mock also helps with potential integration tests that might look for this component.
apps/web/app/(app)/environments/[environmentId]/project/page.test.tsx (1)
29-33: Well-structured re-export verification testThe test correctly verifies that the Page component is a direct re-export of ProjectSettingsPage. This helps ensure the consistency of the component export structure throughout the application.
apps/web/app/(app)/environments/[environmentId]/settings/(organization)/billing/page.test.tsx (1)
1-21: Well-structured test for the Billing Page componentThe test implementation follows best practices with proper mocking, focused assertions, and cleanup. It effectively verifies that the Page component correctly renders the PricingPage component with the expected content.
apps/web/app/(app)/environments/[environmentId]/settings/(organization)/api-keys/page.test.tsx (1)
1-21: Effective test for the API Keys Page componentThe test correctly verifies that the Page component renders the APIKeysPage component with the expected content. Good use of mocking, cleanup and assertions to isolate the component under test.
apps/web/app/(app)/environments/[environmentId]/settings/(organization)/api-keys/loading.test.tsx (1)
1-29: Good test coverage for the API Keys loading componentThe test effectively verifies that the LoadingPage correctly forwards the isFormbricksCloud prop to the underlying Loading component. The mocking approach is appropriate for isolating the component behavior and validating the prop passing mechanism.
apps/web/app/(app)/environments/[environmentId]/project/general/loading.test.tsx (1)
1-17: Appropriate test for component re-exportThis test uses a direct equality check rather than rendering, which is an efficient approach for verifying re-exports. The test confirms that GeneralSettingsLoadingPage correctly re-exports the original GeneralSettingsLoading component.
apps/web/app/(app)/environments/[environmentId]/project/layout.test.tsx (1)
1-24: Well-structured test suite with proper component isolation!This test file effectively verifies both the rendering behavior of the ProjectLayout component and its correct metadata exports. The mocking approach for ProjectSettingsLayout is clean and focuses the test on just the behavior of the layout component.
apps/web/app/(app)/environments/[environmentId]/settings/(account)/notifications/components/IntegrationsTip.test.tsx (1)
1-36: Well-implemented component test with appropriate mocks!Good job implementing this test for the IntegrationsTip component. You've properly mocked dependencies (SlackIcon and translation functions) and verified both the component rendering and the correct link URL construction with the environmentId parameter.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/WebAppTab.test.tsx (1)
1-34: Well-structured test with comprehensive mocking!The test effectively verifies the WebAppTab component's rendering with good isolation through mocking of UI components, icons, and clipboard API. The assertions check that proper text and links are rendered with the correct URL.
Also applies to: 37-53
apps/web/app/(app)/environments/[environmentId]/settings/(account)/notifications/loading.test.tsx (1)
1-32: Well-structured test with thorough verification of loading state elements!This test thoroughly verifies the loading state for the notifications settings page, ensuring all expected UI elements are rendered correctly. Good job mocking the page components and verifying the text content for both notification cards.
Also applies to: 38-44, 49-50
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/MobileAppTab.test.tsx (1)
1-70: Well-structured test file that effectively validates the MobileAppTab component!The test correctly mocks all required dependencies and verifies that the component renders all expected elements with proper attributes. The translation keys, alert components, and link attributes are thoroughly checked.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/PanelInfoView.test.tsx (3)
6-35: Good mock implementation for Next.js components and UI elements.The mocks effectively isolate the component under test from its dependencies, allowing for focused testing of the PanelInfoView component's behavior.
50-100: Comprehensive test for PanelInfoView with back button enabled.This test thoroughly checks all aspects of the component, including the presence of text content, images, and interaction with the back button.
102-107: Good negative test case for PanelInfoView without back button.Testing the absence of elements when disableBack is true is an important edge case to verify.
apps/web/app/(app)/environments/[environmentId]/settings/(account)/profile/loading.test.tsx (1)
1-63: Well-structured test for the loading component.This test effectively verifies that the loading state renders correctly with all expected elements and proper props passed to child components. The assertions cover all the key aspects of the component.
apps/web/app/(app)/environments/[environmentId]/settings/(account)/layout.test.tsx (2)
76-79: Good test for successful rendering scenario.This test correctly verifies that the layout component renders its children when all required data is available.
81-94: Thorough error handling tests.The tests for error cases when organization, project, or session is not found are well-implemented and verify that the correct error messages are thrown.
apps/web/app/(app)/environments/[environmentId]/settings/components/SettingsCard.test.tsx (1)
1-72: Well-structured test suite with comprehensive coverage!The test suite effectively verifies all key aspects of the SettingsCard component including title/description rendering, conditional badge display, padding application, and CSS classes. The mocks for Badge and translation functions are properly implemented.
apps/web/app/(app)/environments/[environmentId]/settings/(organization)/layout.test.tsx (1)
66-98: Well-structured test suite with good error case coverageThe test suite effectively verifies successful rendering when all dependencies are available and correctly handles error cases when essential data is missing. The beforeEach reset ensures test isolation.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/AppTab.test.tsx (2)
43-51: Good test for default rendering stateThe test effectively verifies that the component renders correctly by default with the WebAppTab visible and checks for the presence of option buttons.
53-62: Well-implemented user interaction testThe test correctly verifies that clicking the mobile option button switches the view from WebAppTab to MobileAppTab.
apps/web/app/(app)/environments/[environmentId]/settings/(organization)/general/loading.test.tsx (4)
8-10: Good use of mock for constantsThe mock implementation for
IS_FORMBRICKS_CLOUDeffectively ensures the component is tested in non-cloud mode, allowing verification of conditional rendering behaviors.
16-21: Well-structured mock for nested componentsThe comprehensive mocking of
OrganizationSettingsNavbarcaptures not just the component replacement but also tracks calls to the component, enabling verification of props in test assertions.
23-30: Effective mock of LoadingCard componentThe mock implementation of
LoadingCardcaptures title and description props, allowing assertions against these values in the tests.
44-66: Thorough test of loading stateThis test case effectively verifies all key aspects of the Loading component:
- Component rendering
- Proper props passing to child components
- Presence of all expected text elements
- Correct loading state configuration
Good job ensuring the loading component properly handles the loading state and passes it down to children.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/LinkTab.test.tsx (6)
8-18: Excellent mock with data-testid attributesThe mock for
ShareSurveyLinkis well-implemented with data-testid attributes, making it easy to verify that the component receives and displays the expected props correctly.
37-49: Good use of mock data for surveyThe mock survey object is properly constructed with all necessary properties, which ensures realistic testing conditions.
56-72: Well-structured test data for documentation linksThe
docsLinksExpectedarray is an excellent example of defining test expectations separately from the test cases themselves, which improves readability and maintainability.
80-93: Clean, focused test for title renderingThis test case is concise and focused on a single assertion, following good testing practices.
95-110: Comprehensive props verificationThe test effectively verifies that all necessary props are correctly passed to the
ShareSurveyLinkcomponent using data-testid attributes.
127-154: Thorough verification of documentation linksThis test case comprehensively verifies:
- All documentation links are rendered
- Links have correct attributes (href, target)
- Descriptions are properly displayed
- Translation functions are called with correct keys
Excellent attention to detail in testing these UI elements.
apps/web/app/(app)/environments/[environmentId]/settings/(organization)/components/OrganizationSettingsNavbar.test.tsx (4)
16-23: Smart approach to capturing component propsYour mock implementation of
SecondaryNavigationintelligently captures the props passed to it for later inspection, which is an excellent testing pattern for verifying component interactions.
35-41: Good use of default propsDefining default props separately keeps the test cases clean and focused on the specific variations being tested, reducing repetition.
43-112: Excellent use of parameterized testingThe parameterized test effectively covers multiple scenarios with different pathname, role, and cloud status combinations. This approach:
- Reduces code duplication
- Makes test cases more readable
- Easily shows the relationship between inputs and expected outcomes
- Clearly documents the component's behavior in different conditions
The detailed expectations for navigation item visibility provide clear documentation of the component's conditional rendering logic.
124-133: Important edge case testingTesting that billing is hidden when loading is true verifies an important edge case in the navigation behavior, ensuring loading states are handled properly.
apps/web/app/(app)/environments/[environmentId]/settings/(account)/components/AccountSettingsNavbar.test.tsx (4)
15-23: Effective translation mockThe mock for the
useTranslatehook is well-implemented with specific key mapping, which allows thorough testing of the component's behavior with translated content.
34-59: Thorough navigation item verificationThis test comprehensively verifies:
- Navigation rendering
- Correct props passing
- Proper "current" state setting based on pathname
- Correct href construction
Using the mock's call arguments to verify all these aspects is an efficient approach.
99-122: Good edge case handlingTesting the behavior with undefined environmentId is important for ensuring the component gracefully handles potential edge cases in the application state.
124-147: Comprehensive null pathname handlingTesting with an empty pathname verifies that the component handles this edge case appropriately by not marking any navigation item as current, demonstrating thorough test coverage.
apps/web/app/(app)/environments/[environmentId]/settings/(account)/profile/components/DeleteAccount.test.tsx (6)
1-12: Good setup of test environment with appropriate mocksThe test setup properly imports required libraries and creates a mock for the DeleteAccountModal component. This focuses the tests on the DeleteAccount component's behavior in isolation.
14-42: Well-structured mock data objectsThe mock user, session, and organizations objects are properly typed and contain all necessary properties for testing. The use of
as unknown astype assertions is appropriate here to satisfy TypeScript while providing the minimum required mock data.
44-52: Good test lifecycle managementProper use of afterEach for cleanup and beforeEach for resetting mocks ensures test isolation and prevents test interdependencies.
53-69: Comprehensive happy path testingThis test effectively verifies the component's primary function - rendering content and opening the modal on click. The assertions check both UI elements presence and interactive behavior.
71-82: Good edge case handlingTesting the null session case is important as it verifies the component's behavior when session data is unavailable, which can happen in real-world scenarios.
84-97: Effective conditional logic testingThis test appropriately verifies a key business rule - that the delete button's enabled state depends on the multiOrgEnabled flag, even when the user is a single owner of organizations.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/ScrollToTop.test.tsx (6)
1-5: Well-chosen testing librariesGood selection of testing libraries with both fireEvent (for low-level events) and userEvent (for high-level user interactions) imported, giving flexibility in testing approaches.
8-24: Excellent test setup with proper mockingThe test setup thoroughly mocks the DOM environment, creating a controlled container with spies on key methods. The use of beforeEach and afterEach ensures clean test isolation.
26-30: Good initial state verificationVerifying the initial hidden state of the button ensures the component behaves correctly when first mounted, before any user interaction.
32-43: Comprehensive interaction testingThis test effectively simulates the complete interaction flow - scrolling to make the button visible, then clicking it to trigger the scroll-to-top functionality. The verification checks that the expected method is called with the correct parameters.
45-58: Excellent edge case handlingTesting the behavior when the container is not found is crucial for preventing runtime errors. This test ensures graceful degradation when the expected DOM element doesn't exist.
60-67: Good cleanup verificationTesting event listener cleanup on component unmount is essential for preventing memory leaks. This test ensures the component properly removes its scroll event listener when unmounted.
apps/web/app/(app)/environments/[environmentId]/settings/(organization)/enterprise/loading.test.tsx (4)
5-40: Comprehensive mocking setupThe test properly mocks all necessary dependencies, including constants, server-side translation, and child components. This approach isolates the component under test and focuses on its behavior.
42-47: Good test lifecycle managementThe afterEach cleanup and clearAllMocks ensure tests are isolated and prevent test interdependencies.
48-60: Thorough structural testingThis test effectively verifies that all required components are rendered with the correct props, ensuring the loading page structure is maintained.
62-68: Good visual loading state testingThis test verifies the presence of animated loading placeholders, which is crucial for user experience during loading. The approach of checking for the animate-pulse class on multiple elements is appropriate.
apps/web/app/(app)/environments/[environmentId]/settings/(organization)/enterprise/page.test.tsx (3)
1-111: Extensive and thorough dependency mockingThe test includes comprehensive mocks for all dependencies, including database access, authentication, navigation, services, and UI components. This isolation ensures the test focuses solely on the component's behavior.
113-160: Well-structured mock dataThe mock objects for environment, user, organization, and membership are properly typed and contain all necessary properties for testing. This provides a realistic testing environment.
161-180: Good test setup with dependency injectionThe beforeEach function properly configures all mocked dependencies to return consistent test data, and afterEach ensures proper cleanup. This approach maintains test isolation.
apps/web/app/(app)/environments/[environmentId]/settings/(organization)/general/components/EditOrganizationNameForm.test.tsx (2)
48-83: Test appears comprehensive and well-structuredThis test thoroughly checks the form's rendering, user interactions, and successful update flow with appropriate assertions for form state changes.
78-83: 🛠️ Refactor suggestionAdd explicit mock for toast functions
The test expects
toast.successto be called but doesn't explicitly mock it. This could lead to test failures if the toast implementation changes.Add the following to the top of your test file:
// Mock toast vi.mock("react-hot-toast", () => ({ default: { success: vi.fn(), error: vi.fn(), }, }));⛔ Skipped due to learnings
Learnt from: victorvhs017 PR: formbricks/formbricks#5592 File: apps/web/modules/survey/editor/components/hidden-fields-card.test.tsx:3-4 Timestamp: 2025-04-30T18:39:18.464Z Learning: The Formbricks project uses a vitestSetup file that includes global mocks for toast functions from react-hot-toast, eliminating the need to mock them in individual test files.apps/web/app/(app)/environments/[environmentId]/settings/(organization)/general/components/DeleteOrganization.test.tsx (1)
68-192: Comprehensive test suite with excellent coverageThe test suite thoroughly covers all aspects of the DeleteOrganization component, including UI rendering based on user permissions, modal interactions, form validation, success and error cases, and side effects such as navigation and localStorage management.
apps/web/app/(app)/environments/[environmentId]/settings/(account)/profile/components/EditProfileDetailsForm.test.tsx (1)
116-124: Good form validation testingThis test correctly verifies the button state based on form changes, which is an important UX aspect to validate.
apps/web/app/(app)/environments/[environmentId]/settings/(organization)/billing/loading.test.tsx (1)
53-65: Well-structured component rendering testThis test effectively verifies that the Loading component renders with the correct structure and passes appropriate props to child components.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/EmbedView.test.tsx (1)
1-154: LGTM! Well-structured and comprehensive test suite.The test coverage for the EmbedView component is excellent, covering conditional rendering, user interactions, and styling validation. The mock setup is clean and focused.
apps/web/app/(app)/environments/[environmentId]/settings/(account)/profile/components/AccountSecurity.test.tsx (1)
1-70: LGTM! Thorough test coverage for the 2FA toggle functionality.The tests properly verify component rendering and modal state changes based on user interactions with the 2FA switch for both enabled and disabled states.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/components/SurveyAnalysisNavigation.test.tsx (1)
126-243: LGTM! Comprehensive test suite with good edge case coverage.The tests thoroughly verify navigation behavior, interval refresh logic, and response count display under various scenarios. The test structure is clean with proper setup and teardown.
apps/web/app/(app)/environments/[environmentId]/settings/(account)/notifications/page.test.tsx (7)
1-11: Dependencies and imports look well-organizedThe imports are clearly organized, separating testing libraries, mock components, and types. Good structure that makes it easy to understand the test dependencies.
12-66: Comprehensive mocking approach with clear structureGreat job setting up the mocks for all external dependencies. The mock implementations provide just enough functionality to test the component in isolation. The approach of mocking entire modules rather than individual functions is appropriate for this kind of unit test.
68-107: Well-structured test data setupThe mock data is comprehensive and structured to match the expected component props. Good job including edge cases in the notification settings (like the "survey-old" property) which helps test backward compatibility.
127-183: Thorough test of component rendering and data processingThis test comprehensively verifies both UI rendering and the data transformation logic. The assertions against the processed notification settings are particularly valuable for catching reg 8000 ressions in the normalization logic.
185-195: Good error case coverageImportant error cases are covered, ensuring proper error handling when session or user is missing. This helps prevent confusing runtime errors for users.
197-225: Edge case handling for empty dataGood test for the case of empty memberships and default notification settings. This ensures the UI remains stable even with minimal data.
227-258: Legacy format support verificationExcellent test for backward compatibility with legacy notification settings format. This kind of test is crucial when dealing with stored user data that might be in an older format.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/responses/components/ResponseCardModal.test.tsx (10)
1-15: Good component isolation through mockingThe mocking approach effectively isolates the component under test from its dependencies. The SingleResponseCard mock is particularly well-implemented, providing a testable reference without needing to render the actual component.
16-32: Effective UI component mockingThe Button and Modal mocks are well implemented, preserving props and providing test hooks (data-testid) that make assertions clearer and more reliable.
33-132: Comprehensive test data setupThe mock data is thorough and covers all the props required by the component. The use of explicit typing (as unknown as TResponse) is appropriate in this testing context to ensure proper type checking while allowing for partial mock objects.
154-167: Good boundary condition testingThis test correctly verifies that the modal doesn't render when no response is selected, which is an important boundary condition that prevents errors in the UI.
175-189: Thorough navigation behavior testingGood coverage of the navigation functionality through next and back buttons, ensuring the correct response IDs are passed to the state update function.
191-205: Edge case handling for boundary conditionsExcellent test for the boundary condition when the first response is displayed, ensuring the back button is properly disabled.
207-213: Correct behavior verification for modal closingGood verification that closing the modal properly clears the selected response by calling setSelectedResponseId with null.
215-221: State synchronization verificationThis test effectively verifies that the component correctly synchronizes its internal state with the props it receives, an important aspect of stateful components.
223-230: Component lifecycle testingGood test for the component's behavior when props change, specifically when the selected response is cleared after being set. This ensures the modal properly updates its open state.
240-249: Icon validation with Lucide mocking approachThe Lucide icons mocking approach is well implemented, allowing for easy component querying and verification through CSS classes. This is a clean way to test icon presence without complex DOM queries.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/responses/components/ResponsePage.test.tsx (12)
1-54: Thorough dependency mockingThe mocking setup is comprehensive, covering all external dependencies including contexts, actions, and UI components. This allows for proper isolation of the component under test.
55-107: Well-structured mock organizationGood approach to organizing the mocks and test data. The use of
await importwithvi.mockedensures type safety when accessing mock methods, which is an excellent practice.
141-157: Proper test setup and teardownThe test setup and teardown are well structured with appropriate mocking resets between tests to prevent test contamination.
158-167: Basic rendering verificationGood baseline test to verify the component renders correctly with its default props, checking for the presence of key child components and verifying API calls.
169-184: Conditional rendering testsExcellent tests for conditional rendering based on props like
isReadOnlyand context like sharing page parameters. These help ensure the UI adapts correctly to different scenarios.
186-211: Pagination functionality testingThis test effectively verifies the pagination behavior by simulating the effect of the
fetchNextPageprop function and checking that the API is called with the correct offset.
213-248: Response deletion testing approachThe approach to testing response deletion indirectly by checking props passed to child components is appropriate given the component's structure. The nested waitFor pattern correctly handles the asynchronous state updates.
250-284: Response update verificationSimilar to the deletion test, this test effectively verifies the response update functionality by checking the props passed to child components after the update function is called.
286-311: Filter change handlingGood test for the filter change behavior, verifying that pagination is reset and new responses are fetched when filters change. This helps catch regression bugs in the user filtering experience.
313-323: Search param handling testsThe two tests for referer search param behavior effectively verify an important UX detail - whether filter state should be reset based on navigation context.
325-344: Empty response handlingGood test for the edge case of empty responses, ensuring the UI gracefully handles this scenario without errors.
346-373: Error handling testsExcellent tests for API error scenarios, verifying that the component gracefully handles errors from both response and count API calls without crashing.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/responses/components/ResponseTableColumns.test.tsx (9)
1-14: Good import organization and function under testThe imports are well-organized, and the function under test is clearly identified. The file structure makes it easy to understand what's being tested.
15-102: Thorough dependency mockingExcellent mocking of all dependencies needed for column generation, including utilities, components, and icons. The mocks provide just enough implementation to verify the behavior of the column generation function.
103-190: Comprehensive test data setupThe mock survey and response data are well structured, covering various question types, variables, hidden fields, and notes, which provides good coverage for testing the column generation logic.
191-200: Proper test setup and teardownGood use of beforeEach and afterEach hooks to reset mocks and clean up between tests, preventing test contamination.
201-211: Selection column rendering testsThese tests effectively verify the conditional inclusion of the selection column based on the read-only prop, which is an important UI behavior.
213-222: Verified email column testsGood tests for the conditional rendering of the verified email column based on survey configuration, ensuring this UI element only appears when needed.
224-235: Variable column generation testThis test effectively verifies that columns are correctly generated for survey variables with the appropriate accessors and cell rendering.
237-250: Hidden fields column testsGood tests for the conditional generation of hidden field columns, checking both the presence when field IDs exist and absence when they don't.
252-259: Notes column testThis test verifies that the notes column is correctly generated and its cell rendering function correctly processes the note data through the processResponseData utility.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/responses/components/ResponseTable.test.tsx (1)
35-55: Scope the DnD mocks to this test file to prevent cross-suite leakage
vi.mock("@dnd-kit/core" …)and friends are declared at module scope, so their mocked versions persist for all other test files executed in the same worker.
If another suite relies on the real implementation it will silently break.Add an
afterAll(orafterEachwith conditionalvi.unmock) to restore:afterAll(() => { vi.restoreAllMocks(); // or vi.unmock("@dnd-kit/core"); etc. });Alternatively, move the mocks into a
setupTest.tsfile and enable them with thescope: "suite"flag so each suite gets an isolated copy.apps/web/app/(app)/environments/[environmentId]/settings/(account)/profile/page.test.tsx (1)
109-113: Pass the plain params object – avoid nesting it inside aPromise
Pagealready returns a promise (it’s an async RSC).
Wrappingparamsin another promise makes the test harder to read and diverges from the real Next.js contract ({ params: { environmentId: string } }).-render(await Page({ params: Promise.resolve(params) })); +render(await Page({ params }));Unless the page implementation explicitly expects a promise, this change will keep the test closer to production behaviour.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/responses/components/ResponseDataView.test.tsx (1)
277-287: Nice defensive test coverage forformatAddressDataGood job covering both happy-path and edge-case scenarios 👍🏼
apps/web/app/(app)/environments/[environmentId]/settings/(account)/notifications/components/NotificationSwitch.test.tsx (1)
1-4:⚠️ Potential issue
toast.successis not mocked – assertions will always fail
react-hot-toastexports real functions; they are not Jest/Vitest mocks by default.
Everyexpect(toast.success).toHaveBeenCalled…will throw because.successlacks a.mockproperty.Add a mock before importing the component:
-import toast from "react-hot-toast"; +import toast from "react-hot-toast"; + +vi.mock("react-hot-toast", () => ({ + // mimic ES-module default export + default: { + success: vi.fn(), + // add other methods if they are used elsewhere + }, +}));After that, your existing expectations will work without changes.
Also applies to: 22-25
⛔ Skipped due to learnings
Learnt from: victorvhs017 PR: formbricks/formbricks#5592 File: apps/web/modules/survey/editor/components/hidden-fields-card.test.tsx:3-4 Timestamp: 2025-04-30T18:39:18.464Z Learning: The Formbricks project uses a vitestSetup file that includes global mocks for toast functions from react-hot-toast, eliminating the need to mock them in individual test files.apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/responses/page.test.tsx (1)
1-242: Great comprehensive test coverage with proper mocking and assertions!The test file effectively isolates the component under test by mocking all dependencies and properly verifies rendering, prop passing, and error handling. Good job setting up clear test cases that cover both the happy path and error scenarios.
apps/web/app/(app)/environments/[environmentId]/settings/(account)/notifications/components/EditAlerts.test.tsx (1)
1-268: Well-structured tests with thoughtful test cases covering various scenarios!The test suite provides excellent coverage for different data configurations, including edge cases like organizations without surveys. Good job verifying both UI rendering and proper passing of props to child components.
apps/web/app/(app)/environments/[environmentId]/settings/(account)/notifications/components/EditWeeklySummary.test.tsx (1)
1-166: Thorough test coverage with good handling of edge cases!The test suite follows a consistent testing pattern and covers multiple scenarios, including edge cases like empty data. The tests verify both UI rendering and proper prop passing to child components. Good job maintaining consistency with the testing approach across related components.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/WebsiteTab.test.tsx (1)
1-254: Excellent test organization with comprehensive coverage of UI rendering and interactions!The test suite is well-structured with nested describe blocks that logically separate different aspects of the component. Great job testing both rendering and interactive functionality, including tab switching, clipboard operations, and toggle state changes. The detailed verification of specific UI elements and their properties ensures robust test coverage.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/EmailTab.test.tsx (11)
1-8: LGTM! Appropriate imports for component testing.The imports include all necessary testing utilities, user event simulation, toast notifications, and the component under test, providing a solid foundation for comprehensive testing.
10-54: Well-structured comprehensive mocks.The mocks effectively isolate the component under test by mocking external dependencies including actions, UI components, icons, and browser APIs. This is crucial for reliable unit testing.
56-59: Good test data setup with clear sample values.The test data provides clear sample values for surveyId, userEmail, and both raw and cleaned HTML content variations.
61-70: Proper test lifecycle management.The beforeEach and afterEach hooks ensure clean test state by clearing mocks before each test and performing cleanup afterward.
71-95: Initial rendering test is thorough.This test effectively verifies the component's initial state, API calls, and presence of all expected UI elements.
97-129: Comprehensive toggle functionality test.The test effectively verifies the UI state changes when toggling between email preview and embed code views, ensuring all elements appear and disappear appropriately.
131-146: Clipboard functionality properly tested.The test verifies that the correct cleaned HTML is copied to the clipboard and that appropriate feedback is shown to the user.
148-158: Excellent error handling coverage.The tests thoroughly cover success cases and various error scenarios (server errors, authentication errors, and generic errors), ensuring the component handles all potential outcomes.
Also applies to: 160-200
202-206: Loading state properly tested.The test verifies that loading indicators are shown while data is being fetched, which is important for user experience.
208-213: Default value handling tested.The test confirms that a default email is shown when no email is provided, testing an important edge case.
215-232: HTML cleaning functionality thoroughly tested.The test verifies multiple variations of preview parameter patterns are correctly removed from the HTML, which is crucial for the component's proper functioning.
apps/web/app/(app)/environments/[environmentId]/project/look/loading.test.tsx
Outdated
Show resolved
Hide resolved
...ronments/[environmentId]/surveys/[surveyId]/(analysis)/components/EmptyInAppSurveys.test.tsx
Show resolved
Hide resolved
...ironmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/AppTab.test.tsx
Outdated
Show resolved
Hide resolved
...nments/[environmentId]/settings/(account)/profile/components/EditProfileDetailsForm.test.tsx
Outdated
Show resolved
Hide resolved
...nments/[environmentId]/settings/(account)/profile/components/EditProfileDetailsForm.test.tsx
Show resolved
Hide resolved
.../web/app/(app)/environments/[environmentId]/settings/(organization)/billing/loading.test.tsx
Show resolved
Hide resolved
...ts/[environmentId]/surveys/[surveyId]/(analysis)/responses/components/ResponseTable.test.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (11)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/lib/emailTemplate.test.tsx (1)
14-36: Consider abstracting common mock constants to a shared test utilities file.The mock constants defined here are quite extensive and many of them don't appear to be directly used in the tests. If these same constants are mocked across multiple test files, consider extracting them to a shared test utilities file to reduce duplication.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/loading.test.tsx (1)
28-37: Consider using data attributes for testing visual elements.The current approach of filtering divs by their CSS classes couples the test to implementation details that might change. Consider adding data-testid attributes to the animated divs to make the tests more resilient to styling changes.
// In your component -<div className="h-9 w-36 rounded-full bg-slate-200"></div> +<div data-testid="animated-pulse" className="h-9 w-36 rounded-full bg-slate-200"></div> // In your test -const animatedDivs = pulseDivs.filter( - (div) => - div.classList.contains("h-9") && - div.classList.contains("w-36") && - div.classList.contains("rounded-full") && - div.classList.contains("bg-slate-200") -); +const animatedDivs = screen.getAllByTestId("animated-pulse");apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/lib/survey-qr-code.test.tsx (2)
1-97: Great test coverage for error handling scenarios.The test suite thoroughly covers all error paths from the QR code lifecycle (initialization, updating, appending to DOM, and downloading). This defensive programming approach will ensure a robust user experience.
I'd suggest adding one more test case for the happy path:
+ test("should successfully update and append QR code on valid surveyUrl change", () => { + const surveyUrl = "https://example.com/valid-survey"; + const { result } = renderHook(() => useSurveyQRCode(surveyUrl)); + const mockDiv = document.createElement("div"); + + act(() => { + result.current.qrCodeRef.current = mockDiv; + }); + + expect(QRCodeStyling).toHaveBeenCalledTimes(1); + expect(mockUpdate).toHaveBeenCalledWith({ data: surveyUrl }); + expect(mockAppend).toHaveBeenCalledWith(mockDiv); + });
68-80: Avoid usinganytype in TypeScript.Using
anytype loses TypeScript's type checking benefits. Consider creating a proper type or interface for the mock implementation.- }) as any + }) as unknown as QRCodeStylingAnd ensure you properly import the QRCodeStyling type.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/ShareSurveyResults.test.tsx (1)
94-119: Consider breaking down complex test cases.This test covers multiple assertions and user interactions. For better readability and easier debugging, consider splitting it into smaller, focused tests.
- test("renders survey public info when showPublishModal is true and surveyUrl is provided", async () => { + test("renders survey public info elements when showPublishModal is true and surveyUrl is provided", () => { render(<ShareSurveyResults {...defaultProps} showPublishModal={true} surveyUrl={surveyUrl} />); expect(screen.getByText("environments.surveys.summary.survey_results_are_public")).toBeInTheDocument(); expect( screen.getByText("environments.surveys.summary.survey_results_are_shared_with_anyone_who_has_the_link") ).toBeInTheDocument(); expect(screen.getByText(surveyUrl)).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Copy survey link to clipboard" })).toBeInTheDocument(); + expect(screen.getByText("environments.surveys.summary.unpublish_from_web")).toBeInTheDocument(); + expect(screen.getByText("environments.surveys.summary.view_site")).toBeInTheDocument(); + }); + + test("copying link works correctly and shows toast", async () => { + render(<ShareSurveyResults {...defaultProps} showPublishModal={true} surveyUrl={surveyUrl} />); const copyButton = screen.getByRole("button", { name: "Copy survey link to clipboard" }); - expect(copyButton).toBeInTheDocument(); await userEvent.click(copyButton); expect(navigator.clipboard.writeText).toHaveBeenCalledWith(surveyUrl); expect(vi.mocked(toast.success)).toHaveBeenCalledWith("common.link_copied"); + }); + + test("unpublish button calls the handleUnpublish function", async () => { + render(<ShareSurveyResults {...defaultProps} showPublishModal={true} surveyUrl={surveyUrl} />); const unpublishButton = screen.getByText("environments.surveys.summary.unpublish_from_web"); - expect(unpublishButton).toBeInTheDocument(); await userEvent.click(unpublishButton); expect(mockHandleUnpublish).toHaveBeenCalledTimes(1); + }); + + test("view site link has correct attributes", () => { + render(<ShareSurveyResults {...defaultProps} showPublishModal={true} surveyUrl={surveyUrl} />); const viewSiteLink = screen.getByText("environments.surveys.summary.view_site"); - expect(viewSiteLink).toBeInTheDocument(); const anchor = viewSiteLink.closest("a"); expect(anchor).toHaveAttribute("href", surveyUrl); expect(anchor).toHaveAttribute("target", "_blank"); expect(anchor).toHaveAttribute("rel", "noopener noreferrer"); });apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/lib/utils.test.ts (1)
5-27: Use Math.PI instead of numeric constants.The static analysis is flagging the use of approximate numeric constants like 3.14159, which is a common code smell. Use Math.PI for better precision and clarity.
describe("convertFloatToNDecimal", () => { test("should round to N decimal places", () => { - expect(convertFloatToNDecimal(3.14159, 2)).toBe(3.14); - expect(convertFloatToNDecimal(3.14159, 3)).toBe(3.142); + expect(convertFloatToNDecimal(Math.PI, 2)).toBe(3.14); + expect(convertFloatToNDecimal(Math.PI, 3)).toBe(3.142); expect(convertFloatToNDecimal(3.1, 2)).toBe(3.1); expect(convertFloatToNDecimal(3, 2)).toBe(3); expect(convertFloatToNDecimal(0.129, 2)).toBe(0.13); }); test("should default to 2 decimal places if N is not provided", () => { - expect(convertFloatToNDecimal(3.14159)).toBe(3.14); + expect(convertFloatToNDecimal(Math.PI)).toBe(3.14); }); }); describe("convertFloatTo2Decimal", () => { test("should round to 2 decimal places", () => { - expect(convertFloatTo2Decimal(3.14159)).toBe(3.14); + expect(convertFloatTo2Decimal(Math.PI)).toBe(3.14); expect(convertFloatTo2Decimal(3.1)).toBe(3.1); expect(convertFloatTo2Decimal(3)).toBe(3); expect(convertFloatTo2Decimal(0.129)).toBe(0.13); }); });🧰 Tools
🪛 Biome (1.9.4)
[error] 8-8: Prefer constants from the standard library.
Unsafe fix: Use Math.PI instead.
(lint/suspicious/noApproximativeNumericConstant)
[error] 9-9: Prefer constants from the standard library.
Unsafe fix: Use Math.PI instead.
(lint/suspicious/noApproximativeNumericConstant)
[error] 9-9: Prefer constants from the standard library.
Unsafe fix: Use Math.PI instead.
(lint/suspicious/noApproximativeNumericConstant)
[error] 16-16: Prefer constants from the standard library.
Unsafe fix: Use Math.PI instead.
(lint/suspicious/noApproximativeNumericConstant)
[error] 22-22: Prefer constants from the standard library.
Unsafe fix: Use Math.PI instead.
(lint/suspicious/noApproximativeNumericConstant)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/SuccessMessage.test.tsx (1)
119-124: Consider reducing implementation detail coupling.These tests are coupled to implementation details like specific toast message parameters. This could make tests brittle if these details change.
Consider focusing on behavior rather than implementation details:
- expect(toast.success).toHaveBeenCalledWith("environments.surveys.summary.almost_there", { - id: "survey-publish-success-toast", - icon: "🤏", - duration: 5000, - position: "bottom-right", - }); + expect(toast.success).toHaveBeenCalledWith("environments.surveys.summary.almost_there", expect.any(Object)); + // If needed, verify only critical properties: + expect(toast.success.mock.calls[0][1]).toHaveProperty("position", "bottom-right");This approach makes the tests more resilient to non-functional changes.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/SummaryList.test.tsx (1)
73-73: Note different parameter structure for HiddenFieldsSummary mockWhile most mocked summary components receive and use
questionSummary.question.id, the HiddenFieldsSummary usesquestionSummary.iddirectly. This appears intentional based on the component structure, but it's worth documenting with a comment to explain the different parameter structure.vi.mock( "@/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/HiddenFieldsSummary", () => ({ + // HiddenFieldsSummary uses a different structure than other summary components HiddenFieldsSummary: vi.fn(({ questionSummary }) => ( <div>Mocked HiddenFieldsSummary: {questionSummary.id}</div> )), }) );apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/page.test.tsx (2)
18-42: Consider using constants for mock valuesThe constants mock contains many hardcoded values. While they work for testing purposes, it would be cleaner to define these as named constants at the top of the file for better maintainability.
+ const MOCK_CONSTANTS = { + POSTHOG_API_KEY: "mock-posthog-api-key", + POSTHOG_HOST: "mock-posthog-host", + ENCRYPTION_KEY: "mock-encryption-key", + ENTERPRISE_LICENSE_KEY: "mock-enterprise-license-key", + // ... other constants + }; vi.mock("@/lib/constants", () => ({ IS_FORMBRICKS_CLOUD: false, - POSTHOG_API_KEY: "mock-posthog-api-key", - POSTHOG_HOST: "mock-posthog-host", + POSTHOG_API_KEY: MOCK_CONSTANTS.POSTHOG_API_KEY, + POSTHOG_HOST: MOCK_CONSTANTS.POSTHOG_HOST, // ... other constants }));
243-252: Consider using more explicit error handling pattern in testThis test relies on a try/catch block to verify an error is thrown, but doesn't explicitly assert that the rendering attempt fails. Consider using a more explicit pattern like
expect(() => render(...)).toThrow().test("throws error if survey is not found", async () => { vi.mocked(getSurvey).mockResolvedValue(null); const params = Promise.resolve({ environmentId: mockEnvironmentId, surveyId: mockSurveyId }); - try { - // We need to await the component itself because it's an async component - const SurveyPageComponent = await SurveyPage({ params }); - render(SurveyPageComponent); - } catch (e: any) { - expect(e.message).toBe("common.survey_not_found"); - } + // We need to await the component itself because it's an async component + const getComponent = async () => { + const SurveyPageComponent = await SurveyPage({ params }); + render(SurveyPageComponent); + }; + await expect(getComponent()).rejects.toThrow("common.survey_not_found"); // Ensure notFound was not called for this specific error expect(vi.mocked(notFound)).not.toHaveBeenCalled(); });apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/ShareEmbedSurvey.test.tsx (1)
233-241: Clarify expected behavior in test commentThe test comment indicates the current behavior, but doesn't clarify if this is the expected behavior. Consider updating the comment to clarify whether this test is validating correct behavior or documenting a potential issue.
- test("useEffect does not change activeId if survey.type changes from web to link (while in embed view)", () => { + test("activeId remains consistent when survey.type changes while in embed view", () => { const { rerender } = render( <ShareEmbedSurvey {...defaultProps} survey={mockSurveyWeb} modalView="embed" /> ); expect(vi.mocked(mockEmbedViewComponent).mock.calls[0][0].activeId).toBe("app"); rerender(<ShareEmbedSurvey {...defaultProps} survey={mockSurveyLink} modalView="embed" />); - expect(vi.mocked(mockEmbedViewComponent).mock.calls[1][0].activeId).toBe("app"); // Current behavior + // When survey type changes, we maintain the current tab selection for better UX + expect(vi.mocked(mockEmbedViewComponent).mock.calls[1][0].activeId).toBe("app"); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/web/app/(app)/environments/[environmentId]/project/look/loading.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/settings/(account)/profile/components/EditProfileDetailsForm.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/layout.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/responses/page.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/ShareEmbedSurvey.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/ShareSurveyResults.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/SuccessMessage.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/SummaryList.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/AppTab.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/lib/emailTemplate.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/lib/get-qr-code-options.test.ts(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/lib/survey-qr-code.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/lib/surveySummary.test.ts(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/lib/utils.test.ts(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/loading.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/page.test.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/responses/page.test.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/app/(app)/environments/[environmentId]/project/look/loading.test.tsx
- apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedModal/AppTab.test.tsx
- apps/web/app/(app)/environments/[environmentId]/settings/(account)/profile/components/EditProfileDetailsForm.test.tsx
🧰 Additional context used
🧬 Code Graph Analysis (3)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/layout.test.tsx (1)
apps/web/modules/auth/lib/authOptions.ts (1)
authOptions(16-247)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/lib/get-qr-code-options.test.ts (1)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/lib/get-qr-code-options.ts (1)
getQRCodeOptions(3-36)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/lib/survey-qr-code.test.tsx (1)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/lib/survey-qr-code.tsx (1)
useSurveyQRCode(9-44)
🪛 Biome (1.9.4)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/lib/utils.test.ts
[error] 8-8: Prefer constants from the standard library.
Unsafe fix: Use Math.PI instead.
(lint/suspicious/noApproximativeNumericConstant)
[error] 9-9: Prefer constants from the standard library.
Unsafe fix: Use Math.PI instead.
(lint/suspicious/noApproximativeNumericConstant)
[error] 9-9: Prefer constants from the standard library.
Unsafe fix: Use Math.PI instead.
(lint/suspicious/noApproximativeNumericConstant)
[error] 16-16: Prefer constants from the standard library.
Unsafe fix: Use Math.PI instead.
(lint/suspicious/noApproximativeNumericConstant)
[error] 22-22: Prefer constants from the standard library.
Unsafe fix: Use Math.PI instead.
(lint/suspicious/noApproximativeNumericConstant)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Build Formbricks-web / Build Formbricks-web
- GitHub Check: Run Unit Tests / Unit Tests
- GitHub Check: Run E2E Tests / Run E2E Tests
- GitHub Check: SonarQube
- GitHub Check: Validate Docker Build
- GitHub Check: Analyze (javascript)
- GitHub Check: Tusk Tester
🔇 Additional comments (20)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/lib/get-qr-code-options.test.ts (1)
1-58: Well-structured and comprehensive test suite.The test cases effectively verify both the full structure of the QR code options object and specific key properties with different input dimensions. Good job covering both scenarios with appropriate assertions.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/layout.test.tsx (1)
1-124: Well-organized test suite with good coverage of component and metadata generation.This test suite effectively covers multiple scenarios for the
generateMetadatafunction:
- When session and survey exist (happy path)
- When survey is null
- When session does not exist
The mock setup is thorough and the assertions verify both the returned values and that dependencies are called with correct parameters.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/lib/emailTemplate.test.tsx (1)
1-170: Comprehensive test coverage for email template generation.The test suite effectively covers both the happy path and error scenarios. Good job verifying that the function correctly calls all dependent services with the right parameters and handles error cases appropriately.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/loading.test.tsx (1)
1-39: Good test for loading state UI component.The test effectively verifies the rendering of the loading component, including the correct page title, skeleton loader, and animated elements. The component mocking approach isolates the component under test appropriately.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/ShareSurveyResults.test.tsx (1)
1-137: Well-structured test suite with comprehensive coverage.The tests cover all main component states and user interactions, making this a robust test suite. The detailed mocking of dependencies is thorough.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/lib/utils.test.ts (2)
181-202: Verify expected behavior for non-existent questionIds.The test confirms that when a questionId doesn't exist in the survey, the function uses a questionIdx of 0. Please confirm if this is the intended behavior or if it should handle missing questions differently.
Is falling back to questionIdx 0 for non-existent questionIds the expected behavior? This might cause confusion by associating filters with the wrong question. Consider if this should be handled differently, perhaps by showing an error or using a special indicator for missing questions.
29-76: Well-structured mock setup for testing.The mock survey and translation function are well-designed to test the toast message construction thoroughly. The code is clear and comprehensive.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/SuccessMessage.test.tsx (1)
1-186: Comprehensive test suite with excellent coverage.The tests thoroughly cover different scenarios based on URL parameters, survey type, and environment setup. The mocking of dependencies and browser APIs is well-implemented.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/SummaryList.test.tsx (5)
1-157: Well-structured comprehensive test mocks setupThe test file includes thorough mocking of all necessary dependencies and child components, which effectively isolates the SummaryList component for testing. The approach of mocking each question type summary component individually is thorough and will help maintain test stability when child components change.
160-205: Good test fixture setup with comprehensive mock dataThe mock environment, survey, and filter data are well-structured and provide all necessary properties for testing. The defaultProps object is particularly helpful for maintaining DRY test code.
207-266: Excellent helper functions for creating test fixturesThe helper functions
createMockQuestionSummaryandcreateMockHiddenFieldSummaryare well-designed to create consistent test data while allowing for type-specific customization. The type map defined intypeToComponentMockNameMapfurther enhances test readability.
285-460: Comprehensive test coverage for component functionalityThe test suite thoroughly covers critical rendering scenarios including empty states, loading states, and various question type renderings. The setFilter tests are particularly valuable as they validate both the filter state management and toast notification functionality.
361-368: Well-structured test for HiddenFieldsSummary componentThis test correctly validates that hidden fields are properly rendered with the appropriate component and data structure, which is different from regular question types.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/page.test.tsx (1)
194-232: Good validation of component props passed to childrenThe test effectively validates that the correct props are passed to child components, ensuring that data is properly propagated through the component hierarchy.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/ShareEmbedSurvey.test.tsx (3)
95-109: Excellent Dialog mock implementationThe mock implementation for the Dialog component is well-designed, capturing the onOpenChange handler for later testing. This approach allows for thorough testing of the component's interaction with the Dialog.
130-147: Well-structured mock implementations for child componentsThe implementations for EmbedView and PanelInfoView mocks are thorough, providing visibility into the props passed and enabling interaction testing.
149-192: Comprehensive user interaction testingThe tests thoroughly cover user interactions, ensuring that clicking various buttons correctly updates component state and triggers the appropriate callbacks.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/lib/surveySummary.test.ts (3)
158-195: Well-structured tests for getSurveySummaryMetaThe tests thoroughly cover the calculation of survey meta metrics, including handling of edge cases like zero responses and zero display counts.
197-326: Comprehensive tests for getSurveySummaryDropOffThe tests thoroughly cover drop-off analysis, including logic jump handling and welcome card scenarios. The use of mock implementations for evaluateLogic and performActions is particularly effective.
328-403: Good coverage for question summary generationThe tests validate the generation of question summaries for different question types, ensuring that each type is properly handled.
...environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/lib/surveySummary.test.ts
Show resolved
Hide resolved
...environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/lib/surveySummary.test.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/components/CustomFilter.test.tsx (1)
190-190: Same issue with button selectionSimilar to the previous comment, the same approach is used here which could be improved.
🧹 Nitpick comments (6)
apps/web/app/(app)/environments/[environmentId]/surveys/loading.test.tsx (1)
10-10: Minor naming inconsistency in the test suite description.The describe block mentions "SurveyListLoadingPage" while the component being tested is named "SurveyListLoading". Consider updating the test description to match the component name for better clarity.
-describe("SurveyListLoadingPage Re-export", () => { +describe("SurveyListLoading Re-export", () => {apps/web/app/(app)/environments/page.test.tsx (1)
6-8: Consider adding a beforeEach to reset mocks between tests.While this works perfectly for a single test, as more tests are added to this suite in the future, it would be beneficial to reset the mock between tests to prevent potential issues with shared state.
describe("Page", () => { + beforeEach(() => { + vi.mocked(redirect).mockClear(); + }); + afterEach(() => { cleanup(); });apps/web/app/(app)/environments/[environmentId]/surveys/page.test.tsx (1)
16-16: Avoid using type assertions withany.Type assertions using
anycan hide potential type mismatches and may lead to runtime errors. Consider creating properly typed mock parameters instead.- const { getByTestId } = render(<SurveysPage params={undefined as any} searchParams={undefined as any} />); + const { getByTestId } = render(<SurveysPage params={{}} searchParams={{}} />);apps/web/app/(app)/environments/[environmentId]/page.test.tsx (1)
50-50: Consider reducing use ofas anytype assertions.While
as anytype assertions are common in tests to simplify mocking, they bypass TypeScript's type checking. Consider using more specific type assertions or creating proper mock objects that satisfy the required interfaces.Also applies to: 60-60, 67-67, 76-76, 88-88, 97-97, 109-109, 127-127
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/components/CustomFilter.test.tsx (2)
58-58: Remove NOSONAR comments if not requiredThe NOSONAR comments on lines 58 and 63 appear unnecessary unless there's a specific SonarQube rule being suppressed.
- data-testid="calendar-hover-day" // NOSONAR + data-testid="calendar-hover-day"- data-testid="calendar-leave-day" // NOSONAR + data-testid="calendar-leave-day"Also applies to: 63-63
150-152: Consider testing dropdown trigger with findByRole instead of closestUsing
.closest("button")!might be fragile if the DOM structure changes. Consider using more semantic selectors likefindByRolefor better test stability.- const dropdownTrigger = screen.getByText("environments.surveys.summary.all_time").closest("button")!; - // Similar to above, assuming direct clickability. - await user.click(dropdownTrigger); + const dropdownTrigger = screen.getByRole("button", { name: /environments\.surveys\.summary\.all_time/i }); + await user.click(dropdownTrigger);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/app/(app)/environments/[environmentId]/page.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/lib/surveySummary.test.ts(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/components/CustomFilter.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/page.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/loading.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/page.test.tsx(1 hunks)apps/web/app/(app)/environments/page.test.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/lib/surveySummary.test.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
apps/web/app/(app)/environments/[environmentId]/surveys/page.test.tsx (1)
apps/web/modules/survey/list/page.tsx (1)
SurveysPage(32-121)
apps/web/app/(app)/environments/[environmentId]/page.test.tsx (3)
apps/web/modules/environments/lib/utils.ts (1)
getEnvironmentAuth(23-79)packages/types/memberships.ts (1)
TMembership(16-16)apps/web/lib/membership/service.ts (1)
getMembershipByUserIdOrganizationId(14-47)
🪛 Biome (1.9.4)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/components/CustomFilter.test.tsx
[error] 184-184: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Tusk Tester
🔇 Additional comments (21)
apps/web/app/(app)/environments/[environmentId]/surveys/loading.test.tsx (1)
1-15: Well-structured test for component re-export!The test correctly verifies that the
SurveyListLoadingcomponent is properly re-exported from the module. The approach of mocking the original component and checking strict equality is an effective way to ensure that the re-export works correctly without modification.apps/web/app/(app)/environments/page.test.tsx (2)
1-19: LGTM! The test properly verifies the component's redirect behavior.The test is well-structured and effectively verifies that the
Pagecomponent redirects to the root path when rendered. The use of Vitest for mocking theredirectfunction from Next.js navigation is appropriate, and the test follows good practices with proper cleanup after each test.
15-18: Test name matches the assertion well.The test name "should redirect to /" clearly communicates what functionality is being tested, which is good practice for maintaining readable and maintainable tests.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/page.test.tsx (1)
1-23: Well-structured test for the survey page redirect.This test effectively verifies that the Survey page component redirects to the summary page with the correct URL parameters. The test structure is clean and follows best practices for testing Next.js navigation components.
apps/web/app/(app)/environments/[environmentId]/surveys/page.test.tsx (2)
5-8: Good use of module mocking.The mock implementation of the SurveysPage component is simple and effective for the test's purpose. This approach correctly isolates the test from the actual implementation details.
10-24: Comprehensive test coverage for the page component.The tests effectively verify both the rendering behavior and the metadata export functionality. This aligns well with the PR objectives to increase test coverage for the environments path.
apps/web/app/(app)/environments/[environmentId]/page.test.tsx (8)
1-8: Well-organized imports and effective type utilization.The imports are well-structured with a logical organization of dependencies, and the types from
@formbricks/typesare appropriately imported for use in the test mocks.
10-24: Good mocking approach for dependencies.The test file correctly mocks all necessary dependencies for isolated testing of the EnvironmentPage component.
26-61: Well-structured test setup with comprehensive test data.The test setup includes:
- Proper cleanup with
afterEach- Well-defined mock data for environment, user, and organization
- Thorough session mock with all required properties
This creates a solid foundation for the test cases.
62-81: Good test coverage for billing access scenario.This test effectively verifies that users with billing access are redirected to the billing settings page, properly mocking the authentication and access flag dependencies.
83-102: Good test coverage for non-billing access scenario.This test effectively verifies that users without billing access are redirected to the surveys page, maintaining consistency with the application's access control logic.
104-120: Good edge case handling for null session.This test properly verifies the application behavior when no user session is present, ensuring graceful handling of this edge case.
122-137: Good edge case handling for null membership.This test properly verifies the application behavior when the user has no membership in the organization, ensuring consistent redirection logic.
1-138: Comprehensive test coverage aligned with PR objectives.This test file successfully addresses the PR objective of adding tests to the environments path. The tests verify critical redirection logic based on user roles and permissions, covering both normal scenarios and edge cases. The thorough mocking strategy ensures tests run in isolation.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/components/CustomFilter.test.tsx (7)
1-13: LGTM: All necessary imports are includedThe imports include all required dependencies for testing the CustomFilter component, including testing utilities, mocked context, and required type definitions.
14-86: LGTM: Well-structured mocks for all dependenciesThe mock implementations are comprehensive and properly isolate the component under test. The Calendar mock is particularly well constructed with detailed props handling.
88-110: LGTM: Mock survey object is properly typedThe mock survey object is correctly typed as TSurvey and includes all necessary properties for testing.
112-122: LGTM: Well-structured state initializationGood practice using a function for initial state and resetting between tests to avoid test interdependencies.
123-138: LGTM: Proper test setup and teardownThe test suite includes appropriate cleanup and reset of mocks before each test, ensuring test isolation.
140-168: LGTM: Tests for rendering and conditional UI behaviorThese tests effectively verify the component renders correctly with initial props and validates conditional UI behavior like hiding the download button on sharing pages.
170-180: LGTM: Thorough testing of component effect behaviorGood test for verifying the component's React effects and checking that resetState is not called unexpectedly when rerendering with different surveys.
...b/app/(app)/environments/[environmentId]/surveys/[surveyId]/components/CustomFilter.test.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (11)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/components/SurveyStatusDropdown.test.tsx (4)
40-42: Consider testing the updateSurveyAction callWhile you've mocked the updateSurveyAction, there's no test verifying that it gets called with the correct parameters when a status change occurs. Consider adding a test that triggers the status change and verifies the action is called correctly.
test("calls updateSurveyAction with correct parameters when status changes", async () => { const { updateSurveyAction } = await import("../actions"); render( <SurveyStatusDropdown environment={mockEnvironment} survey={{ ...baseSurvey, status: "inProgress" }} /> ); screen.getByTestId("select-trigger").click(); expect(updateSurveyAction).toHaveBeenCalledWith({ environmentId: "env_1", surveyId: "survey_1", surveyBody: expect.objectContaining({ status: "paused" }) }); });
7-9: Add test for error handlingThe code mocks the error handling utility, but there's no test for how the component behaves when an error occurs during status update. Consider adding a test that simulates an error response from updateSurveyAction.
test("handles errors when updating survey status", async () => { const { updateSurveyAction } = await import("../actions"); (updateSurveyAction as any).mockRejectedValueOnce(new Error("Update failed")); render( <SurveyStatusDropdown environment={mockEnvironment} survey={{ ...baseSurvey, status: "inProgress" }} /> ); screen.getByTestId("select-trigger").click(); // Verify error handling behavior (e.g., error toast, state rollback) // This will depend on how your component handles errors });
133-135: Consider mocking the translation systemThe test is checking for a hardcoded translation key. Consider mocking the translation system to make the tests more resilient to translation key changes.
// Add at the top with other mocks vi.mock("next-intl", () => ({ useTranslations: () => (key) => `Translated: ${key}` })); // Then in the test expect(screen.getByTestId("tooltip-content")).toHaveTextContent( "Translated: environments.surveys.survey_status_tooltip" );
138-148: Add complementary test for future closeOnDateYou've tested the case when closeOnDate is in the past, but there's no test for when it's in the future. Adding this test would complete the coverage for this condition.
test("enables select when closeOnDate is in the future", () => { const futureDate = new Date(); futureDate.setDate(futureDate.getDate() + 1); render( <SurveyStatusDropdown environment={mockEnvironment} survey={{ ...baseSurvey, status: "inProgress", closeOnDate: futureDate }} /> ); expect(screen.getByTestId("select-container")).toHaveAttribute("data-disabled", "false"); });apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/components/ResultsShareButton.test.tsx (2)
85-109: Consider refining the mock survey object.The mock survey object is being cast using
as unknown as TSurveywhich suggests it might not fully conform to the TSurvey type. Additionally, the welcomeCard property has an explicit type cast.Consider creating a more type-safe mock by ensuring all required properties match the TSurvey type:
const mockSurvey = { id: "survey1", name: "Test Survey", type: "app", status: "inProgress", questions: [], hiddenFields: { enabled: false }, displayOption: "displayOnce", recontactDays: 0, autoClose: null, delay: 0, autoComplete: null, surveyClosedMessage: null, singleUse: null, resultShareKey: null, languages: [], triggers: [], - welcomeCard: { enabled: false } as TSurvey["welcomeCard"], + welcomeCard: { enabled: false, headline: "", html: "" }, styling: null, createdAt: new Date(), updatedAt: new Date(), environmentId: "env1", variables: [], closeOnDate: null, - } as unknown as TSurvey; + } satisfies Partial<TSurvey>;
1-258: Consider adding error handling tests.The current tests focus primarily on the happy path. To improve test coverage, consider adding tests for error scenarios such as API call failures.
For example, you could add a test like:
test("handles error when publishing to web", async () => { const errorMessage = "Failed to generate share URL"; mockGetResultShareUrlAction.mockResolvedValue({ data: null }); mockGenerateResultShareUrlAction.mockRejectedValue(new Error(errorMessage)); render(<ResultsShareButton survey={mockSurvey} webAppUrl={webAppUrl} />); fireEvent.click(screen.getByTestId("dropdown-menu-trigger")); const publishButton = (await screen.findAllByTestId("dropdown-menu-item")).find((item) => item.textContent?.includes("environments.surveys.summary.publish_to_web") ); await userEvent.click(publishButton!); expect(screen.getByTestId("share-survey-results-modal")).toBeInTheDocument(); await userEvent.click(screen.getByTestId("handle-publish-button")); expect(mockGenerateResultShareUrlAction).toHaveBeenCalledWith({ surveyId: mockSurvey.id }); expect(mockToastError).toHaveBeenCalledWith(errorMessage); });This would verify that errors are properly handled and displayed to the user.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/components/ResponseFilter.test.tsx (5)
9-31: Consider reducing the scope of mocked constantsThe test mocks many constants from
@/lib/constantsthat might not be directly relevant to testing the ResponseFilter component. Consider mocking only the constants that are actually used by the component or its dependencies to make the test more focused and maintainable.vi.mock("@/lib/constants", () => ({ IS_FORMBRICKS_CLOUD: false, POSTHOG_API_KEY: "mock-posthog-api-key", POSTHOG_HOST: "mock-posthog-host", IS_POSTHOG_CONFIGURED: true, - ENCRYPTION_KEY: "mock-encryption-key", - ENTERPRISE_LICENSE_KEY: "mock-enterprise-license-key", - GITHUB_ID: "mock-github-id", - GITHUB_SECRET: "test-githubID", - GOOGLE_CLIENT_ID: "test-google-client-id", - GOOGLE_CLIENT_SECRET: "test-google-client-secret", - AZUREAD_CLIENT_ID: "test-azuread-client-id", - AZUREAD_CLIENT_SECRET: "test-azure", - AZUREAD_TENANT_ID: "test-azuread-tenant-id", - OIDC_DISPLAY_NAME: "test-oidc-display-name", - OIDC_CLIENT_ID: "test-oidc-client-id", - OIDC_ISSUER: "test-oidc-issuer", - OIDC_CLIENT_SECRET: "test-oidc-client-secret", - OIDC_SIGNING_ALGORITHM: "test-oidc-signing-algorithm", WEBAPP_URL: "test-webapp-url", IS_PRODUCTION: false, - SENTRY_DSN: "mock-sentry-dsn", }));
16-26: Avoid using credential-like patterns in test mocksEven though these are mock values, using patterns that resemble secrets (like client secrets and IDs) could cause confusion or false positives in security scans. Consider using clearly fake values with prefixes like "mock-" for all credentials.
- GITHUB_SECRET: "test-githubID", - GOOGLE_CLIENT_ID: "test-google-client-id", - GOOGLE_CLIENT_SECRET: "test-google-client-secret", - AZUREAD_CLIENT_ID: "test-azuread-client-id", - AZUREAD_CLIENT_SECRET: "test-azure", - AZUREAD_TENANT_ID: "test-azuread-tenant-id", - OIDC_DISPLAY_NAME: "test-oidc-display-name", - OIDC_CLIENT_ID: "test-oidc-client-id", - OIDC_ISSUER: "test-oidc-issuer", - OIDC_CLIENT_SECRET: "test-oidc-client-secret", + GITHUB_SECRET: "mock-github-secret", + GOOGLE_CLIENT_ID: "mock-google-client-id", + GOOGLE_CLIENT_SECRET: "mock-google-client-secret", + AZUREAD_CLIENT_ID: "mock-azuread-client-id", + AZUREAD_CLIENT_SECRET: "mock-azuread-client-secret", + AZUREAD_TENANT_ID: "mock-azuread-tenant-id", + OIDC_DISPLAY_NAME: "mock-oidc-display-name", + OIDC_CLIENT_ID: "mock-oidc-client-id", + OIDC_ISSUER: "mock-oidc-issuer", + OIDC_CLIENT_SECRET: "mock-oidc-client-secret",
104-110: Use proper typing for the mock survey objectThe mock survey object is cast using
as unknown as TSurvey, which bypasses TypeScript's type checking. This could hide potential type mismatches. Consider creating a properly typed mock object.const mockSurvey = { id: "survey1", name: "Test Survey", questions: [{ id: "q1", headline: { default: "Question 1" } }], hiddenFields: { enabled: true, fieldIds: ["hf1"] }, languages: [{ code: "en", default: true, enabled: true }], -} as unknown as TSurvey; + createdAt: new Date(), + updatedAt: new Date(), + // Add other required properties from TSurvey +} satisfies Partial<TSurvey> as TSurvey;
217-240: Test is using hardcoded date valuesThe test is setting fixed dates for the date range without any explanation for why these specific dates were chosen. Consider adding a comment explaining the significance of these dates or using more descriptive variable names.
test("changes date range", async () => { render( <ResponseFilter environmentId="env1" surveyId="survey1" survey={mockSurvey} responses={[]} totalResponseCount={0} appliedFilters={{ filter: [], onlyComplete: false, dateRange: { from: undefined, to: undefined } }} setAppliedFilters={mockSetAppliedFilters} /> ); const openMenuButton = screen.getByText(/^filter-responses\.filter-button-default-text/); await userEvent.click(openMenuButton); const setDateButton = screen.getByText("Set Date Range"); await userEvent.click(setDateButton); + // Using fixed January 2023 dates for testing date range functionality expect(mockUseResponseFilterReturn.setDateRange).toHaveBeenCalledWith({ from: new Date("2023-01-01T00:00:00.000Z"), to: new Date("2023-01-31T00:00:00.000Z"), }); });
242-265: Add assertions for modal visibilityWhile the test checks for the presence and absence of the CustomFilter component, it might be clearer to also assert that the modal's visibility state has changed in the component.
test("shows and hides CustomFilter modal", async () => { render( <ResponseFilter environmentId="env1" surveyId="survey1" survey={mockSurvey} responses={[]} totalResponseCount={0} appliedFilters={{ filter: [], onlyComplete: false, dateRange: { from: undefined, to: undefined } }} setAppliedFilters={mockSetAppliedFilters} /> ); const openMenuButton = screen.getByText(/^filter-responses\.filter-button-default-text/); await userEvent.click(openMenuButton); + // Verify modal is initially not shown + expect(screen.queryByTestId("custom-filter")).not.toBeInTheDocument(); const addFilterButton = screen.getByText("filter-responses.add-filter-button"); await userEvent.click(addFilterButton); expect(screen.getByTestId("custom-filter")).toBeInTheDocument(); const closeCustomFilterButton = screen.getByText("Close CustomFilter"); await userEvent.click(closeCustomFilterButton); expect(screen.queryByTestId("custom-filter")).not.toBeInTheDocument(); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/components/ResponseFilter.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/components/ResultsShareButton.test.tsx(1 hunks)apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/components/SurveyStatusDropdown.test.tsx(1 hunks)apps/web/vite.config.mts(1 hunks)sonar-project.properties(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- sonar-project.properties
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/components/ResultsShareButton.test.tsx (1)
packages/types/surveys/types.ts (1)
TSurvey(2435-2435)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Tusk Tester
🔇 Additional comments (16)
apps/web/vite.config.mts (1)
140-159: Well-structured test coverage exclusion patterns.The additional exclusion patterns appropriately filter out non-application code files from test coverage metrics, which aligns well with the PR objective of increasing test coverage for the environments path. These exclusions prevent test files, mocks, configuration files, and other non-functional assets from artificially affecting coverage statistics.
I particularly appreciate the comprehensive approach to exclude:
- Test-related files (
**/*.test.*,**/*.mock.*,**/mocks/**)- Configuration files (
**/*.config.*)- UI assets (
**/*.css,apps/web/modules/ui/components/icons/*)- Documentation and data files (
**/*.mdx,**/*.json)This ensures that coverage metrics will more accurately reflect the actual application code coverage rather than being diluted by including files that shouldn't count toward functional test coverage.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/components/SurveyStatusDropdown.test.tsx (1)
1-182: Well-structured and comprehensive test suiteThis is a well-organized test suite with good coverage of the SurveyStatusDropdown component's behavior across different scenarios. The mocks are well-implemented and the test cases are clearly described.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/components/ResultsShareButton.test.tsx (12)
1-5: Well-structured test setup with comprehensive imports.The test file is properly set up with all the necessary imports from React Testing Library and Vitest. Good job importing the exact types needed from the types package rather than importing everything.
7-16: Good mock implementation of actions.You've properly mocked the external actions that the component depends on. This isolates the component under test and prevents actual API calls during testing.
18-22: Clean helper function mock.The error formatting helper is properly mocked with a simplified implementation that maintains the expected behavior without depending on the actual implementation.
24-39: Well-structured UI component mocks.The dropdown components are properly mocked with testable data attributes. This approach makes it easy to identify and interact with these components in your tests.
41-83: Comprehensive mocking of dependencies.All external dependencies (Tolgee translations, icons, toast notifications, and the ShareSurveyResults modal) are properly mocked. The ShareSurveyResults mock is particularly well done, providing test-specific actions and data attributes.
114-137: Good test setup and cleanup.The beforeEach and afterEach hooks properly set up and clean up the test environment. Mocking the window.location and navigator.clipboard is well done.
139-149: Good initial state test.This test properly verifies the initial state of the component and that it makes the expected API call to get the share URL.
151-164: Well-structured user interaction test.The test for copying a private link to the clipboard thoroughly checks the user interaction flow and verifies that the expected actions occur.
166-182: Good test for public link handling.This test properly verifies that the public link is copied to the clipboard when the user clicks the appropriate button.
184-207: Comprehensive test for publish functionality.The test for publishing survey results to the web thoroughly checks the user interaction flow, modal display, and API calls.
209-233: Well-structured test for unpublish functionality.The test for unpublishing survey results properly verifies the complete user flow and API interactions.
235-257: Good modal interaction test.This test properly verifies that the ShareSurveyResults modal can be opened and closed correctly.
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/components/ResponseFilter.test.tsx (2)
123-132: LGTM: Test setup and teardownThe beforeEach and afterEach hooks are well-structured, clearing mocks and cleaning up after each test.
389-420: Well structured test for component state transitionThis test thoroughly validates the component's behavior when transitioning from empty to selected filter state, and properly tests the interactions with child components. Good job!
...app/(app)/environments/[environmentId]/surveys/[surveyId]/components/ResponseFilter.test.tsx
Outdated
Show resolved
Hide resolved
...app/(app)/environments/[environmentId]/surveys/[surveyId]/components/ResponseFilter.test.tsx
Outdated
Show resolved
Hide resolved
|
What does this PR do?
Add tests to all files in the apps/web/app/(app)/environments path
How should this be tested?
Run the test command in the web package.
Checklist
Required
pnpm buildconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
Tests
Style
Chores
No changes to user-facing features or exported entities.