-
Notifications
You must be signed in to change notification settings - Fork 106
Logic to call contact button #235
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Logic to call contact button #235
Conversation
|
@Malayt04 is attempting to deploy a commit to the cloudec31-gmailcom's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis change enhances the call creation and contact selection workflow by enabling preloading and preselecting contacts in modals. It introduces a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ContactsList
participant useModal
participant CreateCallModal
participant Backend
User->>ContactsList: Clicks "Call Contact"
ContactsList->>useModal: onOpen("start-call", {selectedContact: email})
useModal->>CreateCallModal: Passes selectedContact via modal data
CreateCallModal->>Backend: Fetch contacts on mount
Backend-->>CreateCallModal: Returns contacts list
CreateCallModal->>CreateCallModal: Preselects contact if selectedContact provided
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (6)
apps/web/hooks/use-modal.tsx (2)
22-22: Naming nit: clarify intent ofselectedContactConsider
selectedContactEmailto make it explicit this is an email string, and to avoid confusion with contact objects elsewhere.
37-39: Defensive copy to prevent accidental mutations ofdataIf callers reuse/mutate the same object reference after calling
onOpen, the store’s state can be mutated outside of Zustand. Make a shallow copy when opening.Apply this diff:
- onOpen: (type, data = {}) => set({ isOpen: true, type, data }), + onOpen: (type, data = {}) => set({ isOpen: true, type, data: { ...data } }),apps/web/components/modal/start-call.tsx (1)
33-42: Guard selection effect to only run when the modal is actually openPreselection works. To avoid any accidental state changes when the modal isn’t open (or if another modal instance reuses the store), add a guard on
isOpenandtype.Apply this diff:
-useEffect(() => { - if (selectedContact) { - setSelectedContacts((prev) => { - if (prev.includes(selectedContact)) { - return prev; - } - return [...prev, selectedContact]; - }); - } -}, [selectedContact]); +useEffect(() => { + if (!isOpen || type !== "start-call" || !selectedContact) return; + setSelectedContacts((prev) => { + if (prev.includes(selectedContact)) return prev; + return [...prev, selectedContact]; + }); +}, [selectedContact, isOpen, type]);apps/web/components/app/section/_components/create-call-modal.tsx (3)
18-23: Prop addition is fine; consider consolidating modal variantsAdding
selectedContact?: stringis fine. There’s overlap now between this modal andStartCallfor preselecting invitees. Consider consolidating to a single source of truth (one modal/selector) to reduce duplication.
34-51: Add AbortController to avoid setting state after unmountThe fetch effect is correct, but can set state on an unmounted component during fast navigations. Add an abort signal and cleanup.
Apply this diff:
useEffect(() => { - const fetchContacts = async () => { + const controller = new AbortController(); + const fetchContacts = async () => { try { const res = await fetch(`${process.env.NEXT_PUBLIC_BACKEND_URL}/api/contacts`, { credentials: "include", + signal: controller.signal, }); if (!res.ok) { console.error("Failed to fetch contacts"); return; - }; + } const data = await res.json(); setContacts(data.contacts || []); } catch (err) { - console.error("Failed to fetch contacts", err); + // Ignore abort errors to avoid noisy logs on unmount + if ((err as any)?.name !== "AbortError") { + console.error("Failed to fetch contacts", err); + } } }; fetchContacts(); -}, []); + return () => controller.abort(); +}, []);
57-66: Preselection works; consider consistent email normalization and surfacing non-list emailsEffect logic is correct. For robustness:
- Normalize email casing before comparing to avoid duplicates.
- If
selectedContactisn’t in the fetched list, consider temporarily surfacing it in the UI (placeholder row) so users see who will be invited.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
COMMIT_MSG.tmp(0 hunks)apps/web/components/app/section/_components/create-call-modal.tsx(2 hunks)apps/web/components/app/section/contacts-list.tsx(4 hunks)apps/web/components/modal/start-call.tsx(3 hunks)apps/web/hooks/use-modal.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- COMMIT_MSG.tmp
🧰 Additional context used
🧬 Code Graph Analysis (2)
apps/web/components/app/section/contacts-list.tsx (3)
apps/web/hooks/use-modal.tsx (1)
useModal(33-39)apps/web/app/(app)/app/contact/page.tsx (1)
ContactPage(8-24)apps/web/components/app/section/_components/call-history.tsx (1)
useModal(278-300)
apps/web/components/modal/start-call.tsx (2)
apps/web/hooks/use-modal.tsx (1)
useModal(33-39)apps/web/lib/QUERIES.ts (1)
CALLS_QUERY(23-45)
🔇 Additional comments (4)
apps/web/components/app/section/contacts-list.tsx (2)
26-26: LGTM: integratesuseModalImport looks correct and aligns with the modal store.
228-228: LGTM: open modal from cardUsing
useModalinsideContactCardkeeps concerns localized.apps/web/components/modal/start-call.tsx (2)
27-29: LGTM: consuming modaldataDestructuring
dataand readingselectedContactis aligned with the updated store shape.
49-51: LGTM: pluralization copyConcise and correct pluralization.
| </DropdownMenuTrigger> | ||
| <DropdownMenuContent align="end"> | ||
| <DropdownMenuItem> | ||
| <DropdownMenuItem onClick={() => onOpen("start-call", { selectedContact: contact.email })}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Pre-filling selected contact is correct; consider email normalization
The onOpen("start-call", { selectedContact: contact.email }) flow is correct. To avoid case-related duplicates in selections, consider normalizing emails (e.g., lowercasing) consistently across the app.
Run a quick scan to see if email normalization is already in place for selections:
🏁 Script executed:
#!/bin/bash
rg -n --no-heading --pretty $'toLowerCase\\(\\).*email|selectedContacts' || trueLength of output: 2810
Normalize emails to lowercase before storing and comparing
I didn’t find any .toLowerCase() usage around contact emails in the selection flows (contacts-list.tsx, contact-selector.tsx, start-call.tsx). To prevent case‐related duplicates, normalize email values (e.g. email.trim().toLowerCase()) when invoking onOpen, adding to selectedContacts, and when checking for membership.
Places to update:
- apps/web/components/app/section/contacts-list.tsx (line 242)
- apps/web/components/modal/contact-selector.tsx (lines 37 & 93)
Example change:
- onOpen("start-call", { selectedContact: contact.email })
+ onOpen("start-call", {
+ selectedContact: contact.email.trim().toLowerCase()
+ })Ensure you apply the same normalization in your setSelectedContacts calls and in any selectedContacts.includes(...) checks.
📝 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.
| <DropdownMenuItem onClick={() => onOpen("start-call", { selectedContact: contact.email })}> | |
| <DropdownMenuItem onClick={() => onOpen("start-call", { | |
| selectedContact: contact.email.trim().toLowerCase() | |
| })}> |
🤖 Prompt for AI Agents
In apps/web/components/app/section/contacts-list.tsx at line 242, the email
passed to onOpen is not normalized, which can cause case-related duplicates. Fix
this by applying email normalization using trim() and toLowerCase() before
passing the email, and ensure similar normalization is applied in all places
where emails are stored or compared, including setSelectedContacts calls and
membership checks.
|
@Malayt04 just noticed a thing in video, when you click on join call button in notification both join call and decline button were in loading state, we can improve this |
|
Yeah, but @abuharish02's pr is already merged for this issue now. |
#207
When a user initiates a call from the contacts list, the "Start Call" modal now opens with the selected contact pre-filled and ready.
Screen.Recording.2025-08-10.121428.mp4
Summary by CodeRabbit
New Features
Improvements