diff --git a/e2e/tests/feature-page.spec.ts b/e2e/tests/feature-page.spec.ts index 96613a272..6c123aeb0 100644 --- a/e2e/tests/feature-page.spec.ts +++ b/e2e/tests/feature-page.spec.ts @@ -15,7 +15,7 @@ */ import {test, expect} from '@playwright/test'; -import {setupFakeNow} from './utils'; +import {setupFakeNow, resetUserData, loginAsUser} from './utils'; test.beforeEach(async ({page}) => { await setupFakeNow(page); @@ -217,3 +217,40 @@ test('shows gone page for a split feature', async ({page}) => { const pageContainer = page.locator('.container'); // Assuming a generic container for the error page. await expect(pageContainer).toHaveScreenshot(); }); + +test.describe('Subscriptions', () => { + test.beforeAll(async () => { + await resetUserData(); + }); + test.afterAll(async () => { + await resetUserData(); + }); + test('Logged-in user can subscribe to updates', async ({page}) => { + await loginAsUser(page, 'test user 1'); + + await page.goto(`http://localhost:5555/features/${featureID}`); + await page.getByRole('button', {name: 'Subscribe'}).click(); + const dialog = page.locator('webstatus-manage-subscriptions-dialog'); + await expect( + dialog.getByRole('heading', {name: 'Manage notifications'}), + ).toBeVisible(); + + await dialog.getByText('test.user.1@example.com').click(); + + await dialog + .locator('sl-checkbox') + .filter({hasText: '...becomes widely available'}) + .locator('label') + .click(); + + const createButton = dialog.getByRole('button', { + name: 'Create Subscription', + }); + await expect(createButton).toBeVisible(); + await createButton.click(); + + await expect( + page.locator('sl-alert', {hasText: 'Subscription saved!'}), + ).toBeVisible(); + }); +}); diff --git a/e2e/tests/feature-page.spec.ts-snapshots/matches-the-screenshot-1-chromium-linux.png b/e2e/tests/feature-page.spec.ts-snapshots/matches-the-screenshot-1-chromium-linux.png index da00f3166..402dba155 100644 Binary files a/e2e/tests/feature-page.spec.ts-snapshots/matches-the-screenshot-1-chromium-linux.png and b/e2e/tests/feature-page.spec.ts-snapshots/matches-the-screenshot-1-chromium-linux.png differ diff --git a/e2e/tests/feature-page.spec.ts-snapshots/matches-the-screenshot-1-firefox-linux.png b/e2e/tests/feature-page.spec.ts-snapshots/matches-the-screenshot-1-firefox-linux.png index 1e29c213b..733d2c02e 100644 Binary files a/e2e/tests/feature-page.spec.ts-snapshots/matches-the-screenshot-1-firefox-linux.png and b/e2e/tests/feature-page.spec.ts-snapshots/matches-the-screenshot-1-firefox-linux.png differ diff --git a/e2e/tests/feature-page.spec.ts-snapshots/matches-the-screenshot-1-webkit-linux.png b/e2e/tests/feature-page.spec.ts-snapshots/matches-the-screenshot-1-webkit-linux.png index 154fbc151..ce1ef0e4e 100644 Binary files a/e2e/tests/feature-page.spec.ts-snapshots/matches-the-screenshot-1-webkit-linux.png and b/e2e/tests/feature-page.spec.ts-snapshots/matches-the-screenshot-1-webkit-linux.png differ diff --git a/e2e/tests/feature-page.spec.ts-snapshots/matches-the-screenshot-for-a-discouraged-feature-1-chromium-linux.png b/e2e/tests/feature-page.spec.ts-snapshots/matches-the-screenshot-for-a-discouraged-feature-1-chromium-linux.png index 5a0083837..a0b701b5e 100644 Binary files a/e2e/tests/feature-page.spec.ts-snapshots/matches-the-screenshot-for-a-discouraged-feature-1-chromium-linux.png and b/e2e/tests/feature-page.spec.ts-snapshots/matches-the-screenshot-for-a-discouraged-feature-1-chromium-linux.png differ diff --git a/e2e/tests/feature-page.spec.ts-snapshots/matches-the-screenshot-for-a-discouraged-feature-1-firefox-linux.png b/e2e/tests/feature-page.spec.ts-snapshots/matches-the-screenshot-for-a-discouraged-feature-1-firefox-linux.png index 760cc7cb1..ab9adcd2c 100644 Binary files a/e2e/tests/feature-page.spec.ts-snapshots/matches-the-screenshot-for-a-discouraged-feature-1-firefox-linux.png and b/e2e/tests/feature-page.spec.ts-snapshots/matches-the-screenshot-for-a-discouraged-feature-1-firefox-linux.png differ diff --git a/e2e/tests/feature-page.spec.ts-snapshots/matches-the-screenshot-for-a-discouraged-feature-1-webkit-linux.png b/e2e/tests/feature-page.spec.ts-snapshots/matches-the-screenshot-for-a-discouraged-feature-1-webkit-linux.png index e831bc08c..26160c076 100644 Binary files a/e2e/tests/feature-page.spec.ts-snapshots/matches-the-screenshot-for-a-discouraged-feature-1-webkit-linux.png and b/e2e/tests/feature-page.spec.ts-snapshots/matches-the-screenshot-for-a-discouraged-feature-1-webkit-linux.png differ diff --git a/e2e/tests/feature-page.spec.ts-snapshots/mobile-chart-displays-on-click-and-matches-screenshot-1-chromium-linux.png b/e2e/tests/feature-page.spec.ts-snapshots/mobile-chart-displays-on-click-and-matches-screenshot-1-chromium-linux.png index 9fca8b50e..78964724e 100644 Binary files a/e2e/tests/feature-page.spec.ts-snapshots/mobile-chart-displays-on-click-and-matches-screenshot-1-chromium-linux.png and b/e2e/tests/feature-page.spec.ts-snapshots/mobile-chart-displays-on-click-and-matches-screenshot-1-chromium-linux.png differ diff --git a/e2e/tests/feature-page.spec.ts-snapshots/mobile-chart-displays-on-click-and-matches-screenshot-1-firefox-linux.png b/e2e/tests/feature-page.spec.ts-snapshots/mobile-chart-displays-on-click-and-matches-screenshot-1-firefox-linux.png index 4444204c9..eb18311ff 100644 Binary files a/e2e/tests/feature-page.spec.ts-snapshots/mobile-chart-displays-on-click-and-matches-screenshot-1-firefox-linux.png and b/e2e/tests/feature-page.spec.ts-snapshots/mobile-chart-displays-on-click-and-matches-screenshot-1-firefox-linux.png differ diff --git a/e2e/tests/feature-page.spec.ts-snapshots/mobile-chart-displays-on-click-and-matches-screenshot-1-webkit-linux.png b/e2e/tests/feature-page.spec.ts-snapshots/mobile-chart-displays-on-click-and-matches-screenshot-1-webkit-linux.png index 855c9181f..b85e6e809 100644 Binary files a/e2e/tests/feature-page.spec.ts-snapshots/mobile-chart-displays-on-click-and-matches-screenshot-1-webkit-linux.png and b/e2e/tests/feature-page.spec.ts-snapshots/mobile-chart-displays-on-click-and-matches-screenshot-1-webkit-linux.png differ diff --git a/e2e/tests/feature-page.spec.ts-snapshots/redirects-for-a-moved-feature-1-chromium-linux.png b/e2e/tests/feature-page.spec.ts-snapshots/redirects-for-a-moved-feature-1-chromium-linux.png index 3b2d458c2..1428bd96d 100644 Binary files a/e2e/tests/feature-page.spec.ts-snapshots/redirects-for-a-moved-feature-1-chromium-linux.png and b/e2e/tests/feature-page.spec.ts-snapshots/redirects-for-a-moved-feature-1-chromium-linux.png differ diff --git a/e2e/tests/feature-page.spec.ts-snapshots/redirects-for-a-moved-feature-1-firefox-linux.png b/e2e/tests/feature-page.spec.ts-snapshots/redirects-for-a-moved-feature-1-firefox-linux.png index 06c43414d..420c19d83 100644 Binary files a/e2e/tests/feature-page.spec.ts-snapshots/redirects-for-a-moved-feature-1-firefox-linux.png and b/e2e/tests/feature-page.spec.ts-snapshots/redirects-for-a-moved-feature-1-firefox-linux.png differ diff --git a/e2e/tests/feature-page.spec.ts-snapshots/redirects-for-a-moved-feature-1-webkit-linux.png b/e2e/tests/feature-page.spec.ts-snapshots/redirects-for-a-moved-feature-1-webkit-linux.png index 2758554b3..2d92e01f2 100644 Binary files a/e2e/tests/feature-page.spec.ts-snapshots/redirects-for-a-moved-feature-1-webkit-linux.png and b/e2e/tests/feature-page.spec.ts-snapshots/redirects-for-a-moved-feature-1-webkit-linux.png differ diff --git a/frontend/src/static/js/components/webstatus-feature-page.ts b/frontend/src/static/js/components/webstatus-feature-page.ts index d68a53bd0..12c0bec29 100644 --- a/frontend/src/static/js/components/webstatus-feature-page.ts +++ b/frontend/src/static/js/components/webstatus-feature-page.ts @@ -48,6 +48,7 @@ import {BaseChartsPage} from './webstatus-base-charts-page.js'; import './webstatus-feature-wpt-progress-chart-panel.js'; import './webstatus-feature-usage-chart-panel.js'; +import './webstatus-subscribe-button.js'; import {DataFetchedEvent} from './webstatus-line-chart-panel.js'; import { FeatureGoneSplitError, @@ -764,6 +765,11 @@ export class FeaturePage extends BaseChartsPage { )} ${this.renderOffsiteLink('MDN', null)} ${this.renderOffsiteLink('CanIUse', canIUseLink)} + + diff --git a/frontend/src/static/js/components/webstatus-manage-subscriptions-dialog.ts b/frontend/src/static/js/components/webstatus-manage-subscriptions-dialog.ts index 04affea28..c84bee66f 100644 --- a/frontend/src/static/js/components/webstatus-manage-subscriptions-dialog.ts +++ b/frontend/src/static/js/components/webstatus-manage-subscriptions-dialog.ts @@ -384,8 +384,9 @@ export class ManageSubscriptionsDialog extends LitElement { label="Manage notifications" class="dialog-main" style="--width: min(90vw, 991px);" - ?open=${this.open} + .open=${this.open} @sl-request-close=${this._handleRequestClose} + @sl-hide=${() => this.dispatchEvent(new SubscriptionDialogCloseEvent())} > ${this._loadingTask.render({ pending: () => html``, @@ -397,7 +398,7 @@ export class ManageSubscriptionsDialog extends LitElement {

You have unsaved changes. Do you want to discard them?

diff --git a/frontend/src/static/js/components/webstatus-subscribe-button.ts b/frontend/src/static/js/components/webstatus-subscribe-button.ts index e2f1eb556..6448e8d40 100644 --- a/frontend/src/static/js/components/webstatus-subscribe-button.ts +++ b/frontend/src/static/js/components/webstatus-subscribe-button.ts @@ -60,7 +60,7 @@ export class SubscribeButton extends LitElement { (this._isSubscriptionDialogOpen = false)} diff --git a/lib/gcpspanner/delete_user_saved_search_test.go b/lib/gcpspanner/delete_user_saved_search_test.go index 82f00d330..895de5327 100644 --- a/lib/gcpspanner/delete_user_saved_search_test.go +++ b/lib/gcpspanner/delete_user_saved_search_test.go @@ -95,4 +95,30 @@ func TestDeleteUserSavedSearch(t *testing.T) { t.Errorf("expected ErrQueryReturnedNoResults. received %s", err) } }) + + t.Run("SYSTEM_MANAGED search cannot be deleted", func(t *testing.T) { + // 1. Create a system managed search directly in the DB. + systemSearchID := "system-search" + _, err := spannerClient.Apply(ctx, []*spanner.Mutation{ + spanner.Insert("SavedSearches", + []string{"ID", "Name", "Query", "Scope", "AuthorID", "CreatedAt", "UpdatedAt"}, + []interface{}{ + systemSearchID, "System Search", "id:foo", "SYSTEM_MANAGED", + "system", spanner.CommitTimestamp, spanner.CommitTimestamp}), + }) + if err != nil { + t.Fatalf("failed to create system search: %v", err) + } + + // 2. Attempt to delete it via the user API. + err = spannerClient.DeleteUserSavedSearch(ctx, DeleteUserSavedSearchRequest{ + RequestingUserID: "userID1", + SavedSearchID: systemSearchID, + }) + + // 3. Verify it fails with 'not found' because the mapper filters it out. + if !errors.Is(err, ErrQueryReturnedNoResults) { + t.Errorf("expected ErrQueryReturnedNoResults, got %v", err) + } + }) } diff --git a/lib/gcpspanner/get_user_saved_search.go b/lib/gcpspanner/get_user_saved_search.go index 9cb401656..304410c0b 100644 --- a/lib/gcpspanner/get_user_saved_search.go +++ b/lib/gcpspanner/get_user_saved_search.go @@ -16,6 +16,7 @@ package gcpspanner import ( "context" + "errors" "fmt" "cloud.google.com/go/spanner" @@ -103,29 +104,69 @@ func (m authenticatedUserSavedSearchMapper) SelectOne( return stmt } +// readSavedSearchMapper provides a way to read any SavedSearch by its ID, regardless of scope. +type readSavedSearchMapper struct{} + +func (m readSavedSearchMapper) SelectOne(id string) spanner.Statement { + stmt := spanner.NewStatement(fmt.Sprintf(` + SELECT + ID, Name, Description, Query, Scope, AuthorID, CreatedAt, UpdatedAt + FROM %s + WHERE ID = @id + LIMIT 1`, + savedSearchesTable)) + stmt.Params["id"] = id + + return stmt +} + +// GetUserSavedSearch returns a single user saved search by its id. +// If the user is authenticated, it will also return their role and bookmark status. func (c *Client) GetUserSavedSearch( ctx context.Context, savedSearchID string, authenticatedUserID *string) (*UserSavedSearch, error) { + + // Use a single read-only transaction for all operations. + txn := c.ReadOnlyTransaction() + defer txn.Close() + + // 1. Fetch the base SavedSearch using the new generic mapper. + savedSearch, err := newEntityReader[readSavedSearchMapper, SavedSearch, string](c). + readRowByKeyWithTransaction(ctx, savedSearchID, txn) + if err != nil { + return nil, err + } + + // 2. If the user is unauthenticated, they can only see public or system searches. if authenticatedUserID == nil { - // For an unauthenticated user, we only read the SavedSearches row then fill in the rest of - // UserSavedSearch struct with nil values. - row, err := newEntityReader[unauthenticatedUserSavedSearchMapper, SavedSearch, string](c). - readRowByKey(ctx, savedSearchID) - if err != nil { - return nil, err + if savedSearch.Scope != UserPublicScope && savedSearch.Scope != SystemManagedScope { + return nil, ErrQueryReturnedNoResults + } + + return &UserSavedSearch{SavedSearch: *savedSearch, Role: nil, IsBookmarked: nil}, nil + } + + // 3. For an authenticated user, fetch their specific role and bookmark status + // using the original restricted mapper within the SAME transaction. + userSpecifics, err := newEntityReader[ + authenticatedUserSavedSearchMapper, + UserSavedSearch, + authenticatedUserSavedSearchMapperKey, + ](c).readRowByKeyWithTransaction(ctx, authenticatedUserSavedSearchMapperKey{ + UserID: *authenticatedUserID, + ID: savedSearchID, + }, txn) + + // If there are no user-specific details (e.g., for a SYSTEM_MANAGED search), + // that's okay. We just return the base search info. + if err != nil { + if errors.Is(err, ErrQueryReturnedNoResults) { + return &UserSavedSearch{SavedSearch: *savedSearch, Role: nil, IsBookmarked: nil}, nil } - return &UserSavedSearch{ - SavedSearch: *row, - IsBookmarked: nil, - Role: nil, - }, nil + return nil, err } - return newEntityReader[authenticatedUserSavedSearchMapper, UserSavedSearch, authenticatedUserSavedSearchMapperKey](c). - readRowByKey(ctx, authenticatedUserSavedSearchMapperKey{ - UserID: *authenticatedUserID, - ID: savedSearchID, - }) + return userSpecifics, nil } diff --git a/lib/gcpspanner/user_search_bookmarks_test.go b/lib/gcpspanner/user_search_bookmarks_test.go index 4635775a4..02fcf1145 100644 --- a/lib/gcpspanner/user_search_bookmarks_test.go +++ b/lib/gcpspanner/user_search_bookmarks_test.go @@ -186,4 +186,30 @@ func TestUserSearchBookmark(t *testing.T) { t.Errorf("expected ErrOwnerCannotDeleteBookmark error. received %s", err) } }) + + t.Run("the test user cannot bookmark a SYSTEM_MANAGED search", func(t *testing.T) { + // 1. Create a system managed search directly in the DB. + systemSearchID := "system-managed-search-id" + _, err := spannerClient.Apply(ctx, []*spanner.Mutation{ + spanner.Insert("SavedSearches", + []string{"ID", "Name", "Query", "Scope", "AuthorID", "CreatedAt", "UpdatedAt"}, + []interface{}{ + systemSearchID, "System Search", "id:feat", "SYSTEM_MANAGED", + "system", spanner.CommitTimestamp, spanner.CommitTimestamp}), + }) + if err != nil { + t.Fatalf("failed to create system search: %v", err) + } + + // 2. Attempt to bookmark it. + err = spannerClient.AddUserSearchBookmark(ctx, UserSavedSearchBookmark{ + UserID: testUser, + SavedSearchID: systemSearchID, + }) + + // 3. Verify it fails with 'not found' because the mapper filters it out. + if !errors.Is(err, ErrQueryReturnedNoResults) { + t.Errorf("expected ErrQueryReturnedNoResults, got %v", err) + } + }) } diff --git a/util/cmd/load_fake_data/main.go b/util/cmd/load_fake_data/main.go index 2541a6515..87f78da58 100644 --- a/util/cmd/load_fake_data/main.go +++ b/util/cmd/load_fake_data/main.go @@ -1008,6 +1008,12 @@ func generateData(ctx context.Context, spannerClient *gcpspanner.Client, datasto } slog.InfoContext(ctx, "feature evolution generation complete") + err = spannerClient.SyncSystemManagedSavedQuery(ctx) + if err != nil { + return fmt.Errorf("system managed saved query sync failed %w", err) + } + slog.InfoContext(ctx, "system managed saved query sync complete") + return nil }