test(web): publish and unpublish items from table #1312
Conversation
WalkthroughThe changes in this pull request primarily involve modifications to the test cases in Changes
Possibly related PRs
Suggested reviewers
Poem
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 (
|
✅ Deploy Preview for reearth-cms ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![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 (3)
web/e2e/project/content/content.spec.ts (3)
51-74: LGTM! Consider extracting status verification helper.The test case is well-structured with clear steps and appropriate assertions. The renaming improves clarity about the test's scope.
Consider extracting the status verification into a helper function to reduce duplication:
async function verifyItemStatus(page: Page, expectedStatus: "DRAFT" | "PUBLIC") { await expect(page.getByText(expectedStatus)).toBeVisible(); }This would make the test more maintainable and reduce the repetitive status checks at lines 60, 63-64, 67, 71-73.
76-98: LGTM! Consider additional test scenarios.The test case effectively covers the basic flow for publishing/unpublishing from table view.
Consider adding these valuable test scenarios:
- Publishing/unpublishing multiple selected items
- Error cases (e.g., network failure during publish)
- Canceling the confirmation dialog
Example for multiple items:
// Select multiple items await page.getByLabel("", { exact: true }).first().check(); await page.getByLabel("", { exact: true }).nth(1).check(); await page.getByText("Publish", { exact: true }).click(); await page.getByRole("button", { name: "Yes" }).click(); // Verify both items are published await expect(page.getByText("PUBLIC")).toHaveCount(2);
51-98: Consider improving test organization and coverage.The test implementation is solid but could benefit from some architectural improvements:
- Create a dedicated test fixture for content-related operations to reduce setup duplication
- Consider grouping these publishing-related tests in a describe block
- Add test coverage for edge cases and error scenarios
Example test organization:
test.describe('Content Publishing', () => { test.beforeEach(async ({ page }) => { // Setup content item }); test('from edit page...', async ({ page }) => { // Existing test }); test('from table...', async ({ page }) => { // Existing test }); test('handles errors...', async ({ page }) => { // New error cases }); });
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
web/e2e/project/content/content.spec.ts (3)
51-74: Consider adding negative test casesThe test only covers the happy path. Consider adding test cases for:
- Attempting to unpublish an already unpublished item
- Attempting to publish an already published item
51-98: Extract common setup into helper functionsBoth test cases share similar setup code. Consider extracting common operations into helper functions:
async function createTestItem(page: Page, text: string = "text") { await page.getByRole("button", { name: "plus New Item" }).click(); await page.getByLabel("text").fill(text); await page.getByRole("button", { name: "Save" }).click(); await closeNotification(page); await expect(page.getByText("DRAFT")).toBeVisible({ timeout: 5000 }); } async function verifyItemStatus(page: Page, status: "DRAFT" | "PUBLIC") { await expect(page.getByText(status)).toBeVisible({ timeout: 5000 }); }This would make the tests more maintainable and reduce code duplication.
51-98: Consider implementing a Page Object Model patternThe test file would benefit from implementing the Page Object Model (POM) pattern to:
- Encapsulate page-specific selectors and actions
- Improve test maintainability
- Make tests more readable and robust
Example structure:
// contentPage.ts class ContentPage { constructor(private page: Page) {} async createItem(text: string) { // Implementation } async publishItem(itemIndex: number) { // Implementation } async unpublishItem(itemIndex: number) { // Implementation } async verifyItemStatus(itemIndex: number, status: "DRAFT" | "PUBLIC") { // Implementation } } // content.spec.ts test("Publishing from table", async ({ page }) => { const contentPage = new ContentPage(page); await contentPage.createItem("test"); await contentPage.publishItem(0); await contentPage.verifyItemStatus(0, "PUBLIC"); });
Overview
This PR adds the test for publishing and unpublishing items from table.
Summary by CodeRabbit