From c664bef0d4b6ef4f2677ffca89bf1fc473802301 Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Tue, 2 Sep 2025 11:07:16 -0700 Subject: [PATCH 01/35] Add project ID as actual column on notification subscriptions and events --- sql/2025-09-02_project-webhooks.sql | 76 +++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 sql/2025-09-02_project-webhooks.sql diff --git a/sql/2025-09-02_project-webhooks.sql b/sql/2025-09-02_project-webhooks.sql new file mode 100644 index 00000000..72d5f1e7 --- /dev/null +++ b/sql/2025-09-02_project-webhooks.sql @@ -0,0 +1,76 @@ +-- This migration codifies the project ID filter on notification subscriptions into a real field rather than a +-- generic JSONB filter. You can still set it to NULL for org-wide subscriptions. + +ALTER TABLE notification_subscriptions + -- The project which this filter is scopped to. + -- If provided, the project_id must be belong to the scope_user_id's user/org. + ADD COLUMN project_id UUID NULL REFERENCES projects(id) ON DELETE CASCADE; + +ALTER TABLE notification_events + ADD COLUMN project_id UUID NULL REFERENCES projects(id) ON DELETE CASCADE; + +-- Migrate existing filters to the new column, and also remove +-- the projectId from the JSONB filter. +UPDATE notification_subscriptions + SET project_id = (filter->>'projectId')::UUID, + filter = filter - 'projectId' + WHERE filter ? 'projectId'; + +UPDATE notification_events + SET project_id = (data->>'projectId')::UUID + WHERE data ? 'projectId'; + +-- Now update the trigger so we use the new projectid columns +CREATE FUNCTION trigger_notification_event_subscriptions() +RETURNS TRIGGER AS $$ +DECLARE + the_subscription_id UUID; + the_event_id UUID; + the_subscriber UUID; +BEGIN + SELECT NEW.id INTO the_event_id; + FOR the_subscription_id, the_subscriber IN + (SELECT ns.id, ns.subscriber_user_id FROM notification_subscriptions ns + WHERE ns.scope_user_id = NEW.scope_user_id + AND NEW.topic = ANY(ns.topics) + AND (ns.project_id IS NULL OR ns.project_id = NEW.project_id) + AND (ns.filter IS NULL OR NEW.data @> ns.filter) + AND + -- A subscriber can be notified if the event is in their scope or if they have permission to the resource. + -- The latter is usually a superset of the former, but the former is trivial to compute so it can help + -- performance to include it. + (NEW.scope_user_id = ns.subscriber_user_id + OR user_has_permission(ns.subscriber_user_id, NEW.resource_id, topic_permission(NEW.topic)) + ) + ) + LOOP + -- Log that this event triggered this subscription. + INSERT INTO notification_providence_log (event_id, subscription_id) + VALUES (the_event_id, the_subscription_id); + + -- Add to the relevant queues. + -- Each delivery method _may_ be triggered by multiple subscriptions, + -- we need ON CONFLICT DO NOTHING. + INSERT INTO notification_webhook_queue (event_id, webhook_id) + SELECT the_event_id, nbw.webhook_id + FROM notification_by_webhook nbw + WHERE nbw.subscription_id = the_subscription_id + ON CONFLICT DO NOTHING; + + INSERT INTO notification_email_queue (event_id, email_id) + SELECT the_event_id AS event_id, nbe.email_id + FROM notification_by_email nbe + WHERE nbe.subscription_id = the_subscription_id + ON CONFLICT DO NOTHING; + + -- Also add the notification to the hub. + -- It's possible it was already added by another subscription for this user, + -- in which case we just carry on. + INSERT INTO notification_hub_entries (event_id, user_id) + VALUES (the_event_id, the_subscriber) + ON CONFLICT DO NOTHING; + END LOOP; + + RETURN NEW; +END; +$$ LANGUAGE plpgsql; From b7dd80d8949cd43a89f14b55079268491a19a25e Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Tue, 2 Sep 2025 11:07:16 -0700 Subject: [PATCH 02/35] Add projectId indexes --- sql/2025-09-02_project-webhooks.sql | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sql/2025-09-02_project-webhooks.sql b/sql/2025-09-02_project-webhooks.sql index 72d5f1e7..2b53846e 100644 --- a/sql/2025-09-02_project-webhooks.sql +++ b/sql/2025-09-02_project-webhooks.sql @@ -6,9 +6,13 @@ ALTER TABLE notification_subscriptions -- If provided, the project_id must be belong to the scope_user_id's user/org. ADD COLUMN project_id UUID NULL REFERENCES projects(id) ON DELETE CASCADE; +CREATE INDEX notification_subscriptions_by_user_and_project ON notification_subscriptions(subscriber_user_id, project_id, created_at DESC); + ALTER TABLE notification_events ADD COLUMN project_id UUID NULL REFERENCES projects(id) ON DELETE CASCADE; +CREATE INDEX notification_events_scope_user_and_project ON notification_events(scope_user_id, project_id, occurred_at DESC); + -- Migrate existing filters to the new column, and also remove -- the projectId from the JSONB filter. UPDATE notification_subscriptions From c410b163227717eba4670b5335dce052049c3122 Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Tue, 2 Sep 2025 11:07:16 -0700 Subject: [PATCH 03/35] Add projectId field to notification events --- src/Share/Notifications/Queries.hs | 8 ++++---- src/Share/Notifications/Types.hs | 12 ++++++++---- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/Share/Notifications/Queries.hs b/src/Share/Notifications/Queries.hs index 7394ac24..b98f2694 100644 --- a/src/Share/Notifications/Queries.hs +++ b/src/Share/Notifications/Queries.hs @@ -48,18 +48,18 @@ import Share.Web.Share.DisplayInfo.Queries qualified as DisplayInfoQ import Share.Web.Share.DisplayInfo.Types (UnifiedDisplayInfo) recordEvent :: (QueryA m) => NewNotificationEvent -> m () -recordEvent (NotificationEvent {eventScope, eventData, eventResourceId, eventActor}) = do +recordEvent (NotificationEvent {eventScope, eventData, eventResourceId, eventProjectId, eventActor}) = do execute_ [sql| - INSERT INTO notification_events (topic, scope_user_id, actor_user_id, resource_id, data) - VALUES (#{eventTopic eventData}::notification_topic, #{eventScope}, #{eventActor}, #{eventResourceId}, #{eventData}) + INSERT INTO notification_events (topic, scope_user_id, actor_user_id, resource_id, project_id, data) + VALUES (#{eventTopic eventData}::notification_topic, #{eventScope}, #{eventActor}, #{eventResourceId}, #{eventProjectId}, #{eventData}) |] expectEvent :: (QueryM m) => NotificationEventId -> m PGNotificationEvent expectEvent eventId = do queryExpect1Row @PGNotificationEvent [sql| - SELECT id, occurred_at, scope_user_id, actor_user_id, resource_id, topic, data + SELECT id, occurred_at, scope_user_id, actor_user_id, resource_id, project_id, topic, data FROM notification_events WHERE id = #{eventId} |] diff --git a/src/Share/Notifications/Types.hs b/src/Share/Notifications/Types.hs index 3b365998..93f10677 100644 --- a/src/Share/Notifications/Types.hs +++ b/src/Share/Notifications/Types.hs @@ -436,6 +436,7 @@ data NotificationEvent id userInfo occurredAt eventPayload = NotificationEvent { eventId :: id, eventOccurredAt :: occurredAt, eventResourceId :: ResourceId, + eventProjectId :: Maybe ProjectId, eventData :: eventPayload, eventScope :: userInfo, eventActor :: userInfo @@ -452,14 +453,15 @@ eventUserInfo_ f NotificationEvent {eventActor, eventScope, ..} = do pure $ NotificationEvent {eventActor = eventActor', eventScope = eventScope', ..} instance (Aeson.ToJSON eventPayload, Aeson.ToJSON userInfo) => Aeson.ToJSON (NotificationEvent NotificationEventId userInfo UTCTime eventPayload) where - toJSON NotificationEvent {eventId, eventOccurredAt, eventData, eventScope, eventActor, eventResourceId} = + toJSON NotificationEvent {eventId, eventOccurredAt, eventData, eventScope, eventActor, eventResourceId, eventProjectId} = Aeson.object [ "id" Aeson..= eventId, "occurredAt" Aeson..= eventOccurredAt, "data" Aeson..= eventData, "scope" Aeson..= eventScope, "actor" Aeson..= eventActor, - "resourceId" Aeson..= eventResourceId + "resourceId" Aeson..= eventResourceId, + "projectId" Aeson..= eventProjectId ] instance (Aeson.FromJSON eventPayload, Aeson.FromJSON userInfo) => Aeson.FromJSON (NotificationEvent NotificationEventId userInfo UTCTime eventPayload) where @@ -470,7 +472,8 @@ instance (Aeson.FromJSON eventPayload, Aeson.FromJSON userInfo) => Aeson.FromJSO eventScope <- o .: "scope" eventActor <- o .: "actor" eventResourceId <- o .: "resourceId" - pure NotificationEvent {eventId, eventOccurredAt, eventData, eventScope, eventActor, eventResourceId} + eventProjectId <- o .: "projectId" + pure NotificationEvent {eventId, eventOccurredAt, eventData, eventScope, eventActor, eventResourceId, eventProjectId} instance Hasql.DecodeRow (NotificationEvent NotificationEventId UserId UTCTime NotificationEventData) where decodeRow = do @@ -479,8 +482,9 @@ instance Hasql.DecodeRow (NotificationEvent NotificationEventId UserId UTCTime N eventScope <- PG.decodeField eventActor <- PG.decodeField eventResourceId <- PG.decodeField + eventProjectId <- PG.decodeField eventData <- PG.decodeRow - pure $ NotificationEvent {eventId, eventOccurredAt, eventData, eventScope, eventActor, eventResourceId} + pure $ NotificationEvent {eventId, eventOccurredAt, eventData, eventScope, eventActor, eventResourceId, eventProjectId} type NewNotificationEvent = NotificationEvent () UserId () NotificationEventData From fbe0b04a2318e524f06aa0bff7dce4063a90c4b5 Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Tue, 2 Sep 2025 11:07:16 -0700 Subject: [PATCH 04/35] Add project ID to notification subscriptions --- src/Share/Notifications/Queries.hs | 17 +++++++++-------- src/Share/Notifications/Types.hs | 10 +++++++--- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/Share/Notifications/Queries.hs b/src/Share/Notifications/Queries.hs index b98f2694..eabbadac 100644 --- a/src/Share/Notifications/Queries.hs +++ b/src/Share/Notifications/Queries.hs @@ -288,18 +288,18 @@ listNotificationSubscriptions :: UserId -> Transaction e [NotificationSubscripti listNotificationSubscriptions subscriberUserId = do queryListRows [sql| - SELECT ns.id, ns.scope_user_id, ns.topics, ns.topic_groups, ns.filter + SELECT ns.id, ns.scope_user_id, ns.project_id, ns.topics, ns.topic_groups, ns.filter FROM notification_subscriptions ns WHERE ns.subscriber_user_id = #{subscriberUserId} ORDER BY ns.created_at DESC |] -createNotificationSubscription :: UserId -> UserId -> Set NotificationTopic -> Set NotificationTopicGroup -> Maybe SubscriptionFilter -> Transaction e NotificationSubscriptionId -createNotificationSubscription subscriberUserId subscriptionScope subscriptionTopics subscriptionTopicGroups subscriptionFilter = do +createNotificationSubscription :: UserId -> UserId -> Maybe ProjectId -> Set NotificationTopic -> Set NotificationTopicGroup -> Maybe SubscriptionFilter -> Transaction e NotificationSubscriptionId +createNotificationSubscription subscriberUserId subscriptionScope subscriptionProjectId subscriptionTopics subscriptionTopicGroups subscriptionFilter = do queryExpect1Col [sql| - INSERT INTO notification_subscriptions (subscriber_user_id, scope_user_id, topics, topic_groups, filter) - VALUES (#{subscriberUserId}, #{subscriptionScope}, #{Foldable.toList subscriptionTopics}::notification_topic[], #{Foldable.toList subscriptionTopicGroups}::notification_topic_group[], #{subscriptionFilter}) + INSERT INTO notification_subscriptions (subscriber_user_id, scope_user_id, project_id, topics, topic_groups, filter) + VALUES (#{subscriberUserId}, #{subscriptionScope}, #{subscriptionProjectId}, #{Foldable.toList subscriptionTopics}::notification_topic[], #{Foldable.toList subscriptionTopicGroups}::notification_topic_group[], #{subscriptionFilter}) RETURNING id |] @@ -510,7 +510,7 @@ hydrateEventPayload = \case -- | Subscribe or unsubscribe to watching a project updateWatchProjectSubscription :: UserId -> ProjectId -> Bool -> Transaction e (Maybe NotificationSubscriptionId) updateWatchProjectSubscription userId projId shouldBeSubscribed = do - let filter = SubscriptionFilter $ Aeson.object ["projectId" Aeson..= projId] + let filter = SubscriptionFilter $ Aeson.object [] existing <- isUserSubscribedToWatchProject userId projId case existing of Just existingId @@ -532,12 +532,12 @@ updateWatchProjectSubscription userId projId shouldBeSubscribed = do FROM projects p WHERE p.id = #{projId} |] - Just <$> createNotificationSubscription userId projectOwnerUserId mempty (Set.singleton WatchProject) (Just filter) + Just <$> createNotificationSubscription userId projectOwnerUserId (Just projId) mempty (Set.singleton WatchProject) (Just filter) _ -> pure Nothing isUserSubscribedToWatchProject :: UserId -> ProjectId -> Transaction e (Maybe NotificationSubscriptionId) isUserSubscribedToWatchProject userId projId = do - let filter = SubscriptionFilter $ Aeson.object ["projectId" Aeson..= projId] + let filter = SubscriptionFilter $ Aeson.object [] query1Col @NotificationSubscriptionId [sql| SELECT ns.id FROM notification_subscriptions ns @@ -545,6 +545,7 @@ isUserSubscribedToWatchProject userId projId = do WHERE ns.subscriber_user_id = #{userId} AND ns.scope_user_id = p.owner_user_id AND ns.topic_groups = ARRAY[#{WatchProject}::notification_topic_group] + AND ns.project_id = #{projId} AND ns.filter = #{filter}::jsonb LIMIT 1 |] diff --git a/src/Share/Notifications/Types.hs b/src/Share/Notifications/Types.hs index 93f10677..f271ba28 100644 --- a/src/Share/Notifications/Types.hs +++ b/src/Share/Notifications/Types.hs @@ -579,6 +579,7 @@ instance Aeson.FromJSON NotificationDeliveryMethod where data NotificationSubscription id = NotificationSubscription { subscriptionId :: id, subscriptionScope :: UserId, + subscriptionProjectId :: Maybe ProjectId, subscriptionTopics :: Set NotificationTopic, subscriptionTopicGroups :: Set NotificationTopicGroup, subscriptionFilter :: Maybe NotificationFilter @@ -588,16 +589,18 @@ instance PG.DecodeRow (NotificationSubscription NotificationSubscriptionId) wher decodeRow = do subscriptionId <- PG.decodeField subscriptionScope <- PG.decodeField + subscriptionProjectId <- PG.decodeField subscriptionTopics <- Set.fromList <$> PG.decodeField subscriptionTopicGroups <- Set.fromList <$> PG.decodeField subscriptionFilter <- PG.decodeField - pure $ NotificationSubscription {subscriptionId, subscriptionScope, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter} + pure $ NotificationSubscription {subscriptionId, subscriptionScope, subscriptionProjectId, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter} instance Aeson.ToJSON (NotificationSubscription NotificationSubscriptionId) where - toJSON NotificationSubscription {subscriptionId, subscriptionScope, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter} = + toJSON NotificationSubscription {subscriptionId, subscriptionScope, subscriptionProjectId, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter} = Aeson.object [ "id" Aeson..= subscriptionId, "scope" Aeson..= subscriptionScope, + "projectId" Aeson..= subscriptionProjectId, "topics" Aeson..= subscriptionTopics, "topicGroups" Aeson..= subscriptionTopicGroups, "filter" Aeson..= subscriptionFilter @@ -607,10 +610,11 @@ instance Aeson.FromJSON (NotificationSubscription NotificationSubscriptionId) wh parseJSON = Aeson.withObject "NotificationSubscription" \o -> do subscriptionId <- o .: "id" subscriptionScope <- o .: "scope" + subscriptionProjectId <- o .: "projectId" subscriptionTopics <- o .: "topics" subscriptionTopicGroups <- o .: "topicGroups" subscriptionFilter <- o .:? "filter" - pure NotificationSubscription {subscriptionId, subscriptionScope, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter} + pure NotificationSubscription {subscriptionId, subscriptionScope, subscriptionProjectId, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter} data NotificationHubEntry userInfo eventPayload = NotificationHubEntry { hubEntryId :: NotificationHubEntryId, From 4adc1a3e42da6000d075106860cac2810fee214d Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Tue, 2 Sep 2025 11:53:10 -0700 Subject: [PATCH 05/35] Propagate new projectIds on events --- src/Share/Notifications/API.hs | 7 +++++-- src/Share/Notifications/Impl.hs | 4 ++-- src/Share/Postgres/Comments/Ops.hs | 7 ++++--- src/Share/Postgres/Contributions/Ops.hs | 2 ++ src/Share/Postgres/Queries.hs | 2 ++ src/Share/Postgres/Releases/Ops.hs | 1 + src/Share/Postgres/Tickets/Ops.hs | 2 ++ 7 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/Share/Notifications/API.hs b/src/Share/Notifications/API.hs index bb1d5da9..55baa842 100644 --- a/src/Share/Notifications/API.hs +++ b/src/Share/Notifications/API.hs @@ -137,15 +137,17 @@ type CreateSubscriptionEndpoint = data CreateSubscriptionRequest = CreateSubscriptionRequest { subscriptionScope :: UserHandle, + subscriptionProjectId :: Maybe ProjectId, subscriptionTopics :: Set NotificationTopic, subscriptionTopicGroups :: Set NotificationTopicGroup, subscriptionFilter :: Maybe SubscriptionFilter } instance ToJSON CreateSubscriptionRequest where - toJSON CreateSubscriptionRequest {subscriptionScope, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter} = + toJSON CreateSubscriptionRequest {subscriptionScope, subscriptionProjectId, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter} = object [ "scope" .= subscriptionScope, + "projectId" .= subscriptionProjectId, "topics" .= subscriptionTopics, "topicGroups" .= subscriptionTopicGroups, "filter" .= subscriptionFilter @@ -154,10 +156,11 @@ instance ToJSON CreateSubscriptionRequest where instance FromJSON CreateSubscriptionRequest where parseJSON = withObject "CreateSubscriptionRequest" $ \o -> do subscriptionScope <- o .: "scope" + subscriptionProjectId <- o .:? "projectId" subscriptionTopics <- o .: "topics" subscriptionTopicGroups <- o .: "topicGroups" subscriptionFilter <- o .:? "filter" - pure CreateSubscriptionRequest {subscriptionScope, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter} + pure CreateSubscriptionRequest {subscriptionScope, subscriptionProjectId, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter} data CreateSubscriptionResponse = CreateSubscriptionResponse diff --git a/src/Share/Notifications/Impl.hs b/src/Share/Notifications/Impl.hs index c911493a..818cfc90 100644 --- a/src/Share/Notifications/Impl.hs +++ b/src/Share/Notifications/Impl.hs @@ -180,7 +180,7 @@ getSubscriptionsEndpoint userHandle callerUserId = do pure $ API.GetSubscriptionsResponse {subscriptions} createSubscriptionEndpoint :: UserHandle -> UserId -> API.CreateSubscriptionRequest -> WebApp API.CreateSubscriptionResponse -createSubscriptionEndpoint subscriberHandle callerUserId API.CreateSubscriptionRequest {subscriptionScope, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter} = do +createSubscriptionEndpoint subscriberHandle callerUserId API.CreateSubscriptionRequest {subscriptionScope, subscriptionProjectId, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter} = do User {user_id = subscriberUserId} <- UserQ.expectUserByHandle subscriberHandle User {user_id = scopeUserId} <- UserQ.expectUserByHandle subscriptionScope _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkSubscriptionsManage callerUserId subscriberUserId @@ -189,7 +189,7 @@ createSubscriptionEndpoint subscriberHandle callerUserId API.CreateSubscriptionR -- the resource of a given event for the permission associated to that topic via the -- 'topic_permission' SQL function. subscription <- PG.runTransaction $ do - subscriptionId <- NotificationQ.createNotificationSubscription subscriberUserId scopeUserId subscriptionTopics subscriptionTopicGroups subscriptionFilter + subscriptionId <- NotificationQ.createNotificationSubscription subscriberUserId scopeUserId subscriptionProjectId subscriptionTopics subscriptionTopicGroups subscriptionFilter NotificationQ.getNotificationSubscription subscriberUserId subscriptionId pure $ API.CreateSubscriptionResponse {subscription} diff --git a/src/Share/Postgres/Comments/Ops.hs b/src/Share/Postgres/Comments/Ops.hs index 54ccb0c9..518c24cb 100644 --- a/src/Share/Postgres/Comments/Ops.hs +++ b/src/Share/Postgres/Comments/Ops.hs @@ -40,7 +40,7 @@ createComment authorId thingId content = do { commentId, commentAuthorUserId = authorId } - (event, projectResourceId, projectOwnerUserId) <- case thingId of + (event, projectResourceId, projectOwnerUserId, projectId) <- case thingId of Left contributionId -> do Contribution {projectId, sourceBranchId, targetBranchId, author} <- ContributionQ.contributionById contributionId (projectData, projectResourceId, projectOwnerUserId) <- ProjectQ.projectNotificationData projectId @@ -51,7 +51,7 @@ createComment authorId thingId content = do toBranchId = targetBranchId, contributorUserId = author } - pure (ProjectContributionCommentData projectData contributionData commentData, projectResourceId, projectOwnerUserId) + pure (ProjectContributionCommentData projectData contributionData commentData, projectResourceId, projectOwnerUserId, projectId) Right ticketId -> do Ticket {projectId, author} <- TicketQ.ticketById ticketId (projectData, projectResourceId, projectOwnerUserId) <- ProjectQ.projectNotificationData projectId @@ -60,12 +60,13 @@ createComment authorId thingId content = do { ticketId, ticketAuthorUserId = author } - pure (ProjectTicketCommentData projectData ticketData commentData, projectResourceId, projectOwnerUserId) + pure (ProjectTicketCommentData projectData ticketData commentData, projectResourceId, projectOwnerUserId, projectId) let notifEvent = NotificationEvent { eventId = (), eventOccurredAt = (), eventResourceId = projectResourceId, + eventProjectId = Just projectId, eventData = event, eventScope = projectOwnerUserId, eventActor = authorId diff --git a/src/Share/Postgres/Contributions/Ops.hs b/src/Share/Postgres/Contributions/Ops.hs index 046a7ef0..602a2fb3 100644 --- a/src/Share/Postgres/Contributions/Ops.hs +++ b/src/Share/Postgres/Contributions/Ops.hs @@ -78,6 +78,7 @@ createContribution authorId projectId title description status sourceBranchId ta { eventId = (), eventOccurredAt = (), eventResourceId = projectResourceId, + eventProjectId = Just projectId, eventData = ProjectContributionCreatedData projectData contributionData, eventScope = projectOwnerUserId, eventActor = authorId @@ -151,6 +152,7 @@ insertContributionStatusChangeEvent projectId contributionId actorUserId oldStat { eventId = (), eventOccurredAt = (), eventResourceId = projectResourceId, + eventProjectId = Just projectId, eventData = ProjectContributionStatusUpdatedData projectData contributionData statusUpdateData, eventScope = projectOwnerUserId, eventActor = actorUserId diff --git a/src/Share/Postgres/Queries.hs b/src/Share/Postgres/Queries.hs index 43fd9f89..b4d7be11 100644 --- a/src/Share/Postgres/Queries.hs +++ b/src/Share/Postgres/Queries.hs @@ -614,6 +614,7 @@ createBranch !_nlReceipt projectId branchName contributorId causalId mergeTarget { eventId = (), eventOccurredAt = (), eventResourceId = projectResourceId, + eventProjectId = Just projectId, eventData = ProjectBranchUpdatedData projectData branchData, eventScope = projectOwnerUserId, eventActor = creatorId @@ -681,6 +682,7 @@ setBranchCausalHash !_nameLookupReceipt description callerUserId branchId causal { eventId = (), eventOccurredAt = (), eventResourceId = projectResourceId, + eventProjectId = Just projectId, eventData = ProjectBranchUpdatedData projectData branchData, eventScope = projectOwnerUserId, eventActor = callerUserId diff --git a/src/Share/Postgres/Releases/Ops.hs b/src/Share/Postgres/Releases/Ops.hs index 346b3d88..8f01c0b3 100644 --- a/src/Share/Postgres/Releases/Ops.hs +++ b/src/Share/Postgres/Releases/Ops.hs @@ -59,6 +59,7 @@ createRelease !_nlReceipt projectId (releaseVersion@ReleaseVersion {major, minor { eventId = (), eventOccurredAt = (), eventResourceId = projectResourceId, + eventProjectId = Just projectId, eventData = ProjectReleaseCreatedData projectData releaseData, eventScope = projectOwnerUserId, eventActor = creatorId diff --git a/src/Share/Postgres/Tickets/Ops.hs b/src/Share/Postgres/Tickets/Ops.hs index 3947eca9..d62a5314 100644 --- a/src/Share/Postgres/Tickets/Ops.hs +++ b/src/Share/Postgres/Tickets/Ops.hs @@ -60,6 +60,7 @@ createTicket authorId projectId title description status = do { eventId = (), eventOccurredAt = (), eventResourceId = projectResourceId, + eventProjectId = Just projectId, eventData = ProjectTicketCreatedData projectData ticketData, eventScope = projectOwnerUserId, eventActor = authorId @@ -112,6 +113,7 @@ insertTicketStatusChangeEvent projectId ticketId actorUserId oldStatus newStatus { eventId = (), eventOccurredAt = (), eventResourceId = projectResourceId, + eventProjectId = Just projectId, eventData = ProjectTicketStatusUpdatedData projectData ticketData statusUpdateData, eventScope = projectOwnerUserId, eventActor = actorUserId From e6478bbdf8286dd5e57e88d5b09664e53c76b195 Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Tue, 2 Sep 2025 11:07:16 -0700 Subject: [PATCH 06/35] Add project webhook api endpoints and types --- src/Share/Notifications/Types.hs | 1 + src/Share/Web/Share/Projects/API.hs | 24 +++++ src/Share/Web/Share/Projects/Types.hs | 128 ++++++++++++++++++++++++++ 3 files changed, 153 insertions(+) diff --git a/src/Share/Notifications/Types.hs b/src/Share/Notifications/Types.hs index f271ba28..4bdb02ed 100644 --- a/src/Share/Notifications/Types.hs +++ b/src/Share/Notifications/Types.hs @@ -584,6 +584,7 @@ data NotificationSubscription id = NotificationSubscription subscriptionTopicGroups :: Set NotificationTopicGroup, subscriptionFilter :: Maybe NotificationFilter } + deriving (Show, Eq) instance PG.DecodeRow (NotificationSubscription NotificationSubscriptionId) where decodeRow = do diff --git a/src/Share/Web/Share/Projects/API.hs b/src/Share/Web/Share/Projects/API.hs index 860add47..836266a6 100644 --- a/src/Share/Web/Share/Projects/API.hs +++ b/src/Share/Web/Share/Projects/API.hs @@ -43,6 +43,7 @@ type ProjectResourceAPI = ) :<|> ("roles" :> MaintainersResourceAPI) :<|> ("subscription" :> ProjectNotificationSubscriptionEndpoint) + :<|> ("webhooks" :> ProjectWebhooksResourceAPI) ) type ProjectDiffNamespacesEndpoint = @@ -116,3 +117,26 @@ type RemoveRolesEndpoint = type ProjectNotificationSubscriptionEndpoint = ReqBody '[JSON] ProjectNotificationSubscriptionRequest :> Put '[JSON] ProjectNotificationSubscriptionResponse + +type ProjectWebhooksResourceAPI = + ( ListProjectWebhooksEndpoint + :<|> CreateProjectWebhookEndpoint + :<|> ( Capture "subscription_id" NotificationSubscriptionId + :> ( UpdateProjectWebhookEndpoint + :<|> DeleteProjectWebhookEndpoint + ) + ) + ) + +type ListProjectWebhooksEndpoint = Get '[JSON] ListProjectWebhooksResponse + +type CreateProjectWebhookEndpoint = + ReqBody '[JSON] CreateProjectWebhookRequest + :> Post '[JSON] CreateProjectWebhookResponse + +type UpdateProjectWebhookEndpoint = + ReqBody '[JSON] UpdateProjectWebhookRequest + :> Patch '[JSON] UpdateProjectWebhookResponse + +type DeleteProjectWebhookEndpoint = + Delete '[JSON] () diff --git a/src/Share/Web/Share/Projects/Types.hs b/src/Share/Web/Share/Projects/Types.hs index 8b8ce284..e3df72a4 100644 --- a/src/Share/Web/Share/Projects/Types.hs +++ b/src/Share/Web/Share/Projects/Types.hs @@ -26,6 +26,12 @@ module Share.Web.Share.Projects.Types APIProjectBranchAndReleaseDetails (..), PermissionsInfo (..), IsSubscribed (..), + ListProjectWebhooksResponse (..), + ProjectWebhook (..), + CreateProjectWebhookRequest (..), + CreateProjectWebhookResponse (..), + UpdateProjectWebhookRequest (..), + UpdateProjectWebhookResponse (..), ) where @@ -35,10 +41,12 @@ import Data.Set qualified as Set import Data.Time (UTCTime) import Share.IDs import Share.IDs qualified as IDs +import Share.Notifications.Types import Share.Postgres qualified as PG import Share.Prelude import Share.Project (Project (..), ProjectTag, ProjectVisibility (..)) import Share.Utils.API +import Share.Utils.URI (URIParam) import Share.Web.Authorization.Types (PermissionsInfo (PermissionsInfo)) projectToAPI :: ProjectOwner -> Project -> APIProject @@ -376,3 +384,123 @@ instance Aeson.ToJSON CatalogCategory where [ "name" .= name, "projects" .= projects ] + +-- | This type provides a view over the subscription <-> webhook many-to-many view. +data ProjectWebhook = ProjectWebhook + { url :: URIParam, + events :: Set NotificationTopic, + notificationSubscriptionId :: NotificationSubscriptionId, + -- Not currently exposed. + -- webhookId :: NotificationWebhookId, + createdAt :: UTCTime, + updatedAt :: UTCTime + } + deriving stock (Eq, Show) + +instance ToJSON ProjectWebhook where + toJSON ProjectWebhook {..} = + object + [ "url" .= url, + "events" .= events, + "notificationSubscriptionId" .= notificationSubscriptionId, + "createdAt" .= createdAt, + "updatedAt" .= updatedAt + ] + +instance FromJSON ProjectWebhook where + parseJSON = Aeson.withObject "ProjectWebhook" $ \o -> do + url <- o .: "url" + events <- o .: "events" + notificationSubscriptionId <- o .: "notificationSubscriptionId" + createdAt <- o .: "createdAt" + updatedAt <- o .: "updatedAt" + pure ProjectWebhook {..} + +data ListProjectWebhooksResponse = ListProjectWebhooksResponse + { webhooks :: [ProjectWebhook] + } + deriving stock (Eq, Show) + +instance ToJSON ListProjectWebhooksResponse where + toJSON ListProjectWebhooksResponse {..} = + object + [ "webhooks" .= webhooks + ] + +instance FromJSON ListProjectWebhooksResponse where + parseJSON = Aeson.withObject "ListProjectWebhooksResponse" $ \o -> do + webhooks <- o .: "webhooks" + pure ListProjectWebhooksResponse {..} + +data CreateProjectWebhookRequest = CreateProjectWebhookRequest + { url :: URIParam, + events :: Set NotificationTopic, + subscriber :: UserHandle + } + deriving stock (Eq, Show) + +instance ToJSON CreateProjectWebhookRequest where + toJSON CreateProjectWebhookRequest {..} = + object + [ "url" .= url, + "events" .= events, + "subscriber" .= (IDs.toText $ PrefixedID @"@" subscriber) + ] + +instance FromJSON CreateProjectWebhookRequest where + parseJSON = Aeson.withObject "CreateProjectWebhookRequest" $ \o -> do + url <- o .: "url" + events <- o .: "events" + subscriber <- o .: "subscriber" + pure CreateProjectWebhookRequest {..} + +data CreateProjectWebhookResponse = CreateProjectWebhookResponse + { webhook :: ProjectWebhook + } + deriving stock (Eq, Show) + +instance ToJSON CreateProjectWebhookResponse where + toJSON CreateProjectWebhookResponse {..} = + object + [ "webhook" .= webhook + ] + +instance FromJSON CreateProjectWebhookResponse where + parseJSON = Aeson.withObject "CreateProjectWebhookResponse" $ \o -> do + webhook <- o .: "webhook" + pure CreateProjectWebhookResponse {..} + +data UpdateProjectWebhookRequest = UpdateProjectWebhookRequest + { url :: Maybe URIParam, + events :: Maybe (Set NotificationTopic) + } + deriving stock (Eq, Show) + +instance ToJSON UpdateProjectWebhookRequest where + toJSON UpdateProjectWebhookRequest {..} = + object + [ "url" .= url, + "events" .= events + ] + +instance FromJSON UpdateProjectWebhookRequest where + parseJSON = Aeson.withObject "UpdateProjectWebhookRequest" $ \o -> do + url <- o .:? "url" + events <- o .:? "events" + pure UpdateProjectWebhookRequest {..} + +data UpdateProjectWebhookResponse = UpdateProjectWebhookResponse + { webhook :: ProjectWebhook + } + deriving stock (Eq, Show) + +instance ToJSON UpdateProjectWebhookResponse where + toJSON UpdateProjectWebhookResponse {..} = + object + [ "webhook" .= webhook + ] + +instance FromJSON UpdateProjectWebhookResponse where + parseJSON = Aeson.withObject "UpdateProjectWebhookResponse" $ \o -> do + webhook <- o .: "webhook" + pure UpdateProjectWebhookResponse {..} From ac525f1dbafbd364aa0ec793824bb8a97818e698 Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Tue, 2 Sep 2025 12:28:18 -0700 Subject: [PATCH 07/35] Implement createProjectWebhookEndpoint --- src/Share/Notifications/Ops.hs | 53 +++++++++++++++++++++++ src/Share/Notifications/Queries.hs | 35 ++++++++++++++- src/Share/Notifications/Types.hs | 19 ++++++--- src/Share/Web/Share/Projects/Impl.hs | 61 +++++++++++++++++++++++++++ src/Share/Web/Share/Projects/Types.hs | 15 +++++-- 5 files changed, 172 insertions(+), 11 deletions(-) diff --git a/src/Share/Notifications/Ops.hs b/src/Share/Notifications/Ops.hs index ad378e1a..4cff9d66 100644 --- a/src/Share/Notifications/Ops.hs +++ b/src/Share/Notifications/Ops.hs @@ -3,21 +3,30 @@ module Share.Notifications.Ops createWebhookDeliveryMethod, updateWebhookDeliveryMethod, deleteWebhookDeliveryMethod, + listProjectWebhooks, + createProjectWebhook, hydrateEvent, ) where +import Control.Lens +import Data.Set.NonEmpty qualified as NESet import Share.IDs import Share.Notifications.Queries qualified as NotifQ import Share.Notifications.Types import Share.Notifications.Webhooks.Secrets (WebhookConfig (..)) import Share.Notifications.Webhooks.Secrets qualified as WebhookSecrets +import Share.Notifications.Webhooks.Secrets qualified as Webhooks import Share.Postgres qualified as PG +import Share.Postgres.Queries qualified as Q import Share.Prelude +import Share.Project (Project (..)) import Share.Utils.URI (URIParam (..)) import Share.Web.App (WebApp) import Share.Web.Errors (respondError) +import Share.Web.Share.Projects.Types (ProjectWebhook (..)) import Share.Web.UI.Links qualified as Links +import UnliftIO qualified listNotificationDeliveryMethods :: UserId -> Maybe NotificationSubscriptionId -> WebApp [NotificationDeliveryMethod] listNotificationDeliveryMethods userId maySubscriptionId = do @@ -82,3 +91,47 @@ hydrateEvent :: HydratedEventPayload -> PG.Transaction e HydratedEvent hydrateEvent hydratedEventPayload = do hydratedEventLink <- Links.notificationLink hydratedEventPayload pure $ HydratedEvent {hydratedEventPayload, hydratedEventLink} + +-- | We provide a wrapper layer on top of notification subscriptions and webhooks +-- to make the frontend experience a bit more intuitive. +listProjectWebhooks :: UserId -> ProjectId -> WebApp [ProjectWebhook] +listProjectWebhooks caller projectId = do + projectWebhooks <- PG.runTransaction $ do NotifQ.listProjectWebhooks caller projectId + results <- + projectWebhooks + & asListOf (traversed . _1) %%~ \webhookIds -> + do + UnliftIO.pooledForConcurrently webhookIds \webhookId -> do + Webhooks.fetchWebhookConfig webhookId >>= \case + Left err -> respondError err + Right (WebhookConfig {uri = URIParam uri}) -> do + pure $ (NotificationWebhookConfig webhookId uri) + let webhooks = + results <&> \(NotificationWebhookConfig {webhookDeliveryUrl = url}, _name, NotificationSubscription {subscriptionTopics, subscriptionId, subscriptionCreatedAt, subscriptionUpdatedAt}) -> + ProjectWebhook + { url = URIParam url, + events = subscriptionTopics, + notificationSubscriptionId = subscriptionId, + createdAt = subscriptionCreatedAt, + updatedAt = subscriptionUpdatedAt + } + pure webhooks + +createProjectWebhook :: UserId -> ProjectId -> URIParam -> Set NotificationTopic -> WebApp ProjectWebhook +createProjectWebhook subscriberUserId projectId url topics = do + webhookId <- createWebhookDeliveryMethod subscriberUserId url "Project Webhook" + let topicGroups = mempty + let filter = Nothing + subscriptionId <- PG.runTransaction $ do + Project {ownerUserId = projectOwner} <- Q.expectProjectById projectId + subscriptionId <- NotifQ.createNotificationSubscription subscriberUserId projectOwner (Just projectId) topics topicGroups filter + NotifQ.addSubscriptionDeliveryMethods subscriberUserId subscriptionId (NESet.singleton $ WebhookDeliveryMethodId webhookId) + pure subscriptionId + pure $ + ProjectWebhook + { url, + events = topics, + notificationSubscriptionId = subscriptionId, + createdAt = Nothing, + updatedAt = Nothing + } diff --git a/src/Share/Notifications/Queries.hs b/src/Share/Notifications/Queries.hs index eabbadac..dadcf53a 100644 --- a/src/Share/Notifications/Queries.hs +++ b/src/Share/Notifications/Queries.hs @@ -1,3 +1,5 @@ +{-# LANGUAGE TypeOperators #-} + module Share.Notifications.Queries ( recordEvent, expectEvent, @@ -21,6 +23,7 @@ module Share.Notifications.Queries hasUnreadNotifications, updateWatchProjectSubscription, isUserSubscribedToWatchProject, + listProjectWebhooks, ) where @@ -288,7 +291,7 @@ listNotificationSubscriptions :: UserId -> Transaction e [NotificationSubscripti listNotificationSubscriptions subscriberUserId = do queryListRows [sql| - SELECT ns.id, ns.scope_user_id, ns.project_id, ns.topics, ns.topic_groups, ns.filter + SELECT ns.id, ns.scope_user_id, ns.project_id, ns.topics, ns.topic_groups, ns.filter, ns.created_at, ns.updated_at FROM notification_subscriptions ns WHERE ns.subscriber_user_id = #{subscriberUserId} ORDER BY ns.created_at DESC @@ -328,7 +331,7 @@ getNotificationSubscription :: UserId -> NotificationSubscriptionId -> Transacti getNotificationSubscription subscriberUserId subscriptionId = do queryExpect1Row [sql| - SELECT ns.id, ns.scope_user_id, ns.topics, ns.topic_groups, ns.filter + SELECT ns.id, ns.scope_user_id, ns.topics, ns.topic_groups, ns.filter, ns.created_at, ns.updated_at FROM notification_subscriptions ns WHERE ns.id = #{subscriptionId} AND ns.subscriber_user_id = #{subscriberUserId} @@ -549,3 +552,31 @@ isUserSubscribedToWatchProject userId projId = do AND ns.filter = #{filter}::jsonb LIMIT 1 |] + +-- | We provide a wrapper layer on top of notification subscriptions and webhooks +-- to make the frontend experience a bit more intuitive. +listProjectWebhooks :: UserId -> ProjectId -> PG.Transaction e [(NotificationWebhookId, Text, NotificationSubscription NotificationSubscriptionId)] +listProjectWebhooks caller projectId = do + PG.queryListRows @((NotificationWebhookId, Text) PG.:. NotificationSubscription NotificationSubscriptionId) + [PG.sql| + -- Only get one webhook per subscription, there _shouldn't_ be multiple webhooks per + -- subscription if everything is working correctly. + SELECT DISTINCT ON (ns.id) + nw.id, + nw.name, + -- We ignore topic groups here for now and only allow the user to configure + -- individual topic events. + ns.id, + ns.scope_user_id, + ns.topics, + ns.topic_groups, + ns.filter, + ns.created_at, + ns.updated_at + FROM notification_subscriptions ns + JOIN notification_by_webhook nbw ON ns.id = nbw.subscription_id + JOIN notification_webhooks nw ON nbw.webhook_id = nw.id + WHERE ns.project_id = #{projectId} + AND ns.subscriber_user_id = #{caller} + |] + <&> fmap (\((webhookId, webhookName) PG.:. subscription) -> (webhookId, webhookName, subscription)) diff --git a/src/Share/Notifications/Types.hs b/src/Share/Notifications/Types.hs index 4bdb02ed..2c1a383f 100644 --- a/src/Share/Notifications/Types.hs +++ b/src/Share/Notifications/Types.hs @@ -582,7 +582,9 @@ data NotificationSubscription id = NotificationSubscription subscriptionProjectId :: Maybe ProjectId, subscriptionTopics :: Set NotificationTopic, subscriptionTopicGroups :: Set NotificationTopicGroup, - subscriptionFilter :: Maybe NotificationFilter + subscriptionFilter :: Maybe NotificationFilter, + subscriptionCreatedAt :: Maybe UTCTime, + subscriptionUpdatedAt :: Maybe UTCTime } deriving (Show, Eq) @@ -594,17 +596,21 @@ instance PG.DecodeRow (NotificationSubscription NotificationSubscriptionId) wher subscriptionTopics <- Set.fromList <$> PG.decodeField subscriptionTopicGroups <- Set.fromList <$> PG.decodeField subscriptionFilter <- PG.decodeField - pure $ NotificationSubscription {subscriptionId, subscriptionScope, subscriptionProjectId, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter} + subscriptionCreatedAt <- PG.decodeField + subscriptionUpdatedAt <- PG.decodeField + pure $ NotificationSubscription {subscriptionId, subscriptionScope, subscriptionProjectId, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter, subscriptionCreatedAt, subscriptionUpdatedAt} instance Aeson.ToJSON (NotificationSubscription NotificationSubscriptionId) where - toJSON NotificationSubscription {subscriptionId, subscriptionScope, subscriptionProjectId, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter} = + toJSON NotificationSubscription {subscriptionId, subscriptionScope, subscriptionProjectId, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter, subscriptionCreatedAt, subscriptionUpdatedAt} = Aeson.object [ "id" Aeson..= subscriptionId, "scope" Aeson..= subscriptionScope, "projectId" Aeson..= subscriptionProjectId, "topics" Aeson..= subscriptionTopics, "topicGroups" Aeson..= subscriptionTopicGroups, - "filter" Aeson..= subscriptionFilter + "filter" Aeson..= subscriptionFilter, + "createdAt" Aeson..= subscriptionCreatedAt, + "updatedAt" Aeson..= subscriptionUpdatedAt ] instance Aeson.FromJSON (NotificationSubscription NotificationSubscriptionId) where @@ -615,7 +621,10 @@ instance Aeson.FromJSON (NotificationSubscription NotificationSubscriptionId) wh subscriptionTopics <- o .: "topics" subscriptionTopicGroups <- o .: "topicGroups" subscriptionFilter <- o .:? "filter" - pure NotificationSubscription {subscriptionId, subscriptionScope, subscriptionProjectId, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter} + subscriptionCreatedAt <- o .:? "createdAt" + subscriptionUpdatedAt <- o .:? "updatedAt" + + pure NotificationSubscription {subscriptionId, subscriptionScope, subscriptionProjectId, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter, subscriptionCreatedAt, subscriptionUpdatedAt} data NotificationHubEntry userInfo eventPayload = NotificationHubEntry { hubEntryId :: NotificationHubEntryId, diff --git a/src/Share/Web/Share/Projects/Impl.hs b/src/Share/Web/Share/Projects/Impl.hs index 2364e444..8d2c7d3e 100644 --- a/src/Share/Web/Share/Projects/Impl.hs +++ b/src/Share/Web/Share/Projects/Impl.hs @@ -22,6 +22,7 @@ import Share.Codebase.CodebaseRuntime qualified as CR import Share.Env qualified as Env import Share.IDs (PrefixedHash (..), ProjectSlug (..), UserHandle, UserId) import Share.IDs qualified as IDs +import Share.Notifications.Ops qualified as NotifOps import Share.Notifications.Queries qualified as NotifsQ import Share.OAuth.Session import Share.Postgres qualified as PG @@ -123,6 +124,7 @@ projectServer session handle = :<|> favProjectEndpoint session handle slug :<|> maintainersResourceServer slug :<|> projectNotificationSubscriptionEndpoint session handle slug + :<|> projectWebhooksServer session handle slug ) where addTags :: forall x. ProjectSlug -> WebApp x -> WebApp x @@ -429,3 +431,62 @@ projectNotificationSubscriptionEndpoint session projectUserHandler projectSlug ( ProjectNotificationSubscriptionResponse { subscriptionId = maySubscriptionId } + +projectWebhooksServer :: Maybe Session -> UserHandle -> ProjectSlug -> ServerT API.ProjectWebhooksResourceAPI WebApp +projectWebhooksServer session projectUserHandle projectSlug = + listProjectWebhooksEndpoint session projectUserHandle projectSlug + :<|> createProjectWebhookEndpoint session projectUserHandle projectSlug + :<|> ( \subscriptionId -> + updateProjectWebhookEndpoint session projectUserHandle projectSlug subscriptionId + :<|> deleteProjectWebhookEndpoint session projectUserHandle projectSlug subscriptionId + ) + +listProjectWebhooksEndpoint :: Maybe Session -> UserHandle -> ProjectSlug -> WebApp ListProjectWebhooksResponse +listProjectWebhooksEndpoint session projectUserHandle projectSlug = do + caller <- AuthN.requireAuthenticatedUser session + Project {projectId, ownerUserId = projectOwner} <- PG.runTransactionOrRespondError $ do + Q.projectByShortHand projectShortHand `whenNothingM` throwError (EntityMissing (ErrorID "project-not-found") ("Project not found: " <> IDs.toText @IDs.ProjectShortHand projectShortHand)) + _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkSubscriptionsView caller projectOwner + webhooks <- NotifOps.listProjectWebhooks caller projectId + pure $ ListProjectWebhooksResponse {webhooks} + where + projectShortHand :: IDs.ProjectShortHand + projectShortHand = IDs.ProjectShortHand {userHandle = projectUserHandle, projectSlug} + +createProjectWebhookEndpoint :: Maybe Session -> UserHandle -> ProjectSlug -> CreateProjectWebhookRequest -> WebApp CreateProjectWebhookResponse +createProjectWebhookEndpoint session projectUserHandle projectSlug (CreateProjectWebhookRequest {url, events}) = do + caller <- AuthN.requireAuthenticatedUser session + Project {projectId, ownerUserId = projectOwner} <- PG.runTransactionOrRespondError $ do + Q.projectByShortHand projectShortHand `whenNothingM` throwError (EntityMissing (ErrorID "project-not-found") ("Project not found: " <> IDs.toText @IDs.ProjectShortHand projectShortHand)) + _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkSubscriptionsManage caller projectOwner + webhook <- NotifOps.createProjectWebhook caller projectId url events + pure $ CreateProjectWebhookResponse {webhook} + where + projectShortHand :: IDs.ProjectShortHand + projectShortHand = IDs.ProjectShortHand {userHandle = projectUserHandle, projectSlug} + +updateProjectWebhookEndpoint :: Maybe Session -> UserHandle -> ProjectSlug -> IDs.NotificationSubscriptionId -> UpdateProjectWebhookRequest -> WebApp UpdateProjectWebhookResponse +updateProjectWebhookEndpoint session projectUserHandle projectSlug subscriptionId (UpdateProjectWebhookRequest {url, events}) = do + caller <- AuthN.requireAuthenticatedUser session + Project {projectId, ownerUserId = projectOwner} <- PG.runTransactionOrRespondError $ do + Q.projectByShortHand projectShortHand `whenNothingM` throwError (EntityMissing (ErrorID "project-not-found") ("Project not found: " <> IDs.toText @IDs.ProjectShortHand projectShortHand)) + _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkSubscriptionsManage caller projectOwner + webhook <- PG.runTransactionOrRespondError $ do + NotifsQ.updateProjectWebhook caller projectId subscriptionId url events `whenNothingM` throwError (EntityMissing (ErrorID "webhook:not-found") "Webhook not found") + pure $ UpdateProjectWebhookResponse {webhook} + where + projectShortHand :: IDs.ProjectShortHand + projectShortHand = IDs.ProjectShortHand {userHandle = projectUserHandle, projectSlug} + +deleteProjectWebhookEndpoint :: Maybe Session -> UserHandle -> ProjectSlug -> IDs.NotificationSubscriptionId -> WebApp () +deleteProjectWebhookEndpoint session projectUserHandle projectSlug subscriptionId = do + caller <- AuthN.requireAuthenticatedUser session + Project {projectId, ownerUserId = projectOwner} <- PG.runTransactionOrRespondError $ do + Q.projectByShortHand projectShortHand `whenNothingM` throwError (EntityMissing (ErrorID "project-not-found") ("Project not found: " <> IDs.toText @IDs.ProjectShortHand projectShortHand)) + _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkSubscriptionsManage caller projectOwner + success <- PG.runTransaction $ NotifsQ.deleteProjectWebhook caller projectId subscriptionId + unless success $ respondError (EntityMissing (ErrorID "webhook:not-found") "Webhook not found") + pure () + where + projectShortHand :: IDs.ProjectShortHand + projectShortHand = IDs.ProjectShortHand {userHandle = projectUserHandle, projectSlug} diff --git a/src/Share/Web/Share/Projects/Types.hs b/src/Share/Web/Share/Projects/Types.hs index e3df72a4..212fad4c 100644 --- a/src/Share/Web/Share/Projects/Types.hs +++ b/src/Share/Web/Share/Projects/Types.hs @@ -390,13 +390,20 @@ data ProjectWebhook = ProjectWebhook { url :: URIParam, events :: Set NotificationTopic, notificationSubscriptionId :: NotificationSubscriptionId, - -- Not currently exposed. - -- webhookId :: NotificationWebhookId, - createdAt :: UTCTime, - updatedAt :: UTCTime + createdAt :: Maybe UTCTime, + updatedAt :: Maybe UTCTime } deriving stock (Eq, Show) +instance PG.DecodeRow ProjectWebhook where + decodeRow = do + url <- PG.decodeField + events <- Set.fromList <$> PG.decodeField + notificationSubscriptionId <- PG.decodeField + createdAt <- PG.decodeField + updatedAt <- PG.decodeField + pure ProjectWebhook {..} + instance ToJSON ProjectWebhook where toJSON ProjectWebhook {..} = object From 17074e92194caecc895cf988ae4fb7d4adad7ecd Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Wed, 3 Sep 2025 10:10:29 -0700 Subject: [PATCH 08/35] Map webhooks to notification subscriptions many to one --- sql/2025-09-02_project-webhooks.sql | 80 ---------------- sql/2025-09-03_project-webhooks.sql | 136 ++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 80 deletions(-) delete mode 100644 sql/2025-09-02_project-webhooks.sql create mode 100644 sql/2025-09-03_project-webhooks.sql diff --git a/sql/2025-09-02_project-webhooks.sql b/sql/2025-09-02_project-webhooks.sql deleted file mode 100644 index 2b53846e..00000000 --- a/sql/2025-09-02_project-webhooks.sql +++ /dev/null @@ -1,80 +0,0 @@ --- This migration codifies the project ID filter on notification subscriptions into a real field rather than a --- generic JSONB filter. You can still set it to NULL for org-wide subscriptions. - -ALTER TABLE notification_subscriptions - -- The project which this filter is scopped to. - -- If provided, the project_id must be belong to the scope_user_id's user/org. - ADD COLUMN project_id UUID NULL REFERENCES projects(id) ON DELETE CASCADE; - -CREATE INDEX notification_subscriptions_by_user_and_project ON notification_subscriptions(subscriber_user_id, project_id, created_at DESC); - -ALTER TABLE notification_events - ADD COLUMN project_id UUID NULL REFERENCES projects(id) ON DELETE CASCADE; - -CREATE INDEX notification_events_scope_user_and_project ON notification_events(scope_user_id, project_id, occurred_at DESC); - --- Migrate existing filters to the new column, and also remove --- the projectId from the JSONB filter. -UPDATE notification_subscriptions - SET project_id = (filter->>'projectId')::UUID, - filter = filter - 'projectId' - WHERE filter ? 'projectId'; - -UPDATE notification_events - SET project_id = (data->>'projectId')::UUID - WHERE data ? 'projectId'; - --- Now update the trigger so we use the new projectid columns -CREATE FUNCTION trigger_notification_event_subscriptions() -RETURNS TRIGGER AS $$ -DECLARE - the_subscription_id UUID; - the_event_id UUID; - the_subscriber UUID; -BEGIN - SELECT NEW.id INTO the_event_id; - FOR the_subscription_id, the_subscriber IN - (SELECT ns.id, ns.subscriber_user_id FROM notification_subscriptions ns - WHERE ns.scope_user_id = NEW.scope_user_id - AND NEW.topic = ANY(ns.topics) - AND (ns.project_id IS NULL OR ns.project_id = NEW.project_id) - AND (ns.filter IS NULL OR NEW.data @> ns.filter) - AND - -- A subscriber can be notified if the event is in their scope or if they have permission to the resource. - -- The latter is usually a superset of the former, but the former is trivial to compute so it can help - -- performance to include it. - (NEW.scope_user_id = ns.subscriber_user_id - OR user_has_permission(ns.subscriber_user_id, NEW.resource_id, topic_permission(NEW.topic)) - ) - ) - LOOP - -- Log that this event triggered this subscription. - INSERT INTO notification_providence_log (event_id, subscription_id) - VALUES (the_event_id, the_subscription_id); - - -- Add to the relevant queues. - -- Each delivery method _may_ be triggered by multiple subscriptions, - -- we need ON CONFLICT DO NOTHING. - INSERT INTO notification_webhook_queue (event_id, webhook_id) - SELECT the_event_id, nbw.webhook_id - FROM notification_by_webhook nbw - WHERE nbw.subscription_id = the_subscription_id - ON CONFLICT DO NOTHING; - - INSERT INTO notification_email_queue (event_id, email_id) - SELECT the_event_id AS event_id, nbe.email_id - FROM notification_by_email nbe - WHERE nbe.subscription_id = the_subscription_id - ON CONFLICT DO NOTHING; - - -- Also add the notification to the hub. - -- It's possible it was already added by another subscription for this user, - -- in which case we just carry on. - INSERT INTO notification_hub_entries (event_id, user_id) - VALUES (the_event_id, the_subscriber) - ON CONFLICT DO NOTHING; - END LOOP; - - RETURN NEW; -END; -$$ LANGUAGE plpgsql; diff --git a/sql/2025-09-03_project-webhooks.sql b/sql/2025-09-03_project-webhooks.sql new file mode 100644 index 00000000..70cb2270 --- /dev/null +++ b/sql/2025-09-03_project-webhooks.sql @@ -0,0 +1,136 @@ +-- This changes webhooks such that each webhook and email delivery method is assigned to exactly one subscription, rather than the +-- previous many-to-many relationship. + +-- WEBHOOKS +ALTER TABLE notification_webhooks + ADD COLUMN subscription_id UUID NULL REFERENCES notification_subscriptions(id) ON DELETE CASCADE, + DROP COLUMN subscriber_user_id; + +CREATE INDEX notification_webhooks_by_subscription ON notification_webhooks(subscription_id); + +UPDATE notification_webhooks nw + SET subscription_id = (SELECT nbw.subscription_id FROM notification_by_webhook nbw WHERE nbw.webhook_id = nw.id LIMIT 1); + +ALTER TABLE notification_webhooks + ALTER COLUMN subscription_id SET NOT NULL; + +-- EMAILS +ALTER TABLE notification_emails + ADD COLUMN subscription_id UUID NULL REFERENCES notification_subscriptions(id) ON DELETE CASCADE, + DROP COLUMN subscriber_user_id; + +CREATE INDEX notification_emails_by_subscription ON notification_emails(subscription_id); + +UPDATE notification_emails ne + SET subscription_id = (SELECT nbe.subscription_id FROM notification_by_email nbe WHERE nbe.email_id = ne.id LIMIT 1); + + +ALTER TABLE notification_emails + ALTER COLUMN subscription_id SET NOT NULL; + + + +-- This migration codifies the project ID filter on notification subscriptions into a real field rather than a +-- generic JSONB filter. You can still set it to NULL for org-wide subscriptions. +ALTER TABLE notification_subscriptions + -- The project which this filter is scoped to. + -- If provided, the project_id must be belong to the scope_user_id's user/org, or match the subscriber_project_id. + ADD COLUMN scope_project_id UUID NULL REFERENCES projects(id) ON DELETE CASCADE, + -- A subscription can belong to a project itself. + ADD COLUMN subscriber_project_id UUID NULL REFERENCES projects(id) ON DELETE CASCADE, + -- Project subscriptions won't have a subscriber_user_id anymore, they're part of the project. + ALTER COLUMN subscriber_user_id SET NOT NULL, + -- Add a constraint that either subscriber_user_id or subscriber_project_id must be set, but not both. + ADD CONSTRAINT notification_subscriptions_user_or_project CHECK ( + (subscriber_user_id IS NOT NULL AND subscriber_project_id IS NULL) + OR (subscriber_user_id IS NULL AND subscriber_project_id IS NOT NULL) + ), + -- Add a constraint that if the subscriber is a project, the scope_project_id must match the subscriber_project_id. + ADD CONSTRAINT notification_subscriptions_scope_project_matches CHECK ( + (subscriber_project_id IS NULL) + OR (scope_project_id IS NOT DISTINCT FROM subscriber_project_id) + ); + +-- Index to find for a user scoped to a specific project. +CREATE INDEX notification_subscriptions_by_user_and_project ON notification_subscriptions(subscriber_user_id, scope_project_id, created_at DESC); + +ALTER TABLE notification_events + ADD COLUMN project_id UUID NULL REFERENCES projects(id) ON DELETE CASCADE; + +CREATE INDEX notification_events_scope_user_and_project ON notification_events(scope_user_id, project_id, occurred_at DESC); + +-- Migrate existing filters to the new column, and also remove +-- the projectId from the JSONB filter. +UPDATE notification_subscriptions + SET project_id = (filter->>'projectId')::UUID, + filter = filter - 'projectId' + WHERE filter ? 'projectId'; + +UPDATE notification_events + SET project_id = (data->>'projectId')::UUID + WHERE data ? 'projectId'; + +-- Now update the trigger so we use the new projectid columns +CREATE FUNCTION trigger_notification_event_subscriptions() +RETURNS TRIGGER AS $$ +DECLARE + the_subscription_id UUID; + the_event_id UUID; + the_subscriber UUID; +BEGIN + SELECT NEW.id INTO the_event_id; + FOR the_subscription_id, the_subscriber IN + (SELECT ns.id, ns.subscriber_user_id FROM notification_subscriptions ns + WHERE ns.scope_user_id = NEW.scope_user_id + AND NEW.topic = ANY(ns.topics) + AND (ns.scope_project_id IS NULL OR ns.scope_project_id = NEW.project_id) + AND (ns.filter IS NULL OR NEW.data @> ns.filter) + AND + -- A subscriber can be notified if: + -- * the event is in their own user + -- * or if they have permission to the resource. + -- * or if the subscription is attached to the project of the event. + ((ns.subscriber_user_id IS NOT NULL + AND (NEW.scope_user_id = ns.subscriber_user_id + OR user_has_permission(ns.subscriber_user_id, NEW.resource_id, topic_permission(NEW.topic)) + ) + ) + OR (ns.subscriber_project_id IS NOT NULL AND ns.subscriber_project_id = NEW.project_id) + ) + ) + LOOP + -- Log that this event triggered this subscription. + INSERT INTO notification_providence_log (event_id, subscription_id) + VALUES (the_event_id, the_subscription_id); + + -- Add to the relevant queues. + -- Each delivery method _may_ be triggered by multiple subscriptions, + -- we need ON CONFLICT DO NOTHING. + INSERT INTO notification_webhook_queue (event_id, webhook_id) + SELECT the_event_id, nw.id + FROM notification_webhooks nw + WHERE nw.subscription_id = the_subscription_id + ON CONFLICT DO NOTHING; + + INSERT INTO notification_email_queue (event_id, email_id) + SELECT the_event_id AS event_id, ne.id + FROM notification_emails ne + WHERE ne.subscription_id = the_subscription_id + ON CONFLICT DO NOTHING; + + -- Also add the notification to the hub. + -- It's possible it was already added by another subscription for this user, + -- in which case we just carry on. + INSERT INTO notification_hub_entries (event_id, user_id) + VALUES (the_event_id, the_subscriber) + ON CONFLICT DO NOTHING; + END LOOP; + + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + + +-- Now we can drop the old tables +DROP TABLE notification_by_webhook; +DROP TABLE notification_by_email; From 34064edb09e00c337882dcd236205fa1d6898456 Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Wed, 3 Sep 2025 10:10:29 -0700 Subject: [PATCH 09/35] Add delete project webhook --- src/Share/Notifications/Ops.hs | 9 +++++++++ src/Share/Notifications/Queries.hs | 10 ++++++++++ src/Share/Web/Share/Projects/Impl.hs | 6 ++---- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/Share/Notifications/Ops.hs b/src/Share/Notifications/Ops.hs index 4cff9d66..66e4a816 100644 --- a/src/Share/Notifications/Ops.hs +++ b/src/Share/Notifications/Ops.hs @@ -5,6 +5,7 @@ module Share.Notifications.Ops deleteWebhookDeliveryMethod, listProjectWebhooks, createProjectWebhook, + deleteProjectWebhook, hydrateEvent, ) where @@ -135,3 +136,11 @@ createProjectWebhook subscriberUserId projectId url topics = do createdAt = Nothing, updatedAt = Nothing } + +deleteProjectWebhook :: UserId -> NotificationSubscriptionId -> WebApp () +deleteProjectWebhook caller subscriptionId = do + -- First fetch the webhook id associated with this subscription + webhooks <- PG.runTransaction $ do NotifQ.webhooksForSubscription subscriptionId + for_ webhooks \webhookId -> do + deleteWebhookDeliveryMethod caller webhookId + PG.runTransaction $ do NotifQ.deleteNotificationSubscription caller subscriptionId diff --git a/src/Share/Notifications/Queries.hs b/src/Share/Notifications/Queries.hs index dadcf53a..d2b17c79 100644 --- a/src/Share/Notifications/Queries.hs +++ b/src/Share/Notifications/Queries.hs @@ -24,6 +24,7 @@ module Share.Notifications.Queries updateWatchProjectSubscription, isUserSubscribedToWatchProject, listProjectWebhooks, + webhooksForSubscription, ) where @@ -580,3 +581,12 @@ listProjectWebhooks caller projectId = do AND ns.subscriber_user_id = #{caller} |] <&> fmap (\((webhookId, webhookName) PG.:. subscription) -> (webhookId, webhookName, subscription)) + +webhooksForSubscription :: NotificationSubscriptionId -> Transaction e [NotificationWebhookId] +webhooksForSubscription subscriptionId = do + PG.queryListCol + [PG.sql| + SELECT nbw.webhook_id + FROM notification_by_webhook nbw + WHERE nbw.subscription_id = #{subscriptionId} + |] diff --git a/src/Share/Web/Share/Projects/Impl.hs b/src/Share/Web/Share/Projects/Impl.hs index 8d2c7d3e..875a04c9 100644 --- a/src/Share/Web/Share/Projects/Impl.hs +++ b/src/Share/Web/Share/Projects/Impl.hs @@ -481,12 +481,10 @@ updateProjectWebhookEndpoint session projectUserHandle projectSlug subscriptionI deleteProjectWebhookEndpoint :: Maybe Session -> UserHandle -> ProjectSlug -> IDs.NotificationSubscriptionId -> WebApp () deleteProjectWebhookEndpoint session projectUserHandle projectSlug subscriptionId = do caller <- AuthN.requireAuthenticatedUser session - Project {projectId, ownerUserId = projectOwner} <- PG.runTransactionOrRespondError $ do + Project {ownerUserId = projectOwner} <- PG.runTransactionOrRespondError $ do Q.projectByShortHand projectShortHand `whenNothingM` throwError (EntityMissing (ErrorID "project-not-found") ("Project not found: " <> IDs.toText @IDs.ProjectShortHand projectShortHand)) _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkSubscriptionsManage caller projectOwner - success <- PG.runTransaction $ NotifsQ.deleteProjectWebhook caller projectId subscriptionId - unless success $ respondError (EntityMissing (ErrorID "webhook:not-found") "Webhook not found") - pure () + NotifOps.deleteProjectWebhook caller subscriptionId where projectShortHand :: IDs.ProjectShortHand projectShortHand = IDs.ProjectShortHand {userHandle = projectUserHandle, projectSlug} From 7e89571b03c0b8e98c708c4bfa72ffaec068cc68 Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Wed, 3 Sep 2025 11:18:48 -0700 Subject: [PATCH 10/35] Remove Delivery Method APIs --- src/Share/Notifications/API.hs | 63 ++------------------------------- src/Share/Notifications/Impl.hs | 38 +------------------- 2 files changed, 3 insertions(+), 98 deletions(-) diff --git a/src/Share/Notifications/API.hs b/src/Share/Notifications/API.hs index 55baa842..92c976f3 100644 --- a/src/Share/Notifications/API.hs +++ b/src/Share/Notifications/API.hs @@ -16,10 +16,6 @@ module Share.Notifications.API CreateSubscriptionRequest (..), CreateSubscriptionResponse (..), UpdateSubscriptionRequest (..), - SubscriptionDeliveryMethodRoutes (..), - SubscriptionResourceRoutes (..), - AddSubscriptionDeliveryMethodsRequest (..), - RemoveSubscriptionDeliveryMethodsRequest (..), GetDeliveryMethodsResponse (..), CreateEmailDeliveryMethodRequest (..), CreateEmailDeliveryMethodResponse (..), @@ -39,7 +35,7 @@ import Data.Text qualified as Text import Data.Time (UTCTime) import Servant import Share.IDs -import Share.Notifications.Types (DeliveryMethodId, HydratedEvent, NotificationDeliveryMethod, NotificationHubEntry, NotificationStatus, NotificationSubscription, NotificationTopic, NotificationTopicGroup, SubscriptionFilter) +import Share.Notifications.Types (HydratedEvent, NotificationDeliveryMethod, NotificationHubEntry, NotificationStatus, NotificationSubscription, NotificationTopic, NotificationTopicGroup, SubscriptionFilter) import Share.OAuth.Session (AuthenticatedUserId) import Share.Prelude import Share.Utils.API (Cursor, Paged) @@ -76,22 +72,7 @@ data SubscriptionRoutes mode { getSubscriptionsEndpoint :: mode :- GetSubscriptionsEndpoint, createSubscriptionEndpoint :: mode :- CreateSubscriptionEndpoint, deleteSubscriptionEndpoint :: mode :- DeleteSubscriptionEndpoint, - updateSubscriptionEndpoint :: mode :- UpdateSubscriptionEndpoint, - subscriptionResourceRoutes :: mode :- Capture "subscriptionId" NotificationSubscriptionId :> NamedRoutes SubscriptionResourceRoutes - } - deriving stock (Generic) - -data SubscriptionResourceRoutes mode - = SubscriptionResourceRoutes - { subscriptionDeliveryMethodRoutes :: mode :- "delivery-methods" :> NamedRoutes SubscriptionDeliveryMethodRoutes - } - deriving stock (Generic) - -data SubscriptionDeliveryMethodRoutes mode - = SubscriptionDeliveryMethodRoutes - { getSubscriptionDeliveryMethodsEndpoint :: mode :- GetDeliveryMethodsEndpoint, - addSubscriptionDeliveryMethodsEndpoint :: mode :- "add" :> AddSubscriptionDeliveryMethodsEndpoint, - removeSubscriptionDeliveryMethodsEndpoint :: mode :- "remove" :> RemoveSubscriptionDeliveryMethodsEndpoint + updateSubscriptionEndpoint :: mode :- UpdateSubscriptionEndpoint } deriving stock (Generic) @@ -406,43 +387,3 @@ instance FromJSON UpdateWebhookRequest where parseJSON = withObject "UpdateWebhookRequest" $ \o -> do url <- o .: "url" pure UpdateWebhookRequest {url} - -type AddSubscriptionDeliveryMethodsEndpoint = - AuthenticatedUserId - :> ReqBody '[JSON] AddSubscriptionDeliveryMethodsRequest - :> Post '[JSON] () - -type RemoveSubscriptionDeliveryMethodsEndpoint = - AuthenticatedUserId - :> ReqBody '[JSON] RemoveSubscriptionDeliveryMethodsRequest - :> Delete '[JSON] () - -data AddSubscriptionDeliveryMethodsRequest - = AddSubscriptionDeliveryMethodsRequest - { deliveryMethods :: NESet DeliveryMethodId - } - deriving stock (Show, Eq, Ord) - -instance ToJSON AddSubscriptionDeliveryMethodsRequest where - toJSON AddSubscriptionDeliveryMethodsRequest {deliveryMethods} = - object ["deliveryMethods" .= deliveryMethods] - -instance FromJSON AddSubscriptionDeliveryMethodsRequest where - parseJSON = withObject "AddSubscriptionDeliveryMethodsRequest" $ \o -> do - deliveryMethods <- o .: "deliveryMethods" - pure AddSubscriptionDeliveryMethodsRequest {deliveryMethods} - -data RemoveSubscriptionDeliveryMethodsRequest - = RemoveSubscriptionDeliveryMethodsRequest - { deliveryMethods :: NESet DeliveryMethodId - } - deriving stock (Show, Eq, Ord) - -instance ToJSON RemoveSubscriptionDeliveryMethodsRequest where - toJSON RemoveSubscriptionDeliveryMethodsRequest {deliveryMethods} = - object ["deliveryMethods" .= deliveryMethods] - -instance FromJSON RemoveSubscriptionDeliveryMethodsRequest where - parseJSON = withObject "RemoveSubscriptionDeliveryMethodsRequest" $ \o -> do - deliveryMethods <- o .: "deliveryMethods" - pure RemoveSubscriptionDeliveryMethodsRequest {deliveryMethods} diff --git a/src/Share/Notifications/Impl.hs b/src/Share/Notifications/Impl.hs index 818cfc90..baabdaef 100644 --- a/src/Share/Notifications/Impl.hs +++ b/src/Share/Notifications/Impl.hs @@ -38,22 +38,7 @@ subscriptionsRoutes userHandle = { getSubscriptionsEndpoint = getSubscriptionsEndpoint userHandle, createSubscriptionEndpoint = createSubscriptionEndpoint userHandle, deleteSubscriptionEndpoint = deleteSubscriptionEndpoint userHandle, - updateSubscriptionEndpoint = updateSubscriptionEndpoint userHandle, - subscriptionResourceRoutes = subscriptionResourceRoutes userHandle - } - -subscriptionResourceRoutes :: UserHandle -> NotificationSubscriptionId -> API.SubscriptionResourceRoutes (AsServerT WebApp) -subscriptionResourceRoutes handle subscriptionId = - API.SubscriptionResourceRoutes - { subscriptionDeliveryMethodRoutes = subscriptionDeliveryMethodRoutes handle subscriptionId - } - -subscriptionDeliveryMethodRoutes :: UserHandle -> NotificationSubscriptionId -> API.SubscriptionDeliveryMethodRoutes (AsServerT WebApp) -subscriptionDeliveryMethodRoutes handle subscriptionId = - API.SubscriptionDeliveryMethodRoutes - { getSubscriptionDeliveryMethodsEndpoint = getSubscriptionDeliveryMethodsEndpoint handle subscriptionId, - addSubscriptionDeliveryMethodsEndpoint = addSubscriptionDeliveryMethodsEndpoint handle subscriptionId, - removeSubscriptionDeliveryMethodsEndpoint = removeSubscriptionDeliveryMethodsEndpoint handle subscriptionId + updateSubscriptionEndpoint = updateSubscriptionEndpoint userHandle } emailDeliveryRoutes :: UserHandle -> API.EmailRoutes (AsServerT WebApp) @@ -130,27 +115,6 @@ updateEmailDeliveryMethodEndpoint userHandle callerUserId emailDeliveryMethodId PG.runTransaction $ NotificationQ.updateEmailDeliveryMethod notificationUserId emailDeliveryMethodId email pure () -getSubscriptionDeliveryMethodsEndpoint :: UserHandle -> NotificationSubscriptionId -> UserId -> WebApp API.GetDeliveryMethodsResponse -getSubscriptionDeliveryMethodsEndpoint userHandle subscriptionId callerUserId = do - User {user_id = notificationUserId} <- UserQ.expectUserByHandle userHandle - _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkDeliveryMethodsView callerUserId notificationUserId - deliveryMethods <- NotifOps.listNotificationDeliveryMethods notificationUserId (Just subscriptionId) - pure $ API.GetDeliveryMethodsResponse {deliveryMethods} - -addSubscriptionDeliveryMethodsEndpoint :: UserHandle -> NotificationSubscriptionId -> UserId -> API.AddSubscriptionDeliveryMethodsRequest -> WebApp () -addSubscriptionDeliveryMethodsEndpoint userHandle subscriptionId callerUserId API.AddSubscriptionDeliveryMethodsRequest {deliveryMethods} = do - User {user_id = notificationUserId} <- UserQ.expectUserByHandle userHandle - _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkDeliveryMethodsManage callerUserId notificationUserId - PG.runTransaction $ NotificationQ.addSubscriptionDeliveryMethods notificationUserId subscriptionId deliveryMethods - pure () - -removeSubscriptionDeliveryMethodsEndpoint :: UserHandle -> NotificationSubscriptionId -> UserId -> API.RemoveSubscriptionDeliveryMethodsRequest -> WebApp () -removeSubscriptionDeliveryMethodsEndpoint userHandle subscriptionId callerUserId API.RemoveSubscriptionDeliveryMethodsRequest {deliveryMethods} = do - User {user_id = notificationUserId} <- UserQ.expectUserByHandle userHandle - _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkDeliveryMethodsManage callerUserId notificationUserId - PG.runTransaction $ NotificationQ.removeSubscriptionDeliveryMethods notificationUserId subscriptionId deliveryMethods - pure () - createWebhookEndpoint :: UserHandle -> UserId -> API.CreateWebhookRequest -> WebApp API.CreateWebhookResponse createWebhookEndpoint userHandle callerUserId API.CreateWebhookRequest {url, name = webhookName} = do User {user_id = notificationUserId} <- UserQ.expectUserByHandle userHandle From d278d34ee5e6f990352ef73c5f6cb8e38ae3c9ee Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Wed, 3 Sep 2025 11:30:45 -0700 Subject: [PATCH 11/35] Delete a bunch of delivery method endpoints --- src/Share/Notifications/API.hs | 174 +---------------------------- src/Share/Notifications/Impl.hs | 74 ------------ src/Share/Notifications/Queries.hs | 74 +----------- 3 files changed, 5 insertions(+), 317 deletions(-) diff --git a/src/Share/Notifications/API.hs b/src/Share/Notifications/API.hs index 92c976f3..ebd4aed5 100644 --- a/src/Share/Notifications/API.hs +++ b/src/Share/Notifications/API.hs @@ -5,10 +5,7 @@ module Share.Notifications.API ( API, Routes (..), HubEntriesRoutes (..), - DeliveryMethodRoutes (..), SubscriptionRoutes (..), - EmailRoutes (..), - WebhookRoutes (..), GetHubEntriesCursor, StatusFilter (..), UpdateHubEntriesRequest (..), @@ -16,13 +13,6 @@ module Share.Notifications.API CreateSubscriptionRequest (..), CreateSubscriptionResponse (..), UpdateSubscriptionRequest (..), - GetDeliveryMethodsResponse (..), - CreateEmailDeliveryMethodRequest (..), - CreateEmailDeliveryMethodResponse (..), - UpdateEmailDeliveryMethodRequest (..), - CreateWebhookRequest (..), - CreateWebhookResponse (..), - UpdateWebhookRequest (..), ) where @@ -35,11 +25,10 @@ import Data.Text qualified as Text import Data.Time (UTCTime) import Servant import Share.IDs -import Share.Notifications.Types (HydratedEvent, NotificationDeliveryMethod, NotificationHubEntry, NotificationStatus, NotificationSubscription, NotificationTopic, NotificationTopicGroup, SubscriptionFilter) +import Share.Notifications.Types (HydratedEvent, NotificationHubEntry, NotificationStatus, NotificationSubscription, NotificationTopic, NotificationTopicGroup, SubscriptionFilter) import Share.OAuth.Session (AuthenticatedUserId) import Share.Prelude import Share.Utils.API (Cursor, Paged) -import Share.Utils.URI (URIParam) import Share.Web.Share.DisplayInfo.Types (UnifiedDisplayInfo) type API = NamedRoutes Routes @@ -47,7 +36,6 @@ type API = NamedRoutes Routes data Routes mode = Routes { hubRoutes :: mode :- "hub" :> NamedRoutes HubEntriesRoutes, - deliveryMethodRoutes :: mode :- "delivery-methods" :> NamedRoutes DeliveryMethodRoutes, subscriptionsRoutes :: mode :- "subscriptions" :> NamedRoutes SubscriptionRoutes } deriving stock (Generic) @@ -59,14 +47,6 @@ data HubEntriesRoutes mode } deriving stock (Generic) -data DeliveryMethodRoutes mode - = DeliveryMethodRoutes - { getDeliveryMethodsEndpoint :: mode :- GetDeliveryMethodsEndpoint, - emailDeliveryRoutes :: mode :- "emails" :> NamedRoutes EmailRoutes, - webhookDeliveryRoutes :: mode :- "webhooks" :> NamedRoutes WebhookRoutes - } - deriving stock (Generic) - data SubscriptionRoutes mode = SubscriptionRoutes { getSubscriptionsEndpoint :: mode :- GetSubscriptionsEndpoint, @@ -76,22 +56,6 @@ data SubscriptionRoutes mode } deriving stock (Generic) -data EmailRoutes mode - = EmailRoutes - { createEmailDeliveryMethodEndpoint :: mode :- CreateEmailDeliveryMethodEndpoint, - deleteEmailDeliveryMethodEndpoint :: mode :- DeleteEmailDeliveryMethodEndpoint, - updateEmailDeliveryMethodEndpoint :: mode :- UpdateEmailDeliveryMethodEndpoint - } - deriving stock (Generic) - -data WebhookRoutes mode - = WebhookRoutes - { createWebhookEndpoint :: mode :- CreateWebhookEndpoint, - deleteWebhookEndpoint :: mode :- DeleteWebhookEndpoint, - updateWebhookEndpoint :: mode :- UpdateWebhookEndpoint - } - deriving stock (Generic) - type GetSubscriptionsEndpoint = AuthenticatedUserId :> Get '[JSON] GetSubscriptionsResponse @@ -190,24 +154,6 @@ instance FromJSON UpdateSubscriptionRequest where subscriptionFilter <- o .:? "filter" pure UpdateSubscriptionRequest {subscriptionTopics, subscriptionTopicGroups, subscriptionFilter} -type GetDeliveryMethodsEndpoint = - AuthenticatedUserId - :> Get '[JSON] GetDeliveryMethodsResponse - -data GetDeliveryMethodsResponse - = GetDeliveryMethodsResponse - { deliveryMethods :: [NotificationDeliveryMethod] - } - -instance ToJSON GetDeliveryMethodsResponse where - toJSON GetDeliveryMethodsResponse {deliveryMethods} = - object ["deliveryMethods" .= deliveryMethods] - -instance FromJSON GetDeliveryMethodsResponse where - parseJSON = withObject "GetDeliveryMethodsResponse" $ \o -> do - deliveryMethods <- o .: "deliveryMethods" - pure GetDeliveryMethodsResponse {deliveryMethods} - newtype StatusFilter = StatusFilter { getStatusFilter :: NESet NotificationStatus } @@ -269,121 +215,3 @@ instance FromJSON UpdateHubEntriesRequest where notificationStatus <- o .: "status" notificationIds <- o .: "notificationIds" pure UpdateHubEntriesRequest {notificationStatus, notificationIds} - -type CreateEmailDeliveryMethodEndpoint = - AuthenticatedUserId - :> ReqBody '[JSON] CreateEmailDeliveryMethodRequest - :> Post '[JSON] CreateEmailDeliveryMethodResponse - -data CreateEmailDeliveryMethodRequest - = CreateEmailDeliveryMethodRequest - { email :: Email - } - -instance ToJSON CreateEmailDeliveryMethodRequest where - toJSON CreateEmailDeliveryMethodRequest {email} = - object ["email" .= email] - -instance FromJSON CreateEmailDeliveryMethodRequest where - parseJSON = withObject "CreateEmailDeliveryMethodRequest" $ \o -> do - email <- o .: "email" - pure CreateEmailDeliveryMethodRequest {email} - -data CreateEmailDeliveryMethodResponse - = CreateEmailDeliveryMethodResponse - { emailDeliveryMethodId :: NotificationEmailDeliveryMethodId - } - -instance ToJSON CreateEmailDeliveryMethodResponse where - toJSON CreateEmailDeliveryMethodResponse {emailDeliveryMethodId} = - object ["emailDeliveryMethodId" .= emailDeliveryMethodId] - -instance FromJSON CreateEmailDeliveryMethodResponse where - parseJSON = withObject "CreateEmailDeliveryMethodResponse" $ \o -> do - emailDeliveryMethodId <- o .: "emailDeliveryMethodId" - pure CreateEmailDeliveryMethodResponse {emailDeliveryMethodId} - -type DeleteEmailDeliveryMethodEndpoint = - AuthenticatedUserId - :> Capture "emailDeliveryMethodId" NotificationEmailDeliveryMethodId - :> Delete '[JSON] () - -type UpdateEmailDeliveryMethodEndpoint = - AuthenticatedUserId - :> Capture "emailDeliveryMethodId" NotificationEmailDeliveryMethodId - :> ReqBody '[JSON] UpdateEmailDeliveryMethodRequest - :> Patch '[JSON] () - -data UpdateEmailDeliveryMethodRequest - = UpdateEmailDeliveryMethodRequest - { email :: Email - } - -instance ToJSON UpdateEmailDeliveryMethodRequest where - toJSON UpdateEmailDeliveryMethodRequest {email} = - object ["email" .= email] - -instance FromJSON UpdateEmailDeliveryMethodRequest where - parseJSON = withObject "UpdateEmailDeliveryMethodRequest" $ \o -> do - email <- o .: "email" - pure UpdateEmailDeliveryMethodRequest {email} - -type CreateWebhookEndpoint = - AuthenticatedUserId - :> ReqBody '[JSON] CreateWebhookRequest - :> Post '[JSON] CreateWebhookResponse - -data CreateWebhookRequest - = CreateWebhookRequest - { url :: URIParam, - name :: Text - } - -instance ToJSON CreateWebhookRequest where - toJSON CreateWebhookRequest {url, name} = - object ["url" .= url, "name" .= name] - -instance FromJSON CreateWebhookRequest where - parseJSON = withObject "CreateWebhookRequest" $ \o -> do - url <- o .: "url" - name <- o .: "name" - pure CreateWebhookRequest {url, name} - -data CreateWebhookResponse - = CreateWebhookResponse - { webhookId :: NotificationWebhookId - } - -instance ToJSON CreateWebhookResponse where - toJSON CreateWebhookResponse {webhookId} = - object ["webhookId" .= webhookId] - -instance FromJSON CreateWebhookResponse where - parseJSON = withObject "CreateWebhookResponse" $ \o -> do - webhookId <- o .: "webhookId" - pure CreateWebhookResponse {webhookId} - -type DeleteWebhookEndpoint = - AuthenticatedUserId - :> Capture "webhookId" NotificationWebhookId - :> Delete '[JSON] () - -type UpdateWebhookEndpoint = - AuthenticatedUserId - :> Capture "webhookId" NotificationWebhookId - :> ReqBody '[JSON] UpdateWebhookRequest - :> Patch '[JSON] () - -data UpdateWebhookRequest - = UpdateWebhookRequest - { url :: URIParam - } - -instance ToJSON UpdateWebhookRequest where - toJSON UpdateWebhookRequest {url} = - object ["url" .= url] - -instance FromJSON UpdateWebhookRequest where - parseJSON = withObject "UpdateWebhookRequest" $ \o -> do - url <- o .: "url" - pure UpdateWebhookRequest {url} diff --git a/src/Share/Notifications/Impl.hs b/src/Share/Notifications/Impl.hs index baabdaef..75cc8500 100644 --- a/src/Share/Notifications/Impl.hs +++ b/src/Share/Notifications/Impl.hs @@ -24,14 +24,6 @@ hubRoutes userHandle = updateHubEntriesEndpoint = updateHubEntriesEndpoint userHandle } -deliveryMethodRoutes :: UserHandle -> API.DeliveryMethodRoutes (AsServerT WebApp) -deliveryMethodRoutes userHandle = - API.DeliveryMethodRoutes - { getDeliveryMethodsEndpoint = getDeliveryMethodsEndpoint userHandle, - emailDeliveryRoutes = emailDeliveryRoutes userHandle, - webhookDeliveryRoutes = webhookDeliveryRoutes userHandle - } - subscriptionsRoutes :: UserHandle -> API.SubscriptionRoutes (AsServerT WebApp) subscriptionsRoutes userHandle = API.SubscriptionRoutes @@ -41,27 +33,10 @@ subscriptionsRoutes userHandle = updateSubscriptionEndpoint = updateSubscriptionEndpoint userHandle } -emailDeliveryRoutes :: UserHandle -> API.EmailRoutes (AsServerT WebApp) -emailDeliveryRoutes userHandle = - API.EmailRoutes - { createEmailDeliveryMethodEndpoint = createEmailDeliveryMethodEndpoint userHandle, - deleteEmailDeliveryMethodEndpoint = deleteEmailDeliveryMethodEndpoint userHandle, - updateEmailDeliveryMethodEndpoint = updateEmailDeliveryMethodEndpoint userHandle - } - -webhookDeliveryRoutes :: UserHandle -> API.WebhookRoutes (AsServerT WebApp) -webhookDeliveryRoutes userHandle = - API.WebhookRoutes - { createWebhookEndpoint = createWebhookEndpoint userHandle, - deleteWebhookEndpoint = deleteWebhookEndpoint userHandle, - updateWebhookEndpoint = updateWebhookEndpoint userHandle - } - server :: UserHandle -> ServerT API.API WebApp server userHandle = API.Routes { hubRoutes = hubRoutes userHandle, - deliveryMethodRoutes = deliveryMethodRoutes userHandle, subscriptionsRoutes = subscriptionsRoutes userHandle } @@ -87,55 +62,6 @@ updateHubEntriesEndpoint userHandle callerUserId API.UpdateHubEntriesRequest {no PG.runTransaction $ NotificationQ.updateNotificationHubEntries notificationIds notificationStatus pure () -getDeliveryMethodsEndpoint :: UserHandle -> UserId -> WebApp API.GetDeliveryMethodsResponse -getDeliveryMethodsEndpoint userHandle callerUserId = do - User {user_id = notificationUserId} <- UserQ.expectUserByHandle userHandle - _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkDeliveryMethodsView callerUserId notificationUserId - deliveryMethods <- NotifOps.listNotificationDeliveryMethods notificationUserId Nothing - pure $ API.GetDeliveryMethodsResponse {deliveryMethods} - -createEmailDeliveryMethodEndpoint :: UserHandle -> UserId -> API.CreateEmailDeliveryMethodRequest -> WebApp API.CreateEmailDeliveryMethodResponse -createEmailDeliveryMethodEndpoint userHandle callerUserId API.CreateEmailDeliveryMethodRequest {email} = do - User {user_id = notificationUserId} <- UserQ.expectUserByHandle userHandle - _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkDeliveryMethodsManage callerUserId notificationUserId - emailDeliveryMethodId <- PG.runTransaction $ NotificationQ.createEmailDeliveryMethod notificationUserId email - pure $ API.CreateEmailDeliveryMethodResponse {emailDeliveryMethodId} - -deleteEmailDeliveryMethodEndpoint :: UserHandle -> UserId -> NotificationEmailDeliveryMethodId -> WebApp () -deleteEmailDeliveryMethodEndpoint userHandle callerUserId emailDeliveryMethodId = do - User {user_id = notificationUserId} <- UserQ.expectUserByHandle userHandle - _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkDeliveryMethodsManage callerUserId notificationUserId - PG.runTransaction $ NotificationQ.deleteEmailDeliveryMethod notificationUserId emailDeliveryMethodId - pure () - -updateEmailDeliveryMethodEndpoint :: UserHandle -> UserId -> NotificationEmailDeliveryMethodId -> API.UpdateEmailDeliveryMethodRequest -> WebApp () -updateEmailDeliveryMethodEndpoint userHandle callerUserId emailDeliveryMethodId API.UpdateEmailDeliveryMethodRequest {email} = do - User {user_id = notificationUserId} <- UserQ.expectUserByHandle userHandle - _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkDeliveryMethodsManage callerUserId notificationUserId - PG.runTransaction $ NotificationQ.updateEmailDeliveryMethod notificationUserId emailDeliveryMethodId email - pure () - -createWebhookEndpoint :: UserHandle -> UserId -> API.CreateWebhookRequest -> WebApp API.CreateWebhookResponse -createWebhookEndpoint userHandle callerUserId API.CreateWebhookRequest {url, name = webhookName} = do - User {user_id = notificationUserId} <- UserQ.expectUserByHandle userHandle - _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkDeliveryMethodsManage callerUserId notificationUserId - webhookId <- NotifOps.createWebhookDeliveryMethod notificationUserId url webhookName - pure $ API.CreateWebhookResponse {webhookId} - -deleteWebhookEndpoint :: UserHandle -> UserId -> NotificationWebhookId -> WebApp () -deleteWebhookEndpoint userHandle callerUserId webhookDeliveryMethodId = do - User {user_id = notificationUserId} <- UserQ.expectUserByHandle userHandle - _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkDeliveryMethodsManage callerUserId notificationUserId - NotifOps.deleteWebhookDeliveryMethod notificationUserId webhookDeliveryMethodId - pure () - -updateWebhookEndpoint :: UserHandle -> UserId -> NotificationWebhookId -> API.UpdateWebhookRequest -> WebApp () -updateWebhookEndpoint userHandle callerUserId webhookDeliveryMethodId API.UpdateWebhookRequest {url} = do - User {user_id = notificationUserId} <- UserQ.expectUserByHandle userHandle - _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkDeliveryMethodsManage callerUserId notificationUserId - NotifOps.updateWebhookDeliveryMethod notificationUserId webhookDeliveryMethodId url - pure () - getSubscriptionsEndpoint :: UserHandle -> UserId -> WebApp API.GetSubscriptionsResponse getSubscriptionsEndpoint userHandle callerUserId = do User {user_id = notificationUserId} <- UserQ.expectUserByHandle userHandle diff --git a/src/Share/Notifications/Queries.hs b/src/Share/Notifications/Queries.hs index d2b17c79..58d41070 100644 --- a/src/Share/Notifications/Queries.hs +++ b/src/Share/Notifications/Queries.hs @@ -5,8 +5,6 @@ module Share.Notifications.Queries expectEvent, listNotificationHubEntryPayloads, updateNotificationHubEntries, - addSubscriptionDeliveryMethods, - removeSubscriptionDeliveryMethods, listEmailDeliveryMethods, listWebhooks, createEmailDeliveryMethod, @@ -133,70 +131,6 @@ updateNotificationHubEntries hubEntryIds status = do WHERE id IN (SELECT notification_id FROM to_update) |] --- | Note: If a given delivery method belongs to a different subscriber user, it will simply be ignored, --- this should never happen in non-malicious workflows so it's fine to ignore it. -addSubscriptionDeliveryMethods :: UserId -> NotificationSubscriptionId -> NESet DeliveryMethodId -> Transaction e () -addSubscriptionDeliveryMethods subscriberUserId subscriptionId deliveryMethods = do - let (emailIds, webhookIds) = - deliveryMethods & foldMap \case - EmailDeliveryMethodId emailId -> ([emailId], mempty) - WebhookDeliveryMethodId webhookId -> (mempty, [webhookId]) - execute_ - [sql| - WITH email_ids(email_id) AS ( - SELECT * FROM ^{singleColumnTable emailIds} - ) - INSERT INTO notification_by_email (subscription_id, email_id) - SELECT #{subscriptionId}, ei.email_id - FROM email_ids ei - JOIN notification_emails ne ON ei.email_id = ne.id - WHERE ne.subscriber_user_id = #{subscriberUserId} - ON CONFLICT DO NOTHING - |] - execute_ - [sql| - WITH webhook_ids(webhook_id) AS ( - SELECT * FROM ^{singleColumnTable webhookIds} - ) - INSERT INTO notification_by_webhook (subscription_id, webhook_id) - SELECT #{subscriptionId}, wi.webhook_id - FROM webhook_ids wi - JOIN notification_webhooks nw ON wi.webhook_id = nw.id - WHERE nw.subscriber_user_id = #{subscriberUserId} - ON CONFLICT DO NOTHING - |] - -removeSubscriptionDeliveryMethods :: UserId -> NotificationSubscriptionId -> NESet DeliveryMethodId -> Transaction e () -removeSubscriptionDeliveryMethods subscriberUserId subscriptionId deliveryMethods = do - let (emailIds, webhookIds) = - deliveryMethods & foldMap \case - EmailDeliveryMethodId emailId -> ([emailId], mempty) - WebhookDeliveryMethodId webhookId -> (mempty, [webhookId]) - execute_ - [sql| - DELETE FROM notification_by_email - WHERE subscription_id = #{subscriptionId} - AND email_id IN (SELECT * FROM ^{singleColumnTable emailIds}) - AND EXISTS ( - SELECT 1 - FROM notification_emails ne - WHERE ne.id = email_id - AND ne.subscriber_user_id = #{subscriberUserId} - ) - |] - execute_ - [sql| - DELETE FROM notification_by_webhook - WHERE subscription_id = #{subscriptionId} - AND webhook_id IN (SELECT * FROM ^{singleColumnTable webhookIds}) - AND EXISTS ( - SELECT 1 - FROM notification_webhooks nw - WHERE nw.id = webhook_id - AND nw.subscriber_user_id = #{subscriberUserId} - ) - |] - listEmailDeliveryMethods :: (QueryA m) => UserId -> Maybe NotificationSubscriptionId -> m [NotificationEmailDeliveryConfig] listEmailDeliveryMethods userId maySubscriptionId = do queryListRows @@ -270,12 +204,12 @@ deleteEmailDeliveryMethod notificationUserId emailDeliveryMethodId = do AND subscriber_user_id = #{notificationUserId} |] -createWebhookDeliveryMethod :: UserId -> Text -> Transaction e NotificationWebhookId -createWebhookDeliveryMethod userId name = do +createWebhookDeliveryMethod :: UserId -> Text -> NotificationSubscriptionId -> Transaction e NotificationWebhookId +createWebhookDeliveryMethod userId name subscriptionId = do queryExpect1Col [sql| - INSERT INTO notification_webhooks (subscriber_user_id, name) - VALUES (#{userId}, #{name}) + INSERT INTO notification_webhooks (subscriber_user_id, name, subscription_id) + VALUES (#{userId}, #{name}, #{subscriptionId}) RETURNING id |] From c62edd9f648333dd21bd737d55a2a9c5bf5fd00c Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Wed, 3 Sep 2025 11:30:45 -0700 Subject: [PATCH 12/35] Remove subscription routes --- src/Share/Notifications/API.hs | 117 +------------------------------- src/Share/Notifications/Impl.hs | 49 +------------ 2 files changed, 3 insertions(+), 163 deletions(-) diff --git a/src/Share/Notifications/API.hs b/src/Share/Notifications/API.hs index ebd4aed5..29c395d6 100644 --- a/src/Share/Notifications/API.hs +++ b/src/Share/Notifications/API.hs @@ -5,14 +5,9 @@ module Share.Notifications.API ( API, Routes (..), HubEntriesRoutes (..), - SubscriptionRoutes (..), GetHubEntriesCursor, StatusFilter (..), UpdateHubEntriesRequest (..), - GetSubscriptionsResponse (..), - CreateSubscriptionRequest (..), - CreateSubscriptionResponse (..), - UpdateSubscriptionRequest (..), ) where @@ -25,7 +20,7 @@ import Data.Text qualified as Text import Data.Time (UTCTime) import Servant import Share.IDs -import Share.Notifications.Types (HydratedEvent, NotificationHubEntry, NotificationStatus, NotificationSubscription, NotificationTopic, NotificationTopicGroup, SubscriptionFilter) +import Share.Notifications.Types (HydratedEvent, NotificationHubEntry, NotificationStatus) import Share.OAuth.Session (AuthenticatedUserId) import Share.Prelude import Share.Utils.API (Cursor, Paged) @@ -35,8 +30,7 @@ type API = NamedRoutes Routes data Routes mode = Routes - { hubRoutes :: mode :- "hub" :> NamedRoutes HubEntriesRoutes, - subscriptionsRoutes :: mode :- "subscriptions" :> NamedRoutes SubscriptionRoutes + { hubRoutes :: mode :- "hub" :> NamedRoutes HubEntriesRoutes } deriving stock (Generic) @@ -47,113 +41,6 @@ data HubEntriesRoutes mode } deriving stock (Generic) -data SubscriptionRoutes mode - = SubscriptionRoutes - { getSubscriptionsEndpoint :: mode :- GetSubscriptionsEndpoint, - createSubscriptionEndpoint :: mode :- CreateSubscriptionEndpoint, - deleteSubscriptionEndpoint :: mode :- DeleteSubscriptionEndpoint, - updateSubscriptionEndpoint :: mode :- UpdateSubscriptionEndpoint - } - deriving stock (Generic) - -type GetSubscriptionsEndpoint = - AuthenticatedUserId - :> Get '[JSON] GetSubscriptionsResponse - -data GetSubscriptionsResponse - = GetSubscriptionsResponse - { subscriptions :: [NotificationSubscription NotificationSubscriptionId] - } - -instance ToJSON GetSubscriptionsResponse where - toJSON GetSubscriptionsResponse {subscriptions} = - object ["subscriptions" .= subscriptions] - -instance FromJSON GetSubscriptionsResponse where - parseJSON = withObject "GetSubscriptionsResponse" $ \o -> do - subscriptions <- o .: "subscriptions" - pure GetSubscriptionsResponse {subscriptions} - -type CreateSubscriptionEndpoint = - AuthenticatedUserId - :> ReqBody '[JSON] CreateSubscriptionRequest - :> Post '[JSON] CreateSubscriptionResponse - -data CreateSubscriptionRequest - = CreateSubscriptionRequest - { subscriptionScope :: UserHandle, - subscriptionProjectId :: Maybe ProjectId, - subscriptionTopics :: Set NotificationTopic, - subscriptionTopicGroups :: Set NotificationTopicGroup, - subscriptionFilter :: Maybe SubscriptionFilter - } - -instance ToJSON CreateSubscriptionRequest where - toJSON CreateSubscriptionRequest {subscriptionScope, subscriptionProjectId, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter} = - object - [ "scope" .= subscriptionScope, - "projectId" .= subscriptionProjectId, - "topics" .= subscriptionTopics, - "topicGroups" .= subscriptionTopicGroups, - "filter" .= subscriptionFilter - ] - -instance FromJSON CreateSubscriptionRequest where - parseJSON = withObject "CreateSubscriptionRequest" $ \o -> do - subscriptionScope <- o .: "scope" - subscriptionProjectId <- o .:? "projectId" - subscriptionTopics <- o .: "topics" - subscriptionTopicGroups <- o .: "topicGroups" - subscriptionFilter <- o .:? "filter" - pure CreateSubscriptionRequest {subscriptionScope, subscriptionProjectId, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter} - -data CreateSubscriptionResponse - = CreateSubscriptionResponse - { subscription :: NotificationSubscription NotificationSubscriptionId - } - -instance ToJSON CreateSubscriptionResponse where - toJSON CreateSubscriptionResponse {subscription} = - object ["subscription" .= subscription] - -instance FromJSON CreateSubscriptionResponse where - parseJSON = withObject "CreateSubscriptionResponse" $ \o -> do - subscription <- o .: "subscription" - pure CreateSubscriptionResponse {subscription} - -type DeleteSubscriptionEndpoint = - AuthenticatedUserId - :> Capture "subscription_id" NotificationSubscriptionId - :> Delete '[JSON] () - -type UpdateSubscriptionEndpoint = - AuthenticatedUserId - :> Capture "subscription_id" NotificationSubscriptionId - :> ReqBody '[JSON] UpdateSubscriptionRequest - :> Patch '[JSON] CreateSubscriptionResponse - -data UpdateSubscriptionRequest - = UpdateSubscriptionRequest - { subscriptionTopics :: Maybe (Set NotificationTopic), - subscriptionTopicGroups :: Maybe (Set NotificationTopicGroup), - subscriptionFilter :: Maybe SubscriptionFilter - } - -instance ToJSON UpdateSubscriptionRequest where - toJSON UpdateSubscriptionRequest {subscriptionTopics, subscriptionTopicGroups, subscriptionFilter} = - object - [ "topics" .= subscriptionTopics, - "topicGroups" .= subscriptionTopicGroups, - "filter" .= subscriptionFilter - ] - -instance FromJSON UpdateSubscriptionRequest where - parseJSON = withObject "UpdateSubscriptionRequest" $ \o -> do - subscriptionTopics <- o .:? "topics" - subscriptionTopicGroups <- o .:? "topicGroups" - subscriptionFilter <- o .:? "filter" - pure UpdateSubscriptionRequest {subscriptionTopics, subscriptionTopicGroups, subscriptionFilter} - newtype StatusFilter = StatusFilter { getStatusFilter :: NESet NotificationStatus } diff --git a/src/Share/Notifications/Impl.hs b/src/Share/Notifications/Impl.hs index 75cc8500..e8113fac 100644 --- a/src/Share/Notifications/Impl.hs +++ b/src/Share/Notifications/Impl.hs @@ -24,20 +24,10 @@ hubRoutes userHandle = updateHubEntriesEndpoint = updateHubEntriesEndpoint userHandle } -subscriptionsRoutes :: UserHandle -> API.SubscriptionRoutes (AsServerT WebApp) -subscriptionsRoutes userHandle = - API.SubscriptionRoutes - { getSubscriptionsEndpoint = getSubscriptionsEndpoint userHandle, - createSubscriptionEndpoint = createSubscriptionEndpoint userHandle, - deleteSubscriptionEndpoint = deleteSubscriptionEndpoint userHandle, - updateSubscriptionEndpoint = updateSubscriptionEndpoint userHandle - } - server :: UserHandle -> ServerT API.API WebApp server userHandle = API.Routes - { hubRoutes = hubRoutes userHandle, - subscriptionsRoutes = subscriptionsRoutes userHandle + { hubRoutes = hubRoutes userHandle } getHubEntriesEndpoint :: @@ -61,40 +51,3 @@ updateHubEntriesEndpoint userHandle callerUserId API.UpdateHubEntriesRequest {no _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkNotificationsUpdate callerUserId notificationUserId PG.runTransaction $ NotificationQ.updateNotificationHubEntries notificationIds notificationStatus pure () - -getSubscriptionsEndpoint :: UserHandle -> UserId -> WebApp API.GetSubscriptionsResponse -getSubscriptionsEndpoint userHandle callerUserId = do - User {user_id = notificationUserId} <- UserQ.expectUserByHandle userHandle - _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkSubscriptionsView callerUserId notificationUserId - subscriptions <- PG.runTransaction $ NotificationQ.listNotificationSubscriptions notificationUserId - pure $ API.GetSubscriptionsResponse {subscriptions} - -createSubscriptionEndpoint :: UserHandle -> UserId -> API.CreateSubscriptionRequest -> WebApp API.CreateSubscriptionResponse -createSubscriptionEndpoint subscriberHandle callerUserId API.CreateSubscriptionRequest {subscriptionScope, subscriptionProjectId, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter} = do - User {user_id = subscriberUserId} <- UserQ.expectUserByHandle subscriberHandle - User {user_id = scopeUserId} <- UserQ.expectUserByHandle subscriptionScope - _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkSubscriptionsManage callerUserId subscriberUserId - -- NOTE: We allow creating any sort of notification subscription, even for things a user - -- doesn't have access to, but the notification system will only actually create notifications if the caller has access to - -- the resource of a given event for the permission associated to that topic via the - -- 'topic_permission' SQL function. - subscription <- PG.runTransaction $ do - subscriptionId <- NotificationQ.createNotificationSubscription subscriberUserId scopeUserId subscriptionProjectId subscriptionTopics subscriptionTopicGroups subscriptionFilter - NotificationQ.getNotificationSubscription subscriberUserId subscriptionId - pure $ API.CreateSubscriptionResponse {subscription} - -deleteSubscriptionEndpoint :: UserHandle -> UserId -> NotificationSubscriptionId -> WebApp () -deleteSubscriptionEndpoint userHandle callerUserId subscriptionId = do - User {user_id = notificationUserId} <- UserQ.expectUserByHandle userHandle - _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkSubscriptionsManage callerUserId notificationUserId - PG.runTransaction $ NotificationQ.deleteNotificationSubscription notificationUserId subscriptionId - pure () - -updateSubscriptionEndpoint :: UserHandle -> UserId -> NotificationSubscriptionId -> API.UpdateSubscriptionRequest -> WebApp API.CreateSubscriptionResponse -updateSubscriptionEndpoint userHandle callerUserId subscriptionId API.UpdateSubscriptionRequest {subscriptionTopics, subscriptionTopicGroups, subscriptionFilter} = do - User {user_id = notificationUserId} <- UserQ.expectUserByHandle userHandle - _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkSubscriptionsManage callerUserId notificationUserId - subscription <- PG.runTransaction $ do - NotificationQ.updateNotificationSubscription notificationUserId subscriptionId subscriptionTopics subscriptionTopicGroups subscriptionFilter - NotificationQ.getNotificationSubscription notificationUserId subscriptionId - pure $ API.CreateSubscriptionResponse {subscription} From e89f0ed6c220da1bdec41289f0e00d67300c73fc Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Wed, 3 Sep 2025 11:30:45 -0700 Subject: [PATCH 13/35] Implement update project webhook --- src/Share/Notifications/Ops.hs | 75 +++++++++++++++++----------- src/Share/Notifications/Queries.hs | 9 ++-- src/Share/Notifications/Types.hs | 6 +++ src/Share/Web/Share/Projects/Impl.hs | 7 ++- 4 files changed, 64 insertions(+), 33 deletions(-) diff --git a/src/Share/Notifications/Ops.hs b/src/Share/Notifications/Ops.hs index 66e4a816..d061574e 100644 --- a/src/Share/Notifications/Ops.hs +++ b/src/Share/Notifications/Ops.hs @@ -1,17 +1,18 @@ module Share.Notifications.Ops ( listNotificationDeliveryMethods, - createWebhookDeliveryMethod, + addWebhookDeliveryMethod, updateWebhookDeliveryMethod, deleteWebhookDeliveryMethod, listProjectWebhooks, createProjectWebhook, deleteProjectWebhook, + updateProjectWebhook, hydrateEvent, ) where import Control.Lens -import Data.Set.NonEmpty qualified as NESet +import Share.App (AppM) import Share.IDs import Share.Notifications.Queries qualified as NotifQ import Share.Notifications.Types @@ -43,32 +44,22 @@ listNotificationDeliveryMethods userId maySubscriptionId = do pure $ (EmailDeliveryMethod <$> emailDeliveryMethods) <> (WebhookDeliveryMethod <$> webhookDeliveryMethods) -createWebhookDeliveryMethod :: UserId -> URIParam -> Text -> WebApp NotificationWebhookId -createWebhookDeliveryMethod userId uriParam webhookName = do - -- Note that we can't be completely transactional between postgres and vault here. - webhookId <- PG.runTransaction do - NotifQ.createWebhookDeliveryMethod userId webhookName +addWebhookDeliveryMethod :: UserId -> URIParam -> Text -> NotificationSubscriptionId -> (AppM r (Either Webhooks.WebhookSecretError ()) -> IO (Either Webhooks.WebhookSecretError ())) -> PG.Transaction Webhooks.WebhookSecretError NotificationWebhookId +addWebhookDeliveryMethod userId uriParam webhookName notificationSubscriptionId runInIO = do let webhookConfig = WebhookConfig {uri = uriParam} - WebhookSecrets.putWebhookConfig webhookId webhookConfig + -- Note that we can't be completely transactional between postgres and vault here. + webhookId <- NotifQ.createWebhookDeliveryMethod userId webhookName notificationSubscriptionId + -- We run this inside the transaction such that, if it fails, the transaction + -- will be rolled back. + -- + -- In the case where it succeeds, but the transaction fails to commit (which is unlikely) + -- we may have a dangling secret in Vault, which isn't ideal, but is not so bad. + PG.transactionUnsafeIO (runInIO $ WebhookSecrets.putWebhookConfig webhookId webhookConfig) >>= \case + Left err -> do + throwError err + Right _ -> pure () pure webhookId -updateWebhookDeliveryMethod :: UserId -> NotificationWebhookId -> URIParam -> WebApp () -updateWebhookDeliveryMethod notificationUser webhookDeliveryMethodId url = do - isValid <- PG.runTransaction $ do - PG.queryExpect1Col - [PG.sql| - SELECT EXISTS( - SELECT FROM notification_webhooks nw - WHERE nw.id = #{webhookDeliveryMethodId} - AND nw.subscriber_user_id = #{notificationUser} - ) - |] - when isValid $ do - -- Update the webhook config in Vault - WebhookSecrets.putWebhookConfig webhookDeliveryMethodId (WebhookConfig url) >>= \case - Left err -> respondError err - Right _ -> pure () - deleteWebhookDeliveryMethod :: UserId -> NotificationWebhookId -> WebApp () deleteWebhookDeliveryMethod notificationUser webhookDeliveryMethodId = do isValid <- PG.runTransaction $ do @@ -120,13 +111,13 @@ listProjectWebhooks caller projectId = do createProjectWebhook :: UserId -> ProjectId -> URIParam -> Set NotificationTopic -> WebApp ProjectWebhook createProjectWebhook subscriberUserId projectId url topics = do - webhookId <- createWebhookDeliveryMethod subscriberUserId url "Project Webhook" let topicGroups = mempty let filter = Nothing - subscriptionId <- PG.runTransaction $ do + runInIO <- UnliftIO.askRunInIO + subscriptionId <- PG.runTransactionOrRespondError $ do Project {ownerUserId = projectOwner} <- Q.expectProjectById projectId subscriptionId <- NotifQ.createNotificationSubscription subscriberUserId projectOwner (Just projectId) topics topicGroups filter - NotifQ.addSubscriptionDeliveryMethods subscriberUserId subscriptionId (NESet.singleton $ WebhookDeliveryMethodId webhookId) + addWebhookDeliveryMethod subscriberUserId url "Project Webhook" subscriptionId runInIO pure subscriptionId pure $ ProjectWebhook @@ -144,3 +135,31 @@ deleteProjectWebhook caller subscriptionId = do for_ webhooks \webhookId -> do deleteWebhookDeliveryMethod caller webhookId PG.runTransaction $ do NotifQ.deleteNotificationSubscription caller subscriptionId + +updateProjectWebhook :: SubscriptionOwner -> NotificationSubscriptionId -> URIParam -> (Maybe (Set NotificationTopic)) -> WebApp () +updateProjectWebhook subscriptionOwner subscriptionId uri topics = do + -- First fetch the webhook ids associated with this subscription + webhooks <- PG.runTransaction $ do NotifQ.webhooksForSubscription subscriptionId + for_ webhooks \webhookId -> do + -- Update the webhook config in Vault + WebhookSecrets.putWebhookConfig webhookId (WebhookConfig uri) >>= \case + Left err -> respondError err + Right _ -> pure () + PG.runTransaction $ NotifQ.updateNotificationSubscription subscriptionOwner subscriptionId topics mempty Nothing + +updateWebhookDeliveryMethod :: UserId -> NotificationWebhookId -> URIParam -> WebApp () +updateWebhookDeliveryMethod notificationUser webhookDeliveryMethodId url = do + isValid <- PG.runTransaction $ do + PG.queryExpect1Col + [PG.sql| + SELECT EXISTS( + SELECT FROM notification_webhooks nw + WHERE nw.id = #{webhookDeliveryMethodId} + AND nw.subscriber_user_id = #{notificationUser} + ) + |] + when isValid $ do + -- Update the webhook config in Vault + WebhookSecrets.putWebhookConfig webhookDeliveryMethodId (WebhookConfig url) >>= \case + Left err -> respondError err + Right _ -> pure () diff --git a/src/Share/Notifications/Queries.hs b/src/Share/Notifications/Queries.hs index 58d41070..d3320b7b 100644 --- a/src/Share/Notifications/Queries.hs +++ b/src/Share/Notifications/Queries.hs @@ -250,8 +250,11 @@ deleteNotificationSubscription subscriberUserId subscriptionId = do AND subscriber_user_id = #{subscriberUserId} |] -updateNotificationSubscription :: UserId -> NotificationSubscriptionId -> Maybe (Set NotificationTopic) -> Maybe (Set NotificationTopicGroup) -> Maybe SubscriptionFilter -> Transaction e () -updateNotificationSubscription subscriberUserId subscriptionId subscriptionTopics subscriptionTopicGroups subscriptionFilter = do +updateNotificationSubscription :: SubscriptionOwner -> NotificationSubscriptionId -> Maybe (Set NotificationTopic) -> Maybe (Set NotificationTopicGroup) -> Maybe SubscriptionFilter -> Transaction e () +updateNotificationSubscription owner subscriptionId subscriptionTopics subscriptionTopicGroups subscriptionFilter = do + let filter = case owner of + UserSubscriptionOwner subscriberUserId -> [sql| subscriber_user_id = #{subscriberUserId}|] + ProjectSubscriptionOwner projectOwnerUserId -> [sql| subscriber_project_id = #{projectOwnerUserId} |] execute_ [sql| UPDATE notification_subscriptions @@ -259,7 +262,7 @@ updateNotificationSubscription subscriberUserId subscriptionId subscriptionTopic topic_groups = COALESCE(#{Foldable.toList <$> subscriptionTopicGroups}::notification_topic_group[], topic_groups), filter = COALESCE(#{subscriptionFilter}, filter) WHERE id = #{subscriptionId} - AND subscriber_user_id = #{subscriberUserId} + AND ^{filter} |] getNotificationSubscription :: UserId -> NotificationSubscriptionId -> Transaction e (NotificationSubscription NotificationSubscriptionId) diff --git a/src/Share/Notifications/Types.hs b/src/Share/Notifications/Types.hs index 2c1a383f..c1db88c0 100644 --- a/src/Share/Notifications/Types.hs +++ b/src/Share/Notifications/Types.hs @@ -33,6 +33,7 @@ module Share.Notifications.Types CommentPayload (..), ReleasePayload (..), StatusUpdatePayload (..), + SubscriptionOwner (..), eventTopic, hydratedEventTopic, eventData_, @@ -974,3 +975,8 @@ hydratedEventTopic (HydratedEvent {hydratedEventPayload}) = case hydratedEventPa HydratedProjectTicketStatusUpdatedPayload {} -> ProjectTicketStatusUpdated HydratedProjectTicketCommentPayload {} -> ProjectTicketComment HydratedProjectReleaseCreatedPayload {} -> ProjectReleaseCreated + +data SubscriptionOwner + = ProjectSubscriptionOwner ProjectId + | UserSubscriptionOwner UserId + deriving stock (Show, Eq, Ord) diff --git a/src/Share/Web/Share/Projects/Impl.hs b/src/Share/Web/Share/Projects/Impl.hs index 875a04c9..edf99ccc 100644 --- a/src/Share/Web/Share/Projects/Impl.hs +++ b/src/Share/Web/Share/Projects/Impl.hs @@ -24,6 +24,7 @@ import Share.IDs (PrefixedHash (..), ProjectSlug (..), UserHandle, UserId) import Share.IDs qualified as IDs import Share.Notifications.Ops qualified as NotifOps import Share.Notifications.Queries qualified as NotifsQ +import Share.Notifications.Types (SubscriptionOwner (ProjectSubscriptionOwner)) import Share.OAuth.Session import Share.Postgres qualified as PG import Share.Postgres.Authorization.Queries qualified as AuthZQ @@ -471,8 +472,10 @@ updateProjectWebhookEndpoint session projectUserHandle projectSlug subscriptionI Project {projectId, ownerUserId = projectOwner} <- PG.runTransactionOrRespondError $ do Q.projectByShortHand projectShortHand `whenNothingM` throwError (EntityMissing (ErrorID "project-not-found") ("Project not found: " <> IDs.toText @IDs.ProjectShortHand projectShortHand)) _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkSubscriptionsManage caller projectOwner - webhook <- PG.runTransactionOrRespondError $ do - NotifsQ.updateProjectWebhook caller projectId subscriptionId url events `whenNothingM` throwError (EntityMissing (ErrorID "webhook:not-found") "Webhook not found") + let owner = ProjectSubscriptionOwner projectId + NotifOps.updateProjectWebhook owner subscriptionId url events + PG.runTransaction $ do + NotifQ.updateNotificationSubscription owner subscriptionId (Just events) mempty Nothing pure $ UpdateProjectWebhookResponse {webhook} where projectShortHand :: IDs.ProjectShortHand From f18620377ba29297c1cb5943e0393a4ca4df2961 Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Wed, 3 Sep 2025 12:34:49 -0700 Subject: [PATCH 14/35] Update a bunch of decoders --- src/Share/Notifications/Ops.hs | 42 +++++++----- src/Share/Notifications/Queries.hs | 96 +++++++++++++++++++-------- src/Share/Notifications/Types.hs | 51 +++++++++++--- src/Share/Web/Share/Projects/Types.hs | 52 +++++++-------- 4 files changed, 162 insertions(+), 79 deletions(-) diff --git a/src/Share/Notifications/Ops.hs b/src/Share/Notifications/Ops.hs index d061574e..a85ad3ec 100644 --- a/src/Share/Notifications/Ops.hs +++ b/src/Share/Notifications/Ops.hs @@ -44,11 +44,11 @@ listNotificationDeliveryMethods userId maySubscriptionId = do pure $ (EmailDeliveryMethod <$> emailDeliveryMethods) <> (WebhookDeliveryMethod <$> webhookDeliveryMethods) -addWebhookDeliveryMethod :: UserId -> URIParam -> Text -> NotificationSubscriptionId -> (AppM r (Either Webhooks.WebhookSecretError ()) -> IO (Either Webhooks.WebhookSecretError ())) -> PG.Transaction Webhooks.WebhookSecretError NotificationWebhookId -addWebhookDeliveryMethod userId uriParam webhookName notificationSubscriptionId runInIO = do +addWebhookDeliveryMethod :: URIParam -> Text -> NotificationSubscriptionId -> (AppM r (Either Webhooks.WebhookSecretError ()) -> IO (Either Webhooks.WebhookSecretError ())) -> PG.Transaction Webhooks.WebhookSecretError NotificationWebhookId +addWebhookDeliveryMethod uriParam webhookName notificationSubscriptionId runInIO = do let webhookConfig = WebhookConfig {uri = uriParam} -- Note that we can't be completely transactional between postgres and vault here. - webhookId <- NotifQ.createWebhookDeliveryMethod userId webhookName notificationSubscriptionId + webhookId <- NotifQ.createWebhookDeliveryMethod webhookName notificationSubscriptionId -- We run this inside the transaction such that, if it fails, the transaction -- will be rolled back. -- @@ -101,31 +101,41 @@ listProjectWebhooks caller projectId = do let webhooks = results <&> \(NotificationWebhookConfig {webhookDeliveryUrl = url}, _name, NotificationSubscription {subscriptionTopics, subscriptionId, subscriptionCreatedAt, subscriptionUpdatedAt}) -> ProjectWebhook - { url = URIParam url, - events = subscriptionTopics, - notificationSubscriptionId = subscriptionId, - createdAt = subscriptionCreatedAt, - updatedAt = subscriptionUpdatedAt + { projectWebhookUri = URIParam url, + projectWebhookEvents = subscriptionTopics, + projectWebhookNotificationSubscriptionId = subscriptionId, + projectWebhookCreatedAt = subscriptionCreatedAt, + projectWebhookUpdatedAt = subscriptionUpdatedAt } pure webhooks -createProjectWebhook :: UserId -> ProjectId -> URIParam -> Set NotificationTopic -> WebApp ProjectWebhook -createProjectWebhook subscriberUserId projectId url topics = do +createProjectWebhook :: ProjectId -> URIParam -> Set NotificationTopic -> WebApp ProjectWebhook +createProjectWebhook projectId uri topics = do let topicGroups = mempty let filter = Nothing runInIO <- UnliftIO.askRunInIO subscriptionId <- PG.runTransactionOrRespondError $ do Project {ownerUserId = projectOwner} <- Q.expectProjectById projectId subscriptionId <- NotifQ.createNotificationSubscription subscriberUserId projectOwner (Just projectId) topics topicGroups filter - addWebhookDeliveryMethod subscriberUserId url "Project Webhook" subscriptionId runInIO + addWebhookDeliveryMethod uri "Project Webhook" subscriptionId runInIO pure subscriptionId + expectProjectWebhook subscriberUserId projectId subscriptionId + +expectProjectWebhook :: UserId -> ProjectId -> NotificationSubscriptionId -> WebApp ProjectWebhook +expectProjectWebhook caller projectId subscriptionId = do + (webhookId, _name) <- PG.runTransaction $ do NotifQ.expectProjectWebhook projectId subscriptionId + uri <- + WebhookSecrets.fetchWebhookConfig webhookId >>= \case + Left err -> respondError err + Right (WebhookConfig {uri}) -> pure uri + subscription <- PG.runTransaction $ do NotifQ.expectNotificationSubscription (ProjectSubscriptionOwner projectId) subscriptionId pure $ ProjectWebhook - { url, - events = topics, - notificationSubscriptionId = subscriptionId, - createdAt = Nothing, - updatedAt = Nothing + { projectWebhookUri = uri, + projectWebhookEvents = subscription.subscriptionTopics, + projectWebhookNotificationSubscriptionId = subscription.subscriptionId, + projectWebhookCreatedAt = subscription.subscriptionCreatedAt, + projectWebhookUpdatedAt = subscription.subscriptionUpdatedAt } deleteProjectWebhook :: UserId -> NotificationSubscriptionId -> WebApp () diff --git a/src/Share/Notifications/Queries.hs b/src/Share/Notifications/Queries.hs index d3320b7b..d9c5afd3 100644 --- a/src/Share/Notifications/Queries.hs +++ b/src/Share/Notifications/Queries.hs @@ -16,13 +16,14 @@ module Share.Notifications.Queries createNotificationSubscription, deleteNotificationSubscription, updateNotificationSubscription, - getNotificationSubscription, + expectNotificationSubscription, hydrateEventPayload, hasUnreadNotifications, updateWatchProjectSubscription, isUserSubscribedToWatchProject, listProjectWebhooks, webhooksForSubscription, + expectProjectWebhook, ) where @@ -204,55 +205,70 @@ deleteEmailDeliveryMethod notificationUserId emailDeliveryMethodId = do AND subscriber_user_id = #{notificationUserId} |] -createWebhookDeliveryMethod :: UserId -> Text -> NotificationSubscriptionId -> Transaction e NotificationWebhookId -createWebhookDeliveryMethod userId name subscriptionId = do +createWebhookDeliveryMethod :: Text -> NotificationSubscriptionId -> Transaction e NotificationWebhookId +createWebhookDeliveryMethod name subscriptionId = do queryExpect1Col [sql| - INSERT INTO notification_webhooks (subscriber_user_id, name, subscription_id) - VALUES (#{userId}, #{name}, #{subscriptionId}) + INSERT INTO notification_webhooks (name, subscription_id) + VALUES (#{name}, #{subscriptionId}) RETURNING id |] -deleteWebhookDeliveryMethod :: UserId -> NotificationWebhookId -> Transaction e () -deleteWebhookDeliveryMethod notificationUserId webhookDeliveryMethodId = do +deleteWebhookDeliveryMethod :: NotificationWebhookId -> Transaction e () +deleteWebhookDeliveryMethod webhookDeliveryMethodId = do execute_ [sql| DELETE FROM notification_webhooks WHERE id = #{webhookDeliveryMethodId} - AND subscriber_user_id = #{notificationUserId} |] listNotificationSubscriptions :: UserId -> Transaction e [NotificationSubscription NotificationSubscriptionId] listNotificationSubscriptions subscriberUserId = do queryListRows [sql| - SELECT ns.id, ns.scope_user_id, ns.project_id, ns.topics, ns.topic_groups, ns.filter, ns.created_at, ns.updated_at + SELECT + ns.id, + ns.scope_user_id, + ns.scope_project_id, + ns.subscriber_user_id, + ns.subscriber_project_id, + ns.topics, + ns.topic_groups, + ns.filter, + ns.created_at, + ns.updated_at FROM notification_subscriptions ns WHERE ns.subscriber_user_id = #{subscriberUserId} ORDER BY ns.created_at DESC |] -createNotificationSubscription :: UserId -> UserId -> Maybe ProjectId -> Set NotificationTopic -> Set NotificationTopicGroup -> Maybe SubscriptionFilter -> Transaction e NotificationSubscriptionId -createNotificationSubscription subscriberUserId subscriptionScope subscriptionProjectId subscriptionTopics subscriptionTopicGroups subscriptionFilter = do +createNotificationSubscription :: SubscriptionOwner -> UserId -> Maybe ProjectId -> Set NotificationTopic -> Set NotificationTopicGroup -> Maybe SubscriptionFilter -> Transaction e NotificationSubscriptionId +createNotificationSubscription owner subscriptionScope subscriptionProjectId subscriptionTopics subscriptionTopicGroups subscriptionFilter = do + let (subscriberUserId, subscriberProjectId) = case owner of + ProjectSubscriptionOwner projectId -> (Nothing, Just projectId) + UserSubscriptionOwner userId -> (Just userId, Nothing) queryExpect1Col [sql| - INSERT INTO notification_subscriptions (subscriber_user_id, scope_user_id, project_id, topics, topic_groups, filter) - VALUES (#{subscriberUserId}, #{subscriptionScope}, #{subscriptionProjectId}, #{Foldable.toList subscriptionTopics}::notification_topic[], #{Foldable.toList subscriptionTopicGroups}::notification_topic_group[], #{subscriptionFilter}) + INSERT INTO notification_subscriptions (subscriber_user_id, subscriber_project_id, scope_user_id, scope_project_id, topics, topic_groups, filter) + VALUES (#{subscriberUserId}, #{subscriberProjectId}, #{subscriptionScope}, #{subscriptionProjectId}, #{Foldable.toList subscriptionTopics}::notification_topic[], #{Foldable.toList subscriptionTopicGroups}::notification_topic_group[], #{subscriptionFilter}) RETURNING id |] -deleteNotificationSubscription :: UserId -> NotificationSubscriptionId -> Transaction e () -deleteNotificationSubscription subscriberUserId subscriptionId = do +deleteNotificationSubscription :: SubscriptionOwner -> NotificationSubscriptionId -> Transaction e () +deleteNotificationSubscription owner subscriptionId = do + let ownerFilter = case owner of + UserSubscriptionOwner subscriberUserId -> [sql| subscriber_user_id = #{subscriberUserId}|] + ProjectSubscriptionOwner projectOwnerUserId -> [sql| subscriber_project_id = #{projectOwnerUserId} |] execute_ [sql| DELETE FROM notification_subscriptions WHERE id = #{subscriptionId} - AND subscriber_user_id = #{subscriberUserId} + AND ^{ownerFilter} |] updateNotificationSubscription :: SubscriptionOwner -> NotificationSubscriptionId -> Maybe (Set NotificationTopic) -> Maybe (Set NotificationTopicGroup) -> Maybe SubscriptionFilter -> Transaction e () updateNotificationSubscription owner subscriptionId subscriptionTopics subscriptionTopicGroups subscriptionFilter = do - let filter = case owner of + let ownerFilter = case owner of UserSubscriptionOwner subscriberUserId -> [sql| subscriber_user_id = #{subscriberUserId}|] ProjectSubscriptionOwner projectOwnerUserId -> [sql| subscriber_project_id = #{projectOwnerUserId} |] execute_ @@ -262,17 +278,30 @@ updateNotificationSubscription owner subscriptionId subscriptionTopics subscript topic_groups = COALESCE(#{Foldable.toList <$> subscriptionTopicGroups}::notification_topic_group[], topic_groups), filter = COALESCE(#{subscriptionFilter}, filter) WHERE id = #{subscriptionId} - AND ^{filter} + AND ^{ownerFilter} |] -getNotificationSubscription :: UserId -> NotificationSubscriptionId -> Transaction e (NotificationSubscription NotificationSubscriptionId) -getNotificationSubscription subscriberUserId subscriptionId = do +expectNotificationSubscription :: SubscriptionOwner -> NotificationSubscriptionId -> Transaction e (NotificationSubscription NotificationSubscriptionId) +expectNotificationSubscription subscriptionOwner subscriptionId = do + let ownerFilter = case subscriptionOwner of + UserSubscriptionOwner subscriberUserId -> [sql| ns.subscriber_user_id = #{subscriberUserId}|] + ProjectSubscriptionOwner projectOwnerUserId -> [sql| ns.subscriber_project_id = #{projectOwnerUserId} |] queryExpect1Row [sql| - SELECT ns.id, ns.scope_user_id, ns.topics, ns.topic_groups, ns.filter, ns.created_at, ns.updated_at + SELECT + ns.id, + ns.scope_user_id, + ns.scope_project_id, + ns.subscriber_user_id, + ns.subscriber_project_id, + ns.topics, + ns.topic_groups, + ns.filter, + ns.created_at, + ns.updated_at FROM notification_subscriptions ns WHERE ns.id = #{subscriptionId} - AND ns.subscriber_user_id = #{subscriberUserId} + AND ^{ownerFilter} |] -- | Events are complex, so for now we hydrate them one at a time using a simple traverse @@ -473,7 +502,7 @@ updateWatchProjectSubscription userId projId shouldBeSubscribed = do FROM projects p WHERE p.id = #{projId} |] - Just <$> createNotificationSubscription userId projectOwnerUserId (Just projId) mempty (Set.singleton WatchProject) (Just filter) + Just <$> createNotificationSubscription (UserSubscriptionOwner userId) projectOwnerUserId (Just projId) mempty (Set.singleton WatchProject) (Just filter) _ -> pure Nothing isUserSubscribedToWatchProject :: UserId -> ProjectId -> Transaction e (Maybe NotificationSubscriptionId) @@ -506,6 +535,9 @@ listProjectWebhooks caller projectId = do -- individual topic events. ns.id, ns.scope_user_id, + ns.scope_project_id, + ns.subscriber_user_id, + ns.subscriber_project_id, ns.topics, ns.topic_groups, ns.filter, @@ -519,11 +551,23 @@ listProjectWebhooks caller projectId = do |] <&> fmap (\((webhookId, webhookName) PG.:. subscription) -> (webhookId, webhookName, subscription)) +-- | Gets the (first) webhook associated with a project webhook notification. +expectProjectWebhook :: ProjectId -> NotificationSubscriptionId -> PG.Transaction e (NotificationWebhookId, Text) +expectProjectWebhook projectId subscriptionId = do + PG.queryExpect1Row @(NotificationWebhookId, Text) + [PG.sql| + SELECT nw.id, nw.name + FROM notification_webhooks nw + WHERE nw.subscription_id = #{subscriptionId} + AND nw.subscriber_project_id = #{projectId} + LIMIT 1 + |] + webhooksForSubscription :: NotificationSubscriptionId -> Transaction e [NotificationWebhookId] webhooksForSubscription subscriptionId = do PG.queryListCol [PG.sql| - SELECT nbw.webhook_id - FROM notification_by_webhook nbw - WHERE nbw.subscription_id = #{subscriptionId} + SELECT nw.id + FROM notification_webhooks nw + WHERE nw.subscription_id = #{subscriptionId} |] diff --git a/src/Share/Notifications/Types.hs b/src/Share/Notifications/Types.hs index c1db88c0..bad3c965 100644 --- a/src/Share/Notifications/Types.hs +++ b/src/Share/Notifications/Types.hs @@ -579,8 +579,9 @@ instance Aeson.FromJSON NotificationDeliveryMethod where data NotificationSubscription id = NotificationSubscription { subscriptionId :: id, - subscriptionScope :: UserId, - subscriptionProjectId :: Maybe ProjectId, + subscriptionScopeUser :: UserId, + subscriptionScopeProject :: Maybe ProjectId, + subscriptionOwner :: SubscriptionOwner, subscriptionTopics :: Set NotificationTopic, subscriptionTopicGroups :: Set NotificationTopicGroup, subscriptionFilter :: Maybe NotificationFilter, @@ -592,21 +593,28 @@ data NotificationSubscription id = NotificationSubscription instance PG.DecodeRow (NotificationSubscription NotificationSubscriptionId) where decodeRow = do subscriptionId <- PG.decodeField - subscriptionScope <- PG.decodeField - subscriptionProjectId <- PG.decodeField + subscriptionScopeUser <- PG.decodeField + subscriptionScopeProject <- PG.decodeField + subscriberUser <- PG.decodeField + subscriberProject <- PG.decodeField + let subscriptionOwner = case (subscriberUser, subscriberProject) of + (Just uid, Nothing) -> UserSubscriptionOwner uid + (Nothing, Just pid) -> ProjectSubscriptionOwner pid + _ -> error "Invalid subscription owner in database" subscriptionTopics <- Set.fromList <$> PG.decodeField subscriptionTopicGroups <- Set.fromList <$> PG.decodeField subscriptionFilter <- PG.decodeField subscriptionCreatedAt <- PG.decodeField subscriptionUpdatedAt <- PG.decodeField - pure $ NotificationSubscription {subscriptionId, subscriptionScope, subscriptionProjectId, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter, subscriptionCreatedAt, subscriptionUpdatedAt} + pure $ NotificationSubscription {subscriptionId, subscriptionScopeUser, subscriptionScopeProject, subscriptionOwner, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter, subscriptionCreatedAt, subscriptionUpdatedAt} instance Aeson.ToJSON (NotificationSubscription NotificationSubscriptionId) where - toJSON NotificationSubscription {subscriptionId, subscriptionScope, subscriptionProjectId, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter, subscriptionCreatedAt, subscriptionUpdatedAt} = + toJSON NotificationSubscription {subscriptionId, subscriptionScopeUser, subscriptionScopeProject, subscriptionOwner, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter, subscriptionCreatedAt, subscriptionUpdatedAt} = Aeson.object [ "id" Aeson..= subscriptionId, - "scope" Aeson..= subscriptionScope, - "projectId" Aeson..= subscriptionProjectId, + "scope" Aeson..= subscriptionScopeUser, + "projectId" Aeson..= subscriptionScopeProject, + "owner" Aeson..= subscriptionOwner, "topics" Aeson..= subscriptionTopics, "topicGroups" Aeson..= subscriptionTopicGroups, "filter" Aeson..= subscriptionFilter, @@ -617,15 +625,16 @@ instance Aeson.ToJSON (NotificationSubscription NotificationSubscriptionId) wher instance Aeson.FromJSON (NotificationSubscription NotificationSubscriptionId) where parseJSON = Aeson.withObject "NotificationSubscription" \o -> do subscriptionId <- o .: "id" - subscriptionScope <- o .: "scope" - subscriptionProjectId <- o .: "projectId" + subscriptionScopeUser <- o .: "scope" + subscriptionScopeProject <- o .:? "scope_project" + subscriptionOwner <- o .: "owner" subscriptionTopics <- o .: "topics" subscriptionTopicGroups <- o .: "topicGroups" subscriptionFilter <- o .:? "filter" subscriptionCreatedAt <- o .:? "createdAt" subscriptionUpdatedAt <- o .:? "updatedAt" - pure NotificationSubscription {subscriptionId, subscriptionScope, subscriptionProjectId, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter, subscriptionCreatedAt, subscriptionUpdatedAt} + pure NotificationSubscription {subscriptionId, subscriptionScopeUser, subscriptionScopeProject, subscriptionOwner, subscriptionTopics, subscriptionTopicGroups, subscriptionFilter, subscriptionCreatedAt, subscriptionUpdatedAt} data NotificationHubEntry userInfo eventPayload = NotificationHubEntry { hubEntryId :: NotificationHubEntryId, @@ -980,3 +989,23 @@ data SubscriptionOwner = ProjectSubscriptionOwner ProjectId | UserSubscriptionOwner UserId deriving stock (Show, Eq, Ord) + +instance ToJSON SubscriptionOwner where + toJSON (ProjectSubscriptionOwner pid) = + Aeson.object + [ "kind" .= ("project" :: Text), + "id" .= pid + ] + toJSON (UserSubscriptionOwner uid) = + Aeson.object + [ "kind" .= ("user" :: Text), + "id" .= uid + ] + +instance FromJSON SubscriptionOwner where + parseJSON = Aeson.withObject "SubscriptionOwner" \o -> do + kind <- o .: "kind" + case kind of + "project" -> ProjectSubscriptionOwner <$> o .: "id" + "user" -> UserSubscriptionOwner <$> o .: "id" + _ -> fail $ "Unknown subscription owner kind: " <> Text.unpack kind diff --git a/src/Share/Web/Share/Projects/Types.hs b/src/Share/Web/Share/Projects/Types.hs index 212fad4c..bf2528ff 100644 --- a/src/Share/Web/Share/Projects/Types.hs +++ b/src/Share/Web/Share/Projects/Types.hs @@ -387,40 +387,40 @@ instance Aeson.ToJSON CatalogCategory where -- | This type provides a view over the subscription <-> webhook many-to-many view. data ProjectWebhook = ProjectWebhook - { url :: URIParam, - events :: Set NotificationTopic, - notificationSubscriptionId :: NotificationSubscriptionId, - createdAt :: Maybe UTCTime, - updatedAt :: Maybe UTCTime + { projectWebhookUri :: URIParam, + projectWebhookEvents :: Set NotificationTopic, + projectWebhookNotificationSubscriptionId :: NotificationSubscriptionId, + projectWebhookCreatedAt :: Maybe UTCTime, + projectWebhookUpdatedAt :: Maybe UTCTime } deriving stock (Eq, Show) instance PG.DecodeRow ProjectWebhook where decodeRow = do - url <- PG.decodeField - events <- Set.fromList <$> PG.decodeField - notificationSubscriptionId <- PG.decodeField - createdAt <- PG.decodeField - updatedAt <- PG.decodeField + projectWebhookUri <- PG.decodeField + projectWebhookEvents <- Set.fromList <$> PG.decodeField + projectWebhookNotificationSubscriptionId <- PG.decodeField + projectWebhookCreatedAt <- PG.decodeField + projectWebhookUpdatedAt <- PG.decodeField pure ProjectWebhook {..} instance ToJSON ProjectWebhook where toJSON ProjectWebhook {..} = object - [ "url" .= url, - "events" .= events, - "notificationSubscriptionId" .= notificationSubscriptionId, - "createdAt" .= createdAt, - "updatedAt" .= updatedAt + [ "uri" .= projectWebhookUri, + "events" .= projectWebhookEvents, + "notificationSubscriptionId" .= projectWebhookNotificationSubscriptionId, + "createdAt" .= projectWebhookCreatedAt, + "updatedAt" .= projectWebhookUpdatedAt ] instance FromJSON ProjectWebhook where parseJSON = Aeson.withObject "ProjectWebhook" $ \o -> do - url <- o .: "url" - events <- o .: "events" - notificationSubscriptionId <- o .: "notificationSubscriptionId" - createdAt <- o .: "createdAt" - updatedAt <- o .: "updatedAt" + projectWebhookUri <- o .: "uri" + projectWebhookEvents <- o .: "events" + projectWebhookNotificationSubscriptionId <- o .: "notificationSubscriptionId" + projectWebhookCreatedAt <- o .: "createdAt" + projectWebhookUpdatedAt <- o .: "updatedAt" pure ProjectWebhook {..} data ListProjectWebhooksResponse = ListProjectWebhooksResponse @@ -440,7 +440,7 @@ instance FromJSON ListProjectWebhooksResponse where pure ListProjectWebhooksResponse {..} data CreateProjectWebhookRequest = CreateProjectWebhookRequest - { url :: URIParam, + { uri :: URIParam, events :: Set NotificationTopic, subscriber :: UserHandle } @@ -449,14 +449,14 @@ data CreateProjectWebhookRequest = CreateProjectWebhookRequest instance ToJSON CreateProjectWebhookRequest where toJSON CreateProjectWebhookRequest {..} = object - [ "url" .= url, + [ "uri" .= uri, "events" .= events, "subscriber" .= (IDs.toText $ PrefixedID @"@" subscriber) ] instance FromJSON CreateProjectWebhookRequest where parseJSON = Aeson.withObject "CreateProjectWebhookRequest" $ \o -> do - url <- o .: "url" + uri <- o .: "uri" events <- o .: "events" subscriber <- o .: "subscriber" pure CreateProjectWebhookRequest {..} @@ -478,7 +478,7 @@ instance FromJSON CreateProjectWebhookResponse where pure CreateProjectWebhookResponse {..} data UpdateProjectWebhookRequest = UpdateProjectWebhookRequest - { url :: Maybe URIParam, + { uri :: Maybe URIParam, events :: Maybe (Set NotificationTopic) } deriving stock (Eq, Show) @@ -486,13 +486,13 @@ data UpdateProjectWebhookRequest = UpdateProjectWebhookRequest instance ToJSON UpdateProjectWebhookRequest where toJSON UpdateProjectWebhookRequest {..} = object - [ "url" .= url, + [ "uri" .= uri, "events" .= events ] instance FromJSON UpdateProjectWebhookRequest where parseJSON = Aeson.withObject "UpdateProjectWebhookRequest" $ \o -> do - url <- o .:? "url" + uri <- o .:? "uri" events <- o .:? "events" pure UpdateProjectWebhookRequest {..} From fea67acb5545a5be2be3ac68e6ba49e289610f19 Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Wed, 3 Sep 2025 12:46:45 -0700 Subject: [PATCH 15/35] Fix up more project webhook endpoints --- src/Share/Notifications/Ops.hs | 50 ++++++++++++++++------------ src/Share/Notifications/Queries.hs | 11 ++++-- src/Share/Web/Share/Projects/Impl.hs | 15 ++++----- 3 files changed, 45 insertions(+), 31 deletions(-) diff --git a/src/Share/Notifications/Ops.hs b/src/Share/Notifications/Ops.hs index a85ad3ec..207b8fb8 100644 --- a/src/Share/Notifications/Ops.hs +++ b/src/Share/Notifications/Ops.hs @@ -7,6 +7,7 @@ module Share.Notifications.Ops createProjectWebhook, deleteProjectWebhook, updateProjectWebhook, + expectProjectWebhook, hydrateEvent, ) where @@ -60,15 +61,20 @@ addWebhookDeliveryMethod uriParam webhookName notificationSubscriptionId runInIO Right _ -> pure () pure webhookId -deleteWebhookDeliveryMethod :: UserId -> NotificationWebhookId -> WebApp () -deleteWebhookDeliveryMethod notificationUser webhookDeliveryMethodId = do +deleteWebhookDeliveryMethod :: SubscriptionOwner -> NotificationWebhookId -> WebApp () +deleteWebhookDeliveryMethod owner webhookDeliveryMethodId = do + let ownerFilter = case owner of + UserSubscriptionOwner userId -> [PG.sql| ns.subscriber_user_id = #{userId} |] + ProjectSubscriptionOwner projectId -> [PG.sql| ns.subscriber_project_id = #{projectId} |] isValid <- PG.runTransaction $ do PG.queryExpect1Col [PG.sql| SELECT EXISTS( SELECT FROM notification_webhooks nw + JOIN notification_subscriptions ns + ON nw.subscription_id = ns.id WHERE nw.id = #{webhookDeliveryMethodId} - AND nw.subscriber_user_id = #{notificationUser} + AND ^{ownerFilter} ) |] when isValid $ do @@ -77,7 +83,7 @@ deleteWebhookDeliveryMethod notificationUser webhookDeliveryMethodId = do Left err -> respondError err Right _ -> do PG.runTransaction $ do - NotifQ.deleteWebhookDeliveryMethod notificationUser webhookDeliveryMethodId + NotifQ.deleteWebhookDeliveryMethod owner webhookDeliveryMethodId hydrateEvent :: HydratedEventPayload -> PG.Transaction e HydratedEvent hydrateEvent hydratedEventPayload = do @@ -116,13 +122,13 @@ createProjectWebhook projectId uri topics = do runInIO <- UnliftIO.askRunInIO subscriptionId <- PG.runTransactionOrRespondError $ do Project {ownerUserId = projectOwner} <- Q.expectProjectById projectId - subscriptionId <- NotifQ.createNotificationSubscription subscriberUserId projectOwner (Just projectId) topics topicGroups filter + subscriptionId <- NotifQ.createNotificationSubscription (ProjectSubscriptionOwner projectId) projectOwner (Just projectId) topics topicGroups filter addWebhookDeliveryMethod uri "Project Webhook" subscriptionId runInIO pure subscriptionId - expectProjectWebhook subscriberUserId projectId subscriptionId + expectProjectWebhook projectId subscriptionId -expectProjectWebhook :: UserId -> ProjectId -> NotificationSubscriptionId -> WebApp ProjectWebhook -expectProjectWebhook caller projectId subscriptionId = do +expectProjectWebhook :: ProjectId -> NotificationSubscriptionId -> WebApp ProjectWebhook +expectProjectWebhook projectId subscriptionId = do (webhookId, _name) <- PG.runTransaction $ do NotifQ.expectProjectWebhook projectId subscriptionId uri <- WebhookSecrets.fetchWebhookConfig webhookId >>= \case @@ -138,23 +144,25 @@ expectProjectWebhook caller projectId subscriptionId = do projectWebhookUpdatedAt = subscription.subscriptionUpdatedAt } -deleteProjectWebhook :: UserId -> NotificationSubscriptionId -> WebApp () -deleteProjectWebhook caller subscriptionId = do +deleteProjectWebhook :: ProjectId -> NotificationSubscriptionId -> WebApp () +deleteProjectWebhook projectId subscriptionId = do + let owner = ProjectSubscriptionOwner projectId -- First fetch the webhook id associated with this subscription webhooks <- PG.runTransaction $ do NotifQ.webhooksForSubscription subscriptionId for_ webhooks \webhookId -> do - deleteWebhookDeliveryMethod caller webhookId - PG.runTransaction $ do NotifQ.deleteNotificationSubscription caller subscriptionId + deleteWebhookDeliveryMethod owner webhookId + PG.runTransaction $ do NotifQ.deleteNotificationSubscription owner subscriptionId -updateProjectWebhook :: SubscriptionOwner -> NotificationSubscriptionId -> URIParam -> (Maybe (Set NotificationTopic)) -> WebApp () -updateProjectWebhook subscriptionOwner subscriptionId uri topics = do - -- First fetch the webhook ids associated with this subscription - webhooks <- PG.runTransaction $ do NotifQ.webhooksForSubscription subscriptionId - for_ webhooks \webhookId -> do - -- Update the webhook config in Vault - WebhookSecrets.putWebhookConfig webhookId (WebhookConfig uri) >>= \case - Left err -> respondError err - Right _ -> pure () +updateProjectWebhook :: SubscriptionOwner -> NotificationSubscriptionId -> Maybe URIParam -> (Maybe (Set NotificationTopic)) -> WebApp () +updateProjectWebhook subscriptionOwner subscriptionId mayURIUpdate topics = do + for_ mayURIUpdate \uri -> do + -- First fetch the webhook ids associated with this subscription + webhooks <- PG.runTransaction $ do NotifQ.webhooksForSubscription subscriptionId + for_ webhooks \webhookId -> do + -- Update the webhook config in Vault + WebhookSecrets.putWebhookConfig webhookId (WebhookConfig uri) >>= \case + Left err -> respondError err + Right _ -> pure () PG.runTransaction $ NotifQ.updateNotificationSubscription subscriptionOwner subscriptionId topics mempty Nothing updateWebhookDeliveryMethod :: UserId -> NotificationWebhookId -> URIParam -> WebApp () diff --git a/src/Share/Notifications/Queries.hs b/src/Share/Notifications/Queries.hs index d9c5afd3..8d48f446 100644 --- a/src/Share/Notifications/Queries.hs +++ b/src/Share/Notifications/Queries.hs @@ -214,12 +214,18 @@ createWebhookDeliveryMethod name subscriptionId = do RETURNING id |] -deleteWebhookDeliveryMethod :: NotificationWebhookId -> Transaction e () -deleteWebhookDeliveryMethod webhookDeliveryMethodId = do +deleteWebhookDeliveryMethod :: SubscriptionOwner -> NotificationWebhookId -> Transaction e () +deleteWebhookDeliveryMethod owner webhookDeliveryMethodId = do + let ownerFilter = case owner of + UserSubscriptionOwner userId -> [sql| nw.subscriber_user_id = #{userId} |] + ProjectSubscriptionOwner projectId -> [sql| nw.subscriber_project_id = #{projectId} |] execute_ [sql| DELETE FROM notification_webhooks + USING notification_subscriptions ns WHERE id = #{webhookDeliveryMethodId} + AND ns.id = nw.subscription_id + AND ^{ownerFilter} |] listNotificationSubscriptions :: UserId -> Transaction e [NotificationSubscription NotificationSubscriptionId] @@ -267,6 +273,7 @@ deleteNotificationSubscription owner subscriptionId = do |] updateNotificationSubscription :: SubscriptionOwner -> NotificationSubscriptionId -> Maybe (Set NotificationTopic) -> Maybe (Set NotificationTopicGroup) -> Maybe SubscriptionFilter -> Transaction e () +updateNotificationSubscription _owner _subscriptionId Nothing Nothing Nothing = pure () updateNotificationSubscription owner subscriptionId subscriptionTopics subscriptionTopicGroups subscriptionFilter = do let ownerFilter = case owner of UserSubscriptionOwner subscriberUserId -> [sql| subscriber_user_id = #{subscriberUserId}|] diff --git a/src/Share/Web/Share/Projects/Impl.hs b/src/Share/Web/Share/Projects/Impl.hs index edf99ccc..4faa3b0f 100644 --- a/src/Share/Web/Share/Projects/Impl.hs +++ b/src/Share/Web/Share/Projects/Impl.hs @@ -455,27 +455,26 @@ listProjectWebhooksEndpoint session projectUserHandle projectSlug = do projectShortHand = IDs.ProjectShortHand {userHandle = projectUserHandle, projectSlug} createProjectWebhookEndpoint :: Maybe Session -> UserHandle -> ProjectSlug -> CreateProjectWebhookRequest -> WebApp CreateProjectWebhookResponse -createProjectWebhookEndpoint session projectUserHandle projectSlug (CreateProjectWebhookRequest {url, events}) = do +createProjectWebhookEndpoint session projectUserHandle projectSlug (CreateProjectWebhookRequest {uri, events}) = do caller <- AuthN.requireAuthenticatedUser session Project {projectId, ownerUserId = projectOwner} <- PG.runTransactionOrRespondError $ do Q.projectByShortHand projectShortHand `whenNothingM` throwError (EntityMissing (ErrorID "project-not-found") ("Project not found: " <> IDs.toText @IDs.ProjectShortHand projectShortHand)) _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkSubscriptionsManage caller projectOwner - webhook <- NotifOps.createProjectWebhook caller projectId url events + webhook <- NotifOps.createProjectWebhook projectId uri events pure $ CreateProjectWebhookResponse {webhook} where projectShortHand :: IDs.ProjectShortHand projectShortHand = IDs.ProjectShortHand {userHandle = projectUserHandle, projectSlug} updateProjectWebhookEndpoint :: Maybe Session -> UserHandle -> ProjectSlug -> IDs.NotificationSubscriptionId -> UpdateProjectWebhookRequest -> WebApp UpdateProjectWebhookResponse -updateProjectWebhookEndpoint session projectUserHandle projectSlug subscriptionId (UpdateProjectWebhookRequest {url, events}) = do +updateProjectWebhookEndpoint session projectUserHandle projectSlug subscriptionId (UpdateProjectWebhookRequest {uri = mayURIUpdate, events}) = do caller <- AuthN.requireAuthenticatedUser session Project {projectId, ownerUserId = projectOwner} <- PG.runTransactionOrRespondError $ do Q.projectByShortHand projectShortHand `whenNothingM` throwError (EntityMissing (ErrorID "project-not-found") ("Project not found: " <> IDs.toText @IDs.ProjectShortHand projectShortHand)) _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkSubscriptionsManage caller projectOwner let owner = ProjectSubscriptionOwner projectId - NotifOps.updateProjectWebhook owner subscriptionId url events - PG.runTransaction $ do - NotifQ.updateNotificationSubscription owner subscriptionId (Just events) mempty Nothing + NotifOps.updateProjectWebhook owner subscriptionId mayURIUpdate events + webhook <- NotifOps.expectProjectWebhook projectId subscriptionId pure $ UpdateProjectWebhookResponse {webhook} where projectShortHand :: IDs.ProjectShortHand @@ -484,10 +483,10 @@ updateProjectWebhookEndpoint session projectUserHandle projectSlug subscriptionI deleteProjectWebhookEndpoint :: Maybe Session -> UserHandle -> ProjectSlug -> IDs.NotificationSubscriptionId -> WebApp () deleteProjectWebhookEndpoint session projectUserHandle projectSlug subscriptionId = do caller <- AuthN.requireAuthenticatedUser session - Project {ownerUserId = projectOwner} <- PG.runTransactionOrRespondError $ do + Project {ownerUserId = projectOwner, projectId} <- PG.runTransactionOrRespondError $ do Q.projectByShortHand projectShortHand `whenNothingM` throwError (EntityMissing (ErrorID "project-not-found") ("Project not found: " <> IDs.toText @IDs.ProjectShortHand projectShortHand)) _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkSubscriptionsManage caller projectOwner - NotifOps.deleteProjectWebhook caller subscriptionId + NotifOps.deleteProjectWebhook projectId subscriptionId where projectShortHand :: IDs.ProjectShortHand projectShortHand = IDs.ProjectShortHand {userHandle = projectUserHandle, projectSlug} From beffdcd483fbf24eb936ea2be58fd0264a951da6 Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Wed, 3 Sep 2025 12:54:42 -0700 Subject: [PATCH 16/35] Update project webhooks permissions --- src/Share/Web/Authorization.hs | 18 ++++++++++++++++++ src/Share/Web/Share/Projects/Impl.hs | 16 ++++++++-------- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/src/Share/Web/Authorization.hs b/src/Share/Web/Authorization.hs index 4a59379a..fd489671 100644 --- a/src/Share/Web/Authorization.hs +++ b/src/Share/Web/Authorization.hs @@ -56,6 +56,8 @@ module Share.Web.Authorization checkDeliveryMethodsManage, checkSubscriptionsView, checkSubscriptionsManage, + checkProjectSubscriptionsView, + checkProjectSubscriptionsManage, permissionGuard, readPath, writePath, @@ -165,6 +167,8 @@ data ProjectPermission | ProjectRolesEdit ProjectId | -- (RootHash, TargetHash) AccessCausalHash CausalId CausalId + | ProjectNotificationSubscriptionsView ProjectId + | ProjectNotificationSubscriptionsManage ProjectId deriving stock (Show, Eq, Ord) data UserPermission @@ -236,6 +240,8 @@ instance Errors.ToServerError AuthZFailure where ProjectRolesList _pid -> (ErrorID "authz:maintainers:list", err403 {errBody = "Permission Denied: " <> msg}) ProjectRolesEdit _pid -> (ErrorID "authz:maintainers:edit", err403 {errBody = "Permission Denied: " <> msg}) AccessCausalHash _ _ -> (ErrorID "authz:causal-hash", err403 {errBody = "Permission Denied: " <> msg}) + ProjectNotificationSubscriptionsView _pid -> (ErrorID "authz:project:notification-subscription-get", err403 {errBody = "Permission Denied: " <> msg}) + ProjectNotificationSubscriptionsManage _pid -> (ErrorID "authz:project:notification-subscription-manage", err403 {errBody = "Permission Denied: " <> msg}) UserPermission userPermission -> case userPermission of UserUpdate _uid -> (ErrorID "authz:user:update", err403 {errBody = "Permission Denied: " <> msg}) @@ -299,6 +305,8 @@ authZFailureMessage (AuthZFailure perm) = case perm of ProjectRolesList _pid -> "Not permitted to list maintainers" ProjectRolesEdit _pid -> "Not permitted to edit maintainers" AccessCausalHash _ _ -> "Not permitted to access this causal hash" + ProjectNotificationSubscriptionsView _pid -> "Not permitted to get project notifications" + ProjectNotificationSubscriptionsManage _pid -> "Not permitted to manage project notifications" UserPermission userPermission -> case userPermission of UserUpdate _uid -> "Not permitted to update this user" @@ -750,6 +758,16 @@ checkSubscriptionsManage caller notificationUser = maybePermissionFailure (UserP assertUsersEqual caller notificationUser <|> assertUserHasOrgPermissionByOrgUser caller notificationUser AuthZ.NotificationSubscriptionManage pure $ AuthZ.UnsafeAuthZReceipt Nothing +checkProjectSubscriptionsView :: UserId -> ProjectId -> WebApp (Either AuthZFailure AuthZ.AuthZReceipt) +checkProjectSubscriptionsView caller projectId = maybePermissionFailure (ProjectPermission $ ProjectNotificationSubscriptionsView projectId) do + assertUserHasProjectPermission AuthZ.ProjectManage (Just caller) projectId + pure $ AuthZ.UnsafeAuthZReceipt Nothing + +checkProjectSubscriptionsManage :: UserId -> ProjectId -> WebApp (Either AuthZFailure AuthZ.AuthZReceipt) +checkProjectSubscriptionsManage caller projectId = maybePermissionFailure (ProjectPermission $ ProjectNotificationSubscriptionsManage projectId) do + assertUserHasProjectPermission AuthZ.ProjectManage (Just caller) projectId + pure $ AuthZ.UnsafeAuthZReceipt Nothing + -- | Check whether the given user has administrative privileges, -- and has a recently created session. This adds additional protection to -- sensitive endpoints. diff --git a/src/Share/Web/Share/Projects/Impl.hs b/src/Share/Web/Share/Projects/Impl.hs index 4faa3b0f..60ef1b80 100644 --- a/src/Share/Web/Share/Projects/Impl.hs +++ b/src/Share/Web/Share/Projects/Impl.hs @@ -445,9 +445,9 @@ projectWebhooksServer session projectUserHandle projectSlug = listProjectWebhooksEndpoint :: Maybe Session -> UserHandle -> ProjectSlug -> WebApp ListProjectWebhooksResponse listProjectWebhooksEndpoint session projectUserHandle projectSlug = do caller <- AuthN.requireAuthenticatedUser session - Project {projectId, ownerUserId = projectOwner} <- PG.runTransactionOrRespondError $ do + Project {projectId} <- PG.runTransactionOrRespondError $ do Q.projectByShortHand projectShortHand `whenNothingM` throwError (EntityMissing (ErrorID "project-not-found") ("Project not found: " <> IDs.toText @IDs.ProjectShortHand projectShortHand)) - _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkSubscriptionsView caller projectOwner + _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkProjectSubscriptionsView caller projectId webhooks <- NotifOps.listProjectWebhooks caller projectId pure $ ListProjectWebhooksResponse {webhooks} where @@ -457,9 +457,9 @@ listProjectWebhooksEndpoint session projectUserHandle projectSlug = do createProjectWebhookEndpoint :: Maybe Session -> UserHandle -> ProjectSlug -> CreateProjectWebhookRequest -> WebApp CreateProjectWebhookResponse createProjectWebhookEndpoint session projectUserHandle projectSlug (CreateProjectWebhookRequest {uri, events}) = do caller <- AuthN.requireAuthenticatedUser session - Project {projectId, ownerUserId = projectOwner} <- PG.runTransactionOrRespondError $ do + Project {projectId} <- PG.runTransactionOrRespondError $ do Q.projectByShortHand projectShortHand `whenNothingM` throwError (EntityMissing (ErrorID "project-not-found") ("Project not found: " <> IDs.toText @IDs.ProjectShortHand projectShortHand)) - _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkSubscriptionsManage caller projectOwner + _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkProjectSubscriptionsManage caller projectId webhook <- NotifOps.createProjectWebhook projectId uri events pure $ CreateProjectWebhookResponse {webhook} where @@ -469,9 +469,9 @@ createProjectWebhookEndpoint session projectUserHandle projectSlug (CreateProjec updateProjectWebhookEndpoint :: Maybe Session -> UserHandle -> ProjectSlug -> IDs.NotificationSubscriptionId -> UpdateProjectWebhookRequest -> WebApp UpdateProjectWebhookResponse updateProjectWebhookEndpoint session projectUserHandle projectSlug subscriptionId (UpdateProjectWebhookRequest {uri = mayURIUpdate, events}) = do caller <- AuthN.requireAuthenticatedUser session - Project {projectId, ownerUserId = projectOwner} <- PG.runTransactionOrRespondError $ do + Project {projectId} <- PG.runTransactionOrRespondError $ do Q.projectByShortHand projectShortHand `whenNothingM` throwError (EntityMissing (ErrorID "project-not-found") ("Project not found: " <> IDs.toText @IDs.ProjectShortHand projectShortHand)) - _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkSubscriptionsManage caller projectOwner + _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkProjectSubscriptionsManage caller projectId let owner = ProjectSubscriptionOwner projectId NotifOps.updateProjectWebhook owner subscriptionId mayURIUpdate events webhook <- NotifOps.expectProjectWebhook projectId subscriptionId @@ -483,9 +483,9 @@ updateProjectWebhookEndpoint session projectUserHandle projectSlug subscriptionI deleteProjectWebhookEndpoint :: Maybe Session -> UserHandle -> ProjectSlug -> IDs.NotificationSubscriptionId -> WebApp () deleteProjectWebhookEndpoint session projectUserHandle projectSlug subscriptionId = do caller <- AuthN.requireAuthenticatedUser session - Project {ownerUserId = projectOwner, projectId} <- PG.runTransactionOrRespondError $ do + Project {projectId} <- PG.runTransactionOrRespondError $ do Q.projectByShortHand projectShortHand `whenNothingM` throwError (EntityMissing (ErrorID "project-not-found") ("Project not found: " <> IDs.toText @IDs.ProjectShortHand projectShortHand)) - _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkSubscriptionsManage caller projectOwner + _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkProjectSubscriptionsManage caller projectId NotifOps.deleteProjectWebhook projectId subscriptionId where projectShortHand :: IDs.ProjectShortHand From 01e77260b054421e9416213ad46121f33ae346a5 Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Wed, 3 Sep 2025 12:56:55 -0700 Subject: [PATCH 17/35] Fix sql typos --- sql/2025-09-03_project-webhooks.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sql/2025-09-03_project-webhooks.sql b/sql/2025-09-03_project-webhooks.sql index 70cb2270..fa23c833 100644 --- a/sql/2025-09-03_project-webhooks.sql +++ b/sql/2025-09-03_project-webhooks.sql @@ -62,7 +62,7 @@ CREATE INDEX notification_events_scope_user_and_project ON notification_events(s -- Migrate existing filters to the new column, and also remove -- the projectId from the JSONB filter. UPDATE notification_subscriptions - SET project_id = (filter->>'projectId')::UUID, + SET scope_project_id = (filter->>'projectId')::UUID, filter = filter - 'projectId' WHERE filter ? 'projectId'; @@ -71,7 +71,7 @@ UPDATE notification_events WHERE data ? 'projectId'; -- Now update the trigger so we use the new projectid columns -CREATE FUNCTION trigger_notification_event_subscriptions() +CREATE OR REPLACE FUNCTION trigger_notification_event_subscriptions() RETURNS TRIGGER AS $$ DECLARE the_subscription_id UUID; From 8fddd20a5abea75fe8b8c1709780ba25a1b61fbc Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Wed, 3 Sep 2025 12:56:55 -0700 Subject: [PATCH 18/35] Fix sql column names --- src/Share/Notifications/Ops.hs | 6 +++--- src/Share/Notifications/Queries.hs | 9 ++++----- src/Share/Web/Share/Projects/Impl.hs | 2 +- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/Share/Notifications/Ops.hs b/src/Share/Notifications/Ops.hs index 207b8fb8..1618bfd5 100644 --- a/src/Share/Notifications/Ops.hs +++ b/src/Share/Notifications/Ops.hs @@ -92,9 +92,9 @@ hydrateEvent hydratedEventPayload = do -- | We provide a wrapper layer on top of notification subscriptions and webhooks -- to make the frontend experience a bit more intuitive. -listProjectWebhooks :: UserId -> ProjectId -> WebApp [ProjectWebhook] -listProjectWebhooks caller projectId = do - projectWebhooks <- PG.runTransaction $ do NotifQ.listProjectWebhooks caller projectId +listProjectWebhooks :: ProjectId -> WebApp [ProjectWebhook] +listProjectWebhooks projectId = do + projectWebhooks <- PG.runTransaction $ do NotifQ.listProjectWebhooks projectId results <- projectWebhooks & asListOf (traversed . _1) %%~ \webhookIds -> diff --git a/src/Share/Notifications/Queries.hs b/src/Share/Notifications/Queries.hs index 8d48f446..d2ddaf8d 100644 --- a/src/Share/Notifications/Queries.hs +++ b/src/Share/Notifications/Queries.hs @@ -522,15 +522,15 @@ isUserSubscribedToWatchProject userId projId = do WHERE ns.subscriber_user_id = #{userId} AND ns.scope_user_id = p.owner_user_id AND ns.topic_groups = ARRAY[#{WatchProject}::notification_topic_group] - AND ns.project_id = #{projId} + AND ns.scope_project_id = #{projId} AND ns.filter = #{filter}::jsonb LIMIT 1 |] -- | We provide a wrapper layer on top of notification subscriptions and webhooks -- to make the frontend experience a bit more intuitive. -listProjectWebhooks :: UserId -> ProjectId -> PG.Transaction e [(NotificationWebhookId, Text, NotificationSubscription NotificationSubscriptionId)] -listProjectWebhooks caller projectId = do +listProjectWebhooks :: ProjectId -> PG.Transaction e [(NotificationWebhookId, Text, NotificationSubscription NotificationSubscriptionId)] +listProjectWebhooks projectId = do PG.queryListRows @((NotificationWebhookId, Text) PG.:. NotificationSubscription NotificationSubscriptionId) [PG.sql| -- Only get one webhook per subscription, there _shouldn't_ be multiple webhooks per @@ -553,8 +553,7 @@ listProjectWebhooks caller projectId = do FROM notification_subscriptions ns JOIN notification_by_webhook nbw ON ns.id = nbw.subscription_id JOIN notification_webhooks nw ON nbw.webhook_id = nw.id - WHERE ns.project_id = #{projectId} - AND ns.subscriber_user_id = #{caller} + WHERE ns.subscriber_project_id = #{projectId} |] <&> fmap (\((webhookId, webhookName) PG.:. subscription) -> (webhookId, webhookName, subscription)) diff --git a/src/Share/Web/Share/Projects/Impl.hs b/src/Share/Web/Share/Projects/Impl.hs index 60ef1b80..41f00da0 100644 --- a/src/Share/Web/Share/Projects/Impl.hs +++ b/src/Share/Web/Share/Projects/Impl.hs @@ -448,7 +448,7 @@ listProjectWebhooksEndpoint session projectUserHandle projectSlug = do Project {projectId} <- PG.runTransactionOrRespondError $ do Q.projectByShortHand projectShortHand `whenNothingM` throwError (EntityMissing (ErrorID "project-not-found") ("Project not found: " <> IDs.toText @IDs.ProjectShortHand projectShortHand)) _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkProjectSubscriptionsView caller projectId - webhooks <- NotifOps.listProjectWebhooks caller projectId + webhooks <- NotifOps.listProjectWebhooks projectId pure $ ListProjectWebhooksResponse {webhooks} where projectShortHand :: IDs.ProjectShortHand From 8ebdc73c2fe563a2b1b4f198f2eea04d82f14462 Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Wed, 3 Sep 2025 13:04:01 -0700 Subject: [PATCH 19/35] Remove unused field --- src/Share/Web/Share/Projects/Types.hs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/Share/Web/Share/Projects/Types.hs b/src/Share/Web/Share/Projects/Types.hs index bf2528ff..d8680ed7 100644 --- a/src/Share/Web/Share/Projects/Types.hs +++ b/src/Share/Web/Share/Projects/Types.hs @@ -441,8 +441,7 @@ instance FromJSON ListProjectWebhooksResponse where data CreateProjectWebhookRequest = CreateProjectWebhookRequest { uri :: URIParam, - events :: Set NotificationTopic, - subscriber :: UserHandle + events :: Set NotificationTopic } deriving stock (Eq, Show) @@ -450,15 +449,13 @@ instance ToJSON CreateProjectWebhookRequest where toJSON CreateProjectWebhookRequest {..} = object [ "uri" .= uri, - "events" .= events, - "subscriber" .= (IDs.toText $ PrefixedID @"@" subscriber) + "events" .= events ] instance FromJSON CreateProjectWebhookRequest where parseJSON = Aeson.withObject "CreateProjectWebhookRequest" $ \o -> do uri <- o .: "uri" events <- o .: "events" - subscriber <- o .: "subscriber" pure CreateProjectWebhookRequest {..} data CreateProjectWebhookResponse = CreateProjectWebhookResponse From 17a14f403fe5b77c5aede7b89b1e7d0741409a1d Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Wed, 3 Sep 2025 13:04:01 -0700 Subject: [PATCH 20/35] Update notifications transcripts --- transcripts/share-apis/notifications/run.zsh | 21 +++----------------- 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/transcripts/share-apis/notifications/run.zsh b/transcripts/share-apis/notifications/run.zsh index ceae0a75..e79e0c7d 100755 --- a/transcripts/share-apis/notifications/run.zsh +++ b/transcripts/share-apis/notifications/run.zsh @@ -11,18 +11,9 @@ subscription_id=$(fetch_data_jq "$test_user" PUT subscribe-to-watch-project '/us "isSubscribed": true }' ) -webhook_id=$(fetch_data_jq "$test_user" POST create-webhook '/users/test/notifications/delivery-methods/webhooks' '.webhookId' "{ - \"url\": \"${echo_server}/good-webhook\", - \"name\": \"Good Webhook\" -}" ) - -failing_webhook_id=$(fetch_data_jq "$test_user" POST create-webhook '/users/test/notifications/delivery-methods/webhooks' '.webhookId' "{ - \"url\": \"${echo_server}/invalid?x-set-response-status-code=500\", - \"name\": \"Bad Webhook\" -}" ) - -fetch "$test_user" POST add-webhook-to-subscription "/users/test/notifications/subscriptions/${subscription_id}/delivery-methods/add" "{ - \"deliveryMethods\": [{\"kind\": \"webhook\", \"id\": \"${webhook_id}\"}, {\"kind\": \"webhook\", \"id\": \"${failing_webhook_id}\"}] +fetch "$test_user" POST add-project-webhook "/users/test/projects/publictestproject/webhooks" "{ + \"uri\": \"${echo_server}/good-webhook\", + \"events\": [\"project:contribution:created\"] }" # Add a subscription within the transcripts user to notifications for some select topics in any project owned by the test @@ -36,12 +27,6 @@ fetch "$transcripts_user" POST create-subscription-for-other-user-project '/user \"topicGroups\": [\"watch_project\"] }" -fetch "$test_user" POST create-email-delivery '/users/test/notifications/delivery-methods/emails' '{ - "email": "me@example.com" -}' - -fetch "$test_user" GET check-delivery-methods '/users/test/notifications/delivery-methods' - # Create a contribution in a public project, which should trigger a notification for both users, but will be omitted # from 'transcripts' notification list since it's a self-notification. fetch "$transcripts_user" POST public-contribution-create '/users/test/projects/publictestproject/contributions' '{ From eb70568ef21792de6dbbafc18af3872b11dc8ac4 Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Wed, 3 Sep 2025 13:04:01 -0700 Subject: [PATCH 21/35] Fix nullability --- sql/2025-09-03_project-webhooks.sql | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sql/2025-09-03_project-webhooks.sql b/sql/2025-09-03_project-webhooks.sql index fa23c833..b7cb32ec 100644 --- a/sql/2025-09-03_project-webhooks.sql +++ b/sql/2025-09-03_project-webhooks.sql @@ -38,8 +38,9 @@ ALTER TABLE notification_subscriptions ADD COLUMN scope_project_id UUID NULL REFERENCES projects(id) ON DELETE CASCADE, -- A subscription can belong to a project itself. ADD COLUMN subscriber_project_id UUID NULL REFERENCES projects(id) ON DELETE CASCADE, - -- Project subscriptions won't have a subscriber_user_id anymore, they're part of the project. - ALTER COLUMN subscriber_user_id SET NOT NULL, + -- Project subscriptions won't have a subscriber_user_id, just a subscriber_project_id. + -- So allow subscriber_user_id to be nullable + ALTER COLUMN subscriber_user_id DROP NOT NULL, -- Add a constraint that either subscriber_user_id or subscriber_project_id must be set, but not both. ADD CONSTRAINT notification_subscriptions_user_or_project CHECK ( (subscriber_user_id IS NOT NULL AND subscriber_project_id IS NULL) From 582d756bc0e8923db3791f3ebdeb6e55a825468e Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Wed, 3 Sep 2025 13:04:01 -0700 Subject: [PATCH 22/35] Fix up bad query --- src/Share/Notifications/Queries.hs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Share/Notifications/Queries.hs b/src/Share/Notifications/Queries.hs index d2ddaf8d..afa4b813 100644 --- a/src/Share/Notifications/Queries.hs +++ b/src/Share/Notifications/Queries.hs @@ -255,7 +255,7 @@ createNotificationSubscription owner subscriptionScope subscriptionProjectId sub UserSubscriptionOwner userId -> (Just userId, Nothing) queryExpect1Col [sql| - INSERT INTO notification_subscriptions (subscriber_user_id, subscriber_project_id, scope_user_id, scope_project_id, topics, topic_groups, filter) + INSERT INTO notification_subscriptions(subscriber_user_id, subscriber_project_id, scope_user_id, scope_project_id, topics, topic_groups, filter) VALUES (#{subscriberUserId}, #{subscriberProjectId}, #{subscriptionScope}, #{subscriptionProjectId}, #{Foldable.toList subscriptionTopics}::notification_topic[], #{Foldable.toList subscriptionTopicGroups}::notification_topic_group[], #{subscriptionFilter}) RETURNING id |] @@ -564,8 +564,9 @@ expectProjectWebhook projectId subscriptionId = do [PG.sql| SELECT nw.id, nw.name FROM notification_webhooks nw + JOIN notification_subscriptions ns ON nw.subscription_id = ns.id WHERE nw.subscription_id = #{subscriptionId} - AND nw.subscriber_project_id = #{projectId} + AND ns.subscriber_project_id = #{projectId} LIMIT 1 |] From fa25b1243e0e70ed7ffac9c8f994ec57ad830ac8 Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Wed, 3 Sep 2025 13:04:01 -0700 Subject: [PATCH 23/35] Fix submitting hub entries on project notification subscriptions --- sql/2025-09-03_project-webhooks.sql | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sql/2025-09-03_project-webhooks.sql b/sql/2025-09-03_project-webhooks.sql index b7cb32ec..5591f32c 100644 --- a/sql/2025-09-03_project-webhooks.sql +++ b/sql/2025-09-03_project-webhooks.sql @@ -124,6 +124,9 @@ BEGIN -- in which case we just carry on. INSERT INTO notification_hub_entries (event_id, user_id) VALUES (the_event_id, the_subscriber) + -- Don't add to the hub if the subscriber is a project, + -- projects don't have a hub. + WHERE the_subscriber IS NOT NULL ON CONFLICT DO NOTHING; END LOOP; From 4522003bd931f8b44150d3b01a3765c3b263bd50 Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Wed, 3 Sep 2025 13:04:01 -0700 Subject: [PATCH 24/35] Fix event data decoding --- src/Share/Notifications/Queries.hs | 2 +- src/Share/Notifications/Types.hs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Share/Notifications/Queries.hs b/src/Share/Notifications/Queries.hs index afa4b813..30356a9f 100644 --- a/src/Share/Notifications/Queries.hs +++ b/src/Share/Notifications/Queries.hs @@ -94,7 +94,7 @@ listNotificationHubEntryPayloads notificationUserId mayLimit mayCursor statusFil query limit cursorFilter = queryListRows @(NotificationHubEntry UserId NotificationEventData) [sql| - SELECT hub.id, hub.status, hub.created_at, event.id, event.occurred_at, event.scope_user_id, event.actor_user_id, event.resource_id, event.topic, event.data + SELECT hub.id, hub.status, hub.created_at, event.id, event.occurred_at, event.scope_user_id, event.actor_user_id, event.resource_id, event.project_id, event.topic, event.data FROM notification_hub_entries hub JOIN notification_events event ON hub.event_id = event.id WHERE hub.user_id = #{notificationUserId} diff --git a/src/Share/Notifications/Types.hs b/src/Share/Notifications/Types.hs index bad3c965..bca1ec8e 100644 --- a/src/Share/Notifications/Types.hs +++ b/src/Share/Notifications/Types.hs @@ -666,7 +666,7 @@ instance Hasql.DecodeRow (NotificationHubEntry UserId NotificationEventData) whe hubEntryId <- PG.decodeField hubEntryStatus <- PG.decodeField hubEntryCreatedAt <- PG.decodeField - hubEntryEvent <- PG.decodeRow + hubEntryEvent <- PG.decodeRow @(NotificationEvent NotificationEventId UserId UTCTime NotificationEventData) pure $ NotificationHubEntry {hubEntryId, hubEntryEvent, hubEntryStatus, hubEntryCreatedAt} hubEntryUserInfo_ :: Traversal (NotificationHubEntry userInfo eventPayload) (NotificationHubEntry userInfo' eventPayload) userInfo userInfo' From cfc3d8408b53bc02902e1e18bea0e413f18ff55a Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Wed, 3 Sep 2025 13:04:01 -0700 Subject: [PATCH 25/35] Fix bad trigger function --- sql/2025-09-03_project-webhooks.sql | 56 +++++++++++++++++------------ 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/sql/2025-09-03_project-webhooks.sql b/sql/2025-09-03_project-webhooks.sql index 5591f32c..3f80e7d0 100644 --- a/sql/2025-09-03_project-webhooks.sql +++ b/sql/2025-09-03_project-webhooks.sql @@ -71,32 +71,33 @@ UPDATE notification_events SET project_id = (data->>'projectId')::UUID WHERE data ? 'projectId'; --- Now update the trigger so we use the new projectid columns +-- Rework the trigger to use the new topic groups. CREATE OR REPLACE FUNCTION trigger_notification_event_subscriptions() RETURNS TRIGGER AS $$ DECLARE the_subscription_id UUID; the_event_id UUID; the_subscriber UUID; + rows_affected INTEGER; BEGIN SELECT NEW.id INTO the_event_id; FOR the_subscription_id, the_subscriber IN (SELECT ns.id, ns.subscriber_user_id FROM notification_subscriptions ns WHERE ns.scope_user_id = NEW.scope_user_id - AND NEW.topic = ANY(ns.topics) - AND (ns.scope_project_id IS NULL OR ns.scope_project_id = NEW.project_id) + AND ( NEW.topic = ANY(ns.topics) + OR EXISTS( + SELECT FROM notification_topic_group_topics ntgt + WHERE ntgt.topic_group = ANY(ns.topic_groups) + AND ntgt.topic = NEW.topic + ) + ) AND (ns.filter IS NULL OR NEW.data @> ns.filter) AND - -- A subscriber can be notified if: - -- * the event is in their own user - -- * or if they have permission to the resource. - -- * or if the subscription is attached to the project of the event. - ((ns.subscriber_user_id IS NOT NULL - AND (NEW.scope_user_id = ns.subscriber_user_id - OR user_has_permission(ns.subscriber_user_id, NEW.resource_id, topic_permission(NEW.topic)) - ) - ) - OR (ns.subscriber_project_id IS NOT NULL AND ns.subscriber_project_id = NEW.project_id) + -- A subscriber can be notified if the event is in their scope or if they have permission to the resource. + -- The latter is usually a superset of the former, but the former is trivial to compute so it can help + -- performance to include it. + (NEW.scope_user_id = ns.subscriber_user_id + OR user_has_permission(ns.subscriber_user_id, NEW.resource_id, topic_permission(NEW.topic)) ) ) LOOP @@ -113,21 +114,32 @@ BEGIN WHERE nw.subscription_id = the_subscription_id ON CONFLICT DO NOTHING; + -- If there are any new webhooks to process, trigger workers via LISTEN/NOTIFY + GET DIAGNOSTICS rows_affected = ROW_COUNT; + IF rows_affected > 0 THEN + NOTIFY webhooks; + END IF; + INSERT INTO notification_email_queue (event_id, email_id) SELECT the_event_id AS event_id, ne.id FROM notification_emails ne WHERE ne.subscription_id = the_subscription_id ON CONFLICT DO NOTHING; - -- Also add the notification to the hub. - -- It's possible it was already added by another subscription for this user, - -- in which case we just carry on. - INSERT INTO notification_hub_entries (event_id, user_id) - VALUES (the_event_id, the_subscriber) - -- Don't add to the hub if the subscriber is a project, - -- projects don't have a hub. - WHERE the_subscriber IS NOT NULL - ON CONFLICT DO NOTHING; + -- If there are any new webhooks to process, trigger workers via LISTEN/NOTIFY + GET DIAGNOSTICS rows_affected = ROW_COUNT; + IF rows_affected > 0 THEN + NOTIFY emails; + END IF; + + IF the_subscriber IS NOT NULL THEN + -- Also add the notification to the hub. + -- It's possible it was already added by another subscription for this user, + -- in which case we just carry on. + INSERT INTO notification_hub_entries (event_id, user_id) + VALUES (the_event_id, the_subscriber) + ON CONFLICT DO NOTHING; + END IF; END LOOP; RETURN NEW; From d2ac1c7a9e96133fae315fd7f26a01016c989dcf Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Wed, 3 Sep 2025 13:04:01 -0700 Subject: [PATCH 26/35] Fix up some transcript calls --- transcripts/share-apis/notifications/run.zsh | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/transcripts/share-apis/notifications/run.zsh b/transcripts/share-apis/notifications/run.zsh index e79e0c7d..f7b1be97 100755 --- a/transcripts/share-apis/notifications/run.zsh +++ b/transcripts/share-apis/notifications/run.zsh @@ -16,16 +16,10 @@ fetch "$test_user" POST add-project-webhook "/users/test/projects/publictestproj \"events\": [\"project:contribution:created\"] }" -# Add a subscription within the transcripts user to notifications for some select topics in any project owned by the test -# user. -# No filter is applied. -fetch "$transcripts_user" POST create-subscription-for-other-user-project '/users/transcripts/notifications/subscriptions' "{ - \"scope\": \"test\", - \"topics\": [ - \"project:branch:updated\", \"project:release:created\" - ], - \"topicGroups\": [\"watch_project\"] -}" +# Subscribe the transcripts user to notifications for a project in the test user. +fetch "$transcripts_user" PUT subscribe-to-other-user-project '/users/test/projects/publictestproject/subscription' '{ + "isSubscribed": true +}' # Create a contribution in a public project, which should trigger a notification for both users, but will be omitted # from 'transcripts' notification list since it's a self-notification. From 3f88ee55fcea448569f3f1de3a47475212c6b91f Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Thu, 4 Sep 2025 15:24:07 -0700 Subject: [PATCH 27/35] Expose different topic selection types in project webhooks --- src/Share/Notifications/Ops.hs | 46 +++++++++++++-------- src/Share/Web/Share/Projects/Impl.hs | 8 ++-- src/Share/Web/Share/Projects/Types.hs | 58 ++++++++++++++++++--------- 3 files changed, 74 insertions(+), 38 deletions(-) diff --git a/src/Share/Notifications/Ops.hs b/src/Share/Notifications/Ops.hs index 1618bfd5..b7f42eff 100644 --- a/src/Share/Notifications/Ops.hs +++ b/src/Share/Notifications/Ops.hs @@ -13,6 +13,8 @@ module Share.Notifications.Ops where import Control.Lens +import Data.Set qualified as Set +import Data.Set.NonEmpty qualified as NESet import Share.App (AppM) import Share.IDs import Share.Notifications.Queries qualified as NotifQ @@ -27,7 +29,7 @@ import Share.Project (Project (..)) import Share.Utils.URI (URIParam (..)) import Share.Web.App (WebApp) import Share.Web.Errors (respondError) -import Share.Web.Share.Projects.Types (ProjectWebhook (..)) +import Share.Web.Share.Projects.Types (ProjectWebhook (..), ProjectWebhookTopics (..)) import Share.Web.UI.Links qualified as Links import UnliftIO qualified @@ -105,19 +107,24 @@ listProjectWebhooks projectId = do Right (WebhookConfig {uri = URIParam uri}) -> do pure $ (NotificationWebhookConfig webhookId uri) let webhooks = - results <&> \(NotificationWebhookConfig {webhookDeliveryUrl = url}, _name, NotificationSubscription {subscriptionTopics, subscriptionId, subscriptionCreatedAt, subscriptionUpdatedAt}) -> - ProjectWebhook - { projectWebhookUri = URIParam url, - projectWebhookEvents = subscriptionTopics, - projectWebhookNotificationSubscriptionId = subscriptionId, - projectWebhookCreatedAt = subscriptionCreatedAt, - projectWebhookUpdatedAt = subscriptionUpdatedAt - } + results <&> \(NotificationWebhookConfig {webhookDeliveryUrl = url}, _name, NotificationSubscription {subscriptionTopics, subscriptionTopicGroups, subscriptionId, subscriptionCreatedAt, subscriptionUpdatedAt}) -> + let webhookTopics = case (Set.toList subscriptionTopicGroups, NESet.nonEmptySet subscriptionTopics) of + ([], Just topics) -> SelectedTopics topics + _ -> AllTopics + in ProjectWebhook + { projectWebhookUri = URIParam url, + projectWebhookTopics = webhookTopics, + projectWebhookNotificationSubscriptionId = subscriptionId, + projectWebhookCreatedAt = subscriptionCreatedAt, + projectWebhookUpdatedAt = subscriptionUpdatedAt + } pure webhooks -createProjectWebhook :: ProjectId -> URIParam -> Set NotificationTopic -> WebApp ProjectWebhook -createProjectWebhook projectId uri topics = do - let topicGroups = mempty +createProjectWebhook :: ProjectId -> URIParam -> ProjectWebhookTopics -> WebApp ProjectWebhook +createProjectWebhook projectId uri webhookTopics = do + let (topics, topicGroups) = case webhookTopics of + AllTopics -> (mempty, Set.singleton WatchProject) + SelectedTopics ts -> (NESet.toSet ts, mempty) let filter = Nothing runInIO <- UnliftIO.askRunInIO subscriptionId <- PG.runTransactionOrRespondError $ do @@ -135,10 +142,13 @@ expectProjectWebhook projectId subscriptionId = do Left err -> respondError err Right (WebhookConfig {uri}) -> pure uri subscription <- PG.runTransaction $ do NotifQ.expectNotificationSubscription (ProjectSubscriptionOwner projectId) subscriptionId + let subscriptionTopics = case (Set.toList $ subscription.subscriptionTopicGroups, NESet.nonEmptySet subscription.subscriptionTopics) of + ([], Just topics) -> SelectedTopics topics + _ -> AllTopics pure $ ProjectWebhook { projectWebhookUri = uri, - projectWebhookEvents = subscription.subscriptionTopics, + projectWebhookTopics = subscriptionTopics, projectWebhookNotificationSubscriptionId = subscription.subscriptionId, projectWebhookCreatedAt = subscription.subscriptionCreatedAt, projectWebhookUpdatedAt = subscription.subscriptionUpdatedAt @@ -153,8 +163,8 @@ deleteProjectWebhook projectId subscriptionId = do deleteWebhookDeliveryMethod owner webhookId PG.runTransaction $ do NotifQ.deleteNotificationSubscription owner subscriptionId -updateProjectWebhook :: SubscriptionOwner -> NotificationSubscriptionId -> Maybe URIParam -> (Maybe (Set NotificationTopic)) -> WebApp () -updateProjectWebhook subscriptionOwner subscriptionId mayURIUpdate topics = do +updateProjectWebhook :: SubscriptionOwner -> NotificationSubscriptionId -> Maybe URIParam -> (Maybe ProjectWebhookTopics) -> WebApp () +updateProjectWebhook subscriptionOwner subscriptionId mayURIUpdate webhookTopics = do for_ mayURIUpdate \uri -> do -- First fetch the webhook ids associated with this subscription webhooks <- PG.runTransaction $ do NotifQ.webhooksForSubscription subscriptionId @@ -163,7 +173,11 @@ updateProjectWebhook subscriptionOwner subscriptionId mayURIUpdate topics = do WebhookSecrets.putWebhookConfig webhookId (WebhookConfig uri) >>= \case Left err -> respondError err Right _ -> pure () - PG.runTransaction $ NotifQ.updateNotificationSubscription subscriptionOwner subscriptionId topics mempty Nothing + let (topics, topicGroups) = case webhookTopics of + Nothing -> (Nothing, Nothing) + Just AllTopics -> (Just mempty, Just $ Set.singleton WatchProject) + Just (SelectedTopics ts) -> (Just $ NESet.toSet ts, Just $ mempty) + PG.runTransaction $ NotifQ.updateNotificationSubscription subscriptionOwner subscriptionId topics topicGroups Nothing updateWebhookDeliveryMethod :: UserId -> NotificationWebhookId -> URIParam -> WebApp () updateWebhookDeliveryMethod notificationUser webhookDeliveryMethodId url = do diff --git a/src/Share/Web/Share/Projects/Impl.hs b/src/Share/Web/Share/Projects/Impl.hs index 41f00da0..9da2a4ef 100644 --- a/src/Share/Web/Share/Projects/Impl.hs +++ b/src/Share/Web/Share/Projects/Impl.hs @@ -455,25 +455,25 @@ listProjectWebhooksEndpoint session projectUserHandle projectSlug = do projectShortHand = IDs.ProjectShortHand {userHandle = projectUserHandle, projectSlug} createProjectWebhookEndpoint :: Maybe Session -> UserHandle -> ProjectSlug -> CreateProjectWebhookRequest -> WebApp CreateProjectWebhookResponse -createProjectWebhookEndpoint session projectUserHandle projectSlug (CreateProjectWebhookRequest {uri, events}) = do +createProjectWebhookEndpoint session projectUserHandle projectSlug (CreateProjectWebhookRequest {uri, topics}) = do caller <- AuthN.requireAuthenticatedUser session Project {projectId} <- PG.runTransactionOrRespondError $ do Q.projectByShortHand projectShortHand `whenNothingM` throwError (EntityMissing (ErrorID "project-not-found") ("Project not found: " <> IDs.toText @IDs.ProjectShortHand projectShortHand)) _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkProjectSubscriptionsManage caller projectId - webhook <- NotifOps.createProjectWebhook projectId uri events + webhook <- NotifOps.createProjectWebhook projectId uri topics pure $ CreateProjectWebhookResponse {webhook} where projectShortHand :: IDs.ProjectShortHand projectShortHand = IDs.ProjectShortHand {userHandle = projectUserHandle, projectSlug} updateProjectWebhookEndpoint :: Maybe Session -> UserHandle -> ProjectSlug -> IDs.NotificationSubscriptionId -> UpdateProjectWebhookRequest -> WebApp UpdateProjectWebhookResponse -updateProjectWebhookEndpoint session projectUserHandle projectSlug subscriptionId (UpdateProjectWebhookRequest {uri = mayURIUpdate, events}) = do +updateProjectWebhookEndpoint session projectUserHandle projectSlug subscriptionId (UpdateProjectWebhookRequest {uri = mayURIUpdate, topics}) = do caller <- AuthN.requireAuthenticatedUser session Project {projectId} <- PG.runTransactionOrRespondError $ do Q.projectByShortHand projectShortHand `whenNothingM` throwError (EntityMissing (ErrorID "project-not-found") ("Project not found: " <> IDs.toText @IDs.ProjectShortHand projectShortHand)) _authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkProjectSubscriptionsManage caller projectId let owner = ProjectSubscriptionOwner projectId - NotifOps.updateProjectWebhook owner subscriptionId mayURIUpdate events + NotifOps.updateProjectWebhook owner subscriptionId mayURIUpdate topics webhook <- NotifOps.expectProjectWebhook projectId subscriptionId pure $ UpdateProjectWebhookResponse {webhook} where diff --git a/src/Share/Web/Share/Projects/Types.hs b/src/Share/Web/Share/Projects/Types.hs index d8680ed7..ee0fc2d0 100644 --- a/src/Share/Web/Share/Projects/Types.hs +++ b/src/Share/Web/Share/Projects/Types.hs @@ -32,12 +32,15 @@ module Share.Web.Share.Projects.Types CreateProjectWebhookResponse (..), UpdateProjectWebhookRequest (..), UpdateProjectWebhookResponse (..), + ProjectWebhookTopics (..), ) where import Data.Aeson import Data.Aeson qualified as Aeson import Data.Set qualified as Set +import Data.Set.NonEmpty (NESet) +import Data.Set.NonEmpty qualified as NESet import Data.Time (UTCTime) import Share.IDs import Share.IDs qualified as IDs @@ -385,30 +388,49 @@ instance Aeson.ToJSON CatalogCategory where "projects" .= projects ] +data ProjectWebhookTopics + = SelectedTopics (NESet NotificationTopic) + | AllTopics + deriving stock (Eq, Show) + +instance ToJSON ProjectWebhookTopics where + toJSON (SelectedTopics topics) = + object + [ "type" .= ("selected" :: Text), + "topics" .= topics + ] + toJSON AllTopics = + object + [ "type" .= ("all" :: Text) + ] + +instance FromJSON ProjectWebhookTopics where + parseJSON = Aeson.withObject "ProjectWebhookTopics" $ \o -> do + typ <- o .: "type" + case typ of + ("selected" :: Text) -> do + topics <- Set.fromList <$> (o .: "topics") + case NESet.nonEmptySet topics of + Nothing -> fail "SelectedTopics must have at least one topic" + Just neset -> pure $ SelectedTopics neset + ("all" :: Text) -> pure AllTopics + _ -> fail $ "Unknown ProjectWebhookTopics type: " <> show typ + -- | This type provides a view over the subscription <-> webhook many-to-many view. data ProjectWebhook = ProjectWebhook { projectWebhookUri :: URIParam, - projectWebhookEvents :: Set NotificationTopic, + projectWebhookTopics :: ProjectWebhookTopics, projectWebhookNotificationSubscriptionId :: NotificationSubscriptionId, projectWebhookCreatedAt :: Maybe UTCTime, projectWebhookUpdatedAt :: Maybe UTCTime } deriving stock (Eq, Show) -instance PG.DecodeRow ProjectWebhook where - decodeRow = do - projectWebhookUri <- PG.decodeField - projectWebhookEvents <- Set.fromList <$> PG.decodeField - projectWebhookNotificationSubscriptionId <- PG.decodeField - projectWebhookCreatedAt <- PG.decodeField - projectWebhookUpdatedAt <- PG.decodeField - pure ProjectWebhook {..} - instance ToJSON ProjectWebhook where toJSON ProjectWebhook {..} = object [ "uri" .= projectWebhookUri, - "events" .= projectWebhookEvents, + "topics" .= projectWebhookTopics, "notificationSubscriptionId" .= projectWebhookNotificationSubscriptionId, "createdAt" .= projectWebhookCreatedAt, "updatedAt" .= projectWebhookUpdatedAt @@ -417,7 +439,7 @@ instance ToJSON ProjectWebhook where instance FromJSON ProjectWebhook where parseJSON = Aeson.withObject "ProjectWebhook" $ \o -> do projectWebhookUri <- o .: "uri" - projectWebhookEvents <- o .: "events" + projectWebhookTopics <- o .: "topics" projectWebhookNotificationSubscriptionId <- o .: "notificationSubscriptionId" projectWebhookCreatedAt <- o .: "createdAt" projectWebhookUpdatedAt <- o .: "updatedAt" @@ -441,7 +463,7 @@ instance FromJSON ListProjectWebhooksResponse where data CreateProjectWebhookRequest = CreateProjectWebhookRequest { uri :: URIParam, - events :: Set NotificationTopic + topics :: ProjectWebhookTopics } deriving stock (Eq, Show) @@ -449,13 +471,13 @@ instance ToJSON CreateProjectWebhookRequest where toJSON CreateProjectWebhookRequest {..} = object [ "uri" .= uri, - "events" .= events + "topics" .= topics ] instance FromJSON CreateProjectWebhookRequest where parseJSON = Aeson.withObject "CreateProjectWebhookRequest" $ \o -> do uri <- o .: "uri" - events <- o .: "events" + topics <- o .: "topics" pure CreateProjectWebhookRequest {..} data CreateProjectWebhookResponse = CreateProjectWebhookResponse @@ -476,7 +498,7 @@ instance FromJSON CreateProjectWebhookResponse where data UpdateProjectWebhookRequest = UpdateProjectWebhookRequest { uri :: Maybe URIParam, - events :: Maybe (Set NotificationTopic) + topics :: Maybe ProjectWebhookTopics } deriving stock (Eq, Show) @@ -484,13 +506,13 @@ instance ToJSON UpdateProjectWebhookRequest where toJSON UpdateProjectWebhookRequest {..} = object [ "uri" .= uri, - "events" .= events + "topics" .= topics ] instance FromJSON UpdateProjectWebhookRequest where parseJSON = Aeson.withObject "UpdateProjectWebhookRequest" $ \o -> do uri <- o .:? "uri" - events <- o .:? "events" + topics <- o .:? "topics" pure UpdateProjectWebhookRequest {..} data UpdateProjectWebhookResponse = UpdateProjectWebhookResponse From 5e27b4dc0524ca9643aa876837532d1639914320 Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Thu, 4 Sep 2025 15:26:33 -0700 Subject: [PATCH 28/35] Update transcripts to subscribe webhook to all events --- src/Share/Notifications/Ops.hs | 8 ++++---- src/Share/Web/Share/Projects/Types.hs | 6 +++--- transcripts/share-apis/notifications/run.zsh | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Share/Notifications/Ops.hs b/src/Share/Notifications/Ops.hs index b7f42eff..c2304d37 100644 --- a/src/Share/Notifications/Ops.hs +++ b/src/Share/Notifications/Ops.hs @@ -110,7 +110,7 @@ listProjectWebhooks projectId = do results <&> \(NotificationWebhookConfig {webhookDeliveryUrl = url}, _name, NotificationSubscription {subscriptionTopics, subscriptionTopicGroups, subscriptionId, subscriptionCreatedAt, subscriptionUpdatedAt}) -> let webhookTopics = case (Set.toList subscriptionTopicGroups, NESet.nonEmptySet subscriptionTopics) of ([], Just topics) -> SelectedTopics topics - _ -> AllTopics + _ -> AllProjectTopics in ProjectWebhook { projectWebhookUri = URIParam url, projectWebhookTopics = webhookTopics, @@ -123,7 +123,7 @@ listProjectWebhooks projectId = do createProjectWebhook :: ProjectId -> URIParam -> ProjectWebhookTopics -> WebApp ProjectWebhook createProjectWebhook projectId uri webhookTopics = do let (topics, topicGroups) = case webhookTopics of - AllTopics -> (mempty, Set.singleton WatchProject) + AllProjectTopics -> (mempty, Set.singleton WatchProject) SelectedTopics ts -> (NESet.toSet ts, mempty) let filter = Nothing runInIO <- UnliftIO.askRunInIO @@ -144,7 +144,7 @@ expectProjectWebhook projectId subscriptionId = do subscription <- PG.runTransaction $ do NotifQ.expectNotificationSubscription (ProjectSubscriptionOwner projectId) subscriptionId let subscriptionTopics = case (Set.toList $ subscription.subscriptionTopicGroups, NESet.nonEmptySet subscription.subscriptionTopics) of ([], Just topics) -> SelectedTopics topics - _ -> AllTopics + _ -> AllProjectTopics pure $ ProjectWebhook { projectWebhookUri = uri, @@ -175,7 +175,7 @@ updateProjectWebhook subscriptionOwner subscriptionId mayURIUpdate webhookTopics Right _ -> pure () let (topics, topicGroups) = case webhookTopics of Nothing -> (Nothing, Nothing) - Just AllTopics -> (Just mempty, Just $ Set.singleton WatchProject) + Just AllProjectTopics -> (Just mempty, Just $ Set.singleton WatchProject) Just (SelectedTopics ts) -> (Just $ NESet.toSet ts, Just $ mempty) PG.runTransaction $ NotifQ.updateNotificationSubscription subscriptionOwner subscriptionId topics topicGroups Nothing diff --git a/src/Share/Web/Share/Projects/Types.hs b/src/Share/Web/Share/Projects/Types.hs index ee0fc2d0..56acdb32 100644 --- a/src/Share/Web/Share/Projects/Types.hs +++ b/src/Share/Web/Share/Projects/Types.hs @@ -390,7 +390,7 @@ instance Aeson.ToJSON CatalogCategory where data ProjectWebhookTopics = SelectedTopics (NESet NotificationTopic) - | AllTopics + | AllProjectTopics deriving stock (Eq, Show) instance ToJSON ProjectWebhookTopics where @@ -399,7 +399,7 @@ instance ToJSON ProjectWebhookTopics where [ "type" .= ("selected" :: Text), "topics" .= topics ] - toJSON AllTopics = + toJSON AllProjectTopics = object [ "type" .= ("all" :: Text) ] @@ -413,7 +413,7 @@ instance FromJSON ProjectWebhookTopics where case NESet.nonEmptySet topics of Nothing -> fail "SelectedTopics must have at least one topic" Just neset -> pure $ SelectedTopics neset - ("all" :: Text) -> pure AllTopics + ("all" :: Text) -> pure AllProjectTopics _ -> fail $ "Unknown ProjectWebhookTopics type: " <> show typ -- | This type provides a view over the subscription <-> webhook many-to-many view. diff --git a/transcripts/share-apis/notifications/run.zsh b/transcripts/share-apis/notifications/run.zsh index f7b1be97..30da9cfa 100755 --- a/transcripts/share-apis/notifications/run.zsh +++ b/transcripts/share-apis/notifications/run.zsh @@ -13,7 +13,7 @@ subscription_id=$(fetch_data_jq "$test_user" PUT subscribe-to-watch-project '/us fetch "$test_user" POST add-project-webhook "/users/test/projects/publictestproject/webhooks" "{ \"uri\": \"${echo_server}/good-webhook\", - \"events\": [\"project:contribution:created\"] + \"topics\": {\"type\": \"all\"} }" # Subscribe the transcripts user to notifications for a project in the test user. From 2c457b1df3568ab76592f3f9f342f08e455b83f8 Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Thu, 4 Sep 2025 15:28:01 -0700 Subject: [PATCH 29/35] Update expected bits of transcripts --- .../notifications/add-project-webhook.json | 18 +++++++++++ .../add-webhook-to-subscription.json | 8 ----- .../notifications/check-delivery-methods.json | 32 ------------------- ...e-subscription-for-other-user-project.json | 21 ------------ .../list-notifications-test-paging.json | 1 + .../list-notifications-test.json | 4 +++ .../list-notifications-transcripts.json | 2 ++ .../list-notifications-unread-test.json | 3 ++ ...n => subscribe-to-other-user-project.json} | 2 +- 9 files changed, 29 insertions(+), 62 deletions(-) create mode 100644 transcripts/share-apis/notifications/add-project-webhook.json delete mode 100644 transcripts/share-apis/notifications/add-webhook-to-subscription.json delete mode 100644 transcripts/share-apis/notifications/check-delivery-methods.json delete mode 100644 transcripts/share-apis/notifications/create-subscription-for-other-user-project.json rename transcripts/share-apis/notifications/{create-email-delivery.json => subscribe-to-other-user-project.json} (64%) diff --git a/transcripts/share-apis/notifications/add-project-webhook.json b/transcripts/share-apis/notifications/add-project-webhook.json new file mode 100644 index 00000000..8e10ed6c --- /dev/null +++ b/transcripts/share-apis/notifications/add-project-webhook.json @@ -0,0 +1,18 @@ +{ + "body": { + "webhook": { + "createdAt": "", + "notificationSubscriptionId": "NS-", + "topics": { + "type": "all" + }, + "updatedAt": "", + "uri": "http://:9999/good-webhook" + } + }, + "status": [ + { + "status_code": 200 + } + ] +} diff --git a/transcripts/share-apis/notifications/add-webhook-to-subscription.json b/transcripts/share-apis/notifications/add-webhook-to-subscription.json deleted file mode 100644 index 5af94a8f..00000000 --- a/transcripts/share-apis/notifications/add-webhook-to-subscription.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "body": [], - "status": [ - { - "status_code": 200 - } - ] -} diff --git a/transcripts/share-apis/notifications/check-delivery-methods.json b/transcripts/share-apis/notifications/check-delivery-methods.json deleted file mode 100644 index 1fba7391..00000000 --- a/transcripts/share-apis/notifications/check-delivery-methods.json +++ /dev/null @@ -1,32 +0,0 @@ -{ - "body": { - "deliveryMethods": [ - { - "config": { - "email": "me@example.com", - "id": "NE-" - }, - "kind": "email" - }, - { - "config": { - "id": "NW-", - "url": "http://:9999/good-webhook" - }, - "kind": "webhook" - }, - { - "config": { - "id": "NW-", - "url": "http://:9999/invalid?x-set-response-status-code=500" - }, - "kind": "webhook" - } - ] - }, - "status": [ - { - "status_code": 200 - } - ] -} diff --git a/transcripts/share-apis/notifications/create-subscription-for-other-user-project.json b/transcripts/share-apis/notifications/create-subscription-for-other-user-project.json deleted file mode 100644 index baf498ad..00000000 --- a/transcripts/share-apis/notifications/create-subscription-for-other-user-project.json +++ /dev/null @@ -1,21 +0,0 @@ -{ - "body": { - "subscription": { - "filter": null, - "id": "NS-", - "scope": "U-", - "topicGroups": [ - "watch_project" - ], - "topics": [ - "project:branch:updated", - "project:release:created" - ] - } - }, - "status": [ - { - "status_code": 200 - } - ] -} diff --git a/transcripts/share-apis/notifications/list-notifications-test-paging.json b/transcripts/share-apis/notifications/list-notifications-test-paging.json index ca9938cd..c3de7b33 100644 --- a/transcripts/share-apis/notifications/list-notifications-test-paging.json +++ b/transcripts/share-apis/notifications/list-notifications-test-paging.json @@ -53,6 +53,7 @@ }, "id": "EVENT-", "occurredAt": "", + "projectId": "P-", "resourceId": "RES-", "scope": { "info": { diff --git a/transcripts/share-apis/notifications/list-notifications-test.json b/transcripts/share-apis/notifications/list-notifications-test.json index 3f5d159a..7a004e97 100644 --- a/transcripts/share-apis/notifications/list-notifications-test.json +++ b/transcripts/share-apis/notifications/list-notifications-test.json @@ -53,6 +53,7 @@ }, "id": "EVENT-", "occurredAt": "", + "projectId": "P-", "resourceId": "RES-", "scope": { "info": { @@ -108,6 +109,7 @@ }, "id": "EVENT-", "occurredAt": "", + "projectId": "P-", "resourceId": "RES-", "scope": { "info": { @@ -189,6 +191,7 @@ }, "id": "EVENT-", "occurredAt": "", + "projectId": "P-", "resourceId": "RES-", "scope": { "info": { @@ -259,6 +262,7 @@ }, "id": "EVENT-", "occurredAt": "", + "projectId": "P-", "resourceId": "RES-", "scope": { "info": { diff --git a/transcripts/share-apis/notifications/list-notifications-transcripts.json b/transcripts/share-apis/notifications/list-notifications-transcripts.json index f94e8cf8..ca40aac6 100644 --- a/transcripts/share-apis/notifications/list-notifications-transcripts.json +++ b/transcripts/share-apis/notifications/list-notifications-transcripts.json @@ -110,6 +110,7 @@ }, "id": "EVENT-", "occurredAt": "", + "projectId": "P-", "resourceId": "RES-", "scope": { "info": { @@ -156,6 +157,7 @@ }, "id": "EVENT-", "occurredAt": "", + "projectId": "P-", "resourceId": "RES-", "scope": { "info": { diff --git a/transcripts/share-apis/notifications/list-notifications-unread-test.json b/transcripts/share-apis/notifications/list-notifications-unread-test.json index 11149940..69d11d86 100644 --- a/transcripts/share-apis/notifications/list-notifications-unread-test.json +++ b/transcripts/share-apis/notifications/list-notifications-unread-test.json @@ -42,6 +42,7 @@ }, "id": "EVENT-", "occurredAt": "", + "projectId": "P-", "resourceId": "RES-", "scope": { "info": { @@ -123,6 +124,7 @@ }, "id": "EVENT-", "occurredAt": "", + "projectId": "P-", "resourceId": "RES-", "scope": { "info": { @@ -193,6 +195,7 @@ }, "id": "EVENT-", "occurredAt": "", + "projectId": "P-", "resourceId": "RES-", "scope": { "info": { diff --git a/transcripts/share-apis/notifications/create-email-delivery.json b/transcripts/share-apis/notifications/subscribe-to-other-user-project.json similarity index 64% rename from transcripts/share-apis/notifications/create-email-delivery.json rename to transcripts/share-apis/notifications/subscribe-to-other-user-project.json index ee91e366..c2b44bbf 100644 --- a/transcripts/share-apis/notifications/create-email-delivery.json +++ b/transcripts/share-apis/notifications/subscribe-to-other-user-project.json @@ -1,6 +1,6 @@ { "body": { - "emailDeliveryMethodId": "NE-" + "subscriptionId": "NS-" }, "status": [ { From 8e32fe82a4c2c9fa5bbe703d921867f2f92d34c3 Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Thu, 4 Sep 2025 15:28:01 -0700 Subject: [PATCH 30/35] Remove deleted endpoints from transcripts --- ...-subscriptions-test-after-unsubscribe.json | 10 --------- .../list-subscriptions-test.json | 22 ------------------- transcripts/share-apis/notifications/run.zsh | 6 ----- 3 files changed, 38 deletions(-) delete mode 100644 transcripts/share-apis/notifications/list-subscriptions-test-after-unsubscribe.json delete mode 100644 transcripts/share-apis/notifications/list-subscriptions-test.json diff --git a/transcripts/share-apis/notifications/list-subscriptions-test-after-unsubscribe.json b/transcripts/share-apis/notifications/list-subscriptions-test-after-unsubscribe.json deleted file mode 100644 index 52126e2e..00000000 --- a/transcripts/share-apis/notifications/list-subscriptions-test-after-unsubscribe.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "body": { - "subscriptions": [] - }, - "status": [ - { - "status_code": 200 - } - ] -} diff --git a/transcripts/share-apis/notifications/list-subscriptions-test.json b/transcripts/share-apis/notifications/list-subscriptions-test.json deleted file mode 100644 index 9d0ccef0..00000000 --- a/transcripts/share-apis/notifications/list-subscriptions-test.json +++ /dev/null @@ -1,22 +0,0 @@ -{ - "body": { - "subscriptions": [ - { - "filter": { - "projectId": "P-" - }, - "id": "NS-", - "scope": "U-", - "topicGroups": [ - "watch_project" - ], - "topics": [] - } - ] - }, - "status": [ - { - "status_code": 200 - } - ] -} diff --git a/transcripts/share-apis/notifications/run.zsh b/transcripts/share-apis/notifications/run.zsh index 30da9cfa..44a205a8 100755 --- a/transcripts/share-apis/notifications/run.zsh +++ b/transcripts/share-apis/notifications/run.zsh @@ -128,13 +128,7 @@ unsuccessful_webhooks=$(pg_sql "SELECT COUNT(*) FROM notification_webhook_queue echo "Successful webhooks: $successful_webhooks\nUnsuccessful webhooks: $unsuccessful_webhooks\n" > ./webhook_results.txt -# List 'test' user's subscriptions -fetch "$test_user" GET list-subscriptions-test '/users/test/notifications/subscriptions' - # Can unsubscribe from project-related notifications for the test user's publictestproject. fetch "$test_user" PUT unsubscribe-from-project '/users/test/projects/publictestproject/subscription' '{ "isSubscribed": false }' - -# List 'test' user's subscriptions again -fetch "$test_user" GET list-subscriptions-test-after-unsubscribe '/users/test/notifications/subscriptions' From 64dc4b55512ac3a7f367b86e24e8a1ea70f7ed84 Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Thu, 4 Sep 2025 15:28:01 -0700 Subject: [PATCH 31/35] Add new topic group for all project activity --- sql/2025-09-03_project-webhooks.sql | 13 +++++++++++++ src/Share/Notifications/Ops.hs | 8 ++++---- src/Share/Notifications/Types.hs | 4 ++++ src/Share/Web/Share/Projects/Types.hs | 6 +++--- 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/sql/2025-09-03_project-webhooks.sql b/sql/2025-09-03_project-webhooks.sql index 3f80e7d0..fa7221df 100644 --- a/sql/2025-09-03_project-webhooks.sql +++ b/sql/2025-09-03_project-webhooks.sql @@ -1,6 +1,19 @@ -- This changes webhooks such that each webhook and email delivery method is assigned to exactly one subscription, rather than the -- previous many-to-many relationship. +-- Add a new topic group which represents _all_ activity in a project. +ALTER TYPE notification_topic_group ADD VALUE IF NOT EXISTS 'all_project_topics'; + +INSERT INTO notification_topic_group_topics (topic_group, topic) + VALUES ('all_project_topics', 'project:branch:updated'), + ('all_project_topics', 'project:contribution:created'), + ('all_project_topics', 'project:contribution:updated'), + ('all_project_topics', 'project:contribution:comment'), + ('all_project_topics', 'project:ticket:created'), + ('all_project_topics', 'project:ticket:updated'), + ('all_project_topics', 'project:ticket:comment'), + ('all_project_topics', 'project:release:created'); + -- WEBHOOKS ALTER TABLE notification_webhooks ADD COLUMN subscription_id UUID NULL REFERENCES notification_subscriptions(id) ON DELETE CASCADE, diff --git a/src/Share/Notifications/Ops.hs b/src/Share/Notifications/Ops.hs index c2304d37..0b199eff 100644 --- a/src/Share/Notifications/Ops.hs +++ b/src/Share/Notifications/Ops.hs @@ -110,7 +110,7 @@ listProjectWebhooks projectId = do results <&> \(NotificationWebhookConfig {webhookDeliveryUrl = url}, _name, NotificationSubscription {subscriptionTopics, subscriptionTopicGroups, subscriptionId, subscriptionCreatedAt, subscriptionUpdatedAt}) -> let webhookTopics = case (Set.toList subscriptionTopicGroups, NESet.nonEmptySet subscriptionTopics) of ([], Just topics) -> SelectedTopics topics - _ -> AllProjectTopics + _ -> AllTopicsInProject in ProjectWebhook { projectWebhookUri = URIParam url, projectWebhookTopics = webhookTopics, @@ -123,7 +123,7 @@ listProjectWebhooks projectId = do createProjectWebhook :: ProjectId -> URIParam -> ProjectWebhookTopics -> WebApp ProjectWebhook createProjectWebhook projectId uri webhookTopics = do let (topics, topicGroups) = case webhookTopics of - AllProjectTopics -> (mempty, Set.singleton WatchProject) + AllTopicsInProject -> (mempty, Set.singleton AllProjectTopics) SelectedTopics ts -> (NESet.toSet ts, mempty) let filter = Nothing runInIO <- UnliftIO.askRunInIO @@ -144,7 +144,7 @@ expectProjectWebhook projectId subscriptionId = do subscription <- PG.runTransaction $ do NotifQ.expectNotificationSubscription (ProjectSubscriptionOwner projectId) subscriptionId let subscriptionTopics = case (Set.toList $ subscription.subscriptionTopicGroups, NESet.nonEmptySet subscription.subscriptionTopics) of ([], Just topics) -> SelectedTopics topics - _ -> AllProjectTopics + _ -> AllTopicsInProject pure $ ProjectWebhook { projectWebhookUri = uri, @@ -175,7 +175,7 @@ updateProjectWebhook subscriptionOwner subscriptionId mayURIUpdate webhookTopics Right _ -> pure () let (topics, topicGroups) = case webhookTopics of Nothing -> (Nothing, Nothing) - Just AllProjectTopics -> (Just mempty, Just $ Set.singleton WatchProject) + Just AllTopicsInProject -> (Just mempty, Just $ Set.singleton AllProjectTopics) Just (SelectedTopics ts) -> (Just $ NESet.toSet ts, Just $ mempty) PG.runTransaction $ NotifQ.updateNotificationSubscription subscriptionOwner subscriptionId topics topicGroups Nothing diff --git a/src/Share/Notifications/Types.hs b/src/Share/Notifications/Types.hs index bca1ec8e..9dd9fc36 100644 --- a/src/Share/Notifications/Types.hs +++ b/src/Share/Notifications/Types.hs @@ -121,11 +121,13 @@ instance Aeson.FromJSON NotificationTopic where data NotificationTopicGroup = WatchProject + | AllProjectTopics deriving (Eq, Show, Ord) instance PG.EncodeValue NotificationTopicGroup where encodeValue = HasqlEncoders.enum \case WatchProject -> "watch_project" + AllProjectTopics -> "all_project_topics" instance PG.DecodeValue NotificationTopicGroup where decodeValue = HasqlDecoders.enum \case @@ -135,10 +137,12 @@ instance PG.DecodeValue NotificationTopicGroup where instance Aeson.ToJSON NotificationTopicGroup where toJSON = \case WatchProject -> "watch_project" + AllProjectTopics -> "all_project_topics" instance Aeson.FromJSON NotificationTopicGroup where parseJSON = Aeson.withText "NotificationTopicGroup" \case "watch_project" -> pure WatchProject + "all_project_topics" -> pure AllProjectTopics s -> fail $ "Invalid notification topic group: " <> Text.unpack s data NotificationStatus diff --git a/src/Share/Web/Share/Projects/Types.hs b/src/Share/Web/Share/Projects/Types.hs index 56acdb32..0d5cdabd 100644 --- a/src/Share/Web/Share/Projects/Types.hs +++ b/src/Share/Web/Share/Projects/Types.hs @@ -390,7 +390,7 @@ instance Aeson.ToJSON CatalogCategory where data ProjectWebhookTopics = SelectedTopics (NESet NotificationTopic) - | AllProjectTopics + | AllTopicsInProject deriving stock (Eq, Show) instance ToJSON ProjectWebhookTopics where @@ -399,7 +399,7 @@ instance ToJSON ProjectWebhookTopics where [ "type" .= ("selected" :: Text), "topics" .= topics ] - toJSON AllProjectTopics = + toJSON AllTopicsInProject = object [ "type" .= ("all" :: Text) ] @@ -413,7 +413,7 @@ instance FromJSON ProjectWebhookTopics where case NESet.nonEmptySet topics of Nothing -> fail "SelectedTopics must have at least one topic" Just neset -> pure $ SelectedTopics neset - ("all" :: Text) -> pure AllProjectTopics + ("all" :: Text) -> pure AllTopicsInProject _ -> fail $ "Unknown ProjectWebhookTopics type: " <> show typ -- | This type provides a view over the subscription <-> webhook many-to-many view. From 18505b9c048e5768ea26c17693bcb95a299cd028 Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Thu, 4 Sep 2025 15:28:01 -0700 Subject: [PATCH 32/35] Update transcripts --- src/Share/Notifications/Types.hs | 1 + .../list-notifications-read-transcripts.json | 44 +++++++++++++---- .../list-notifications-transcripts.json | 49 ------------------- .../notifications/webhook_results.txt | 4 +- 4 files changed, 38 insertions(+), 60 deletions(-) diff --git a/src/Share/Notifications/Types.hs b/src/Share/Notifications/Types.hs index 9dd9fc36..564a9a1c 100644 --- a/src/Share/Notifications/Types.hs +++ b/src/Share/Notifications/Types.hs @@ -132,6 +132,7 @@ instance PG.EncodeValue NotificationTopicGroup where instance PG.DecodeValue NotificationTopicGroup where decodeValue = HasqlDecoders.enum \case "watch_project" -> Just WatchProject + "all_project_topics" -> Just AllProjectTopics _ -> Nothing instance Aeson.ToJSON NotificationTopicGroup where diff --git a/transcripts/share-apis/notifications/list-notifications-read-transcripts.json b/transcripts/share-apis/notifications/list-notifications-read-transcripts.json index ad33a97e..18a095cf 100644 --- a/transcripts/share-apis/notifications/list-notifications-read-transcripts.json +++ b/transcripts/share-apis/notifications/list-notifications-read-transcripts.json @@ -14,16 +14,37 @@ "kind": "user" }, "data": { - "kind": "project:branch:updated", - "link": "http://:1234/@test/publictestproject/code/newbranch/latest", + "kind": "project:contribution:updated", + "link": "http://:1234/@test/publictestproject/contributions/1", "payload": { - "branch": { - "branchContributorHandle": null, - "branchContributorUserId": null, - "branchId": "B-", - "branchName": "newbranch", - "branchShortHand": "newbranch", - "projectBranchShortHand": "@test/publictestproject/newbranch" + "contribution": { + "author": { + "avatarUrl": null, + "handle": "test", + "name": null, + "userId": "U-" + }, + "contributionId": "C-", + "description": "This contribution addresses an issue where users were unable to log in due to a validation error in the authentication process.\n\n## Changes made:\n\n* Modified the validation logic for the Auth type to properly authenticate users.\n* Added unit tests to ensure the authentication process works as expected.\n\n## Testing:\n\nI tested this change locally on my development environment and confirmed that users can now log in without any issues. All unit tests are passing.", + "number": 1, + "sourceBranch": { + "branchContributorHandle": null, + "branchContributorUserId": null, + "branchId": "B-", + "branchName": "main", + "branchShortHand": "main", + "projectBranchShortHand": "@test/publictestproject/main" + }, + "status": "closed", + "targetBranch": { + "branchContributorHandle": "transcripts", + "branchContributorUserId": "U-", + "branchId": "B-", + "branchName": "contribution", + "branchShortHand": "@transcripts/contribution", + "projectBranchShortHand": "@test/publictestproject/@transcripts/contribution" + }, + "title": "Fix issue with user authentication" }, "project": { "projectId": "P-", @@ -31,11 +52,16 @@ "projectOwnerUserId": "U-", "projectShortHand": "@test/publictestproject", "projectSlug": "publictestproject" + }, + "status_update": { + "newStatus": "closed", + "oldStatus": "in_review" } } }, "id": "EVENT-", "occurredAt": "", + "projectId": "P-", "resourceId": "RES-", "scope": { "info": { diff --git a/transcripts/share-apis/notifications/list-notifications-transcripts.json b/transcripts/share-apis/notifications/list-notifications-transcripts.json index ca40aac6..0dc45120 100644 --- a/transcripts/share-apis/notifications/list-notifications-transcripts.json +++ b/transcripts/share-apis/notifications/list-notifications-transcripts.json @@ -1,55 +1,6 @@ { "body": { "items": [ - { - "createdAt": "", - "event": { - "actor": { - "info": { - "avatarUrl": null, - "handle": "test", - "name": null, - "userId": "U-" - }, - "kind": "user" - }, - "data": { - "kind": "project:branch:updated", - "link": "http://:1234/@test/publictestproject/code/newbranch/latest", - "payload": { - "branch": { - "branchContributorHandle": null, - "branchContributorUserId": null, - "branchId": "B-", - "branchName": "newbranch", - "branchShortHand": "newbranch", - "projectBranchShortHand": "@test/publictestproject/newbranch" - }, - "project": { - "projectId": "P-", - "projectOwnerHandle": "test", - "projectOwnerUserId": "U-", - "projectShortHand": "@test/publictestproject", - "projectSlug": "publictestproject" - } - } - }, - "id": "EVENT-", - "occurredAt": "", - "resourceId": "RES-", - "scope": { - "info": { - "avatarUrl": null, - "handle": "test", - "name": null, - "userId": "U-" - }, - "kind": "user" - } - }, - "id": "NOT-", - "status": "unread" - }, { "createdAt": "", "event": { diff --git a/transcripts/share-apis/notifications/webhook_results.txt b/transcripts/share-apis/notifications/webhook_results.txt index 7b98feeb..f1be98ec 100644 --- a/transcripts/share-apis/notifications/webhook_results.txt +++ b/transcripts/share-apis/notifications/webhook_results.txt @@ -1,3 +1,3 @@ -Successful webhooks: 6 -Unsuccessful webhooks: 6 +Successful webhooks: 7 +Unsuccessful webhooks: 0 From 5af711ba095d32cb82bca7df3a6a388395087f2a Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Thu, 4 Sep 2025 16:08:02 -0700 Subject: [PATCH 33/35] Fix queries on notification_by_* tables --- src/Share/Notifications/Queries.hs | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/Share/Notifications/Queries.hs b/src/Share/Notifications/Queries.hs index 30356a9f..41b46de1 100644 --- a/src/Share/Notifications/Queries.hs +++ b/src/Share/Notifications/Queries.hs @@ -140,11 +140,7 @@ listEmailDeliveryMethods userId maySubscriptionId = do FROM notification_emails ne WHERE ne.subscriber_user_id = #{userId} AND (#{maySubscriptionId} IS NULL - OR EXISTS( - SELECT FROM notification_by_email nbe - WHERE nbe.subscription_id = #{maySubscriptionId} - AND nbe.email_id = ne.id - ) + OR ne.subscription_id = #{maySubscriptionId} ) ORDER BY ne.email |] @@ -157,11 +153,7 @@ listWebhooks userId maySubscriptionId = do FROM notification_webhooks nw WHERE nw.subscriber_user_id = #{userId} AND (#{maySubscriptionId} IS NULL - OR EXISTS( - SELECT FROM notification_by_webhook nbw - WHERE nbw.subscription_id = #{maySubscriptionId} - AND nbw.webhook_id = nw.id - ) + OR nw.subscription_id = #{maySubscriptionId} ) ORDER BY nw.created_at |] @@ -551,8 +543,7 @@ listProjectWebhooks projectId = do ns.created_at, ns.updated_at FROM notification_subscriptions ns - JOIN notification_by_webhook nbw ON ns.id = nbw.subscription_id - JOIN notification_webhooks nw ON nbw.webhook_id = nw.id + JOIN notification_webhooks nw ON nw.subscription_id = ns.id WHERE ns.subscriber_project_id = #{projectId} |] <&> fmap (\((webhookId, webhookName) PG.:. subscription) -> (webhookId, webhookName, subscription)) From 4db8e84a624ae54c20f3468eca12b9768ff381a0 Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Thu, 4 Sep 2025 16:08:02 -0700 Subject: [PATCH 34/35] Test Update/Delete/List for webhooks --- src/Share/Notifications/Ops.hs | 6 ++++- .../notifications/delete-project-webhook.json | 8 +++++++ .../list-project-webhooks-after-delete.json | 10 ++++++++ .../list-project-webhooks-after-update.json | 23 +++++++++++++++++++ .../notifications/list-project-webhooks.json | 20 ++++++++++++++++ transcripts/share-apis/notifications/run.zsh | 17 ++++++++++++++ .../notifications/update-project-webhook.json | 21 +++++++++++++++++ 7 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 transcripts/share-apis/notifications/delete-project-webhook.json create mode 100644 transcripts/share-apis/notifications/list-project-webhooks-after-delete.json create mode 100644 transcripts/share-apis/notifications/list-project-webhooks-after-update.json create mode 100644 transcripts/share-apis/notifications/list-project-webhooks.json create mode 100644 transcripts/share-apis/notifications/update-project-webhook.json diff --git a/src/Share/Notifications/Ops.hs b/src/Share/Notifications/Ops.hs index 0b199eff..0f6c5faa 100644 --- a/src/Share/Notifications/Ops.hs +++ b/src/Share/Notifications/Ops.hs @@ -159,9 +159,13 @@ deleteProjectWebhook projectId subscriptionId = do let owner = ProjectSubscriptionOwner projectId -- First fetch the webhook id associated with this subscription webhooks <- PG.runTransaction $ do NotifQ.webhooksForSubscription subscriptionId + -- Next delete the subscription, which cascades to delete the webhook rows. + PG.runTransaction $ do NotifQ.deleteNotificationSubscription owner subscriptionId + -- Now delete the webhook configs in Vault, it's possible that this fails, but + -- it's okay if there are dangling webhooks in vault, we can't be fully transactional here, + -- and it's better than having rows that point to missing secrets. for_ webhooks \webhookId -> do deleteWebhookDeliveryMethod owner webhookId - PG.runTransaction $ do NotifQ.deleteNotificationSubscription owner subscriptionId updateProjectWebhook :: SubscriptionOwner -> NotificationSubscriptionId -> Maybe URIParam -> (Maybe ProjectWebhookTopics) -> WebApp () updateProjectWebhook subscriptionOwner subscriptionId mayURIUpdate webhookTopics = do diff --git a/transcripts/share-apis/notifications/delete-project-webhook.json b/transcripts/share-apis/notifications/delete-project-webhook.json new file mode 100644 index 00000000..5af94a8f --- /dev/null +++ b/transcripts/share-apis/notifications/delete-project-webhook.json @@ -0,0 +1,8 @@ +{ + "body": [], + "status": [ + { + "status_code": 200 + } + ] +} diff --git a/transcripts/share-apis/notifications/list-project-webhooks-after-delete.json b/transcripts/share-apis/notifications/list-project-webhooks-after-delete.json new file mode 100644 index 00000000..c74e5e33 --- /dev/null +++ b/transcripts/share-apis/notifications/list-project-webhooks-after-delete.json @@ -0,0 +1,10 @@ +{ + "body": { + "webhooks": [] + }, + "status": [ + { + "status_code": 200 + } + ] +} diff --git a/transcripts/share-apis/notifications/list-project-webhooks-after-update.json b/transcripts/share-apis/notifications/list-project-webhooks-after-update.json new file mode 100644 index 00000000..9f9c3184 --- /dev/null +++ b/transcripts/share-apis/notifications/list-project-webhooks-after-update.json @@ -0,0 +1,23 @@ +{ + "body": { + "webhooks": [ + { + "createdAt": "", + "notificationSubscriptionId": "NS-", + "topics": { + "topics": [ + "project:contribution:created" + ], + "type": "selected" + }, + "updatedAt": "", + "uri": "http://:9999/good-webhook-updated" + } + ] + }, + "status": [ + { + "status_code": 200 + } + ] +} diff --git a/transcripts/share-apis/notifications/list-project-webhooks.json b/transcripts/share-apis/notifications/list-project-webhooks.json new file mode 100644 index 00000000..c534cbcd --- /dev/null +++ b/transcripts/share-apis/notifications/list-project-webhooks.json @@ -0,0 +1,20 @@ +{ + "body": { + "webhooks": [ + { + "createdAt": "", + "notificationSubscriptionId": "NS-", + "topics": { + "type": "all" + }, + "updatedAt": "", + "uri": "http://:9999/good-webhook" + } + ] + }, + "status": [ + { + "status_code": 200 + } + ] +} diff --git a/transcripts/share-apis/notifications/run.zsh b/transcripts/share-apis/notifications/run.zsh index 44a205a8..c379e0bb 100755 --- a/transcripts/share-apis/notifications/run.zsh +++ b/transcripts/share-apis/notifications/run.zsh @@ -16,6 +16,8 @@ fetch "$test_user" POST add-project-webhook "/users/test/projects/publictestproj \"topics\": {\"type\": \"all\"} }" +fetch "$test_user" GET list-project-webhooks "/users/test/projects/publictestproject/webhooks" + # Subscribe the transcripts user to notifications for a project in the test user. fetch "$transcripts_user" PUT subscribe-to-other-user-project '/users/test/projects/publictestproject/subscription' '{ "isSubscribed": true @@ -132,3 +134,18 @@ echo "Successful webhooks: $successful_webhooks\nUnsuccessful webhooks: $unsucce fetch "$test_user" PUT unsubscribe-from-project '/users/test/projects/publictestproject/subscription' '{ "isSubscribed": false }' + +# Get the project webhook id. +project_webhook_id=$(fetch_data_jq "$test_user" GET project-webhooks-fetch '/users/test/projects/publictestproject/webhooks' '.webhooks[0].notificationSubscriptionId') + +# Can update project webhooks +fetch "$test_user" PATCH update-project-webhook "/users/test/projects/publictestproject/webhooks/$project_webhook_id" "{ + \"uri\": \"${echo_server}/good-webhook-updated\", + \"topics\": {\"type\": \"selected\", \"topics\": [\"project:contribution:created\"]} +}" + +fetch "$test_user" GET list-project-webhooks-after-update "/users/test/projects/publictestproject/webhooks" + +fetch "$test_user" DELETE delete-project-webhook "/users/test/projects/publictestproject/webhooks/$project_webhook_id" + +fetch "$test_user" GET list-project-webhooks-after-delete "/users/test/projects/publictestproject/webhooks" diff --git a/transcripts/share-apis/notifications/update-project-webhook.json b/transcripts/share-apis/notifications/update-project-webhook.json new file mode 100644 index 00000000..237c74ac --- /dev/null +++ b/transcripts/share-apis/notifications/update-project-webhook.json @@ -0,0 +1,21 @@ +{ + "body": { + "webhook": { + "createdAt": "", + "notificationSubscriptionId": "NS-", + "topics": { + "topics": [ + "project:contribution:created" + ], + "type": "selected" + }, + "updatedAt": "", + "uri": "http://:9999/good-webhook-updated" + } + }, + "status": [ + { + "status_code": 200 + } + ] +} From e0a82883d82f16286e33943cf7471ba9f3fb2af7 Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Mon, 8 Sep 2025 13:39:30 -0700 Subject: [PATCH 35/35] Split off dropping columns until after deployment succeeds --- sql/2025-09-03_project-webhooks.sql | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/sql/2025-09-03_project-webhooks.sql b/sql/2025-09-03_project-webhooks.sql index fa7221df..cff86455 100644 --- a/sql/2025-09-03_project-webhooks.sql +++ b/sql/2025-09-03_project-webhooks.sql @@ -16,8 +16,7 @@ INSERT INTO notification_topic_group_topics (topic_group, topic) -- WEBHOOKS ALTER TABLE notification_webhooks - ADD COLUMN subscription_id UUID NULL REFERENCES notification_subscriptions(id) ON DELETE CASCADE, - DROP COLUMN subscriber_user_id; + ADD COLUMN subscription_id UUID NULL REFERENCES notification_subscriptions(id) ON DELETE CASCADE; CREATE INDEX notification_webhooks_by_subscription ON notification_webhooks(subscription_id); @@ -29,8 +28,7 @@ ALTER TABLE notification_webhooks -- EMAILS ALTER TABLE notification_emails - ADD COLUMN subscription_id UUID NULL REFERENCES notification_subscriptions(id) ON DELETE CASCADE, - DROP COLUMN subscriber_user_id; + ADD COLUMN subscription_id UUID NULL REFERENCES notification_subscriptions(id) ON DELETE CASCADE; CREATE INDEX notification_emails_by_subscription ON notification_emails(subscription_id); @@ -160,6 +158,12 @@ END; $$ LANGUAGE plpgsql; +ALTER TABLE notification_webhooks + DROP COLUMN subscriber_user_id; + +ALTER TABLE notification_emails + DROP COLUMN subscriber_user_id; + -- Now we can drop the old tables DROP TABLE notification_by_webhook; DROP TABLE notification_by_email;