Fix/issue 3794 whatsapp flow components#3842
Fix/issue 3794 whatsapp flow components#3842khushthecoder wants to merge 2 commits intoglific:masterfrom
Conversation
WalkthroughThis PR adds support for eight previously unhandled WhatsApp Flow component types (CalendarPicker, ChipsSelector, EmbeddedLink, RichText, If, Switch, PhotoPicker, DocumentPicker) across the form builder. It introduces preservation of flow metadata by adding Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
11d986b to
1b66f4b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.utils.test.ts (1)
406-410: Avoid substring-based error filtering in validation assertions.Filtering by
navigate/terminalmessage text is brittle and can mask unrelated regressions. Prefer a fixture that is valid for navigation rules and assertresult.errorsdirectly for this test’s intent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.utils.test.ts` around lines 406 - 410, The test currently filters result.errors by substrings 'navigate' and 'terminal' into componentErrors before asserting zero, which is brittle; update the test so it uses a navigation-valid fixture (or adjust the fixture data used by the test helper) and assert on result.errors directly (e.g., expect(result.errors).toHaveLength(0)) instead of creating componentErrors; locate the assertion in FormBuilder.utils.test.ts that defines componentErrors and replace the filtering logic with a direct assertion against result.errors, ensuring the fixture covers navigation rules so no navigation-related errors appear.src/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.types.ts (1)
10-10: Replace newanyusages with safer structural types.Line 10 and Line 31 introduce
any, which weakens strict-mode guarantees in newly added preservation fields. Preferunknown-based structural types.♻️ Suggested typing update
+type FlowDataMap = Record<string, unknown>; +type RawFlowComponent = Record<string, unknown>; + export interface Screen { @@ - flowData?: Record<string, any>; + flowData?: FlowDataMap; } @@ export interface ContentItemData { @@ - rawComponent?: any; + rawComponent?: RawFlowComponent; }As per coding guidelines,
src/**/*.{ts,tsx}should use TypeScript strict mode with React 19 and Vite.Also applies to: 30-31
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.types.ts` at line 10, The flowData field currently typed as Record<string, any> (and other newly added "preservation" fields that use any) weakens strict-mode guarantees; change these to safe unknown-based structural types such as Record<string, unknown> or more specific interfaces/unions that reflect the expected shape (or use unknown[] / Array<Record<string, unknown>> for collections), update any consumers to narrow/cast the unknown at use sites, and ensure the declarations (e.g., flowData and the other preservation fields referenced) are replaced accordingly so the module adheres to TypeScript strict-mode rules.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/containers/WhatsAppForms/Configure/FormBuilder/ContentItem/ContentItemComponent.tsx`:
- Around line 61-63: The two hardcoded user-facing strings in the
ContentItemComponent must be replaced with i18n keys using react-i18next; import
useTranslation and call const { t } = useTranslation() inside the
ContentItemComponent, then replace the literal "This component
(<strong>{item.name}</strong>) is view-only and will be preserved on export."
with t('whatsappForms.contentItem.viewOnly', { name: item.name }) (using
HTML-safe rendering or interpolation as existing code does) and replace the
other literal at line 69 with a separate key such as
t('whatsappForms.contentItem.someOtherKey'); add those keys to the locale
resource files for extraction.
In
`@src/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.utils.test.ts`:
- Around line 112-126: The test fixture contains an object with type 'If' that
includes a schema-required then property which triggers Biome's noThenProperty
rule; add an inline suppression comment immediately before the then key in that
object (the object with type: 'If' and condition: '${data.cal_date}') using the
Biome ignore syntax (e.g., /* biome-ignore noThenProperty */) so the required
then array remains and the lint rule is silenced.
---
Nitpick comments:
In `@src/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.types.ts`:
- Line 10: The flowData field currently typed as Record<string, any> (and other
newly added "preservation" fields that use any) weakens strict-mode guarantees;
change these to safe unknown-based structural types such as Record<string,
unknown> or more specific interfaces/unions that reflect the expected shape (or
use unknown[] / Array<Record<string, unknown>> for collections), update any
consumers to narrow/cast the unknown at use sites, and ensure the declarations
(e.g., flowData and the other preservation fields referenced) are replaced
accordingly so the module adheres to TypeScript strict-mode rules.
In
`@src/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.utils.test.ts`:
- Around line 406-410: The test currently filters result.errors by substrings
'navigate' and 'terminal' into componentErrors before asserting zero, which is
brittle; update the test so it uses a navigation-valid fixture (or adjust the
fixture data used by the test helper) and assert on result.errors directly
(e.g., expect(result.errors).toHaveLength(0)) instead of creating
componentErrors; locate the assertion in FormBuilder.utils.test.ts that defines
componentErrors and replace the filtering logic with a direct assertion against
result.errors, ensuring the fixture covers navigation rules so no
navigation-related errors appear.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1868d58f-1018-44f3-a3cf-3800946dbc25
📒 Files selected for processing (9)
src/containers/Trigger/Trigger.tsxsrc/containers/WhatsAppForms/Configure/Configure.test.tsxsrc/containers/WhatsAppForms/Configure/FormBuilder/ContentItem/ContentItemComponent.module.csssrc/containers/WhatsAppForms/Configure/FormBuilder/ContentItem/ContentItemComponent.tsxsrc/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.types.tssrc/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.utils.test.tssrc/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.utils.tssrc/i18n/en/en.jsonsrc/i18n/hi/hi.json
| This component (<strong>{item.name}</strong>) is view-only and will be preserved on | ||
| export. | ||
| </span> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/containers/WhatsAppForms/Configure/FormBuilder/ContentItem/ContentItemComponent.tsx | head -80Repository: glific/glific-frontend
Length of output: 3663
🏁 Script executed:
# Check if this file has any i18next imports
rg "useTranslation|i18next" src/containers/WhatsAppForms/Configure/FormBuilder/ContentItem/ContentItemComponent.tsxRepository: glific/glific-frontend
Length of output: 48
🏁 Script executed:
# Check other component files in the same directory for i18next pattern
ls -la src/containers/WhatsAppForms/Configure/FormBuilder/ContentItem/Repository: glific/glific-frontend
Length of output: 387
🏁 Script executed:
# Check sibling components to see how they handle localization
rg "useTranslation" src/containers/WhatsAppForms/Configure/FormBuilder/ -l | head -5Repository: glific/glific-frontend
Length of output: 48
🏁 Script executed:
# Check which files in FormBuilder use useTranslation
rg "useTranslation" src/containers/WhatsAppForms/Configure/FormBuilder/ -lRepository: glific/glific-frontend
Length of output: 48
🏁 Script executed:
# Check content of one of the types to see if they use i18next
cat -n src/containers/WhatsAppForms/Configure/FormBuilder/ContentItem/types/TextContent.tsx | head -50Repository: glific/glific-frontend
Length of output: 2112
🏁 Script executed:
# Check how widespread i18next adoption is in src/containers
rg "useTranslation" src/containers/ -l | wc -lRepository: glific/glific-frontend
Length of output: 69
🏁 Script executed:
# Sample a few files that do use i18next to confirm pattern
rg "useTranslation" src/containers/ -l | head -3 | xargs -I {} sh -c 'echo "=== {} ===" && head -15 {}'Repository: glific/glific-frontend
Length of output: 2222
Localize hardcoded user-facing strings via i18next.
This component adds two hardcoded strings at lines 61–63 and 69 that need i18n support. The codebase standardly uses useTranslation() from react-i18next across 100+ files in src/containers, and these strings must follow that pattern for extraction to Lokalise.
Suggested fix
+import { useTranslation } from 'react-i18next';
export const ContentItemComponent = ({
item,
isExpanded,
onToggleExpanded,
onDelete,
onUpdate,
isViewOnly = false,
}: ContentItemComponentProps) => {
+ const { t } = useTranslation();
const { attributes, listeners, setNodeRef, transform, transition, isDragging } = useSortable({
id: item.id,
});
// ...
case 'Unsupported':
return (
<div className={styles.UnsupportedContent} data-testid="unsupported-content">
<div className={styles.UnsupportedInfo}>
<InfoOutlinedIcon fontSize="small" style={{ color: '#666' }} />
<span>
- This component (<strong>{item.name}</strong>) is view-only and will be preserved on
- export.
+ {t('This component ({{name}}) is view-only and will be preserved on export.', {
+ name: item.name,
+ })}
</span>
</div>
{(item.data.rawComponent?.type === 'PhotoPicker' ||
item.data.rawComponent?.type === 'DocumentPicker') && (
<div className={styles.EndpointWarning} data-testid="endpoint-warning">
<WarningIcon fontSize="small" style={{ color: '#ed6c02' }} />
- <span>This component requires an upload endpoint to be configured.</span>
+ <span>{t('This component requires an upload endpoint to be configured.')}</span>
</div>
)}
</div>
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/containers/WhatsAppForms/Configure/FormBuilder/ContentItem/ContentItemComponent.tsx`
around lines 61 - 63, The two hardcoded user-facing strings in the
ContentItemComponent must be replaced with i18n keys using react-i18next; import
useTranslation and call const { t } = useTranslation() inside the
ContentItemComponent, then replace the literal "This component
(<strong>{item.name}</strong>) is view-only and will be preserved on export."
with t('whatsappForms.contentItem.viewOnly', { name: item.name }) (using
HTML-safe rendering or interpolation as existing code does) and replace the
other literal at line 69 with a separate key such as
t('whatsappForms.contentItem.someOtherKey'); add those keys to the locale
resource files for extraction.
src/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.utils.test.ts
Outdated
Show resolved
Hide resolved
…ed UI, and extended validation Building on PR glific#3819's upstream unsupported component handling: - Add flowId/flowData to Screen type for round-trip metadata - Replace generateUniqueScreenIds with buildScreenIds to preserve original IDs - Deep-copy rawComponent on import to prevent mutation - Merge preserved flowData with generated data on export (preserved wins) - Store flowId and flowData during flow JSON import - Extend INPUT_COMPONENT_TYPES with CalendarPicker, ChipsSelector, PhotoPicker, DocumentPicker - Enhance Unsupported UI with CSS classes, InfoOutlinedIcon, and endpoint warning for PhotoPicker/DocumentPicker - Add 24 unit tests for round-trip preservation - Update 1 existing test for new ID preservation behavior
1b66f4b to
22775b4
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.utils.test.ts (1)
35-35:⚠️ Potential issue | 🟡 MinorAdd Biome suppression for schema-required
thenkey.Line 35 uses a WhatsApp Flow schema key (
then) that triggerslint/suspicious/noThenProperty; this test fixture should be explicitly suppressed.Suggested fix
- { type: 'If', condition: '${data.flag}', then: [{ type: 'TextBody', text: 'Yes' }], else: [{ type: 'TextBody', text: 'No' }] }, + { + type: 'If', + condition: '${data.flag}', + // biome-ignore lint/suspicious/noThenProperty: Required by WhatsApp Flow schema. + then: [{ type: 'TextBody', text: 'Yes' }], + else: [{ type: 'TextBody', text: 'No' }], + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.utils.test.ts` at line 35, The test fixture object with { type: 'If', condition: '${data.flag}', then: [...], else: [...] } in FormBuilder.utils.test.ts triggers the biome lint rule lint/suspicious/noThenProperty; add a Biome suppression comment to silence this rule for the fixture (for example add a // biome-ignore lint/suspicious/noThenProperty or /* biome-ignore lint/suspicious/noThenProperty */ immediately above the object or at the top of the test file) so the schema-required then key on the If object is allowed by the linter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.utils.test.ts`:
- Around line 2-8: Replace the relative import of the utility module with the
project's src alias: update the import that currently pulls
'./FormBuilder.utils' so it uses the configured src alias path (e.g. import {
convertFlowJSONToFormBuilder, convertFormBuilderToFlowJSON, computeFieldNames,
hasContentItemError, validateFlowJson } from
'src/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.utils'), leaving
the exported symbol names unchanged.
In `@src/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.utils.ts`:
- Around line 103-127: The buildScreenIds function currently emits preserved
flowId values verbatim which allows duplicate flowId entries when multiple
screens share the same flowId; update buildScreenIds to check for duplicates
before pushing a preserved flowId: when iterating screens (inside
buildScreenIds), if screen.flowId is present but already exists in usedIds,
generate a unique id (using toSnakeCaseId(screen.name) + fallback to
randomAlphaId or screen_<suffix>) and use that instead, while still adding the
chosen id to usedIds; ensure both the first-pass collection of preserved flowIds
and the second-pass emission respect uniqueness by de-duplicating preserved
flowIds via usedIds.
---
Duplicate comments:
In
`@src/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.utils.test.ts`:
- Line 35: The test fixture object with { type: 'If', condition: '${data.flag}',
then: [...], else: [...] } in FormBuilder.utils.test.ts triggers the biome lint
rule lint/suspicious/noThenProperty; add a Biome suppression comment to silence
this rule for the fixture (for example add a // biome-ignore
lint/suspicious/noThenProperty or /* biome-ignore lint/suspicious/noThenProperty
*/ immediately above the object or at the top of the test file) so the
schema-required then key on the If object is allowed by the linter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4e7b22bb-1d0f-4351-9de0-0ee71d868df7
📒 Files selected for processing (6)
src/containers/WhatsAppForms/Configure/Configure.test.tsxsrc/containers/WhatsAppForms/Configure/FormBuilder/ContentItem/ContentItemComponent.module.csssrc/containers/WhatsAppForms/Configure/FormBuilder/ContentItem/ContentItemComponent.tsxsrc/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.types.tssrc/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.utils.test.tssrc/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.utils.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/containers/WhatsAppForms/Configure/Configure.test.tsx
- src/containers/WhatsAppForms/Configure/FormBuilder/ContentItem/ContentItemComponent.tsx
- src/containers/WhatsAppForms/Configure/FormBuilder/ContentItem/ContentItemComponent.module.css
- src/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.types.ts
| import { | ||
| convertFlowJSONToFormBuilder, | ||
| convertFormBuilderToFlowJSON, | ||
| computeFieldNames, | ||
| hasContentItemError, | ||
| validateFlowJson, | ||
| } from './FormBuilder.utils'; |
There was a problem hiding this comment.
Use src alias imports instead of relative imports in tests.
Line 8 imports ./FormBuilder.utils; this should use the configured src-relative alias path.
Suggested fix
import {
convertFlowJSONToFormBuilder,
convertFormBuilderToFlowJSON,
computeFieldNames,
hasContentItemError,
validateFlowJson,
-} from './FormBuilder.utils';
+} from 'containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.utils';As per coding guidelines "Use import aliases relative to src/ with baseUrl configured in tsconfig.json instead of relative imports".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.utils.test.ts`
around lines 2 - 8, Replace the relative import of the utility module with the
project's src alias: update the import that currently pulls
'./FormBuilder.utils' so it uses the configured src alias path (e.g. import {
convertFlowJSONToFormBuilder, convertFormBuilderToFlowJSON, computeFieldNames,
hasContentItemError, validateFlowJson } from
'src/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.utils'), leaving
the exported symbol names unchanged.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| /** Builds screen IDs for export, preserving original flowId when available. */ | ||
| const buildScreenIds = (screens: Screen[]): string[] => { | ||
| const usedIds = new Set<string>(); | ||
| const ids: string[] = []; | ||
|
|
||
| // First pass: collect all preserved flowIds | ||
| screens.forEach((screen) => { | ||
| if (screen.flowId) { | ||
| usedIds.add(screen.flowId); | ||
| } | ||
| }); | ||
|
|
||
| // Second pass: assign IDs | ||
| screens.forEach((screen) => { | ||
| if (screen.flowId) { | ||
| ids.push(screen.flowId); | ||
| } else { | ||
| let id = toSnakeCaseId(screen.name); | ||
| if (!id || usedIds.has(id)) { | ||
| const suffix = randomAlphaId(); | ||
| id = id ? `${id}_${suffix}` : `screen_${suffix}`; | ||
| } | ||
| usedIds.add(id); | ||
| ids.push(id); | ||
| } |
There was a problem hiding this comment.
Duplicate preserved flowId values can produce invalid exported flows.
At Line 117-Line 119, every preserved flowId is emitted as-is. If two screens share the same flowId (e.g., duplicated screen/edit flow), export can contain duplicate screen IDs and broken navigation targets.
Suggested fix
const buildScreenIds = (screens: Screen[]): string[] => {
const usedIds = new Set<string>();
- const ids: string[] = [];
-
- // First pass: collect all preserved flowIds
- screens.forEach((screen) => {
- if (screen.flowId) {
- usedIds.add(screen.flowId);
- }
- });
-
- // Second pass: assign IDs
- screens.forEach((screen) => {
- if (screen.flowId) {
- ids.push(screen.flowId);
- } else {
- let id = toSnakeCaseId(screen.name);
- if (!id || usedIds.has(id)) {
- const suffix = randomAlphaId();
- id = id ? `${id}_${suffix}` : `screen_${suffix}`;
- }
- usedIds.add(id);
- ids.push(id);
- }
- });
-
- return ids;
+ return screens.map((screen) => {
+ const preservedId = screen.flowId?.trim();
+ if (preservedId && !usedIds.has(preservedId)) {
+ usedIds.add(preservedId);
+ return preservedId;
+ }
+
+ const base = toSnakeCaseId(screen.name) || 'screen';
+ let id = base;
+ while (usedIds.has(id)) {
+ id = `${base}_${randomAlphaId()}`;
+ }
+ usedIds.add(id);
+ return id;
+ });
};📝 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.
| /** Builds screen IDs for export, preserving original flowId when available. */ | |
| const buildScreenIds = (screens: Screen[]): string[] => { | |
| const usedIds = new Set<string>(); | |
| const ids: string[] = []; | |
| // First pass: collect all preserved flowIds | |
| screens.forEach((screen) => { | |
| if (screen.flowId) { | |
| usedIds.add(screen.flowId); | |
| } | |
| }); | |
| // Second pass: assign IDs | |
| screens.forEach((screen) => { | |
| if (screen.flowId) { | |
| ids.push(screen.flowId); | |
| } else { | |
| let id = toSnakeCaseId(screen.name); | |
| if (!id || usedIds.has(id)) { | |
| const suffix = randomAlphaId(); | |
| id = id ? `${id}_${suffix}` : `screen_${suffix}`; | |
| } | |
| usedIds.add(id); | |
| ids.push(id); | |
| } | |
| /** Builds screen IDs for export, preserving original flowId when available. */ | |
| const buildScreenIds = (screens: Screen[]): string[] => { | |
| const usedIds = new Set<string>(); | |
| return screens.map((screen) => { | |
| const preservedId = screen.flowId?.trim(); | |
| if (preservedId && !usedIds.has(preservedId)) { | |
| usedIds.add(preservedId); | |
| return preservedId; | |
| } | |
| const base = toSnakeCaseId(screen.name) || 'screen'; | |
| let id = base; | |
| while (usedIds.has(id)) { | |
| id = `${base}_${randomAlphaId()}`; | |
| } | |
| usedIds.add(id); | |
| return id; | |
| }); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.utils.ts`
around lines 103 - 127, The buildScreenIds function currently emits preserved
flowId values verbatim which allows duplicate flowId entries when multiple
screens share the same flowId; update buildScreenIds to check for duplicates
before pushing a preserved flowId: when iterating screens (inside
buildScreenIds), if screen.flowId is present but already exists in usedIds,
generate a unique id (using toSnakeCaseId(screen.name) + fallback to
randomAlphaId or screen_<suffix>) and use that instead, while still adding the
chosen id to usedIds; ensure both the first-pass collection of preserved flowIds
and the second-pass emission respect uniqueness by de-duplicating preserved
flowIds via usedIds.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.utils.test.ts (1)
6-12:⚠️ Potential issue | 🟡 MinorUse the configured
srcalias import instead of a relative path.At Line 12,
./FormBuilder.utilsshould use the project alias path to match repository standards.Suggested fix
import { convertFlowJSONToFormBuilder, convertFormBuilderToFlowJSON, computeFieldNames, hasContentItemError, validateFlowJson, -} from './FormBuilder.utils'; +} from 'containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.utils';As per coding guidelines "Use import aliases relative to
src/withbaseUrlconfigured in tsconfig.json instead of relative imports".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.utils.test.ts` around lines 6 - 12, The import in the test uses a relative path for FormBuilder.utils; replace the relative import './FormBuilder.utils' with the project src alias import (i.e., import the same named exports convertFlowJSONToFormBuilder, convertFormBuilderToFlowJSON, computeFieldNames, hasContentItemError, validateFlowJson from the configured 'src/...' alias) so the module is imported via the repository's src alias rather than a relative path.
🧹 Nitpick comments (2)
src/components/UI/SearchBar/SearchBar.tsx (1)
22-22: NarrowsearchParamand pass a real boolean toAdvancedSearch.
searchParam?: anydrops strict typing right where this component assumes object semantics, and the current&&expression can handisActivea non-boolean value. Please reuse the actual filter type here, or at least narrow it toRecord<string, unknown>and deriveisActivefromsearchParam ?? {}.Proposed cleanup
export interface SearchBarProps { handleChange?: (e: any) => void; handleSubmit?: (e: React.FormEvent<HTMLFormElement>) => void; onReset: () => void; searchVal?: string; className?: any; handleClick?: any; endAdornment?: any; searchMode: boolean; iconFront?: boolean; - searchParam?: any; + searchParam?: Record<string, unknown>; } export const SearchBar = ({ searchMode, searchVal = '', onReset, endAdornment, handleClick, handleSubmit, handleChange, className, iconFront = false, searchParam, }: SearchBarProps) => { const [localSearchValue, setLocalSearchValue] = useState(searchVal); const { t } = useTranslation(); + const isAdvancedSearchActive = Object.keys(searchParam ?? {}).length > 0;- <AdvancedSearch isActive={searchParam && Object.keys(searchParam).length !== 0} /> + <AdvancedSearch isActive={isAdvancedSearchActive} />As per coding guidelines,
src/**/*.{ts,tsx}: Use TypeScript strict mode with React 19 and Vite.Also applies to: 35-35, 81-81
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/UI/SearchBar/SearchBar.tsx` at line 22, The prop searchParam in SearchBar.tsx is currently typed as any and can pass non-boolean values into AdvancedSearch's isActive; change searchParam's type to a narrower type (prefer the real filter/interface used in the app or at minimum Record<string, unknown>) and compute a real boolean for AdvancedSearch (e.g., derive isActive from searchParam ?? {} by checking keys or a known flag such as Object.keys(searchParam ?? {}).length > 0 or Boolean(searchParam?.someFlag)), then pass that boolean into <AdvancedSearch isActive={...}>; update any other occurrences (lines referenced around 35 and 81) to use the same typed prop and boolean derivation.src/containers/AskMeBot/AskMeBot.module.css (1)
99-106: Consider usingoverflow-wrapinstead ofword-wrapfor consistency.At Line 104,
word-wrapis a legacy alias foroverflow-wrapper W3C spec. While both are valid, modern code prefersoverflow-wrap. Note: similar instances exist elsewhere in the codebase (e.g.,src/containers/Chat/ChatMessages/ChatMessage/ChatMessage.module.css:84). Consider applying this modernization consistently across all CSS modules.Suggested fix
.Message { padding: 0 0.75rem; max-width: 70%; font-size: 0.9rem; border-radius: 1.25rem; - word-wrap: break-word; + overflow-wrap: break-word; line-height: 1.4; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/containers/AskMeBot/AskMeBot.module.css` around lines 99 - 106, Replace the legacy CSS property word-wrap with the modern overflow-wrap in the .Message rule (and other similar CSS module rules such as the ChatMessage message rule) to follow the W3C spec; update the property name only (keep the value break-word and other declarations unchanged) and apply this same replacement consistently across all CSS modules where word-wrap is used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/assets/images/icons/Discord/DiscordIcon.tsx`:
- Line 3: The SVG title is hardcoded as "Discord Icon" in the DiscordIcon
component; replace it with a translated string by importing and using i18next
(e.g., useTranslation or t) inside DiscordIcon and render
<title>{t('discord.icon_title')}</title> (choose a key like
"discord.icon_title"), then add that key to your translation files and run yarn
extract-translations to pick it up for Lokalise; ensure the import and t usage
are added to the DiscordIcon function so accessibility text is localized.
---
Duplicate comments:
In
`@src/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.utils.test.ts`:
- Around line 6-12: The import in the test uses a relative path for
FormBuilder.utils; replace the relative import './FormBuilder.utils' with the
project src alias import (i.e., import the same named exports
convertFlowJSONToFormBuilder, convertFormBuilderToFlowJSON, computeFieldNames,
hasContentItemError, validateFlowJson from the configured 'src/...' alias) so
the module is imported via the repository's src alias rather than a relative
path.
---
Nitpick comments:
In `@src/components/UI/SearchBar/SearchBar.tsx`:
- Line 22: The prop searchParam in SearchBar.tsx is currently typed as any and
can pass non-boolean values into AdvancedSearch's isActive; change searchParam's
type to a narrower type (prefer the real filter/interface used in the app or at
minimum Record<string, unknown>) and compute a real boolean for AdvancedSearch
(e.g., derive isActive from searchParam ?? {} by checking keys or a known flag
such as Object.keys(searchParam ?? {}).length > 0 or
Boolean(searchParam?.someFlag)), then pass that boolean into <AdvancedSearch
isActive={...}>; update any other occurrences (lines referenced around 35 and
81) to use the same typed prop and boolean derivation.
In `@src/containers/AskMeBot/AskMeBot.module.css`:
- Around line 99-106: Replace the legacy CSS property word-wrap with the modern
overflow-wrap in the .Message rule (and other similar CSS module rules such as
the ChatMessage message rule) to follow the W3C spec; update the property name
only (keep the value break-word and other declarations unchanged) and apply this
same replacement consistently across all CSS modules where word-wrap is used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0e722585-1ad3-4e71-a2ed-44fa1e48b682
📒 Files selected for processing (43)
src/assets/images/icons/Discord/DiscordIcon.tsxsrc/components/UI/DotLoader/DotLoader.module.csssrc/components/UI/Form/DateTimePicker/DateTimePicker.tsxsrc/components/UI/Form/EmojiInput/Editor.module.csssrc/components/UI/Heading/Heading.tsxsrc/components/UI/Layout/Navigation/SideMenus/SideMenus.module.csssrc/components/UI/SearchBar/SearchBar.tsxsrc/components/floweditor/FlowEditor.module.csssrc/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsxsrc/containers/AskMeBot/AskMeBot.module.csssrc/containers/Auth/Promotion/Promotion.module.csssrc/containers/Certificates/Certificate.module.csssrc/containers/Chat/ChatConversations/ChatConversations.module.csssrc/containers/Chat/ChatMessages/ChatMessage/ChatMessage.module.csssrc/containers/Chat/ChatMessages/ChatMessage/PollMessage/PollMessage.module.csssrc/containers/Chat/ChatMessages/ChatMessage/WhatsappFormResponse/WhatsAppFormResponse.module.csssrc/containers/Chat/ChatMessages/ChatMessages.module.csssrc/containers/Collection/CollectionList/CollectionList.test.tsxsrc/containers/ContactManagement/AdminContactManagement/AdminContactManagement.module.csssrc/containers/ContactManagement/Instructions/Instructions.module.csssrc/containers/ContactManagement/UploadContactsDialog/UploadContactsDialog.module.csssrc/containers/Flow/ShareResponderLink/ShareResponderLink.module.csssrc/containers/HSM/HSM.test.tsxsrc/containers/InteractiveMessage/InteractiveOptions/InteractiveOptions.module.csssrc/containers/MyAccount/MyAccount.tsxsrc/containers/Search/Search.module.csssrc/containers/SettingList/SettingList.test.helper.tssrc/containers/SpeedSend/SpeedSend.module.csssrc/containers/TemplateOptions/TemplateOptions.module.csssrc/containers/Ticket/Ticket.module.csssrc/containers/Ticket/TicketList/ExportTicket/ExportTicket.module.csssrc/containers/WhatsAppForms/Configure/Configure.module.csssrc/containers/WhatsAppForms/Configure/FormBuilder/ContentItem/ContentItemComponent.module.csssrc/containers/WhatsAppForms/Configure/FormBuilder/ContentItem/ContentItemComponent.tsxsrc/containers/WhatsAppForms/Configure/FormBuilder/ContentItem/types/ContentTypes.module.csssrc/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.types.tssrc/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.utils.test.tssrc/containers/WhatsAppForms/Configure/FormBuilder/Screen/Screen.module.csssrc/containers/WhatsAppForms/Configure/JSONViewer/JSONViewer.module.csssrc/containers/WhatsAppForms/Configure/Variables/Variables.module.csssrc/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.module.csssrc/declaration.d.tssrc/routes/AuthenticatedRoute/AuthenticatedRoute.test.tsx
💤 Files with no reviewable changes (1)
- src/containers/Collection/CollectionList/CollectionList.test.tsx
✅ Files skipped from review due to trivial changes (23)
- src/containers/MyAccount/MyAccount.tsx
- src/containers/HSM/HSM.test.tsx
- src/components/UI/Form/DateTimePicker/DateTimePicker.tsx
- src/containers/SettingList/SettingList.test.helper.ts
- src/containers/WhatsAppForms/WhatsAppFormList/WhatsAppFormList.module.css
- src/routes/AuthenticatedRoute/AuthenticatedRoute.test.tsx
- src/containers/WhatsAppForms/Configure/Configure.module.css
- src/containers/WhatsAppForms/Configure/Variables/Variables.module.css
- src/containers/WhatsAppForms/Configure/JSONViewer/JSONViewer.module.css
- src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsx
- src/components/UI/DotLoader/DotLoader.module.css
- src/containers/Certificates/Certificate.module.css
- src/containers/Search/Search.module.css
- src/containers/WhatsAppForms/Configure/FormBuilder/ContentItem/types/ContentTypes.module.css
- src/declaration.d.ts
- src/containers/WhatsAppForms/Configure/FormBuilder/Screen/Screen.module.css
- src/containers/Chat/ChatMessages/ChatMessage/ChatMessage.module.css
- src/components/UI/Layout/Navigation/SideMenus/SideMenus.module.css
- src/containers/Ticket/TicketList/ExportTicket/ExportTicket.module.css
- src/containers/ContactManagement/AdminContactManagement/AdminContactManagement.module.css
- src/containers/Ticket/Ticket.module.css
- src/components/floweditor/FlowEditor.module.css
- src/containers/ContactManagement/Instructions/Instructions.module.css
🚧 Files skipped from review as they are similar to previous changes (2)
- src/containers/WhatsAppForms/Configure/FormBuilder/FormBuilder.types.ts
- src/containers/WhatsAppForms/Configure/FormBuilder/ContentItem/ContentItemComponent.tsx
| ); | ||
| export default SvgComponent; | ||
|
No newline at end of file |
||
| <title>{'Discord Icon'}</title> |
There was a problem hiding this comment.
Localize the SVG title text instead of hardcoding English.
<title>{'Discord Icon'}</title> is user-facing accessibility text and should come from i18next so it can be translated.
💡 Suggested change
+import { useTranslation } from 'react-i18next';
+
-const SvgComponent = ({ color }: any) => (
- <svg xmlns="http://www.w3.org/2000/svg" width={22} height={22}>
- <title>{'Discord Icon'}</title>
+const SvgComponent = ({ color }: any) => {
+ const { t } = useTranslation();
+ return (
+ <svg xmlns="http://www.w3.org/2000/svg" width={22} height={22}>
+ <title>{t('icons.discord')}</title>
<g fill="none" fillRule="evenodd">
<path
fill={color}
fillRule="nonzero"
d="M18.59 5.88997C17.36 5.31997 16.05 4.89997 14.67 4.65997C14.5 4.95997 14.3 5.36997 14.17 5.69997C12.71 5.47997 11.26 5.47997 9.83001 5.69997C9.69001 5.36997 9.49001 4.95997 9.32001 4.65997C7.94001 4.89997 6.63001 5.31997 5.40001 5.88997C2.92001 9.62997 2.25001 13.28 2.58001 16.87C4.23001 18.1 5.82001 18.84 7.39001 19.33C7.78001 18.8 8.12001 18.23 8.42001 17.64C7.85001 17.43 7.31001 17.16 6.80001 16.85C6.94001 16.75 7.07001 16.64 7.20001 16.54C10.33 18 13.72 18 16.81 16.54C16.94 16.65 17.07 16.75 17.21 16.85C16.7 17.16 16.15 17.42 15.59 17.64C15.89 18.23 16.23 18.8 16.62 19.33C18.19 18.84 19.79 18.1 21.43 16.87C21.82 12.7 20.76 9.08997 18.61 5.88997H18.59ZM8.84001 14.67C7.90001 14.67 7.13001 13.8 7.13001 12.73C7.13001 11.66 7.88001 10.79 8.84001 10.79C9.80001 10.79 10.56 11.66 10.55 12.73C10.55 13.79 9.80001 14.67 8.84001 14.67ZM15.15 14.67C14.21 14.67 13.44 13.8 13.44 12.73C13.44 11.66 14.19 10.79 15.15 10.79C16.11 10.79 16.87 11.66 16.86 12.73C16.86 13.79 16.11 14.67 15.15 14.67Z"
/>
</g>
</svg>
-);
+ );
+};As per coding guidelines, use i18next for internationalization with string extraction to Lokalise via yarn extract-translations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/images/icons/Discord/DiscordIcon.tsx` at line 3, The SVG title is
hardcoded as "Discord Icon" in the DiscordIcon component; replace it with a
translated string by importing and using i18next (e.g., useTranslation or t)
inside DiscordIcon and render <title>{t('discord.icon_title')}</title> (choose a
key like "discord.icon_title"), then add that key to your translation files and
run yarn extract-translations to pick it up for Lokalise; ensure the import and
t usage are added to the DiscordIcon function so accessibility text is
localized.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3842 +/- ##
==========================================
- Coverage 82.47% 82.33% -0.15%
==========================================
Files 339 339
Lines 13207 13228 +21
Branches 2939 2946 +7
==========================================
- Hits 10893 10891 -2
- Misses 1396 1420 +24
+ Partials 918 917 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| ); | ||
| export default SvgComponent; | ||
|
No newline at end of file |
||
| <title>{'Discord Icon'}</title> |
There was a problem hiding this comment.
pls revert this change
priyanshu6238
left a comment
There was a problem hiding this comment.
I noticed that in many files, there are formatting changes which are causing a large number of diffs. Could you please revert those formatting changes in this PR first (like the one I mentioned above), so we can focus only on the actual functional updates?
priyanshu6238
left a comment
There was a problem hiding this comment.
Also pls fix the deep scan check
| @@ -1,26 +1,26 @@ | |||
| .FormResponseContainer { | |||
There was a problem hiding this comment.
revert all this changes
| @@ -1,23 +1,22 @@ | |||
| .ErrorText { | |||
| @@ -4,7 +4,12 @@ import { BrowserRouter } from 'react-router'; | |||
| import { MockedProvider } from '@apollo/client/testing'; | |||
| @@ -1,4 +1,4 @@ | |||
| declare module "*.module.css" { | |||
| declare module '*.module.css' { | |||
Summary
Issue: Fixes #3794 — Whatsapp Forms: Add Support for Missing WhatsApp Flow Component Types
Problem:
The FormBuilder's import/export pipeline silently dropped or corrupted 8 newer WhatsApp Flow component types (CalendarPicker, ChipsSelector, EmbeddedLink, RichText, If, Switch, PhotoPicker, DocumentPicker). Additionally, three data-preservation bugs existed:
datadeclarations lost — import discardedscreen.data, export overwrote with generated schema${data.some_value}could be alteredSolution:
Unsupportedcontent type that stores the full raw component JSON on import and re-emits it verbatim on export — lossless round-trip for any unknown componentflowIdandflowDatametadata to preserve original IDs and data declarationsflowIdas the screen ID when available, and merge preservedflowDatawith generated data (preserved values take precedence)INPUT_COMPONENT_TYPESwith 4 new input-like types for duplicate-name validationFiles Changed (6)
flowId,flowDatato Screen;rawComponentto ContentItemDataUnsupportedcase with info message + PhotoPicker/DocumentPicker endpoint warning.UnsupportedContent,.UnsupportedInfo,.EndpointWarningTest Plan
New Unit Tests (24 tests)
✓ preserves screen IDs through import→export
✓ preserves screen.data declarations through import→export
✓ preserves Footer label with dynamic expression exactly
✓ stores the Footer label on the screen.buttonLabel without alteration
✓ imports CalendarPicker as Unsupported with raw JSON
✓ imports ChipsSelector as Unsupported with raw JSON
✓ imports EmbeddedLink as Unsupported
✓ imports RichText as Unsupported
✓ imports If with nested children as Unsupported (preserves structure)
✓ imports Switch with nested children as Unsupported (preserves structure)
✓ imports PhotoPicker as Unsupported with raw JSON
✓ imports DocumentPicker as Unsupported with raw JSON
✓ re-exports all 8 unsupported component types as deep-equal raw JSON
✓ does not mutate original raw component objects during round-trip
✓ preserves known TextHeading through round-trip
✓ preserves known TextInput through round-trip
✓ tracks unsupported components that have a name property
✓ returns false for Unsupported type items (no false errors)
✓ validates flow JSON containing new component types without errors
✓ detects duplicate component names across new input types
✓ uses flowId for imported screens and generates IDs for new screens
✓ handles empty flow JSON gracefully
✓ preserves flowId and flowData on the imported Screen objects
✓ does not share references between flowData and original
Test Files 1 passed (1)
Tests 24 passed (24)
Combined Result
Summary by CodeRabbit
New Features
Bug Fixes