-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: P4ADEV-1851 add P4PA-SEND openAPI #31
base: main
Are you sure you want to change the base?
Conversation
design/send/openApi.yaml
Outdated
paths: | ||
/notification: | ||
post: | ||
security: [] |
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.
why without security?
All APIs should expect to be under security (copy it from the openApi you can find on the repo)
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.
it's an error, i'll fix it
description: Notification not found | ||
'500': | ||
description: Internal server error | ||
post: |
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.
a POST on the entity path could be ambiguous, what do you think about to explicit configure the start operation?
/notification/{sendIdNotification}/start
design/send/openApi.yaml
Outdated
requestBody: | ||
content: | ||
application/json: | ||
schema: | ||
type: array | ||
items: | ||
$ref: '#/components/schemas/CreateNotificationRequest' | ||
responses: | ||
'200': | ||
description: OK | ||
content: | ||
application/json: | ||
schema: | ||
type: array | ||
items: | ||
$ref: '#/components/schemas/CreateNotificationResponse' |
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.
Why a request body?
The request response could be just the workflowId instead
design/send/openApi.yaml
Outdated
description: Wrong notification status | ||
'500': | ||
description: Internal server error | ||
/send/preload/{sendIdNotification}: |
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.
it's more meaningfull to configure the path to locate the entity as first and the configure the operation made on the entity (also for the other entities)
/send/{sendIdNotification}/preload
List of Changes
Motivation and Context
Screenshots (example: rendering of a Mermaid diagram):
Types of changes
Impacted Internal Process: