[NotificationService] Add Notification API #38181
Conversation
Next Steps to MergeNext steps that must be taken to merge this PR:
Comment generated by summarize-checks workflow run. |
github-actions
bot
added
data-plane
TypeSpec
API Change CheckAPIView identified API level changes in this PR and created the following API reviews
|
| @doc("Foundry notification definition") | ||
| @resource("notifications") | ||
| @added(Versions.v2025_05_15_preview) | ||
| model FoundryNotificationDto { |
There was a problem hiding this comment.
Don't use DTO in the name of the model.
| }> | ||
| >; | ||
|
|
||
| @doc("Dismiss all notifications of a specific project for a given user.") |
There was a problem hiding this comment.
How is a "user" defined? There is nothing in the payload that is a user?
There was a problem hiding this comment.
Yes. We validate and extract the user information from the JWT token.
|
|
||
| @doc("Notification title information") | ||
| @added(Versions.v2025_05_15_preview) | ||
| model NotificationTitle { |
There was a problem hiding this comment.
nit: since this contains multiple fields, maybe name it NotificationInformation/Label/something that makes it clear that it's not just a title
| name: string; | ||
|
|
||
| @doc("The description of the notification") | ||
| description: string; |
| @added(Versions.v2025_05_15_preview) | ||
| model ViewSourceActionDetail { | ||
| @doc("The type of action") | ||
| type: string; |
There was a problem hiding this comment.
maybe use a discrete union instead of an open string?
| type: string; | ||
|
|
||
| @doc("The target of the action") | ||
| target: string; |
There was a problem hiding this comment.
how does the targeting work? is it based on an ID? some kind of expression language? something else? we should probably provide a more explicit name/documentation for this property.
| @added(Versions.v2025_05_15_preview) | ||
| model FoundryNotificationDto { | ||
| @doc("NGNC version") | ||
| ngncVersion: string; |
There was a problem hiding this comment.
nit: from a naming convention, should this be NGNC since it's an acronym?
| @doc("The scope of the notification") | ||
| scope: string; | ||
|
|
||
| @doc("The source of the notification") | ||
| source: string; | ||
|
|
||
| @doc("The type of the notification") | ||
| type: string; | ||
|
|
There was a problem hiding this comment.
all those feel like they should be discrete unions instead of open strings
| type: string; | ||
|
|
||
| @doc("The data schema") | ||
| dataschema: string; |
There was a problem hiding this comment.
| dataschema: string; | |
| dataSchema: string; |
| type: string; | ||
|
|
||
| @doc("The data schema") | ||
| dataschema: string; |
There was a problem hiding this comment.
what is this schema in? JSON schema? in which case we should review the type here
| severity: string; | ||
|
|
||
| @doc("The status of the notification") | ||
| status: string; |
3 participants
Choose a PR Template
Switch to "Preview" on this description then select one of the choices below.
Click here to open a PR for a Data Plane API.
Click here to open a PR for a Control Plane (ARM) API.
Click here to open a PR for only SDK configuration.