-
Notifications
You must be signed in to change notification settings - Fork 4
Add endpoints for subscribing to a project #91
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
d12cca7
to
41be935
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds endpoints for users to subscribe or unsubscribe to project notifications, surfaces subscription status in the GetProject API, and auto-subscribes project creators (with a data migration for existing owners).
- Expose
PUT /users/:handle/projects/:slug/subscription
withisSubscribed
request andsubscriptionId
response. - Extend
GetProject
response to include anisSubscribed
flag and compute it for authenticated users. - Auto-subscribe project creators in
createProject
and provide a SQL migration to backfill existing projects.
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
transcripts/share-apis/roles/maintainer-project-view.json | Add isSubscribed to project-view transcript |
transcripts/share-apis/releases/project-latest-release.json | Add isSubscribed to release transcript |
transcripts/share-apis/projects-flow/*.json | Add isSubscribed to various project-get transcripts |
transcripts/share-apis/notifications/unsubscribe-from-project.json | New transcript for unsubscribe endpoint |
transcripts/share-apis/notifications/run.zsh | Update script to exercise subscribe/unsubscribe endpoints |
transcripts/share-apis/notifications/list-subscriptions-*.json | New transcripts for listing subscriptions |
src/Share/Web/UCM/Projects/Impl.hs | Propagate caller ID into project creation and require auth |
src/Share/Web/Share/Projects/Types.hs | Define IsSubscribed type and include in GetProjectResponse |
src/Share/Web/Share/Projects/Impl.hs | Compute subscription status in getProject and add subscription endpoint |
src/Share/Web/Share/Projects/API.hs | Add subscription route to API type |
src/Share/Web/Authorization/Types.hs | Define request/response types for subscription endpoint |
src/Share/Web/Authorization.hs | Update checkProjectCreate signature to use non-Maybe caller ID |
src/Share/Postgres/Ops.hs | Auto-subscribe creators in createProject |
src/Share/Notifications/Queries.hs | Implement updateWatchProjectSubscription and isUserSubscribedToWatchProject |
sql/2025-06-04_watch-projects.sql | Migration to subscribe existing project owners to their projects |
Comments suppressed due to low confidence (2)
src/Share/Web/Share/Projects/Impl.hs:424
- The authorization check here uses the caller as both subject and scope. Consider passing the project owner (derived from
projectUserHandler
or fetchedowner_user_id
) tocheckSubscriptionsManage
to correctly enforce subscription permissions.
_authZReceipt <- AuthZ.permissionGuard $ AuthZ.checkSubscriptionsManage caller caller
src/Share/Postgres/Ops.hs:57
- [nitpick] The new auto-subscription logic for project creators isn't validated by existing transcripts or tests. Consider adding a test or transcript that verifies creators are subscribed immediately after project creation.
_ <- Notifs.updateWatchProjectSubscription caller projId True
-- | 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] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The local variable name filter
shadows Prelude.filter
, which can be confusing. Consider renaming it to subscriptionFilter
for clarity.
let filter = SubscriptionFilter $ Aeson.object ["projectId" Aeson..= projId] | |
let subscriptionFilter = SubscriptionFilter $ Aeson.object ["projectId" Aeson..= projId] |
Copilot uses AI. Check for mistakes.
Overview
Adds endpoints to allow users to watch or unwatch a given project.
Adds
isSubscribed
to the GetProject response.Auto-subscribe to projects that a given user creates.
TODO:
Implementation notes
Adds:
Adds
isSubscribed
to the Get Project response.Test coverage
Transcripts