Nc refactor/ds #8510
Nc refactor/ds #8510
Conversation
WalkthroughWalkthroughThe recent updates aim to improve data source management by introducing role-based UI permissions, reorganizing computed properties, and implementing conditional rendering of components. These changes extend to UI layout, styles, documentation updates, and adjustments in Playwright tests to enhance user interaction with data sources and streamline related processes. Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 18
Out of diff range and nitpick comments (3)
packages/noco-docs/docs/100.data-sources/030.sync-with-data-source.md (1)
Line range hint
21-21: Ensure the file ends with a single newline character.After the sync is complete, you can see the updated state in the `Sync State` column. Sync modal also marks `Tables metadata is in Sync` in the header.  +packages/noco-docs/docs/100.data-sources/010.data-source-overview.md (1)
Line range hint
39-39: Remove multiple consecutive blank lines for cleaner code.- -packages/noco-docs/docs/100.data-sources/040.actions-on-data-sources.md (1)
Line range hint
77-77: Possible missing comma and correct the indentation.- Enabling which you should be able to see junction tables and their table names. + Enabling which, you should be able to see junction tables and their table names.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (15)
packages/nc-gui/lang/en.jsonis excluded by!**/*.jsonpackages/noco-docs/static/img/v2/data-source/data-source-1.pngis excluded by!**/*.png,!**/*.pngpackages/noco-docs/static/img/v2/data-source/data-source-2.pngis excluded by!**/*.png,!**/*.pngpackages/noco-docs/static/img/v2/data-source/data-source-audit.pngis excluded by!**/*.png,!**/*.pngpackages/noco-docs/static/img/v2/data-source/data-source-edit.pngis excluded by!**/*.png,!**/*.pngpackages/noco-docs/static/img/v2/data-source/data-source-erd.pngis excluded by!**/*.png,!**/*.pngpackages/noco-docs/static/img/v2/data-source/data-source-hide.pngis excluded by!**/*.png,!**/*.pngpackages/noco-docs/static/img/v2/data-source/data-source-meta-sync-1.pngis excluded by!**/*.png,!**/*.pngpackages/noco-docs/static/img/v2/data-source/data-source-meta-sync-2.pngis excluded by!**/*.png,!**/*.pngpackages/noco-docs/static/img/v2/data-source/data-source-remove.pngis excluded by!**/*.png,!**/*.pngpackages/noco-docs/static/img/v2/data-source/data-source-uiacl.pngis excluded by!**/*.png,!**/*.pngpackages/noco-docs/static/img/v2/data-source/ds-connect-1.pngis excluded by!**/*.png,!**/*.pngpackages/noco-docs/static/img/v2/data-source/ds-connect-2.pngis excluded by!**/*.png,!**/*.pngpackages/noco-docs/static/img/v2/data-source/ds-connect-3.pngis excluded by!**/*.png,!**/*.pngpackages/noco-docs/static/img/v2/data-source/ds-connect-4.pngis excluded by!**/*.png,!**/*.png
Files selected for processing (17)
- packages/nc-gui/components/dashboard/TreeView/TableNode.vue (1 hunks)
- packages/nc-gui/components/dashboard/settings/DataSources.vue (5 hunks)
- packages/nc-gui/components/dashboard/settings/Modal.vue (7 hunks)
- packages/nc-gui/components/dashboard/settings/UIAcl.vue (1 hunks)
- packages/nc-gui/components/dlg/ProjectAudit.vue (1 hunks)
- packages/nc-gui/components/project/AllTables.vue (3 hunks)
- packages/nc-gui/components/project/View.vue (2 hunks)
- packages/noco-docs/docs/100.data-sources/010.data-source-overview.md (1 hunks)
- packages/noco-docs/docs/100.data-sources/020.connect-to-data-source.md (3 hunks)
- packages/noco-docs/docs/100.data-sources/030.sync-with-data-source.md (1 hunks)
- packages/noco-docs/docs/100.data-sources/040.actions-on-data-sources.md (3 hunks)
- tests/playwright/pages/Dashboard/ProjectView/Metadata.ts (2 hunks)
- tests/playwright/pages/Dashboard/ProjectView/index.ts (1 hunks)
- tests/playwright/pages/Dashboard/Settings/DataSources.ts (1 hunks)
- tests/playwright/pages/Dashboard/TreeView.ts (1 hunks)
- tests/playwright/tests/db/features/erd.spec.ts (1 hunks)
- tests/playwright/tests/db/features/metaSync.spec.ts (2 hunks)
Files skipped from review due to trivial changes (4)
- packages/nc-gui/components/dashboard/TreeView/TableNode.vue
- packages/nc-gui/components/dashboard/settings/UIAcl.vue
- packages/nc-gui/components/project/AllTables.vue
- packages/nc-gui/components/project/View.vue
Additional Context Used
LanguageTool (12)
packages/noco-docs/docs/100.data-sources/010.data-source-overview.md (3)
Near line 10: You might be missing the article “the” here.
Context: ...ternal data sources can be managed fromData Sourcestab inBase dashboard. ...
Near line 12: You might be missing the article “the” here.
Context: ...ashboard.Data Sources` tab includes following functionalities 1. Connect/manage exter...
Near line 24: You might be missing the article “the” here.
Context: ... ## AccessingData Sources1. Access Base context menu by clicking on the...i...packages/noco-docs/docs/100.data-sources/020.connect-to-data-source.md (3)
Near line 10: You might be missing the article “the” here.
Context: ...rce, follow the steps below: 1. Access Base context menu by clicking on the...i...
Near line 37: You might be missing the article “the” here.
Context: ...configuration-options). 6. Click onTest Database Connectionbutton to veri...
Near line 40: You might be missing the article “the” here.
Context: ...button will be enabled. - Click onCounnect to Data Source` button to save ...packages/noco-docs/docs/100.data-sources/030.sync-with-data-source.md (3)
Near line 10: You might be missing the article “the” here.
Context: ...u wish to sync metadata for 2. Click onMeta Syncbutton listed underActions...
Near line 11: You might be missing the article “the” here.
Context: ...u wish to sync metadata for 3. Click onReloadbutton to refresh Sync state (Op...
Near line 11: You might be missing the article “the” here.
Context: ... 3. Click onReloadbutton to refresh Sync state (Optional) 4. Any changes to the ...packages/noco-docs/docs/100.data-sources/040.actions-on-data-sources.md (3)
Near line 13: Consider adding a comma here.
Context: ...abase credentials as required :::info Please make sure database configuration parame...
Near line 42: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... UI Access Control is available only in Open Source version of NocoDB. ::: Access `Data So...
Near line 77: Possible missing comma found.
Context: ... checkboxes get enabled. - Enabling which you should be able to see junction tabl...
Additional comments not posted (10)
packages/nc-gui/components/dashboard/settings/Modal.vue (3)
115-129: Added conditional rendering for the "Data Sources" tab based on user roles.Ensure that the
isUIAllowedfunction correctly handles the role checks and that the roles are properly defined and accessible in the context where this component is used.
187-187: Conditional rendering for menu items based on user roles.Verify that the
isUIAllowedfunction is robust and covers all necessary roles to prevent unauthorized access to sensitive tabs.
221-232: Ensure that the dynamic class binding and conditional rendering are functioning as expected.Verification successful
Verification of Dynamic Class Binding and Conditional Rendering
The dynamic class binding and conditional rendering based on
selectedSubTabKeysandselectedSubTabappear to be functioning correctly. The conditions and bindings are properly set up in the code.
- The class
h-fullis applied whenselectedSubTabKeys[0] === 'dataSources'.- The component is conditionally rendered with
v-if="selectedSubTabKeys[0] === 'dataSources'".- The component's
:isattribute is dynamically set toselectedSubTab?.body.These conditions ensure that the class and component rendering are correctly tied to the state of
selectedSubTabKeys.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the dynamic class binding and conditional rendering. # Test: Check for proper binding and rendering conditions. Expect: Correct behavior based on `selectedSubTabKeys`. rg --type vue $'selectedSubTabKeys' packages/nc-gui/components/dashboard/settings/Modal.vueLength of output: 118
Script:
#!/bin/bash # Description: Verify the dynamic class binding and conditional rendering. # Test: Check for proper binding and rendering conditions. Expect: Correct behavior based on `selectedSubTabKeys` and `selectedSubTab`. rg 'selectedSubTabKeys' packages/nc-gui/components/dashboard/settings/Modal.vue rg 'selectedSubTab' packages/nc-gui/components/dashboard/settings/Modal.vueLength of output: 1243
tests/playwright/tests/db/features/metaSync.spec.ts (2)
42-44: Updated navigation and interaction paths to reflect changes in the UI structure.Ensure that the new navigation paths are correctly implemented and that all necessary elements are accessible with the updated selectors.
254-256: Updated test steps to align with the new UI structure for data source settings.Confirm that these steps correctly interact with the UI components and that the expected elements are present and interactable in the updated UI.
tests/playwright/tests/db/features/erd.spec.ts (1)
48-50: Updated navigation and interaction paths to reflect changes in the UI structure for ERD settings.Ensure that the new navigation paths are correctly implemented and that all necessary elements are accessible with the updated selectors.
tests/playwright/pages/Dashboard/TreeView.ts (1)
185-185: Updated the locator for the "Delete" menu item to include the newnc-table-deleteCSS class.Verify that the new class is correctly applied in the UI and that the locator accurately targets the elements with this class.
packages/nc-gui/components/dashboard/settings/DataSources.vue (3)
28-29: IntroducedisUIAllowedfromuseRolesfor role-based UI rendering.This change aligns with the PR's objective to refine user permissions and enhance conditional rendering based on roles.
263-485: Usage ofactiveSourceandopenedTabin the template for conditional rendering and data binding.The implementation in the template correctly utilizes the newly introduced reactive properties for dynamic UI updates. This is a good practice in Vue.js applications.
Line range hint
490-540: Updated CSS for better UI consistency and responsiveness.The CSS changes enhance the visual aspects and responsiveness of the UI components, aligning with the PR's objectives to improve the user interface.
packages/noco-docs/docs/100.data-sources/030.sync-with-data-source.md
Outdated
Show resolved
Hide resolved
packages/noco-docs/docs/100.data-sources/030.sync-with-data-source.md
Outdated
Show resolved
Hide resolved
packages/noco-docs/docs/100.data-sources/040.actions-on-data-sources.md
Outdated
Show resolved
Hide resolved
packages/noco-docs/docs/100.data-sources/020.connect-to-data-source.md
Outdated
Show resolved
Hide resolved
packages/noco-docs/docs/100.data-sources/020.connect-to-data-source.md
Outdated
Show resolved
Hide resolved
packages/noco-docs/docs/100.data-sources/020.connect-to-data-source.md
Outdated
Show resolved
Hide resolved
packages/noco-docs/docs/100.data-sources/020.connect-to-data-source.md
Outdated
Show resolved
Hide resolved
Signed-off-by: Pranav C <pranavxc@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/nc-gui/components/dlg/ProjectAudit.vue (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/nc-gui/components/dlg/ProjectAudit.vue
Signed-off-by: Pranav C <pranavxc@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- tests/playwright/pages/Dashboard/ProjectView/Metadata.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- tests/playwright/pages/Dashboard/ProjectView/Metadata.ts
…urce.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…ources.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…ource.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…ource.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…ource.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…ource.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…urce.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Out of diff range and nitpick comments (1)
packages/noco-docs/docs/100.data-sources/030.sync-with-data-source.md (1)
Line range hint
21-21: Ensure the file ends with a single newline character.+
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- packages/noco-docs/docs/100.data-sources/030.sync-with-data-source.md (1 hunks)
- tests/playwright/pages/Dashboard/ProjectView/Metadata.ts (1 hunks)
- tests/playwright/pages/Dashboard/ProjectView/index.ts (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- tests/playwright/pages/Dashboard/ProjectView/Metadata.ts
- tests/playwright/pages/Dashboard/ProjectView/index.ts
Additional comments not posted (2)
packages/noco-docs/docs/100.data-sources/030.sync-with-data-source.md (2)
9-9: Ensure lists are surrounded by blank lines for better readability.
11-11: Add the article "the" for grammatical correctness.
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- packages/noco-docs/docs/100.data-sources/020.connect-to-data-source.md (3 hunks)
- packages/noco-docs/docs/100.data-sources/030.sync-with-data-source.md (1 hunks)
- packages/noco-docs/docs/100.data-sources/040.actions-on-data-sources.md (2 hunks)
Additional comments not posted (7)
packages/noco-docs/docs/100.data-sources/030.sync-with-data-source.md (2)
8-13: Ensure lists are surrounded by blank lines for better readability.+ Access `Data Sources` tab in the `Base Settings` to sync changes done in the external data source with NocoDB. 1. Select the data source that you wish to sync metadata for 2. Click on the `Meta Sync` button listed under `Actions` column for the data source that you wish to sync metadata for 3. Click on the `Reload` button to refresh Sync state (Optional) 4. Any changes to the metadata identified will be listed in the `Sync State` column 5. Click on `Sync Now` button to sync the metadata changes +
Line range hint
21-21: Ensure the file ends with a single newline character.+packages/noco-docs/docs/100.data-sources/040.actions-on-data-sources.md (4)
9-11: Ensure lists are surrounded by blank lines for better readability.+ - Access `Data Sources` tab in the `Base Settings` - Click on `Connection Details` tab - Re-configure database credentials as required +
46-48: Ensure lists are surrounded by blank lines for better readability.+ 1. Click on `UI ACL` button listed under `Actions` column for the data source that you wish to manage UI access control for 2. On the UI ACL modal, you can see the list of tables available in the data source as rows & roles available as columns. Toggle checkboxes to enable/disable access to tables for specific roles. 3. Click on `Save` button to save the changes +
56-58: Ensure lists are surrounded by blank lines for better readability.+ Access `Data Sources` tab in the `Base Settings` to access Audit logs for the data source. - Click on `Default` datasource & then - Access `Audit` tab to view the audit logs. +
68-69: Ensure lists are surrounded by blank lines for better readability.+ - Select the data source that you wish to access ERD (Relations view) for - Click on `ERD` tab +packages/noco-docs/docs/100.data-sources/020.connect-to-data-source.md (1)
38-39: Ensure lists are surrounded by blank lines for better readability.+ 6. Click on the `Test Database Connection` button to verify the connection 7. Wait for the connection to be verified. - After test is successful, `Counnect to Data Source` button will be enabled. - Click on `Connect to Data Source` button to save the data source. +
|
Uffizzi Preview |
Signed-off-by: Pranav C <pranavxc@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- packages/nc-gui/components/dashboard/TreeView/TableNode.vue (2 hunks)
- packages/nc-gui/components/dashboard/settings/DataSources.vue (5 hunks)
- tests/playwright/pages/Dashboard/Settings/DataSources.ts (1 hunks)
- tests/playwright/pages/Dashboard/TreeView.ts (2 hunks)
- tests/playwright/tests/db/general/tableOperations.spec.ts (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- packages/nc-gui/components/dashboard/TreeView/TableNode.vue
- packages/nc-gui/components/dashboard/settings/DataSources.vue
- tests/playwright/pages/Dashboard/Settings/DataSources.ts
- tests/playwright/pages/Dashboard/TreeView.ts
Additional comments not posted (1)
tests/playwright/tests/db/general/tableOperations.spec.ts (1)
36-38: Integration of audit functionality looks good.Ensure the integration with the
openAuditmethod and theverifyRowfunctionality is thoroughly tested to confirm that audit logs are correctly captured and verified.Verification successful
Verification of
openAuditandverifyRowIntegrationThe
openAuditandverifyRowmethods are properly used in thetableOperations.spec.tsfile:
openAuditis called to open the audit log for the first row.verifyRowis used twice to verify the audit log entries.These usages indicate that the integration of the audit functionality is correctly implemented in the test suite.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of audit functionality in table operations tests. # Test: Search for the usage of `openAudit` and `verifyRow` methods. Expect: Proper usage and handling in the test suite. rg --type typescript $'openAudit|verifyRow' tests/playwright/tests/db/general/tableOperations.spec.tsLength of output: 137
Script:
#!/bin/bash # Description: Verify the integration of audit functionality in table operations tests. # Test: Search for the usage of `openAudit` and `verifyRow` methods. Expect: Proper usage and handling in the test suite. rg 'openAudit|verifyRow' tests/playwright/tests/db/general/tableOperations.spec.tsLength of output: 215
Change Summary
Provide summary of changes with issue number if any.
Change type
Test/ Verification
Provide summary of changes.
Additional information / screenshots (optional)
Anything for maintainers to be made aware of