Skip to content

Add onboarding images and enhance risk and vendor management layouts - #229

Merged
carhartlewis merged 2 commits into
mainfrom
lewis/ui-fixes
Mar 31, 2025
Merged

carhartlewis merged 2 commits into
mainfrom
lewis/ui-fixes

Conversation

@carhartlewis

@carhartlewis carhartlewis commented Mar 31, 2025 •

Copy link
Copy Markdown
Contributor
  • Added risk-management.webp and vendor-management.webp images for onboarding.
  • Updated middleware to include onboarding routes in the matcher.
  • Enhanced the Layout components for risk and vendor management to include onboarding prompts when no risks or vendors are present.
  • Introduced AppOnboarding component for displaying onboarding information and actions.
  • Implemented loading states for better user experience during data fetching.
  • Refactored risk and vendor overview logic to utilize caching for performance optimization.

Summary by CodeRabbit

  • New Features
    • Enhanced both risk and vendor management views with an interactive onboarding experience for users with no existing data, featuring introductory guides, FAQs, and clear calls-to-action.
    • Introduced a new loading interface to improve responsiveness during organization setup.
    • Added a new AppOnboarding component for a structured onboarding experience.
    • Implemented a new Loading component for displaying loading states in organization settings.
  • Refactor
    • Streamlined risk and vendor management interfaces to provide smoother content loading and more intuitive navigation.
  • Documentation
    • Updated and reorganized translation content to ensure clearer onboarding instructions and enhanced user guidance.

- Added `risk-management.webp` and `vendor-management.webp` images for onboarding.
- Updated middleware to include onboarding routes in the matcher.
- Enhanced the `Layout` components for risk and vendor management to include onboarding prompts when no risks or vendors are present.
- Introduced `AppOnboarding` component for displaying onboarding information and actions.
- Implemented loading states for better user experience during data fetching.
- Refactored risk and vendor overview logic to utilize caching for performance optimization.
@vercel

vercel Bot commented Mar 31, 2025 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 31, 2025 9:58pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
comp-portal ⬜️ Skipped (Inspect) Mar 31, 2025 9:58pm

@coderabbitai

coderabbitai Bot commented Mar 31, 2025 •

Copy link
Copy Markdown

Walkthrough

This pull request updates both risk and vendor management views by introducing conditional onboarding flows. In the risk layout, a new getRiskOverview function checks the number of risks, rendering an onboarding interface when there are none, while the corresponding page eliminates the previous risk-fetching logic. Similarly, the vendor layout now calls a new getVendorOverview to conditionally display onboarding or the main content. Additional changes include a new Loading component, an AppOnboarding component, tool and localization updates, and a minor middleware matcher adjustment.

Changes

File(s) Change Summary
apps/.../risk/(overview)/layout.tsx
apps/.../risk/(overview)/loading.tsx
apps/.../risk/(overview)/page.tsx
Risk Management: Enhanced Layout to conditionally render an onboarding interface using getRiskOverview; new Loading component added; removed risk-fetching and redirection logic from page.
apps/.../vendors/(overview)/layout.tsx
apps/.../vendors/(overview)/page.tsx
apps/.../vendors/(overview)/components/charts/vendor-overview.tsx
Vendor Management: Updated Layout to use getVendorOverview for conditional onboarding rendering; simplified page logic; removed Card wrappers from vendor charts.
apps/.../components/app-onboarding.tsx Introduced a new AppOnboarding component with configurable title, description, CTA, image, and FAQ accordion.
apps/.../components/settings/team/members-list.tsx Applied formatting adjustments for consistent indentation.
apps/.../data/tools/index.ts
apps/.../data/tools/risks.ts
apps/.../data/tools/user.ts
Tools Update: Added new getUserTools and getUser tool; extended getRisks with an optional owner parameter and updated tool integrations.
apps/.../locales/en.ts
apps/.../locales/features/risk.ts
apps/.../locales/onboarding/app-onboarding.ts
Localization: Added onboarding translation entries and reorganized translation sections; minor formatting updates.
apps/.../middleware.ts Modified matcher configuration to include the onboarding route in its processing.

