test: UI ACL #8927
test: UI ACL #8927
Conversation
WalkthroughWalkthroughThe changes primarily include introducing a method for verifying ACL settings in the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant ds as DataSourcesPage
participant acl as AclPage
Note over U,ds: Accessing ACL Tab
U->>ds: openAcl({ dataSourceName })
ds->>ds: Click data source row
ds->>ds: Click ACL tab
Note over U,acl: Verifying ACL
U->>acl: verify({ table, role, expectedValue })
acl->>acl: Assert checkbox state
This diagram illustrates the sequence of user interactions with the 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: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- tests/playwright/pages/Dashboard/Settings/Acl.ts (2 hunks)
- tests/playwright/pages/Dashboard/Settings/DataSources.ts (1 hunks)
- tests/playwright/tests/db/usersAccounts/accountTokenManagement.spec.ts (1 hunks)
- tests/playwright/tests/db/usersAccounts/rolesPreview.spec.ts (3 hunks)
Files skipped from review due to trivial changes (1)
- tests/playwright/tests/db/usersAccounts/accountTokenManagement.spec.ts
Additional comments not posted (3)
tests/playwright/tests/db/usersAccounts/rolesPreview.spec.ts (3)
6-10: LGTM! New imports are necessary and correct.The new imports for
Api,AccountUsersPage,AccountPage,LoginPage, andisEEare necessary for the new functionality.
11-11: LGTM! New variable declarations are necessary and correct.The new variable declarations for
api,accountPage,accountUsersPage,loginPage, anduserEmailare necessary for the new functionality.Also applies to: 21-23, 26-26
Line range hint
84-139:
LGTM! Ensure comprehensive test coverage.The new test logic for ACL configuration, verification, and user sign-in procedures looks good. Ensure that the tests cover all edge cases and potential errors.
| async verify({ table, role, expectedValue }: { table: string; role: string; expectedValue: boolean }) { | ||
| const isChecked = await this.get().locator(`.nc-acl-${table}-${role}-chkbox`).isChecked(); | ||
| expect(isChecked).toBe(expectedValue); | ||
| } |
There was a problem hiding this comment.
Ensure proper error handling for Playwright actions.
The verify method looks good, but consider adding error handling to manage potential issues with Playwright actions.
async verify({ table, role, expectedValue }: { table: string; role: string; expectedValue: boolean }) {
try {
const isChecked = await this.get().locator(`.nc-acl-${table}-${role}-chkbox`).isChecked();
expect(isChecked).toBe(expectedValue);
} catch (error) {
throw new Error(`Verification failed for table: ${table}, role: ${role}. Error: ${error.message}`);
}
}| await this.get().locator('.ds-table-row', { hasText: dataSourceName }).click(); | ||
| await this.get().locator('[data-testid="nc-acl-tab"]').click(); |
There was a problem hiding this comment.
Ensure proper error handling for Playwright actions.
The openAcl method looks good, but consider adding error handling to manage potential issues with Playwright actions.
async openAcl({ dataSourceName = defaultBaseName }: { dataSourceName?: string } = {}) {
try {
await this.get().locator('.ds-table-row', { hasText: dataSourceName }).click();
await this.get().locator('[data-testid="nc-acl-tab"]').click();
} catch (error) {
throw new Error(`Failed to open ACL for data source: ${dataSourceName}. Error: ${error.message}`);
}
}| accountPage = new AccountPage(page); | ||
| accountUsersPage = new AccountUsersPage(accountPage); | ||
| loginPage = new LoginPage(accountPage.rootPage); | ||
|
|
||
| try { | ||
| api = new Api({ | ||
| baseURL: `http://localhost:8080/`, | ||
| headers: { | ||
| 'xc-auth': context.token, | ||
| }, | ||
| }); | ||
| } catch (e) { | ||
| console.log(e); | ||
| } | ||
|
|
||
| userEmail = accountUsersPage.prefixEmail('uiACL@nocodb.com'); | ||
|
|
There was a problem hiding this comment.
Ensure proper error handling in setup logic.
The setup logic looks good, but consider improving error handling for API initialization and user management.
test.beforeEach(async ({ page }) => {
context = await setup({ page, isEmptyProject: false });
dashboard = new DashboardPage(page, context.base);
toolbar = dashboard.grid.toolbar;
settings = dashboard.settings;
accountPage = new AccountPage(page);
accountUsersPage = new AccountUsersPage(accountPage);
loginPage = new LoginPage(accountPage.rootPage);
try {
api = new Api({
baseURL: `http://localhost:8080/`,
headers: {
'xc-auth': context.token,
},
});
} catch (error) {
console.error(`Failed to initialize API. Error: ${error.message}`);
}
userEmail = accountUsersPage.prefixEmail('uiACL@nocodb.com');
try {
// check if user already exists; if so- remove them
const user = await api.auth.baseUserList(context.base.id);
if (user.users.list.length > 0) {
const u = user.users.list.find((u: any) => u.email === userEmail);
if (u) {
await api.auth.baseUserRemove(context.base.id, u.id);
}
}
// create user if not exists
try {
await api.auth.signup({
email: userEmail,
password: '12345678',
});
} catch (error) {
console.error(`Failed to sign up user. Error: ${error.message}`);
}
await api.auth.baseUserAdd(context.base.id, {
roles: 'editor',
email: userEmail,
});
} catch (error) {
console.error(`Failed to manage user. Error: ${error.message}`);
}
});
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