Add space-configure-from-template plugin#737
Conversation
WalkthroughThe pull request introduces the Changes
Sequence DiagramsequenceDiagram
participant Listener as Space Listener
participant Plugin as ConfigureSpaceFromTemplate
participant API as Flatfile API
Listener->>Plugin: Trigger 'space:configure' event
Plugin->>API: Retrieve Space Templates
API-->>Plugin: Return Templates
Plugin->>API: Select Oldest Template
Plugin->>API: Fetch Template Workbooks
Plugin->>API: Create Workbooks in New Space
Plugin->>API: Update Space Metadata
Plugin->>API: Create Associated Documents
Plugin->>Listener: Execute Callback Function
The sequence diagram illustrates the core workflow of the ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
plugins/space-configure-from-template/src/space.configure.from.template.ts (2)
81-81: Use meaningful progress messages withtickfunctionThe progress message
'Workbook created'at 50% might be misleading if multiple workbooks are being created. Consider updating the message to reflect the actual progress or specifying how many workbooks have been created.Apply this diff to improve the progress message:
- await tick(50, 'Workbook created') + await tick(50, 'Workbooks created')🧰 Tools
🪛 GitHub Actions: Lint
[warning] Code style issues found. Prettier formatting check failed. Run Prettier with --write to fix.
1-105: Address code formatting issuesThe Prettier formatting check failed in the CI pipeline. Please run Prettier with the
--writeflag to automatically format the code according to the project's style guidelines.🧰 Tools
🪛 GitHub Actions: Lint
[warning] Code style issues found. Prettier formatting check failed. Run Prettier with --write to fix.
plugins/space-configure-from-template/src/space.configure.e2e.spec.ts (2)
41-41: Awaitdriver.shutdown()to ensure proper teardownThe
driver.shutdown()function may be asynchronous. To guarantee that the driver shuts down completely before moving on, consider awaiting this operation.Apply this diff to await the shutdown:
- driver.shutdown() + await driver.shutdown()
48-50: Reset mocks inafterEachhookWhile
listener.reset()clears the listener state, it doesn't reset the mock functions likemockFn. Consider addingvi.clearAllMocks()in theafterEachhook to ensure that mocks are reset between tests.Apply this diff to reset mocks:
afterEach(() => { listener.reset() + vi.clearAllMocks() })plugins/space-configure-from-template/README.md (1)
16-16: Adjust heading levels to maintain proper hierarchyThe headings at lines 16, 35, and 56 are level 4 (
####), but they should be level 3 (###) to maintain a proper heading structure, as they follow a level 2 heading.Apply this diff to fix the heading levels:
-#### `callback` - `function` +### `callback` - `function` -#### Install +### Install -#### Example with Callback +### Example with CallbackAlso applies to: 35-35, 56-56
🧰 Tools
🪛 Markdownlint (0.37.0)
16-16: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time(MD001, heading-increment)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.json,!**/*.jsonplugins/space-configure-from-template/package.jsonis excluded by!**/*.json
📒 Files selected for processing (6)
plugins/space-configure-from-template/README.md(1 hunks)plugins/space-configure-from-template/src/index.ts(1 hunks)plugins/space-configure-from-template/src/space.configure.e2e.spec.ts(1 hunks)plugins/space-configure-from-template/src/space.configure.from.template.ts(1 hunks)plugins/space-configure-from-template/tsup.config.mjs(1 hunks)plugins/space-configure-from-template/vitest.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- plugins/space-configure-from-template/src/index.ts
- plugins/space-configure-from-template/tsup.config.mjs
- plugins/space-configure-from-template/vitest.config.ts
🧰 Additional context used
🪛 GitHub Actions: Lint
plugins/space-configure-from-template/src/space.configure.from.template.ts
[warning] Code style issues found. Prettier formatting check failed. Run Prettier with --write to fix.
🪛 LanguageTool
plugins/space-configure-from-template/README.md
[misspelling] ~33-~33: This word is normally spelled as one.
Context: ... Designed for server-side listeners, it auto-configures the Space using the supplied settings. ...
(EN_COMPOUNDS_AUTO_CONFIGURES)
🪛 Markdownlint (0.37.0)
plugins/space-configure-from-template/README.md
16-16: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time
(MD001, heading-increment)
35-35: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time
(MD001, heading-increment)
56-56: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time
(MD001, heading-increment)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Setup
- GitHub Check: Nullify Dependencies
🔇 Additional comments (1)
plugins/space-configure-from-template/README.md (1)
33-33: Correct the spelling of 'auto-configures'The term "auto-configures" is typically spelled as a single word without a hyphen: "autoconfigures".
[typo]
Apply this diff to fix the spelling:
-Designed for server-side listeners, it auto-configures the Space using the supplied settings. +Designed for server-side listeners, it autoconfigures the Space using the supplied settings.🧰 Tools
🪛 LanguageTool
[misspelling] ~33-~33: This word is normally spelled as one.
Context: ... Designed for server-side listeners, it auto-configures the Space using the supplied settings. ...(EN_COMPOUNDS_AUTO_CONFIGURES)
plugins/space-configure-from-template/src/space.configure.from.template.ts
Outdated
Show resolved
Hide resolved
plugins/space-configure-from-template/src/space.configure.from.template.ts
Outdated
Show resolved
Hide resolved
….template.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
plugins/space-configure-from-template/src/space.configure.from.template.ts (5)
1-6: Consider dependency injection for the API client.Instead of creating a global API client instance, consider injecting it as a parameter to allow for better testing and flexibility.
-const api = new FlatfileClient() +export function configureSpaceFromTemplate( + options: { + api?: FlatfileClient; + callback?: (event: FlatfileEvent, workbookIds: string[], tick: TickFunction) => any | Promise<any>; + } +) { + const api = options.api || new FlatfileClient()🧰 Tools
🪛 GitHub Actions: Lint
[warning] Code style issues found. Prettier formatting check failed. Run Prettier with --write to fix.
8-22: Update JSDoc to match implementation.The documentation mentions a "setup factory" and "setup configuration" which aren't part of the current implementation. Please update the documentation to accurately reflect the template-based configuration approach.
🧰 Tools
🪛 GitHub Actions: Lint
[warning] Code style issues found. Prettier formatting check failed. Run Prettier with --write to fix.
23-29: Consider stricter typing for callback return value.The callback's return type
any | Promise<any>is very permissive. Consider usingvoid | Promise<void>if the return value isn't used, or define a specific return type if it is.🧰 Tools
🪛 GitHub Actions: Lint
[warning] Code style issues found. Prettier formatting check failed. Run Prettier with --write to fix.
48-50: Reconsider template selection strategy.Always selecting the oldest template might not be the best strategy. Consider:
- Adding a parameter to specify the template ID
- Adding metadata to identify the default template
- Using the most recently updated template instead
🧰 Tools
🪛 GitHub Actions: Lint
[warning] Code style issues found. Prettier formatting check failed. Run Prettier with --write to fix.
98-102: Improve callback execution handling.Consider adding:
- Error handling for callback execution
- Final progress update after successful completion
- More detailed return information
- if (callback) { - await callback(event, workbookIds, tick) - } - return { info: 'Space configured' } + try { + if (callback) { + await callback(event, workbookIds, tick) + } + await tick(100, 'Configuration completed') + return { + info: 'Space configured', + workbookIds, + spaceId, + templateId: spaceTemplateId + } + } catch (error) { + throw new Error(`Callback execution failed: ${error.message}`) + }🧰 Tools
🪛 GitHub Actions: Lint
[warning] Code style issues found. Prettier formatting check failed. Run Prettier with --write to fix.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugins/space-configure-from-template/src/space.configure.from.template.ts(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Lint
plugins/space-configure-from-template/src/space.configure.from.template.ts
[warning] Code style issues found. Prettier formatting check failed. Run Prettier with --write to fix.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Setup
- GitHub Check: Nullify Dependencies
🔇 Additional comments (1)
plugins/space-configure-from-template/src/space.configure.from.template.ts (1)
37-42: UpdateisCollaborativetoisTemplate.The TODO comment indicates that
isCollaborativeshould be changed toisTemplate.🧰 Tools
🪛 GitHub Actions: Lint
[warning] Code style issues found. Prettier formatting check failed. Run Prettier with --write to fix.
plugins/space-configure-from-template/src/space.configure.from.template.ts
Show resolved
Hide resolved
plugins/space-configure-from-template/src/space.configure.from.template.ts
Show resolved
Hide resolved
plugins/space-configure-from-template/src/space.configure.from.template.ts
Show resolved
Hide resolved
plugins/space-configure-from-template/src/space.configure.from.template.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
plugins/space-configure-from-template/src/space.configure.from.template.ts (2)
1-6: Consider injecting FlatfileClient for better testability.Creating the API client at the module level makes the code harder to test. Consider passing it as a parameter to the
configureSpaceFromTemplatefunction.-const api = new FlatfileClient() +export function configureSpaceFromTemplate( + callback?: (event: FlatfileEvent, workbookIds: string[], tick: TickFunction) => any | Promise<any>, + api: FlatfileClient = new FlatfileClient() +) {
44-46: Enhance error handling with more context.The error message could be more descriptive by including the app namespace.
- throw new Error('No space template found') + throw new Error(`No space template found for namespace: ${app.data.namespace}`)plugins/space-configure-from-template/src/space.configure.from.template.e2e.spec.ts (1)
28-36: Add error handling to test setup.The setup process should handle potential failures gracefully to ensure proper cleanup.
beforeAll(async () => { - await driver.start() - listener.mount(driver) + try { + await driver.start() + listener.mount(driver) - listener.use(configureSpaceFromTemplate(mockFn)) + listener.use(configureSpaceFromTemplate(mockFn)) - const space = await setupSpace() - spaceId = space.id + const space = await setupSpace() + spaceId = space.id + } catch (error) { + console.error('Test setup failed:', error) + throw error + } })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
plugins/space-configure-from-template/src/space.configure.from.template.e2e.spec.ts(1 hunks)plugins/space-configure-from-template/src/space.configure.from.template.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Setup
- GitHub Check: Lint
- GitHub Check: Nullify Dependencies
🔇 Additional comments (4)
plugins/space-configure-from-template/src/space.configure.from.template.ts (4)
37-42: UpdateisCollaborativetoisTemplate.The TODO comment indicates this needs to be updated.
84-89: 🛠️ Refactor suggestionAdd validation and error handling for space metadata update.
The space update should validate the settings object and handle potential API errors.
- await api.spaces.update(spaceId, { - primaryWorkbookId: - workbookIds && workbookIds.length > 0 ? workbookIds[0] : '', - settings: spaceTemplate.settings, - }) + try { + if (!workbookIds?.length) { + throw new Error('No workbooks were created') + } + await api.spaces.update(spaceId, { + primaryWorkbookId: workbookIds[0], + settings: spaceTemplate.settings || {} + }) + await tick(75, 'Space metadata updated') + } catch (error) { + throw new Error(`Failed to update space metadata: ${error.message}`) + }Likely invalid or redundant comment.
74-82: 🛠️ Refactor suggestionImprove workbook creation error handling and progress reporting.
The current implementation doesn't handle individual workbook creation failures and provides only a single progress update.
const workbookIds = await Promise.all( - workbookConfigs.map(async (workbookConfig) => { - const workbook = await api.workbooks.create(workbookConfig) - return workbook.data.id + workbookConfigs.map(async (workbookConfig, index) => { + try { + const workbook = await api.workbooks.create(workbookConfig) + await tick(25 + (25 * index / workbookConfigs.length), + `Created workbook ${index + 1} of ${workbookConfigs.length}`) + return workbook.data.id + } catch (error) { + throw new Error(`Failed to create workbook ${index + 1}: ${error.message}`) + } }) ) -await tick(50, 'Workbook created')Likely invalid or redundant comment.
91-98: 🛠️ Refactor suggestionImprove document handling with pagination and progress updates.
The current implementation doesn't handle pagination for document listing and lacks progress updates during document creation.
- const documents = await api.documents.list(spaceTemplateId) + const allDocuments: Flatfile.Document[] = [] + let page = 1 + while (true) { + const documents = await api.documents.list(spaceTemplateId, { page }) + if (documents.data.length === 0) break + allDocuments.push(...documents.data) + page++ + } - for (const document of documents.data) { - await api.documents.create(spaceId, document) + for (const [index, document] of allDocuments.entries()) { + try { + await api.documents.create(spaceId, document) + await tick(75 + (20 * index / allDocuments.length), + `Created document ${index + 1} of ${allDocuments.length}`) + } catch (error) { + throw new Error(`Failed to create document: ${error.message}`) + } }Likely invalid or redundant comment.
plugins/space-configure-from-template/src/space.configure.from.template.ts
Show resolved
Hide resolved
plugins/space-configure-from-template/src/space.configure.from.template.e2e.spec.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
plugins/space-configure-from-template/src/space.configure.from.template.ts (3)
6-6: Consider dependency injection for the API client.Instead of creating a global API client instance, consider passing it as a parameter to the
configureSpaceFromTemplatefunction. This would improve testability and allow for better dependency management.-const api = new FlatfileClient() - -export function configureSpaceFromTemplate( +export function configureSpaceFromTemplate( + api: FlatfileClient = new FlatfileClient(), callback?: ( event: FlatfileEvent, workbookIds: string[], tick: TickFunction ) => any | Promise<any> )
8-29: Enhance documentation and type safety.
- Add usage examples in the JSDoc to demonstrate the callback pattern.
- Consider using a more specific return type for the callback instead of
any./** * `configureSpaceFromTemplate` is a higher-order function that creates a plugin to configure a * workspace using the Flatfile API. This function takes a setup factory that may either * be a static setup configuration or a function that creates a setup configuration * given a FlatfileEvent. * * When the job:ready for space:configure is emitted, the plugin will extract the necessary IDs, create a * configuration using the provided setup factory, create a workbook with this configuration, * and update the space with the workbook ID as the primary workbook ID. * * @param {Function} callback - A callback function that is called after the workbook is created. * + * @example + * ```typescript + * const plugin = configureSpaceFromTemplate(async (event, workbookIds, tick) => { + * await tick(90, 'Performing post-configuration tasks'); + * // Custom logic here + * }); + * ``` * * @returns {Function} Returns a function that takes a FlatfileListener, adding a "space:configure" event listener * and configuring the space when the event is emitted. */ - callback?: (event: FlatfileEvent, workbookIds: string[], tick: TickFunction) => any | Promise<any> + callback?: (event: FlatfileEvent, workbookIds: string[], tick: TickFunction) => void | Promise<void>
108-108: Enhance the response message with relevant details.The current success message could be more informative by including details about the configured space.
- return { info: 'Space configured' } + return { + info: 'Space configured successfully', + details: { + spaceId, + workbookCount: workbookIds.length, + documentCount: documents.data.length, + primaryWorkbookId: workbookIds[0] || null + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugins/space-configure-from-template/src/space.configure.from.template.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Setup
- GitHub Check: Nullify Dependencies
🔇 Additional comments (1)
plugins/space-configure-from-template/src/space.configure.from.template.ts (1)
39-39: UpdateisCollaborativetoisTemplateThere's a TODO comment indicating that
isCollaborativeshould be changed toisTemplate. IfisTemplateis now available, please update the code to reflect this change to ensure the plugin functions correctly.
| await api.spaces.update(spaceId, { | ||
| primaryWorkbookId: | ||
| workbookIds && workbookIds.length > 0 ? workbookIds[0] : '', | ||
| settings: spaceTemplate.settings, | ||
| metadata: spaceTemplate.metadata, | ||
| actions: spaceTemplate.actions, | ||
| access: spaceTemplate.access, | ||
| labels: spaceTemplate.labels, | ||
| translationsPath: spaceTemplate.translationsPath, | ||
| languageOverride: spaceTemplate.languageOverride, | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add property validation before copying template properties.
The code assumes all properties from the template are safe to copy. Consider adding validation and using optional chaining to handle potentially undefined properties safely.
await api.spaces.update(spaceId, {
primaryWorkbookId:
workbookIds && workbookIds.length > 0 ? workbookIds[0] : '',
- settings: spaceTemplate.settings,
- metadata: spaceTemplate.metadata,
- actions: spaceTemplate.actions,
- access: spaceTemplate.access,
- labels: spaceTemplate.labels,
- translationsPath: spaceTemplate.translationsPath,
- languageOverride: spaceTemplate.languageOverride,
+ ...(spaceTemplate.settings && { settings: spaceTemplate.settings }),
+ ...(spaceTemplate.metadata && { metadata: spaceTemplate.metadata }),
+ ...(spaceTemplate.actions && { actions: spaceTemplate.actions }),
+ ...(spaceTemplate.access && { access: spaceTemplate.access }),
+ ...(spaceTemplate.labels && { labels: spaceTemplate.labels }),
+ ...(spaceTemplate.translationsPath && { translationsPath: spaceTemplate.translationsPath }),
+ ...(spaceTemplate.languageOverride && { languageOverride: spaceTemplate.languageOverride }),
})📝 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.
| await api.spaces.update(spaceId, { | |
| primaryWorkbookId: | |
| workbookIds && workbookIds.length > 0 ? workbookIds[0] : '', | |
| settings: spaceTemplate.settings, | |
| metadata: spaceTemplate.metadata, | |
| actions: spaceTemplate.actions, | |
| access: spaceTemplate.access, | |
| labels: spaceTemplate.labels, | |
| translationsPath: spaceTemplate.translationsPath, | |
| languageOverride: spaceTemplate.languageOverride, | |
| }) | |
| await api.spaces.update(spaceId, { | |
| primaryWorkbookId: | |
| workbookIds && workbookIds.length > 0 ? workbookIds[0] : '', | |
| ...(spaceTemplate.settings && { settings: spaceTemplate.settings }), | |
| ...(spaceTemplate.metadata && { metadata: spaceTemplate.metadata }), | |
| ...(spaceTemplate.actions && { actions: spaceTemplate.actions }), | |
| ...(spaceTemplate.access && { access: spaceTemplate.access }), | |
| ...(spaceTemplate.labels && { labels: spaceTemplate.labels }), | |
| ...(spaceTemplate.translationsPath && { translationsPath: spaceTemplate.translationsPath }), | |
| ...(spaceTemplate.languageOverride && { languageOverride: spaceTemplate.languageOverride }), | |
| }) |
| jobHandler('space:configure', async (event, tick) => { | ||
| const { spaceId, environmentId, appId } = event.context | ||
| const app = await api.apps.get(appId) | ||
|
|
||
| // Get all the space templates for the app sorted by creation date, oldest first | ||
| const spaceTemplates = await api.spaces.list({ | ||
| namespace: app.data.namespace, | ||
| isCollaborative: true, // TODO: change to isTemplate | ||
| sortField: 'createdAt', | ||
| sortDirection: 'asc', | ||
| }) | ||
|
|
||
| if (spaceTemplates.data.length === 0) { | ||
| throw new Error('No space template found') | ||
| } | ||
|
|
||
| // Get the oldest space template | ||
| const spaceTemplate = spaceTemplates.data[0] | ||
| const spaceTemplateId = spaceTemplate.id | ||
|
|
||
| // Get all the workbooks for the space template | ||
| const workbooks = await api.workbooks.list({ | ||
| spaceId: spaceTemplateId, | ||
| }) | ||
|
|
||
| // Convert workbooks from the template to workbook configs | ||
| const workbookConfigs: Flatfile.CreateWorkbookConfig[] = | ||
| workbooks.data.map((workbook) => ({ | ||
| name: workbook.name, | ||
| labels: workbook.labels, | ||
| spaceId: spaceId, | ||
| environmentId: environmentId, | ||
| namespace: app.data.namespace, | ||
| sheets: workbook.sheets.map((sheet) => ({ | ||
| ...sheet.config, | ||
| })), | ||
| actions: workbook.actions, | ||
| settings: workbook.settings, | ||
| metadata: workbook.metadata, | ||
| treatments: workbook.treatments, | ||
| })) | ||
|
|
||
| // Create the workbooks | ||
| const workbookIds = await Promise.all( | ||
| workbookConfigs.map(async (workbookConfig) => { | ||
| const workbook = await api.workbooks.create(workbookConfig) | ||
| return workbook.data.id | ||
| }) | ||
| ) | ||
|
|
||
| await tick(50, 'Workbook created') | ||
|
|
||
| // Set some metadata on the space | ||
| await api.spaces.update(spaceId, { | ||
| primaryWorkbookId: | ||
| workbookIds && workbookIds.length > 0 ? workbookIds[0] : '', | ||
| settings: spaceTemplate.settings, | ||
| metadata: spaceTemplate.metadata, | ||
| actions: spaceTemplate.actions, | ||
| access: spaceTemplate.access, | ||
| labels: spaceTemplate.labels, | ||
| translationsPath: spaceTemplate.translationsPath, | ||
| languageOverride: spaceTemplate.languageOverride, | ||
| }) | ||
|
|
||
| // Get all the documents for the space template... | ||
| const documents = await api.documents.list(spaceTemplateId) | ||
|
|
||
| // ...and create them in the new space | ||
| for (const document of documents.data) { | ||
| await api.documents.create(spaceId, document) | ||
| } | ||
|
|
||
| if (callback) { | ||
| await callback(event, workbookIds, tick) | ||
| } | ||
| return { info: 'Space configured' } | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implement comprehensive error handling.
The job handler would benefit from a try-catch block with proper error reporting and cleanup in case of failures at any stage (template retrieval, workbook creation, document copying).
jobHandler('space:configure', async (event, tick) => {
+ try {
const { spaceId, environmentId, appId } = event.context
// ... existing code ...
return { info: 'Space configured' }
+ } catch (error) {
+ const errorMessage = error instanceof Error ? error.message : 'Unknown error occurred'
+ await tick(100, `Configuration failed: ${errorMessage}`)
+ throw new Error(`Failed to configure space from template: ${errorMessage}`)
+ }
})📝 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.
| jobHandler('space:configure', async (event, tick) => { | |
| const { spaceId, environmentId, appId } = event.context | |
| const app = await api.apps.get(appId) | |
| // Get all the space templates for the app sorted by creation date, oldest first | |
| const spaceTemplates = await api.spaces.list({ | |
| namespace: app.data.namespace, | |
| isCollaborative: true, // TODO: change to isTemplate | |
| sortField: 'createdAt', | |
| sortDirection: 'asc', | |
| }) | |
| if (spaceTemplates.data.length === 0) { | |
| throw new Error('No space template found') | |
| } | |
| // Get the oldest space template | |
| const spaceTemplate = spaceTemplates.data[0] | |
| const spaceTemplateId = spaceTemplate.id | |
| // Get all the workbooks for the space template | |
| const workbooks = await api.workbooks.list({ | |
| spaceId: spaceTemplateId, | |
| }) | |
| // Convert workbooks from the template to workbook configs | |
| const workbookConfigs: Flatfile.CreateWorkbookConfig[] = | |
| workbooks.data.map((workbook) => ({ | |
| name: workbook.name, | |
| labels: workbook.labels, | |
| spaceId: spaceId, | |
| environmentId: environmentId, | |
| namespace: app.data.namespace, | |
| sheets: workbook.sheets.map((sheet) => ({ | |
| ...sheet.config, | |
| })), | |
| actions: workbook.actions, | |
| settings: workbook.settings, | |
| metadata: workbook.metadata, | |
| treatments: workbook.treatments, | |
| })) | |
| // Create the workbooks | |
| const workbookIds = await Promise.all( | |
| workbookConfigs.map(async (workbookConfig) => { | |
| const workbook = await api.workbooks.create(workbookConfig) | |
| return workbook.data.id | |
| }) | |
| ) | |
| await tick(50, 'Workbook created') | |
| // Set some metadata on the space | |
| await api.spaces.update(spaceId, { | |
| primaryWorkbookId: | |
| workbookIds && workbookIds.length > 0 ? workbookIds[0] : '', | |
| settings: spaceTemplate.settings, | |
| metadata: spaceTemplate.metadata, | |
| actions: spaceTemplate.actions, | |
| access: spaceTemplate.access, | |
| labels: spaceTemplate.labels, | |
| translationsPath: spaceTemplate.translationsPath, | |
| languageOverride: spaceTemplate.languageOverride, | |
| }) | |
| // Get all the documents for the space template... | |
| const documents = await api.documents.list(spaceTemplateId) | |
| // ...and create them in the new space | |
| for (const document of documents.data) { | |
| await api.documents.create(spaceId, document) | |
| } | |
| if (callback) { | |
| await callback(event, workbookIds, tick) | |
| } | |
| return { info: 'Space configured' } | |
| }) | |
| jobHandler('space:configure', async (event, tick) => { | |
| try { | |
| const { spaceId, environmentId, appId } = event.context | |
| const app = await api.apps.get(appId) | |
| // Get all the space templates for the app sorted by creation date, oldest first | |
| const spaceTemplates = await api.spaces.list({ | |
| namespace: app.data.namespace, | |
| isCollaborative: true, // TODO: change to isTemplate | |
| sortField: 'createdAt', | |
| sortDirection: 'asc', | |
| }) | |
| if (spaceTemplates.data.length === 0) { | |
| throw new Error('No space template found') | |
| } | |
| // Get the oldest space template | |
| const spaceTemplate = spaceTemplates.data[0] | |
| const spaceTemplateId = spaceTemplate.id | |
| // Get all the workbooks for the space template | |
| const workbooks = await api.workbooks.list({ | |
| spaceId: spaceTemplateId, | |
| }) | |
| // Convert workbooks from the template to workbook configs | |
| const workbookConfigs: Flatfile.CreateWorkbookConfig[] = | |
| workbooks.data.map((workbook) => ({ | |
| name: workbook.name, | |
| labels: workbook.labels, | |
| spaceId: spaceId, | |
| environmentId: environmentId, | |
| namespace: app.data.namespace, | |
| sheets: workbook.sheets.map((sheet) => ({ | |
| ...sheet.config, | |
| })), | |
| actions: workbook.actions, | |
| settings: workbook.settings, | |
| metadata: workbook.metadata, | |
| treatments: workbook.treatments, | |
| })) | |
| // Create the workbooks | |
| const workbookIds = await Promise.all( | |
| workbookConfigs.map(async (workbookConfig) => { | |
| const workbook = await api.workbooks.create(workbookConfig) | |
| return workbook.data.id | |
| }) | |
| ) | |
| await tick(50, 'Workbook created') | |
| // Set some metadata on the space | |
| await api.spaces.update(spaceId, { | |
| primaryWorkbookId: | |
| workbookIds && workbookIds.length > 0 ? workbookIds[0] : '', | |
| settings: spaceTemplate.settings, | |
| metadata: spaceTemplate.metadata, | |
| actions: spaceTemplate.actions, | |
| access: spaceTemplate.access, | |
| labels: spaceTemplate.labels, | |
| translationsPath: spaceTemplate.translationsPath, | |
| languageOverride: spaceTemplate.languageOverride, | |
| }) | |
| // Get all the documents for the space template... | |
| const documents = await api.documents.list(spaceTemplateId) | |
| // ...and create them in the new space | |
| for (const document of documents.data) { | |
| await api.documents.create(spaceId, document) | |
| } | |
| if (callback) { | |
| await callback(event, workbookIds, tick) | |
| } | |
| return { info: 'Space configured' } | |
| } catch (error) { | |
| const errorMessage = error instanceof Error ? error.message : 'Unknown error occurred' | |
| await tick(100, `Configuration failed: ${errorMessage}`) | |
| throw new Error(`Failed to configure space from template: ${errorMessage}`) | |
| } | |
| }) |
ncronquist
left a comment
ncronquist
left a comment
There was a problem hiding this comment.
Approved pending isAppTemplate change.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
plugins/space-configure-from-template/src/space.configure.from.template.ts (4)
6-6: Consider injecting the API client instead of using a global instance.Creating a global API client instance makes the code harder to test and less flexible. Consider passing the client as a parameter to improve testability and dependency management.
-const api = new FlatfileClient() +export function configureSpaceFromTemplate( + options: { + api?: FlatfileClient, + callback?: (event: FlatfileEvent, workbookIds: string[], tick: TickFunction) => any | Promise<any> + } +) { + const api = options.api || new FlatfileClient()
8-22: Update documentation to match implementation.The JSDoc mentions concepts like "setup factory" and "setup configuration" that aren't used in the actual implementation. Please revise the documentation to accurately reflect the current functionality.
23-29: Add explicit return type for better type safety.Consider adding an explicit return type to improve type safety and documentation.
export function configureSpaceFromTemplate( callback?: ( event: FlatfileEvent, workbookIds: string[], tick: TickFunction ) => any | Promise<any> -) { +): (listener: FlatfileListener) => void {
1-112: Consider implementing a more robust error recovery mechanism.The plugin would benefit from a comprehensive error recovery strategy that includes:
- Rolling back created resources on failure
- Retrying failed operations with exponential backoff
- Maintaining an audit log of actions
- Implementing proper cleanup on partial failures
This would make the plugin more resilient in production environments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.json,!**/*.jsonpackage.jsonis excluded by!**/*.json
📒 Files selected for processing (1)
plugins/space-configure-from-template/src/space.configure.from.template.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Setup
- GitHub Check: Lint
- GitHub Check: Nullify Dependencies
plugins/space-configure-from-template/src/space.configure.from.template.ts
Show resolved
Hide resolved
plugins/space-configure-from-template/src/space.configure.from.template.ts
Show resolved
Hide resolved
2 participants
Please explain how to summarize this PR for the Changelog:
Tell code reviewer how and what to test: