WhatsApp From: Add Sync Button on WhatsApp Forms List Page #3683
Conversation
WalkthroughAdds a new GraphQL mutation Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx}⚙️ CodeRabbit configuration file
Files:
🧬 Code graph analysis (1)src/mocks/WhatsAppForm.tsx (1)
⏰ 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). (4)
🔇 Additional comments (1)
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 |
|
🚀 Deployed on https://deploy-preview-3683--glific-frontend.netlify.app |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx (2)
16-16: Remove unused imports.The
useQueryandgqlimports are not used anywhere in this file. TheSYNC_WHATSAPP_FORMmutation is already imported from the mutations file.Apply this diff:
-import { useQuery, gql } from '@apollo/client';
101-127: Consider using Apollo's callbacks for consistency.The current
.then/.catchapproach works but is inconsistent with other mutations in this file (e.g.,publishFormon lines 90-99 usesonCompleted/onError). Apollo's callbacks provide automatic refetch and cache updates.Consider this refactor:
- const handleHsmUpdates = () => { - setSyncTemplateLoad(true); - - syncWhatsappForm({ + const [syncWhatsappForm, { loading: syncTemplateLoad }] = useMutation(SYNC_WHATSAPP_FORM, { + onCompleted: (res) => { + const result = res?.syncWhatsappForm; + if (result?.errors?.length) { + const errorMessages = result.errors.map((err: any) => err.message).join(', '); + setErrorMessage(errorMessages, 'An error occurred'); + } else { + setNotification(result?.message || 'WhatsApp Forms synced successfully'); + } + }, + onError: (errors) => { + setErrorMessage(formatError(errors.message), 'An error occurred'); + }, + }); + + const handleHsmUpdates = () => { + syncWhatsappForm({ variables: { organization_id: getUserSession('organizationId'), }, - }) - .then((res) => { - setSyncTemplateLoad(false); - - const result = res?.data?.syncWhatsappForm; - console.log(result, 'result'); - - if (result?.errors?.length) { - const errorMessages = result.errors.map((err: any) => err.message).join(', '); - setErrorMessage(errorMessages, 'An error occurred'); - } else { - setNotification(result?.message || 'WhatsApp Forms synced successfully'); - } - }) - .catch((errors) => { - console.log('error'); - setSyncTemplateLoad(false); - setErrorMessage(formatError(errors.message), 'An error occurred'); - }); + }); };This would also eliminate the manual
syncTemplateLoadstate (line 85) and use Apollo's built-inloadingstate instead.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx(6 hunks)src/graphql/mutations/WhatsAppForm.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.
Files:
src/graphql/mutations/WhatsAppForm.tssrc/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx
🧬 Code graph analysis (1)
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx (5)
src/graphql/mutations/WhatsAppForm.ts (1)
SYNC_WHATSAPP_FORM(93-103)src/services/AuthService.tsx (1)
getUserSession(140-176)src/common/notification.ts (2)
setErrorMessage(15-40)setNotification(4-13)src/containers/WhatsAppForms/WhatsAppForms.tsx (1)
formatError(28-31)src/components/UI/Form/Button/Button.tsx (1)
Button(12-29)
⏰ 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). (4)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: CI
- GitHub Check: glific (1.18.3-otp-27, 27.3.3)
- GitHub Check: build
🔇 Additional comments (4)
src/graphql/mutations/WhatsAppForm.ts (1)
92-103: LGTM! Mutation follows established patterns.The
SYNC_WHATSAPP_FORMmutation is well-structured and consistent with other mutations in this file.src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx (3)
85-88: LGTM! State and mutation hook properly defined.The loading state and mutation hook are correctly set up for the sync operation.
242-254: LGTM! Button implementation is correct.The secondary button properly integrates loading state and triggers the sync operation.
302-302: LGTM! Secondary button properly integrated.The button is correctly passed to the List component.
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx
Outdated
Show resolved
Hide resolved
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx
Outdated
Show resolved
Hide resolved
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx (1)
8-14: Remove unusedsyncWhatsappFormentry fromqueriesobject.
SYNC_WHATSAPP_FORMis correctly used via thesyncWhatsappFormmutation hook, but thesyncWhatsappFormkey insidequeriesis not consumed by theListcomponent and breaks the existing naming convention of that object. It just adds noise to the props being spread into<List />.You can keep the import for the hook usage and drop the extra entry:
const queries = { filterItemsQuery: LIST_WHATSAPP_FORMS, deleteItemQuery: DELETE_FORM, getItemQuery: GET_WHATSAPP_FORM, publishFlowQuery: PUBLISH_FORM, - syncWhatsappForm: SYNC_WHATSAPP_FORM, };Also applies to: 33-34
🧹 Nitpick comments (1)
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx (1)
21-22: Tighten sync handler: guard missing org id, de‑duplicate loading reset, and avoidanyfor errors.The overall flow looks good, but a few small tweaks will make it safer and cleaner:
- Guard against a missing
organizationIdbefore firing the mutation, so you fail fast rather than sending an invalid variable.- Move
setSyncWhatsappFormLoad(false)into a.finally()to ensure it always runs and to avoid duplication.- Replace
err: anywith a minimal structural type to keep type safety without over‑specifying the GraphQL shape.Example refactor:
- const handleHsmUpdates = () => { - setSyncWhatsappFormLoad(true); - - syncWhatsappForm({ - variables: { - organization_id: getUserSession('organizationId'), - }, - }) - .then((res) => { - setSyncWhatsappFormLoad(false); - - const result = res?.data?.syncWhatsappForm; - if (result?.errors?.length) { - const errorMessages = result.errors.map((err: any) => err.message).join(', '); - setErrorMessage(errorMessages, 'An error occurred'); - } else { - setNotification(result?.message || 'WhatsApp Forms synced successfully'); - } - }) - .catch((errors) => { - setSyncWhatsappFormLoad(false); - setErrorMessage(formatError(errors.message), 'An error occurred'); - }); - }; + const handleHsmUpdates = () => { + const organizationId = getUserSession('organizationId'); + if (!organizationId) { + setErrorMessage('Missing organization id in session', 'An error occurred'); + return; + } + + setSyncWhatsappFormLoad(true); + + syncWhatsappForm({ + variables: { + organization_id: organizationId, + }, + }) + .then((res) => { + const result = res?.data?.syncWhatsappForm; + if (result?.errors?.length) { + const errorMessages = result.errors + .map((err: { message: string }) => err.message) + .join(', '); + setErrorMessage(errorMessages, 'An error occurred'); + } else { + setNotification(result?.message || 'WhatsApp Forms synced successfully'); + } + }) + .catch((errors) => { + setErrorMessage(formatError(errors.message), 'An error occurred'); + }) + .finally(() => { + setSyncWhatsappFormLoad(false); + }); + };Also applies to: 84-87, 100-123
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx(6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.
Files:
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx
⏰ 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). (4)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build
- GitHub Check: glific (1.18.3-otp-27, 27.3.3)
- GitHub Check: CI
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3683 +/- ##
==========================================
+ Coverage 82.53% 82.55% +0.01%
==========================================
Files 348 348
Lines 11729 11748 +19
Branches 2478 2480 +2
==========================================
+ Hits 9681 9699 +18
- Misses 1298 1299 +1
Partials 750 750 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Glific
|
||||||||||||||||||||||||||||
| Project |
Glific
|
| Branch Review |
feat/add_sync_button
|
| Run status |
|
| Run duration | 28m 40s |
| Commit |
|
| Committer | Priyanshu singh |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
2
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
191
|
| View all changes introduced in this branch ↗︎ | |
priyanshu6238
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/mocks/WhatsAppForm.tsx (1)
373-392: Align error mock response shape and variables with the mutation schemaThe error mock currently returns:
syncWhatsappForm: { whatsappForm: null, errors: [{ message: 'Something went wrong' }], }while the documented selection set for
SYNC_WHATSAPP_FORMonly includesmessageanderrors { key, message }. The extrawhatsappFormfield and missingmessage/keymay confuse future readers and could break if the UI later relies onmessageorerror.key.Also, this mock uses
organization_id: 'org-123'while the success mock usednull, so only one of them will match depending on whatWhatsAppFormListactually sends.Consider:
- Using the same (non‑null)
organization_idhere as in the success mock, and- Shaping the result to mirror the mutation:
const syncWhatsappFormQueryWithErrors = { request: { query: SYNC_WHATSAPP_FORM, variables: { - organization_id: 'org-123', + organization_id: 'org-123', // keep in sync with success mock / component usage }, }, result: { data: { syncWhatsappForm: { - whatsappForm: null, - errors: [ - { - message: 'Something went wrong', - }, - ], + message: 'Something went wrong', + errors: [ + { + key: 'base', + message: 'Something went wrong', + }, + ], }, }, }, };(Feel free to adjust
keyand the exact message to whatever your backend actually returns.)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.test.tsx(2 hunks)src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx(6 hunks)src/mocks/WhatsAppForm.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.
Files:
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.test.tsxsrc/mocks/WhatsAppForm.tsx
🧬 Code graph analysis (2)
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.test.tsx (1)
src/mocks/WhatsAppForm.tsx (2)
syncWhatsappForm(407-407)syncWhatsappFormQueryWithErrors(407-407)
src/mocks/WhatsAppForm.tsx (1)
src/graphql/mutations/WhatsAppForm.ts (1)
SYNC_WHATSAPP_FORM(93-103)
⏰ 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). (3)
- GitHub Check: CI
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: glific (1.18.3-otp-27, 27.3.3)
🔇 Additional comments (3)
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.test.tsx (1)
10-12: Sync mocks import wiring looks correctThe added imports for
syncWhatsappFormandsyncWhatsappFormQueryWithErrorsline up with the new mocks and keep the test setup consistent with the rest of the file.src/mocks/WhatsAppForm.tsx (2)
1-7: Mutation import integration is consistentBringing
SYNC_WHATSAPP_FORMinto the existing WhatsApp form mutation imports keeps the mocks aligned with the real GraphQL API and follows the established pattern in this file.
406-407: Exports for new sync mocks look goodRe‑exporting
syncWhatsappFormQueryWithErrorsandsyncWhatsappFormfrom this module cleanly exposes the new mocks to tests and is consistent with existing patterns.
Target issue: #3684