fix: fixed material generation and database connection for components #1152
Conversation
…t libraries - Add support for TinyVue and Element Plus component libraries in material generation - Update database connection script with new table and column names - Modify component insertion logic to match new database schema - Add version and script details for third-party component libraries
WalkthroughThe changes involve two files. In Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Build Process
participant GC as generateComponents
Caller->>GC: Initiate component generation
GC->>GC: Destructure npm object (packageName, version, exportName)
GC->>GC: Assign appInfo.materialHistory.components
GC->>GC: Construct mapItem using the destructured values
GC-->>Caller: Return processed components
sequenceDiagram
participant App as Application
participant Conn as MysqlConnection
participant DB as Database
App->>Conn: Call relationMaterialBlockHistory(id)
Conn->>DB: Execute SQL query on "r_material_history_component"
DB-->>Conn: Return result
Conn-->>App: Return data
App->>Conn: Call new relationMaterialHistory(id)
Conn->>DB: Execute updated SQL query (with deprecation notice)
DB-->>Conn: Return result
Conn-->>App: Return data
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (2)
scripts/connection.mjs (2)
205-208: Provide concrete deprecation timeline and usage details.
Using@deprecatedis a good start, but consider documenting the removal version or guiding developers how to migrate to the newerrelationMaterialHistorymethod. This ensures a clear path forward.
301-308: Hard-coded defaults might limit flexibility.
The new fields (component_metadata,library_id,tenant_id,renter_id,site_id,created_by,last_updated_by) default tonullor1. If multiple tenants or sites exist, these static values may cause conflicts.Do you want to parameterize these fields or load them from environment variables for increased flexibility?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
scripts/buildMaterials.mjs(1 hunks)scripts/connection.mjs(6 hunks)
🔇 Additional comments (2)
scripts/buildMaterials.mjs (1)
153-154: Destructuring logic looks solid.
Extractingpackage,version, andexportNamefromnpmearly on prevents undefined errors and maintains readability. The fallback to{}ensures safe defaults ifnpmis missing.scripts/connection.mjs (1)
18-20: Verify references after renaming table and altering history ID.
Renaming the table to't_component'and resettingmaterialHistoryIdto1could potentially break references in other parts of the code if they rely on the old table name or on the previous history ID.Would you like me to generate a script to find any remaining references to the old table name or history ID?
| @@ -335,7 +359,7 @@ class MysqlConnection { | |||
| * @param {object} component 组件数据 | |||
| */ | |||
| initDB(component) { | |||
| const selectSqlContent = `SELECT * FROM ${this.config.database}.${componentsTableName} WHERE component = '${component.component}'` | |||
| const selectSqlContent = `SELECT * FROM ${this.config.database}.${componentsTableName} WHERE name_en = '${component.component}'` | |||
There was a problem hiding this comment.
Mismatch between “name_en” and “component.component”.
The query filters on name_en, yet we're passing the property component.component. If component.component doesn’t match the stored name_en, the lookup will fail.
Apply this diff to maintain consistency with the actual stored column and data:
- const selectSqlContent = `SELECT * FROM ${this.config.database}.${componentsTableName} WHERE name_en = '${component.component}'`
+ const selectSqlContent = `SELECT * FROM ${this.config.database}.${componentsTableName} WHERE component = '${component.component}'`📝 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 selectSqlContent = `SELECT * FROM ${this.config.database}.${componentsTableName} WHERE name_en = '${component.component}'` | |
| const selectSqlContent = `SELECT * FROM ${this.config.database}.${componentsTableName} WHERE component = '${component.component}'` |
| const sqlContent = `INSERT INTO ${componentsTableName} (version, name, name_en, icon, description, doc_url, | ||
| screenshot, tags, keywords, dev_mode, npm, \`group\`, \`category\`, priority, snippets, | ||
| schema_fragment, configure, \`public\`, framework, isOfficial, isDefault, tiny_reserved, | ||
| tenant, createdBy, updatedBy) VALUES ${values}`.replace(/\n/g, '') | ||
| schema_fragment, configure, \`public\`, framework, is_official, is_default, tiny_reserved,component_metadata, | ||
| library_id, tenant_id, renter_id, site_id, created_by, last_updated_by) VALUES ${values}`.replace(/\n/g, '') |
There was a problem hiding this comment.
Schema mismatch for “name_en” and “component_metadata”.
This insert statement references name_en and component_metadata, but the table definition does not declare a name_en column. This will fail unless the table schema is updated accordingly.
A possible fix in your CREATE TABLE statement might be:
CREATE TABLE t_component (
...
+ name_en varchar(255) CHARACTER SET utf8 COLLATE utf8_general_ci NULL DEFAULT NULL,
component_metadata longtext CHARACTER SET utf8 COLLATE utf8_general_ci NULL,
...
);Committable suggestion skipped: line range outside the PR's diff.
| /** | ||
| * 新建的组件关联物料资产包 | ||
| * @param {number} id 新建的组件id | ||
| */ | ||
| relationMaterialHistory(id) { | ||
| const uniqSql = `SELECT * FROM \`r_material_history_component\` WHERE \`material_history_id\`=${materialHistoryId} AND \`component_id\`=${id}` | ||
| this.query(uniqSql).then((result) => { | ||
| if (!result.length) { | ||
| const sqlContent = `INSERT INTO \`r_material_history_component\` (\`material_history_id\`, \`component_id\`) VALUES (${materialHistoryId}, ${id})` | ||
| this.query(sqlContent) | ||
| } | ||
| }) | ||
| } | ||
|
|
There was a problem hiding this comment.
Use parameterized queries to prevent SQL injection.
Directly concatenating id into the SQL statement may be risky if the value is not guaranteed to be an integer. Parameterized queries will secure this method and help avoid potential injection vulnerabilities.
Possible fix using placeholders (for example in MySQL Node libraries):
- const uniqSql = `SELECT * FROM \`r_material_history_component\` WHERE \`material_history_id\`=${materialHistoryId} AND \`component_id\`=${id}`
- ...
- const sqlContent = `INSERT INTO \`r_material_history_component\` (\`material_history_id\`, \`component_id\`) VALUES (${materialHistoryId}, ${id})`
+ const uniqSql = 'SELECT * FROM `r_material_history_component` WHERE `material_history_id` = ? AND `component_id` = ?'
+ ...
+ const sqlContent = 'INSERT INTO `r_material_history_component` (`material_history_id`, `component_id`) VALUES (?, ?)'
...Committable suggestion skipped: line range outside the PR's diff.
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