Enhance customer selection and advisor display in UI - #6
Merged
Conversation
- Updated `RoleSelect.tsx` to include customer names in the selection process, improving user experience by displaying human-readable labels. - Modified `Topbar.tsx` to show customer names alongside IDs for better clarity. - Enhanced `CustomerContext.tsx` to store and manage customer names. - Improved API interactions in `api.ts` to support new customer note functionalities. - Various UI refinements and bug fixes across components to enhance overall usability.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @tsimsekburgan, your pull request is larger than the review limit of 150000 diff characters
📝 WalkthroughWalkthroughThis PR integrates Matrix-based chat into advisor and customer video calls, adds a customer notes feature, implements human-readable display names across the app, normalizes Matrix localparts, and refactors RoleSelect to dynamically fetch advisors from APIs. Changes span context, shared components, admin pages, and both advisor and customer workflows. ChangesMatrix Chat Integration with Display Name Resolution
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the Wealth App by improving user identity visibility and communication capabilities. Key additions include a new CustomerNotesModal for managing advisor notes and a VideoCallMatrixChat component for messaging during video calls. The application now resolves human-readable names for customers and advisors across the dashboard, chat management, and administrative pages, replacing raw IDs. Additionally, the Matrix synchronization logic was improved with better throttling and error handling. Feedback was provided regarding the duplication of advisor role labels in src/lib/constants.ts, suggesting consolidation to improve maintainability.
Comment on lines
+69
to
73
| /** Long Turkish labels for advisor role codes (PY = portföy yöneticisi, YD = yatırım danışmanı). */ | ||
| export const ADVISOR_ROLE_LABELS: Record<string, string> = { | ||
| PY: 'Portföy Yöneticisi', | ||
| YD: 'Yatırım Danışmanı', | ||
| }; |
There was a problem hiding this comment.
The ADVISOR_ROLE_LABELS constant appears to be a duplicate of the advisor role entries recently added to STATE_LABELS. To improve maintainability and avoid having two sources of truth, consider removing this duplication. You could either use STATE_LABELS for all advisor role labels or define one constant in terms of the other.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (8)
src/pages/advisor/Absence.tsx (2)
1166-1176: ⚡ Quick winSame validation concern applies to per-row Matrix ID inputs.
Similar to the bulk assignment field, these per-row inputs accept free-text Matrix IDs without client-side validation. Consider adding:
- Placeholder example (already present ✓)
- Optional pattern validation
- Consistent help text across the UI
The monospace font styling is good for technical identifiers.
Also applies to: 1417-1430
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/advisor/Absence.tsx` around lines 1166 - 1176, The per-row Matrix ID text input that reads/writes permanentAssignments via setPermanentAssignments currently accepts free-form text; add the same client-side validation used for the bulk Matrix ID field by applying a pattern/regex (e.g., the Matrix ID format) to the input, add title/aria-invalid and a small help/error text element next to the input to show validation feedback, and reuse the existing validation helper (or central regex) so behavior is consistent with the bulk assignment field; ensure the input for permanentAssignments[k] triggers validation onChange/onBlur and prevents invalid values from being saved to state.
1103-1127: ⚡ Quick winConsider adding validation or help text for Matrix ID input.
The change from dropdown to free-text input increases flexibility but removes client-side validation. Users can now enter arbitrary text, which could lead to API errors if the Matrix ID is malformed. Consider adding:
- Pattern validation (e.g., regex for valid Matrix ID format)
- Example help text below the input
- Client-side format checking before enabling the apply button
The API will reject invalid IDs, but client-side feedback would improve UX.
💡 Example validation
<input type="text" className="form-input" placeholder="Hedef danışman (Matrix ID veya sicil)" value={bulkAssignAdvisor} onChange={(e) => setBulkAssignAdvisor(e.target.value)} style={{ minWidth: 220 }} /> +<span className="text-muted text-xs mt-1"> + Örnek: morph-touch.portfolio-manager.pm-002 +</span>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/advisor/Absence.tsx` around lines 1103 - 1127, Add client-side validation and user guidance for the free-text Matrix ID input: validate the bulkAssignAdvisor value (use the bulkAssignAdvisor state and setBulkAssignAdvisor handler) against a Matrix ID pattern (e.g., regex for `@user`:domain) before enabling the "Seçilen satırlara uygula" button (in addition to existing checks on selectedRoomKeys), show inline help/example text under the input when empty or invalid, and prevent/apply button clicks from updating setPermanentAssignments unless the value passes the format check; surface validation feedback (error or example) to the user so malformed IDs are caught before the API call.src/components/VideoCallMatrixChat.tsx (2)
301-314: 💤 Low valueSync-loop
setTimeoutis not cleared on unmount.When the effect tears down,
syncAbortedRef.current = trueprevents the next loop iteration from doing work, but the already-scheduledsetTimeout(runSyncLoop, delay)(line 307) keeps a pending timer alive until it fires. Capture the handle and clear it in the cleanup so unmount is fully deterministic (and React Strict-Mode double-invocation doesn't pile up timers).♻️ Proposed change
- runSyncLoop(); - return () => { - syncAbortedRef.current = true; - }; + let pendingTimer: ReturnType<typeof setTimeout> | null = null; + const schedule = (fn: () => void, delay: number) => { + pendingTimer = setTimeout(fn, delay); + }; + // replace `setTimeout(runSyncLoop, delay)` with `schedule(runSyncLoop, delay)` + runSyncLoop(); + return () => { + syncAbortedRef.current = true; + if (pendingTimer) clearTimeout(pendingTimer); + };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/VideoCallMatrixChat.tsx` around lines 301 - 314, The scheduled setTimeout inside the runSyncLoop is left uncleared on unmount; capture the timer handle (e.g., store returned id in a ref like syncTimerRef) whenever you call setTimeout(runSyncLoop, delay), clear any existing timer before scheduling a new one, and call clearTimeout(syncTimerRef.current) in the effect cleanup in addition to setting syncAbortedRef.current = true so no timers remain after unmount; locate runSyncLoop, the setTimeout call, and syncAbortedRef to add the ref and clearTimeout logic.
73-133: ⚡ Quick winConsolidate
mergeCustomerandmergeAdvisorto remove duplication.The two functions differ only in the sender predicate. Parameterize on the predicate to keep merge/optimistic-reconcile logic in one place; this also avoids drift if one branch is later patched without the other (the same pattern already exists for
isCustomerSender/isAdvisorSenderdivergence).♻️ Proposed refactor
-function mergeCustomer( - prev: ChatMessage[], - newMsgs: Array<{ eventId?: string; sender?: string; body?: string; timestamp?: string; msgtype?: string }>, - customerId: string, -): ChatMessage[] { - const existingIds = new Set(prev.map((m) => m.eventId).filter(Boolean)); - const toAdd = newMsgs - .filter((m) => m.eventId && !existingIds.has(m.eventId)) - .map((m) => ({ - ...m, - isMine: isCustomerSender(m.sender, customerId), - read: false, - })); - if (toAdd.length === 0) return prev; - const fromUs = toAdd.filter((m) => isCustomerSender(m.sender, customerId)); - const withoutOptimistic = - fromUs.length > 0 - ? prev.filter((m) => { - if (m.eventId?.startsWith(PENDING_PREFIX) && m.isMine) { - return !fromUs.some((n) => n.body === m.body); - } - return true; - }) - : prev; - return [...withoutOptimistic, ...toAdd].sort((a, b) => { - const ta = Number(a.timestamp) || 0; - const tb = Number(b.timestamp) || 0; - return ta - tb; - }); -} - -function mergeAdvisor( - prev: ChatMessage[], - newMsgs: Array<{ eventId?: string; sender?: string; body?: string; timestamp?: string; msgtype?: string }>, - advisorId: string, -): ChatMessage[] { - // ...identical body with isAdvisorSender -} +function mergeMessages( + prev: ChatMessage[], + newMsgs: Array<{ eventId?: string; sender?: string; body?: string; timestamp?: string; msgtype?: string }>, + isMineFn: (sender: string | undefined) => boolean, +): ChatMessage[] { + const existingIds = new Set(prev.map((m) => m.eventId).filter(Boolean)); + const toAdd = newMsgs + .filter((m) => m.eventId && !existingIds.has(m.eventId)) + .map((m) => ({ ...m, isMine: isMineFn(m.sender), read: false })); + if (toAdd.length === 0) return prev; + const fromUs = toAdd.filter((m) => isMineFn(m.sender)); + const withoutOptimistic = + fromUs.length > 0 + ? prev.filter((m) => + m.eventId?.startsWith(PENDING_PREFIX) && m.isMine + ? !fromUs.some((n) => n.body === m.body) + : true, + ) + : prev; + return [...withoutOptimistic, ...toAdd].sort( + (a, b) => (Number(a.timestamp) || 0) - (Number(b.timestamp) || 0), + ); +}Call sites become
mergeMessages(prev, roomEvents, (s) => isCustomerSender(s, customerId))/isAdvisorSender(s, advisorId).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/VideoCallMatrixChat.tsx` around lines 73 - 133, The two nearly identical functions mergeCustomer and mergeAdvisor should be consolidated into a single mergeMessages utility that accepts the same prev: ChatMessage[], newMsgs array and a sender predicate (e.g., isSender: (s?: string)=>boolean) so you can call mergeMessages(prev, roomEvents, s => isCustomerSender(s, customerId)) or s => isAdvisorSender(s, advisorId); inside mergeMessages use the predicate to set isMine, to compute fromUs, and to perform the optimistic-message filter that checks PENDING_PREFIX and m.isMine, then return the combined array sorted by Number(timestamp) as the existing functions do—replace both mergeCustomer and mergeAdvisor usages with calls to mergeMessages and ensure PENDING_PREFIX, timestamp sort, and read=false behavior are preserved.src/pages/advisor/Appointments.tsx (1)
218-244: 💤 Low valuePolling
setTimeoutis not cancelled on cleanup.
setTimeout(poll, 3000)(line 238) keeps scheduling new timers as long as the modal is open, but the cleanup only clears the initialtimer(line 243). After the effect re-runs (e.g.,videoCallModalchanges) or the component unmounts, the dangling timer still fires; thecancelledflag prevents stale state writes, but it leaves a pending timer that re-invokesgetInstance. Track the latest handle so cleanup is fully deterministic.♻️ Proposed change
- const poll = async () => { + let pendingTimer: ReturnType<typeof setTimeout> | null = null; + const poll = async () => { try { const res = await getInstance('rezervation', id); if (cancelled) return; /* ... */ } catch { /* retry */ } - if (!cancelled) setTimeout(poll, 3000); + if (!cancelled) pendingTimer = setTimeout(poll, 3000); }; const timer = setTimeout(poll, 2000); return () => { cancelled = true; clearTimeout(timer); + if (pendingTimer) clearTimeout(pendingTimer); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/advisor/Appointments.tsx` around lines 218 - 244, The polling schedules recurring timers with setTimeout inside the poll function but only clears the initial timer variable on cleanup, leaving later timers running; modify the implementation to track the latest timer handle (e.g., replace the constant timer with a mutable timeoutHandle captured by poll), assign timeoutHandle = setTimeout(poll, 3000) whenever scheduling the next poll, and in the cleanup return function clearTimeout(timeoutHandle) and set cancelled = true so every scheduled timer is cleared deterministically; update references to timer/cancelled in the poll closure accordingly (functions: poll, setTimeout usage, clearTimeout in the return cleanup).src/pages/advisor/ChatManagement.tsx (1)
815-820: ⚡ Quick winUse Turkish locale-aware lowercasing for search.
The rest of this PR consistently uses
toLocaleLowerCase('tr')(e.g.,sicilEquals,StaffManagementper PR summary). PlaintoLowerCase()here mishandlesİ/Ifor Turkish names, so a search foribrahimwon't match a customer stored asİBRAHİM. Cheap consistency win.♻️ Proposed change
- const haystack = `${customerDisplayName(r.attributes?.user)} ${userName(r.attributes?.user)}`.toLowerCase(); - const matchSearch = !search || haystack.includes(search.toLowerCase()); + const haystack = `${customerDisplayName(r.attributes?.user)} ${userName(r.attributes?.user)}`.toLocaleLowerCase('tr'); + const matchSearch = !search || haystack.includes(search.toLocaleLowerCase('tr'));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/advisor/ChatManagement.tsx` around lines 815 - 820, The filter uses plain toLowerCase() causing Turkish I/İ mismatches; update the normalization to use locale-aware lowercasing by calling toLocaleLowerCase('tr') on both the haystack (built from customerDisplayName(r.attributes?.user) and userName(r.attributes?.user)) and on search before matching in the filteredRooms logic (keep references to filteredRooms, haystack, search, customerDisplayName, userName, favorites, rooms).src/pages/customer/Dashboard.tsx (1)
645-653: ⚡ Quick winVariable name
fifteenMincontradicts its value.The constant is set to
30 * 60 * 1000(30 minutes) and the empty-state UI on line 1081 reads "30 dakika içinde başlayacak randevu yok", but the identifier still saysfifteenMin. This is a renaming leftover that will mislead future readers.♻️ Suggested rename
- const fifteenMin = 30 * 60 * 1000; + const thirtyMin = 30 * 60 * 1000; const upcomingSoon = activeReservations.filter((r) => { const start = r.attributes?.startDateTime; const end = r.attributes?.endDateTime; if (!start || !end) return false; const startTs = new Date(start).getTime(); const endTs = new Date(end).getTime(); - return now >= startTs - fifteenMin && now <= endTs; + return now >= startTs - thirtyMin && now <= endTs; });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/customer/Dashboard.tsx` around lines 645 - 653, The constant name fifteenMin is misleading because its value is 30 minutes; rename the identifier (e.g., to thirtyMin or thirtyMinutesMs) and update its usage in the upcomingSoon filter so the variable name matches its value; change the declaration (currently const fifteenMin = 30 * 60 * 1000) and all references in the upcomingSoon computation that uses fifteenMin, leaving activeReservations and upcomingSoon logic unchanged.src/pages/customer/Chat.tsx (1)
277-348: 💤 Low valueResolution effect refetches and runs Promise.all on every
advisorNamesupdate.Two related observations:
- When a room's
advisorTypeis missing, the code pushes two targets for the same advisor key (lines 290-293), so each unknown-type advisor triggers two parallelgetInstance+ potentiallistInstancescalls. With several such rooms this multiplies API load. Since both lookups would resolve to the same display name, you can race them and accept the first non-empty result.advisorNamesis in the deps array (line 348), and the effect callssetAdvisorNames. While themissing.length === 0guard prevents unbounded loops, every successful partial resolution re-runs the effect, rebuilds the target list, and fires anotherPromise.allfor any keys that previously failed. For consistency and idempotence, consider tracking previously-attempted keys (e.g., in a ref) so failed lookups don't get retried on everyadvisorNamesupdate unlessroomsactually changes.Not a correctness issue—just unnecessary work that can be tightened up.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/customer/Chat.tsx` around lines 277 - 348, The effect rebuilds targets with duplicate entries for the same advisor key and re-runs on every advisorNames change causing repeated API calls; fix by deduplicating targets by advisor key (or store for each key the list of candidate workflows) so you don't push the same key twice in the loop that constructs targets, then change the effect dependencies to only depend on rooms (remove advisorNames) and add a ref like attemptedKeysRef to record keys that you already tried so you skip retries for previously-failed keys unless rooms changes; also change the parallel lookup logic (fetchByKey/Promise.all) to accept the first non-empty result when multiple workflows exist for a key (race or iterate candidate workflows until one returns a name) and keep the existing cancelled guard and setAdvisorNames merge behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/VideoCallMatrixChat.tsx`:
- Around line 59-63: isCustomerSender currently does a loose substring match
which can mis-classify senders; change isCustomerSender to mirror
isAdvisorSender by extracting the Matrix localpart (strip leading '@' and split
at ':'), then compare the localpart strictly to customerId, also handling an
optional leading 'u' prefix exactly as isAdvisorSender does. Update
isCustomerSender so its return logic uses the strict localpart equality (and the
explicit `@${customerId}:localhost` equality fallback if you want parity),
ensuring mergeCustomer and renderSenderName behavior is consistent with
isAdvisorSender.
In `@src/pages/advisor/Appointments.tsx`:
- Around line 191-245: The useEffect watching videoCallModal?.status and other
subfields triggers a lint warning for the missing videoCallModal dependency; fix
by either adding videoCallModal to the dependency array or intentionally
silencing the rule with a clear comment and guard—update the effect in
Appointments.tsx (the useEffect that references videoCallModal,
videoCallModal.reservation, and ADVISOR_ID) so the dependency array includes
videoCallModal (and keep the early-return guard if status !== 'waiting') or add
an eslint-disable-next-line comment above the dependency array with an
explanation referencing videoCallModal and ADVISOR_ID to suppress the warning.
In `@src/pages/advisor/ChatManagement.tsx`:
- Around line 442-524: The effect re-runs lookups for keys that previously
failed because missing = targets.filter((t) => !advisorNames[t.key]) treats
absent and attempted-but-missing the same; update the logic to record attempted
misses (e.g. store a sentinel value like '' or 'NOT_FOUND' in advisorNames or
maintain a separate attempted set/ref) so fetchByKey results that return empty
are marked as attempted and excluded from future missing; modify where
fetchByKey results are applied in the setAdvisorNames updater to write that
sentinel for keys with no name (and/or maintain an attemptedRef used instead of
advisorNames in the dependency array) so failed lookups are not retried on every
advisorNames change.
In `@src/pages/customer/Dashboard.tsx`:
- Around line 459-488: The effect that polls for video-call URLs (useEffect
reading videoCallModal and running async poll/getInstance) needs a bounded retry
and correct dependencies: add a maxAttempts counter (e.g., const MAX_ATTEMPTS =
45 and a local attempts variable incremented each poll) and stop retrying when
attempts >= MAX_ATTEMPTS by calling setVideoCallModal to move status from
'waiting' to a new 'timeout' (or 'failed') state so the UI can show feedback;
also fix the dependency array by referencing the exact reservation identity used
in the effect (e.g., include videoCallModal and/or
videoCallModal?.reservation?.id ?? videoCallModal?.reservation?.key) instead of
only status and reservation.key so changes to r.id retrigger the effect, and
ensure cancelled semantics remain to clear timers on cleanup.
In `@src/pages/RoleSelect.tsx`:
- Around line 98-100: The effect in RoleSelect.tsx references fetchAdvisors but
omits it from the dependency array, which can cause stale closures; update the
useEffect that currently reads "useEffect(() => { if (step === 'advisor')
fetchAdvisors(); }, [step]);" to include fetchAdvisors in the dependency array
or else stabilize fetchAdvisors by wrapping it with useCallback; specifically
either change the dependency array to [step, fetchAdvisors] or memoize the
fetchAdvisors function so its identity is stable and then include it in the
dependencies to satisfy React's exhaustive-deps rule.
---
Nitpick comments:
In `@src/components/VideoCallMatrixChat.tsx`:
- Around line 301-314: The scheduled setTimeout inside the runSyncLoop is left
uncleared on unmount; capture the timer handle (e.g., store returned id in a ref
like syncTimerRef) whenever you call setTimeout(runSyncLoop, delay), clear any
existing timer before scheduling a new one, and call
clearTimeout(syncTimerRef.current) in the effect cleanup in addition to setting
syncAbortedRef.current = true so no timers remain after unmount; locate
runSyncLoop, the setTimeout call, and syncAbortedRef to add the ref and
clearTimeout logic.
- Around line 73-133: The two nearly identical functions mergeCustomer and
mergeAdvisor should be consolidated into a single mergeMessages utility that
accepts the same prev: ChatMessage[], newMsgs array and a sender predicate
(e.g., isSender: (s?: string)=>boolean) so you can call mergeMessages(prev,
roomEvents, s => isCustomerSender(s, customerId)) or s => isAdvisorSender(s,
advisorId); inside mergeMessages use the predicate to set isMine, to compute
fromUs, and to perform the optimistic-message filter that checks PENDING_PREFIX
and m.isMine, then return the combined array sorted by Number(timestamp) as the
existing functions do—replace both mergeCustomer and mergeAdvisor usages with
calls to mergeMessages and ensure PENDING_PREFIX, timestamp sort, and read=false
behavior are preserved.
In `@src/pages/advisor/Absence.tsx`:
- Around line 1166-1176: The per-row Matrix ID text input that reads/writes
permanentAssignments via setPermanentAssignments currently accepts free-form
text; add the same client-side validation used for the bulk Matrix ID field by
applying a pattern/regex (e.g., the Matrix ID format) to the input, add
title/aria-invalid and a small help/error text element next to the input to show
validation feedback, and reuse the existing validation helper (or central regex)
so behavior is consistent with the bulk assignment field; ensure the input for
permanentAssignments[k] triggers validation onChange/onBlur and prevents invalid
values from being saved to state.
- Around line 1103-1127: Add client-side validation and user guidance for the
free-text Matrix ID input: validate the bulkAssignAdvisor value (use the
bulkAssignAdvisor state and setBulkAssignAdvisor handler) against a Matrix ID
pattern (e.g., regex for `@user`:domain) before enabling the "Seçilen satırlara
uygula" button (in addition to existing checks on selectedRoomKeys), show inline
help/example text under the input when empty or invalid, and prevent/apply
button clicks from updating setPermanentAssignments unless the value passes the
format check; surface validation feedback (error or example) to the user so
malformed IDs are caught before the API call.
In `@src/pages/advisor/Appointments.tsx`:
- Around line 218-244: The polling schedules recurring timers with setTimeout
inside the poll function but only clears the initial timer variable on cleanup,
leaving later timers running; modify the implementation to track the latest
timer handle (e.g., replace the constant timer with a mutable timeoutHandle
captured by poll), assign timeoutHandle = setTimeout(poll, 3000) whenever
scheduling the next poll, and in the cleanup return function
clearTimeout(timeoutHandle) and set cancelled = true so every scheduled timer is
cleared deterministically; update references to timer/cancelled in the poll
closure accordingly (functions: poll, setTimeout usage, clearTimeout in the
return cleanup).
In `@src/pages/advisor/ChatManagement.tsx`:
- Around line 815-820: The filter uses plain toLowerCase() causing Turkish I/İ
mismatches; update the normalization to use locale-aware lowercasing by calling
toLocaleLowerCase('tr') on both the haystack (built from
customerDisplayName(r.attributes?.user) and userName(r.attributes?.user)) and on
search before matching in the filteredRooms logic (keep references to
filteredRooms, haystack, search, customerDisplayName, userName, favorites,
rooms).
In `@src/pages/customer/Chat.tsx`:
- Around line 277-348: The effect rebuilds targets with duplicate entries for
the same advisor key and re-runs on every advisorNames change causing repeated
API calls; fix by deduplicating targets by advisor key (or store for each key
the list of candidate workflows) so you don't push the same key twice in the
loop that constructs targets, then change the effect dependencies to only depend
on rooms (remove advisorNames) and add a ref like attemptedKeysRef to record
keys that you already tried so you skip retries for previously-failed keys
unless rooms changes; also change the parallel lookup logic
(fetchByKey/Promise.all) to accept the first non-empty result when multiple
workflows exist for a key (race or iterate candidate workflows until one returns
a name) and keep the existing cancelled guard and setAdvisorNames merge
behavior.
In `@src/pages/customer/Dashboard.tsx`:
- Around line 645-653: The constant name fifteenMin is misleading because its
value is 30 minutes; rename the identifier (e.g., to thirtyMin or
thirtyMinutesMs) and update its usage in the upcomingSoon filter so the variable
name matches its value; change the declaration (currently const fifteenMin = 30
* 60 * 1000) and all references in the upcomingSoon computation that uses
fifteenMin, leaving activeReservations and upcomingSoon logic unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aad91adb-46fa-4436-90b7-db4cc4a61f1c
📒 Files selected for processing (21)
src/App.tsxsrc/components/CustomerNotesModal.tsxsrc/components/VideoCallMatrixChat.tsxsrc/components/layout/Topbar.tsxsrc/contexts/CustomerContext.tsxsrc/data/customers.tssrc/lib/api.tssrc/lib/constants.tssrc/lib/matrixPresence.tssrc/lib/rezervationChatIntegration.tssrc/pages/RoleSelect.tsxsrc/pages/admin/AbsenceManagement.tsxsrc/pages/admin/StaffManagement.tsxsrc/pages/advisor/Absence.tsxsrc/pages/advisor/Appointments.tsxsrc/pages/advisor/ChatManagement.tsxsrc/pages/advisor/VideoCall.tsxsrc/pages/customer/Chat.tsxsrc/pages/customer/Dashboard.tsxsrc/pages/customer/VideoCall.tsxsrc/styles/global.css
💤 Files with no reviewable changes (1)
- src/styles/global.css
Comment on lines
+59
to
+63
| function isCustomerSender(sender: string | undefined, customerId: string): boolean { | ||
| if (!sender || !customerId) return false; | ||
| const customerMatrixId = `@${customerId}:localhost`; | ||
| return sender.includes(customerId) || sender === customerMatrixId; | ||
| } |
There was a problem hiding this comment.
isCustomerSender uses substring match — inconsistent with isAdvisorSender and prone to false positives.
sender.includes(customerId) is a loose substring check; isAdvisorSender (lines 65-71) correctly extracts the localpart and compares it strictly with optional u prefix. For numeric TCKN values this is mostly safe, but the asymmetry means a customer-side substring collision (e.g., a sender whose room/server portion contains the digits) would mis-classify the bubble side. Align the two helpers on the same strict-localpart logic so isMine reconciliation in mergeCustomer (line 87) and renderSenderName (line 200) behave consistently.
🛡️ Proposed fix to mirror the advisor check
function isCustomerSender(sender: string | undefined, customerId: string): boolean {
if (!sender || !customerId) return false;
- const customerMatrixId = `@${customerId}:localhost`;
- return sender.includes(customerId) || sender === customerMatrixId;
+ const localpart = sender.replace(/^@/, '').split(':')[0]?.trim().toLowerCase() ?? '';
+ if (!localpart) return false;
+ const cid = customerId.trim().toLowerCase();
+ return localpart === cid || localpart === `u${cid}`;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/VideoCallMatrixChat.tsx` around lines 59 - 63,
isCustomerSender currently does a loose substring match which can mis-classify
senders; change isCustomerSender to mirror isAdvisorSender by extracting the
Matrix localpart (strip leading '@' and split at ':'), then compare the
localpart strictly to customerId, also handling an optional leading 'u' prefix
exactly as isAdvisorSender does. Update isCustomerSender so its return logic
uses the strict localpart equality (and the explicit `@${customerId}:localhost`
equality fallback if you want parity), ensuring mergeCustomer and
renderSenderName behavior is consistent with isAdvisorSender.
Comment on lines
+191
to
+245
| useEffect(() => { | ||
| if (!videoCallModal || videoCallModal.status !== 'waiting' || !ADVISOR_ID) return; | ||
| const r = videoCallModal.reservation; | ||
| const id = r.id ?? r.key; | ||
| const advRef = r.attributes?.advisor; | ||
| const advisorAttrKey = | ||
| typeof advRef === 'string' | ||
| ? advRef | ||
| : advRef && typeof advRef === 'object' && 'key' in advRef | ||
| ? String((advRef as { key: string }).key) | ||
| : ''; | ||
| let cancelled = false; | ||
|
|
||
| const pickAdvisorUrl = (urls: Record<string, string>[]): string | null => { | ||
| for (const u of urls) { | ||
| if (!u) continue; | ||
| if (ADVISOR_ID in u && u[ADVISOR_ID]) return u[ADVISOR_ID]; | ||
| } | ||
| if (advisorAttrKey) { | ||
| for (const u of urls) { | ||
| if (!u) continue; | ||
| if (advisorAttrKey in u && u[advisorAttrKey]) return u[advisorAttrKey]; | ||
| } | ||
| } | ||
| return null; | ||
| }; | ||
|
|
||
| const poll = async () => { | ||
| try { | ||
| const res = await getInstance('rezervation', id); | ||
| if (cancelled) return; | ||
| const data = res.data as { | ||
| attributes?: { videoCallUrls?: Record<string, string>[] }; | ||
| videoCallUrls?: Record<string, string>[]; | ||
| } | null; | ||
| const urls = data?.attributes?.videoCallUrls ?? data?.videoCallUrls; | ||
| if (urls && Array.isArray(urls) && urls.length > 0) { | ||
| const url = pickAdvisorUrl(urls); | ||
| if (url) { | ||
| window.open(url, '_blank', 'noopener,noreferrer'); | ||
| setVideoCallModal((prev) => (prev ? { ...prev, status: 'ready', videoUrl: url } : null)); | ||
| return; | ||
| } | ||
| } | ||
| } catch { | ||
| /* retry */ | ||
| } | ||
| if (!cancelled) setTimeout(poll, 3000); | ||
| }; | ||
| const timer = setTimeout(poll, 2000); | ||
| return () => { | ||
| cancelled = true; | ||
| clearTimeout(timer); | ||
| }; | ||
| }, [videoCallModal?.status, videoCallModal?.reservation?.key, ADVISOR_ID]); |
There was a problem hiding this comment.
Address the build warning about the missing videoCallModal dependency.
The CI build (Node 20/22) flags this useEffect for a missing 'videoCallModal' dependency. Today the effect happens to be correct because every accessed field (status, reservation, videoUrl) is reached through the listed sub-properties, but the lint rule is a project-wide signal and will keep firing on every build. Either include the full object in the dependency array, or scope the effect to a stable identifier and silence the rule explicitly so reviewers can see the intent.
🛠️ Suggested resolutions
Option A — narrow with explicit disable:
- }, [videoCallModal?.status, videoCallModal?.reservation?.key, ADVISOR_ID]);
+ // The effect intentionally reads videoCallModal only when status === 'waiting';
+ // status/key are the only fields that should retrigger polling.
+ // eslint-disable-next-line react-hooks/exhaustive-deps
+ }, [videoCallModal?.status, videoCallModal?.reservation?.key, ADVISOR_ID]);Option B — include the full state and guard re-entry inside the effect body, similar to the existing status !== 'waiting' early-return.
📝 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
| useEffect(() => { | |
| if (!videoCallModal || videoCallModal.status !== 'waiting' || !ADVISOR_ID) return; | |
| const r = videoCallModal.reservation; | |
| const id = r.id ?? r.key; | |
| const advRef = r.attributes?.advisor; | |
| const advisorAttrKey = | |
| typeof advRef === 'string' | |
| ? advRef | |
| : advRef && typeof advRef === 'object' && 'key' in advRef | |
| ? String((advRef as { key: string }).key) | |
| : ''; | |
| let cancelled = false; | |
| const pickAdvisorUrl = (urls: Record<string, string>[]): string | null => { | |
| for (const u of urls) { | |
| if (!u) continue; | |
| if (ADVISOR_ID in u && u[ADVISOR_ID]) return u[ADVISOR_ID]; | |
| } | |
| if (advisorAttrKey) { | |
| for (const u of urls) { | |
| if (!u) continue; | |
| if (advisorAttrKey in u && u[advisorAttrKey]) return u[advisorAttrKey]; | |
| } | |
| } | |
| return null; | |
| }; | |
| const poll = async () => { | |
| try { | |
| const res = await getInstance('rezervation', id); | |
| if (cancelled) return; | |
| const data = res.data as { | |
| attributes?: { videoCallUrls?: Record<string, string>[] }; | |
| videoCallUrls?: Record<string, string>[]; | |
| } | null; | |
| const urls = data?.attributes?.videoCallUrls ?? data?.videoCallUrls; | |
| if (urls && Array.isArray(urls) && urls.length > 0) { | |
| const url = pickAdvisorUrl(urls); | |
| if (url) { | |
| window.open(url, '_blank', 'noopener,noreferrer'); | |
| setVideoCallModal((prev) => (prev ? { ...prev, status: 'ready', videoUrl: url } : null)); | |
| return; | |
| } | |
| } | |
| } catch { | |
| /* retry */ | |
| } | |
| if (!cancelled) setTimeout(poll, 3000); | |
| }; | |
| const timer = setTimeout(poll, 2000); | |
| return () => { | |
| cancelled = true; | |
| clearTimeout(timer); | |
| }; | |
| }, [videoCallModal?.status, videoCallModal?.reservation?.key, ADVISOR_ID]); | |
| useEffect(() => { | |
| if (!videoCallModal || videoCallModal.status !== 'waiting' || !ADVISOR_ID) return; | |
| const r = videoCallModal.reservation; | |
| const id = r.id ?? r.key; | |
| const advRef = r.attributes?.advisor; | |
| const advisorAttrKey = | |
| typeof advRef === 'string' | |
| ? advRef | |
| : advRef && typeof advRef === 'object' && 'key' in advRef | |
| ? String((advRef as { key: string }).key) | |
| : ''; | |
| let cancelled = false; | |
| const pickAdvisorUrl = (urls: Record<string, string>[]): string | null => { | |
| for (const u of urls) { | |
| if (!u) continue; | |
| if (ADVISOR_ID in u && u[ADVISOR_ID]) return u[ADVISOR_ID]; | |
| } | |
| if (advisorAttrKey) { | |
| for (const u of urls) { | |
| if (!u) continue; | |
| if (advisorAttrKey in u && u[advisorAttrKey]) return u[advisorAttrKey]; | |
| } | |
| } | |
| return null; | |
| }; | |
| const poll = async () => { | |
| try { | |
| const res = await getInstance('rezervation', id); | |
| if (cancelled) return; | |
| const data = res.data as { | |
| attributes?: { videoCallUrls?: Record<string, string>[] }; | |
| videoCallUrls?: Record<string, string>[]; | |
| } | null; | |
| const urls = data?.attributes?.videoCallUrls ?? data?.videoCallUrls; | |
| if (urls && Array.isArray(urls) && urls.length > 0) { | |
| const url = pickAdvisorUrl(urls); | |
| if (url) { | |
| window.open(url, '_blank', 'noopener,noreferrer'); | |
| setVideoCallModal((prev) => (prev ? { ...prev, status: 'ready', videoUrl: url } : null)); | |
| return; | |
| } | |
| } | |
| } catch { | |
| /* retry */ | |
| } | |
| if (!cancelled) setTimeout(poll, 3000); | |
| }; | |
| const timer = setTimeout(poll, 2000); | |
| return () => { | |
| cancelled = true; | |
| clearTimeout(timer); | |
| }; | |
| // The effect intentionally reads videoCallModal only when status === 'waiting'; | |
| // status/key are the only fields that should retrigger polling. | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, [videoCallModal?.status, videoCallModal?.reservation?.key, ADVISOR_ID]); |
🧰 Tools
🪛 GitHub Check: Build (Node 20)
[warning] 245-245:
React Hook useEffect has a missing dependency: 'videoCallModal'. Either include it or remove the dependency array
🪛 GitHub Check: Build (Node 22)
[warning] 245-245:
React Hook useEffect has a missing dependency: 'videoCallModal'. Either include it or remove the dependency array
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/advisor/Appointments.tsx` around lines 191 - 245, The useEffect
watching videoCallModal?.status and other subfields triggers a lint warning for
the missing videoCallModal dependency; fix by either adding videoCallModal to
the dependency array or intentionally silencing the rule with a clear comment
and guard—update the effect in Appointments.tsx (the useEffect that references
videoCallModal, videoCallModal.reservation, and ADVISOR_ID) so the dependency
array includes videoCallModal (and keep the early-return guard if status !==
'waiting') or add an eslint-disable-next-line comment above the dependency array
with an explanation referencing videoCallModal and ADVISOR_ID to suppress the
warning.
Comment on lines
+442
to
+524
| useEffect(() => { | ||
| if (rooms.length === 0) return; | ||
| let cancelled = false; | ||
|
|
||
| type Target = { key: string; workflow: AdvisorWorkflow }; | ||
| const seen = new Set<string>(); | ||
| const targets: Target[] = []; | ||
| const pushTarget = (id: string | undefined, advisorType?: string) => { | ||
| if (!id) return; | ||
| const trimmed = id.trim(); | ||
| if (!trimmed || seen.has(trimmed)) return; | ||
| seen.add(trimmed); | ||
| const wf = workflowFromAdvisorType(advisorType); | ||
| if (wf) targets.push({ key: trimmed, workflow: wf }); | ||
| else { | ||
| targets.push({ key: trimmed, workflow: 'portfolio-manager' }); | ||
| targets.push({ key: trimmed, workflow: 'investment-advisor' }); | ||
| } | ||
| }; | ||
|
|
||
| for (const r of rooms) { | ||
| pushTarget(r.attributes?.advisorId, r.attributes?.advisorType); | ||
| for (const m of r.attributes?.members ?? []) { | ||
| const role = (m.role ?? '').trim(); | ||
| // owner = müşteri (TCKN), advisor/member = sicil → sicil olanları çöz | ||
| if (role === 'owner') continue; | ||
| pushTarget(m.memberId, r.attributes?.advisorType); | ||
| } | ||
| } | ||
| const missing = targets.filter((t) => !advisorNames[t.key]); | ||
| if (missing.length === 0) return; | ||
|
|
||
| const buildName = (attrs: Record<string, unknown> | undefined): string => { | ||
| if (!attrs) return ''; | ||
| const first = String(attrs.firstName ?? '').trim(); | ||
| const last = String(attrs.lastName ?? '').trim(); | ||
| return `${first} ${last}`.trim(); | ||
| }; | ||
|
|
||
| const fetchByKey = async (key: string, workflow: AdvisorWorkflow): Promise<string> => { | ||
| try { | ||
| const res = await getInstance(workflow, key); | ||
| if (res.ok && res.data) { | ||
| const d = res.data as { attributes?: Record<string, unknown> }; | ||
| const name = buildName(d.attributes); | ||
| if (name) return name; | ||
| } | ||
| } catch { | ||
| /* listInstances fallback aşağıda */ | ||
| } | ||
| try { | ||
| const list = await listInstances(workflow, { pageSize: 100 }); | ||
| if (list.ok && list.data) { | ||
| const items = (list.data as { items?: { key?: string; attributes?: Record<string, unknown> }[] }).items ?? []; | ||
| const match = items.find((i) => i.key === key); | ||
| if (match) return buildName(match.attributes); | ||
| } | ||
| } catch { | ||
| /* yut */ | ||
| } | ||
| return ''; | ||
| }; | ||
|
|
||
| (async () => { | ||
| const resolved = await Promise.all( | ||
| missing.map(async (t) => [t.key, await fetchByKey(t.key, t.workflow)] as const), | ||
| ); | ||
| if (cancelled) return; | ||
| const updates = resolved.filter(([, name]) => name.length > 0); | ||
| if (updates.length === 0) return; | ||
| setAdvisorNames((prev) => { | ||
| const next = { ...prev }; | ||
| for (const [k, name] of updates) { | ||
| if (!next[k]) next[k] = name; | ||
| } | ||
| return next; | ||
| }); | ||
| })(); | ||
|
|
||
| return () => { | ||
| cancelled = true; | ||
| }; | ||
| }, [rooms, advisorNames]); |
There was a problem hiding this comment.
Failed name lookups re-run on every successful resolution.
missing = targets.filter((t) => !advisorNames[t.key]) keeps any key that didn't resolve, but the effect depends on advisorNames and only stores entries when name.length > 0 (line 510). Each successful resolution mutates advisorNames, retriggers this effect, and re-issues getInstance + listInstances for every key that previously returned an empty string. For unknown advisorType keys this also doubles the work because both PM and IA workflows are queued. Cache the "attempted" set (or store a sentinel for misses) so a failed lookup isn't retried each time a sibling succeeds.
⚡ Proposed change
+ const attemptedAdvisorKeysRef = useRef<Set<string>>(new Set());
useEffect(() => {
if (rooms.length === 0) return;
let cancelled = false;
/* ... */
- const missing = targets.filter((t) => !advisorNames[t.key]);
+ const missing = targets.filter(
+ (t) => !advisorNames[t.key] && !attemptedAdvisorKeysRef.current.has(`${t.key}|${t.workflow}`),
+ );
if (missing.length === 0) return;
+ for (const t of missing) attemptedAdvisorKeysRef.current.add(`${t.key}|${t.workflow}`);
/* ... */
}, [rooms, advisorNames]);Alternative: drop advisorNames from the dependency array (read via a ref) and only re-resolve when rooms change.
📝 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
| useEffect(() => { | |
| if (rooms.length === 0) return; | |
| let cancelled = false; | |
| type Target = { key: string; workflow: AdvisorWorkflow }; | |
| const seen = new Set<string>(); | |
| const targets: Target[] = []; | |
| const pushTarget = (id: string | undefined, advisorType?: string) => { | |
| if (!id) return; | |
| const trimmed = id.trim(); | |
| if (!trimmed || seen.has(trimmed)) return; | |
| seen.add(trimmed); | |
| const wf = workflowFromAdvisorType(advisorType); | |
| if (wf) targets.push({ key: trimmed, workflow: wf }); | |
| else { | |
| targets.push({ key: trimmed, workflow: 'portfolio-manager' }); | |
| targets.push({ key: trimmed, workflow: 'investment-advisor' }); | |
| } | |
| }; | |
| for (const r of rooms) { | |
| pushTarget(r.attributes?.advisorId, r.attributes?.advisorType); | |
| for (const m of r.attributes?.members ?? []) { | |
| const role = (m.role ?? '').trim(); | |
| // owner = müşteri (TCKN), advisor/member = sicil → sicil olanları çöz | |
| if (role === 'owner') continue; | |
| pushTarget(m.memberId, r.attributes?.advisorType); | |
| } | |
| } | |
| const missing = targets.filter((t) => !advisorNames[t.key]); | |
| if (missing.length === 0) return; | |
| const buildName = (attrs: Record<string, unknown> | undefined): string => { | |
| if (!attrs) return ''; | |
| const first = String(attrs.firstName ?? '').trim(); | |
| const last = String(attrs.lastName ?? '').trim(); | |
| return `${first} ${last}`.trim(); | |
| }; | |
| const fetchByKey = async (key: string, workflow: AdvisorWorkflow): Promise<string> => { | |
| try { | |
| const res = await getInstance(workflow, key); | |
| if (res.ok && res.data) { | |
| const d = res.data as { attributes?: Record<string, unknown> }; | |
| const name = buildName(d.attributes); | |
| if (name) return name; | |
| } | |
| } catch { | |
| /* listInstances fallback aşağıda */ | |
| } | |
| try { | |
| const list = await listInstances(workflow, { pageSize: 100 }); | |
| if (list.ok && list.data) { | |
| const items = (list.data as { items?: { key?: string; attributes?: Record<string, unknown> }[] }).items ?? []; | |
| const match = items.find((i) => i.key === key); | |
| if (match) return buildName(match.attributes); | |
| } | |
| } catch { | |
| /* yut */ | |
| } | |
| return ''; | |
| }; | |
| (async () => { | |
| const resolved = await Promise.all( | |
| missing.map(async (t) => [t.key, await fetchByKey(t.key, t.workflow)] as const), | |
| ); | |
| if (cancelled) return; | |
| const updates = resolved.filter(([, name]) => name.length > 0); | |
| if (updates.length === 0) return; | |
| setAdvisorNames((prev) => { | |
| const next = { ...prev }; | |
| for (const [k, name] of updates) { | |
| if (!next[k]) next[k] = name; | |
| } | |
| return next; | |
| }); | |
| })(); | |
| return () => { | |
| cancelled = true; | |
| }; | |
| }, [rooms, advisorNames]); | |
| const attemptedAdvisorKeysRef = useRef<Set<string>>(new Set()); | |
| useEffect(() => { | |
| if (rooms.length === 0) return; | |
| let cancelled = false; | |
| type Target = { key: string; workflow: AdvisorWorkflow }; | |
| const seen = new Set<string>(); | |
| const targets: Target[] = []; | |
| const pushTarget = (id: string | undefined, advisorType?: string) => { | |
| if (!id) return; | |
| const trimmed = id.trim(); | |
| if (!trimmed || seen.has(trimmed)) return; | |
| seen.add(trimmed); | |
| const wf = workflowFromAdvisorType(advisorType); | |
| if (wf) targets.push({ key: trimmed, workflow: wf }); | |
| else { | |
| targets.push({ key: trimmed, workflow: 'portfolio-manager' }); | |
| targets.push({ key: trimmed, workflow: 'investment-advisor' }); | |
| } | |
| }; | |
| for (const r of rooms) { | |
| pushTarget(r.attributes?.advisorId, r.attributes?.advisorType); | |
| for (const m of r.attributes?.members ?? []) { | |
| const role = (m.role ?? '').trim(); | |
| // owner = müşteri (TCKN), advisor/member = sicil → sicil olanları çöz | |
| if (role === 'owner') continue; | |
| pushTarget(m.memberId, r.attributes?.advisorType); | |
| } | |
| } | |
| const missing = targets.filter( | |
| (t) => !advisorNames[t.key] && !attemptedAdvisorKeysRef.current.has(`${t.key}|${t.workflow}`), | |
| ); | |
| if (missing.length === 0) return; | |
| for (const t of missing) attemptedAdvisorKeysRef.current.add(`${t.key}|${t.workflow}`); | |
| const buildName = (attrs: Record<string, unknown> | undefined): string => { | |
| if (!attrs) return ''; | |
| const first = String(attrs.firstName ?? '').trim(); | |
| const last = String(attrs.lastName ?? '').trim(); | |
| return `${first} ${last}`.trim(); | |
| }; | |
| const fetchByKey = async (key: string, workflow: AdvisorWorkflow): Promise<string> => { | |
| try { | |
| const res = await getInstance(workflow, key); | |
| if (res.ok && res.data) { | |
| const d = res.data as { attributes?: Record<string, unknown> }; | |
| const name = buildName(d.attributes); | |
| if (name) return name; | |
| } | |
| } catch { | |
| /* listInstances fallback aşağıda */ | |
| } | |
| try { | |
| const list = await listInstances(workflow, { pageSize: 100 }); | |
| if (list.ok && list.data) { | |
| const items = (list.data as { items?: { key?: string; attributes?: Record<string, unknown> }[] }).items ?? []; | |
| const match = items.find((i) => i.key === key); | |
| if (match) return buildName(match.attributes); | |
| } | |
| } catch { | |
| /* yut */ | |
| } | |
| return ''; | |
| }; | |
| (async () => { | |
| const resolved = await Promise.all( | |
| missing.map(async (t) => [t.key, await fetchByKey(t.key, t.workflow)] as const), | |
| ); | |
| if (cancelled) return; | |
| const updates = resolved.filter(([, name]) => name.length > 0); | |
| if (updates.length === 0) return; | |
| setAdvisorNames((prev) => { | |
| const next = { ...prev }; | |
| for (const [k, name] of updates) { | |
| if (!next[k]) next[k] = name; | |
| } | |
| return next; | |
| }); | |
| })(); | |
| return () => { | |
| cancelled = true; | |
| }; | |
| }, [rooms, advisorNames]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/advisor/ChatManagement.tsx` around lines 442 - 524, The effect
re-runs lookups for keys that previously failed because missing =
targets.filter((t) => !advisorNames[t.key]) treats absent and
attempted-but-missing the same; update the logic to record attempted misses
(e.g. store a sentinel value like '' or 'NOT_FOUND' in advisorNames or maintain
a separate attempted set/ref) so fetchByKey results that return empty are marked
as attempted and excluded from future missing; modify where fetchByKey results
are applied in the setAdvisorNames updater to write that sentinel for keys with
no name (and/or maintain an attemptedRef used instead of advisorNames in the
dependency array) so failed lookups are not retried on every advisorNames
change.
Comment on lines
+459
to
+488
| useEffect(() => { | ||
| if (!videoCallModal || videoCallModal.status !== 'waiting' || !customerId) return; | ||
| const r = videoCallModal.reservation; | ||
| const id = r.id ?? r.key; | ||
| let cancelled = false; | ||
|
|
||
| const poll = async () => { | ||
| try { | ||
| const res = await getInstance('rezervation', id); | ||
| if (cancelled) return; | ||
| const data = res.data as { attributes?: { videoCallUrls?: Record<string, string>[] }; videoCallUrls?: Record<string, string>[] } | null; | ||
| const urls = data?.attributes?.videoCallUrls ?? data?.videoCallUrls; | ||
| if (urls && Array.isArray(urls) && urls.length > 0) { | ||
| const myEntry = urls.find((u) => u && customerId in u); | ||
| if (myEntry && myEntry[customerId]) { | ||
| setVideoCallModal((prev) => prev ? { ...prev, status: 'ready', videoUrl: myEntry[customerId] } : null); | ||
| return; | ||
| } | ||
| } | ||
| } catch { | ||
| /* retry */ | ||
| } | ||
| if (!cancelled) setTimeout(poll, 3000); | ||
| }; | ||
| const timer = setTimeout(poll, 2000); | ||
| return () => { | ||
| cancelled = true; | ||
| clearTimeout(timer); | ||
| }; | ||
| }, [videoCallModal?.status, videoCallModal?.reservation.key, customerId]); |
There was a problem hiding this comment.
Video-call polling effect: no max attempts and stale-deps warning.
Two related concerns on this polling effect:
- The build is emitting an ESLint warning that
videoCallModalis missing from the dependency array (line 488). The current array tracksvideoCallModal?.statusandvideoCallModal?.reservation.key, but the effect readsvideoCallModal.reservationdirectly and usesr.id ?? r.key. If the reservation object'sidchanges without thekeychanging (re-fetch path), the effect won't pick it up. - Unlike the
confirmReservationpolling above (which hasmaxAttempts = 45), thispollchain has no upper bound. If the advisor never approves and the user leaves the modal open, the effect chainssetTimeout(poll, 3000)indefinitely. Thecancelledflag only stops it when the modal closes or status changes.
Consider bounding the poll attempts and surfacing a timeout state in the modal so the user gets feedback instead of a perpetual "waiting" spinner.
🛡️ Suggested adjustment
useEffect(() => {
if (!videoCallModal || videoCallModal.status !== 'waiting' || !customerId) return;
const r = videoCallModal.reservation;
const id = r.id ?? r.key;
let cancelled = false;
+ let attempts = 0;
+ const maxAttempts = 60; // ~3 minutes at 3s cadence
const poll = async () => {
+ attempts += 1;
try {
const res = await getInstance('rezervation', id);
if (cancelled) return;
const data = res.data as { attributes?: { videoCallUrls?: Record<string, string>[] }; videoCallUrls?: Record<string, string>[] } | null;
const urls = data?.attributes?.videoCallUrls ?? data?.videoCallUrls;
if (urls && Array.isArray(urls) && urls.length > 0) {
const myEntry = urls.find((u) => u && customerId in u);
if (myEntry && myEntry[customerId]) {
setVideoCallModal((prev) => prev ? { ...prev, status: 'ready', videoUrl: myEntry[customerId] } : null);
return;
}
}
} catch {
/* retry */
}
+ if (attempts >= maxAttempts) {
+ toast('Görüşme bağlantısı zaman aşımına uğradı.', 'error');
+ setVideoCallModal(null);
+ return;
+ }
if (!cancelled) setTimeout(poll, 3000);
};🧰 Tools
🪛 GitHub Check: Build (Node 20)
[warning] 488-488:
React Hook useEffect has a missing dependency: 'videoCallModal'. Either include it or remove the dependency array
🪛 GitHub Check: Build (Node 22)
[warning] 488-488:
React Hook useEffect has a missing dependency: 'videoCallModal'. Either include it or remove the dependency array
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/customer/Dashboard.tsx` around lines 459 - 488, The effect that
polls for video-call URLs (useEffect reading videoCallModal and running async
poll/getInstance) needs a bounded retry and correct dependencies: add a
maxAttempts counter (e.g., const MAX_ATTEMPTS = 45 and a local attempts variable
incremented each poll) and stop retrying when attempts >= MAX_ATTEMPTS by
calling setVideoCallModal to move status from 'waiting' to a new 'timeout' (or
'failed') state so the UI can show feedback; also fix the dependency array by
referencing the exact reservation identity used in the effect (e.g., include
videoCallModal and/or videoCallModal?.reservation?.id ??
videoCallModal?.reservation?.key) instead of only status and reservation.key so
changes to r.id retrigger the effect, and ensure cancelled semantics remain to
clear timers on cleanup.
Comment on lines
+98
to
+100
| useEffect(() => { | ||
| if (step === 'advisor') fetchAdvisors(); | ||
| }, [step]); |
There was a problem hiding this comment.
Add fetchAdvisors to the useEffect dependency array.
The effect references fetchAdvisors but doesn't list it in the dependencies. React's exhaustive-deps rule requires all closure references to be declared, and omitting fetchAdvisors could lead to stale closure bugs if the function identity changes.
🔧 Proposed fix
useEffect(() => {
if (step === 'advisor') fetchAdvisors();
-}, [step]);
+}, [step, fetchAdvisors]);📝 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
| useEffect(() => { | |
| if (step === 'advisor') fetchAdvisors(); | |
| }, [step]); | |
| useEffect(() => { | |
| if (step === 'advisor') fetchAdvisors(); | |
| }, [step, fetchAdvisors]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/RoleSelect.tsx` around lines 98 - 100, The effect in RoleSelect.tsx
references fetchAdvisors but omits it from the dependency array, which can cause
stale closures; update the useEffect that currently reads "useEffect(() => { if
(step === 'advisor') fetchAdvisors(); }, [step]);" to include fetchAdvisors in
the dependency array or else stabilize fetchAdvisors by wrapping it with
useCallback; specifically either change the dependency array to [step,
fetchAdvisors] or memoize the fetchAdvisors function so its identity is stable
and then include it in the dependencies to satisfy React's exhaustive-deps rule.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RoleSelect.tsxto include customer names in the selection process, improving user experience by displaying human-readable labels.Topbar.tsxto show customer names alongside IDs for better clarity.CustomerContext.tsxto store and manage customer names.api.tsto support new customer note functionalities.Summary by CodeRabbit
New Features
Improvements