Sales fix #370
Sales fix #370
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Reviewer's GuideThis PR refactors the deals module by replacing query-string state with Jotai atoms, enhancing Apollo hooks with error handling and optimized fetch policies, and introduces a new drag-and-drop board UI along with reusable select components for priority and dates. Class diagram for new and updated Jotai atoms in deals state managementclassDiagram
class dealCreateSheetState {
+boolean
}
class dealCreateDefaultValuesState {
+Partial<any> | undefined
}
class dealDetailSheetState {
+string | null
}
class dealTotalCountAtom {
+number | null
}
class dealCountByBoardAtom {
+Record<string, number>
}
class dealTotalCountBoardAtom {
+number
}
class dealsViewAtom {
+'list' | 'grid'
}
dealTotalCountBoardAtom --|> dealCountByBoardAtom : derives from
Class diagram for DealsBoard and DealsBoardCard componentsclassDiagram
class DealsBoard {
+columns: BoardColumnProps[]
+deals: BoardItemProps[]
+handleDragEnd(event: DragEndEvent)
}
class DealsBoardCard {
+id: string
+startDate: Date
+closeDate: Date
+name: string
+assignedUsers: User[]
+priority: string
+createdAt: Date
+labels: Label[]
}
DealsBoardCard --> DealsBoard : uses
DealsBoardCard --> DateSelectDeal : uses
DealsBoardCard --> SelectDealPriority : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In useDeals you’re calling toast on errors but you haven’t imported the toast helper or hook—add the correct import to prevent runtime failures.
- useDealsEdit now relies solely on refetchQueries and drops the optimisticResponse/cache updates, which could hurt responsiveness—consider re-adding cache update logic or optimistic updates to keep the UI snappy.
- DealsBoard and DealsBoardCards both manage fetchedDealsState and mapping logic in a similar way—extract and reset that state when pipelineId changes to simplify the code and avoid stale entries.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In useDeals you’re calling toast on errors but you haven’t imported the toast helper or hook—add the correct import to prevent runtime failures.
- useDealsEdit now relies solely on refetchQueries and drops the optimisticResponse/cache updates, which could hurt responsiveness—consider re-adding cache update logic or optimistic updates to keep the UI snappy.
- DealsBoard and DealsBoardCards both manage fetchedDealsState and mapping logic in a similar way—extract and reset that state when pipelineId changes to simplify the code and avoid stale entries.
## Individual Comments
### Comment 1
<location> `frontend/plugins/sales_ui/src/modules/deals/cards/hooks/useDeals.tsx:230-240` </location>
<code_context>
- const [_id] = useQueryState('salesItemId');
- const [stageId] = useQueryState('stageId');
+ const [_id] = useAtom(dealDetailSheetState);
+ const [defaultValues] = useAtom(dealCreateDefaultValuesState);
const [addDeals, { loading, error }] = useMutation(ADD_DEALS, {
...options,
variables: {
...options?.variables,
_id,
- stageId,
+ stageId: defaultValues?.stageId,
},
refetchQueries: [
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Default values for stageId may be undefined.
Validate that 'defaultValues' and 'defaultValues.stageId' are defined before assigning to mutation variables, or set a fallback value to prevent sending undefined.
```suggestion
const [_id] = useAtom(dealDetailSheetState);
const [defaultValues] = useAtom(dealCreateDefaultValuesState);
const stageId = defaultValues && typeof defaultValues.stageId !== 'undefined'
? defaultValues.stageId
: null; // fallback value, adjust as needed
const [addDeals, { loading, error }] = useMutation(ADD_DEALS, {
...options,
variables: {
...options?.variables,
_id,
stageId,
},
refetchQueries: [
```
</issue_to_address>
### Comment 2
<location> `frontend/plugins/sales_ui/src/modules/deals/stage/components/StageHeader.tsx:25-27` </location>
<code_context>
};
export const StageHeader = ({ stage = {} as IStage }: Props) => {
- const [open, setOpen] = useState<boolean>(false);
+ const [open, setOpen] = useAtom(dealCreateSheetState);
const [showSortOptions, setShowSortOptions] = useState(false);
const [, setStageId] = useQueryState<string | null>('stageId');
</code_context>
<issue_to_address>
**issue:** Using a global atom for sheet open state may cause unintended side effects.
Using a global atom here means all StageHeader components will share the same open state, which may result in unintended UI behavior if multiple instances are rendered. Consider using local state or a scoped atom instead.
</issue_to_address>
### Comment 3
<location> `frontend/plugins/sales_ui/src/modules/deals/cards/components/item/Footer.tsx:20` </location>
<code_context>
</div>
<div className="flex">
<MembersInline.Provider
- memberIds={assignedUsers.map((user) => user._id)}
+ memberIds={(assignedUsers || []).map((user) => user._id)}
>
<MembersInline.Avatar />
</code_context>
<issue_to_address>
**nitpick:** Redundant fallback for assignedUsers.
If assignedUsers defaults to an empty array, you can remove the fallback to [].
</issue_to_address>
### Comment 4
<location> `frontend/plugins/sales_ui/src/modules/deals/components/DealsBoard.tsx:74-83` </location>
<code_context>
+ stageId: overColumn,
+ },
+ });
+ setDeals((prev) =>
+ prev.map((deal) => {
+ if (deal.id === activeItem?._id) {
+ return {
+ ...deal,
+ column: overColumn,
+ sort: new Date().toISOString(),
+ };
+ }
+ return deal;
+ }),
+ );
+ setDealCountByBoard((prev) => ({
</code_context>
<issue_to_address>
**issue (bug_risk):** Directly mutating deal state on drag end may cause inconsistencies.
Consider implementing optimistic UI updates and ensure changes are reverted if the mutation fails, to prevent UI inconsistencies.
</issue_to_address>
### Comment 5
<location> `frontend/plugins/sales_ui/src/modules/deals/components/DealsBoard.tsx:132-139` </location>
<code_context>
+ }
+ }
+
+ const boardCards = dealCards
+ .filter((deal) => deal.column === column.id)
+ .sort((a, b) => {
+ if (a.sort && b.sort) {
+ return b.sort.toString().localeCompare(a.sort.toString());
</code_context>
<issue_to_address>
**suggestion:** Sorting by 'sort' property may not be reliable if values are missing.
If 'a.sort' or 'b.sort' is missing or invalid, the sort order may be inconsistent. Please add a fallback or improve the sorting logic to handle these cases.
```suggestion
const boardCards = dealCards
.filter((deal) => deal.column === column.id)
.sort((a, b) => {
const aSort = Number(a.sort);
const bSort = Number(b.sort);
const aValid = !isNaN(aSort);
const bValid = !isNaN(bSort);
if (aValid && bValid) {
// Sort descending
return bSort - aSort;
}
if (aValid) {
// a has valid sort, b does not
return -1;
}
if (bValid) {
// b has valid sort, a does not
return 1;
}
// Neither has valid sort, keep original order
return 0;
});
```
</issue_to_address>
### Comment 6
<location> `frontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectDealPriority.tsx:21-25` </location>
<code_context>
+ Critical: 4,
+};
+
+function getPriorityString(value: number) {
+ return (Object.keys(PRIORITY_MAP) as Array<keyof typeof PRIORITY_MAP>).find(
+ (key) => PRIORITY_MAP[key] === value,
+ );
+}
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** getPriorityString may return undefined for invalid values.
Returning undefined for invalid values may lead to problems when updating the deal's priority. Please add a default value or error handling for cases where the priority is not found.
```suggestion
function getPriorityString(value: number) {
const priority = (Object.keys(PRIORITY_MAP) as Array<keyof typeof PRIORITY_MAP>).find(
(key) => PRIORITY_MAP[key] === value,
);
if (priority !== undefined) {
return priority;
}
// Return default value or handle error
return 'No Priority';
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const [_id] = useAtom(dealDetailSheetState); | ||
| const [defaultValues] = useAtom(dealCreateDefaultValuesState); | ||
|
|
||
| const [addDeals, { loading, error }] = useMutation(ADD_DEALS, { | ||
| ...options, | ||
| variables: { | ||
| ...options?.variables, | ||
| _id, | ||
| stageId, | ||
| stageId: defaultValues?.stageId, | ||
| }, | ||
| refetchQueries: [ |
There was a problem hiding this comment.
suggestion (bug_risk): Default values for stageId may be undefined.
Validate that 'defaultValues' and 'defaultValues.stageId' are defined before assigning to mutation variables, or set a fallback value to prevent sending undefined.
| const [_id] = useAtom(dealDetailSheetState); | |
| const [defaultValues] = useAtom(dealCreateDefaultValuesState); | |
| const [addDeals, { loading, error }] = useMutation(ADD_DEALS, { | |
| ...options, | |
| variables: { | |
| ...options?.variables, | |
| _id, | |
| stageId, | |
| stageId: defaultValues?.stageId, | |
| }, | |
| refetchQueries: [ | |
| const [_id] = useAtom(dealDetailSheetState); | |
| const [defaultValues] = useAtom(dealCreateDefaultValuesState); | |
| const stageId = defaultValues && typeof defaultValues.stageId !== 'undefined' | |
| ? defaultValues.stageId | |
| : null; // fallback value, adjust as needed | |
| const [addDeals, { loading, error }] = useMutation(ADD_DEALS, { | |
| ...options, | |
| variables: { | |
| ...options?.variables, | |
| _id, | |
| stageId, | |
| }, | |
| refetchQueries: [ |
| </div> | ||
| <div className="flex"> | ||
| <MembersInline.Provider | ||
| memberIds={assignedUsers.map((user) => user._id)} |
There was a problem hiding this comment.
nitpick: Redundant fallback for assignedUsers.
If assignedUsers defaults to an empty array, you can remove the fallback to [].
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to 3f95ea8 in 2 minutes and 13 seconds. Click for details.
- Reviewed
1708lines of code in24files - Skipped
1files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. frontend/plugins/sales_ui/src/modules/deals/cards/hooks/useDeals.tsx:69
- Draft comment:
Include additional dependencies (e.g. 'currentUser' and 'lastPipelineId') in the useEffect dependency array to ensure the subscription updates correctly. - Reason this comment was not posted:
Comment was on unchanged code.
2. frontend/plugins/sales_ui/src/modules/deals/components/DealsBoard.tsx:86
- Draft comment:
When updating board counts, ensure the count does not fall below zero. Consider validating the new count before applying the decrement. - Reason this comment was not posted:
Comment looked like it was already resolved.
3. frontend/plugins/sales_ui/src/modules/deals/graphql/queries/DealsQueries.ts:38
- Draft comment:
Review the formatting in the 'commonParams' string. Although GraphQL permits whitespace-separated arguments, explicit commas can improve clarity and maintainability. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
4. frontend/plugins/sales_ui/src/modules/deals/stage/components/StageHeader.tsx:38
- Draft comment:
Toggling the sheet open state using '!open' can be error‐prone when the same state is managed by multiple components. Consider a more explicit state update. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_lLMoWeeVP9dMbU6c
You can customize 
|
|
||
| return ( | ||
| <Command.Item | ||
| value={clsx(priorityLabel, value)} |
There was a problem hiding this comment.
The 'value' prop for Command.Item uses clsx() combining a label and a number; ensure this produces a unique and intended string identifier for each item.
| value={clsx(priorityLabel, value)} | |
| value={`${priorityLabel}-${priority}`} |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughIntroduces a kanban-style DealsBoard with card components, Jotai-based shared state (detail/create sheet, counts, view), cursor-based deals fetching with subscriptions and edits, new date/priority selectors, and page wiring to use the board. Also includes minor safeguards/formatting and GraphQL field updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Page as SalesIndexPage
participant Board as DealsBoard
participant Hook as useDeals/useDealsEdit
participant GQL as GraphQL API
participant State as Jotai Atoms
User->>Page: Navigate to Sales
Page->>Board: Render
Board->>Hook: useDeals({ pipelineId, stageId, cursor })
Hook->>GQL: Query deals (cache-and-network)
GQL-->>Hook: Cursor list {items, pageInfo, totalCount}
Hook-->>Board: deals, pageInfo, totalCount
User->>Board: Scroll to bottom
Board->>Hook: fetchMore(nextCursor)
Hook->>GQL: Query next page
GQL-->>Hook: More items
Hook-->>Board: Append items
User->>Board: Click card
Board->>State: set(dealDetailSheetState, dealId)
User->>Board: Drag card to new stage
Board->>Hook: editDeals({_id, stageId})
Hook->>GQL: Mutation update
GQL-->>Hook: Updated deal
Hook-->>Board: Local list update
Note over Hook,GQL: subscribeToMore for deal changes
sequenceDiagram
autonumber
actor User
participant Header as StageHeader/AddDealSheet
participant State as dealCreateSheetState
participant Form as AddCardForm
participant Hook as useDealsAdd
participant GQL as GraphQL API
User->>Header: Open "Add deal"
Header->>State: set(true)
State-->>Header: open = true
Header->>Form: Render with onClose
User->>Form: Submit new deal
Form->>Hook: useDealsAdd({ stageId, fields })
Hook->>GQL: Mutation create
GQL-->>Hook: Created deal
Hook-->>Form: onCompleted
Form->>State: set(false)
sequenceDiagram
autonumber
actor User
participant Card as DealsBoardCard
participant DateSel as DateSelectDeal
participant PrioSel as SelectDealPriority
participant Hook as useDealsEdit
participant GQL as GraphQL API
User->>Card: Change start/close date
Card->>DateSel: Open popover, pick date
DateSel->>Hook: editDeals({_id, startDate/closeDate})
Hook->>GQL: Mutation
GQL-->>Hook: Updated deal
User->>Card: Change priority
Card->>PrioSel: Select new priority
PrioSel->>Hook: editDeals({_id, priority})
Hook->>GQL: Mutation
GQL-->>Hook: Updated deal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
frontend/libs/erxes-ui/src/modules/board/components/Board.tsx (1)
179-184: Handle undefined column values consistently.Now that
BoardItemProps.columncan beundefined(per the type change inboardTypes.ts), the type assertion at line 181 (overItem.column as string) is unsafe. IfoverItem.columnis undefined, it bypasses type safety and passes undefined tosetDragOverBoardColumnId, which is inconsistent with the null-handling pattern at line 183.Apply this diff to handle undefined properly:
const overItem = data.find((item) => item.id === over.id); if (overItem) { - setDragOverBoardColumnId(overItem.column as string); + setDragOverBoardColumnId(overItem.column ?? null); } else { setDragOverBoardColumnId(null); }frontend/plugins/sales_ui/src/modules/deals/pipelines/components/PipelineFormBar.tsx (1)
55-66: Preserve 0 probability; avoid||fallback losing valid 0 valuesUsing
|| ''sets probability to empty string when it's 0. Use nullish coalescing to keep 0.Apply:
- probability: stage.probability || '', + probability: stage.probability ?? '',frontend/plugins/sales_ui/src/modules/deals/cards/hooks/useDeals.tsx (2)
69-125: Subscription may use stale pipeline/user; fix effect deps and resubscribeThe effect depends on
lastPipelineId,currentUser?._id, andsubscribeToMore, but only listsoptions?.variables. This can miss resubscriptions on pipeline/user changes.- useEffect(() => { + useEffect(() => { const unsubscribe = subscribeToMore<IDealChanged>({ document: DEAL_LIST_CHANGED, variables: { pipelineId: lastPipelineId, userId: currentUser?._id, filter: options?.variables, }, updateQuery: (prev, { subscriptionData }) => { if (!prev || !subscriptionData.data) return prev; @@ }, }); return unsubscribe; - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [options?.variables]); + }, [subscribeToMore, lastPipelineId, currentUser?._id, options?.variables]);
136-146: UsecursorModeinstead ofdirectionin fetchMore variables
TheDealsquery defines$cursorMode, not$direction, so update thefetchMorecall to passcursorMode: direction(and removedirection).
♻️ Duplicate comments (3)
frontend/plugins/sales_ui/src/modules/deals/cards/components/item/Footer.tsx (1)
20-20: Redundant null-check with default parameter.The
|| []fallback is unnecessary sinceassignedUsersalready defaults to[]in the function signature (line 11). The default parameter ensures the value is never undefined during the mapping operation.Consider simplifying to:
- memberIds={(assignedUsers || []).map((user) => user._id)} + memberIds={assignedUsers.map((user) => user._id)}frontend/plugins/sales_ui/src/modules/deals/cards/hooks/useDeals.tsx (1)
230-239: GuardstageIddefault (duplicate of prior review)
defaultValues?.stageIdmay be undefined; set a safe fallback if the server expects nullable/omitted.- _id, - stageId: defaultValues?.stageId, + _id, + stageId: defaultValues && typeof defaultValues.stageId !== 'undefined' + ? defaultValues.stageId + : null,frontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectPriority.tsx (1)
95-111: Fix the Command.Item value prop to ensure unique identifiers.Line 101 uses
clsx(priorityLabel, value)which concatenates the priority label string with the current selected value number. This produces inconsistent and potentially non-unique identifiers (e.g., "High3" if the selected value is 3). The value prop should uniquely identify this specific item, not include the current selection state.As noted in the past review comment, use a combination of the label and the item's own priority value:
<Command.Item - value={clsx(priorityLabel, value)} + value={`${priorityLabel}-${priority}`} onSelect={() => onValueChange(priority)} >
🧹 Nitpick comments (17)
frontend/plugins/sales_ui/src/modules/deals/states/dealsViewState.ts (1)
1-10: LGTM! Clean implementation of persisted view state.The atom correctly uses
atomWithStorageto persist the deals view preference between 'list' and 'grid'. ThegetOnInit: trueoption appropriately ensures the stored value is loaded on initialization.Optional refinement: The explicit
undefinedfor the storage parameter can be omitted since it's optional:-export const dealsViewAtom = atomWithStorage<'list' | 'grid'>( - 'dealsView', - 'list', - undefined, - { - getOnInit: true, - }, -); +export const dealsViewAtom = atomWithStorage<'list' | 'grid'>( + 'dealsView', + 'list', + { + getOnInit: true, + }, +);This reduces verbosity without changing behavior, as the default storage (localStorage in browsers) is used when the parameter is omitted.
frontend/plugins/sales_ui/src/modules/deals/pipelines/components/PipelineFormBar.tsx (2)
210-212: Button label should reflect mode (Create vs Save)Show “Save” when editing to avoid UX confusion.
- <Button type="submit" disabled={addLoading || editLoading}> - {addLoading || editLoading ? <Spinner /> : 'Create'} - </Button> + <Button type="submit" disabled={addLoading || editLoading}> + {addLoading || editLoading ? <Spinner /> : pipelineId ? 'Save' : 'Create'} + </Button>
34-38: Simplify destructuring for readabilityCurrent double-destructuring is harder to scan. Prefer a two‑step destructure.
- const { - methods, - methods: { reset, handleSubmit }, - } = usePipelineForm(); + const { methods } = usePipelineForm(); + const { reset, handleSubmit } = methods;frontend/plugins/sales_ui/src/modules/deals/cards/components/detail/SalesItemDetail.tsx (1)
22-25: Open sheet immediately; don’t gate by loadingGating
openby!loadingcan cause flicker or delay. Open whenactiveDealIdexists and show skeleton/spinner inside.- <Sheet - open={!!activeDealId && !loading} - onOpenChange={() => setActiveDealId(null)} - > + <Sheet + open={!!activeDealId} + onOpenChange={() => setActiveDealId(null)} + >frontend/plugins/sales_ui/src/modules/deals/cards/hooks/useDeals.tsx (2)
67-68: Defaultdealsto empty arrayAvoid propagating
undefinedto consumers.- const { list: deals, pageInfo, totalCount } = data?.deals || {}; + const { + list: deals = [], + pageInfo, + totalCount, + } = data?.deals || {};
171-182: UseuseAtomValuewhen you don’t need the setterSlightly cleaner and avoids creating an unused setter.
- const [activeDealId] = useAtom(dealDetailSheetState); + const activeDealId = useAtomValue(dealDetailSheetState);frontend/plugins/sales_ui/src/modules/deals/cards/components/detail/overview/Priority.tsx (1)
19-19: Hook choice is fine; minor export/id polish
- Using
useDealsEdithere is OK; you override_idwithdealIdper-call, which is explicit.- Consider named export for components per guidelines, and a more specific trigger id.
-const Priority = ({ +export const Priority = ({ @@ - <Select.Trigger id="time-unit" className="mt-1"> + <Select.Trigger id="deal-priority" className="mt-1"> @@ -export default Priority; +// no default exportAs per coding guidelines
frontend/plugins/sales_ui/src/modules/deals/states/dealCreateSheetState.ts (1)
4-6: Tighten atom typing; avoid PartialDefine a minimal interface for defaults (stageId is used today). Improves safety and autocomplete.
-export const dealCreateDefaultValuesState = atom<Partial<any> | undefined>( - undefined, -); +export interface DealCreateDefaults { + stageId?: string; +} +export const dealCreateDefaultValuesState = atom<DealCreateDefaults | undefined>( + undefined, +);frontend/plugins/sales_ui/src/modules/deals/components/AddDealSheet.tsx (1)
17-28: LGTM; optional simplificationLooks good. You could simplify onOpenChange to setOpen if no extra side effects are needed.
-<Sheet open={open} onOpenChange={(open) => (open ? onOpen() : onClose())}> +<Sheet open={open} onOpenChange={setOpen}>frontend/plugins/sales_ui/src/modules/deals/components/DealsBoard.tsx (5)
44-49: Guard columns with a fallback arrayAvoid passing undefined to Board.Provider.
- const columns = stages?.map((stage) => ({ + const columns = (stages ?? []).map((stage) => ({ id: stage._id, name: stage.name, type: stage.type, }));
86-91: Safer count updates; avoid '' key and clamp at 0Old stage id may be undefined. Also (x - 1) || 0 can mask negatives unexpectedly.
- setDealCountByBoard((prev) => ({ - ...prev, - [activeItem?.stage?._id || '']: - (prev[activeItem?.stage?._id || ''] || 0) - 1 || 0, - [overColumn]: (prev[overColumn] || 0) + 1, - })); + setDealCountByBoard((prev) => { + const prevStageId = activeItem?.stage?._id; + const next: Record<string, number> = { ...prev }; + if (prevStageId) { + const nextVal = (prev[prevStageId] ?? 0) - 1; + next[prevStageId] = nextVal < 0 ? 0 : nextVal; + } + next[overColumn!] = (prev[overColumn!] ?? 0) + 1; + return next; + });
94-101: Prefer a stable, delimiter-free boardIdSpaces in ids can be awkward. Use a hyphenated id.
- boardId={clsx('deals-board', pipelineId)} + boardId={`deals-board-${pipelineId ?? 'all'}`}
32-43: Reset board state when pipeline changesAtoms persist across pipelines; clear them to avoid stale data and memory growth.
export const DealsBoard = () => { - const allDealsMap = useAtomValue(allDealsMapState); + const allDealsMap = useAtomValue(allDealsMapState); + const setAllDealsMap = useSetAtom(allDealsMapState); @@ const [pipelineId] = useQueryState<string>('pipelineId'); @@ const [deals, setDeals] = useAtom(fetchedDealsState); @@ + useEffect(() => { + setDeals([]); + setAllDealsMap({}); + }, [pipelineId, setDeals, setAllDealsMap]);
208-216: Avoid passing undefined name to Board.CardboardCards items don’t carry name. Either omit or read from allDealsMap.
- <Board.Card - key={deal.id} - id={deal.id} - name={deal.name} - column={column.id} - > + <Board.Card key={deal.id} id={deal.id} column={column.id}>frontend/plugins/sales_ui/src/modules/deals/components/DealsBoardCard.tsx (2)
5-5: Remove commented-out code.The commented-out imports and code should be removed to improve code cleanliness. If these features are planned for future implementation, track them with TODO comments or issues instead.
Apply this diff to remove the commented code:
-// import { dealCountByBoardAtom } from '@/deals/states/dealsTotalCountState'; import { DateSelectDeal } from '@/deals/components/deal-selects/DateSelectDeal'; import { IconCalendarEventFilled } from '@tabler/icons-react'; import { ItemFooter } from '@/deals/cards/components/item/Footer'; -// import { SelectCompany } from 'ui-modules'; import { SelectDealPriority } from '@/deals/components/deal-selects/SelectDealPriority';const setActiveDeal = useSetAtom(dealDetailSheetState); - // const setDealCountByBoard = useSetAtom(dealCountByBoardAtom); - return (variant="card" /> - {/* <SelectCompany - value={field.value} - onValueChange={field.onChange} - /> */} </div>Also applies to: 32-32, 73-76
13-13: Standardize on a single date formatting library.The code uses
date-fnsfor formatting on line 87, butItemFooter(line 89) usesdayjsfor formatting (as seen in the relevant code snippet). Mixing date libraries increases bundle size and can lead to inconsistent behavior across locale/timezone handling.Standardize on either
date-fnsordayjsthroughout the deals module. Ifdayjsis the preferred library, replace thedate-fnsimport and usage:-import { format } from 'date-fns'; +import dayjs from 'dayjs';<Button variant="ghost" size="sm" className="text-muted-foreground px-1 hover:bg-background" > <IconCalendarEventFilled /> - {createdAt && format(new Date(createdAt), 'MMM dd, yyyy')} + {createdAt && dayjs(createdAt).format('MMM DD, YYYY')} </Button>Also applies to: 87-89
frontend/plugins/sales_ui/src/modules/deals/components/deal-selects/DateSelectDeal.tsx (1)
163-165: Use enum constant instead of string literal.Line 164 uses the string literal
'table'instead of the enum constantDateSelectVariant.TABLE. This is inconsistent with the pattern used elsewhere in the file and reduces type safety.Apply this diff to use the enum constant:
const Content = - variant === 'table' ? RecordTableInlineCell.Content : Combobox.Content; + variant === DateSelectVariant.TABLE ? RecordTableInlineCell.Content : Combobox.Content;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (24)
frontend/libs/erxes-ui/src/modules/board/components/Board.tsx(1 hunks)frontend/libs/erxes-ui/src/modules/board/types/boardTypes.ts(1 hunks)frontend/plugins/sales_ui/src/modules/deals/cards/components/detail/SalesItemDetail.tsx(2 hunks)frontend/plugins/sales_ui/src/modules/deals/cards/components/detail/overview/Priority.tsx(2 hunks)frontend/plugins/sales_ui/src/modules/deals/cards/components/item/Footer.tsx(1 hunks)frontend/plugins/sales_ui/src/modules/deals/cards/hooks/useDeals.tsx(6 hunks)frontend/plugins/sales_ui/src/modules/deals/components/AddDealSheet.tsx(1 hunks)frontend/plugins/sales_ui/src/modules/deals/components/DealsBoard.tsx(1 hunks)frontend/plugins/sales_ui/src/modules/deals/components/DealsBoardCard.tsx(1 hunks)frontend/plugins/sales_ui/src/modules/deals/components/deal-selects/DateSelectDeal.tsx(1 hunks)frontend/plugins/sales_ui/src/modules/deals/components/deal-selects/PriorityInline.tsx(1 hunks)frontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectDealPriority.tsx(1 hunks)frontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectOperation.tsx(1 hunks)frontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectPriority.tsx(1 hunks)frontend/plugins/sales_ui/src/modules/deals/graphql/queries/DealsQueries.ts(2 hunks)frontend/plugins/sales_ui/src/modules/deals/graphql/queries/StagesQueries.ts(0 hunks)frontend/plugins/sales_ui/src/modules/deals/pipelines/components/PipelineFormBar.tsx(1 hunks)frontend/plugins/sales_ui/src/modules/deals/stage/components/StageHeader.tsx(3 hunks)frontend/plugins/sales_ui/src/modules/deals/stage/hooks/useStages.tsx(1 hunks)frontend/plugins/sales_ui/src/modules/deals/states/dealCreateSheetState.ts(1 hunks)frontend/plugins/sales_ui/src/modules/deals/states/dealDetailSheetState.ts(1 hunks)frontend/plugins/sales_ui/src/modules/deals/states/dealsTotalCountState.ts(1 hunks)frontend/plugins/sales_ui/src/modules/deals/states/dealsViewState.ts(1 hunks)frontend/plugins/sales_ui/src/pages/SalesIndexPage.tsx(3 hunks)
💤 Files with no reviewable changes (1)
- frontend/plugins/sales_ui/src/modules/deals/graphql/queries/StagesQueries.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Use functional and declarative programming patterns; avoid classes.
Use TypeScript for all code; prefer interfaces over types.
Avoid enums; use maps instead.
Files:
frontend/plugins/sales_ui/src/modules/deals/cards/components/detail/overview/Priority.tsxfrontend/plugins/sales_ui/src/modules/deals/cards/components/item/Footer.tsxfrontend/plugins/sales_ui/src/modules/deals/states/dealDetailSheetState.tsfrontend/plugins/sales_ui/src/modules/deals/states/dealsTotalCountState.tsfrontend/plugins/sales_ui/src/modules/deals/cards/components/detail/SalesItemDetail.tsxfrontend/plugins/sales_ui/src/modules/deals/stage/hooks/useStages.tsxfrontend/libs/erxes-ui/src/modules/board/types/boardTypes.tsfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectPriority.tsxfrontend/plugins/sales_ui/src/modules/deals/components/DealsBoard.tsxfrontend/plugins/sales_ui/src/modules/deals/components/DealsBoardCard.tsxfrontend/plugins/sales_ui/src/modules/deals/stage/components/StageHeader.tsxfrontend/plugins/sales_ui/src/modules/deals/components/AddDealSheet.tsxfrontend/plugins/sales_ui/src/modules/deals/graphql/queries/DealsQueries.tsfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectDealPriority.tsxfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/PriorityInline.tsxfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/DateSelectDeal.tsxfrontend/plugins/sales_ui/src/modules/deals/states/dealCreateSheetState.tsfrontend/plugins/sales_ui/src/modules/deals/pipelines/components/PipelineFormBar.tsxfrontend/plugins/sales_ui/src/modules/deals/cards/hooks/useDeals.tsxfrontend/libs/erxes-ui/src/modules/board/components/Board.tsxfrontend/plugins/sales_ui/src/pages/SalesIndexPage.tsxfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectOperation.tsxfrontend/plugins/sales_ui/src/modules/deals/states/dealsViewState.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx,js,jsx}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Avoid console logs.
Always use absolute paths when importing.
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
**/*.{ts,tsx,js,jsx}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Use absolute path when import.
Files:
frontend/plugins/sales_ui/src/modules/deals/cards/components/detail/overview/Priority.tsxfrontend/plugins/sales_ui/src/modules/deals/cards/components/item/Footer.tsxfrontend/plugins/sales_ui/src/modules/deals/states/dealDetailSheetState.tsfrontend/plugins/sales_ui/src/modules/deals/states/dealsTotalCountState.tsfrontend/plugins/sales_ui/src/modules/deals/cards/components/detail/SalesItemDetail.tsxfrontend/plugins/sales_ui/src/modules/deals/stage/hooks/useStages.tsxfrontend/libs/erxes-ui/src/modules/board/types/boardTypes.tsfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectPriority.tsxfrontend/plugins/sales_ui/src/modules/deals/components/DealsBoard.tsxfrontend/plugins/sales_ui/src/modules/deals/components/DealsBoardCard.tsxfrontend/plugins/sales_ui/src/modules/deals/stage/components/StageHeader.tsxfrontend/plugins/sales_ui/src/modules/deals/components/AddDealSheet.tsxfrontend/plugins/sales_ui/src/modules/deals/graphql/queries/DealsQueries.tsfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectDealPriority.tsxfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/PriorityInline.tsxfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/DateSelectDeal.tsxfrontend/plugins/sales_ui/src/modules/deals/states/dealCreateSheetState.tsfrontend/plugins/sales_ui/src/modules/deals/pipelines/components/PipelineFormBar.tsxfrontend/plugins/sales_ui/src/modules/deals/cards/hooks/useDeals.tsxfrontend/libs/erxes-ui/src/modules/board/components/Board.tsxfrontend/plugins/sales_ui/src/pages/SalesIndexPage.tsxfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectOperation.tsxfrontend/plugins/sales_ui/src/modules/deals/states/dealsViewState.ts
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{tsx,jsx}: Favor named exports for components.
Use declarative JSX.
Use Shadcn UI, Radix, and Tailwind for components and styling.
Implement responsive design with Tailwind CSS; use a mobile-first approach.
**/*.{tsx,jsx}: Structure files: exported component, subcomponents, helpers, static content, types.
Favor named exports for components.
Use declarative JSX.
Use Shadcn UI, Radix, and Tailwind for components and styling.
Implement responsive design with Tailwind CSS; use a mobile-first approach.
Files:
frontend/plugins/sales_ui/src/modules/deals/cards/components/detail/overview/Priority.tsxfrontend/plugins/sales_ui/src/modules/deals/cards/components/item/Footer.tsxfrontend/plugins/sales_ui/src/modules/deals/cards/components/detail/SalesItemDetail.tsxfrontend/plugins/sales_ui/src/modules/deals/stage/hooks/useStages.tsxfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectPriority.tsxfrontend/plugins/sales_ui/src/modules/deals/components/DealsBoard.tsxfrontend/plugins/sales_ui/src/modules/deals/components/DealsBoardCard.tsxfrontend/plugins/sales_ui/src/modules/deals/stage/components/StageHeader.tsxfrontend/plugins/sales_ui/src/modules/deals/components/AddDealSheet.tsxfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectDealPriority.tsxfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/PriorityInline.tsxfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/DateSelectDeal.tsxfrontend/plugins/sales_ui/src/modules/deals/pipelines/components/PipelineFormBar.tsxfrontend/plugins/sales_ui/src/modules/deals/cards/hooks/useDeals.tsxfrontend/libs/erxes-ui/src/modules/board/components/Board.tsxfrontend/plugins/sales_ui/src/pages/SalesIndexPage.tsxfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectOperation.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional components with TypeScript interfaces.
Minimize 'useEffect', and 'setState'.
Wrap client components in Suspense with fallback.
Use dynamic loading for non-critical components.
Limit 'use client': Favor server components and Next.js SSR. Use only for Web API access in small components. Avoid for data fetching or state management.
Files:
frontend/plugins/sales_ui/src/modules/deals/cards/components/detail/overview/Priority.tsxfrontend/plugins/sales_ui/src/modules/deals/cards/components/item/Footer.tsxfrontend/plugins/sales_ui/src/modules/deals/cards/components/detail/SalesItemDetail.tsxfrontend/plugins/sales_ui/src/modules/deals/stage/hooks/useStages.tsxfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectPriority.tsxfrontend/plugins/sales_ui/src/modules/deals/components/DealsBoard.tsxfrontend/plugins/sales_ui/src/modules/deals/components/DealsBoardCard.tsxfrontend/plugins/sales_ui/src/modules/deals/stage/components/StageHeader.tsxfrontend/plugins/sales_ui/src/modules/deals/components/AddDealSheet.tsxfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectDealPriority.tsxfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/PriorityInline.tsxfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/DateSelectDeal.tsxfrontend/plugins/sales_ui/src/modules/deals/pipelines/components/PipelineFormBar.tsxfrontend/plugins/sales_ui/src/modules/deals/cards/hooks/useDeals.tsxfrontend/libs/erxes-ui/src/modules/board/components/Board.tsxfrontend/plugins/sales_ui/src/pages/SalesIndexPage.tsxfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectOperation.tsx
**/*.{webp,tsx,jsx}
📄 CodeRabbit inference engine (.cursorrules)
Optimize images: use WebP format, include size data, implement lazy loading.
Files:
frontend/plugins/sales_ui/src/modules/deals/cards/components/detail/overview/Priority.tsxfrontend/plugins/sales_ui/src/modules/deals/cards/components/item/Footer.tsxfrontend/plugins/sales_ui/src/modules/deals/cards/components/detail/SalesItemDetail.tsxfrontend/plugins/sales_ui/src/modules/deals/stage/hooks/useStages.tsxfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectPriority.tsxfrontend/plugins/sales_ui/src/modules/deals/components/DealsBoard.tsxfrontend/plugins/sales_ui/src/modules/deals/components/DealsBoardCard.tsxfrontend/plugins/sales_ui/src/modules/deals/stage/components/StageHeader.tsxfrontend/plugins/sales_ui/src/modules/deals/components/AddDealSheet.tsxfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectDealPriority.tsxfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/PriorityInline.tsxfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/DateSelectDeal.tsxfrontend/plugins/sales_ui/src/modules/deals/pipelines/components/PipelineFormBar.tsxfrontend/plugins/sales_ui/src/modules/deals/cards/hooks/useDeals.tsxfrontend/libs/erxes-ui/src/modules/board/components/Board.tsxfrontend/plugins/sales_ui/src/pages/SalesIndexPage.tsxfrontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectOperation.tsx
🧬 Code graph analysis (14)
frontend/plugins/sales_ui/src/modules/deals/cards/components/detail/overview/Priority.tsx (1)
frontend/plugins/sales_ui/src/modules/deals/cards/hooks/useDeals.tsx (1)
useDealsEdit(188-227)
frontend/plugins/sales_ui/src/modules/deals/cards/components/detail/SalesItemDetail.tsx (3)
frontend/plugins/sales_ui/src/modules/deals/states/dealDetailSheetState.ts (1)
dealDetailSheetState(3-3)frontend/plugins/sales_ui/src/modules/deals/cards/hooks/useDeals.tsx (1)
useDealDetail(168-186)frontend/libs/erxes-ui/src/components/sheet.tsx (1)
Sheet(173-183)
frontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectPriority.tsx (7)
frontend/plugins/sales_ui/src/modules/deals/components/deal-selects/PriorityInline.tsx (3)
PriorityBadge(76-89)PriorityIcon(6-50)PriorityTitle(54-72)frontend/plugins/sales_ui/src/modules/deals/constants/cards.ts (1)
PROJECT_PRIORITIES_OPTIONS(49-55)frontend/libs/erxes-ui/src/components/command.tsx (1)
Command(223-233)frontend/libs/erxes-ui/src/components/combobox.tsx (1)
Combobox(200-208)frontend/libs/erxes-ui/src/modules/hotkey/components/PopoverScoped.tsx (1)
PopoverScoped(12-71)frontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectOperation.tsx (2)
SelectTriggerOperation(20-87)SelectOperationContent(89-109)frontend/libs/erxes-ui/src/hooks/use-query-state.tsx (1)
useQueryState(6-49)
frontend/plugins/sales_ui/src/modules/deals/components/DealsBoard.tsx (8)
frontend/libs/erxes-ui/src/modules/board/types/boardTypes.ts (2)
BoardItemProps(10-14)BoardColumnProps(16-19)frontend/plugins/sales_ui/src/modules/deals/cards/hooks/useDeals.tsx (2)
useDealsEdit(188-227)useDeals(42-166)frontend/libs/erxes-ui/src/hooks/use-query-state.tsx (1)
useQueryState(6-49)frontend/plugins/sales_ui/src/modules/deals/stage/hooks/useStages.tsx (1)
useStages(19-33)frontend/plugins/sales_ui/src/modules/deals/states/dealsTotalCountState.ts (1)
dealCountByBoardAtom(5-5)frontend/libs/erxes-ui/src/components/skeleton.tsx (2)
Skeleton(3-13)SkeletonArray(15-29)frontend/plugins/sales_ui/src/modules/deals/components/DealsBoardCard.tsx (1)
DealsBoardCard(19-93)frontend/plugins/sales_ui/src/modules/deals/states/dealCreateSheetState.ts (2)
dealCreateSheetState(3-3)dealCreateDefaultValuesState(4-6)
frontend/plugins/sales_ui/src/modules/deals/components/DealsBoardCard.tsx (6)
frontend/plugins/sales_ui/src/modules/deals/components/DealsBoard.tsx (1)
allDealsMapState(30-30)frontend/libs/erxes-ui/src/modules/board/types/boardTypes.ts (1)
BoardCardProps(53-56)frontend/plugins/sales_ui/src/modules/deals/states/dealDetailSheetState.ts (1)
dealDetailSheetState(3-3)frontend/plugins/sales_ui/src/modules/deals/components/deal-selects/DateSelectDeal.tsx (1)
DateSelectDeal(215-220)frontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectDealPriority.tsx (1)
SelectDealPriority(27-54)frontend/plugins/sales_ui/src/modules/deals/cards/components/item/Footer.tsx (1)
ItemFooter(11-27)
frontend/plugins/sales_ui/src/modules/deals/stage/components/StageHeader.tsx (2)
frontend/plugins/sales_ui/src/modules/deals/states/dealCreateSheetState.ts (1)
dealCreateSheetState(3-3)frontend/libs/erxes-ui/src/components/sheet.tsx (1)
Sheet(173-183)
frontend/plugins/sales_ui/src/modules/deals/components/AddDealSheet.tsx (3)
frontend/plugins/sales_ui/src/modules/deals/states/dealCreateSheetState.ts (1)
dealCreateSheetState(3-3)frontend/libs/erxes-ui/src/components/sheet.tsx (1)
Sheet(173-183)frontend/plugins/sales_ui/src/modules/deals/cards/components/AddCardForm.tsx (1)
AddCardForm(8-85)
frontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectDealPriority.tsx (2)
frontend/plugins/sales_ui/src/modules/deals/cards/hooks/useDeals.tsx (1)
useDealsEdit(188-227)frontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectPriority.tsx (1)
SelectPriority(229-233)
frontend/plugins/sales_ui/src/modules/deals/components/deal-selects/PriorityInline.tsx (2)
frontend/plugins/sales_ui/src/modules/deals/constants/cards.ts (1)
PROJECT_PRIORITIES_OPTIONS(49-55)frontend/libs/erxes-ui/src/components/badge.tsx (1)
Badge(34-60)
frontend/plugins/sales_ui/src/modules/deals/components/deal-selects/DateSelectDeal.tsx (6)
frontend/libs/erxes-ui/src/modules/record-table/components/RecordTableCellInline.tsx (1)
RecordTableInlineCell(100-104)frontend/libs/erxes-ui/src/components/popover.tsx (1)
Popover(52-57)frontend/libs/erxes-ui/src/components/combobox.tsx (1)
Combobox(200-208)frontend/plugins/sales_ui/src/modules/deals/cards/hooks/useDeals.tsx (1)
useDealsEdit(188-227)frontend/libs/erxes-ui/src/modules/hotkey/components/PopoverScoped.tsx (1)
PopoverScoped(12-71)frontend/libs/erxes-ui/src/components/form.tsx (1)
Form(166-174)
frontend/plugins/sales_ui/src/modules/deals/pipelines/components/PipelineFormBar.tsx (1)
frontend/plugins/sales_ui/src/modules/deals/boards/hooks/usePipelineForm.tsx (1)
usePipelineForm(6-24)
frontend/plugins/sales_ui/src/modules/deals/cards/hooks/useDeals.tsx (6)
frontend/plugins/sales_ui/src/modules/deals/types/deals.ts (1)
IDeal(73-78)frontend/libs/erxes-ui/src/types/Gql.ts (1)
ICursorListResponse(3-14)frontend/plugins/sales_ui/src/modules/deals/graphql/queries/DealsQueries.ts (1)
GET_DEALS(137-183)frontend/libs/erxes-ui/src/hooks/use-query-state.tsx (1)
useQueryState(6-49)frontend/plugins/sales_ui/src/modules/deals/states/dealDetailSheetState.ts (1)
dealDetailSheetState(3-3)frontend/plugins/sales_ui/src/modules/deals/states/dealCreateSheetState.ts (1)
dealCreateDefaultValuesState(4-6)
frontend/plugins/sales_ui/src/pages/SalesIndexPage.tsx (1)
frontend/plugins/sales_ui/src/modules/deals/components/AddDealSheet.tsx (1)
AddDealSheet(6-29)
frontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectOperation.tsx (6)
frontend/libs/erxes-ui/src/modules/record-table/components/RecordTableCellInline.tsx (1)
RecordTableInlineCell(100-104)frontend/libs/erxes-ui/src/components/popover.tsx (1)
Popover(52-57)frontend/libs/erxes-ui/src/components/badge.tsx (1)
Badge(34-60)frontend/libs/erxes-ui/src/modules/filter/components/Filter.tsx (1)
Filter(493-512)frontend/libs/erxes-ui/src/components/form.tsx (1)
Form(166-174)frontend/libs/erxes-ui/src/components/combobox.tsx (1)
Combobox(200-208)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (6)
frontend/plugins/sales_ui/src/modules/deals/stage/hooks/useStages.tsx (1)
32-32: LGTM! Good defensive practice.The fallback to an empty array ensures
stagesis always defined, preventing potential runtime errors when consumers iterate over the result. This also improves TypeScript type inference.frontend/plugins/sales_ui/src/modules/deals/graphql/queries/DealsQueries.ts (3)
115-115: LGTM! Follows the established pattern.The addition of
stageIdalongside thestageobject is consistent with the existing pattern in this file where both direct ID fields and full objects are provided (e.g.,assignedUserIdson line 105 andassignedUserson lines 98-104). This provides flexibility for downstream components that may only need the ID without traversing the object.
224-236: LGTM! Improves readability.The reformatting with expanded line breaks makes the query arguments more readable without changing the query's functionality.
238-250: LGTM! Improves readability.The reformatting with expanded line breaks makes the query arguments more readable without changing the query's functionality.
frontend/plugins/sales_ui/src/modules/deals/states/dealDetailSheetState.ts (1)
3-3: LGTM — minimal, focused atomAtom shape and default make sense for driving the sheet by dealId.
frontend/plugins/sales_ui/src/modules/deals/states/dealsTotalCountState.ts (1)
3-9: Counts state reads wellPer-board map + derived total is clean and safe with 0-initial reduction.
| export type BoardItemProps = { | ||
| id: string; | ||
| column: string; | ||
| column: string | undefined; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify handling of undefined column values.
The type change correctly acknowledges that column can be undefined at runtime. However, this reveals a potential issue in Board.tsx at line 181 where overItem.column as string is used without checking for undefined. The type assertion could mask a bug where undefined is passed to setDragOverBoardColumnId.
Run the following script to verify all usages handle undefined properly:
🏁 Script executed:
#!/bin/bash
# Description: Find all usages of BoardItemProps column property to verify undefined handling
# Search for direct property access of .column
rg -nP --type=ts --type=tsx -C3 '\.column\b' -g '!**/node_modules/**' -g '!**/dist/**'
# Search for BoardItemProps usage patterns
ast-grep --pattern 'const $_ = $_.$column'Length of output: 27
🏁 Script executed:
#!/bin/bash
# Locate Board.tsx
file=$(fd -e tsx Board.tsx || find . -type f -name 'Board.tsx')
echo "Board.tsx path: $file"
# Show lines 170-190 around line 181 to inspect undefined handling
sed -n '170,190p' "$file"
# Search for all .column accesses in TS/TSX files to verify checks
rg -nP '\.column\b' -C3 -g '*.ts' -g '*.tsx' -g '!**/node_modules/**' -g '!**/dist/**'Length of output: 13158
Use nullish coalescing instead of a type assertion for overItem.column.
Board.tsx:181: replace
setDragOverBoardColumnId(overItem.column as string);with
setDragOverBoardColumnId(overItem.column ?? null);🤖 Prompt for AI Agents
In frontend/libs/erxes-ui/src/modules/board/types/boardTypes.ts around line 12,
the column property is typed as string | undefined which leads to a type
assertion when assigning overItem.column to setDragOverBoardColumnId; change the
column type to allow null (e.g., string | null or string | null | undefined) and
then in Board.tsx replace the type assertion with a nullish coalescing
assignment so you pass overItem.column ?? null to setDragOverBoardColumnId.
| export const PriorityIcon = React.forwardRef< | ||
| SVGSVGElement, | ||
| React.SVGProps<SVGSVGElement> & { priority: number } | ||
| >(({ priority, className, ...props }, ref) => { | ||
| const color = [ | ||
| 'text-muted-foreground', | ||
| 'text-success', | ||
| 'text-info', | ||
| 'text-warning', | ||
| 'text-destructive', | ||
| ][priority]; | ||
|
|
||
| return ( | ||
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| viewBox="0 0 24 24" | ||
| fill="none" | ||
| strokeWidth={3} | ||
| stroke="currentColor" | ||
| strokeLinecap="round" | ||
| strokeLinejoin="round" | ||
| className={cn('size-4', color, className)} | ||
| {...props} | ||
| ref={ref} | ||
| > | ||
| <path stroke="none" d="M0 0h24v24H0z" fill="none" /> | ||
| <path | ||
| d="M6 18l0 -3" | ||
| className={priority > 0 ? 'stroke-current' : 'stroke-scroll'} | ||
| /> | ||
| <path | ||
| d="M10 18l0 -6" | ||
| className={priority > 1 ? 'stroke-current' : 'stroke-scroll'} | ||
| /> | ||
| <path | ||
| d="M14 18l0 -9" | ||
| className={priority > 2 ? 'stroke-current' : 'stroke-scroll'} | ||
| /> | ||
| <path | ||
| d="M18 18l0 -12" | ||
| className={priority > 3 ? 'stroke-current' : 'stroke-scroll'} | ||
| /> | ||
| </svg> | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Add bounds checking for priority array access.
Lines 10-16 access a color array using the priority parameter without bounds checking. If priority is outside the range [0, 4], the access returns undefined, which could cause rendering issues. The same issue exists in PriorityBadge (lines 80-82).
Add bounds checking or clamp the priority value:
export const PriorityIcon = React.forwardRef<
SVGSVGElement,
React.SVGProps<SVGSVGElement> & { priority: number }
>(({ priority, className, ...props }, ref) => {
+ const clampedPriority = Math.max(0, Math.min(4, priority));
const color = [
'text-muted-foreground',
'text-success',
'text-info',
'text-warning',
'text-destructive',
- ][priority];
+ ][clampedPriority];
return (
<svg
xmlns="http://www.w3.org/2000/svg"
viewBox="0 0 24 24"
fill="none"
strokeWidth={3}
stroke="currentColor"
strokeLinecap="round"
strokeLinejoin="round"
- className={cn('size-4', color, className)}
+ className={cn('size-4', color, className)}
{...props}
ref={ref}
>
<path stroke="none" d="M0 0h24v24H0z" fill="none" />
<path
d="M6 18l0 -3"
- className={priority > 0 ? 'stroke-current' : 'stroke-scroll'}
+ className={clampedPriority > 0 ? 'stroke-current' : 'stroke-scroll'}
/>
<path
d="M10 18l0 -6"
- className={priority > 1 ? 'stroke-current' : 'stroke-scroll'}
+ className={clampedPriority > 1 ? 'stroke-current' : 'stroke-scroll'}
/>
<path
d="M14 18l0 -9"
- className={priority > 2 ? 'stroke-current' : 'stroke-scroll'}
+ className={clampedPriority > 2 ? 'stroke-current' : 'stroke-scroll'}
/>
<path
d="M18 18l0 -12"
- className={priority > 3 ? 'stroke-current' : 'stroke-scroll'}
+ className={clampedPriority > 3 ? 'stroke-current' : 'stroke-scroll'}
/>
</svg>
);
});Similarly, update PriorityBadge:
export const PriorityBadge = React.forwardRef<
HTMLDivElement,
React.ComponentProps<typeof Badge> & { priority?: number }
>(({ priority, ...props }, ref) => {
+ const clampedPriority = Math.max(0, Math.min(4, priority ?? 0));
const variant = ['secondary', 'success', 'info', 'warning', 'destructive'][
- priority ?? 0
+ clampedPriority
];
return (
<Badge ref={ref} variant={variant as any} {...props}>
- <PriorityIcon priority={priority ?? 0} />
- <PriorityTitle priority={priority ?? 0} />
+ <PriorityIcon priority={clampedPriority} />
+ <PriorityTitle priority={clampedPriority} />
</Badge>
);
});🤖 Prompt for AI Agents
In
frontend/plugins/sales_ui/src/modules/deals/components/deal-selects/PriorityInline.tsx
around lines 6 to 50, the code indexes the color array with the incoming
priority without bounds checking; clamp the priority to the valid range 0..4
(e.g. Math.max(0, Math.min(4, Number(priority) || 0))) before using it for array
access and for conditional checks, then use the clamped value for className
selection and the priority comparisons in the path elements; also apply the same
clamping fix in PriorityBadge (lines ~80-82) so array access never returns
undefined and provide a safe default class if needed.
| export enum SelectTriggerVariant { | ||
| TABLE = 'table', | ||
| CARD = 'card', | ||
| DETAIL = 'detail', | ||
| FORM = 'form', | ||
| FILTER = 'filter', | ||
| ICON = 'icon', | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Avoid enum duplication across files.
The SelectTriggerVariant enum is duplicated in both SelectOperation.tsx and this file. This violates the DRY principle and can lead to inconsistencies if one is updated without the other.
Import the enum from SelectOperation.tsx instead of duplicating it:
+import { SelectTriggerVariant } from '@/deals/components/deal-selects/SelectOperation';
import { SelectPriority } from '@/deals/components/deal-selects/SelectPriority';
import { useDealsEdit } from '@/deals/cards/hooks/useDeals';
-export enum SelectTriggerVariant {
- TABLE = 'table',
- CARD = 'card',
- DETAIL = 'detail',
- FORM = 'form',
- FILTER = 'filter',
- ICON = 'icon',
-}
-
const PRIORITY_MAP: Record<string, number> = {📝 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 enum SelectTriggerVariant { | |
| TABLE = 'table', | |
| CARD = 'card', | |
| DETAIL = 'detail', | |
| FORM = 'form', | |
| FILTER = 'filter', | |
| ICON = 'icon', | |
| } | |
| import { SelectTriggerVariant } from '@/deals/components/deal-selects/SelectOperation'; | |
| import { SelectPriority } from '@/deals/components/deal-selects/SelectPriority'; | |
| import { useDealsEdit } from '@/deals/cards/hooks/useDeals'; | |
| const PRIORITY_MAP: Record<string, number> = { | |
| // …existing entries… | |
| }; |
🤖 Prompt for AI Agents
In
frontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectDealPriority.tsx
around lines 4 to 11, the SelectTriggerVariant enum is duplicated here but
already exists in SelectOperation.tsx; remove the local enum declaration and
instead import SelectTriggerVariant from SelectOperation.tsx (or move the enum
to a shared module and import it from there) so there’s a single source of
truth; update any export/usage in this file to reference the imported enum and
run type checks to ensure no name collisions.
| function getPriorityString(value: number) { | ||
| return (Object.keys(PRIORITY_MAP) as Array<keyof typeof PRIORITY_MAP>).find( | ||
| (key) => PRIORITY_MAP[key] === value, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Handle undefined return from getPriorityString.
Array.find() can return undefined if no matching key is found, but the return value is used directly in the GraphQL mutation on line 42 without validation. This could result in passing undefined as the priority value.
Add a fallback or validation to handle the undefined case:
function getPriorityString(value: number) {
return (Object.keys(PRIORITY_MAP) as Array<keyof typeof PRIORITY_MAP>).find(
(key) => PRIORITY_MAP[key] === value,
- );
+ ) ?? 'No Priority';
}📝 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.
| function getPriorityString(value: number) { | |
| return (Object.keys(PRIORITY_MAP) as Array<keyof typeof PRIORITY_MAP>).find( | |
| (key) => PRIORITY_MAP[key] === value, | |
| ); | |
| } | |
| function getPriorityString(value: number) { | |
| return (Object.keys(PRIORITY_MAP) as Array<keyof typeof PRIORITY_MAP>).find( | |
| (key) => PRIORITY_MAP[key] === value, | |
| ) ?? 'No Priority'; | |
| } |
🤖 Prompt for AI Agents
In
frontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectDealPriority.tsx
around lines 21-25, getPriorityString currently returns possibly undefined from
Array.find; update it to always return a concrete priority string (e.g., return
the found key or a defined fallback like PRIORITY_DEFAULT or the first key of
PRIORITY_MAP) and also validate before using the result in the GraphQL mutation
on line 42 (if the function still returns undefined for any reason, either
substitute the fallback priority or abort/throw a clear error) so the mutation
never receives undefined.
| export enum SelectTriggerVariant { | ||
| TABLE = 'table', | ||
| CARD = 'card', | ||
| DETAIL = 'detail', | ||
| FORM = 'form', | ||
| FILTER = 'filter', | ||
| ICON = 'icon', | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the DETAIL variant is intentionally unhandled.
The DETAIL variant is defined in the enum but has no explicit handler in SelectTriggerOperation (lines 20-87), causing it to fall through to the default Combobox.TriggerBase case. If this is intentional, the code is correct; otherwise, add a handler for the DETAIL variant.
Run the following script to check if DETAIL variant is used elsewhere in the codebase:
🏁 Script executed:
#!/bin/bash
# Description: Search for usages of DETAIL variant to determine if a handler is needed.
# Search for DETAIL variant usage
rg -nP 'SelectTriggerVariant\.DETAIL|variant.*=.*["\']detail["\']' --type=tsx --type=tsLength of output: 68
🏁 Script executed:
#!/bin/bash
echo "Searching for SelectTriggerVariant.DETAIL usage..."
rg -nP "SelectTriggerVariant\\.DETAIL" --type=tsx --type=ts || echo "No direct enum usage found"
echo "Searching for literal 'detail' passed as variant prop..."
rg -nP "\\bvariant\\b.*=\\s*['\\\"]detail['\\\"]" --type=tsx --type=ts || echo "No literal 'detail' variant usage found"Length of output: 230
🏁 Script executed:
#!/bin/bash
echo "Searching for direct enum usage across .ts and .tsx files..."
rg -n "SelectTriggerVariant\\.DETAIL" --glob "*.ts" --glob "*.tsx" || echo "No direct enum usage found"
echo "Searching for literal 'detail' passed as variant prop across .ts and .tsx files..."
rg -n "variant\s*=\s*['\"]detail['\"]" --glob "*.ts" --glob "*.tsx" || echo "No literal 'detail' variant usage found"Length of output: 1494
Handle DETAIL variant in SelectTriggerOperation
SelectTriggerVariant.DETAIL currently falls through to the default Combobox.TriggerBase branch; add an explicit case for DETAIL to render its intended trigger styling (it’s used in multiple detail-variant selects across the app).
🤖 Prompt for AI Agents
In
frontend/plugins/sales_ui/src/modules/deals/components/deal-selects/SelectOperation.tsx
around lines 11-18, the SelectTriggerOperation switch lacks an explicit case for
SelectTriggerVariant.DETAIL so DETAIL falls through to the default
Combobox.TriggerBase; add a new case SelectTriggerVariant.DETAIL that returns
the Detail-specific trigger element (match the styling/props used elsewhere for
detail-variant selects), import or reuse the appropriate styled trigger
component or CSS class, and ensure props (children, onClick, className, aria
attributes) are forwarded the same way as other cases so the detail variant
renders correctly across the app.
| import { dealCountByBoardAtom } from '@/deals/states/dealsTotalCountState'; | ||
| import { useEffect } from 'react'; | ||
| import { useInView } from 'react-intersection-observer'; | ||
| import { useSearchParams } from 'react-router'; |
There was a problem hiding this comment.
Wrong import source for useSearchParams
Use react-router-dom in browser apps to avoid runtime issues.
-import { useSearchParams } from 'react-router';
+import { useSearchParams } from 'react-router-dom';📝 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 { useSearchParams } from 'react-router'; | |
| import { useSearchParams } from 'react-router-dom'; |
🤖 Prompt for AI Agents
In frontend/plugins/sales_ui/src/modules/deals/components/DealsBoard.tsx around
line 26, the hook useSearchParams is imported from 'react-router' which is
incorrect for browser apps; change the import to come from 'react-router-dom'
(i.e., replace the import source with 'react-router-dom') so the
browser-compatible implementation is used and runtime issues are avoided.
| editDeals({ | ||
| variables: { | ||
| _id: activeItem?._id, | ||
| stageId: overColumn, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
Drag move may mutate wrong deal (_id overridden by hook)
useDealsEdit merges options.variables with _id from dealDetailSheetState, overriding the _id you pass here. Dragging can update the selected detail deal instead of the dragged one.
Two fixes (pick one):
- Preferred: make useDealsEdit respect a provided _id.
Patch in useDealsEdit (outside this file):
- variables: {
- ...options?.variables,
- _id,
- },
+ variables: {
+ ...options?.variables,
+ _id: options?.variables?._id ?? _id,
+ },- Or: call the mutation directly here instead of the hook (bypassing the atom-bound _id).
// locally:
// const [editDeals] = useMutation(EDIT_DEALS);
editDeals({ variables: { _id: activeItem!._id, stageId: overColumn } });Run a quick search to confirm hook behavior:
🏁 Script executed:
#!/bin/bash
rg -n -C2 "useDealsEdit" frontend/plugins/sales_ui/src/modules/deals/cards/hooks/useDeals.tsxLength of output: 143
🏁 Script executed:
#!/bin/bash
FILE="frontend/plugins/sales_ui/src/modules/deals/cards/hooks/useDeals.tsx"
rg -nC5 "export function useDealsEdit" "$FILE"
sed -n '185,230p' "$FILE"Length of output: 1378
Fix useDealsEdit to respect passed _id
In frontend/plugins/sales_ui/src/modules/deals/cards/hooks/useDeals.tsx, the hook merges:
variables: {
- ...options?.variables,
- _id,
+ ...options?.variables,
+ _id: options?.variables?._id ?? _id,
},which preserves a provided _id instead of always using the atom.
Alternatively, in DealsBoard.tsx call the mutation directly via useMutation(EDIT_DEALS) to pass activeItem._id and bypass the atom override.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/plugins/sales_ui/src/modules/deals/components/DealsBoard.tsx around
lines 68-73, the call to editDeals({ variables: { _id: activeItem?._id, stageId:
overColumn } }) is being overridden by the atom inside useDealsEdit; update the
hook (frontend/plugins/sales_ui/src/modules/deals/cards/hooks/useDeals.tsx) so
that when variables include an explicit _id it is used instead of always
merging/overwriting with the atom value, or alternatively replace this call with
a direct useMutation(EDIT_DEALS) in DealsBoard and pass activeItem._id to bypass
the atom; ensure the final variables object uses the passed _id and not the
atom, and add a unit test or manual check to confirm the provided id is
respected.
| useEffect(() => { | ||
| if (totalCount) { | ||
| setDealCountByBoard((prev) => ({ | ||
| ...prev, | ||
| [column.id]: totalCount || 0, | ||
| })); | ||
| } | ||
| }, [totalCount, setDealCountByBoard, column.id]); |
There was a problem hiding this comment.
Update per-board counts even when zero
The truthy check prevents writing 0, leaving stale counts. Always set the value.
- useEffect(() => {
- if (totalCount) {
- setDealCountByBoard((prev) => ({
- ...prev,
- [column.id]: totalCount || 0,
- }));
- }
- }, [totalCount, setDealCountByBoard, column.id]);
+ useEffect(() => {
+ setDealCountByBoard((prev) => ({
+ ...prev,
+ [column.id]: totalCount ?? 0,
+ }));
+ }, [totalCount, setDealCountByBoard, column.id]);🤖 Prompt for AI Agents
In frontend/plugins/sales_ui/src/modules/deals/components/DealsBoard.tsx around
lines 177 to 184, the current truthy check skips writing zero counts which
leaves stale values; remove the if (totalCount) guard and always call
setDealCountByBoard so zeros are recorded, e.g. invoke setDealCountByBoard(prev
=> ({ ...prev, [column.id]: totalCount ?? 0 })) unconditionally (keep
dependencies the same).
| import { dealCreateSheetState } from '@/deals/states/dealCreateSheetState'; | ||
| import { useAtom } from 'jotai'; | ||
| import { useDealsArchive } from '@/deals/cards/hooks/useDeals'; |
There was a problem hiding this comment.
Fix duplicate sheet + restore stageId sync (critical)
This component renders a tied to the same atom used by the global , so opening here will open two modals simultaneously. Also, replacing onOpenChange with setOpen removed stageId sync on open. Use a simple trigger that sets defaults and opens the global sheet.
Apply this refactor to remove the local Sheet and trigger the global one:
@@
-import { dealCreateSheetState } from '@/deals/states/dealCreateSheetState';
-import { useAtom } from 'jotai';
+import { dealCreateDefaultValuesState, dealCreateSheetState } from '@/deals/states/dealCreateSheetState';
+import { useAtom, useSetAtom } from 'jotai';
@@
- const [open, setOpen] = useAtom(dealCreateSheetState);
+ const setOpenCreateDeal = useSetAtom(dealCreateSheetState);
+ const setDefaultValues = useSetAtom(dealCreateDefaultValuesState);
@@
- const onSheetChange = () => {
- setOpen(!open);
- setStageId(!open ? stage._id : null);
- };
+ // Removed in favor of global sheet trigger
@@
- <Sheet open={open} onOpenChange={setOpen}>
- <Sheet.Trigger asChild>
- <Button variant="ghost" size="icon" className="size-6 relative">
- <IconPlus />
- </Button>
- </Sheet.Trigger>
- <Sheet.View
- className="sm:max-w-lg p-0"
- onEscapeKeyDown={(e) => {
- e.preventDefault();
- }}
- >
- <AddCardForm onCloseSheet={onSheetChange} />
- </Sheet.View>
- </Sheet>
+ <Button
+ variant="ghost"
+ size="icon"
+ className="size-6 relative"
+ onClick={() => {
+ setDefaultValues({ stageId: stage._id });
+ setOpenCreateDeal(true);
+ setStageId(stage._id);
+ }}
+ >
+ <IconPlus />
+ </Button>If you must keep a local , at least restore query-param sync:
-<Sheet open={open} onOpenChange={setOpen}>
+<Sheet
+ open={open}
+ onOpenChange={(next) => {
+ setOpen(next);
+ setStageId(next ? stage._id : null);
+ }}
+>Also applies to: 27-27, 38-41, 165-179
🤖 Prompt for AI Agents
In frontend/plugins/sales_ui/src/modules/deals/stage/components/StageHeader.tsx
around lines 17-19 (and also adjust at 27, 38-41, 165-179), remove the local
<Sheet> usage that opens the same dealCreateSheetState atom (which causes
duplicate modals) and replace it with a simple trigger handler that sets the
dealCreateSheetState defaults (including stageId) and then opens the global
AddDealSheet by setting the atom's open flag to true; ensure you use the atom
setter in a single handler (e.g., set defaults then set open) instead of
replacing onOpenChange, and if you keep any local Sheet restore
query-param/stageId sync by using an onOpenChange callback that updates the
URL/query param with the selected stageId when opening.
| import { lazy } from 'react'; | ||
|
|
||
| const DealBoard = lazy(() => | ||
| import('@/deals/components/DealsBoard').then((mod) => ({ | ||
| default: mod.DealsBoard, | ||
| })), | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Wrap lazy DealBoard with Suspense and a fallback to avoid runtime suspend error
React.lazy components must be rendered inside . Add Suspense import and wrap with a small fallback. As per coding guidelines.
-import { lazy } from 'react';
+import { lazy, Suspense } from 'react';
@@
- <DealBoard />
+ <Suspense
+ fallback={
+ <div className="p-5 text-muted-foreground">Loading board…</div>
+ }
+ >
+ <DealBoard />
+ </Suspense>Also applies to: 62-63
🤖 Prompt for AI Agents
In frontend/plugins/sales_ui/src/pages/SalesIndexPage.tsx around lines 16 to 22
(and also apply the same change for lines 62-63), the lazily loaded DealBoard is
used without being wrapped in React.Suspense; import Suspense from React and
wrap the <DealBoard /> usages with <Suspense fallback={/* small spinner or
placeholder e.g. <div>Loading...</div> or null */}>...</Suspense> so the lazy
component has a fallback while loading and avoids runtime suspend errors.
Summary by Sourcery
Add kanban-style deals board with drag & drop and infinite scroll, introduce modal sheets for creating and viewing deals, and enable inline editing of priority and dates while refactoring state management to Jotai atoms and improving Apollo hooks.
New Features:
Enhancements:
Chores:
Important
Enhances sales UI with new deal management components, improved state management using Jotai, and updated GraphQL queries.
DealsBoardandDealsBoardCardcomponents for managing deals in a board view.AddDealSheetcomponent for creating new deals.SalesItemDetailto usedealDetailSheetStatefor managing active deal state.DealsBoardusing@dnd-kit/core.SelectDealPriorityandPriorityInlinecomponents.dealCreateSheetState,dealDetailSheetState,dealCountByBoardAtom, anddealsViewAtom.useAtomanduseSetAtomfor state updates in components.GET_DEALSandGET_DEAL_DETAILqueries to include new fields and parameters.useDeals,useDealsEdit,useStages, and others.DateSelectDealandSelectPrioritycomponents for date and priority selection.PipelineFormBarandStageHeaderto integrate new deal management features.This description was created by
for 3f95ea8. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes