Skip to content

TO-291: Add notification API endpoints#640

Open
almeidaraul wants to merge 21 commits intomainfrom
TO-291/add-api-endpoints
Open

TO-291: Add notification API endpoints#640
almeidaraul wants to merge 21 commits intomainfrom
TO-291/add-api-endpoints

Conversation

@almeidaraul
Copy link
Contributor

Description

Added API functionality related to notifications

Resolved issues

Part of TO-291

Web service API changes

Added new endpoints for editing notifications

@almeidaraul almeidaraul changed the title Add notification API endpoints TO-291: Add notification API endpoints Mar 11, 2026
@almeidaraul almeidaraul requested a review from Copilot March 13, 2026 16:35
Copy link

Copilot AI left a 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 first-pass notifications API endpoints to the backend so clients can list notifications, fetch unread counts, and mark notifications as read.

Changes:

  • Introduces /v1/notifications, /v1/notifications/unread-count, and /v1/notifications/{notification_id}/read endpoints.
  • Adds controller tests and a test data generator helper for notifications.
  • Updates the generated OpenAPI schema and registers the new router.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
backend/tests/data_generator.py Adds gen_notification() to generate Notification records for tests.
backend/tests/controllers/notifications/test_notifications.py Adds test coverage for listing, unread count, and marking notifications as read.
backend/test_observer/controllers/router.py Registers the notifications router under /v1/notifications.
backend/test_observer/controllers/notifications/notifications.py Implements notification listing, unread-count, and mark-as-read endpoints.
backend/test_observer/controllers/notifications/init.py Exposes the notifications router.
backend/schemata/openapi.json Updates OpenAPI paths/schemas for the new notification endpoints.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Base automatically changed from TO-291/add-notifications-table to main March 16, 2026 17:05
Copy link
Collaborator

@rpbritton rpbritton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall I think this looks pretty good! A couple comments though.

Copy link
Collaborator

@rpbritton rpbritton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another comment, what about prefixing these with:

GET /v1/users/{id}/notifications
GET /v1/users/me/notifications

That would make expand the general use case of these endpoints

@almeidaraul almeidaraul requested a review from rpbritton March 17, 2026 19:12
router.include_router(teams.router, prefix="/v1/teams")
router.include_router(permissions.router, prefix="/v1/permissions")
router.include_router(applications.router, prefix="/v1/applications")
router.include_router(notifications.router, prefix="/v1/users/me/notifications")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a incomplete to only hardcode me. If you go this route I think you should at least file a ticket to make the endpoint /v1/users/{id}/notifications, with an option for me.

@almeidaraul almeidaraul requested a review from rpbritton March 18, 2026 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants