-
Notifications
You must be signed in to change notification settings - Fork 47
add Slack webhook UI and E2E tests #2333
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: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -17,58 +17,189 @@ | |||||||||
| import {test, expect} from '@playwright/test'; | ||||||||||
| import {loginAsUser, BASE_URL, expectDualThemeScreenshot} from './utils'; | ||||||||||
|
|
||||||||||
| test.beforeEach(async () => {}); | ||||||||||
| test('redirects unauthenticated user to home and shows toast', async ({ | ||||||||||
| page, | ||||||||||
| }) => { | ||||||||||
| await page.goto(`${BASE_URL}/settings/notification-channels`); | ||||||||||
|
|
||||||||||
| // Expect to be redirected to the home page. | ||||||||||
| await expect(page).toHaveURL(`${BASE_URL}/`); | ||||||||||
| // FYI: We do not assert the toast because it flashes on the screen due to the redirect. | ||||||||||
| }); | ||||||||||
|
|
||||||||||
| test.describe('Notification Channels Page', () => { | ||||||||||
| test('redirects unauthenticated user to home and shows toast', async ({ | ||||||||||
| page, | ||||||||||
| }) => { | ||||||||||
| test.beforeEach(async ({page}) => { | ||||||||||
| await loginAsUser(page, 'test user 1'); | ||||||||||
| await page.goto(`${BASE_URL}/settings/notification-channels`); | ||||||||||
|
|
||||||||||
| // Expect to be redirected to the home page. | ||||||||||
| await expect(page).toHaveURL(BASE_URL); | ||||||||||
| // FYI: We do not assert the toast because it flashes on the screen due to the redirect. | ||||||||||
| }); | ||||||||||
|
|
||||||||||
| test('authenticated user sees their email channel and coming soon messages', async ({ | ||||||||||
| page, | ||||||||||
| }) => { | ||||||||||
| // Log in as a test user | ||||||||||
| await loginAsUser(page, 'test user 1'); | ||||||||||
|
|
||||||||||
| // Navigate to the notification channels page | ||||||||||
| await page.goto(`${BASE_URL}/settings/notification-channels`); | ||||||||||
|
|
||||||||||
| // Move the mouse to a neutral position to avoid hover effects on the screenshot | ||||||||||
| await page.mouse.move(0, 0); | ||||||||||
|
|
||||||||||
| // Expect the URL to be correct | ||||||||||
| // Expect the URL to be correct. | ||||||||||
| await expect(page).toHaveURL(`${BASE_URL}/settings/notification-channels`); | ||||||||||
|
|
||||||||||
| // Verify Email panel content | ||||||||||
| // Verify Email panel content. | ||||||||||
| const emailPanel = page.locator('webstatus-notification-email-channels'); | ||||||||||
| await expect(emailPanel).toBeVisible(); | ||||||||||
| await expect(emailPanel).toContainText('test.user.1@example.com'); | ||||||||||
| await expect(emailPanel).toContainText('Enabled'); | ||||||||||
|
|
||||||||||
| // Verify RSS panel content | ||||||||||
| // Verify RSS panel content. | ||||||||||
| const rssPanel = page.locator('webstatus-notification-rss-channels'); | ||||||||||
| await expect(rssPanel).toBeVisible(); | ||||||||||
| await expect(rssPanel).toContainText('Coming soon'); | ||||||||||
|
|
||||||||||
| // Verify Webhook panel content | ||||||||||
| // Verify Webhook panel content. | ||||||||||
| const webhookPanel = page.locator( | ||||||||||
| 'webstatus-notification-webhook-channels', | ||||||||||
| ); | ||||||||||
| await expect(webhookPanel).toBeVisible(); | ||||||||||
| await expect(webhookPanel).toContainText('Coming soon'); | ||||||||||
|
|
||||||||||
| // Take a screenshot for visual regression | ||||||||||
| // Move the mouse to a neutral position to avoid hover effects on the screenshot. | ||||||||||
| await page.mouse.move(0, 0); | ||||||||||
|
|
||||||||||
| // Take a screenshot for visual regression. | ||||||||||
| const pageContainer = page.locator('.page-container'); | ||||||||||
| await expectDualThemeScreenshot( | ||||||||||
| page, | ||||||||||
| pageContainer, | ||||||||||
| 'notification-channels-authenticated', | ||||||||||
| ); | ||||||||||
| }); | ||||||||||
|
|
||||||||||
| test('authenticated user can create and delete a slack webhook channel', async ({ | ||||||||||
| page, | ||||||||||
| }) => { | ||||||||||
| const nonce = Date.now(); | ||||||||||
| const webhookName = 'PlaywrightTestCreateDeleteTest ' + nonce; | ||||||||||
| const webhookUrl = | ||||||||||
| 'https://hooks.slack.com/services/PLAYWRIGHT/TEST/' + nonce; | ||||||||||
|
|
||||||||||
| const webhookPanel = page.locator( | ||||||||||
| 'webstatus-notification-webhook-channels', | ||||||||||
| ); | ||||||||||
|
|
||||||||||
| // Don't assert that no webhook channels are configured. | ||||||||||
| // There may be some from previous test runs or from manual testing. | ||||||||||
|
|
||||||||||
| // Click Create button. | ||||||||||
| const createButton = webhookPanel.getByRole('button', { | ||||||||||
| name: 'Create Webhook channel', | ||||||||||
| }); | ||||||||||
| await expect(createButton).toBeVisible(); | ||||||||||
| await createButton.click(); | ||||||||||
|
|
||||||||||
| // Fill the dialog. | ||||||||||
| const dialog = webhookPanel | ||||||||||
| .locator('webstatus-manage-notification-channel-dialog') | ||||||||||
| .locator('sl-dialog'); | ||||||||||
| await expect(dialog).toBeVisible(); | ||||||||||
|
Comment on lines
+94
to
+97
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a particular heading we can assert on besides the component. webstatus.dev/e2e/tests/subscriptions.spec.ts Lines 67 to 70 in 7e8601e
|
||||||||||
|
|
||||||||||
| await dialog.getByRole('textbox', {name: 'Name'}).fill(webhookName); | ||||||||||
| await dialog | ||||||||||
| .getByRole('textbox', {name: 'Slack Webhook URL'}) | ||||||||||
| .fill(webhookUrl); | ||||||||||
|
|
||||||||||
| await dialog.getByRole('button', {name: 'Create', exact: true}).click(); | ||||||||||
|
|
||||||||||
| // Verify it's in the list. | ||||||||||
| await expect(dialog).not.toBeVisible(); | ||||||||||
| const channelItem = webhookPanel.locator('.channel-item', { | ||||||||||
| hasText: webhookName, | ||||||||||
| }); | ||||||||||
| await expect(channelItem).toBeVisible(); | ||||||||||
|
|
||||||||||
| await channelItem.getByLabel('Delete').click(); | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It should be possible to get That is more flexible than getByLabel. Docs |
||||||||||
|
|
||||||||||
| const deleteDialog = webhookPanel.locator( | ||||||||||
| 'sl-dialog[label="Delete Webhook Channel"]', | ||||||||||
| ); | ||||||||||
|
Comment on lines
+115
to
+117
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should work instead using the preferred locator.
Suggested change
|
||||||||||
| await expect(deleteDialog).toBeVisible(); | ||||||||||
| await deleteDialog | ||||||||||
| .getByRole('button', {name: 'Delete', exact: true}) | ||||||||||
| .click(); | ||||||||||
|
|
||||||||||
| // Verify it's gone. | ||||||||||
| await expect(channelItem).not.toBeVisible(); | ||||||||||
| }); | ||||||||||
|
|
||||||||||
| test('authenticated user can update a slack webhook channel', async ({ | ||||||||||
| page, | ||||||||||
| }) => { | ||||||||||
| // Use a nonce to make sure we don't have any stale data from previous test runs. | ||||||||||
| // Avoid using resetUserData() since it's an expensive operation. | ||||||||||
| const nonce = Date.now(); | ||||||||||
| const originalName = 'PlaywrightTestUpdateOriginal ' + nonce; | ||||||||||
| const originalUrl = | ||||||||||
| 'https://hooks.slack.com/services/PLAYWRIGHT/TEST/original-' + nonce; | ||||||||||
| const updatedName = 'PlaywrightTestUpdateUpdated ' + nonce; | ||||||||||
| const updatedUrl = | ||||||||||
| 'https://hooks.slack.com/services/PLAYWRIGHT/TEST/updated-' + nonce; | ||||||||||
|
|
||||||||||
| // Create a channel first. | ||||||||||
| const webhookPanel = page.locator( | ||||||||||
| 'webstatus-notification-webhook-channels', | ||||||||||
| ); | ||||||||||
| await page.waitForLoadState('networkidle'); | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We try to avoid using 'networkidle' anymore. It has been marked as discouraged. https://playwright.dev/docs/next/api/class-page#page-wait-for-load-state |
||||||||||
| await webhookPanel | ||||||||||
| .getByRole('button', {name: 'Create Webhook channel'}) | ||||||||||
| .click(); | ||||||||||
| const dialog = webhookPanel | ||||||||||
| .locator('webstatus-manage-notification-channel-dialog') | ||||||||||
| .locator('sl-dialog'); | ||||||||||
|
Comment on lines
+148
to
+150
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||||
| await expect(dialog).toBeVisible({timeout: 10000}); | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same question and follow up as pointed out in https://github.com/GoogleChrome/webstatus.dev/pull/2333/changes#r2960189732 |
||||||||||
| await dialog.getByRole('textbox', {name: 'Name'}).fill(originalName); | ||||||||||
| await dialog | ||||||||||
| .getByRole('textbox', {name: 'Slack Webhook URL'}) | ||||||||||
| .fill(originalUrl); | ||||||||||
| await dialog.getByRole('button', {name: 'Create', exact: true}).click(); | ||||||||||
|
|
||||||||||
| // Verify it was created. | ||||||||||
| await expect(dialog).not.toBeVisible({timeout: 10000}); | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same question and follow up as pointed out in https://github.com/GoogleChrome/webstatus.dev/pull/2333/changes#r2960189732 |
||||||||||
| const originalItem = webhookPanel.locator('.channel-item', { | ||||||||||
| hasText: originalName, | ||||||||||
| }); | ||||||||||
| await expect(originalItem).toBeVisible(); | ||||||||||
|
|
||||||||||
| await originalItem.getByLabel('Edit').click(); | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||||
|
|
||||||||||
| // Verify current values in dialog. | ||||||||||
| await expect(dialog).toBeVisible(); | ||||||||||
| await expect(dialog.getByRole('textbox', {name: 'Name'})).toHaveValue( | ||||||||||
| originalName, | ||||||||||
| ); | ||||||||||
| await expect( | ||||||||||
| dialog.getByRole('textbox', {name: 'Slack Webhook URL'}), | ||||||||||
| ).toHaveValue(originalUrl); | ||||||||||
|
|
||||||||||
| // Update the values. | ||||||||||
| await dialog.getByRole('textbox', {name: 'Name'}).fill(updatedName); | ||||||||||
| await dialog | ||||||||||
| .getByRole('textbox', {name: 'Slack Webhook URL'}) | ||||||||||
| .fill(updatedUrl); | ||||||||||
|
|
||||||||||
| await dialog.getByRole('button', {name: 'Save', exact: true}).click(); | ||||||||||
|
|
||||||||||
| // Verify it was updated. | ||||||||||
| await expect(dialog).not.toBeVisible({timeout: 10000}); | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason for this increased timeout? If you're waiting for a response to complete, use page.waitForResponse instead before checking the dialog's visibility. This will help prevent flakiness in resource constrained environments like our CI.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here are places where we do it already: https://github.com/search?q=repo%3AGoogleChrome%2Fwebstatus.dev%20waitforresponse&type=code |
||||||||||
| const updatedItem = webhookPanel.locator('.channel-item', { | ||||||||||
| hasText: updatedName, | ||||||||||
| }); | ||||||||||
| await expect(updatedItem).toBeVisible(); | ||||||||||
| await expect(originalItem).not.toBeVisible(); | ||||||||||
|
|
||||||||||
| const deleteButton = updatedItem.locator('sl-button[aria-label="Delete"]'); | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: try something like this when able.
Suggested change
|
||||||||||
| await expect(deleteButton).toBeVisible(); | ||||||||||
| await deleteButton.click(); | ||||||||||
|
|
||||||||||
| const deleteDialog = webhookPanel.locator( | ||||||||||
| 'sl-dialog[label="Delete Webhook Channel"]', | ||||||||||
| ); | ||||||||||
|
Comment on lines
+196
to
+198
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| await expect(deleteDialog).toBeVisible(); | ||||||||||
| await deleteDialog | ||||||||||
| .getByRole('button', {name: 'Delete', exact: true}) | ||||||||||
| .click(); | ||||||||||
| await expect(updatedItem).not.toBeVisible(); | ||||||||||
| }); | ||||||||||
| }); | ||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| /** | ||
| * Copyright 2026 Google LLC | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| import {html, TemplateResult} from 'lit'; | ||
| import {type components} from 'webstatus.dev-backend'; | ||
| import './webhook-config-form.js'; | ||
|
|
||
| import {ChannelConfigUpdate} from './channel-config-types.js'; | ||
|
|
||
| type ChannelType = components['schemas']['NotificationChannel']['type']; | ||
| type ChannelResponse = components['schemas']['NotificationChannelResponse']; | ||
|
|
||
| export const ChannelConfigRegistry = { | ||
| renderConfig( | ||
| type: ChannelType, | ||
| channel: ChannelResponse | undefined, | ||
| onUpdate: (update: ChannelConfigUpdate) => void, | ||
| ): TemplateResult { | ||
| switch (type) { | ||
| case 'webhook': | ||
| return html`<webhook-config-form | ||
| class="config-form" | ||
| .channel=${channel} | ||
| @change=${(e: CustomEvent<ChannelConfigUpdate>) => onUpdate(e.detail)} | ||
| ></webhook-config-form>`; | ||
| case 'email': | ||
| return html`<div> | ||
| Email: | ||
| ${channel?.config.type === 'email' | ||
| ? (channel.config as components['schemas']['EmailConfig']).address | ||
neilv-g marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| : ''} | ||
| (Verified) | ||
| </div>`; | ||
| default: | ||
| return html`<p>Unsupported channel type: ${type}</p>`; | ||
| } | ||
| }, | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| /** | ||
| * Copyright 2026 Google LLC | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| import {components} from 'webstatus.dev-backend'; | ||
| import {LitElement} from 'lit'; | ||
| import {property} from 'lit/decorators.js'; | ||
|
|
||
| type UpdateRequest = components['schemas']['UpdateNotificationChannelRequest']; | ||
| type UpdateMask = UpdateRequest['update_mask'][number]; | ||
| type ChannelResponse = components['schemas']['NotificationChannelResponse']; | ||
|
|
||
| export interface ChannelConfigUpdate { | ||
| updates: Partial<UpdateRequest>; | ||
| mask: UpdateMask[]; | ||
| } | ||
|
|
||
| export interface ChannelConfigComponent extends HTMLElement { | ||
| channel?: ChannelResponse; | ||
| getUpdate(): ChannelConfigUpdate; | ||
| isDirty(): boolean; | ||
| validate(): boolean; | ||
| } | ||
|
|
||
| export abstract class ChannelConfigForm extends LitElement { | ||
| @property({type: Object}) abstract channel?: ChannelResponse; | ||
| abstract validate(): boolean; | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
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.
Question: Did this make a difference. I wonder if we should add an optional argument to expectDualThemeScreenshot which will move that cursor for us automatically.
webstatus.dev/e2e/tests/utils.ts
Lines 37 to 53 in 7e8601e
Don't address that in this PR but something for the future if we see this happening a lot. Just FYI for the future.