-
Notifications
You must be signed in to change notification settings - Fork 400
feat: Modify the data source #1502
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
base: release/v2.7.x
Are you sure you want to change the base?
Conversation
WalkthroughThe changes refactor form validation and tab-switching logic in the data source management components. The main form's save function is now asynchronous, performing sequential validations and switching tabs upon errors. Utility functions for accessing form and grid components were added, and tab change handling was modularized and exposed for event-driven UI updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DataSourceForm
participant ServiceForm
participant RecordGrid
participant ParentComponent
User->>DataSourceForm: Click Save
DataSourceForm->>ServiceForm: validate()
alt Validation fails
ServiceForm-->>DataSourceForm: Error
DataSourceForm->>DataSourceForm: Switch tab to 'remote'
DataSourceForm-->>User: Show validation error
else Validation succeeds
DataSourceForm->>DataSourceForm: Validate data source name
alt Name invalid
DataSourceForm->>DataSourceForm: Switch tab to 'field'
DataSourceForm-->>User: Show name error
else Name valid
DataSourceForm->>RecordGrid: fullValidate()
alt Grid validation fails
RecordGrid-->>DataSourceForm: Error
DataSourceForm->>DataSourceForm: Switch tab to 'record'
DataSourceForm-->>User: Show grid error
else Grid valid
DataSourceForm->>DataSourceForm: Save record, update/add data source
DataSourceForm->>DataSourceForm: Clear state
DataSourceForm->>ParentComponent: Emit 'save' event, close form
end
end
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 2
🔭 Outside diff range comments (1)
packages/plugins/datasource/src/DataSourceForm.vue (1)
220-305
: Refactor validation flow for better maintainabilityThe current implementation mixes callback-based validation (
getDataSourceName().validate()
) with async/await patterns, making the code harder to follow and maintain.Consider refactoring to use consistent async/await patterns:
- getDataSourceName().validate(async (valid) => { - if (valid) { + const isNameValid = await new Promise((resolve) => { + getDataSourceName().validate((valid) => resolve(valid)) + }) + + if (!isNameValid) { + activeTabChange('field') + message({ message: '请输入有效的数据源名称', status: 'error' }) + return + } + + // Continue with the rest of the validation and save logicThis makes the flow more linear and easier to understand.
🧹 Nitpick comments (1)
packages/plugins/datasource/src/DataSourceForm.vue (1)
232-238
: Consider validation error detailsThe record grid validation might fail for multiple reasons. Consider providing more specific error feedback to help users identify and fix issues.
try { // await validate() 如果验证不通过会抛出异常,而不是返回 false - await getRecordGrid().fullValidate() + const validationResult = await getRecordGrid().fullValidate() + if (!validationResult) { + const errors = getRecordGrid().getValidationErrors() + // Log or display specific validation errors + } } catch (error) { activeTabChange('record') - throw new Error(`请先完成表格验证: ${error?.message || ''}`) + const errorDetails = error?.message || '数据验证失败' + throw new Error(`请先完成表格验证: ${errorDetails}`) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/plugins/datasource/src/DataSourceForm.vue
(5 hunks)packages/plugins/datasource/src/DataSourceList.vue
(2 hunks)packages/plugins/datasource/src/DataSourceSettingRecordList.vue
(1 hunks)packages/plugins/datasource/src/DataSourceSettings.vue
(3 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: gene9831
PR: opentiny/tiny-engine#1041
File: packages/plugins/datasource/src/DataSourceList.vue:138-138
Timestamp: 2025-01-14T10:06:25.508Z
Learning: PR #1041 in opentiny/tiny-engine is specifically for reverting Prettier v3 formatting to v2, without any logical code changes or syntax improvements.
Learnt from: rhlin
PR: opentiny/tiny-engine#1011
File: packages/canvas/render/src/RenderMain.ts:82-88
Timestamp: 2025-01-14T08:50:50.226Z
Learning: For PR #1011, the focus is on resolving conflicts and migrating code, with architectural improvements deferred for future PRs.
Learnt from: rhlin
PR: opentiny/tiny-engine#1011
File: packages/canvas/container/src/components/CanvasMenu.vue:228-232
Timestamp: 2025-01-14T04:29:26.886Z
Learning: Error handling for page switching is centralized in the `switchPageWithConfirm` method of the page service, rather than being duplicated in UI handlers like menu actions.
packages/plugins/datasource/src/DataSourceList.vue (2)
Learnt from: gene9831
PR: opentiny/tiny-engine#1011
File: packages/configurator/src/router-select-configurator/RouterSelectConfigurator.vue:63-73
Timestamp: 2025-01-14T06:49:00.797Z
Learning: In the tiny-engine project, the SvgIcon component is globally registered and available throughout Vue components without requiring explicit imports.
Learnt from: gene9831
PR: opentiny/tiny-engine#1011
File: packages/configurator/src/router-select-configurator/RouterSelectConfigurator.vue:95-98
Timestamp: 2025-01-14T06:55:59.692Z
Learning: The tiny-select component from @opentiny/vue library ensures selected options are valid internally, requiring no additional validation in the change handler.
packages/plugins/datasource/src/DataSourceSettings.vue (1)
Learnt from: gene9831
PR: opentiny/tiny-engine#1011
File: packages/configurator/src/router-select-configurator/RouterSelectConfigurator.vue:95-98
Timestamp: 2025-01-14T06:55:59.692Z
Learning: The tiny-select component from @opentiny/vue library ensures selected options are valid internally, requiring no additional validation in the change handler.
packages/plugins/datasource/src/DataSourceForm.vue (4)
Learnt from: gene9831
PR: opentiny/tiny-engine#1011
File: packages/configurator/src/router-select-configurator/RouterSelectConfigurator.vue:63-73
Timestamp: 2025-01-14T06:49:00.797Z
Learning: In the tiny-engine project, the SvgIcon component is globally registered and available throughout Vue components without requiring explicit imports.
Learnt from: chilingling
PR: opentiny/tiny-engine#837
File: packages/vue-generator/src/plugins/genDependenciesPlugin.js:66-66
Timestamp: 2024-09-30T07:51:10.036Z
Learning: In the `tiny-engine` project, `@opentiny/tiny-engine-dsl-vue` refers to the current package itself, and importing types from it may cause circular dependencies.
Learnt from: gene9831
PR: opentiny/tiny-engine#1011
File: packages/configurator/src/router-select-configurator/RouterSelectConfigurator.vue:95-98
Timestamp: 2025-01-14T06:55:59.692Z
Learning: The tiny-select component from @opentiny/vue library ensures selected options are valid internally, requiring no additional validation in the change handler.
Learnt from: gene9831
PR: opentiny/tiny-engine#1011
File: packages/configurator/src/router-select-configurator/RouterSelectConfigurator.vue:63-73
Timestamp: 2025-01-14T06:49:00.797Z
Learning: In the tiny-engine project, the SvgIcon component is globally registered using `app.component('SvgIcon', SvgIcon)` in `packages/svgs/index.js`, making it available throughout Vue components without requiring explicit imports.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (3)
packages/plugins/datasource/src/DataSourceList.vue (1)
36-36
: Good practice: Clear validation state on form openClearing validation errors when opening the data source form improves user experience by not showing stale validation messages. The optional chaining operator appropriately handles cases where the form might not be initialized yet.
Also applies to: 78-78
packages/plugins/datasource/src/DataSourceSettings.vue (1)
23-23
: Clean implementation of tab switching on editThe
changeTab
method and its integration with the edit event provide a smooth user experience by automatically switching to the relevant tab when editing records.Also applies to: 97-99, 152-152
packages/plugins/datasource/src/DataSourceForm.vue (1)
275-275
: Good addition: Clear remote config on saveAdding
dataSourceState.remoteConfig = {}
ensures that stale remote configuration doesn't persist after saving, preventing potential data inconsistencies.Also applies to: 292-292
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
Refactor