Skip to content

TeamTable changes #37

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
visz11 wants to merge 1 commit into
base: main
Choose a base branch
from
+335 −0
Open

TeamTable changes #37

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
335 changes: 335 additions & 0 deletions TeamTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,335 @@
import {
Box,
Button,
Flex,
IconButton,
ListIcon,
ListItem,
Text,
Tooltip,
UnorderedList,
useTheme,
useToast,
Comment on lines +6 to +12
Copy link

@codeant-ai codeant-ai bot Nov 10, 2025

Choose a reason for hiding this comment

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

Suggestion: Remove the unused imports ListIcon and useToast from the Chakra import to avoid dead code. [maintainability]

Severity Level: Minor ⚠️

Suggested change
ListIcon,
ListItem,
Text,
Tooltip,
UnorderedList,
useTheme,
useToast,
ListItem,
Text,
Tooltip,
UnorderedList,
useTheme,
Why it matters? ⭐

The import block in TeamTable.tsx (shown in the current file) includes ListIcon and useToast, neither of which are referenced anywhere else in the file. The file uses useToastHook (from @devd-client/devd/components) and newToast; there are no usages of Chakra's useToast or ListIcon. Removing them will eliminate dead imports and any linter warnings without changing runtime behavior.

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** TeamTable.tsx
**Line:** 6:12
**Comment:**
	*Maintainability: Remove the unused imports `ListIcon` and `useToast` from the Chakra import to avoid dead code.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

Image,
} from '@chakra-ui/react';
import React, { useEffect, useMemo } from 'react';
import Present from '../assets/Present.svg';
import NotPresent from '../assets/notPresent.svg';

import {
ConfirmButton,
DataGrid,
Loader,
useToastHook,
} from '@devd-client/devd/components';
import { useNavigate } from 'react-router-dom';
import { apiClient } from '@devd-client/api';
import { createColumnHelper } from '@tanstack/react-table';
import { QUERY_KEYS, useFetchTeams } from '../apis';
import { AiOutlineMinusSquare, AiOutlinePlusSquare } from 'react-icons/ai';
import { useQueryClient } from '@tanstack/react-query';
import { MdOutlineEdit, MdPersonAddAlt } from 'react-icons/md';
import { TEAM_DETAILS_QUERY_KEYS } from '@devd-client/devd/team-details';

interface TeamViewProps {
team: string;
setTeamLength: any;
appState?: any;
teamLength?: number;
plan?: string;
}
Comment on lines +34 to +40
Copy link

@gemini-code-assist gemini-code-assist bot Nov 10, 2025

Choose a reason for hiding this comment

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

high

Throughout the component, the any type is used for props (setTeamLength, appState), state (rows), and generic type arguments (createColumnHelper<any>). This practice negates the benefits of TypeScript, such as type safety and improved developer experience. It's highly recommended to define and use specific interfaces for your data structures.

For example:

  • setTeamLength could be (length: number) => void.
  • A Team interface should be defined for the data from useFetchTeams and used for rows and createColumnHelper<Team>.

Comment on lines +34 to +40
Copy link

@coderabbitai coderabbitai bot Nov 10, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Improve type safety for the interface.

Multiple properties are typed as any, which defeats TypeScript's type checking and can lead to runtime errors. Consider defining proper types:

  • setTeamLength should be typed as (length: number) => void
  • appState should have a proper type definition or interface
  • Consider creating a proper type for team data instead of using any in column helper

Apply this diff to improve the setTeamLength type:

 interface TeamViewProps {
   team: string;
-  setTeamLength: any;
+  setTeamLength: (length: number) => void;
   appState?: any;
   teamLength?: number;
   plan?: string;
 }
📝 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
interface TeamViewProps {
team: string;
setTeamLength: any;
appState?: any;
teamLength?: number;
plan?: string;
}
interface TeamViewProps {
team: string;
setTeamLength: (length: number) => void;
appState?: any;
teamLength?: number;
plan?: string;
}
🤖 Prompt for AI Agents 
In TeamTable.tsx around lines 34 to 40, the TeamViewProps interface uses several
any types; update setTeamLength to have the signature (length: number) => void,
replace appState: any with a specific interface/type (create AppState or import
existing type) and avoid any in other places by creating a Team type (e.g., Team
or TeamRow) and use it for props and column helpers; update any column helper
usages to reference the Team type so TypeScript can enforce correct field types.


