Development: Fix course messages e2e tests
#9720
Conversation
github-actions
bot
added
tests
client
WalkthroughThe changes in this pull request primarily involve modifications to the HTML template of the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
src/main/webapp/app/shared/metis/post/post.component.html (1)
152-159: LGTM! The added CSS classes improve test stability.The addition of
editIconanddeleteIconclasses provides reliable selectors for e2e tests, addressing the issue of tests failing to locate these buttons after UI changes.Consider adding
data-testidattributes for even more stable test selectors:- <button class="dropdown-item d-flex editIcon" (click)="editPosting()"> + <button class="dropdown-item d-flex editIcon" data-testid="edit-message-btn" (click)="editPosting()"> - <button class="dropdown-item d-flex deleteIcon" (click)="deletePost()"> + <button class="dropdown-item d-flex deleteIcon" data-testid="delete-message-btn" (click)="deletePost()">src/test/playwright/support/pageobjects/course/CourseMessagesPage.ts (3)
245-253: LGTM with suggestion for error handling.The implementation correctly adapts to the new UI design with context menu and provides a good fallback mechanism. However, consider adding error handling for cases where neither the context menu nor the fallback button is available.
Consider adding error handling:
const editButton = postLocator.locator('.dropdown-menu.show .editIcon'); if (await editButton.isVisible()) { await editButton.click(); } else { - await postLocator.locator('.reaction-button.edit').click(); + const fallbackButton = postLocator.locator('.reaction-button.edit'); + if (await fallbackButton.isVisible()) { + await fallbackButton.click(); + } else { + throw new Error('Neither context menu edit button nor fallback edit button is visible'); + } }
267-276: LGTM with suggestion for error handling.The implementation maintains consistency with the editMessage() method and correctly adapts to the UI changes. Consider adding similar error handling here as well.
Consider adding error handling:
const deleteButton = postLocator.locator('.dropdown-menu.show .deleteIcon'); if (await deleteButton.isVisible()) { await deleteButton.click(); } else { - await postLocator.locator('.reaction-button.delete').click(); + const fallbackButton = postLocator.locator('.reaction-button.delete'); + if (await fallbackButton.isVisible()) { + await fallbackButton.click(); + } else { + throw new Error('Neither context menu delete button nor fallback delete button is visible'); + } }
Line range hint
245-276: Consider extracting common context menu interaction logic.Both
editMessage()anddeleteMessage()share similar context menu interaction patterns. Consider extracting this logic into a reusable helper method to improve maintainability and reduce code duplication.Example implementation:
private async clickContextMenuOption( container: Locator, menuOptionSelector: string, fallbackButtonSelector: string, actionName: string ): Promise<void> { await container.locator('.message-container').click({ button: 'right' }); await this.page.waitForSelector('.dropdown-menu.show'); const menuButton = container.locator(`.dropdown-menu.show ${menuOptionSelector}`); if (await menuButton.isVisible()) { await menuButton.click(); } else { const fallbackButton = container.locator(fallbackButtonSelector); if (await fallbackButton.isVisible()) { await fallbackButton.click(); } else { throw new Error(`Neither context menu ${actionName} button nor fallback ${actionName} button is visible`); } } }Usage in the methods:
// In editMessage() await this.clickContextMenuOption(postLocator, '.editIcon', '.reaction-button.edit', 'edit'); // In deleteMessage() await this.clickContextMenuOption(postLocator, '.deleteIcon', '.reaction-button.delete', 'delete');
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
src/main/webapp/app/shared/metis/post/post.component.html(1 hunks)src/test/playwright/support/pageobjects/course/CourseMessagesPage.ts(2 hunks)
🧰 Additional context used
🔇 Additional comments (1)
src/main/webapp/app/shared/metis/post/post.component.html (1)
Line range hint 1-179: Template follows Angular syntax guidelines.
The template correctly uses the new @if syntax throughout, adhering to the coding guidelines that specify using @if over *ngIf.
Checklist
General
Motivation and Context
Some of the tests in CourseMessages.spec.ts were failing after the changes in message design. The tests could not identify the relocated button for delete/edit messages.
Description
deleteMessage()andeditMessage()functions have been updated so that tests can identify these buttons again.Steps for Testing
Check that the test runs through correctly.
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Code Review
Manual Tests
Test Coverage
Screenshots
Summary by CodeRabbit
New Features
Bug Fixes
Refactor