feat: WIP for employee portal, styling of dashboard#68
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThis update introduces multiple enhancements across the codebase. In the app actions, the employee creation flow now also creates a corresponding portal user, and a new logout action has been added. New UI components and layouts have been implemented for the employee portal, including redesigned pages for localization and unauthorized access. Additional middleware logic enforces redirection based on session organization data. Authentication is updated with an extra field in the user configuration, localization files are revised with new sections, database schema changes support an optional organization ID, and email components have updated content and styling. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CreateEmployeeAction
participant Prisma
Client->>CreateEmployeeAction: Request to create employee
CreateEmployeeAction->>Prisma: Create employee record
Prisma-->>CreateEmployeeAction: Employee data
CreateEmployeeAction->>Prisma: Create associated portalUser
Prisma-->>CreateEmployeeAction: PortalUser created
CreateEmployeeAction-->>Client: Success response
sequenceDiagram
participant User
participant LogoutComponent
participant AuthModule
User->>LogoutComponent: Initiate logout
LogoutComponent->>AuthModule: Call signOut with headers
AuthModule-->>LogoutComponent: Confirmation
LogoutComponent->>User: Redirect to login page
sequenceDiagram
participant User
participant Middleware
User->>Middleware: Request page
alt Session has organizationId and path is /unauthorized
Middleware-->>User: Redirect to "/"
else Session exists without organizationId and not on /unauthorized
Middleware-->>User: Redirect to "/unauthorized"
else
Middleware-->>User: Allow request to proceed
end
Possibly related PRs
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (15)
apps/portal/src/app/[locale]/(app)/layout.tsx (1)
4-19: LGTM! Consider adding ARIA landmarks.The layout structure and responsive styling look good. Consider adding ARIA landmarks to improve accessibility:
- <div className="relative"> + <div className="relative" role="region" aria-label="Main layout"> <Sidebar /> <div className="mx-4 md:ml-[95px] md:mr-10 pb-8"> <Header /> - <main>{children}</main> + <main role="main">{children}</main> </div> </div>apps/portal/src/app/[locale]/(app)/(home)/layout.tsx (1)
4-18: LGTM! Consider adding ARIA landmarks.The layout structure and i18n implementation look good. Consider adding ARIA landmarks to improve accessibility:
- <div className="max-w-[1200px]"> + <div className="max-w-[1200px]" role="region" aria-label="Home layout"> <SecondaryMenu items={[{ path: "/", label: t("sidebar.dashboard") }]} /> - <main className="mt-8">{children}</main> + <main role="main" className="mt-8">{children}</main> </div>packages/email/components/footer.tsx (1)
16-18: Consider adding semantic markup for the address.The address text could benefit from semantic HTML markup:
- <Text className="text-[#B8B8B8] text-xs"> - Bubba AI, Inc. 2261 Market Street, San Francisco, CA 94114 - </Text> + <Text className="text-[#B8B8B8] text-xs"> + <address>Bubba AI, Inc. 2261 Market Street, San Francisco, CA 94114</address> + </Text>apps/portal/src/app/[locale]/(app)/(home)/page.tsx (1)
20-32: Enhance metadata for better SEO.Add more metadata fields for improved SEO and social sharing.
Apply this diff:
const { locale } = await params; setStaticParamsLocale(locale); const t = await getI18n(); return { title: t("sidebar.dashboard"), + description: t("portal.description"), + openGraph: { + title: t("sidebar.dashboard"), + description: t("portal.description"), + type: "website", + }, };apps/portal/src/app/components/logout.tsx (1)
26-31: Improve accessibility and localization.Add ARIA attributes and localize the loading text.
Apply this diff:
return ( - <DropdownMenuItem onClick={handleLogout}> - {isLoading ? "Loading..." : t("user_menu.sign_out")} + <DropdownMenuItem + onClick={handleLogout} + disabled={isLoading} + aria-busy={isLoading} + > + {isLoading ? t("common.loading") : t("user_menu.sign_out")} </DropdownMenuItem> );apps/portal/src/app/components/mobile-menu.tsx (1)
24-30: Consider adding a close button in the sheet content.For better UX, consider adding a close button in the sheet content, especially for accessibility.
<SheetContent side="left" className="border-none rounded-none -ml-2"> + <Button + variant="ghost" + size="icon" + onClick={() => setOpen(false)} + className="absolute right-4 top-4" + aria-label="Close menu" + > + <Icons.X size={16} /> + </Button> <div className="ml-2 mb-8"> <Icons.Logo /> </div> <MainMenu onSelect={() => setOpen(false)} /> </SheetContent>apps/portal/src/app/locales/en.ts (1)
29-31: Consider adding more sidebar menu items.The sidebar section currently only has a dashboard entry. Since this is a WIP PR, consider planning for additional menu items that will be needed for the employee portal.
apps/portal/src/app/locales/fr.ts (1)
14-20: Consider moving the URL to a configuration file.The hardcoded URL
https://trycomp.aishould be moved to a configuration file to make it easier to update across all locales if needed.apps/portal/src/app/components/header.tsx (1)
17-17: Consider extracting CSS classes to a constant or using a CSS-in-JS solution.The long string of CSS classes reduces readability. Consider extracting these to a constant or using a CSS-in-JS solution for better maintainability.
apps/portal/src/app/components/user-menu.tsx (2)
20-28: Add error handling for image loading.Consider adding error handling for the avatar image to gracefully fallback to initials when the image fails to load.
{session?.user?.image && ( <AvatarImageNext src={session?.user?.image} alt={session?.user?.name ?? ""} width={32} height={32} quality={100} + onError={(e) => { + e.currentTarget.style.display = 'none'; + }} /> )}
30-33: Consider handling empty name edge case.The current implementation might show an empty fallback when the user's name is empty.
<span className="text-xs"> - {session?.user?.name?.charAt(0)?.toUpperCase()} + {session?.user?.name?.charAt(0)?.toUpperCase() ?? '?'} </span>apps/portal/src/middleware.ts (2)
39-43: Consider using URL utility functions.For consistency and maintainability, consider using Next.js's URL utilities.
-const url = new URL("/", request.url); +const url = request.nextUrl.clone(); +url.pathname = "/";
45-49: Add logging for unauthorized redirects.Consider adding logging to track unauthorized access attempts for security monitoring.
if (session && !session.user.organizationId && newUrl.pathname !== "/unauthorized") { + console.warn(`Unauthorized access attempt for user ${session.user.id}`); const url = new URL("/unauthorized", request.url); return NextResponse.redirect(url); }packages/email/emails/otp.tsx (1)
50-50: Enhance email preview text.Consider making the preview text more informative and engaging.
-<Preview>One-Time Password for Comp AI</Preview> +<Preview>Your secure login code for Comp AI - Valid for 10 minutes</Preview>apps/portal/src/app/components/main-menu.tsx (1)
124-132: Consider persisting menu item order.The reordered items are only stored in component state and will reset on page refresh.
Consider persisting the order in localStorage or backend:
const onReorder = (items: { path: string; name: string; disabled: boolean }[]) => { setItems(items); + // Persist order + localStorage.setItem('menu-order', JSON.stringify(items)); }; +// In useState initialization +const savedItems = typeof window !== 'undefined' + ? JSON.parse(localStorage.getItem('menu-order') || 'null') + : null; +const [items, setItems] = useState(savedItems ?? initialItems ?? defaultItems);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
apps/app/src/actions/people/create-employee-action.ts(2 hunks)apps/app/src/components/magic-link.tsx(1 hunks)apps/portal/src/app/[locale]/(app)/(home)/layout.tsx(1 hunks)apps/portal/src/app/[locale]/(app)/(home)/page.tsx(1 hunks)apps/portal/src/app/[locale]/(app)/layout.tsx(1 hunks)apps/portal/src/app/[locale]/(app)/page.tsx(0 hunks)apps/portal/src/app/[locale]/(app)/unauthorized/page.tsx(1 hunks)apps/portal/src/app/[locale]/(public)/auth/page.tsx(2 hunks)apps/portal/src/app/actions/logout.ts(1 hunks)apps/portal/src/app/components/header.tsx(1 hunks)apps/portal/src/app/components/logout.tsx(1 hunks)apps/portal/src/app/components/main-menu.tsx(1 hunks)apps/portal/src/app/components/mobile-menu.tsx(1 hunks)apps/portal/src/app/components/otp.tsx(2 hunks)apps/portal/src/app/components/sidebar.tsx(1 hunks)apps/portal/src/app/components/user-menu.tsx(1 hunks)apps/portal/src/app/lib/auth-client.ts(1 hunks)apps/portal/src/app/lib/auth.ts(1 hunks)apps/portal/src/app/locales/en.ts(1 hunks)apps/portal/src/app/locales/es.ts(1 hunks)apps/portal/src/app/locales/fr.ts(1 hunks)apps/portal/src/app/locales/no.ts(1 hunks)apps/portal/src/app/locales/pt.ts(1 hunks)apps/portal/src/middleware.ts(1 hunks)packages/db/prisma/schema.prisma(2 hunks)packages/email/components/footer.tsx(1 hunks)packages/email/emails/otp.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- apps/portal/src/app/[locale]/(app)/page.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/portal/src/app/lib/auth-client.ts
🔇 Additional comments (15)
apps/portal/src/app/actions/logout.ts (1)
1-11: LGTM! Clean implementation of the logout action.The implementation follows Next.js best practices for server actions and maintains a single responsibility.
packages/email/components/footer.tsx (1)
8-14: LGTM! Clean implementation of the footer text.The font sizes and link implementation look good.
apps/portal/src/app/locales/en.ts (1)
14-20:Details
❓ Verification inconclusive
Review marketing copy and external link.
The marketing copy and external link in the
powered_bysection need attention:
- Verify the accuracy of compliance claims
- Consider security implications of the external link
🏁 Script executed:
#!/bin/bash # Check if the external link is valid and secure curl -I https://trycomp.aiLength of output: 1510
Action: Revisit and verify the marketing copy and external link in the
powered_bysection.
- The external link
https://trycomp.aihas been verified via a curl check and is returning an HTTP/2 200 response with proper security headers (including HSTS), indicating it is secure and active.- The marketing copy ("Get SOC 2, ISO 27001, and GDPR compliant in weeks, not months...") should be confirmed for accuracy with the compliance/marketing teams to ensure the claims are correct and align with our product positioning.
apps/portal/src/app/locales/no.ts (1)
1-32: LGTM! Norwegian translations are well-structured.The Norwegian translations maintain consistency with the English version while providing natural-sounding Norwegian text.
apps/portal/src/app/locales/pt.ts (1)
1-32: LGTM! Portuguese translations are well-structured.The Portuguese translations maintain consistency with other locale files while providing natural-sounding Portuguese text.
apps/portal/src/app/locales/fr.ts (3)
3-4: LGTM! Text changes improve clarity and user experience.The updated French translations are more concise and user-friendly.
Also applies to: 8-8, 10-11
21-23: LGTM! Sign out translation is appropriate.The French translation for the sign out option is clear and consistent.
29-31: LGTM! Dashboard translation is appropriate.The French translation for the dashboard label is clear and consistent.
apps/portal/src/app/locales/es.ts (1)
1-32: LGTM! Spanish translations are consistent with other locales.The Spanish translations maintain consistency with other language files while providing natural and clear translations.
apps/portal/src/app/lib/auth.ts (1)
15-20:Details
✅ Verification successful
Verify handling of users without organizationId.
The
organizationIdfield is marked as optional. Ensure that the application properly handles cases where this field is not set.Run the following script to check for potential issues:
🏁 Script executed:
#!/bin/bash # Description: Check for proper handling of optional organizationId # Search for direct references to organizationId echo "Checking direct references to organizationId..." rg "organizationId" -A 3 # Search for null checks on organizationId echo "Checking null/undefined handling..." rg "(\?\.|\|\||\?\?|===\s*(null|undefined)).*organizationId"Length of output: 69037
Organization ID Handling Verified
After running searches across the codebase, it appears that the application consistently checks for and handles missing organizationId values. For example, middleware and various routes and actions appropriately check for the absence of organizationId (using conditions such as
if (!organizationId)) and take corrective actions (e.g., redirection or error throwing). This verification confirms that marking the field as optional does not lead to unhandled scenarios when the field isn’t set.No changes are required regarding organizationId handling.
apps/portal/src/app/components/header.tsx (1)
24-28: LGTM! Good use of Suspense for loading state.The implementation of Suspense with a Skeleton fallback provides a good user experience during loading.
packages/email/emails/otp.tsx (1)
62-68: LGTM! Clear and secure communication.The email content effectively communicates the OTP while emphasizing security best practices.
apps/portal/src/app/components/otp.tsx (1)
9-9: LGTM! UX improvements for the OTP form.The changes enhance the user experience by:
- Preventing multiple submissions during loading state
- Using a more appropriate icon (ArrowRight) for the action
Also applies to: 91-91, 98-98
apps/app/src/components/magic-link.tsx (1)
113-113:Details
❓ Verification inconclusive
Verify button visibility after background color removal.
The
bg-primaryclass has been removed from the button. Please ensure the button remains visible against all possible parent background colors.
🏁 Script executed:
#!/bin/bash # Search for potential parent background color classes rg -A 2 -B 2 'className=.*bg-' apps/app/src/components/Length of output: 44894
Verify Button Contrast on Various Backgrounds
Please ensure that the magic link button (in apps/app/src/components/magic-link.tsx, line 113) remains clearly visible after the removal of the
bg-primaryclass. Because the button no longer has an explicit background color, test its appearance against all anticipated parent backgrounds (e.g.,bg-card,bg-background, etc.) to verify sufficient contrast and overall visibility. Adjust additional styling (such as borders or shadows) if needed to maintain a strong visual cue for users.packages/db/prisma/schema.prisma (1)
1020-1021: New Optional Organization Relationship in PortalUserThe addition of the optional
organizationIdfield along with the corresponding relation to theOrganizationmodel is implemented correctly. This change allows a portal user to be linked with an organization, which aligns with the new employee portal requirements.Key Points:
- The field type
String?properly indicates that the relation is optional.- The relation is set up with
onDelete: Cascade, meaning that if an organization is deleted, any associated portal users will also be deleted. Ensure that this behavior aligns with your business requirements—if you want to preserve portal user records even after an organization is removed, consider usingonDelete: SetNullinstead.
| export default async function Unauthorized() { | ||
| const session = await auth.api.getSession({ | ||
| headers: await headers(), | ||
| }); | ||
|
|
||
| return ( | ||
| <div className="grid grid-rows-[20px_1fr_20px] items-center justify-items-center min-h-screen p-8 pb-20 gap-16 sm:p-20 font-[family-name:var(--font-geist-sans)]"> | ||
| <main className="flex flex-col gap-8 row-start-2 items-center sm:items-start"> | ||
| We couldn't find an organization for you. Please contact your | ||
| administrator. | ||
| </main> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add localization support and error handling.
The component needs the following improvements:
- The error message should be localized since it's in a locale route.
- Add error handling for the session fetch.
- Add proper heading and ARIA attributes for better accessibility.
Apply this diff to improve the component:
+import { getI18n } from "@/app/locales/server";
export default async function Unauthorized() {
+ const t = await getI18n();
const session = await auth.api.getSession({
headers: await headers(),
- });
+ }).catch((error) => {
+ console.error("Failed to fetch session:", error);
+ return null;
+ });
return (
<div className="grid grid-rows-[20px_1fr_20px] items-center justify-items-center min-h-screen p-8 pb-20 gap-16 sm:p-20 font-[family-name:var(--font-geist-sans)]">
- <main className="flex flex-col gap-8 row-start-2 items-center sm:items-start">
+ <main className="flex flex-col gap-8 row-start-2 items-center sm:items-start" role="main" aria-labelledby="unauthorized-title">
+ <h1 id="unauthorized-title" className="sr-only">{t("unauthorized.title")}</h1>
{t("unauthorized.message")}
</main>
</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.
| export default async function Unauthorized() { | |
| const session = await auth.api.getSession({ | |
| headers: await headers(), | |
| }); | |
| return ( | |
| <div className="grid grid-rows-[20px_1fr_20px] items-center justify-items-center min-h-screen p-8 pb-20 gap-16 sm:p-20 font-[family-name:var(--font-geist-sans)]"> | |
| <main className="flex flex-col gap-8 row-start-2 items-center sm:items-start"> | |
| We couldn't find an organization for you. Please contact your | |
| administrator. | |
| </main> | |
| </div> | |
| ); | |
| } | |
| import { getI18n } from "@/app/locales/server"; | |
| export default async function Unauthorized() { | |
| const t = await getI18n(); | |
| const session = await auth.api.getSession({ | |
| headers: await headers(), | |
| }).catch((error) => { | |
| console.error("Failed to fetch session:", error); | |
| return null; | |
| }); | |
| return ( | |
| <div className="grid grid-rows-[20px_1fr_20px] items-center justify-items-center min-h-screen p-8 pb-20 gap-16 sm:p-20 font-[family-name:var(--font-geist-sans)]"> | |
| <main className="flex flex-col gap-8 row-start-2 items-center sm:items-start" role="main" aria-labelledby="unauthorized-title"> | |
| <h1 id="unauthorized-title" className="sr-only">{t("unauthorized.title")}</h1> | |
| {t("unauthorized.message")} | |
| </main> | |
| </div> | |
| ); | |
| } |
| import { Icons } from "@bubba/ui/icons"; | ||
| import { cookies } from "next/headers"; | ||
| import Link from "next/link"; | ||
| import { MainMenu } from "./main-menu"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused import.
The cookies import is not used in this component.
import { Icons } from "@bubba/ui/icons";
-import { cookies } from "next/headers";
import Link from "next/link";
import { MainMenu } from "./main-menu";📝 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.
| import { Icons } from "@bubba/ui/icons"; | |
| import { cookies } from "next/headers"; | |
| import Link from "next/link"; | |
| import { MainMenu } from "./main-menu"; | |
| import { Icons } from "@bubba/ui/icons"; | |
| import Link from "next/link"; | |
| import { MainMenu } from "./main-menu"; |
| export async function Sidebar() { | ||
| return ( | ||
| <aside className="h-screen flex-shrink-0 flex-col justify-between fixed top-0 ml-4 pb-4 items-center hidden md:flex"> | ||
| <div className="flex flex-col items-center justify-center"> | ||
| <div className="mt-2 todesktop:mt-[35px]"> | ||
| <div className="mt-2 items-center justify-center"> | ||
| <Link href="/"> | ||
| <Icons.Logo /> | ||
| </Link> | ||
| </div> | ||
| </div> | ||
| <MainMenu /> | ||
| </div> | ||
| </aside> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve accessibility and component structure.
- Add alt text to the logo for screen readers.
- Consider z-index for fixed positioning to prevent overlay issues.
- Remove async since no async operations are performed.
Apply this diff:
-export async function Sidebar() {
+export function Sidebar() {
return (
- <aside className="h-screen flex-shrink-0 flex-col justify-between fixed top-0 ml-4 pb-4 items-center hidden md:flex">
+ <aside className="h-screen flex-shrink-0 flex-col justify-between fixed top-0 ml-4 pb-4 items-center hidden md:flex z-10">
<div className="flex flex-col items-center justify-center">
<div className="mt-2 todesktop:mt-[35px]">
<div className="mt-2 items-center justify-center">
- <Link href="/">
+ <Link href="/" aria-label="Home">
<Icons.Logo />
</Link>
</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.
| export async function Sidebar() { | |
| return ( | |
| <aside className="h-screen flex-shrink-0 flex-col justify-between fixed top-0 ml-4 pb-4 items-center hidden md:flex"> | |
| <div className="flex flex-col items-center justify-center"> | |
| <div className="mt-2 todesktop:mt-[35px]"> | |
| <div className="mt-2 items-center justify-center"> | |
| <Link href="/"> | |
| <Icons.Logo /> | |
| </Link> | |
| </div> | |
| </div> | |
| <MainMenu /> | |
| </div> | |
| </aside> | |
| ); | |
| } | |
| export function Sidebar() { | |
| return ( | |
| <aside className="h-screen flex-shrink-0 flex-col justify-between fixed top-0 ml-4 pb-4 items-center hidden md:flex z-10"> | |
| <div className="flex flex-col items-center justify-center"> | |
| <div className="mt-2 todesktop:mt-[35px]"> | |
| <div className="mt-2 items-center justify-center"> | |
| <Link href="/" aria-label="Home"> | |
| <Icons.Logo /> | |
| </Link> | |
| </div> | |
| </div> | |
| <MainMenu /> | |
| </div> | |
| </aside> | |
| ); | |
| } |
| export default async function Portal({ | ||
| params, | ||
| }: { | ||
| params: Promise<{ locale: string }>; | ||
| }) { | ||
| const { locale } = await params; | ||
| setStaticParamsLocale(locale); | ||
|
|
||
| return ( | ||
| <div> | ||
| <h1>Employee Portal</h1> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add localization and improve semantic structure.
The portal page needs localization for the heading and better semantic HTML structure.
Apply this diff:
export default async function Portal({
params,
}: {
params: Promise<{ locale: string }>;
}) {
const { locale } = await params;
setStaticParamsLocale(locale);
+ const t = await getI18n();
return (
- <div>
- <h1>Employee Portal</h1>
+ <div className="container mx-auto p-4">
+ <h1 className="text-2xl font-bold mb-4">{t("portal.title")}</h1>
+ <main>
+ {/* Add your portal content here */}
+ </main>
</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.
| export default async function Portal({ | |
| params, | |
| }: { | |
| params: Promise<{ locale: string }>; | |
| }) { | |
| const { locale } = await params; | |
| setStaticParamsLocale(locale); | |
| return ( | |
| <div> | |
| <h1>Employee Portal</h1> | |
| </div> | |
| ); | |
| } | |
| export default async function Portal({ | |
| params, | |
| }: { | |
| params: Promise<{ locale: string }>; | |
| }) { | |
| const { locale } = await params; | |
| setStaticParamsLocale(locale); | |
| const t = await getI18n(); | |
| return ( | |
| <div className="container mx-auto p-4"> | |
| <h1 className="text-2xl font-bold mb-4">{t("portal.title")}</h1> | |
| <main> | |
| {/* Add your portal content here */} | |
| </main> | |
| </div> | |
| ); | |
| } |
| const handleLogout = async () => { | ||
| setLoading(true); | ||
| await authClient.signOut({ | ||
| fetchOptions: { | ||
| onSuccess: () => { | ||
| router.push("/login"); // redirect to login page | ||
| }, | ||
| }, | ||
| }); | ||
| setLoading(false); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling to logout process.
The logout handler needs error handling and proper loading state management.
Apply this diff:
const handleLogout = async () => {
setLoading(true);
- await authClient.signOut({
- fetchOptions: {
- onSuccess: () => {
- router.push("/login"); // redirect to login page
- },
- },
- });
- setLoading(false);
+ try {
+ await authClient.signOut({
+ fetchOptions: {
+ onSuccess: () => {
+ router.push("/login");
+ },
+ },
+ });
+ } catch (error) {
+ console.error("Logout failed:", error);
+ // TODO: Add error notification
+ } finally {
+ setLoading(false);
+ }
};📝 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.
| const handleLogout = async () => { | |
| setLoading(true); | |
| await authClient.signOut({ | |
| fetchOptions: { | |
| onSuccess: () => { | |
| router.push("/login"); // redirect to login page | |
| }, | |
| }, | |
| }); | |
| setLoading(false); | |
| }; | |
| const handleLogout = async () => { | |
| setLoading(true); | |
| try { | |
| await authClient.signOut({ | |
| fetchOptions: { | |
| onSuccess: () => { | |
| router.push("/login"); | |
| }, | |
| }, | |
| }); | |
| } catch (error) { | |
| console.error("Logout failed:", error); | |
| // TODO: Add error notification | |
| } finally { | |
| setLoading(false); | |
| } | |
| }; |
| <div className="flex space-x-2 ml-auto"> | ||
| <div className="flex gap-2">Employee Portal</div> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider making the portal title configurable.
The hardcoded "Employee Portal" text should be moved to the localization files for consistency with other translated content.
| <Link | ||
| href="https://trycomp.ai" | ||
| target="_blank" | ||
| className="hover:underline hover:underline-offset-2" | ||
| > | ||
| <span className="mt-2 inline-flex items-center text-xs font-medium text-primary gap-2"> | ||
| {t("powered_by.learn_more")} | ||
| <ArrowRight className="h-3 w-3" /> | ||
| </span> | ||
| </Link> | ||
| </Button> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add security attributes for external link.
The external link should include security-related attributes to protect against potential vulnerabilities.
<Link
href="https://trycomp.ai"
target="_blank"
+ rel="noopener noreferrer"
className="hover:underline hover:underline-offset-2"
>📝 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.
| <Link | |
| href="https://trycomp.ai" | |
| target="_blank" | |
| className="hover:underline hover:underline-offset-2" | |
| > | |
| <span className="mt-2 inline-flex items-center text-xs font-medium text-primary gap-2"> | |
| {t("powered_by.learn_more")} | |
| <ArrowRight className="h-3 w-3" /> | |
| </span> | |
| </Link> | |
| </Button> | |
| <Link | |
| href="https://trycomp.ai" | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| className="hover:underline hover:underline-offset-2" | |
| > | |
| <span className="mt-2 inline-flex items-center text-xs font-medium text-primary gap-2"> | |
| {t("powered_by.learn_more")} | |
| <ArrowRight className="h-3 w-3" /> | |
| </span> | |
| </Link> | |
| </Button> |
| const portalUser = await db.portalUser.create({ | ||
| data: { | ||
| id: employee.id, | ||
| name, | ||
| email, | ||
| organizationId: user.organizationId, | ||
| emailVerified: false, | ||
| createdAt: new Date(), | ||
| updatedAt: new Date(), | ||
| employees: { | ||
| connect: { | ||
| id: employee.id, | ||
| }, | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Wrap employee and portal user creation in a transaction.
The current implementation could lead to data inconsistency if the portal user creation fails after the employee is created. This would result in an employee without a corresponding portal user.
Wrap both operations in a transaction:
- const employee = await db.employee.create({
- data: {
- name,
- email,
- department,
- organizationId: user.organizationId,
- isActive: true,
- externalEmployeeId,
- },
- });
-
- const portalUser = await db.portalUser.create({
+ const [employee, portalUser] = await db.$transaction(async (tx) => {
+ const employee = await tx.employee.create({
+ data: {
+ name,
+ email,
+ department,
+ organizationId: user.organizationId,
+ isActive: true,
+ externalEmployeeId,
+ },
+ });
+
+ const portalUser = await tx.portalUser.create({
+ data: {
+ id: employee.id,
+ name,
+ email,
+ organizationId: user.organizationId,
+ emailVerified: false,
+ createdAt: new Date(),
+ updatedAt: new Date(),
+ employees: {
+ connect: {
+ id: employee.id,
+ },
+ },
+ },
+ });
+
+ return [employee, portalUser];
+ });📝 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.
| const portalUser = await db.portalUser.create({ | |
| data: { | |
| id: employee.id, | |
| name, | |
| email, | |
| organizationId: user.organizationId, | |
| emailVerified: false, | |
| createdAt: new Date(), | |
| updatedAt: new Date(), | |
| employees: { | |
| connect: { | |
| id: employee.id, | |
| }, | |
| }, | |
| }, | |
| }); | |
| const [employee, portalUser] = await db.$transaction(async (tx) => { | |
| const employee = await tx.employee.create({ | |
| data: { | |
| name, | |
| email, | |
| department, | |
| organizationId: user.organizationId, | |
| isActive: true, | |
| externalEmployeeId, | |
| }, | |
| }); | |
| const portalUser = await tx.portalUser.create({ | |
| data: { | |
| id: employee.id, | |
| name, | |
| email, | |
| organizationId: user.organizationId, | |
| emailVerified: false, | |
| createdAt: new Date(), | |
| updatedAt: new Date(), | |
| employees: { | |
| connect: { | |
| id: employee.id, | |
| }, | |
| }, | |
| }, | |
| }); | |
| return [employee, portalUser]; | |
| }); |
| const icons = { | ||
| "/": () => <Icons.Overview size={22} />, | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Expand icons mapping for all routes.
The icons object only includes mapping for the root route ("/"). Add mappings for all possible routes to prevent undefined icons.
🏁 Script executed:
#!/bin/bash
# Search for all defined routes in the codebase
rg -A 1 'path:\s*"/.+?"' apps/portal/src/app/Length of output: 233
Action Required: Expand Icons Mapping for All Routes
The current implementation in apps/portal/src/app/components/main-menu.tsx only defines the icon for the root route ("/"), using <Icons.Overview size={22} />. Our search confirmed that this is the only defined mapping for route usage (e.g., in [locale]/(app)/(home)/layout.tsx the route for "/" is used), which suggests that additional routes still lack an icon assignment.
- Update the
iconsobject: Include explicit icon mappings for all routes that are rendered in the application. - Consider a default case: If a route does not have a unique icon, a default icon mapping might be added to avoid undefined icon errors.
| const Item = ({ item, isActive, onSelect, disabled }: ItemProps) => { | ||
| const Icon = icons[item.path as keyof typeof icons]; | ||
| const linkDisabled = disabled || item.disabled; | ||
|
|
||
| return ( | ||
| <TooltipProvider delayDuration={70}> | ||
| {linkDisabled ? ( | ||
| <div className="w-[45px] h-[45px] flex items-center md:justify-center"> | ||
| Coming | ||
| </div> | ||
| ) : ( | ||
| <Link prefetch href={item.path} onClick={() => onSelect?.()}> | ||
| <Tooltip> | ||
| <TooltipTrigger className="w-full"> | ||
| <Reorder.Item | ||
| key={item.path} | ||
| value={item} | ||
| id={item.path} | ||
| layoutRoot | ||
| className={cn( | ||
| "relative border border-transparent md:w-[45px] h-[45px] flex items-center md:justify-center", | ||
| "hover:bg-accent hover:border-[#DCDAD2] hover:dark:border-[#2C2C2C]", | ||
| isActive && | ||
| "bg-[#F2F1EF] dark:bg-secondary border-[#DCDAD2] dark:border-[#2C2C2C]", | ||
| )} | ||
| > | ||
| <motion.div | ||
| className="relative" | ||
| initial={{ opacity: 1 }} | ||
| animate={{ opacity: 1 }} | ||
| exit={{ opacity: 0 }} | ||
| > | ||
| <div> | ||
| <Icon /> | ||
| <span className="flex md:hidden">{item.name}</span> | ||
| </div> | ||
| </motion.div> | ||
| </Reorder.Item> | ||
| </TooltipTrigger> | ||
| <TooltipContent | ||
| side="left" | ||
| className="px-3 py-1.5 text-xs hidden md:flex" | ||
| sideOffset={10} | ||
| > | ||
| {item.name} | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| </Link> | ||
| )} | ||
| </TooltipProvider> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance accessibility of the Item component.
Add ARIA labels and roles to improve accessibility:
<Link prefetch href={item.path} onClick={() => onSelect?.()}>
- <Tooltip>
+ <Tooltip role="navigation">
<TooltipTrigger className="w-full">
<Reorder.Item
key={item.path}
value={item}
id={item.path}
layoutRoot
+ role="menuitem"
+ aria-label={item.name}
className={cn(📝 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.
| const Item = ({ item, isActive, onSelect, disabled }: ItemProps) => { | |
| const Icon = icons[item.path as keyof typeof icons]; | |
| const linkDisabled = disabled || item.disabled; | |
| return ( | |
| <TooltipProvider delayDuration={70}> | |
| {linkDisabled ? ( | |
| <div className="w-[45px] h-[45px] flex items-center md:justify-center"> | |
| Coming | |
| </div> | |
| ) : ( | |
| <Link prefetch href={item.path} onClick={() => onSelect?.()}> | |
| <Tooltip> | |
| <TooltipTrigger className="w-full"> | |
| <Reorder.Item | |
| key={item.path} | |
| value={item} | |
| id={item.path} | |
| layoutRoot | |
| className={cn( | |
| "relative border border-transparent md:w-[45px] h-[45px] flex items-center md:justify-center", | |
| "hover:bg-accent hover:border-[#DCDAD2] hover:dark:border-[#2C2C2C]", | |
| isActive && | |
| "bg-[#F2F1EF] dark:bg-secondary border-[#DCDAD2] dark:border-[#2C2C2C]", | |
| )} | |
| > | |
| <motion.div | |
| className="relative" | |
| initial={{ opacity: 1 }} | |
| animate={{ opacity: 1 }} | |
| exit={{ opacity: 0 }} | |
| > | |
| <div> | |
| <Icon /> | |
| <span className="flex md:hidden">{item.name}</span> | |
| </div> | |
| </motion.div> | |
| </Reorder.Item> | |
| </TooltipTrigger> | |
| <TooltipContent | |
| side="left" | |
| className="px-3 py-1.5 text-xs hidden md:flex" | |
| sideOffset={10} | |
| > | |
| {item.name} | |
| </TooltipContent> | |
| </Tooltip> | |
| </Link> | |
| )} | |
| </TooltipProvider> | |
| ); | |
| }; | |
| const Item = ({ item, isActive, onSelect, disabled }: ItemProps) => { | |
| const Icon = icons[item.path as keyof typeof icons]; | |
| const linkDisabled = disabled || item.disabled; | |
| return ( | |
| <TooltipProvider delayDuration={70}> | |
| {linkDisabled ? ( | |
| <div className="w-[45px] h-[45px] flex items-center md:justify-center"> | |
| Coming | |
| </div> | |
| ) : ( | |
| <Link prefetch href={item.path} onClick={() => onSelect?.()}> | |
| <Tooltip role="navigation"> | |
| <TooltipTrigger className="w-full"> | |
| <Reorder.Item | |
| key={item.path} | |
| value={item} | |
| id={item.path} | |
| layoutRoot | |
| role="menuitem" | |
| aria-label={item.name} | |
| className={cn( | |
| "relative border border-transparent md:w-[45px] h-[45px] flex items-center md:justify-center", | |
| "hover:bg-accent hover:border-[#DCDAD2] hover:dark:border-[#2C2C2C]", | |
| isActive && | |
| "bg-[#F2F1EF] dark:bg-secondary border-[#DCDAD2] dark:border-[#2C2C2C]" | |
| )} | |
| > | |
| <motion.div | |
| className="relative" | |
| initial={{ opacity: 1 }} | |
| animate={{ opacity: 1 }} | |
| exit={{ opacity: 0 }} | |
| > | |
| <div> | |
| <Icon /> | |
| <span className="flex md:hidden">{item.name}</span> | |
| </div> | |
| </motion.div> | |
| </Reorder.Item> | |
| </TooltipTrigger> | |
| <TooltipContent | |
| side="left" | |
| className="px-3 py-1.5 text-xs hidden md:flex" | |
| sideOffset={10} | |
| > | |
| {item.name} | |
| </TooltipContent> | |
| </Tooltip> | |
| </Link> | |
| )} | |
| </TooltipProvider> | |
| ); | |
| }; |
1 participant
Summary by CodeRabbit
New Features
Style