const TeamView: React.FC<TeamViewProps> = ({
Copy link

@refacto-visz refacto-visz bot Nov 10, 2025

Choose a reason for hiding this comment

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

God Component Violation

Single component handles multiple unrelated responsibilities violating SRP. TeamView manages data fetching, UI rendering, delete operations, navigation, toast notifications, and column definitions. This creates tight coupling and makes changes risky across multiple domains.

const useTeamOperations = () => {
  const queryClient = useQueryClient();
  const [newToast] = useToastHook();
  const [loading, setLoading] = React.useState(false);
  
  const handleDelete = (name: string) => {
    // Move delete logic here
  };
  
  return { handleDelete, loading };
};

const TeamView: React.FC<TeamViewProps> = ({ team, setTeamLength, appState, teamLength, plan }) => {
  const { handleDelete, loading } = useTeamOperations();
  const columns = useTeamColumns(handleDelete);
Commitable Suggestion
Suggested change
const TeamView: React.FC<TeamViewProps> = ({
const useTeamOperations = () => {
const queryClient = useQueryClient();
const [newToast] = useToastHook();
const [loading, setLoading] = React.useState(false);
const handleDelete = (name: string) => {
// Move delete logic here
};
return { handleDelete, loading };
};
const TeamView: React.FC<TeamViewProps> = ({ team, setTeamLength, appState, teamLength, plan }) => {
const { handleDelete, loading } = useTeamOperations();
const columns = useTeamColumns(handleDelete);
Standards
  • SOLID-SRP
  • Clean-Code-Class-Organization
  • Refactoring-Extract-Class

team,
setTeamLength,
appState,
teamLength,
plan,
}) => {
const queryClient = useQueryClient();
const { colors } = useTheme();
const navigate = useNavigate();
const [loading, setLoading] = React.useState(false);
const [newToast] = useToastHook();

const { data: teamsData, isFetching: teamsDataLoading } = useFetchTeams(team);

const rows: any[] = useMemo(() => teamsData?.dto || [], [teamsData?.dto]);

useEffect(() => {
setTeamLength(rows.length);
}, [rows, setTeamLength]);

const columnHelper = createColumnHelper<any>();

const columns = useMemo(
() => [
columnHelper.accessor('name', {
cell: (info) => {
return (
<Box pl={`${info.row.depth * 3}rem`}>
{info.row.getCanExpand() ? (
<>
<Button
variant={'unstyled'}
minWidth={0}
onClick={info.row.getToggleExpandedHandler()}
>
{info.row.getIsExpanded() ? (
<AiOutlineMinusSquare
size={18}
color={colors.text.primary}
/>
) : (
<AiOutlinePlusSquare
size={18}
color={colors.text.primary}
/>
)}
</Button>
</>
) : (
''
)}{' '}
{info.getValue()}
</Box>
);
},
header: 'Team Name',
}),

columnHelper.accessor('description', {
cell: (info) => {
if (!info.getValue()) {
return <Box color="orange.600">No Description Added</Box>;
} else {
return info.getValue();
}
},
header: 'Description',
size: 360,
}),

columnHelper.accessor('managers', {
cell: (info) => {
let count = 0;
info.row.original.subTeams.forEach((item: any) => {
count += item.managers.length;
});
Comment on lines +115 to +118
Copy link

@refacto-visz refacto-visz bot Nov 10, 2025

Choose a reason for hiding this comment

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

Inconsistent Variable Usage

Logic error in manager count calculation: variable 'count' is calculated but never used in the display logic. The forEach loop accumulates manager counts from subteams but this calculated value doesn't affect the rendered output. This creates misleading code that suggests functionality that doesn't exist.

          const subTeamManagerCount = info.row.original.subTeams.reduce((acc: number, item: any) => acc + item.managers.length, 0);
Commitable Suggestion
Suggested change
let count = 0;
info.row.original.subTeams.forEach((item: any) => {
count += item.managers.length;
});
const subTeamManagerCount = info.row.original.subTeams.reduce((acc: number, item: any) => acc + item.managers.length, 0);
Standards
  • Algorithm-Correctness-Variable-Usage
  • Logic-Verification-Data-Flow
  • Business-Rule-Display-Consistency

Comment on lines +115 to +118
Copy link

@refacto-visz refacto-visz bot Nov 10, 2025

Choose a reason for hiding this comment

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

N+1 Query Pattern

Nested iteration over subTeams array within cell renderer creates O(n*m) complexity where n is team count and m is average subteams per team. This computation executes on every render causing performance degradation with large team hierarchies. Consider pre-computing manager counts during data transformation.

          const subTeamManagerCount = info.row.original.subTeamManagerCount || 0;
Commitable Suggestion
Suggested change
let count = 0;
info.row.original.subTeams.forEach((item: any) => {
count += item.managers.length;
});
const subTeamManagerCount = info.row.original.subTeamManagerCount || 0;
Standards
  • ISO-IEC-25010-Performance-Efficiency-Time-Behavior
  • Algorithmic-Complexity-Nested-Loop-Optimization

return (
<Flex flexWrap="wrap">
{info?.getValue()?.length > 0 ? (
<Box>
{info?.getValue()?.map((item: string, idx: number) => (
<Text mr={2} key={`manager-${idx}`}>
{item}
{info.getValue().length - 1 !== idx && ','}
</Text>
))}
{info.row.original.subTeams.length > 0 ? (
<Text>+{count} in subteams</Text>
) : (
''
)}
Comment on lines +129 to +133
Copy link

@refacto-visz refacto-visz bot Nov 10, 2025

Choose a reason for hiding this comment

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

Duplicate Computation Pattern

Duplicate conditional logic for subteams count display creates redundant DOM evaluations and increases render complexity. The same subTeams.length check and count display logic appears twice within the same cell renderer. Consolidate logic to reduce computational overhead and improve maintainability.

Standards
  • ISO-IEC-25010-Performance-Efficiency-Resource-Utilization
  • Optimization-Pattern-Code-Deduplication

</Box>
) : (
<Box>
<Text color={'gray.400'} fontFamily={'heading'}>
No Manager Assigned
</Text>
{info.row.original.subTeams.length > 0 ? (
<Text>+{count} in subteams</Text>
) : (
''
)}
</Box>
)}
</Flex>
);
},
Comment on lines +114 to +149
Copy link

@gemini-code-assist gemini-code-assist bot Nov 10, 2025

Choose a reason for hiding this comment

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

medium

The logic for rendering managers is complex and likely has a rendering bug (managers will appear on separate lines because <Text> is a block element by default). It can be greatly simplified by using Array.prototype.join for the managers list and calculating the sub-team manager count with reduce. This also removes duplicated code.

        cell: (info) => {
          const managers = info.getValue() ?? [];
          const subTeamManagerCount = info.row.original.subTeams.reduce(
            (acc: number, team: any) => acc + team.managers.length,
            0
          );

          return (
            <Flex flexWrap="wrap" alignItems="baseline">
              {managers.length > 0 ? (
                <Text>{managers.join(', ')}</Text>
              ) : (
                <Text color="gray.400" fontFamily="heading">
                  No Manager Assigned
                </Text>
              )}
              {subTeamManagerCount > 0 && (
                <Text ml={2}>+{subTeamManagerCount} in subteams</Text>
              )}
            </Flex>
          );
        },

header: 'Managers',
}),
columnHelper.accessor('memberCount', {
cell: (info) => {
const rowData = info.row.original as any;
const managersCount = rowData?.managers
? rowData?.managers.length
: 0;
const membersCount = rowData?.memberCount ? rowData?.memberCount : 0;
const subTeamsCount = rowData?.subTeams ? rowData.subTeams.length : 0;


return (
<Flex
flexDirection="column"
gap="2"
width="100px"
alignItems="flex-start"
>
<Text>{managersCount} Managers</Text>
<Text>{membersCount} Members</Text>
<Text>{subTeamsCount} Subteams</Text>
</Flex>
);
},
header: 'Members',
size: 50,
meta: {
isNumeric: true,
},
}),
columnHelper.accessor('setupStates', {
cell: (info) => (
<UnorderedList listStyleType={'none'}>
<ListItem display={'flex'} gap={1}>
{info.getValue().Communication ? (
<Image src={Present}></Image>
) : (
<Image src={NotPresent}></Image>
)}
Communication
</ListItem>
<ListItem display={'flex'} gap={1}>
{info.getValue().Dora ? (
<Image src={Present}></Image>
) : (
<Image src={NotPresent}></Image>
)}
DORA
</ListItem>
<ListItem display={'flex'} gap={1}>
{info.getValue().Issue ? (
<Image src={Present}></Image>
) : (
<Image src={NotPresent}></Image>
)}
Issue
</ListItem>
<ListItem display={'flex'} gap={1}>
{info.getValue().Git ? (
<Image src={Present}></Image>
) : (
<Image src={NotPresent}></Image>
Comment on lines +186 to +212
Copy link

@codeant-ai codeant-ai bot Nov 10, 2025

Choose a reason for hiding this comment

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

Suggestion: Add descriptive alt attributes to the Image elements in the setup states cell for accessibility. [best practice]

Severity Level: Minor ⚠️

Suggested change
<Image src={Present}></Image>
) : (
<Image src={NotPresent}></Image>
)}
Communication
</ListItem>
<ListItem display={'flex'} gap={1}>
{info.getValue().Dora ? (
<Image src={Present}></Image>
) : (
<Image src={NotPresent}></Image>
)}
DORA
</ListItem>
<ListItem display={'flex'} gap={1}>
{info.getValue().Issue ? (
<Image src={Present}></Image>
) : (
<Image src={NotPresent}></Image>
)}
Issue
</ListItem>
<ListItem display={'flex'} gap={1}>
{info.getValue().Git ? (
<Image src={Present}></Image>
) : (
<Image src={NotPresent}></Image>
<Image src={Present} alt="Communication configured" />
) : (
<Image src={NotPresent} alt="Communication not configured" />
)}
Communication
</ListItem>
<ListItem display={'flex'} gap={1}>
{info.getValue().Dora ? (
<Image src={Present} alt="DORA configured" />
) : (
<Image src={NotPresent} alt="DORA not configured" />
)}
DORA
</ListItem>
<ListItem display={'flex'} gap={1}>
{info.getValue().Issue ? (
<Image src={Present} alt="Issue configured" />
) : (
<Image src={NotPresent} alt="Issue not configured" />
)}
Issue
</ListItem>
<ListItem display={'flex'} gap={1}>
{info.getValue().Git ? (
<Image src={Present} alt="Git configured" />
) : (
<Image src={NotPresent} alt="Git not configured" />
Why it matters? ⭐

The setup state icons are rendered with elements that currently lack alt attributes. Adding descriptive alt text (e.g., "Communication configured" / "Communication not configured") improves accessibility and removes potential JSX/ESLint warnings about missing alt on images. This change doesn't alter business logic and directly improves usability for assistive technologies.

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** TeamTable.tsx
**Line:** 186:212
**Comment:**
	*Best Practice: Add descriptive `alt` attributes to the `Image` elements in the setup states cell for accessibility.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

)}
Git
</ListItem>
</UnorderedList>
),
Comment on lines +182 to +217
Copy link

@gemini-code-assist gemini-code-assist bot Nov 10, 2025

Choose a reason for hiding this comment

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

medium

The code for rendering setup states is very repetitive. This can be made more concise and easier to maintain by iterating over an array of status configurations.

        cell: (info) => {
          const statuses = [
            { key: 'Communication', label: 'Communication' },
            { key: 'Dora', label: 'DORA' },
            { key: 'Issue', label: 'Issue' },
            { key: 'Git', label: 'Git' },
          ];
          const setupValue = info.getValue();

          return (
            <UnorderedList listStyleType="none">
              {statuses.map((status) => (
                <ListItem display="flex" gap={1} key={status.key}>
                  <Image src={setupValue[status.key] ? Present : NotPresent} />
                  {status.label}
                </ListItem>
              ))}
            </UnorderedList>
          );
        },

header: 'Config Status',
}),
Comment on lines +181 to +219
Copy link

@coderabbitai coderabbitai bot Nov 10, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add alt text to images for accessibility.

The Image components are missing alt attributes, which is an accessibility issue that prevents screen readers from describing the setup status icons to visually impaired users.

Apply this diff to add alt attributes:

           <ListItem display={'flex'} gap={1}>
             {info.getValue().Communication ? (
-              <Image src={Present}></Image>
+              <Image src={Present} alt="Communication setup completed"></Image>
             ) : (
-              <Image src={NotPresent}></Image>
+              <Image src={NotPresent} alt="Communication setup not completed"></Image>
             )}
             Communication
           </ListItem>
           <ListItem display={'flex'} gap={1}>
             {info.getValue().Dora ? (
-              <Image src={Present}></Image>
+              <Image src={Present} alt="DORA setup completed"></Image>
             ) : (
-              <Image src={NotPresent}></Image>
+              <Image src={NotPresent} alt="DORA setup not completed"></Image>
             )}
             DORA
           </ListItem>
           <ListItem display={'flex'} gap={1}>
             {info.getValue().Issue ? (
-              <Image src={Present}></Image>
+              <Image src={Present} alt="Issue setup completed"></Image>
             ) : (
-              <Image src={NotPresent}></Image>
+              <Image src={NotPresent} alt="Issue setup not completed"></Image>
             )}
             Issue
           </ListItem>
           <ListItem display={'flex'} gap={1}>
             {info.getValue().Git ? (
-              <Image src={Present}></Image>
+              <Image src={Present} alt="Git setup completed"></Image>
             ) : (
-              <Image src={NotPresent}></Image>
+              <Image src={NotPresent} alt="Git setup not completed"></Image>
             )}
             Git
           </ListItem>

Additionally, consider refactoring this repetitive code using an array map to reduce duplication:

{['Communication', 'Dora', 'Issue', 'Git'].map((setupType) => (
  <ListItem key={setupType} display={'flex'} gap={1}>
    {info.getValue()[setupType] ? (
      <Image src={Present} alt={`${setupType} setup completed`} />
    ) : (
      <Image src={NotPresent} alt={`${setupType} setup not completed`} />
    )}
    {setupType === 'Dora' ? 'DORA' : setupType}
  </ListItem>
))}
🤖 Prompt for AI Agents 
In TeamTable.tsx around lines 181 to 219, the Image components lack alt
attributes which is an accessibility issue; add descriptive alt text to every
<Image> (e.g., "Communication setup completed" / "Communication setup not
completed", etc.) so screen readers can convey status, and to reduce duplication
refactor the repeated ListItem blocks into a single map over an array like
['Communication','Dora','Issue','Git'] that renders each ListItem with the
correct alt text and uses 'DORA' for the display label when the key is 'Dora'.


columnHelper.accessor('actions', {
cell: (info) => (
<Flex>
<Tooltip label="Edit" hasArrow>
<IconButton
aria-label="edit"
bg="transparent"
size="sm"
color="text.secondary"
icon={<MdOutlineEdit size={16} />}
position={'static'}
onClick={(e) => {
e.preventDefault();
e.stopPropagation();
navigate(`/settings/teams/${info.row.original?.name}`);
}}
/>
</Tooltip>

<ConfirmButton
buttonText="Delete"
showIcon
bodyText={`Do you really want to delete this team - ${info.row.original?.name}?`}
headerText="Are you sure?"
onSuccessAction={() => {
if (info.row.original.subTeams.length >= 1) {
newToast({
message:
'Cannot delete top level team if sub teams available.',
status: 'error',
});
} else {
handleDelete(info.row.original?.name);
}
}}
/>

<Tooltip label="Add Member" hasArrow>
<IconButton
aria-label="edit"
Copy link

@gemini-code-assist gemini-code-assist bot Nov 10, 2025

Choose a reason for hiding this comment

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

medium

The aria-label for this IconButton is "edit", which is incorrect for an "Add Member" action. This is misleading for users of assistive technologies. It should be updated to accurately describe the button's function.

                aria-label="add member"

size="sm"
bg="transparent"
color="primary"
icon={<MdPersonAddAlt size={16} />}
position={'static'}
onClick={(e) => {
e.preventDefault();
e.stopPropagation();
navigate(`/settings/teams/${info.row.original?.name}`, {
state: { tab: 1 },
});
}}
/>
</Tooltip>
Comment on lines +258 to +274
Copy link

@coderabbitai coderabbitai bot Nov 10, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fix incorrect aria-label.

The "Add Member" button has aria-label="edit" which misrepresents its function to screen reader users.

Apply this diff:

           <Tooltip label="Add Member" hasArrow>
             <IconButton
-              aria-label="edit"
+              aria-label="add member"
               size="sm"
               bg="transparent"
               color="primary"
               icon={<MdPersonAddAlt size={16} />}
               position={'static'}
               onClick={(e) => {
                 e.preventDefault();
                 e.stopPropagation();
                 navigate(`/settings/teams/${info.row.original?.name}`, {
                   state: { tab: 1 },
                 });
               }}
             />
           </Tooltip>
📝 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
<Tooltip label="Add Member" hasArrow>
<IconButton
aria-label="edit"
size="sm"
bg="transparent"
color="primary"
icon={<MdPersonAddAlt size={16} />}
position={'static'}
onClick={(e) => {
e.preventDefault();
e.stopPropagation();
navigate(`/settings/teams/${info.row.original?.name}`, {
state: { tab: 1 },
});
}}
/>
</Tooltip>
<Tooltip label="Add Member" hasArrow>
<IconButton
aria-label="add member"
size="sm"
bg="transparent"
color="primary"
icon={<MdPersonAddAlt size={16} />}
position={'static'}
onClick={(e) => {
e.preventDefault();
e.stopPropagation();
navigate(`/settings/teams/${info.row.original?.name}`, {
state: { tab: 1 },
});
}}
/>
</Tooltip>
🤖 Prompt for AI Agents 
In TeamTable.tsx around lines 258 to 274, the IconButton for "Add Member"
incorrectly uses aria-label="edit"; change the aria-label to a descriptive,
accurate value such as "add member" (or "add team member") so screen readers
convey the correct action, and ensure the label is lower-case or title-cased
consistently with the app's accessibility conventions.

</Flex>
),
header: 'Actions',
}),
],
[teamsData?.dto]
);
Comment on lines +65 to +281
Copy link

@gemini-code-assist gemini-code-assist bot Nov 10, 2025

Choose a reason for hiding this comment

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

medium

The useMemo hook for columns is not being used effectively, which can lead to performance issues.

  1. The dependency array [teamsData?.dto] is incorrect. The column definitions don't depend on the data itself, causing unnecessary recalculations when data changes.
  2. The handleDelete function, used within the 'Actions' column, is defined after this useMemo block and is not wrapped in useCallback. This means it's recreated on every render, which would break memoization.

To fix this, you should:

  1. Move the handleDelete function definition to be before the columns useMemo block.
  2. Wrap handleDelete in React.useCallback with its dependencies (e.g., [queryClient, newToast]).
  3. Update the useMemo dependency array for columns to include its actual dependencies, like [colors, navigate, newToast, handleDelete].


const handleDelete = (name: string) => {
setLoading(true);
apiClient(`/v1/team/${localStorage.getItem('orgId')}/${name}`, {
Copy link

@refacto-visz refacto-visz bot Nov 10, 2025

Choose a reason for hiding this comment

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

Missing Input Validation

URL constructed with unsanitized parameters from localStorage and function parameter violates organization guideline to validate all external inputs. Path traversal or injection attacks possible through manipulated orgId or name values enabling unauthorized resource access.

    const sanitizeInput = (input: string | null): string => {
      if (!input || typeof input !== 'string') {
        throw new Error('Invalid input');
      }
      return input.replace(/[^a-zA-Z0-9_-]/g, '');
    };
    
    const sanitizedOrgId = sanitizeInput(localStorage.getItem('orgId'));
    const sanitizedName = sanitizeInput(name);
    
    apiClient(`/v1/team/${sanitizedOrgId}/${sanitizedName}`, {
Commitable Suggestion
Suggested change
apiClient(`/v1/team/${localStorage.getItem('orgId')}/${name}`, {
const sanitizeInput = (input: string | null): string => {
if (!input || typeof input !== 'string') {
throw new Error('Invalid input');
}
return input.replace(/[^a-zA-Z0-9_-]/g, '');
};
const sanitizedOrgId = sanitizeInput(localStorage.getItem('orgId'));
const sanitizedName = sanitizeInput(name);
apiClient(`/v1/team/${sanitizedOrgId}/${sanitizedName}`, {
Standards
  • CWE-20
  • OWASP-A03
  • Org-Guideline-Validate all external inputs

method: 'DELETE',
headers: {
Accept: 'application/json',
'Content-Type': 'application/json',
authorization: `bearer ${localStorage.getItem('token')}`,
Copy link

@gemini-code-assist gemini-code-assist bot Nov 10, 2025

Choose a reason for hiding this comment

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

high

Storing sensitive information like authentication tokens in localStorage is a security risk as it's vulnerable to Cross-Site Scripting (XSS) attacks. An attacker could steal the token and impersonate the user. Consider using httpOnly cookies for storing tokens, which are not accessible via JavaScript.

},
Comment on lines +285 to +291
Copy link

@refacto-visz refacto-visz bot Nov 10, 2025

Choose a reason for hiding this comment

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

Hardcoded Secrets Storage

Authentication token retrieved from localStorage without validation or encryption violates organization guideline requiring no hard-coded secrets or credentials. Attackers with XSS access can steal tokens enabling account takeover and unauthorized API access.

    const secureAuth = useSecureAuth();
    const { orgId, token } = await secureAuth.getValidatedCredentials();
    
    if (!orgId || !token) {
      throw new Error('Authentication required');
    }
    
    apiClient(`/v1/team/${orgId}/${name}`, {
      method: 'DELETE',
      headers: {
        Accept: 'application/json',
        'Content-Type': 'application/json',
        authorization: `bearer ${token}`,
Commitable Suggestion
Suggested change
apiClient(`/v1/team/${localStorage.getItem('orgId')}/${name}`, {
method: 'DELETE',
headers: {
Accept: 'application/json',
'Content-Type': 'application/json',
authorization: `bearer ${localStorage.getItem('token')}`,
},
const secureAuth = useSecureAuth();
const { orgId, token } = await secureAuth.getValidatedCredentials();
if (!orgId || !token) {
throw new Error('Authentication required');
}
apiClient(`/v1/team/${orgId}/${name}`, {
method: 'DELETE',
headers: {
Accept: 'application/json',
'Content-Type': 'application/json',
authorization: `bearer ${token}`,
Standards
  • CWE-522
  • OWASP-A02
  • Org-Guideline-No hard-coded secrets or credentials

})
.then((res: any) => {
Promise.all([
queryClient.invalidateQueries([QUERY_KEYS.teams]),
queryClient.invalidateQueries([TEAM_DETAILS_QUERY_KEYS.teamList]),
queryClient.invalidateQueries(['teamMenu']),
]);

newToast({
message: `"${name}" Team has been deleted successfully.`,
status: 'success',
});
})
.catch((err: any) => {
newToast({
message:
err?.message ?? `Some error occurred while deleting ${name}.`,
status: 'error',
});
})
.finally(() => {
setLoading(false);
});
};
Comment on lines +283 to +315
Copy link

@coderabbitai coderabbitai bot Nov 10, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Await query invalidation to ensure data consistency.

The Promise.all on lines 294-298 invalidates multiple queries but is not awaited, which means the success toast may appear before the UI is fully refreshed. This could lead to temporary inconsistent state where the deleted team still appears briefly.

Apply this diff:

     .then((res: any) => {
-      Promise.all([
+      await Promise.all([
         queryClient.invalidateQueries([QUERY_KEYS.teams]),
         queryClient.invalidateQueries([TEAM_DETAILS_QUERY_KEYS.teamList]),
         queryClient.invalidateQueries(['teamMenu']),
       ]);

       newToast({
         message: `"${name}" Team has been deleted successfully.`,
         status: 'success',
       });
     })

And make the function async:

-  const handleDelete = (name: string) => {
+  const handleDelete = async (name: string) => {

Additionally, consider validating that the token and orgId exist before making the API call to provide better error handling if they're missing.

🏁 Script executed:

#!/bin/bash
# Locate TeamTable.tsx and verify the handleDelete function
fd -t f "TeamTable.tsx" --exec cat {}

Length of output: 9768

🏁 Script executed:

#!/bin/bash
# Search for how queryClient.invalidateQueries is used elsewhere in the codebase
rg -n "invalidateQueries" -A2 -B2 --type=tsx --type=ts

Length of output: 27

🏁 Script executed:

#!/bin/bash
# Check for async handleDelete patterns and Promise.all usage with await
rg -n "handleDelete|Promise\.all" -A3 -B1 --type=tsx --type=ts | head -100

Length of output: 27

🏁 Script executed:

#!/bin/bash
# Search for queryClient and invalidateQueries patterns without type flags
rg -n "invalidateQueries" -A2 -B2 | head -50

Length of output: 381

🏁 Script executed:

#!/bin/bash
# Search for other promise patterns and async handlers in the codebase
rg -n "\.then\(" -A3 -B1 | head -80

Length of output: 284

🏁 Script executed:

#!/bin/bash
# Check React Query package to understand invalidateQueries behavior
find . -name "package*.json" -type f -exec grep -l "react-query\|@tanstack/react-query" {} \;

Length of output: 0

🌐 Web query:

React Query invalidateQueries return type promise behavior

💡 Result:

Short answer: invalidateQueries returns a Promise — it resolves when the (background) refetches it triggered are finished (or immediately if no refetch needed). So you can await it (e.g. return await queryClient.invalidateQueries(...) from an onSuccess) to keep a mutation pending until invalidation/refetch completes. [1][2][3]

Notes:

  • You can control whether active/inactive queries refetch with options (refetchActive/refetchInactive) and whether the Promise throws on refetch errors (throwOnError). [2]
  • Common pattern: await Promise.all([...invalidateQueries(...)]) when invalidating multiple keys. [1][3]

Sources:
[1] TanStack Query — Invalidations from Mutations (example using await).
[2] TanStack Query — QueryClient reference (invalidateQueries/refetch and Promise behavior/options).
[3] tkdodo / dev.to — explanation and examples showing invalidateQueries returns a Promise.

Await query invalidation to ensure data consistency.

The invalidateQueries calls return Promises that resolve when background refetches complete. Currently, Promise.all on lines 294-298 is not awaited, allowing the success toast to display before queries are actually invalidated and refetched. This creates a race condition where the deleted team may briefly reappear in the UI.

Apply this diff:

     .then((res: any) => {
-      Promise.all([
+      await Promise.all([
         queryClient.invalidateQueries([QUERY_KEYS.teams]),
         queryClient.invalidateQueries([TEAM_DETAILS_QUERY_KEYS.teamList]),
         queryClient.invalidateQueries(['teamMenu']),
       ]);

       newToast({
         message: `"${name}" Team has been deleted successfully.`,
         status: 'success',
       });
     })

And make the function async:

-  const handleDelete = (name: string) => {
+  const handleDelete = async (name: string) => {

Additionally, validate that token and orgId exist before making the API call to catch missing credentials early.

📝 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 handleDelete = (name: string) => {
setLoading(true);
apiClient(`/v1/team/${localStorage.getItem('orgId')}/${name}`, {
method: 'DELETE',
headers: {
Accept: 'application/json',
'Content-Type': 'application/json',
authorization: `bearer ${localStorage.getItem('token')}`,
},
})
.then((res: any) => {
Promise.all([
queryClient.invalidateQueries([QUERY_KEYS.teams]),
queryClient.invalidateQueries([TEAM_DETAILS_QUERY_KEYS.teamList]),
queryClient.invalidateQueries(['teamMenu']),
]);
newToast({
message: `"${name}" Team has been deleted successfully.`,
status: 'success',
});
})
.catch((err: any) => {
newToast({
message:
err?.message ?? `Some error occurred while deleting ${name}.`,
status: 'error',
});
})
.finally(() => {
setLoading(false);
});
};
const handleDelete = async (name: string) => {
setLoading(true);
apiClient(`/v1/team/${localStorage.getItem('orgId')}/${name}`, {
method: 'DELETE',
headers: {
Accept: 'application/json',
'Content-Type': 'application/json',
authorization: `bearer ${localStorage.getItem('token')}`,
},
})
.then((res: any) => {
await Promise.all([
queryClient.invalidateQueries([QUERY_KEYS.teams]),
queryClient.invalidateQueries([TEAM_DETAILS_QUERY_KEYS.teamList]),
queryClient.invalidateQueries(['teamMenu']),
]);
newToast({
message: `"${name}" Team has been deleted successfully.`,
status: 'success',
});
})
.catch((err: any) => {
newToast({
message:
err?.message ?? `Some error occurred while deleting ${name}.`,
status: 'error',
});
})
.finally(() => {
setLoading(false);
});
};
🤖 Prompt for AI Agents 
In TeamTable.tsx around lines 283 to 315, the delete handler should be made
async and must validate token and orgId from localStorage before calling the
API; if either is missing, return early with an error toast and
setLoading(false). Replace the current promise chain with an async/await flow:
await the apiClient DELETE call inside try, then await Promise.all of the three
queryClient.invalidateQueries calls so invalidations/refetches finish before
showing the success toast; handle errors in catch (show error toast) and always
setLoading(false) in finally.


return (
<>
<DataGrid
page="team"
teamLength={teamLength}
plan={plan}
appState={appState}
showLoader={teamsDataLoading}
columns={columns}
data={rows}
maxH="40rem"
sticky="firstCol"
/>
{(loading || teamsDataLoading) && <Loader />}
</>
);
};

export default TeamView;