fix(inbox) ui #349
fix(inbox) ui #349
Conversation
Reviewer's GuideThis PR implements a new facebookPage resolver in the inbox integration to fetch and expose Facebook page details, extends backend types and GraphQL schemas/queries accordingly, updates the Facebook integration detail form to display the selected page and rename channel fields, and enhances the MyInbox navigation item with a dropdown for quick access to inbox settings. Sequence diagram for facebookPage resolver fetching Facebook page detailssequenceDiagram
participant Client
participant GraphQL API
participant FacebookIntegrations Model
participant Facebook Graph API
Client->>GraphQL API: Query facebookPage for integration
GraphQL API->>FacebookIntegrations Model: Find integration by erxesApiId
FacebookIntegrations Model-->>GraphQL API: Return facebookIntegration
alt facebookIntegration and facebookPageIds exist
loop For each pageId
GraphQL API->>Facebook Graph API: GET /{pageId}?fields=id,name with token
Facebook Graph API-->>GraphQL API: Return IFacebookPageResponse
GraphQL API->>GraphQL API: Add page details to results
end
GraphQL API-->>Client: Return array of page details
else facebookIntegration or facebookPageIds missing
GraphQL API-->>Client: Return null
end
Entity relationship diagram for FacebookIntegration and FacebookPageResponseerDiagram
FACEBOOKINTEGRATION {
string _id
string[] facebookPageIds
map facebookPageTokensMap
}
FACEBOOKPAGERESPONSE {
string id
string name
}
FACEBOOKINTEGRATION ||--o| FACEBOOKPAGERESPONSE : "facebookPageIds"
Class diagram for updated Facebook integration types and resolverclassDiagram
class IFacebookIntegrationDocument {
_id: string
}
class IFacebookPageResponse {
id: string
name: string
}
class IntegrationResolvers {
+integrationStatus()
+facebookPage(integration, _args, context)
}
IntegrationResolvers --> IFacebookPageResponse : uses
IntegrationResolvers --> IFacebookIntegrationDocument : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a GraphQL resolver to fetch Facebook page info for integrations, introduces a JSON facebookPage field to the Integration schema, adds an interface for Facebook page responses, updates frontend queries to request channels.name and facebookPage, and updates the Facebook integration detail UI to display the page name and rename channelIds to channels. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant FE as Frontend UI
participant GQL as Backend GraphQL
participant R as integration.facebookPage Resolver
participant M as Models/DB
participant FB as Facebook Graph API
U->>FE: Open Integration Detail
FE->>GQL: query integrationDetail { ..., channels { _id name }, facebookPage }
alt facebook integration
GQL->>R: resolve facebookPage(integration)
R->>M: find FacebookIntegration by erxesApiId
alt tokens and page IDs available
loop for each pageId
R->>R: lookup token for pageId
alt token found
R->>FB: GET /{pageId}?fields=id,name&access_token
FB-->>R: { id, name }
R->>R: collect { pageId, id, name }
else token missing
R->>R: skip pageId (warn)
end
end
R-->>GQL: [{ pageId, id, name }] or null if empty
else missing config
R-->>GQL: null (warn)
end
else non-facebook integration
GQL-->>FE: facebookPage = null
end
GQL-->>FE: channels with name, facebookPage
FE-->>U: Show channel list and Facebook Page Name (read-only)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `backend/plugins/frontline_api/src/modules/inbox/graphql/resolvers/customResolvers/integration.ts:190` </location>
<code_context>
+
+ return results.length > 0 ? results : null;
+ } catch (error) {
+ return { error };
+ }
+ },
</code_context>
<issue_to_address>
**issue (bug_risk):** Returning an object with an error may break expected return types.
Returning different types may confuse consumers and cause runtime errors. Consider standardizing the return value, such as always returning null or throwing the error, based on your GraphQL error handling approach.
</issue_to_address>
### Comment 2
<location> `backend/plugins/frontline_api/src/modules/inbox/graphql/resolvers/customResolvers/integration.ts:183-185` </location>
<code_context>
+ results.push({ pageId, ...response });
+ }
+ } catch (err) {
+ console.error(`Failed to fetch page ${pageId}:`, err);
+ }
+ }
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Logging errors for individual page fetches may expose sensitive information.
Sanitize or restrict error log details to prevent exposure of sensitive Facebook API data, such as tokens or user information, in production.
```suggestion
} catch (err) {
// Sanitize error logging to avoid exposing sensitive Facebook API data
console.error(`Failed to fetch page ${pageId}: An error occurred while fetching page data.`);
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to 37d6ca4 in 2 minutes and 1 seconds. Click for details.
- Reviewed
268lines of code in7files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. frontend/core-ui/src/modules/notification/my-inbox/components/MyInboxNavigationItem.tsx:28
- Draft comment:
Ensure that the custom CSS class (e.g. 'group-hover/trigger:visible') used for the dropdown trigger is properly configured in your styling setup. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the PR author to ensure that a CSS class is properly configured, which falls under the rule of not asking the author to ensure something is tested or configured correctly. It doesn't provide a specific suggestion or point out a specific issue with the code.
2. frontend/plugins/frontline_ui/src/modules/integrations/facebook/components/FacebookIntegrationDetail.tsx:112
- Draft comment:
If multiple Facebook pages exist, displaying only the first element's name may be misleading. Consider iterating over all pages or clarifying this behavior. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The field is disabled and read-only, suggesting this is just a display field, not for user input. Without seeing the schema or understanding the business logic, we don't know if multiple pages are even possible or intended. The UI pattern of showing [0] suggests this is intentionally designed to show just one page. I could be wrong about the intentionality - maybe this is a bug where multiple pages should be shown. The [0] access could be a quick fix that needs improvement. However, the disabled/readonly nature strongly suggests this is just for display. If multiple pages were intended to be shown, the UI pattern would likely be completely different (e.g. a list or table). The comment is speculative without strong evidence that showing multiple pages is actually required. We should delete it.
3. frontend/plugins/frontline_ui/src/modules/integrations/erxes-messenger/graphql/queries/integrationDetailQuery.ts:11
- Draft comment:
The channels field now includes 'name'; verify that the backend schema and downstream consumers properly handle this added field. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
4. frontend/plugins/frontline_ui/src/modules/integrations/graphql/queries/getIntegrationDetail.ts:24
- Draft comment:
The inclusion of 'facebookPage' in the integration detail query should be cross-checked with the GraphQL schema to ensure consistent client data handling. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
Workflow ID: wflow_m7SznS86pjrsy6Zi
You can customize 
|
|
||
| return results.length > 0 ? results : null; | ||
| } catch (error) { | ||
| return { error }; |
There was a problem hiding this comment.
Instead of returning an object with an error, throw the error to maintain consistent GraphQL error handling.
| return { error }; | |
| throw error; |
|
|
||
| const results: ({ pageId: string } & IFacebookPageResponse)[] = []; | ||
|
|
||
| for (const pageId of facebookIntegration.facebookPageIds) { |
There was a problem hiding this comment.
Consider parallelizing the independent Facebook page requests (e.g. with Promise.all) for potential performance improvements.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
backend/plugins/frontline_api/src/modules/inbox/graphql/resolvers/customResolvers/integration.ts (1)
190-191: Throw instead of returning { error } for GraphQL errors.This keeps error handling consistent with GraphQL semantics. This mirrors prior feedback.
- } catch (error) { - return { error }; + } catch (error) { + throw error; }
🧹 Nitpick comments (9)
frontend/core-ui/src/modules/notification/my-inbox/components/MyInboxNavigationItem.tsx (4)
29-36: Add an accessible label to the overflow button.Improve a11y for screen readers.
Apply this diff:
- <Button + <Button + aria-label={t('Open inbox menu')} variant="ghost" size="icon" className="invisible group-hover/trigger:visible absolute top-1/2 -translate-y-1/2 right-1 text-muted-foreground" onClick={(e) => e.stopPropagation()} >
45-47: Navigate using an absolute path to avoid route-relative surprises.Prevents accidental relative navigation when the current route changes.
Apply this diff:
- onSelect={() => navigate(`settings/inbox/integrations`)} + onSelect={() => navigate('/settings/inbox/integrations')}
67-71: Optional: cap large counts and add a11y label.Keeps badge compact and clearer for SR users.
Apply this diff:
- return ( - <Badge className="ml-auto text-xs min-w-6 px-1 justify-center"> - {unreadNotificationsCount} - </Badge> - ); + const display = unreadNotificationsCount > 999 ? '999+' : unreadNotificationsCount; + return ( + <Badge + aria-label={`${unreadNotificationsCount} unread notifications`} + className="ml-auto text-xs min-w-6 px-1 justify-center" + > + {display} + </Badge> + );
12-12: Import useNavigate from 'react-router-dom'Change to match repo usage — all other useNavigate imports come from 'react-router-dom'; this file imports from 'react-router'.
-import { useNavigate } from 'react-router'; +import { useNavigate } from 'react-router-dom';frontend/core-ui/src/modules/notification/my-inbox/components/MyInboxNavigationItem.tsx
backend/plugins/frontline_api/src/modules/inbox/graphql/schemas/integration.ts (1)
50-51: Prefer a typed GraphQL shape over JSON (optional).Typing improves discoverability and client safety. If feasible, expose a typed list.
Proposed schema tweak:
- facebookPage: JSON + facebookPage: [FacebookPage!]And add types near other type defs:
+ type FacebookPageData { + id: String! + name: String! + } + + type FacebookPage { + pageId: String! + data: FacebookPageData + }frontend/plugins/frontline_ui/src/modules/integrations/facebook/components/FacebookIntegrationDetail.tsx (2)
111-129: Read-only page name: guard shape and show all linked pages, not just the first.Current code assumes an array and displays only the first page. Guard with Array.isArray and render all names (comma‑separated) to reflect multiple links.
- render={({ field }) => { - const facebookPageName = field.value?.[0] ?? null; - return ( + render={({ field }) => { + const names = + Array.isArray(field.value) + ? field.value.map(p => p?.name).filter(Boolean).join(', ') + : ''; + return ( <Form.Item> <Form.Label>Page Name</Form.Label> <Form.Control> <Input - name={field.name} - value={facebookPageName?.name || ''} + value={names} disabled readOnly /> </Form.Control> </Form.Item> ); }}
106-108: Drop console.log in submit error handler.Per guidelines, avoid console logs. Surface validation errors via toast or form messages.
- onSubmit={form.handleSubmit(onSubmit, (error) => { - console.log(error); - })} + onSubmit={form.handleSubmit(onSubmit, () => { + toast({ title: 'Please fix the highlighted errors', variant: 'destructive' }); + })}backend/plugins/frontline_api/src/modules/inbox/graphql/resolvers/customResolvers/integration.ts (2)
143-149: Simplify early-return check for non-Facebook kinds.No need to compute serviceName; short‑circuit on kind.
- const serviceName = integration.kind.includes('facebook') - ? 'facebook' - : integration.kind; - - if (serviceName !== 'facebook') { + if (!integration.kind.includes('facebook')) { return null; }
156-163: Replace console.warn with structured logger.Prefer the app’s logger (with context like integrationId/subdomain) over console.* for observability and consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
backend/plugins/frontline_api/src/modules/inbox/graphql/resolvers/customResolvers/integration.ts(2 hunks)backend/plugins/frontline_api/src/modules/inbox/graphql/schemas/integration.ts(1 hunks)backend/plugins/frontline_api/src/modules/integrations/facebook/@types/integrations.ts(1 hunks)frontend/core-ui/src/modules/notification/my-inbox/components/MyInboxNavigationItem.tsx(1 hunks)frontend/plugins/frontline_ui/src/modules/integrations/erxes-messenger/graphql/queries/integrationDetailQuery.ts(1 hunks)frontend/plugins/frontline_ui/src/modules/integrations/facebook/components/FacebookIntegrationDetail.tsx(2 hunks)frontend/plugins/frontline_ui/src/modules/integrations/graphql/queries/getIntegrationDetail.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Use functional and declarative programming patterns; avoid classes.
Use TypeScript for all code; prefer interfaces over types.
Avoid enums; use maps instead.
Files:
backend/plugins/frontline_api/src/modules/inbox/graphql/schemas/integration.tsfrontend/plugins/frontline_ui/src/modules/integrations/erxes-messenger/graphql/queries/integrationDetailQuery.tsfrontend/plugins/frontline_ui/src/modules/integrations/graphql/queries/getIntegrationDetail.tsbackend/plugins/frontline_api/src/modules/inbox/graphql/resolvers/customResolvers/integration.tsbackend/plugins/frontline_api/src/modules/integrations/facebook/@types/integrations.tsfrontend/core-ui/src/modules/notification/my-inbox/components/MyInboxNavigationItem.tsxfrontend/plugins/frontline_ui/src/modules/integrations/facebook/components/FacebookIntegrationDetail.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx,js,jsx}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Avoid console logs.
Always use absolute paths when importing.
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
**/*.{ts,tsx,js,jsx}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Use absolute path when import.
Files:
backend/plugins/frontline_api/src/modules/inbox/graphql/schemas/integration.tsfrontend/plugins/frontline_ui/src/modules/integrations/erxes-messenger/graphql/queries/integrationDetailQuery.tsfrontend/plugins/frontline_ui/src/modules/integrations/graphql/queries/getIntegrationDetail.tsbackend/plugins/frontline_api/src/modules/inbox/graphql/resolvers/customResolvers/integration.tsbackend/plugins/frontline_api/src/modules/integrations/facebook/@types/integrations.tsfrontend/core-ui/src/modules/notification/my-inbox/components/MyInboxNavigationItem.tsxfrontend/plugins/frontline_ui/src/modules/integrations/facebook/components/FacebookIntegrationDetail.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{tsx,jsx}: Favor named exports for components.
Use declarative JSX.
Use Shadcn UI, Radix, and Tailwind for components and styling.
Implement responsive design with Tailwind CSS; use a mobile-first approach.
**/*.{tsx,jsx}: Structure files: exported component, subcomponents, helpers, static content, types.
Favor named exports for components.
Use declarative JSX.
Use Shadcn UI, Radix, and Tailwind for components and styling.
Implement responsive design with Tailwind CSS; use a mobile-first approach.
Files:
frontend/core-ui/src/modules/notification/my-inbox/components/MyInboxNavigationItem.tsxfrontend/plugins/frontline_ui/src/modules/integrations/facebook/components/FacebookIntegrationDetail.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional components with TypeScript interfaces.
Minimize 'useEffect', and 'setState'.
Wrap client components in Suspense with fallback.
Use dynamic loading for non-critical components.
Limit 'use client': Favor server components and Next.js SSR. Use only for Web API access in small components. Avoid for data fetching or state management.
Files:
frontend/core-ui/src/modules/notification/my-inbox/components/MyInboxNavigationItem.tsxfrontend/plugins/frontline_ui/src/modules/integrations/facebook/components/FacebookIntegrationDetail.tsx
**/*.{webp,tsx,jsx}
📄 CodeRabbit inference engine (.cursorrules)
Optimize images: use WebP format, include size data, implement lazy loading.
Files:
frontend/core-ui/src/modules/notification/my-inbox/components/MyInboxNavigationItem.tsxfrontend/plugins/frontline_ui/src/modules/integrations/facebook/components/FacebookIntegrationDetail.tsx
🧬 Code graph analysis (3)
backend/plugins/frontline_api/src/modules/inbox/graphql/resolvers/customResolvers/integration.ts (3)
backend/plugins/frontline_api/src/modules/inbox/@types/integrations.ts (1)
IIntegrationDocument(181-190)backend/plugins/frontline_api/src/modules/integrations/facebook/@types/integrations.ts (1)
IFacebookPageResponse(20-23)backend/plugins/frontline_api/src/modules/integrations/facebook/utils.ts (1)
graphRequest(13-39)
frontend/core-ui/src/modules/notification/my-inbox/components/MyInboxNavigationItem.tsx (2)
frontend/libs/erxes-ui/src/modules/navigation-menu/components/NavigationMenu.tsx (1)
NavigationMenuLinkItem(7-56)frontend/libs/erxes-ui/src/components/dropdown-menu.tsx (1)
DropdownMenu(171-186)
frontend/plugins/frontline_ui/src/modules/integrations/facebook/components/FacebookIntegrationDetail.tsx (2)
frontend/libs/erxes-ui/src/components/form.tsx (1)
Form(166-174)frontend/plugins/frontline_ui/src/modules/inbox/channel/components/SelectChannel.tsx (1)
SelectChannel(269-277)
🪛 Biome (2.1.2)
frontend/core-ui/src/modules/notification/my-inbox/components/MyInboxNavigationItem.tsx
[error] 9-9: Shouldn't redeclare 'NavigationMenuLinkItem'. Consider to delete it or rename it.
'NavigationMenuLinkItem' is defined here:
(lint/suspicious/noRedeclare)
[error] 9-9: Shouldn't redeclare 'Skeleton'. Consider to delete it or rename it.
'Skeleton' is defined here:
(lint/suspicious/noRedeclare)
🪛 GitHub Actions: core-ui-ci
frontend/core-ui/src/modules/notification/my-inbox/components/MyInboxNavigationItem.tsx
[error] 4-4: Module build failed: the name NavigationMenuLinkItem is defined multiple times. Command: pnpm nx build core-ui --configuration=production --verbose
[error] 9-9: the name Skeleton is defined multiple times. Command: pnpm nx build core-ui --configuration=production --verbose
🔇 Additional comments (6)
backend/plugins/frontline_api/src/modules/integrations/facebook/@types/integrations.ts (1)
20-23: Type looks good; minimal surface is appropriate.No issues spotted.
frontend/plugins/frontline_ui/src/modules/integrations/erxes-messenger/graphql/queries/integrationDetailQuery.ts (1)
10-13: LGTM: fetching channel name is useful for UI labels.frontend/plugins/frontline_ui/src/modules/integrations/graphql/queries/getIntegrationDetail.ts (2)
18-21: LGTM: include channel name.
24-25: Resolver matches frontend usage — no action required. Backend facebookPage resolver returns an array of objects { pageId, id, name } (backend/plugins/frontline_api/src/modules/inbox/graphql/resolvers/customResolvers/integration.ts) and the frontend reads facebookPage[0].name (frontend/plugins/frontline_ui/src/modules/integrations/facebook/components/FacebookIntegrationDetail.tsx); shapes align.backend/plugins/frontline_api/src/modules/inbox/graphql/resolvers/customResolvers/integration.ts (2)
4-6: Typed imports look good.Using graphRequest with IFacebookPageResponse keeps the resolver strongly typed.
166-186: Keep sequential fetch or refactor graphRequest to be token-safe before parallelizingThis loop must remain sequential unless you confirm graphRequest.base does not mutate a global fbgraph access token — Promise.all would otherwise race tokens and return incorrect page data. To enable safe parallelism, refactor graphRequest to accept a per-request token, instantiate a client per token, or call the Graph API directly (fetch/axios with access_token query param), then switch to Promise.all.
frontend/core-ui/src/modules/notification/my-inbox/components/MyInboxNavigationItem.tsx
Outdated
Show resolved
Hide resolved
| name="channels" | ||
| render={({ field }) => { | ||
| return ( | ||
| <Form.Item> | ||
| <Form.Label>Channels</Form.Label> | ||
| <SelectChannel.FormItem | ||
| value={field.value} | ||
| onValueChange={field.onChange} | ||
| mode="multiple" | ||
| /> | ||
| <Form.Message /> | ||
| </Form.Item> | ||
| ); | ||
| }} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
channels field rename: verify mutation variables and schema expect channels (not channelIds).
Ensure editIntegration/integrationsEdit mutations, Zod schema, and server resolvers accept channels. If backend still expects channelIds, submission will fail.
🏁 Script executed:
#!/bin/bash
# Verify remaining channelIds usages and the expected mutation variables.
rg -nP -C3 '\bchannelIds\b'
rg -nP -C3 '(integrations?Edit|editIntegration)\s*\(' --type=ts --type=tsx --type=js --type=jsx
# Inspect the FB schema to confirm shape of `channels`
fd -a 'FbMessengerSchema' | xargs -I{} sh -c 'echo "==> {}"; sed -n "1,200p" "{}" | sed -n "1,200p"'Length of output: 54491
Revert to using channelIds (or update backend) — backend still expects channelIds
Form field was renamed to name="channels" but frontend Zod schemas, GraphQL mutations, and backend resolvers/schemas still use channelIds; this will break submissions. Revert the Form.Field to name="channelIds" or update/map all layers to accept "channels" -> "channelIds".
Files to fix or align:
- frontend/plugins/frontline_ui/src/modules/integrations/facebook/components/FacebookIntegrationDetail.tsx (current change)
- frontend/plugins/frontline_ui/src/modules/integrations/facebook/constants/FbMessengerSchema.ts
- frontend/plugins/frontline_ui/src/modules/integrations/facebook/components/FacebookIntegrationSetup.tsx
- frontend/plugins/frontline_ui/src/modules/integrations/graphql/mutations/AddIntegration.ts
- frontend/plugins/frontline_ui/src/modules/integrations/graphql/mutations/EditIntegration.ts
- backend/plugins/frontline_api/src/modules/inbox/graphql/schemas/integration.ts
- backend/plugins/frontline_api/src/modules/inbox/graphql/resolvers/mutations/integrations.ts
🤖 Prompt for AI Agents
In
frontend/plugins/frontline_ui/src/modules/integrations/facebook/components/FacebookIntegrationDetail.tsx
lines ~155-169, the Form.Field was renamed to name="channels" but the rest of
the stack (Zod schemas, GraphQL mutations, backend resolvers/schemas) still
expect channelIds; revert this Form.Field back to name="channelIds" so
submissions match existing contracts, or alternatively add a mapping step before
submit that copies form.values.channels to form.values.channelIds. Also update
the related files listed (FbMessengerSchema.ts, FacebookIntegrationSetup.tsx,
AddIntegration.ts, EditIntegration.ts, and the backend integration schema and
resolver files) to keep the field name consistent across frontend Zod types,
GraphQL input types, and backend resolver payloads if you opt to rename instead
of reverting.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 39ef6ef in 28 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. backend/plugins/frontline_api/src/modules/inbox/graphql/resolvers/customResolvers/integration.ts:190
- Draft comment:
Throwing the error here instead of returning an error object ensures proper GraphQL error propagation and avoids ambiguous responses. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, explaining why a certain approach is taken. It doesn't suggest a change or ask for clarification on the code. It violates the rule against making purely informative comments.
Workflow ID: wflow_G9DAAOwUWSHdpID5
You can customize
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
backend/plugins/frontline_api/src/modules/inbox/graphql/resolvers/customResolvers/integration.ts (1)
166-186: Parallelize independent Facebook page fetches.Fetching pages sequentially increases latency. Run them in parallel with safe per-item error handling. This also addresses a prior suggestion.
Apply this diff:
- for (const pageId of facebookIntegration.facebookPageIds) { - const token = facebookIntegration.facebookPageTokensMap?.[pageId]; - - if (!token) { - console.warn(`Token not found for pageId: ${pageId}`); - continue; - } - - try { - const response = (await graphRequest.get( - `/${pageId}?fields=id,name`, - token, - )) as IFacebookPageResponse; - - if (response) { - results.push({ pageId, ...response }); - } - } catch (err) { - console.error(`Failed to fetch page ${pageId}:`, err); - } - } + const responses = await Promise.all( + facebookIntegration.facebookPageIds.map(async (pageId: string) => { + const token = facebookIntegration.facebookPageTokensMap?.[pageId]; + if (!token) { + console.warn(`Token not found for pageId: ${pageId}`); + return null; + } + try { + const response = (await graphRequest.get( + `/${pageId}?fields=id,name`, + token + )) as IFacebookPageResponse; + return response ? ({ pageId, ...response }) : null; + } catch (err) { + console.error(`Failed to fetch page ${pageId}:`, err); + return null; + } + }) + ); + results.push( + ...responses.filter(Boolean) as ({ pageId: string } & IFacebookPageResponse)[] + );
🧹 Nitpick comments (5)
backend/plugins/frontline_api/src/modules/inbox/graphql/resolvers/customResolvers/integration.ts (5)
143-149: Simplify the kind guard; drop theserviceNamevariable.Short-circuit early and reduce branching.
Apply this diff:
- const serviceName = integration.kind.includes('facebook') - ? 'facebook' - : integration.kind; - - if (serviceName !== 'facebook') { - return null; - } + if (!integration.kind.includes('facebook')) return null;
151-155: Limit DB payload with a projection.Only the needed fields are required here.
Apply this diff:
- const facebookIntegration = await models.FacebookIntegrations.findOne({ - erxesApiId: integration._id, - }); + const facebookIntegration = await models.FacebookIntegrations.findOne( + { erxesApiId: integration._id }, + { facebookPageIds: 1, facebookPageTokensMap: 1 } + );
188-189: Consider returning an empty array instead of null.If the GraphQL field type allows it, returning
[]avoids a tri-state and simplifies clients.Apply this diff if the schema
facebookPageis nullable-list or JSON accepting arrays:- return results.length > 0 ? results : null; + return results;If the schema enforces
facebookPage: JSONbut expectsnullwhen empty, keep current behavior.
151-192: Remove outer try/catch; it only rethrows (ESLint: no-useless-catch).Let errors bubble to GraphQL. Inner per-page try/catch already handles partial failures.
Apply this diff:
- try { - const facebookIntegration = await models.FacebookIntegrations.findOne( - { erxesApiId: integration._id }, - { facebookPageIds: 1, facebookPageTokensMap: 1 } - ); + const facebookIntegration = await models.FacebookIntegrations.findOne( + { erxesApiId: integration._id }, + { facebookPageIds: 1, facebookPageTokensMap: 1 } + ); @@ - return results.length > 0 ? results : null; - } catch (error) { - throw error; - } + return results.length > 0 ? results : null;
156-162: Use the project logger instead of console in this resolverFile: backend/plugins/frontline_api/src/modules/inbox/graphql/resolvers/customResolvers/integration.ts (around lines 156–172)
Replace console.warn/console.error with the repo's structured logger (use the existing createLogger/logger pattern used elsewhere, e.g. call services' createLogger). Example: instantiate the standard logger for this module and call logger.warn(...) / logger.error(...) instead of console.*.
Snippet still relevant:
if ( !facebookIntegration || !Array.isArray(facebookIntegration.facebookPageIds) ) { console.warn('No facebookIntegration or no facebookPageIds found'); return null; }
- Minimal change: add a module logger (e.g. const logger = createLogger('frontline:integration') or import the shared logger) and replace console.warn / console.error calls in this file with logger.warn / logger.error.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/plugins/frontline_api/src/modules/inbox/graphql/resolvers/customResolvers/integration.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Use functional and declarative programming patterns; avoid classes.
Use TypeScript for all code; prefer interfaces over types.
Avoid enums; use maps instead.
Files:
backend/plugins/frontline_api/src/modules/inbox/graphql/resolvers/customResolvers/integration.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx,js,jsx}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Avoid console logs.
Always use absolute paths when importing.
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
**/*.{ts,tsx,js,jsx}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Use absolute path when import.
Files:
backend/plugins/frontline_api/src/modules/inbox/graphql/resolvers/customResolvers/integration.ts
🧬 Code graph analysis (1)
backend/plugins/frontline_api/src/modules/inbox/graphql/resolvers/customResolvers/integration.ts (3)
backend/plugins/frontline_api/src/modules/inbox/@types/integrations.ts (1)
IIntegrationDocument(181-190)backend/plugins/frontline_api/src/modules/integrations/facebook/@types/integrations.ts (1)
IFacebookPageResponse(20-23)backend/plugins/frontline_api/src/modules/integrations/facebook/utils.ts (1)
graphRequest(13-39)
🪛 ESLint
backend/plugins/frontline_api/src/modules/inbox/graphql/resolvers/customResolvers/integration.ts
[error] 151-191: Unnecessary try/catch wrapper.
(no-useless-catch)
⏰ 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). (2)
- GitHub Check: frontline_ui-ci
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
backend/plugins/frontline_api/src/modules/inbox/graphql/resolvers/customResolvers/integration.ts (1)
4-6: LGTM: imports align with absolute-path guideline.
graphRequestandIFacebookPageResponseimports look correct.
Summary by Sourcery
Add UI enhancements for the inbox navigation and Facebook integration form, and introduce a GraphQL facebookPage field to surface linked Facebook pages in the inbox integration detail.
New Features:
Enhancements:
Summary by CodeRabbit
New Features
UI
Tests
Chores
Important
Enhance inbox UI and Facebook integration by adding a GraphQL field for Facebook pages and updating the integration form.
facebookPagefield toIntegrationtype inintegration.ts.facebookPageresolver inintegration.tsto fetch linked Facebook pages.FacebookIntegrationDetail.tsx.channelIdstochannelsinFacebookIntegrationDetail.tsx.facebookPageand channel names inintegrationDetailQuery.tsandgetIntegrationDetail.ts.This description was created by
for 39ef6ef. You can customize this summary. It will automatically update as commits are pushed.