Sequence Diagram(s)

sequenceDiagram
  participant U as User
  participant R as Risk Layout Component
  participant G as getRiskOverview
  participant AO as AppOnboarding
  participant SM as SecondaryMenu

  U->>R: Load risk dashboard
  R->>G: Call getRiskOverview()
  G-->>R: Return risk count (N)
  alt No Risks (count = 0)
    R->>AO: Render onboarding UI
    R->>R: Render CreateRiskSheet
  else Risks exist
    R->>SM: Render SecondaryMenu within Suspense
    R->>R: Render children content within Suspense
  end
Loading
sequenceDiagram
  participant U as User
  participant V as Vendor Layout Component
  participant G as getVendorOverview
  participant AO as AppOnboarding
  participant SM as SecondaryMenu

  U->>V: Load vendor dashboard
  V->>G: Call getVendorOverview()
  G-->>V: Return vendor count (N)
  alt No Vendors (count = 0)
    V->>AO: Render onboarding UI
  else Vendors exist
    V->>SM: Render SecondaryMenu within Suspense
    V->>V: Render main content within Suspense
  end
Loading

Possibly related PRs

Poem

I'm a rabbit of code, hopping with delight,
New flows and features keeping our future bright.
Onboarding blooms when no risks are in sight,
Vendors and risks now dancing in the light.
With every change, our app's taken a giant flight!
🐰💻✨

✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (9)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/(overview)/components/charts/vendor-overview.tsx (1)

4-4: Remove unused Card component imports

The Card-related components are no longer used in the implementation but are still being imported.

-import { Card, CardContent, CardHeader, CardTitle } from "@bubba/ui/card";
apps/app/src/data/tools/user.ts (2)

11-26: Enhance error handling in the getUser tool

While the authentication check works well, consider adding try/catch for handling potential errors from the auth() function call.

export const getUser = tool({
	description: "Get the user's id and organization id",
	parameters: z.object({}),
	execute: async () => {
+		try {
			const session = await auth();

			if (!session?.user.organizationId) {
				return { error: "Unauthorized" };
			}

			return {
				userId: session.user.id,
				organizationId: session.user.organizationId,
			};
+		} catch (error) {
+			console.error("Error fetching user session:", error);
+			return { error: "Failed to authenticate user" };
+		}
	},
});

11-26: Consider defining a consistent return type

The tool currently returns either an error object or a user data object. Consider using a consistent return type to improve type safety.

+type GetUserResult = 
+  | { error: string; userId?: never; organizationId?: never }
+  | { error?: never; userId: string; organizationId: string };

