-
Notifications
You must be signed in to change notification settings - Fork 48
Whatsapp Forms #3624
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: master
Are you sure you want to change the base?
Whatsapp Forms #3624
Conversation
…omponents (#3607) * feat: add WhatsApp Forms feature with CRUD operations and UI components * fix: correct SVG attribute casing in WhatsAppForms component * fix: correct component name from WhatsAppForms to WhatsAppForm for consistency * feat: update WhatsAppForms component with new queries and edit route * feat: ✨ Move WhatsApp Forms menu item and add service-based visibility The WhatsApp Forms menu item is now positioned earlier in the menu order and includes a visibility condition based on the whatsappFormsEnabled organization service setting. * feat: refactor WhatsAppForms component to use formJson and formCategories, add error handling for JSON parsing * feat: add FlowBuilderInfo section with guidance and button to open Meta Flow Builder in WhatsAppForms component * feat: add tests for WhatsAppForms component and mock data for form handling * fix: add missing newline at end of WhatsAppForm.module.css --------- Co-authored-by: Shijith <[email protected]>
WalkthroughAdds end-to-end WhatsApp Forms support: new UI components (form builder, list, response view, icons, CSS), GraphQL queries/mutations/subscriptions for CRUD and status transitions, route and menu wiring, HSM and TemplateOptions integration to support a new WHATSAPP_FORM button type, new constants and service flag, updated mocks/tests across many suites, minor List typing change, i18n key adjustments, and a GitHub Actions branch checkout change. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (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-3624--glific-frontend.netlify.app |
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: 4
🧹 Nitpick comments (4)
src/graphql/queries/WhatsAppForm.ts (1)
21-25: Add an operation name to the query.The query lacks an operation name, which is recommended for better debugging and tooling support.
Apply this diff:
export const LIST_FORM_CATEGORIES = gql` - query { + query WhatsappFormCategories { whatsappFormCategories } `;src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx (1)
7-30: Complete the list implementation.The component currently renders only a header and placeholder text. The TODO comment indicates that the actual
<List />component needs to be integrated to display the WhatsApp Forms list.Do you want me to help implement the list component integration or track this as a separate issue?
src/containers/WhatsAppForms/WhatsAppForms.tsx (2)
27-37: Improve type safety and variable declarations.Two minor improvements:
- Line 30:
formJsonstate lacks a type annotation, reducing type safety.- Line 34:
isEditingshould beconstsince it's never reassigned.Apply this diff:
- const [formJson, setFormJson] = useState(); + const [formJson, setFormJson] = useState<any>(); const [formCategories, setFormCategories] = useState([]); const [categories, setCategories] = useState([]); const params = useParams(); - let isEditing = false; + const isEditing = !!params.id; - if (params.id) { - isEditing = true; - }
140-140: Remove unused variable.The
dialogMessagevariable is declared but never assigned a value or used. Consider removing it if it's not needed, or implement it if it's meant for future functionality.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/assets/images/icons/SideDrawer/WhatsappForm.tsx(1 hunks)src/common/HelpData.tsx(1 hunks)src/components/UI/ListIcon/ListIcon.tsx(2 hunks)src/config/menu.ts(1 hunks)src/containers/WhatsAppForms/WhatsAppForm.module.css(1 hunks)src/containers/WhatsAppForms/WhatsAppForm.test.tsx(1 hunks)src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx(1 hunks)src/containers/WhatsAppForms/WhatsAppForms.tsx(1 hunks)src/graphql/mutations/WhatsAppForm.ts(1 hunks)src/graphql/queries/Organization.ts(1 hunks)src/graphql/queries/WhatsAppForm.ts(1 hunks)src/mocks/WhatsApp.tsx(1 hunks)src/routes/AuthenticatedRoute/AuthenticatedRoute.tsx(2 hunks)src/services/AuthService.tsx(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/routes/AuthenticatedRoute/AuthenticatedRoute.tsxsrc/graphql/queries/WhatsAppForm.tssrc/graphql/mutations/WhatsAppForm.tssrc/assets/images/icons/SideDrawer/WhatsappForm.tsxsrc/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsxsrc/common/HelpData.tsxsrc/containers/WhatsAppForms/WhatsAppForm.test.tsxsrc/config/menu.tssrc/graphql/queries/Organization.tssrc/mocks/WhatsApp.tsxsrc/components/UI/ListIcon/ListIcon.tsxsrc/containers/WhatsAppForms/WhatsAppForms.tsxsrc/services/AuthService.tsx
🧬 Code graph analysis (6)
src/routes/AuthenticatedRoute/AuthenticatedRoute.tsx (1)
src/containers/WhatsAppForms/WhatsAppForms.tsx (1)
WhatsAppForms(27-197)
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx (3)
src/components/UI/HelpIcon/HelpIcon.tsx (1)
HelpIcon(12-54)src/common/HelpData.tsx (1)
whatsappFormsInfo(143-146)src/components/UI/Form/Button/Button.tsx (1)
Button(12-29)
src/containers/WhatsAppForms/WhatsAppForm.test.tsx (3)
src/mocks/WhatsApp.tsx (2)
WHATSAPP_FORM_MOCKS(124-129)formJson(4-22)src/containers/WhatsAppForms/WhatsAppForms.tsx (1)
WhatsAppForms(27-197)src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx (1)
WhatsAppFormList(7-30)
src/config/menu.ts (1)
src/services/AuthService.tsx (1)
getOrganizationServices(187-192)
src/mocks/WhatsApp.tsx (2)
src/graphql/queries/WhatsAppForm.ts (2)
LIST_FORM_CATEGORIES(21-25)GET_WHATSAPP_FORM(3-19)src/graphql/mutations/WhatsAppForm.ts (2)
CREATE_FORM(3-15)UPDATE_FORM(17-29)
src/containers/WhatsAppForms/WhatsAppForms.tsx (8)
src/graphql/queries/WhatsAppForm.ts (2)
GET_WHATSAPP_FORM(3-19)LIST_FORM_CATEGORIES(21-25)src/graphql/mutations/WhatsAppForm.ts (3)
CREATE_FORM(3-15)UPDATE_FORM(17-29)DELETE_FORM(31-43)src/components/UI/Form/Input/Input.tsx (1)
Input(34-154)src/components/UI/Form/AutoComplete/AutoComplete.tsx (1)
AutoComplete(46-281)src/components/UI/Layout/Loading/Loading.tsx (1)
Loading(14-41)src/components/UI/Heading/Heading.tsx (1)
Heading(23-59)src/common/HelpData.tsx (1)
whatsappFormsInfo(143-146)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: glific (1.18.3-otp-27, 27.3.3)
- GitHub Check: CI
- GitHub Check: build
🔇 Additional comments (19)
src/graphql/queries/Organization.ts (1)
139-139: LGTM!The addition of
whatsappFormsEnabledto the organization services query is consistent with the existing service flag pattern and integrates cleanly with the feature flag infrastructure.src/components/UI/ListIcon/ListIcon.tsx (2)
37-37: LGTM!The import and icon mapping for WhatsApp Forms follows the established pattern and integrates cleanly with the existing icon system.
83-83: LGTM!The form icon mapping is correctly registered in the icon dictionary.
src/routes/AuthenticatedRoute/AuthenticatedRoute.tsx (2)
71-72: LGTM!The lazy-loaded imports for WhatsApp Forms components follow the established pattern and enable proper code splitting.
164-168: LGTM!The route configuration for WhatsApp Forms is well-structured with proper nested routes for list, add, and edit operations, consistent with other features like certificates and polls.
src/services/AuthService.tsx (1)
23-24: LGTM!The
ServiceTypeunion is correctly extended to include'whatsappFormsEnabled', enabling type-safe access to the WhatsApp Forms service flag throughout the application.src/containers/WhatsAppForms/WhatsAppForm.test.tsx (1)
1-86: LGTM!The test suite provides good coverage of the WhatsApp Forms functionality:
- Initial render and loading states
- Form validation (including JSON validation)
- Create flow with field interactions and autocomplete
- Edit flow with pre-populated data
The tests follow established testing patterns and use appropriate assertions. The JSON validation test correctly verifies that invalid JSON triggers an error before proceeding with valid input.
src/assets/images/icons/SideDrawer/WhatsappForm.tsx (1)
1-23: LGTM!The WhatsApp Form icon component is correctly implemented:
- Accepts and applies the
colorprop appropriately- Uses
currentColorfor stroke, which will inherit from thecolorattribute- SVG attributes are properly camelCased for React
- Consistent with other icon components in the codebase
src/graphql/queries/WhatsAppForm.ts (1)
3-19: LGTM!The query structure is correct and field selection is comprehensive for fetching WhatsApp form details.
src/mocks/WhatsApp.tsx (1)
4-129: LGTM!The mock data structure is consistent with the GraphQL queries and mutations, and provides comprehensive test coverage for form creation, retrieval, and updates.
src/graphql/mutations/WhatsAppForm.ts (1)
3-29: LGTM!The CREATE_FORM and UPDATE_FORM mutations are well-structured and consistent.
src/containers/WhatsAppForms/WhatsAppForm.module.css (1)
1-76: LGTM!The CSS module provides well-structured styling for the Flow Builder UI with appropriate visual feedback (hover effects, transitions) and modern layout techniques.
src/containers/WhatsAppForms/WhatsAppForms.tsx (7)
39-51: LGTM!The category fetching and transformation logic correctly converts snake_case categories to user-friendly Title Case format.
53-58: LGTM!The states object correctly aggregates form state for the FormLayout component.
60-69: LGTM!The payload transformation correctly maps form categories to their IDs for API submission.
70-84: Potential inconsistency in category mapping.Line 74 maps categories to
{ id: c, name: c }, where both id and name are the same raw category string. However, the categories list from lines 42-48 uses id as snake_case and name as Title Case. This inconsistency might cause display issues or selection problems in the AutoComplete component.Verify that the AutoComplete component correctly matches these category objects. Consider applying the same transformation used when loading categories:
- setFormCategories(categories.map((c: string) => ({ id: c, name: c }))); + setFormCategories( + categories.map((c: string) => ({ + id: c, + name: c + .toLowerCase() + .replace(/_/g, ' ') + .replace(/\b\w/g, (char) => char.toUpperCase()), + })) + );
85-121: LGTM!The form field definitions are well-structured with appropriate input types, placeholders, and helper text.
122-138: LGTM!The validation schema is well-defined with appropriate constraints for all required fields. The custom JSON validation test is a good addition.
145-196: LGTM!The component rendering logic is well-structured with appropriate loading states, helpful UI hints for the Meta Flow Builder, and secure external link handling.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3624 +/- ##
==========================================
- Coverage 82.55% 82.35% -0.20%
==========================================
Files 341 348 +7
Lines 11416 11648 +232
Branches 2415 2451 +36
==========================================
+ Hits 9424 9593 +169
- Misses 1260 1313 +53
- Partials 732 742 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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: 5
🧹 Nitpick comments (1)
src/containers/Chat/ChatMessages/ChatMessage/WhatsappFormResponse/WhatsAppFormResponse.tsx (1)
54-62: Refactor inline styles to CSS modules and add null handling.
- Move inline styles to the CSS module for consistency
- Handle null/undefined values properly
- {Object.entries(parsedResponse).map(([key, value]) => ( - <div key={key} style={{ marginBottom: '8px' }}> - <Typography variant="subtitle2" style={{ fontWeight: 'bold' }}> + {Object.entries(parsedResponse).map(([key, value]) => ( + <div key={key} className={styles.ResponseItem}> + <Typography variant="subtitle2" className={styles.ResponseKey}> {key}: </Typography> <Typography variant="body2"> - {typeof value === 'object' ? JSON.stringify(value, null, 2) : String(value)} + {value == null ? 'N/A' : typeof value === 'object' ? JSON.stringify(value, null, 2) : String(value)} </Typography> </div> ))}Add to CSS module:
.ResponseItem { margin-bottom: 8px; } .ResponseKey { font-weight: bold; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/containers/Chat/ChatMessages/ChatMessage/ChatMessage.tsx(4 hunks)src/containers/Chat/ChatMessages/ChatMessage/WhatsappFormResponse/WhatsAppFormResponse.module.css(1 hunks)src/containers/Chat/ChatMessages/ChatMessage/WhatsappFormResponse/WhatsAppFormResponse.tsx(1 hunks)src/graphql/queries/Search.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/containers/Chat/ChatMessages/ChatMessage/WhatsappFormResponse/WhatsAppFormResponse.tsxsrc/graphql/queries/Search.tssrc/containers/Chat/ChatMessages/ChatMessage/ChatMessage.tsx
🧬 Code graph analysis (2)
src/containers/Chat/ChatMessages/ChatMessage/WhatsappFormResponse/WhatsAppFormResponse.tsx (1)
src/components/UI/DialogBox/DialogBox.tsx (1)
DialogBox(32-149)
src/containers/Chat/ChatMessages/ChatMessage/ChatMessage.tsx (1)
src/containers/Chat/ChatMessages/ChatMessage/WhatsappFormResponse/WhatsAppFormResponse.tsx (1)
WhatsAppFormResponse(11-72)
⏰ 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: glific (1.18.3-otp-27, 27.3.3)
- GitHub Check: build
- GitHub Check: CI
🔇 Additional comments (4)
src/graphql/queries/Search.ts (1)
72-75: LGTM!The addition of
whatsappFormResponsefield to the query is properly structured and aligns with the WhatsApp Forms feature requirements.src/containers/Chat/ChatMessages/ChatMessage/ChatMessage.tsx (2)
29-29: LGTM!The import statement is correct and follows the existing import patterns in the file.
336-343: LGTM!The rendering logic for WhatsApp form responses follows the established pattern for other message types and correctly passes the raw response data to the component.
src/containers/Chat/ChatMessages/ChatMessage/WhatsappFormResponse/WhatsAppFormResponse.module.css (1)
1-26: LGTM!The CSS module is well-structured with appropriate styles for the WhatsApp form response UI component.
src/containers/Chat/ChatMessages/ChatMessage/WhatsappFormResponse/WhatsAppFormResponse.tsx
Show resolved
Hide resolved
src/containers/Chat/ChatMessages/ChatMessage/WhatsappFormResponse/WhatsAppFormResponse.tsx
Show resolved
Hide resolved
src/containers/Chat/ChatMessages/ChatMessage/WhatsappFormResponse/WhatsAppFormResponse.tsx
Outdated
Show resolved
Hide resolved
src/containers/Chat/ChatMessages/ChatMessage/WhatsappFormResponse/WhatsAppFormResponse.tsx
Outdated
Show resolved
Hide resolved
* feat: add WhatsApp Forms feature with CRUD operations and UI components * fix: correct SVG attribute casing in WhatsAppForms component * fix: correct component name from WhatsAppForms to WhatsAppForm for consistency * feat: update WhatsAppForms component with new queries and edit route * feat: ✨ Move WhatsApp Forms menu item and add service-based visibility The WhatsApp Forms menu item is now positioned earlier in the menu order and includes a visibility condition based on the whatsappFormsEnabled organization service setting. * feat: refactor WhatsAppForms component to use formJson and formCategories, add error handling for JSON parsing * feat: add FlowBuilderInfo section with guidance and button to open Meta Flow Builder in WhatsAppForms component * feat: add tests for WhatsAppForms component and mock data for form handling * feat: add listing ui for whatsapp form * fix: css * refactor: remove unused css * fix: css and remove screen debug function from test case * fix: deep scan * refactor: add status field in the column * reafctor: add test case * fix:test case * refactor: add mock * reafctor: add deactivate option * reafctor: add + symbol for category * refactor: add dialog box * fix: first test case * fix: test case * fix: add test case * fix : deep scan * refactor: add hover to the + more option * fix: css and cypress * refactor: rename dialog message * fix: add delete api * fix: css * fix: css * fix: css * fix: css * fix: naming --------- Co-authored-by: Akansha Sakhre <[email protected]> Co-authored-by: Shijith <[email protected]>
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: 6
🧹 Nitpick comments (3)
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx (3)
54-63: RenameInactiveItemtodeactivateItemfor consistency.The function name should match the action it performs and align with the naming pattern of
publishItem.Apply this diff:
- const InactiveItem = async (item: any) => { + const deactivateItem = async (item: WhatsAppForm) => { try { await deactivateForm({ variables: { id: item.id }, }); setNotification('Form deactivated successfully'); } catch (error) { setErrorMessage(error); } };Also update the call site at line 194:
if (dialogType === 'publish') { publishItem(currentItem); } else if (dialogType === 'inactive') { - InactiveItem(currentItem); + deactivateItem(currentItem); }
178-178: Avoid variable shadowing in the map function.The variable name
filterin the map callback shadows the outerfilterstate variable, which can cause confusion.Apply this diff:
- {filterList.map((filter: any) => ( - <MenuItem key={filter.label} value={filter.value}> - {filter.label} + {filterList.map((filterOption) => ( + <MenuItem key={filterOption.label} value={filterOption.value}> + {filterOption.label} </MenuItem> ))}
151-163: Add proper typing for the filters object.The
filtersvariable usesanytype, reducing type safety.Apply this diff:
const filters = useMemo(() => { - let filters: any = {}; + let filters: { status?: 'PUBLISHED' | 'DRAFT' | 'INACTIVE' } = {}; if (filter === 'published') { filters = { status: 'PUBLISHED' }; } else if (filter === 'draft') { filters = { status: 'DRAFT' }; } else if (filter === 'inactive') { filters = { status: 'INACTIVE' }; } return filters; }, [filter]);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/assets/images/icons/DeactivateIcon.svgis excluded by!**/*.svgsrc/assets/images/icons/PublishGood.svgis excluded by!**/*.svg
📒 Files selected for processing (9)
.github/workflows/cypress-testing.yml(1 hunks)src/containers/WhatsAppForms/WhatsAppForm.module.css(1 hunks)src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.module.css(1 hunks)src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.test.tsx(1 hunks)src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx(1 hunks)src/graphql/mutations/WhatsAppForm.ts(1 hunks)src/graphql/queries/WhatsAppForm.ts(1 hunks)src/i18n/en/en.json(1 hunks)src/mocks/WhatsApp.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.module.css
🧰 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/mocks/WhatsApp.tsxsrc/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsxsrc/graphql/queries/WhatsAppForm.tssrc/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.test.tsxsrc/graphql/mutations/WhatsAppForm.ts
🧬 Code graph analysis (3)
src/mocks/WhatsApp.tsx (2)
src/graphql/queries/WhatsAppForm.ts (3)
LIST_FORM_CATEGORIES(21-25)LIST_WHATSAPP_FORMS(27-39)GET_WHATSAPP_FORM(3-19)src/graphql/mutations/WhatsAppForm.ts (4)
CREATE_FORM(3-15)PUBLISH_FORM(50-67)DEACTIVATE_FORM(69-82)UPDATE_FORM(17-29)
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx (4)
src/graphql/queries/WhatsAppForm.ts (2)
LIST_WHATSAPP_FORMS(27-39)GET_WHATSAPP_FORM(3-19)src/graphql/mutations/WhatsAppForm.ts (3)
DELETE_FORM(31-48)PUBLISH_FORM(50-67)DEACTIVATE_FORM(69-82)src/components/UI/DialogBox/DialogBox.tsx (1)
DialogBox(32-149)src/common/HelpData.tsx (1)
whatsappFormsInfo(143-146)
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.test.tsx (2)
src/mocks/WhatsApp.tsx (5)
WHATSAPP_FORM_MOCKS(275-284)publishWhatsappForm(69-85)publishWhatsappFormError(104-112)deactivateWhatsappForm(87-103)deactivateWhatsappFormError(114-122)src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx (1)
WhatsAppFormList(24-242)
🪛 Biome (2.1.2)
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.test.tsx
[error] 15-16: Do not export from a test file.
(lint/suspicious/noExportsInTest)
src/i18n/en/en.json
[error] 14-14: The key Last Updated was already declared.
This where a duplicated key was declared again.
If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.
(lint/suspicious/noDuplicateObjectKeys)
⏰ 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
🔇 Additional comments (3)
src/graphql/queries/WhatsAppForm.ts (1)
1-39: LGTM!The GraphQL queries are well-structured with appropriate field selections that align with the feature requirements and mock data.
src/graphql/mutations/WhatsAppForm.ts (1)
1-82: LGTM!All five mutations are properly structured with correct operation names, variables, and response shapes that align with the expected GraphQL schema.
src/containers/WhatsAppForms/WhatsAppForm.module.css (1)
1-75: LGTM!The CSS module is well-structured with modern styling patterns, smooth transitions, and consistent visual hierarchy.
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.test.tsx
Outdated
Show resolved
Hide resolved
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx
Outdated
Show resolved
Hide resolved
* Add WhatsApp form icons and update styles; refactor form-related tests and mocks * Refactor dialog handling in WhatsAppFormList component; improve delete, publish, and deactivate confirmation messages * minor fixes * remove null check * added error message * fixed test cases
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: 3
♻️ Duplicate comments (2)
src/mocks/WhatsAppForm.tsx (1)
69-102: Align publish/deactivate mocks with mutation schema (field name + response shape)Both mutation mocks here diverge from the GraphQL schema:
publishWhatsappFormmock (Lines 69–84) should wrap the form inwhatsappFormand includeerrors, perPUBLISH_FORM.deactivateWhatsappFormmock (Lines 86–102) uses the wrong root field name (publishWhatsappForm) and returns a flat object with lowercase status, instead ofdeactivateWhatsappForm.whatsappForm.status.This can easily trip up future code/tests that expect the schema shape (e.g.,
data.deactivateWhatsappForm.whatsappForm.status), and it contradicts the documented mutations.Consider updating both mocks like this:
export const publishWhatsappForm = { request: { query: PUBLISH_FORM, variables: { id: '3', }, }, result: { data: { - publishWhatsappForm: { - id: '1', - status: 'PUBLISHED', - }, + publishWhatsappForm: { + whatsappForm: { + id: '1', + status: 'PUBLISHED', + }, + errors: null, + }, }, }, }; export const deactivateWhatsappForm = { request: { query: DEACTIVATE_FORM, variables: { id: '3', }, }, result: { data: { - publishWhatsappForm: { - id: '1', - status: 'inactive', - __typename: 'WhatsappForm', - }, + deactivateWhatsappForm: { + whatsappForm: { + id: '1', + status: 'INACTIVE', + }, + errors: null, + }, }, }, };This keeps mocks faithful to the schema and avoids subtle bugs if result data is ever accessed instead of just side effects.
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.test.tsx (1)
16-16: Remove unnecessary export from the test fileThis test file shouldn’t re-export mocks; it’s meant to be a leaf module, and Biome is already flagging this. The tests use the imported mocks directly, so the export is redundant.
Apply:
-import { WhatsAppFormList } from './WhatsAppFormList'; - -export { publishWhatsappForm, publishWhatsappFormError } from 'mocks/WhatsAppForm'; - -const mockNavigate = vi.fn(); +import { WhatsAppFormList } from './WhatsAppFormList'; + +const mockNavigate = vi.fn();
🧹 Nitpick comments (6)
src/containers/List/List.tsx (1)
126-134:additionalActionicon optionality matches usage; consider tightening action typingMaking
iconoptional is correct given some actions (e.g., WhatsApp forms activate/deactivate) are text-only and React safely rendersundefinedicons. You may also want to extend this inline type to cover other properties actually used inList(hidden?,insideMore?) so call sites get better TS help when building action objects, but that’s a nicety, not a blocker.src/graphql/mutations/WhatsAppForm.ts (2)
44-56: Inconsistent mutation naming convention.The mutation name
publishWhatsappFormuses a lowercase 'p', while other mutations in this file follow the patternCreateWhatsappForm,UpdateWhatsappForm,DeleteWhatsappForm, andDeactivateWhatsappForm(all capitalized).Consider standardizing to
PublishWhatsappFormfor consistency:export const PUBLISH_FORM = gql` - mutation publishWhatsappForm($id: ID!) { + mutation PublishWhatsappForm($id: ID!) { publishWhatsappForm(id: $id) { whatsappForm { id status } errors { message } } } `;Verify that the backend schema supports the capitalized mutation name, or document if the lowercase naming is required by the backend.
72-91: Inconsistent variable naming and extensive field selection.Two observations:
Variable naming: The variable is named
$activateWhatsappFormIdwhile all other mutations in this file use$id. This breaks the established pattern and adds unnecessary verbosity.Field selection: This mutation returns 9 fields (categories, definition, description, id, insertedAt, metaFlowId, name, status, updatedAt) while other mutations return only 2-3 fields.
For consistency, consider renaming the variable:
export const ACTIVATE_FORM = gql` - mutation ActivateWhatsappForm($activateWhatsappFormId: ID!) { - activateWhatsappForm(id: $activateWhatsappFormId) { + mutation ActivateWhatsappForm($id: ID!) { + activateWhatsappForm(id: $id) { whatsappForm { categories definition description id insertedAt metaFlowId name status updatedAt } errors { message } } } `;Verify whether the extensive field selection is intentional (e.g., to update the UI after activation) or if it should be reduced to match other mutations (id, status, errors) for consistency.
src/containers/WhatsAppForms/WhatsAppForms.tsx (3)
46-49: Simplify boolean logic.The
isEditingassignment is unnecessarily verbose.Apply this diff to simplify:
- let isEditing = false; - if (params.id) { - isEditing = true; - } + const isEditing = Boolean(params.id);
72-81: Replaceanytype with proper TypeScript interface.The
setPayloadfunction usesanytype, which defeats TypeScript's type safety benefits.Define a proper interface for the form data:
interface WhatsAppFormData { name: string; formJson: string; formCategories: Array<{ id: string; name: string }>; description: string; }Then apply this diff:
- const setPayload = ({ name, formJson, formCategories, description }: any) => { + const setPayload = ({ name, formJson, formCategories, description }: WhatsAppFormData) => { const payload = { name, formJson, description, categories: formCategories.map((category: any) => category.id), }; return payload; };Also remove the
anytype from the map callback:- categories: formCategories.map((category: any) => category.id), + categories: formCategories.map((category) => category.id),
175-191: Simplify button implementation.The button uses
component="a"(line 180) combined with anonClickhandler (line 181), which is unconventional. When usingonClickwithwindow.open, thecomponent="a"prop is unnecessary.Apply this diff to simplify:
<Button color="primary" variant="contained" className={styles.FlowBuilderButton} startIcon={<OpenInNew />} - component="a" onClick={() => window.open( 'https://developers.facebook.com/docs/whatsapp/flows/playground', '_blank', 'noopener,noreferrer' ) } > Open Flow Builder </Button>Alternatively, extract the handler to a separate function for better readability:
const handleOpenFlowBuilder = () => { window.open( 'https://developers.facebook.com/docs/whatsapp/flows/playground', '_blank', 'noopener,noreferrer' ); }; // In JSX: <Button color="primary" variant="contained" className={styles.FlowBuilderButton} startIcon={<OpenInNew />} onClick={handleOpenFlowBuilder} > Open Flow Builder </Button>
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/assets/images/icons/Publish/PublishGray.svgis excluded by!**/*.svgsrc/assets/images/icons/Publish/PublishWhite.svgis excluded by!**/*.svg
📒 Files selected for processing (11)
src/components/floweditor/FlowEditor.tsx(1 hunks)src/containers/List/List.tsx(1 hunks)src/containers/WhatsAppForms/WhatsAppForm.module.css(1 hunks)src/containers/WhatsAppForms/WhatsAppForm.test.tsx(1 hunks)src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.module.css(1 hunks)src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.test.tsx(1 hunks)src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx(1 hunks)src/containers/WhatsAppForms/WhatsAppForms.tsx(1 hunks)src/graphql/mutations/WhatsAppForm.ts(1 hunks)src/i18n/en/en.json(1 hunks)src/mocks/WhatsAppForm.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/components/floweditor/FlowEditor.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx
- src/containers/WhatsAppForms/WhatsAppForm.test.tsx
- src/i18n/en/en.json
🧰 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/List/List.tsxsrc/mocks/WhatsAppForm.tsxsrc/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.test.tsxsrc/containers/WhatsAppForms/WhatsAppForms.tsxsrc/graphql/mutations/WhatsAppForm.ts
🧬 Code graph analysis (3)
src/mocks/WhatsAppForm.tsx (2)
src/graphql/queries/WhatsAppForm.ts (3)
LIST_FORM_CATEGORIES(21-25)LIST_WHATSAPP_FORMS(27-39)GET_WHATSAPP_FORM(3-19)src/graphql/mutations/WhatsAppForm.ts (4)
CREATE_FORM(3-15)PUBLISH_FORM(44-56)DEACTIVATE_FORM(58-70)UPDATE_FORM(17-29)
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.test.tsx (3)
src/mocks/WhatsAppForm.tsx (5)
WHATSAPP_FORM_MOCKS(274-283)publishWhatsappForm(69-84)publishWhatsappFormError(103-111)deactivateWhatsappForm(86-102)deactivateWhatsappFormError(113-121)src/containers/WhatsAppForms/WhatsAppForms.tsx (1)
WhatsAppForms(27-215)src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.tsx (1)
WhatsAppFormList(67-259)
src/containers/WhatsAppForms/WhatsAppForms.tsx (3)
src/graphql/queries/WhatsAppForm.ts (2)
GET_WHATSAPP_FORM(3-19)LIST_FORM_CATEGORIES(21-25)src/graphql/mutations/WhatsAppForm.ts (3)
CREATE_FORM(3-15)UPDATE_FORM(17-29)DELETE_FORM(31-42)src/common/HelpData.tsx (1)
whatsappFormsInfo(143-146)
🪛 Biome (2.1.2)
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.test.tsx
[error] 15-16: Do not export from a test file.
(lint/suspicious/noExportsInTest)
⏰ 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: Codacy Static Code Analysis
- GitHub Check: glific (1.18.3-otp-27, 27.3.3)
- GitHub Check: CI
🔇 Additional comments (2)
src/containers/WhatsAppForms/WhatsAppForm.module.css (1)
1-75: Flow builder styles look consistent and self-containedThe new WhatsApp Flow builder styles are coherent, scoped via CSS modules, and won’t affect behavior. No issues from a code perspective.
src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.module.css (1)
1-151: WhatsApp form list styles are well-structured and scopedThe new list view styles (search bar, status badges, category tags, “more” menu, etc.) are cleanly organized and contained in the CSS module, with no apparent behavioral or maintainability issues.
| const [name, setName] = useState(''); | ||
| const [description, setDescription] = useState(''); | ||
| const [formJson, setFormJson] = useState(); | ||
| const [formCategories, setFormCategories] = useState([]); | ||
| const [categories, setCategories] = useState([]); | ||
| const [disabled, setDisabled] = useState(false); | ||
| const params = useParams(); |
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.
Fix formJson state typing and initialization.
The formJson state on line 30 lacks a type annotation and is initialized as undefined. This causes issues:
- Type safety: No TypeScript type checking for this critical state.
- Line 67 impact:
JSON.stringify(undefined)returns the string"undefined", not a valid JSON string, which will cause validation and backend errors. - Line 143-151 validation: The Yup validation expects a string but receives
undefinedinitially.
Apply this diff to fix the typing and initialization:
- const [formJson, setFormJson] = useState();
+ const [formJson, setFormJson] = useState<any>(null);And update line 67 to handle the null case:
const states = {
name,
- formJson: JSON.stringify(formJson),
+ formJson: formJson ? JSON.stringify(formJson) : '',
formCategories,
description,
};Alternatively, for better type safety, define a proper interface for the form JSON structure instead of using any.
Committable suggestion skipped: line range outside the PR's diff.
| useQuery(GET_WHATSAPP_FORM, { | ||
| skip: !params.id, | ||
| variables: { id: params.id }, | ||
| onCompleted: ({ whatsappForm }) => { | ||
| if (whatsappForm?.whatsappForm?.status === 'PUBLISHED') { | ||
| setDisabled(true); | ||
| } | ||
| }, | ||
| }); |
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.
Handle loading and error states for form data query.
The first useQuery hook fetches form data for editing but doesn't extract or handle loading or error states. This can cause issues:
- The form may render before data is loaded, showing empty fields briefly.
- Users won't see feedback if the query fails.
- The
setStatesfunction (line 82) is never called with this data, so the form fields won't be populated when editing.
Apply this diff to handle loading/error states and populate the form:
- useQuery(GET_WHATSAPP_FORM, {
+ const { loading: formLoading } = useQuery(GET_WHATSAPP_FORM, {
skip: !params.id,
variables: { id: params.id },
onCompleted: ({ whatsappForm }) => {
+ const form = whatsappForm?.whatsappForm;
+ if (form) {
+ setStates({
+ name: form.name,
+ definition: form.definition,
+ description: form.description,
+ categories: form.categories,
+ });
+ }
- if (whatsappForm?.whatsappForm?.status === 'PUBLISHED') {
+ if (form?.status === 'PUBLISHED') {
setDisabled(true);
}
},
});Then update the loading check on line 156:
- if (loading) {
+ if (loading || formLoading) {
return <Loading />;
}| const setStates = ({ name, definition, description, categories }: any) => { | ||
| setName(name); | ||
| setDescription(description); | ||
|
|
||
| setFormCategories(categories.map((c: string) => ({ id: c, name: c }))); | ||
|
|
||
| let parsedDefinition; | ||
| try { | ||
| parsedDefinition = JSON.parse(definition); | ||
| } catch (e) { | ||
| setLogs('Error parsing whatsapp form definition JSON:', 'error'); | ||
| parsedDefinition = definition; | ||
| } | ||
| setFormJson(parsedDefinition); | ||
| }; |
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.
Add type safety and null check for categories.
Two issues:
- The function parameter uses
anytype, losing TypeScript benefits. - Line 86 assumes
categoriesexists and is an array without validation.
Define an interface and add validation:
interface WhatsAppFormBackendData {
name: string;
definition: string;
description: string;
categories: string[];
}Apply this diff:
- const setStates = ({ name, definition, description, categories }: any) => {
+ const setStates = ({ name, definition, description, categories }: WhatsAppFormBackendData) => {
setName(name);
setDescription(description);
- setFormCategories(categories.map((c: string) => ({ id: c, name: c })));
+ if (categories && Array.isArray(categories)) {
+ setFormCategories(categories.map((c: string) => ({ id: c, name: c })));
+ }
let parsedDefinition;
try {
parsedDefinition = JSON.parse(definition);
} catch (e) {
setLogs('Error parsing whatsapp form definition JSON:', 'error');
parsedDefinition = definition;
}
setFormJson(parsedDefinition);
};🤖 Prompt for AI Agents
In src/containers/WhatsAppForms/WhatsAppForms.tsx around lines 82 to 96, the
setStates parameter is typed as any and categories is used without validation;
define a typed interface (e.g., WhatsAppFormBackendData with name:string,
definition:string, description:string, categories?: string[]) and change the
function signature to accept that type, then validate categories with
Array.isArray(categories) before mapping (otherwise setFormCategories([])); keep
the existing JSON.parse try/catch but ensure types align with the new interface.
* Add WhatsApp Form support with new interface and UI components * Enhance WhatsApp Form integration by updating button structure and handling in HSM and TemplateOptions components * Add Gupshup WhatsApp Form constant and update TemplateOptions for structured data collection * fix: test case * Add WhatsApp Form support with new interface and UI components * Enhance WhatsApp Form integration by updating button structure and handling in HSM and TemplateOptions components * Add Gupshup WhatsApp Form constant and update TemplateOptions for structured data collection * fix: css * fix: css * fix: add test case * Enhance WhatsApp Form handling by adding validation for form fields and displaying error messages * fix: update git checkout branch in cypress testing workflow * added dropdown for screen name * replaced '!!!' with '!' * updated cypress branch * fix: test case --------- Co-authored-by: priyanshu6238 <[email protected]>
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/containers/HSM/HSM.tsx (1)
360-375: Remove unusedsetTypeprop from TemplateOptions component instantiationThe review comment is verified as accurate.
setTypeis passed to theTemplateOptionscomponent at line 634 in HSM.tsx, butTemplateOptionsProps(lines 32-44 in TemplateOptions.tsx) does not declare this prop, and the component does not consume it. This causes a TypeScript type mismatch and represents unused code.Remove
setTypefrom the component props in HSM.tsx around line 634, as recommended.
♻️ Duplicate comments (1)
src/mocks/WhatsAppForm.tsx (1)
69-102: Fix publish/deactivate mutation mock shapes to match the GraphQL schemaBoth publish and deactivate mocks currently return the wrong structure and will not match the
PUBLISH_FORM/DEACTIVATE_FORMmutation definitions:
publishWhatsappFormshould wrap the payload underwhatsappFormand includeerrors.deactivateWhatsappFormis using the wrong field name (publishWhatsappForm), a flat payload, and a lowercaseinactivestatus instead ofINACTIVE.Align them with the schema like this:
export const publishWhatsappForm = { request: { query: PUBLISH_FORM, variables: { id: '3', }, }, result: { data: { - publishWhatsappForm: { - id: '1', - status: 'PUBLISHED', - }, + publishWhatsappForm: { + whatsappForm: { + id: '1', + status: 'PUBLISHED', + }, + errors: null, + }, }, }, }; export const deactivateWhatsappForm = { request: { query: DEACTIVATE_FORM, variables: { id: '3', }, }, result: { data: { - publishWhatsappForm: { - id: '1', - status: 'inactive', - __typename: 'WhatsappForm', - }, + deactivateWhatsappForm: { + whatsappForm: { + id: '1', + status: 'INACTIVE', + }, + errors: null, + }, }, }, };
🧹 Nitpick comments (19)
src/mocks/WhatsAppForm.tsx (1)
103-121: Tweak deactivate error mock message for clarity (optional)For
deactivateWhatsappFormError, consider updating the message to something like"Failed to deactivate"so logs and test failures clearly distinguish it from the publish path.export const deactivateWhatsappFormError = { request: { query: DEACTIVATE_FORM, variables: { id: '3', }, }, - error: new Error('Failed to publish'), + error: new Error('Failed to deactivate'), };src/containers/HSM/HSM.helper.ts (1)
1-88: WhatsApp form button mapping is consistent; consider tightening typesThe new WHATSAPP_FORM handling (
flow_id → form_idand template string[text, navigate_screen, form_id]) is consistent acrossgetTemplateAndButtonsandconvertButtonsToTemplateand should work as expected.You could optionally reduce
anyusage by wiringWhatsappFormTemplateinto the signatures where applicable (e.g., the WHATSAPP_FORM branch ofgetTemplateAndButtonsandconvertButtonsToTemplate) to catch shape regressions at compile time.src/containers/TemplateOptions/TemplateOptions.module.css (1)
1-3: New template option styles integrate cleanlyThe added container/wrapper/error classes and the tightened child selectors preserve existing behavior while giving the WhatsApp form template its own structured layout; nothing problematic from a CSS‑modules perspective.
If you find multiple modules defining generic
.Errorsin future, you might consider a more specific name, but it’s fine as-is here.Also applies to: 26-27, 31-37, 45-47, 55-56, 67-69, 110-112, 138-155
src/mocks/Template.tsx (1)
236-279: CREATE_SESSION_TEMPLATE_MOCK shape looks good; consider aligning translations typeThe mock matches the
CREATE_TEMPLATEmutation structure and correctly returns aWHATSAPP_FORM/FLOWbutton payload withbuttonType: 'WHATSAPP_FORM', which should exercise the new flow well. The only nit istranslations: []— other mocks in this file tend to use a JSON string (e.g.'{}') or serialized object, so you may want to align this for consistency with how the field is usually shaped in tests.src/containers/TemplateOptions/TemplateOptions.test.tsx (2)
18-30: Tests rely heavily on positional queries; prefer more specific selectorsBoth tests use
getAllByRole('textbox')/getAllByRole('radio')and index into the arrays to find controls. This is brittle against layout or field‑order changes. It would be more robust to:
- Target radios by label text (e.g., role
radiowith name /Phone number/).- Target textboxes by
name,placeholder, oraria-labelinstead of by index.
Also,await waitFor(() => {});is effectively a no‑op; either assert a condition insidewaitForor remove it.Also applies to: 32-57
59-77: “quick reply button” test doesn’t exercise Quick Reply pathThis test selects “Call to Action” in the button‑type dropdown but is named “renders quick reply button template”. It never selects the “Quick Reply” option or asserts quick‑reply‑specific behavior, so either:
- Rename the test to reflect the Call to Action path, or
- Change the selection to “Quick Reply” and assert the quick‑reply UI/behavior.
src/containers/HSM/HSM.test.tsx (3)
197-268: WhatsApp Form creation test is thorough but duplicates earlier test flowThe WhatsApp Form test is comprehensive and closely mirrors the basic “create template message” test (same setup, text entry, category selection, variable/ footer handling). Consider extracting a small helper that performs the shared setup (up to “Add buttons”) so this test only covers the WhatsApp‑specific combobox interactions and assertions.
81-89: Avoid combobox access by hard‑coded indicesMultiple tests retrieve comboboxes and other inputs via
getAllByRole('combobox')[N]or similar indexing. This is fragile as any UI change in field ordering will break these tests. Prefer:
getByLabelText('Select Button Type')for the button‑type dropdown,- Other
getByLabelText/findByLabelTextor data-testid selectors for subsequent selects,
so the tests depend on semantics rather than DOM order.Also applies to: 169-173, 297-300, 351-355, 419-431, 476-483
468-496: userEvent usage inside waitFor may be flakyIn the “should not allow adding more than 10 quick reply buttons” test,
user.click(addButton!)is called insidewaitForwithoutawait, and repeated in a loop. This mixesuserEvent’s async behavior withwaitForpolling and can introduce flakiness.
Consider simplifying to something like:for (let i = 0; i < 9; i += 1) { const addButton = await screen.findByText('Add Quick Reply'); await user.click(addButton); }and then assert the absence of the button.
src/containers/TemplateOptions/TemplateOptions.tsx (2)
79-97: LIST_WHATSAPP_FORMS query: consider minimal error/null‑safety handlingThe
useQuery(LIST_WHATSAPP_FORMS, { onCompleted })block correctly maps forms to{ label, id, definition }, but it assumesdata.listWhatsappFormsis always an array. If the backend ever returnsnullor an empty value,data.listWhatsappForms.mapwill throw.A small guard, e.g.
const list = data?.listWhatsappForms ?? []; setForms(list.map(...)), would make this more resilient without much extra code.
386-425: Header Autocomplete + tooltip wiring looks correct overallThe new “Select Button Type” Autocomplete and tooltip correctly bind to
buttonOptionsand usegetInfo(templateType?.id)to show contextual help for CALL_TO_ACTION, QUICK_REPLY, and WHATSAPP_FORM. GivendisableClearableis set,templateTypeshould always be one of the options, so the tooltip text will stay consistent with the selected button type.src/containers/HSM/HSM.tsx (8)
57-67: buttonTypes and buttonOptions correctly extended for WhatsApp FormAdding
WHATSAPP_FORMtobuttonTypesand exportingbuttonOptionswith{ id, label }objects aligns well with the new TemplateOptions UI and with theWhatsappFormTemplateshape (form_id, text, navigate_screen). This keeps button initialization and selection consistent across the HSM form.
82-85: templateButtons and templateType state changes are coherentWidening
templateButtonstoCallToActionTemplate | QuickReplyTemplate | WhatsappFormTemplateand storingtemplateTypeas a full option object (buttonOptions[0]) is a good direction and works with thetemplateType?.idchecks used elsewhere (payload, validation, simulator). This keeps the type/source of button templates centralized and consistent.Also applies to: 92-96
238-283: WHATSAPP_FORM payload generation in getButtonTemplatePayload looks correctThe new branch:
if (templateType?.id === 'WHATSAPP_FORM') { const obj = { type: 'FLOW', navigate_screen, text, flow_id: form_id, flow_action: 'NAVIGATE' }; result.push(obj); }matches what the mock (
CREATE_SESSION_TEMPLATE_MOCK) returns and what the backend is likely expecting for WhatsApp Form buttons (FLOW type with flow_id, navigate_screen, and text). ReusingbuttonType: templateType?.idkeeps the payload coherent with other button types.
333-345: setStates correctly maps persisted buttonType to buttonOptions entryWhen templates have
hasButtons,setStatesnow calls:setTemplateType(buttonOptions.find((btn: any) => btn.id === templateButtonType));This properly rehydrates the
templateTypeobject for CALL_TO_ACTION, QUICK_REPLY, or WHATSAPP_FORM so the UI and validation operate on the right branch. You might optionally guard with a fallback for unknown types, but the current behavior is fine if only these three values are ever stored.
421-429: addTemplateButtons correctly uses templateType.id but depends on templateType being set
addTemplateButtonsnow usesbuttonTypes[templateType?.id], which aligns with the newbuttonOptionsstructure. This works well withhandleTemplateTypeChangeand theuseEffectthat seeds default buttons whentemplateTypechanges.Just ensure
templateTypeis always one of the known options before callingaddTemplateButtons; the current guards (if (templateType?.id)) are sufficient as long astemplateTypeis never set to an arbitrary object.
457-479: Template button input handler change is consistent with new TemplateOptions contract
handeInputChangenow takes avalueinstead of an event object, and TemplateOptions calls it withe.target.value. The logic resetting the row foreventType === 'type'and otherwise mutatingobj[eventType]fits all three button types (CALL_TO_ACTION fields, QUICK_REPLY value, and WHATSAPP_FORM form_id/text/navigate_screen). This keeps templateButtons updates centralized.
793-812: Yup validation correctly extended for WHATSAPP_FORM buttonsThe lazy schema now includes:
else if (templateType?.id === 'WHATSAPP_FORM') { return Yup.object().shape({ form_id: Yup.string().required('Form is required.'), text: Yup.string().required('Button title is required.'), navigate_screen: Yup.string().required('Screen is required.'), }); }This matches the WhatsApp Form template shape and ensures incomplete button definitions are blocked before submission. The use of
templateType?.idis consistent with the rest of the component.
845-849: Simulator message updates correctly consider new templateType shapeThe effects that:
- add default buttons when
templateTypechanges, and- convert
templateButtonsto text viaconvertButtonsToTemplate(templateButtons, templateType?.id)are coherent with the new object‑shaped
templateTypeand extendedconvertButtonsToTemplatehelper. This keeps preview text in sync with CALL_TO_ACTION, QUICK_REPLY, and WHATSAPP_FORM buttons.Also applies to: 861-865
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/common/RichEditor.tsx(1 hunks)src/common/constants.ts(1 hunks)src/containers/HSM/HSM.helper.ts(4 hunks)src/containers/HSM/HSM.test.tsx(8 hunks)src/containers/HSM/HSM.tsx(15 hunks)src/containers/TemplateOptions/TemplateOptions.module.css(7 hunks)src/containers/TemplateOptions/TemplateOptions.test.tsx(2 hunks)src/containers/TemplateOptions/TemplateOptions.tsx(12 hunks)src/mocks/Template.tsx(1 hunks)src/mocks/WhatsAppForm.tsx(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/common/constants.tssrc/common/RichEditor.tsxsrc/containers/HSM/HSM.helper.tssrc/mocks/Template.tsxsrc/containers/HSM/HSM.test.tsxsrc/containers/TemplateOptions/TemplateOptions.tsxsrc/containers/HSM/HSM.tsxsrc/containers/TemplateOptions/TemplateOptions.test.tsxsrc/mocks/WhatsAppForm.tsx
🧬 Code graph analysis (7)
src/containers/HSM/HSM.helper.ts (1)
src/common/constants.ts (1)
WHATSAPP_FORM(183-183)
src/mocks/Template.tsx (1)
src/graphql/mutations/Template.ts (1)
CREATE_TEMPLATE(14-47)
src/containers/HSM/HSM.test.tsx (2)
src/mocks/WhatsAppForm.tsx (1)
WHATSAPP_FORM_MOCKS(298-308)src/mocks/Template.tsx (1)
CREATE_SESSION_TEMPLATE_MOCK(236-279)
src/containers/TemplateOptions/TemplateOptions.tsx (3)
src/common/constants.ts (6)
CALL_TO_ACTION(180-180)GUPSHUP_CALL_TO_ACTION(170-171)QUICK_REPLY(182-182)GUPSHUP_QUICK_REPLY(174-175)WHATSAPP_FORM(183-183)GUPSHUP_WHATSAPP_FORM(177-177)src/graphql/queries/WhatsAppForm.ts (1)
LIST_WHATSAPP_FORMS(27-39)src/containers/HSM/HSM.tsx (1)
buttonOptions(63-67)
src/containers/HSM/HSM.tsx (3)
src/containers/HSM/HSM.helper.ts (4)
CallToActionTemplate(3-7)QuickReplyTemplate(9-11)WhatsappFormTemplate(13-17)convertButtonsToTemplate(37-50)src/containers/InteractiveMessage/InteractiveOptions/QuickReplyTemplate.tsx (1)
QuickReplyTemplate(17-78)src/common/constants.ts (2)
CALL_TO_ACTION(180-180)QUICK_REPLY(182-182)
src/containers/TemplateOptions/TemplateOptions.test.tsx (2)
src/mocks/WhatsAppForm.tsx (1)
WHATSAPP_FORM_MOCKS(298-308)src/containers/HSM/HSM.tsx (1)
HSM(69-919)
src/mocks/WhatsAppForm.tsx (2)
src/graphql/queries/WhatsAppForm.ts (3)
LIST_FORM_CATEGORIES(21-25)LIST_WHATSAPP_FORMS(27-39)GET_WHATSAPP_FORM(3-19)src/graphql/mutations/WhatsAppForm.ts (4)
CREATE_FORM(3-15)PUBLISH_FORM(44-56)DEACTIVATE_FORM(58-70)UPDATE_FORM(17-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: glific (1.18.3-otp-27, 27.3.3)
- GitHub Check: build
- GitHub Check: CI
🔇 Additional comments (4)
src/common/RichEditor.tsx (1)
137-155: Icon selection for call-to-action buttons looks goodThe added digit check to render
CallIconfor phone-like values cleanly complements the existing link handling and keeps the behavior explicit without introducing regressions.src/common/constants.ts (1)
177-184: WhatsApp form constants are consistent and well-placed
GUPSHUP_WHATSAPP_FORMandWHATSAPP_FORMalign with existing naming and are positioned correctly next to other template-type constants.src/containers/TemplateOptions/TemplateOptions.test.tsx (1)
1-16: Ensure HSM has all required GraphQL mocks in this wrapperThe wrapper only injects
WHATSAPP_FORM_MOCKSintoMockedProvider, butHSMalso fires template, tags, languages, and categories queries. Please double‑check thatWHATSAPP_FORM_MOCKS(or upstream defaults) cover all of those; otherwise these tests may intermittently error with missing mock warnings.src/containers/HSM/HSM.test.tsx (1)
94-101: Good consolidation of mocks for add modeMerging
HSM_TEMPLATE_MOCKS,WHATSAPP_FORM_MOCKS, andCREATE_SESSION_TEMPLATE_MOCKinto a singleMOCKSarray is a clean way to cover all GraphQL interactions for add mode, including WhatsApp forms and template creation.
| import { | ||
| GUPSHUP_CALL_TO_ACTION, | ||
| GUPSHUP_QUICK_REPLY, | ||
| CALL_TO_ACTION, | ||
| QUICK_REPLY, | ||
| WHATSAPP_FORM, | ||
| GUPSHUP_WHATSAPP_FORM, | ||
| } from 'common/constants'; | ||
| import styles from './TemplateOptions.module.css'; | ||
| import { Fragment } from 'react'; | ||
| import { Fragment, useState } from 'react'; | ||
| import { buttonOptions } from 'containers/HSM/HSM'; | ||
| import { useQuery } from '@apollo/client'; | ||
| import { LIST_WHATSAPP_FORMS } from 'graphql/queries/WhatsAppForm'; | ||
|
|
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.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Avoid circular dependency between HSM and TemplateOptions via buttonOptions
TemplateOptions imports buttonOptions from containers/HSM/HSM, while HSM imports TemplateOptions. This creates a circular module dependency which can lead to temporal‑dead‑zone issues and makes the module graph harder to reason about.
Consider moving buttonOptions (and the associated type) into a small shared module (e.g., containers/HSM/buttonOptions.ts or a constants file) and importing it from both HSM and TemplateOptions instead.
🏁 Script executed:
#!/bin/bash
# Verify circular dependency: search for buttonOptions import in TemplateOptions
echo "=== Searching for buttonOptions import in TemplateOptions.tsx ==="
rg -n "buttonOptions" "src/containers/TemplateOptions/TemplateOptions.tsx" -A2 -B2
echo ""
echo "=== Searching for all imports from HSM in TemplateOptions ==="
rg -n "from.*HSM" "src/containers/TemplateOptions/TemplateOptions.tsx" -A2 -B2
echo ""
echo "=== Searching for TemplateOptions import in HSM file ==="
rg -n "TemplateOptions" "src/containers/HSM/HSM.tsx" -A2 -B2
rg -n "TemplateOptions" "src/containers/HSM/HSM.ts" -A2 -B2
echo ""
echo "=== Finding HSM file location ==="
fd -t f "HSM\." "src/containers/HSM/"
echo ""
echo "=== Checking buttonOptions definition ==="
rg -n "buttonOptions\s*=" "src/containers/HSM/" -B5 -A5
Length of output: 1743
🏁 Script executed:
#!/bin/bash
# Search for buttonOptions more broadly
echo "=== Search for buttonOptions definition across codebase ==="
rg -n "export.*buttonOptions|buttonOptions\s*=" "src/" -B3 -A3
echo ""
echo "=== Check HSM.tsx exports around line 28 and before TemplateOptions import ==="
rg -n "export" "src/containers/HSM/HSM.tsx" -B2 -A2 | head -50
echo ""
echo "=== Check import structure of HSM.tsx ==="
head -100 "src/containers/HSM/HSM.tsx"Length of output: 5562
Move buttonOptions to a shared module to break circular dependency
Circular dependency confirmed: TemplateOptions imports buttonOptions from containers/HSM/HSM, while HSM imports TemplateOptions (line 21). This creates a module cycle that can lead to initialization issues and makes the dependency graph fragile.
buttonOptions (defined at HSM.tsx line 63) is a simple constant array and suitable for extraction. Move it to a shared module such as containers/HSM/buttonOptions.ts or a constants file, then import it in both files to cleanly resolve the circular dependency.
🤖 Prompt for AI Agents
In src/containers/TemplateOptions/TemplateOptions.tsx around lines 18 to 31,
TemplateOptions currently imports buttonOptions from containers/HSM/HSM which
creates a circular dependency because HSM imports TemplateOptions; extract the
buttonOptions constant (defined in HSM at line ~63) into a new shared module
(e.g., src/containers/HSM/buttonOptions.ts or a common constants file), export
it from there, then update both HSM and TemplateOptions to import buttonOptions
from the new module to break the circular import.
| if (templateType?.id === WHATSAPP_FORM) { | ||
| template = ( | ||
| <div className={styles.WhatsappFormTemplateWrapper}> | ||
| <div> | ||
| <Autocomplete | ||
| options={forms} | ||
| value={forms.find((form: any) => form.id === form_id) || null} | ||
| renderInput={(params) => <TextField {...params} label="Select Form " />} | ||
| onChange={(event: any, newValue: any) => { | ||
| onInputChange(newValue.id, row, index, 'form_id'); | ||
|
|
||
| try { | ||
| const definition = JSON.parse(newValue.definition); | ||
| const screenNames = definition.screens.map((screen: any) => screen.id); | ||
| setScreens(screenNames.map((screen: string) => ({ label: screen, id: screen }))); | ||
| } catch (e) { | ||
| setScreens([]); | ||
| console.error('Error parsing form definition:', e); | ||
| } | ||
| }} | ||
| disabled={disabled} | ||
| /> | ||
| {errors.templateButtons && touched.templateButtons && touched.templateButtons[index] ? ( | ||
| <p className={styles.Errors}>{errors.templateButtons[index]?.form_id}</p> | ||
| ) : null} | ||
| </div> | ||
|
|
||
| <div> | ||
| <Autocomplete | ||
| options={screens} | ||
| value={navigate_screen} | ||
| renderInput={(params) => <TextField {...params} label="Screen Name" />} | ||
| onChange={(event: any, newValue: any) => { | ||
| console.log(newValue); | ||
| onInputChange(newValue.id, row, index, 'navigate_screen'); | ||
| }} | ||
| disabled={disabled || !form_id} | ||
| /> | ||
|
|
||
| {errors.templateButtons && touched.templateButtons && touched.templateButtons[index] ? ( | ||
| <p className={styles.Errors}>{errors.templateButtons[index]?.navigate_screen}</p> | ||
| ) : null} | ||
| </div> | ||
|
|
||
| <div> | ||
| <TextField | ||
| value={text} | ||
| title={title} | ||
| placeholder={`Button Title`} | ||
| variant="outlined" | ||
| onChange={(e: any) => onInputChange(e.target.value, row, index, 'text')} | ||
| className={styles.TextField} | ||
| error={isError('value')} | ||
| disabled={disabled} | ||
| /> | ||
| {errors.templateButtons && touched.templateButtons && touched.templateButtons[index] ? ( | ||
| <p className={styles.Errors}>{errors.templateButtons[index]?.text}</p> | ||
| ) : null} | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
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.
🧩 Analysis chain
WhatsApp Form Autocomplete block has null‑handling and value‑type issues
In the WHATSAPP_FORM branch:
onChange={(event, newValue) => { onInputChange(newValue.id, ...) ... }}assumesnewValueis always non‑null. If the user clears the selection, this will throw.- The screens Autocomplete uses
options={screens}butvalue={navigate_screen}, which is a string. MUI Autocomplete expects the value to be either one of the option objects ornull; this can produce inconsistent UI and warnings. - The Button Title
TextFielduseserror={isError('value')}while the actual error message iserrors.templateButtons[index]?.text, so the red error state may not match validation errors ontext.
Suggested adjustments:
// Form selection
onChange={(event: any, newValue: any) => {
if (!newValue) {
onInputChange('', row, index, 'form_id');
setScreens([]);
return;
}
onInputChange(newValue.id, row, index, 'form_id');
try {
const definition = JSON.parse(newValue.definition);
const screenNames = definition.screens.map((screen: any) => screen.id);
setScreens(screenNames.map((screen: string) => ({ label: screen, id: screen })));
} catch {
setScreens([]);
}
}}
// Screens Autocomplete
<Autocomplete
options={screens}
value={screens.find((s: any) => s.id === navigate_screen) || null}
...
onChange={(event: any, newValue: any) => {
onInputChange(newValue ? newValue.id : '', row, index, 'navigate_screen');
}}
/>
// Button title error flag
<TextField
...
error={isError('text')}
/>This keeps the stored form state as IDs while satisfying the Autocomplete’s expectations and aligns the error prop with the validated field.
---
🏁 Script executed:
```shell
#!/bin/bash
# Inspect the component file to understand state management and function definitions
cat src/containers/TemplateOptions/TemplateOptions.tsx | head -400 | tail -200
Length of output: 8176
🏁 Script executed:
#!/bin/bash
# Search for onInputChange function definition
rg -n "onInputChange" src/containers/TemplateOptions/TemplateOptions.tsx -B3 -A8Length of output: 5083
🏁 Script executed:
#!/bin/bash
# Search for isError function definition
rg -n "isError" src/containers/TemplateOptions/TemplateOptions.tsx -B3 -A5Length of output: 4437
🏁 Script executed:
#!/bin/bash
# Search for state declarations (form_id, navigate_screen, screens)
rg -n "form_id|navigate_screen|setScreens|useState" src/containers/TemplateOptions/TemplateOptions.tsx -B2 -A2Length of output: 2530
🏁 Script executed:
#!/bin/bash
# Get the full component structure and hooks at the top
head -100 src/containers/TemplateOptions/TemplateOptions.tsxLength of output: 2677
Null-handling and type mismatches confirmed in WhatsApp Form section—fixes required
All three concerns are legitimate:
- Line 330 null crash:
onChangeaccessesnewValue.idwithout null check. Clearing the form selection will throw. - Line 351 type mismatch:
value={navigate_screen}is a string, butoptions={screens}contains objects{ label, id }. MUI Autocomplete requires value to match option type. - Line 373 error flag mismatch: Uses
error={isError('value')}but the field is fortextand the actual validation error iserrors.templateButtons[index]?.text(line 377).
Also line 355 accesses newValue.id without null checking in the screens Autocomplete onChange.
// Form selection—add null guard
onChange={(event: any, newValue: any) => {
if (!newValue) {
onInputChange('', row, index, 'form_id');
setScreens([]);
return;
}
onInputChange(newValue.id, row, index, 'form_id');
try {
const definition = JSON.parse(newValue.definition);
const screenNames = definition.screens.map((screen: any) => screen.id);
setScreens(screenNames.map((screen: string) => ({ label: screen, id: screen })));
} catch (e) {
setScreens([]);
console.error('Error parsing form definition:', e);
}
}}
// Screens—match value type to options and add null guard
value={screens.find((s: any) => s.id === navigate_screen) || null}
onChange={(event: any, newValue: any) => {
onInputChange(newValue ? newValue.id : '', row, index, 'navigate_screen');
}}
// Button title—fix error prop
error={isError('text')}🤖 Prompt for AI Agents
In src/containers/TemplateOptions/TemplateOptions.tsx around lines 321-382, the
WhatsApp form block can crash or misbehave: guard against null newValue in the
form Autocomplete onChange (when selection is cleared) by setting form_id to ''
and clearing screens; parse definition only when newValue is present; for the
screens Autocomplete make value match option type by using the object from
screens (find by id) or null and guard its onChange to handle newValue === null
(set navigate_screen to ''); and fix the TextField error prop to call
isError('text') so it reflects the text validation error.
Summary by CodeRabbit
New Features
Tests
UI & Translations
✏️ Tip: You can customize this high-level summary in your review settings.