feat: redesign integration upload with template selection#804
Conversation
…improvements Add template-based integration creation flow alongside the existing manual upload: - New template step with pre-built integrations grid (GitHub, Slack, Gmail, etc.) - Templates fetch config.json, connector, and icon from GitHub examples - Tabbed dialog with "Template" and "Upload" options - Inline delete section in manage dialog using shared DeleteDialog component - Improved credentials form layout - Updated integration configs with expanded operation definitions
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughThis change introduces integration template support and extends synchronization capabilities across the platform. It adds a new Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The repetitive Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip CodeRabbit can use oxc to improve the quality of JavaScript and TypeScript code reviews.Add a configuration file to your project to customize how CodeRabbit runs oxc. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@services/platform/app/features/settings/integrations/components/integration-upload/constants/integration-templates.ts`:
- Around line 1-2: The GITHUB_RAW_BASE constant currently points at the mutable
"main" branch which can drift; update the constant GITHUB_RAW_BASE (or make it
configurable) to reference an immutable ref (a specific commit SHA or release
tag) or use a server-provided versioned ref so template URLs are pinned and
cannot change without a deploy; replace the "main" path segment with the chosen
tag/SHA or wire the value to a config/env variable that the server supplies.
In
`@services/platform/app/features/settings/integrations/components/integration-upload/utils/fetch-template-files.ts`:
- Around line 24-28: Replace the Promise.all(fetch...) call that produces
configResp, connectorResp, and iconResp with Promise.allSettled so network/CORS
rejections are captured; then inspect each settled result (configResp,
connectorResp, iconResp) and convert rejected entries into appropriate
ParseResult error objects or null responses before proceeding (preserve the
function's ParseResult contract). In the function fetchTemplateFiles (and any
helper methods it calls), handle each settled entry: if status === "fulfilled"
use the Response, if "rejected" create a structured error outcome (including the
error message) and ensure the function returns that ParseResult instead of
throwing. Apply the identical allSettled + explicit handling pattern to the
later connector and icon fetch code paths referenced around the original
connector rejection handling (lines ~51-56) so no fetch rejection can escape as
an unhandled promise rejection.
In
`@services/platform/app/features/settings/integrations/components/integration-upload/utils/validate-config.ts`:
- Around line 86-93: The capabilities schema currently allows any string for
syncFrequency and doesn't enforce that canSync implies a schedule; update the
zod schema(s) where capabilities is defined (the capabilities: z.object({...})
block and the similar block at lines 104-116) to (1) validate syncFrequency
against a strict schedule format (e.g., a cron/recurrence regex or ISO-8601
interval pattern) instead of z.string().optional(), and (2) add a refinement or
.superRefine on the capabilities object to require that when canSync is true
then syncFrequency is present and valid (and optionally forbid syncFrequency
when canSync is false if desired). Use the existing schema symbol names
(capabilities, syncFrequency, canSync) so the change is easy to locate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e082c216-3491-406b-a151-258c12b236fc
📒 Files selected for processing (23)
examples/integrations/circuly/config.jsonexamples/integrations/discord/config.jsonexamples/integrations/github/config.jsonexamples/integrations/gmail/config.jsonexamples/integrations/outlook/config.jsonexamples/integrations/protel/config.jsonexamples/integrations/shopify/config.jsonexamples/integrations/slack/config.jsonexamples/integrations/teams/config.jsonexamples/integrations/twilio/config.jsonservices/platform/app/features/settings/integrations/components/integration-manage-dialog.tsxservices/platform/app/features/settings/integrations/components/integration-manage/integration-credentials-form.tsxservices/platform/app/features/settings/integrations/components/integration-manage/integration-delete-section.tsxservices/platform/app/features/settings/integrations/components/integration-upload/constants/integration-templates.tsservices/platform/app/features/settings/integrations/components/integration-upload/hooks/use-upload-integration.tsservices/platform/app/features/settings/integrations/components/integration-upload/integration-upload-dialog.tsxservices/platform/app/features/settings/integrations/components/integration-upload/steps/template-step.tsxservices/platform/app/features/settings/integrations/components/integration-upload/steps/upload-step.tsxservices/platform/app/features/settings/integrations/components/integration-upload/utils/__tests__/fetch-template-files.test.tsservices/platform/app/features/settings/integrations/components/integration-upload/utils/fetch-template-files.tsservices/platform/app/features/settings/integrations/components/integration-upload/utils/validate-config.tsservices/platform/app/features/settings/integrations/components/integrations.tsxservices/platform/messages/en.json
💤 Files with no reviewable changes (1)
- services/platform/app/features/settings/integrations/components/integration-manage/integration-delete-section.tsx
| const [configResp, connectorResp, iconResp] = await Promise.all([ | ||
| fetch(configUrl), | ||
| template.type !== 'sql' ? fetch(connectorUrl) : Promise.resolve(null), | ||
| fetch(iconUrl), | ||
| ]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "fetch-template-files.ts" -type fRepository: tale-project/tale
Length of output: 175
🏁 Script executed:
cat -n services/platform/app/features/settings/integrations/components/integration-upload/utils/fetch-template-files.tsRepository: tale-project/tale
Length of output: 2545
Handle network/CORS rejections in Promise.all() to maintain structured error responses.
Line 24 uses Promise.all() over fetch(), which throws an unhandled rejection on network or CORS errors. The function contract requires always returning ParseResult, but unhandled rejections bypass this. Even the optional icon fetch can crash the template flow if the network request fails.
Switch to Promise.allSettled() to capture both resolved responses and rejections, then handle each explicitly:
Suggested approach
- const [configResp, connectorResp, iconResp] = await Promise.all([
+ const [configResult, connectorResult, iconResult] = await Promise.allSettled([
fetch(configUrl),
template.type !== 'sql' ? fetch(connectorUrl) : Promise.resolve(null),
fetch(iconUrl),
]);
+
+ if (configResult.status === 'rejected') {
+ return {
+ success: false,
+ error: 'Failed to fetch template configuration from GitHub',
+ };
+ }
+
+ const configResp = configResult.value;
+ const connectorResp =
+ connectorResult.status === 'fulfilled' ? connectorResult.value : null;
+ const iconResp = iconResult.status === 'fulfilled' ? iconResult.value : null;Apply the same pattern to lines 51-56 (connector rejection handling) and icon handling.
📝 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.
| const [configResp, connectorResp, iconResp] = await Promise.all([ | |
| fetch(configUrl), | |
| template.type !== 'sql' ? fetch(connectorUrl) : Promise.resolve(null), | |
| fetch(iconUrl), | |
| ]); | |
| const [configResult, connectorResult, iconResult] = await Promise.allSettled([ | |
| fetch(configUrl), | |
| template.type !== 'sql' ? fetch(connectorUrl) : Promise.resolve(null), | |
| fetch(iconUrl), | |
| ]); | |
| if (configResult.status === 'rejected') { | |
| return { | |
| success: false, | |
| error: 'Failed to fetch template configuration from GitHub', | |
| }; | |
| } | |
| const configResp = configResult.value; | |
| const connectorResp = | |
| connectorResult.status === 'fulfilled' ? connectorResult.value : null; | |
| const iconResp = iconResult.status === 'fulfilled' ? iconResult.value : null; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/settings/integrations/components/integration-upload/utils/fetch-template-files.ts`
around lines 24 - 28, Replace the Promise.all(fetch...) call that produces
configResp, connectorResp, and iconResp with Promise.allSettled so network/CORS
rejections are captured; then inspect each settled result (configResp,
connectorResp, iconResp) and convert rejected entries into appropriate
ParseResult error objects or null responses before proceeding (preserve the
function's ParseResult contract). In the function fetchTemplateFiles (and any
helper methods it calls), handle each settled entry: if status === "fulfilled"
use the Response, if "rejected" create a structured error outcome (including the
error message) and ensure the function returns that ParseResult instead of
throwing. Apply the identical allSettled + explicit handling pattern to the
later connector and icon fetch code paths referenced around the original
connector rejection handling (lines ~51-56) so no fetch rejection can escape as
an unhandled promise rejection.
| capabilities: z | ||
| .object({ | ||
| canSync: z.boolean().optional(), | ||
| canPush: z.boolean().optional(), | ||
| canWebhook: z.boolean().optional(), | ||
| syncFrequency: z.string().optional(), | ||
| }) | ||
| .optional(), |
There was a problem hiding this comment.
Validate capabilities.syncFrequency and enforce canSync dependency.
Right now, any string is accepted for syncFrequency (Line 91), and canSync: true does not require a schedule. This can allow invalid runtime sync configs through validation.
🔧 Proposed fix
+const cron5FieldPattern = /^\S+\s+\S+\s+\S+\s+\S+\s+\S+$/;
+
export const integrationConfigSchema = z
.object({
@@
capabilities: z
.object({
canSync: z.boolean().optional(),
canPush: z.boolean().optional(),
canWebhook: z.boolean().optional(),
- syncFrequency: z.string().optional(),
+ syncFrequency: z
+ .string()
+ .trim()
+ .regex(
+ cron5FieldPattern,
+ 'syncFrequency must be a 5-field cron expression',
+ )
+ .optional(),
})
.optional(),
})
@@
.superRefine((data, ctx) => {
+ if (data.capabilities?.canSync && !data.capabilities.syncFrequency) {
+ ctx.addIssue({
+ code: z.ZodIssueCode.custom,
+ path: ['capabilities', 'syncFrequency'],
+ message: 'syncFrequency is required when capabilities.canSync is true',
+ });
+ }
+
if (data.type === 'sql') {
data.operations.forEach((op, index) => {Also applies to: 104-116
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/settings/integrations/components/integration-upload/utils/validate-config.ts`
around lines 86 - 93, The capabilities schema currently allows any string for
syncFrequency and doesn't enforce that canSync implies a schedule; update the
zod schema(s) where capabilities is defined (the capabilities: z.object({...})
block and the similar block at lines 104-116) to (1) validate syncFrequency
against a strict schedule format (e.g., a cron/recurrence regex or ISO-8601
interval pattern) instead of z.string().optional(), and (2) add a refinement or
.superRefine on the capabilities object to require that when canSync is true
then syncFrequency is present and valid (and optionally forbid syncFrequency
when canSync is false if desired). Use the existing schema symbol names
(capabilities, syncFrequency, canSync) so the change is easy to locate.
- Use Promise.allSettled() instead of Promise.all() in fetchTemplateFiles to handle network/CORS rejections gracefully instead of throwing - Validate syncFrequency as a 5-field cron expression and require canSync: true when syncFrequency is set - Extract TEMPLATES_REF constant for template source pinning
Summary
examples/integrations/DeleteDialogcomponentTest plan
bun run --filter @tale/platform test🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Improvements