export const getUser = tool({
	description: "Get the user's id and organization id",
	parameters: z.object({}),
-	execute: async () => {
+	execute: async (): Promise<GetUserResult> => {
		const session = await auth();

		if (!session?.user.organizationId) {
			return { error: "Unauthorized" };
		}

		return {
			userId: session.user.id,
			organizationId: session.user.organizationId,
		};
	},
});
apps/app/src/data/tools/risks.ts (1)

41-41: Consider renaming the parameter for consistency.

There's a slight naming inconsistency between the parameter name (owner) and its usage in the database query (ownerId). Consider renaming the parameter to ownerId for better clarity and consistency with the database field.

-		owner: z.string().optional(),
+		ownerId: z.string().optional(),

And update the parameter destructuring:

-	execute: async ({ status, department, category, owner }) => {
+	execute: async ({ status, department, category, ownerId }) => {
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/risk/(overview)/loading.tsx (1)

10-10: Conflicting layout classNames.

The div contains both space-y-12 (which adds vertical spacing between children) and grid with gap-4 (which has its own spacing). This could lead to inconsistent spacing in the layout.

-    <div className="space-y-12 grid grid-cols-1 md:grid-cols-2 gap-4">
+    <div className="grid grid-cols-1 md:grid-cols-2 gap-4">
apps/app/src/components/app-onboarding.tsx (2)

31-53: Good layout and FAQ rendering.
The conditional rendering of FAQs with the Accordion is well organized. Consider adding an ARIA label or accessible heading to clarify that these items are frequently asked questions.


71-75: CTA button accessibility.
The button approach is clean, but adding a brief aria-label or descriptive text for assistive technologies could further improve accessibility for the plus icon.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/(overview)/layout.tsx (1)

78-98: Consider data revalidation strategy.
getVendorOverview uses React’s cache, which might not automatically revalidate after a new vendor is created. Consider manually invalidating or refreshing the cached data post-creation to reflect changes immediately.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/risk/(overview)/layout.tsx (1)

75-95: Caching considerations.
Like with vendors, the caching mechanism here might need a forced refresh once a new risk is created. Align the revalidation strategy with common patterns to ensure the UI stays in sync.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9e5e744 and 69853e9.

📒 Files selected for processing (15)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/risk/(overview)/layout.tsx (1 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/risk/(overview)/loading.tsx (1 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/risk/(overview)/page.tsx (0 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/(overview)/components/charts/vendor-overview.tsx (1 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/(overview)/layout.tsx (1 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/(overview)/page.tsx (1 hunks)
  • apps/app/src/components/app-onboarding.tsx (1 hunks)
  • apps/app/src/components/settings/team/members-list.tsx (1 hunks)
  • apps/app/src/data/tools/index.ts (1 hunks)
  • apps/app/src/data/tools/risks.ts (2 hunks)
  • apps/app/src/data/tools/user.ts (1 hunks)
  • apps/app/src/locales/en.ts (1 hunks)
  • apps/app/src/locales/features/risk.ts (1 hunks)
  • apps/app/src/locales/onboarding/app-onboarding.ts (1 hunks)
  • apps/app/src/middleware.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/risk/(overview)/page.tsx
🔇 Additional comments (28)
apps/app/src/components/settings/team/members-list.tsx (6)

6-12: Improved import formatting for card components.
The updated named import list for the card components (“Card”, “CardContent”, etc.) enhances readability and consistency.


18-34: Clear and well-typed OrganizationMember interface.
The interface now explicitly defines all required properties, which improves type safety. Ensure that the use of Date for joinedAt and lastActive aligns with how these fields are handled during serialization/deserialization.


36-40: Consistent MembersListProps interface formatting.
The updated indentation and structure make the interface clearer.


42-61: Readable MembersList component for missing organization case.
The component now cleanly checks for !hasOrganization and renders a localized Card with header and description. The formatting updates improve clarity.


64-76: Effective handling of empty members list.
The conditional check (members.length === 0) and corresponding Card display are clearly structured and use i18n translations appropriately.


160-173: Clean role-to-icon mapping implementation.
The getMemberRoleIcon function is straightforward and leverages a switch-case to select the appropriate icon. This aids in keeping the UI consistent with minimal complexity.

apps/app/src/data/tools/index.ts (1)

4-4: LGTM! Good addition of user tools

The user tools integration follows the established pattern for tools in this application. This change effectively expands the available toolset with user-related functionality.

Also applies to: 10-10

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/(overview)/components/charts/vendor-overview.tsx (1)

17-24: UI structure simplified by removing Card components

The vendor overview UI has been streamlined by removing the Card wrapper components while maintaining the same grid layout and functionality.

apps/app/src/locales/features/risk.ts (1)

1-185: Formatting improvements to risk localization file

The changes improve readability through consistent indentation and formatting while maintaining the same semantic content.

apps/app/src/data/tools/user.ts (1)

5-9: Good implementation of the getUserTools function

The function follows the same pattern used by other tool functions in the application, making it consistent and easy to understand.

apps/app/src/locales/onboarding/app-onboarding.ts (1)

1-45: Well-structured localization content for onboarding experiences.

The localization data is well-organized with consistent structure between risk management and vendor management sections. The content provides clear, informative explanations about both domains and their relevance to SOC 2 compliance.

apps/app/src/middleware.ts (1)

5-8: Onboarding path excluded from middleware processing.

The matcher pattern has been updated to include "onboarding" in the exclusion list, meaning the middleware will not process requests to paths containing "onboarding". This aligns with the PR's objective of enhancing the onboarding experience by allowing onboarding routes to bypass this middleware.

apps/app/src/data/tools/risks.ts (2)

26-27: Parameter added for owner-based filtering.

The addition of an optional owner parameter enhances the flexibility of the risk filtering functionality.


28-28: Added owner parameter to execute function.

The owner parameter is correctly destructured in the execute function.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/risk/(overview)/loading.tsx (1)

1-44: Loading component displays organization settings instead of risk-related content.

The loading component for the risk overview page displays organization settings (name and website) rather than risk management related content. This seems inconsistent with the context of a risk management page.

Consider replacing the content with risk-specific loading placeholders, such as:

  • Risk overview cards
  • Risk metrics or statistics
  • Risk table/list with skeleton loaders

This would provide a more consistent user experience that matches the expected content of the risk management page.

apps/app/src/components/app-onboarding.tsx (2)

1-10: Imports and client setup look good.
All imported packages and the use client directive are appropriate for a client-side component. Overall, this file is well-structured at the top level.


26-30: Check query state behavior.
Using useQueryState(sheetName) to manage open/close state is a neat approach. Ensure that this query key doesn't conflict with other uses of the same sheet name across the app.

Would you like to quickly verify there are no other inadvertent uses of the same query parameter key? I can provide a script to search the codebase for usage of sheetName.

apps/app/src/locales/en.ts (2)

27-29: New onboarding import is coherent.
Importing app_onboarding aligns well with existing structure. Good job keeping the code organized.


33-67: Translations block is well-organized.
The new app_onboarding property in the translations object is consistent with the existing sections. This promotes readability and maintainability.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/(overview)/layout.tsx (3)

2-8: Imports and references look good.
Bringing in AppOnboarding, database references, React caching tools, and the CreateVendorSheet component is coherent.


19-20: Separation of concerns.
getVendorOverview being called outside the component is a clean approach, nicely separating data fetching from rendering logic.


21-57: Conditional onboarding flow is user-friendly.
Rendering AppOnboarding when there are zero vendors provides clear guidance for new users. Good use of Suspense to handle loading states.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/risk/(overview)/layout.tsx (3)

4-7: Imports are straightforward.
Including the new AppOnboarding and the CreateRiskSheet keep the file focused on risk management logic and user guidance.


19-20: getRiskOverview usage is clean.
Fetching the count of existing risks is well-separated from the rendering logic. Smooth approach for dynamic checks.


21-57: Effective zero-risk onboarding.
Displaying onboarding content when risks === 0 provides clarity for new users who have yet to add any risks. The fallback loading state is also well-handled.

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/(overview)/page.tsx (3)

8-29: Code simplification improves clarity and maintainability.

The VendorManagement function has been effectively simplified, removing the conditional vendor check logic that was previously here. This aligns with the PR objective of refactoring to utilize caching and enhance the vendor management layout. The authentication check is clean and straightforward.


31-43: Properly updated metadata title.

The metadata title has been correctly updated from "sidebar.risk" to "sidebar.vendors", which accurately reflects the purpose of this page and ensures consistent labeling across the application.


22-28: Clean component rendering with improved structure.

The updated implementation now directly renders the VendorOverview component in a well-structured div with proper spacing classes, which provides a cleaner UI organization. This change works in conjunction with the layout changes mentioned in the PR summary, where onboarding prompts are conditionally displayed when no vendors are available.

Comment on lines +92 to +95
{members.map((member) => {
const isCurrentUser =
member.userId ===
members.find((m) => m.role === currentUserRole)?.userId;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Review the current user identification logic.
In the mapping callback, the logic

const isCurrentUser =
  member.userId === members.find((m) => m.role === currentUserRole)?.userId;

may not reliably identify the current user. Comparing the member’s userId with the userId of the first member found by role can lead to false positives if multiple members share the same role. Typically, a dedicated currentUserId should be used.
Consider modifying the component’s props and updating the check as shown below:

- const isCurrentUser =
-   member.userId === members.find((m) => m.role === currentUserRole)?.userId;
+ const isCurrentUser =
+   member.userId === currentUserId;

You would also update the MembersListProps interface accordingly:

-interface MembersListProps {
-  members: OrganizationMember[];
-  currentUserRole?: string;
-  hasOrganization: boolean;
-}
+interface MembersListProps {
+  members: OrganizationMember[];
+  currentUserRole?: string;
+  currentUserId?: string;
+  hasOrganization: boolean;
+}

- Added onboarding image for employee management.
- Removed outdated layout component for employee management.
- Introduced new layout for employee overview with onboarding prompts when no employees are present.
- Created `EmployeesOverview` component to display employee completion charts and relevant data.
- Implemented caching for employee data retrieval to enhance performance.
- Updated localization for onboarding content related to employee management.
@vercel
vercel Bot temporarily deployed to Preview – comp-portal March 31, 2025 21:54 Inactive

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (9)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/(overview)/loading.tsx (4)

1-44: Review file naming and purpose alignment

This loading component appears to be showing organization settings forms instead of vendor-specific content, despite being in the vendors directory path. Consider creating a more vendor-specific loading state or renaming the file to better reflect its purpose.

To better match typical loading patterns, consider implementing a skeleton UI that matches the actual vendor content:

- <div className="space-y-12 grid grid-cols-1 md:grid-cols-2 gap-4">
-   <Card>
-     <CardHeader>
-       <CardTitle>{t("settings.general.org_name")}</CardTitle>
-       <CardDescription>
-         {t("settings.general.org_name_description")}
-       </CardDescription>
-     </CardHeader>
-     <CardContent>
-       <Input type="text" placeholder="Loading..." className="max-w-[300px]" />
-     </CardContent>
-     <CardFooter className="flex justify-between">
-       <div>{t("settings.general.org_name_tip")}</div>
-       <Button disabled aria-label={t("common.actions.save")}>{t("common.actions.save")}</Button>
-     </CardFooter>
-   </Card>
+ <div className="space-y-12">
+   <div className="grid grid-cols-1 md:grid-cols-2 gap-4">
+     <Card>
+       <CardHeader>
+         <div className="h-7 w-48 bg-gray-200 animate-pulse rounded" />
+         <div className="h-5 w-64 bg-gray-100 animate-pulse rounded mt-2" />
+       </CardHeader>
+       <CardContent>
+         <div className="h-10 w-full max-w-[300px] bg-gray-200 animate-pulse rounded" />
+       </CardContent>
+       <CardFooter className="flex justify-between">
+         <div className="h-5 w-32 bg-gray-100 animate-pulse rounded" />
+         <div className="h-10 w-24 bg-gray-200 animate-pulse rounded" />
+       </CardFooter>
+     </Card>

10-42: Enhance accessibility with appropriate ARIA attributes

While the button includes an aria-label, the loading state itself could benefit from additional accessibility attributes.

Add appropriate ARIA attributes to improve accessibility:

- <div className="space-y-12 grid grid-cols-1 md:grid-cols-2 gap-4">
+ <div 
+   className="space-y-12 grid grid-cols-1 md:grid-cols-2 gap-4"
+   aria-live="polite"
+   aria-busy="true"
+ >
  <Card>

19-20: Consider adding a visual loading indicator

The input field has a "Loading..." placeholder, but adding a visual loading indicator would improve the user experience.

<CardContent>
-  <Input type="text" placeholder="Loading..." className="max-w-[300px]" />
+  <div className="relative max-w-[300px]">
+    <Input type="text" placeholder="Loading..." className="max-w-[300px]" />
+    <div className="absolute right-3 top-1/2 transform -translate-y-1/2">
+      <div className="animate-spin h-4 w-4 border-2 border-gray-300 border-t-primary rounded-full" />
+    </div>
+  </div>
</CardContent>

23-24: Improve button loading state visualization

The button is correctly disabled, but adding a loading spinner inside the button would provide better visual feedback.

- <Button disabled aria-label={t("common.actions.save")}>{t("common.actions.save")}</Button>
+ <Button disabled aria-label={t("common.actions.save")}>
+   <span className="flex items-center">
+     <span className="animate-spin mr-2 h-4 w-4 border-2 border-current border-t-transparent rounded-full"></span>
+     {t("common.actions.save")}
+   </span>
+ </Button>
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/employees/[employeeId]/layout.tsx (1)

1-30: Consider consolidating duplicate layout logic

This file is identical to apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/employees/all/layout.tsx. Consider extracting the common layout logic into a shared component to avoid duplication and improve maintainability.

You could create a shared layout component:

// apps/app/src/components/layouts/employees-layout.tsx
import { auth } from "@/auth";
import { getI18n } from "@/locales/server";
import { SecondaryMenu } from "@bubba/ui/secondary-menu";

export async function EmployeesLayout({
  children,
}: {
  children: React.ReactNode;
}) {
  const t = await getI18n();
  const session = await auth();
  const user = session?.user;
  const orgId = user?.organizationId;

  return (
    <div className="max-w-[1200px] m-auto">
      <SecondaryMenu
        items={[
          {
            path: `/${orgId}/employees`,
            label: t("people.dashboard.title"),
          },
          { path: `/${orgId}/employees/all`, label: t("people.all") },
        ]}
      />

      <main className="mt-8">{children}</main>
    </div>
  );
}

Then import and use it in both layout files to eliminate duplication.

apps/app/src/locales/onboarding/app-onboarding.ts (1)

28-28: Consider removing trailing whitespace in description

There appears to be unnecessary whitespace after the description text.

  description:
-			"Manage your vendors and ensure your organization is protected.",
+			"Manage your vendors and ensure your organization is protected.",
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/employees/(overview)/layout.tsx (3)

24-24: Consider replacing generic loading div with a dedicated Loading component

Instead of using a simple div with "Loading..." text, consider using a dedicated Loading component for better user experience and consistency across the application.

- <Suspense fallback={<div>Loading...</div>}>
+ <Suspense fallback={<LoadingSpinner />}>

59-59: Remove extra blank line

There's an unnecessary extra blank line that can be removed.

  }

-

  return (

81-85: Add try/catch block for database query

The database query should be wrapped in a try/catch block to handle potential database errors gracefully.

  const orgId = session?.user.organizationId;

+  try {
    const employees = await db.employee.findMany({
      where: {
        organizationId: orgId,
      },
    });
    
    return employees;
+  } catch (error) {
+    console.error("Failed to fetch employees:", error);
+    return []; // Return empty array on error
+  }
-  return employees;
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 69853e9 and eef5e4a.

📒 Files selected for processing (7)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/employees/(overview)/layout.tsx (1 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/employees/[employeeId]/layout.tsx (1 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/employees/all/layout.tsx (1 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/employees/layout.tsx (0 hunks)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/vendors/(overview)/loading.tsx (1 hunks)
  • apps/app/src/components/main-menu.tsx (1 hunks)
  • apps/app/src/locales/onboarding/app-onboarding.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/employees/layout.tsx
✅ Files skipped from review due to trivial changes (1)
  • apps/app/src/components/main-menu.tsx
🧰 Additional context used
🧬 Code Definitions (1)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/employees/[employeeId]/layout.tsx (2)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/employees/(overview)/layout.tsx (1)
  • Layout (9-75)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/employees/all/layout.tsx (1)
  • Layout (5-30)
🔇 Additional comments (4)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/employees/all/layout.tsx (1)

15-29: LGTM - Layout structure is clean and follows established patterns

The layout component correctly organizes the secondary navigation and main content area while maintaining consistent max-width constraints.

apps/app/src/locales/onboarding/app-onboarding.ts (2)

1-65: Localization structure is well organized

The localization structure for onboarding content is comprehensive, covering all three management sections with consistent structure for titles, descriptions, CTAs, and FAQs.


2-23: Consistent content formatting across all management sections

The structure for all three management sections (risk, vendor, employee) follows the same consistent pattern with similar FAQ questions adapted to each context. This provides a cohesive onboarding experience.

Also applies to: 24-44, 45-64

apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/employees/(overview)/layout.tsx (1)

19-57: Excellent implementation of conditional onboarding UI

The layout correctly implements the conditional rendering of an onboarding interface when no employees are found. The use of Suspense for loading states is a good practice.

Comment on lines +6 to +8
export default async function Loading() {
const t = await getI18n();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling for async operations

The async function doesn't include error handling for the getI18n() call, which could fail silently.

export default async function Loading() {
-  const t = await getI18n();
+  try {
+    const t = await getI18n();
+    
+    return (
+      // Component JSX...
+    );
+  } catch (error) {
+    console.error("Failed to load translations:", error);
+    // Fallback to a simpler loading state that doesn't require translations
+    return <div className="flex items-center justify-center p-8">Loading vendors...</div>;
+  }
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export default async function Loading() {
const t = await getI18n();
export default async function Loading() {
try {
const t = await getI18n();
return (
// Component JSX...
);
} catch (error) {
console.error("Failed to load translations:", error);
// Fallback to a simpler loading state that doesn't require translations
return <div className="flex items-center justify-center p-8">Loading vendors...</div>;
}
}

Comment on lines +10 to +13
const t = await getI18n();
const session = await auth();
const user = session?.user;
const orgId = user?.organizationId;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling for session data

The code currently doesn't handle cases where the session or user might be null, which could lead to runtime errors when accessing organizationId.

  const t = await getI18n();
  const session = await auth();
  const user = session?.user;
- const orgId = user?.organizationId;
+ const orgId = user?.organizationId ?? ""; // Provide a fallback or handle the case when organizationId is undefined
+ 
+ if (!orgId) {
+   // Handle missing organization ID (redirect or show error message)
+ }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const t = await getI18n();
const session = await auth();
const user = session?.user;
const orgId = user?.organizationId;
const t = await getI18n();
const session = await auth();
const user = session?.user;
const orgId = user?.organizationId ?? ""; // Provide a fallback or handle the case when organizationId is undefined
if (!orgId) {
// Handle missing organization ID (redirect or show error message)
}

Comment on lines +77 to +88
const getEmployeesOverview = cache(async () => {
const session = await auth();
const orgId = session?.user.organizationId;

const employees = await db.employee.findMany({
where: {
organizationId: orgId,
},
});

return employees;
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling in getEmployeesOverview function

The function doesn't handle cases where the session might be null or organizationId is undefined, which could lead to runtime errors.

const getEmployeesOverview = cache(async () => {
  const session = await auth();
-  const orgId = session?.user.organizationId;
+  const orgId = session?.user?.organizationId;
+  
+  if (!orgId) {
+    return []; // Return empty array if no organization ID is found
+  }

  const employees = await db.employee.findMany({
    where: {
      organizationId: orgId,
    },
  });

  return employees;
});
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const getEmployeesOverview = cache(async () => {
const session = await auth();
const orgId = session?.user.organizationId;
const employees = await db.employee.findMany({
where: {
organizationId: orgId,
},
});
return employees;
});
const getEmployeesOverview = cache(async () => {
const session = await auth();
const orgId = session?.user?.organizationId;
if (!orgId) {
return []; // Return empty array if no organization ID is found
}
const employees = await db.employee.findMany({
where: {
organizationId: orgId,
},
});
return employees;
});

This branch was successfully deployed

1 active and 1 inactive deployments
Preview – app — eef5e4a3 Deployed Mar 31, 2025 by vercel[bot]
Preview – comp-portal — eef5e4a3 Deployed Mar 31, 2025 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant