-
Notifications
You must be signed in to change notification settings - Fork 43
feat:add component and componentLibrary #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis update introduces a new "component-library" resource to the API, including route definitions, a model schema, controller logic for deletion, and a service scaffold. The "component-library" model supports metadata, framework specification, access control, and relationships with tenants and materials. Related models—such as "materials," "tenant," and "material-category-relation"—are updated to establish new associations with the component library. The "user-components" controller has been refactored with the removal of its Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Router
participant ComponentLibraryController
participant ComponentLibraryService
participant UserComponentsService
Client->>API_Router: DELETE /component-library/:id
API_Router->>ComponentLibraryController: delete(ctx)
ComponentLibraryController->>ComponentLibraryService: Delete component library by ID
ComponentLibraryService-->>ComponentLibraryController: Deletion result
ComponentLibraryController->>UserComponentsService: Delete user components linked to library
UserComponentsService-->>ComponentLibraryController: Deletion result (or error)
ComponentLibraryController-->>API_Router: Response
API_Router-->>Client: Response
Poem
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. 🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (5)
api/component-library/services/component-library.js (1)
12-12: Consider adding a TODO or documentation for future service logic.This file is currently a placeholder. If you plan to implement business logic here, consider adding a TODO comment or brief documentation to guide future contributors.
api/component-library/models/component-library.js (1)
12-12: Add a comment or TODO to clarify the purpose of this placeholder model file.If this file is required by the framework, consider adding a comment to explain its presence for future maintainers.
api/user-components/controllers/user-components.js (1)
51-51: Document the removed methods.The
find,update, andcreatemethods have been removed from this controller. Consider adding a comment explaining why these methods were removed and where these operations are now handled.+ // Note: find, update, and create methods have been moved to component-library service async associated(ctx) {api/component-library/models/component-library.settings.json (2)
4-7: Add a descriptive description for the model.The description field is empty, which misses an opportunity to document the purpose and usage of this model.
"info": { "name": "Component-Library", - "description": "" + "description": "Collection of reusable UI components that can be shared across tenants and materials" },
76-77: Consider adding additional fields for component management.The current model is missing some fields that might be useful for component library management, such as tags, documentation links, and dependency information.
Consider adding these fields:
"materials": { "via": "component_library", "collection": "materials" - } + }, + "tags": { + "type": "json", + "description": "Array of tags for categorization and filtering" + }, + "documentationUrl": { + "type": "string", + "description": "URL to component documentation" + }, + "dependencies": { + "type": "json", + "description": "Key-value pairs of package dependencies" + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
api/component-library/config/routes.json(1 hunks)api/component-library/controllers/component-library.js(1 hunks)api/component-library/models/component-library.js(1 hunks)api/component-library/models/component-library.settings.json(1 hunks)api/component-library/services/component-library.js(1 hunks)api/material-category-relation/models/material-category-relation.settings.json(1 hunks)api/materials/models/materials.settings.json(1 hunks)api/tenant/models/tenant.settings.json(1 hunks)api/user-components/controllers/user-components.js(1 hunks)
🧰 Additional context used
🪛 ESLint
api/component-library/controllers/component-library.js
[error] 12-12: '_' is assigned a value but never used.
(no-unused-vars)
[error] 13-13: 'sanitizeEntity' is assigned a value but never used.
(no-unused-vars)
[error] 14-14: 'ERROR_TYPE' is assigned a value but never used.
(no-unused-vars)
[error] 14-14: 'UNIT_TYPE' is assigned a value but never used.
(no-unused-vars)
[error] 14-14: 'AUTH_TYPE' is assigned a value but never used.
(no-unused-vars)
[error] 15-15: 'throwErrors' is assigned a value but never used.
(no-unused-vars)
[error] 15-15: 'getPublicSuperAdmin' is assigned a value but never used.
(no-unused-vars)
[error] 15-15: 'isTenantAdmin' is assigned a value but never used.
(no-unused-vars)
[error] 17-17: 'isTruthy' is assigned a value but never used.
(no-unused-vars)
[error] 18-18: 'findAllMaterial' is assigned a value but never used.
(no-unused-vars)
[error] 19-19: 'handlePublicScope' is assigned a value but never used.
(no-unused-vars)
[error] 25-25: 'entity' is assigned a value but never used.
(no-unused-vars)
[error] 30-30: 'res' is not defined.
(no-undef)
[error] 32-32: 'res' is not defined.
(no-undef)
🔇 Additional comments (3)
api/tenant/models/tenant.settings.json (1)
46-49: Component library relation added correctly to tenant model.The many-to-many relationship with the component-library collection is well-defined and consistent with existing schema conventions.
api/material-category-relation/models/material-category-relation.settings.json (1)
20-25: Many-to-many relationship with materials defined correctly.The new
materialsattribute establishes a proper many-to-many relationship with the materials collection, including join table and dominance settings.api/materials/models/materials.settings.json (1)
95-96: Schema relationships updated correctly for material categories and component libraries.The updates to the
material_category_relationsandcomponent_libraryattributes are correct and align with the new relational structure.Also applies to: 98-100
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
api/component-library/controllers/component-library.js (2)
13-15: Remove unused imports if they're not needed elsewhere.Your code currently only references
sanitizeEntity,throwErrors, andERROR_TYPE. Consider removing any imports that aren't used.
18-36:⚠️ Potential issueFix critical issues in delete method implementation.
The current implementation has several issues:
- There's unreachable code at line 35 (the final return statement will never be executed)
- The second return statement has an extra semicolon
- The error handling could be improved to be more resilient
Apply these changes:
async delete(ctx) { const { id } = ctx.params; try{ const res = await strapi.services['component-library'].delete({ id }); try{ await strapi.services['user-components'].delete({ library: id }); } catch (error) { strapi.log.error('user-component delete failed', error); - throwErrors('user-component delete failed.', ERROR_TYPE.badRequest); + // Log the error but continue execution since the main operation succeeded + // Consider adding a warning flag in the response if needed } return sanitizeEntity(res, {model: strapi.models['component-library']}); }catch(error) { strapi.log.error('component-library delete failed', error); throwErrors('component-library delete failed.', ERROR_TYPE.badRequest); } - return sanitizeEntity(res, {model: strapi.models['component-library']});; }This implementation logs user-component deletion failures without aborting the operation, which is more resilient as it allows the primary operation to succeed even if cleanup of related records fails.
🧰 Tools
🪛 Biome (1.9.4)
[error] 35-35: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
🪛 ESLint
[error] 35-35: 'res' is not defined.
(no-undef)
[error] 35-35: Unnecessary semicolon.
(no-extra-semi)
🧹 Nitpick comments (1)
api/component-library/controllers/component-library.js (1)
18-36: Consider a more comprehensive error handling strategy.Currently, if user-component deletion fails, the entire operation fails with a 400 bad request error. This might not be the best user experience as the main operation (deleting the component library) succeeded.
Consider implementing a more nuanced error handling approach:
async delete(ctx) { const { id } = ctx.params; try { const res = await strapi.services['component-library'].delete({ id }); + + // Track warnings for partial success scenarios + const warnings = []; + try { await strapi.services['user-components'].delete({ library: id }); } catch (error) { strapi.log.error('user-component delete failed', error); - throwErrors('user-component delete failed.', ERROR_TYPE.badRequest); + warnings.push('Failed to delete some associated user components'); } - return sanitizeEntity(res, {model: strapi.models['component-library']}); + // Return a structured response with both the entity and any warnings + return { + data: sanitizeEntity(res, {model: strapi.models['component-library']}), + warnings: warnings.length ? warnings : undefined + }; } catch(error) { strapi.log.error('component-library delete failed', error); throwErrors('component-library delete failed.', ERROR_TYPE.badRequest); } - - return sanitizeEntity(res, {model: strapi.models['component-library']});; }This approach:
- Provides a more detailed response structure
- Allows the API to communicate partial success scenarios
- Gives clients information about what succeeded and what failed
🧰 Tools
🪛 Biome (1.9.4)
[error] 35-35: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
🪛 ESLint
[error] 35-35: 'res' is not defined.
(no-undef)
[error] 35-35: Unnecessary semicolon.
(no-extra-semi)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
api/component-library/controllers/component-library.js(1 hunks)api/component-library/models/component-library.settings.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- api/component-library/models/component-library.settings.json
🧰 Additional context used
🧬 Code Graph Analysis (1)
api/component-library/controllers/component-library.js (2)
api/user-components/controllers/user-components.js (4)
require(12-12)require(13-13)require(14-14)require(15-21)config/toolkits.js (1)
throwErrors(155-157)
🪛 Biome (1.9.4)
api/component-library/controllers/component-library.js
[error] 35-35: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
🪛 ESLint
api/component-library/controllers/component-library.js
[error] 35-35: 'res' is not defined.
(no-undef)
[error] 35-35: Unnecessary semicolon.
(no-extra-semi)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
api/materials/models/materials.settings.json(2 hunks)api/user-components/models/user-components.settings.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- api/materials/models/materials.settings.json
🔇 Additional comments (1)
api/user-components/models/user-components.settings.json (1)
117-120: Verify thematerialsmany-to-many relationship.
This adds a bidirectional relation to thematerialscollection viauser_components. Confirm that the reciprocal field inapi/materials/models/materials.settings.jsonis marked with"dominant": true"so Strapi manages the pivot table correctly. If already set, no further action is needed.
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Improvements
Bug Fixes