Skip to content
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

fix: add api param for delete message #3410

Merged
merged 3 commits into from
May 14, 2023
Merged

fix: add api param for delete message #3410

merged 3 commits into from
May 14, 2023

Conversation

jainpawan21
Copy link
Member

@jainpawan21 jainpawan21 commented May 14, 2023

What change does this PR introduce?

Add @ApiParam() Decorator in the delete message operation
Fix few array-related issues with OAS swagger
Mark payload as optional

Why was this change needed?

closes #2671

Other information (Screenshots)

@linear
Copy link

linear bot commented May 14, 2023

NV-1628 🐛 Bug Report: Invalid openapi spec

📜 Description

Thank you for adding an API spec in #571 . Unfortunately when trying to generate a client against it we discovered the spec has some validation errors. They seem like minor problems but I'd figure it was good to raise it.

👟 Reproduction steps

  1. On a running api server navigate to /api-yaml or /api-json and copy the output
  2. Paste it in an openapi spec validator, for example https://editor.swagger.io/
  3. As you can see in the output of the validator failures pop up. See actual behavior for logs

👍 Expected behavior

It should not produce any validation errors.

👎 Actual Behavior with Screenshots

0.11.0 errors

Semantic error at paths./v1/messages/{messageId}
Declared path parameter "messageId" needs to be defined as a path parameter at either the path or operation level

Structural error at components.schemas.UpdateEnvironmentRequestDto.properties.dns.properties.inboundParseDomain.required
should be an array of property names required within an object schema

Structural error at components.schemas.TopicPayloadDto.properties.type.enum
should NOT have fewer than 1 items
limit: 1

Structural error at components.schemas.TriggerEventRequestDto.properties.to.oneOf.1.type
should be equal to one of the allowed values
allowedValues: array, boolean, integer, number, object, string

Structural error at components.schemas.TriggerEventRequestDto.properties.to.oneOf.3.type
should be equal to one of the allowed values
allowedValues: array, boolean, integer, number, object, string

Structural error at components.schemas.TriggerEventRequestDto.properties.to.oneOf.5.type
should be equal to one of the allowed values
allowedValues: array, boolean, integer, number, object, string

Structural error at components.schemas.MessageResponseDto.properties.content.oneOf.0.type
should be equal to one of the allowed values
allowedValues: array, boolean, integer, number, object, string 

0.10.1 errors

Semantic error at paths./v1/messages/{messageId}
Declared path parameter "messageId" needs to be defined as a path parameter at either the path or operation level

Structural error at components.schemas.TopicPayloadDto.properties.type.enum
should NOT have fewer than 1 items
limit: 1

Structural error at components.schemas.TriggerEventRequestDto.properties.to.oneOf.1.type
should be equal to one of the allowed values
allowedValues: array, boolean, integer, number, object, string

Structural error at components.schemas.TriggerEventRequestDto.properties.to.oneOf.3.type
should be equal to one of the allowed values
allowedValues: array, boolean, integer, number, object, string

Structural error at components.schemas.TriggerEventRequestDto.properties.to.oneOf.5.type
should be equal to one of the allowed values
allowedValues: array, boolean, integer, number, object, string

Structural error at components.schemas.MessageResponseDto.properties.content.oneOf.0.type
should be equal to one of the allowed values
allowedValues: array, boolean, integer, number, object, string

📃 Provide any additional context for the Bug.

Found in ghcr.io/novuhq/novu/api:0.10.1, exact same issues also present in 0.11.0

👀 Have you spent some time to check if this bug has been raised before?

  • I checked and didn't find similar issue

🏢 Have you read the Contributing Guidelines?

Are you willing to submit PR?

None

Copy link
Contributor

@scopsy scopsy left a comment

Choose a reason for hiding this comment

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

🚀

@jainpawan21 jainpawan21 marked this pull request as ready for review May 14, 2023 18:59
@jainpawan21 jainpawan21 added this pull request to the merge queue May 14, 2023
Merged via the queue into next with commit 20ba6ab May 14, 2023
20 checks passed
@jainpawan21 jainpawan21 deleted the NV-1628 branch May 14, 2023 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[NV-1628] 🐛 Bug Report: Invalid openapi spec
2 participants