Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { PreviewBanner } from "@/components/layout/Navbar/components/PreviewBann
import { useBreakpoint } from "@/lib/hooks/useBreakpoint";
import { useSupabase } from "@/lib/supabase/hooks/useSupabase";
import { Flag, useGetFlag } from "@/services/feature-flags/use-get-flag";
import { useMemo } from "react";
import { okData } from "@/app/api/helpers";
import { getAccountMenuItems, loggedInLinks, loggedOutLinks } from "../helpers";
import { AccountMenu } from "./AccountMenu/AccountMenu";
Expand Down Expand Up @@ -41,11 +40,6 @@ export function NavbarView({ isLoggedIn, previewBranchName }: NavbarViewProps) {
const { isUserLoading } = useSupabase();
const isLoadingProfile = isProfileLoading || isUserLoading;

const linksWithChat = useMemo(() => {
const chatLink = { name: "Chat", href: "/chat" };
return isChatEnabled ? [...loggedInLinks, chatLink] : loggedInLinks;
}, [isChatEnabled]);

const shouldShowPreviewBanner = Boolean(isLoggedIn && previewBranchName);

return (
Expand All @@ -59,13 +53,19 @@ export function NavbarView({ isLoggedIn, previewBranchName }: NavbarViewProps) {
{!isSmallScreen ? (
<div className="flex flex-1 items-center gap-5">
{isLoggedIn
? linksWithChat.map((link) => (
<NavbarLink
key={link.name}
name={link.name}
href={link.href}
/>
))
? loggedInLinks.map((link) => {
if (link.name === "Chat" && !isChatEnabled) {
return null;
}

return (
<NavbarLink
key={link.name}
name={link.name}
href={link.href}
/>
);
})
Copy link

Choose a reason for hiding this comment

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

Chat link never displays because it's not in source array

The loggedInLinks array in helpers.tsx does not contain a "Chat" entryβ€”it only includes Marketplace, Library, and Build. The old code dynamically added the Chat link to the array when isChatEnabled was true. The new code instead attempts to filter out a Chat link that doesn't exist in the source array. As a result, the Chat link will never be displayed regardless of whether isChatEnabled is true or false.

Additional Locations (1)

Fix in CursorΒ Fix in Web

: loggedOutLinks.map((link) => (
<NavbarLink
key={link.name}
Comment on lines +61 to 71

This comment was marked as outdated.

Expand Down Expand Up @@ -114,22 +114,34 @@ export function NavbarView({ isLoggedIn, previewBranchName }: NavbarViewProps) {
menuItemGroups={[
{
groupName: "Navigation",
items: linksWithChat.map((link) => ({
icon:
link.name === "Marketplace"
? IconType.Marketplace
: link.name === "Library"
? IconType.Library
: link.name === "Build"
? IconType.Builder
: link.name === "Chat"
? IconType.Chat
: link.name === "Monitor"
? IconType.Library
: IconType.LayoutDashboard,
text: link.name,
href: link.href,
})),
items: loggedInLinks
.map((link) => {
if (link.name === "Chat" && !isChatEnabled) {
return null;
}

return {
icon:
link.name === "Marketplace"
? IconType.Marketplace
: link.name === "Library"
? IconType.Library
: link.name === "Build"
? IconType.Builder
: link.name === "Chat"
? IconType.Chat
: link.name === "Monitor"
? IconType.Library
: IconType.LayoutDashboard,
text: link.name,
href: link.href,
};
})
.filter((item) => item !== null) as Array<{
icon: IconType;
text: string;
href: string;
}>,
},
...dynamicMenuItems,
]}
Comment on lines +137 to 147
Copy link

Choose a reason for hiding this comment

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

Bug: The refactoring removes the dynamic creation of the "Chat" link. The new filtering logic on loggedInLinks will never add the Chat link because it's not in the base array.
Severity: CRITICAL | Confidence: High

πŸ” Detailed Analysis

The refactoring in NavbarView.tsx replaces the dynamic creation of the "Chat" link with a filtering mechanism. The code now iterates over the loggedInLinks array and checks for a link with name === "Chat". However, the loggedInLinks array is a static constant that does not include a "Chat" link. As a result, the condition link.name === "Chat" will always be false, and the Chat link will never be rendered in either the desktop or mobile navigation bars, even when the isChatEnabled flag is true. This makes the entire Chat feature inaccessible from the UI.

πŸ’‘ Suggested Fix

Reintroduce the logic to dynamically add the "Chat" link to the navigation links when isChatEnabled is true. This could be done by conditionally adding the Chat link object to the loggedInLinks array before it is mapped over, similar to how the previous useMemo implementation worked.

πŸ€– Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location:
autogpt_platform/frontend/src/components/layout/Navbar/components/NavbarView.tsx#L114-L147

Potential issue: The refactoring in `NavbarView.tsx` replaces the dynamic creation of
the "Chat" link with a filtering mechanism. The code now iterates over the
`loggedInLinks` array and checks for a link with `name === "Chat"`. However, the
`loggedInLinks` array is a static constant that does not include a "Chat" link. As a
result, the condition `link.name === "Chat"` will always be false, and the Chat link
will never be rendered in either the desktop or mobile navigation bars, even when the
`isChatEnabled` flag is true. This makes the entire Chat feature inaccessible from the
UI.

Did we get this right? πŸ‘ / πŸ‘Ž to inform future reviews.
Reference ID: 7947776

Expand Down
Loading