-
Notifications
You must be signed in to change notification settings - Fork 41
WIP #2156
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
Draft
jcscottiii
wants to merge
27
commits into
jcscottiii/ui-notifications-learnings
Choose a base branch
from
jcscottiii/ui-subscriptions
base: jcscottiii/ui-notifications-learnings
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
WIP #2156
jcscottiii
wants to merge
27
commits into
jcscottiii/ui-notifications-learnings
from
jcscottiii/ui-subscriptions
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Enables the Event Producer to fetch the previous state and coordinate execution via distributed locking. Changes: - **Spanner**: Added `GetLatestSavedSearchNotificationEvent` to fetch the last known state for a search/frequency pair. - **Schema**: Added index `SavedSearchNotificationEvents_BySearchAndSnapshotType` to optimize state lookups. - **Producer**: Updated `ProcessSearch` to acquire/release locks (currently using triggerID) and fetch the previous state before diffing. - **Refactor**: Exported `DefaultBrowsers` in `backendtypes` for shared use between the API server and worker adapters
Introduces the concrete Spanner adapters that connect the `EventProducer` domain logic to the underlying Spanner client. This includes: - `EventProducerDiffer`: Adapts the backend's `FeaturesSearch` and `GetFeature` logic to the interface required by the differ package. - `EventProducer`: The main adapter that handles the orchestration of locking, state retrieval, and event publishing, including the JSON serialization of event summaries.
Introduces the `gcppubsubadapters` package to connect the `EventProducer` domain logic to Google Cloud Pub/Sub. This includes: - `EventProducerSubscriberAdapter`: A high-level adapter that routes incoming Pub/Sub messages (RefreshSearch, BatchRefresh, ConfigurationChanged) to the appropriate `EventProducer` methods. - `EventProducerPublisherAdapter`: An adapter for publishing `FeatureDiffEvent` notifications back to Pub/Sub. - `RunGroup`: A concurrency utility for managing the lifecycle of multiple blocking subscribers.
Introduces the `gcpgcsadapters` package to connect the `EventProducer`
domain logic to Google Cloud Storage.
This includes:
- `EventProducer`: An adapter that implements the `BlobStorage` interface,
handling path construction (bucket/dirs/key) and delegating read/write
operations to the underlying GCS client.
- Support for setting content-type ("application/json") on uploads.
…er adapters Introduces the necessary adapters to support the batch fan-out process for scheduled refreshes. This includes: - **Pub/Sub Adapter (`gcppubsubadapters`)**: Added `BatchFanOutPublisherAdapter` which converts `workertypes.RefreshSearchCommand` into a `refreshsearchcommand/v1` CloudEvent and publishes it to the ingestion topic. - **Spanner Adapter (`spanneradapters`)**: Added `BatchEventProducer` and its `ListAllSavedSearches` method to fetch all saved searches for processing. - **Spanner Library (`gcpspanner`)**: Updated `ListAllSavedSearches` to return `SavedSearchBriefDetails` (ID and Query) instead of just IDs, allowing the batch handler to populate the command fully without downstream lookups. - **Worker Types**: Added `RefreshSearchCommand` and `SearchJob` struct definitions.
Connects all the adapters and logic components in the `event_producer` main entry point (`cmd/job/main.go`). This enables the worker to: 1. Subscribe to ingestion events (search refresh requests). 2. Subscribe to batch update triggers (fan-out requests). 3. Publish notifications to the downstream topic. 4. Interact with Spanner (metadata) and GCS (blobs). Infrastructure updates: - Updated `setup_pubsub.sh` to create Dead Letter Queues (DLQs) for ingestion, notification, and delivery topics, providing robust error handling. - Updated `DEVELOPMENT.md` with the correct local Pub/Sub port. - Updated `pod.yaml` with necessary environment variables for the new subscriptions.
Overhauls the `EventSummary` structure in `workertypes` to provide strongly-typed, structured data for downstream processing. Key changes: - Introduced generic `Change[T]` type and strongly-typed value structs (`BaselineValue`, `BrowserValue`, `FeatureRef`) to replace raw strings in the summary. Note, there is an existing Change struct that we may want to eventually consolidate with this one. - Added `SummaryHighlightType` and `BrowserName` enums for type safety. - Switched `browser_changes` to a map keyed by `BrowserName`. - Add `MaxHighlights` variable and set it to 10,000 to make the Spanner summary the authoritative "hot" data source (as opposed to the non truncated blob in GCS). Note: We only anticipate 2k-3k features total so this is a very high limit. - Updated `FeatureDiffV1SummaryGenerator` to populate these structured fields.
Introduces the core logic for the Push Delivery Worker, including the `Dispatcher` which orchestrates event processing, subscriber filtering, and delivery job queuing. This includes: - **Worker Types**: Added `EmailSubscriber`, `SubscriberSet`, `DeliveryMetadata`, and `EmailDeliveryJob` to `lib/workertypes/types.go` as shared domain types. - **Event Types**: Added `ToWorkertypes` helper method to `FeatureDiffEvent`. - **Dispatcher Logic**: Implemented `Dispatcher.ProcessEvent` using a visitor pattern (`deliveryJobGenerator`) to parse the event summary, find subscribers, filter based on triggers, and queue delivery jobs. - **Interfaces**: Defined `SubscriptionFinder`, `DeliveryPublisher`, and `SummaryParser` interfaces in `pkg/dispatcher`.
Updates the OpenAPI definition, internal storage types, and worker tests to
align with the V1 notification capabilities, specifically removing `DAILY`
frequency (as it's not in scope for user selection yet) and standardizing
on `IMMEDIATE`, `WEEKLY`, and `MONTHLY`.
Changes:
- **API**:
- Removed `DAILY` frequency.
- Added `IMMEDIATE`, `WEEKLY`, and `MONTHLY` frequencies.
- Renamed and expanded subscription triggers (e.g. `feature_baseline_to_widely`).
- **Storage**:
- Updated `SavedSearchSubscription` struct to use typed `SubscriptionTrigger`
and `JobFrequency` enums.
- Updated Spanner adapters to convert between backend and storage types.
- **Workers**:
- Updated `EventProducer` logic to map `IMMEDIATE` jobs correctly.
- Updated `Dispatcher` tests to use `IMMEDIATE` frequency instead of `DAILY`.
- **Backend**:
- Updated validation logic and tests to match the new enums.
…scribers
Introduces the `PushDeliverySubscriberFinder` adapter in `spanneradapters`.
This component is responsible for retrieving subscribers from Spanner and
mapping them into the domain-layer `SubscriberSet`.
Key changes:
- **Spanner Adapter (`spanneradapters/push_delivery.go`)**: Implemented `FindSubscribers`
which calls `FindAllActivePushSubscriptions` and maps the results.
- **Spanner Client (`gcpspanner`)**:
- Updated `SubscriberDestination` to hold a strongly-typed `EmailConfig`
instead of raw `spanner.NullJSON`.
- Moved JSON unmarshalling logic into the Spanner client layer (`toPublic` and
`loadSubscriptionConfigs`), ensuring raw bytes don't leak into the adapter.
- Added `Triggers` (typed as `[]SubscriptionTrigger`) to the destination struct.
This commit introduces `PushDeliveryPublisher` and `PushDeliverySubscriberAdapter` within the `lib/gcppubsub/gcppubsubadapters` package. - `PushDeliveryPublisher` provides functionality to publish email delivery jobs to a Pub/Sub topic. - `PushDeliverySubscriberAdapter` is responsible for subscribing to a Pub/Sub topic, routing incoming messages, and processing `FeatureDiffEvent`s by dispatching them to a `PushDeliveryMessageHandler`.
…iption This commit unifies the `push_delivery` worker's event processing pipeline by connecting all necessary components, implementing event filtering, and enforcing typed triggers for subscriptions. **Core Features & Changes:** - **Full Event Pipeline Integration**: Uses the `PushDeliverySubscriberAdapter` to consume `FeatureDiffEvent` notifications from Pub/Sub, parsing incoming messages, and feeding them to the `Dispatcher`. - **Intelligent Event Filtering**: Implements the core event filtering logic within the Push Delivery Dispatcher using `shouldNotifyV1` and `matchesTrigger`. This logic inspects `EventSummary` highlights, ensuring that subscribers only receive notifications for events that precisely match their defined criteria (e.g., "Baseline Status Changed to Newly"). - **Enhanced Worker Types**: Adds `DispatchEventMetadata` to `lib/workertypes` to carry essential context for notification rendering. - **Main Entry Point Setup**: Modifies `cmd/job/main.go` to correctly initialize the Spanner finder, Pub/Sub publisher, and the new `PushDeliverySubscriberAdapter`, thereby enabling the full worker loop. - **Infrastructure Alignment**: Updates `manifests/pod.yaml` with the correct environment variables and topic IDs for seamless deployment and operation.
Updates the `Subscriber` and `DeliveryJob` types (and their corresponding Spanner adapters and tests) to include `ChannelID`. This identifier is needed for the Email Worker to report delivery status (success/failure) back to the `NotificationChannelState` table. Changes: - **Worker Types**: Added `ChannelID` to `EmailSubscriber` and `EmailDeliveryJob`. - **Event Types**: Added `ChannelID` to `EmailJobEvent`. - **Spanner Adapter**: Updated `FindSubscribers` to populate `ChannelID` from the query results. - **Pub/Sub Adapter**: Updated `PublishEmailJob` to pass the ID through. - **Email Worker**: Initial scaffolding for `sender` package (interfaces/mocks) that will utilize this ID.
Implements the data layer for tracking notification channel health and logging
delivery attempts, which is critical for the Email Worker's reliability logic.
Changes:
- **State Management**: Added `RecordNotificationChannelSuccess` and
`RecordNotificationChannelFailure` methods.
- `Success`: Resets failure count and logs a success attempt.
- `Failure`: Increments failure count (if permanent), checks disable threshold
(5 failures), and logs a failure attempt. Supports transient errors (no
penalty).
- **Transactions**: State updates and attempt logging are performed atomically
within a single read-write transaction.
Other changes: Added some more tests for the list functionality.
Updates the email worker's sender logic to handle delivery robustly and extends
event types to propagate necessary context (ChannelID and Event IDs).
Changes:
- **Event Types**: Added `ChannelID` to `EmailJobEvent` to ensure the consumer
can identify the target channel.
- **Worker Types**: Introduced `IncomingEmailDeliveryJob` to capture the
`EmailEventID` (Pub/Sub message ID) for audit logging. Added sentinel errors
(`ErrUnrecoverableUserFailureEmailSending`, `ErrUnrecoverableSystemFailureEmailSending`)
to distinguish failure modes.
- **Sender Logic**: Refactored `ProcessMessage` to:
- Use `IncomingEmailDeliveryJob` for better context.
- Handle distinct error types: Permanent user errors are recorded and ACKed;
transient errors trigger NACKs for retry.
- Pass `EmailEventID` and error classification to the `ChannelStateManager`
for precise attempt tracking.
- **Pub/Sub Adapter**: Added logging to `PublishEmailJob` to trace published IDs.
Implements the Pub/Sub subscriber adapter for the email worker and the Spanner adapter for managing notification channel state. Changes: - **Pub/Sub Adapter**: Created `EmailWorkerSubscriberAdapter` to receive `EmailJobEvent` messages from Pub/Sub and route them to the email worker's message handler. Includes unit tests. - **Spanner Adapter**: Added `EmailWorkerChannelStateManager` to record notification channel success and failure events. Includes unit tests. - **Event Types**: Added a `ToWorkerTypeJobFrequency` method to the `JobFrequency` type in `lib/event/emailjob/v1` to convert between the event type and the worker type.
…features Introduces a clear distinction between features that are truly deleted from the database versus those that are simply no longer present in a given search result (which could be due to a move, split, or rename). Previously, the reconciler would mark a feature as "Removed" and set its reason to "Deleted" if it no longer existed. This approach was ambiguous because the feature remained in the `Removed` list, which is primarily intended for features that might have been moved or split. This change introduces a new `Deleted` slice to the `FeatureDiff` struct. The reconciler logic is updated to check for a feature's existence when it's reported as removed. If it returns an `ErrEntityDoesNotExist`, the feature is moved from the `Removed` list to the new `Deleted` list. This ensures the `Removed` list now accurately contains only features that need to be further analyzed for potential moves or splits, making the diffing process more robust and the resulting data structure clearer.
This commit introduces a more structured approach to handling documentation links within the email templates. Previously, documentation links were represented by a `DocLinks []DocLink` slice, implicitly assuming all links were MDN. This limited the extensibility for future documentation sources. This change refactors the `DocLinks` field in `SummaryHighlight` to `Docs *Docs`, where `Docs` is a new struct containing `MDNDocs []DocLink`. This allows for explicit categorization of documentation types in the next PR. The `toDocLinks` function has been updated to construct and return the new `*Docs` struct.
This commit enhances the email delivery job by including the associated job triggers. Previously, the `EmailDeliveryJob` struct did not contain trigger information, which limited the ability to customize email content based on why a notification was sent. This change introduces a `Triggers []JobTrigger` field to `lib/workertypes/EmailDeliveryJob` and `lib/event/emailjob/v1/EmailJobEvent`. Helper functions are added to `lib/event/emailjob/v1/types.go` for converting between internal `workertypes.JobTrigger` and `emailjob/v1.JobTrigger` types. Additionally, the `SummaryHighlight` struct in `lib/workertypes/types.go` now includes a `MatchesTrigger` method, encapsulating the logic for checking if a highlight satisfies a given trigger. This method is then utilized in `workers/push_delivery/pkg/dispatcher/dispatcher.go` within the `matchesTrigger` function, centralizing the trigger matching logic. The `EmailWorkerSubscriberAdapter` in `lib/gcppubsub/gcppubsubadapters/email_worker.go` and `PushDeliveryPublisher` in `lib/gcppubsub/gcppubsubadapters/push_delivery.go` are updated to correctly pass and receive these triggers. Corresponding tests have also been updated to reflect these changes. This refactoring allows the email worker to eventually render more dynamic and personalized email content based on the specific triggers that caused the notification.
This commit introduces a comprehensive email delivery system, complete with structured templates, reusable components, and a dedicated styling system. New email templates, including a `defaultEmailTemplate`, have been added, leveraging Go's `html/template` package for dynamic content rendering. These templates are designed with a component-based architecture, making them modular and easy to maintain. A key feature of this commit is the introduction of reusable email components, such as badges, banners, and stat boxes. These components are defined in `workers/email/pkg/digest/components.go` and are designed to be easily configurable and reusable across different email types. To ensure a consistent and polished look, a dedicated styling system has been implemented in `workers/email/pkg/digest/styles.go`. This file contains all the CSS styles used in the email templates, defined as template snippets. This approach centralizes all styling information, making it easy to manage and update the visual appearance of the emails. A golden file, `digest.golden.html`, has been added to the test data to ensure that the email rendering remains consistent and predictable. The `.prettierignore` and `Makefile` have been updated to exclude this file from formatting and license checks. Additionally, a suite of email icons and logos has been added to `frontend/src/static/img/email` to enhance the visual appeal of the emails.
Implements a new email sending service using the Chime API. - Adds a new Chime client in `lib/email/chime` for handling API requests, authentication, and error classification (transient, permanent user, permanent system). - Introduces an adapter in `lib/email/chime/chimeadapters` to integrate the new client with the existing email worker. - Updates the email worker `Send` interface to include a unique ID per message, which is passed to Chime's `external_id` field for deduplication in the future.
This commit connects the email worker's Pub/Sub subscriber to the message handler. It initializes the HTML renderer and the sender, and starts the subscription to process incoming email jobs. The frontend base URL is now configured via an environment variable to support link generation in emails.
Implements the new "Notification Channels" page, providing users with a centralized location to manage how they receive updates. This feature is a foundational step towards enabling subscriptions and personalized notifications. Key changes include: - **New Page Component:** Adds a new page at `/settings/notification-channels`, accessible only to authenticated users. - **Component Architecture:** The page is composed of a main container and three distinct panel components for each notification type (Email, RSS, Webhook), following a composition-over-inheritance pattern. - **Base Panel Component:** A new reusable `<webstatus-notification-panel>` was created to ensure a consistent look and feel across all panels and to handle loading states gracefully with skeleton placeholders. - **Email Channel Display:** The page now fetches and displays the user's email notification channels, which are synced from their verified GitHub emails via the existing `pingUser` flow. A tooltip has been added to inform users of this behavior. - **Coming Soon Placeholders:** The RSS and Webhook panels are included as placeholders with "Coming soon" messages and disabled "Create" buttons, preparing the UI for future implementation. - **Sidebar Refactoring:** The main sidebar has been refactored to use a `renderNavItem` helper function for top-level links, improving code consistency. The "Notification Channels" link now appears as a top-level item under a "Settings" section that is only visible to logged-in users. - **Testing:** Comprehensive unit tests for the new page and all panel components have been added. A new end-to-end (Playwright) test ensures the page functions correctly for both authenticated and unauthenticated users, and includes visual regression snapshots.
I had the gemini CLI examine some of its shortcomings while working on #2152 It updated GEMINI.md with its learnings
Introduces the `BackendAdapter` in `gcppubsubadapters` to handle publishing `SearchConfigurationChangedEvent` messages from the API layer. This adapter encapsulates the logic for mapping the API's `SavedSearchResponse` model to the internal event struct. This simplifies the handler code by allowing it to pass the response object directly along with context like `userID` and `isCreation`. This infrastructure is required to trigger the "Cold Start" (initial state generation) or "Query Update" workflows in the Event Producer whenever a user creates or modifies a saved search. Changes: - **Adapter**: Added `BackendAdapter` with `PublishSearchConfigurationChanged`. - **Wiring**: Integrated the publisher into the Backend Server and its handlers (`CreateSavedSearch`, `UpdateSavedSearch`).
f814596 to
a2b7b9a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.