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: only add mutually exclusive description if argument is a lookup argument set #81

Merged
merged 2 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion examples/core_api/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class Base < Apia::API
add "time", "Allows time telling functions"
end

routes do
routes do # rubocop:disable Metrics/BlockLength
schema

get "time_formatting/incredibly/super/duper/long/format", controller: Controllers::TimeController,
Expand All @@ -27,6 +27,7 @@ class Base < Apia::API
post "example/format_multiple", controller: Controllers::TimeController, endpoint: :format_multiple

get "plain_text", endpoint: Endpoints::PlainTextEndpoint
post "plain_text", endpoint: Endpoints::PlainTextEndpoint

get "paginated", endpoint: Endpoints::PaginatedEndpoint

Expand Down
6 changes: 4 additions & 2 deletions lib/apia/open_api/objects/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,10 @@ def generate_child_schemas
@children = @definition.fields.values
elsif @definition.type.argument_set?
@children = @definition.type.klass.definition.arguments.values
@schema[:description] ||=
"All '#{@definition.name}[]' params are mutually exclusive, only one can be provided."
if @definition.type.klass.ancestors.include?(Apia::LookupArgumentSet)
@schema[:description] ||=
"All '#{@definition.name}[]' params are mutually exclusive, only one can be provided."
end
Comment on lines -83 to +86
Copy link
Contributor

Choose a reason for hiding this comment

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

change looks good, but can we update the specs to ensure we don't accidentally break this in future?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a post for the plain_text endpoint which uses the regular KeyValue argument set in the body. This doesn't use the mutually exclusive description.
Does that cover what you're looking for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I also disabled the BlockLength cop in example/base.rb because refactoring that feels like it would make things less readable

elsif @definition.type.object?
@children = @definition.type.klass.definition.fields.values
elsif enum_definition?
Expand Down
53 changes: 53 additions & 0 deletions spec/support/fixtures/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,45 @@
"$ref": "#/components/responses/APIAuthenticator403Response"
}
}
},
"post": {
"operationId": "post:plain_text",
"summary": "Plain Text Endpoint",
"description": "Return a plain text response",
"tags": [
"Core"
],
"requestBody": {
"content": {
"application/json": {
"schema": {
"properties": {
"disk_template_options": {
"type": "array",
"items": {
"$ref": "#/components/schemas/KeyValue"
}
}
}
}
}
}
},
"responses": {
"200": {
"description": "Return a plain text response",
"content": {
"text/plain": {
"schema": {
"type": "string"
}
}
}
},
"403": {
"$ref": "#/components/responses/APIAuthenticator403Response"
}
}
}
},
"/paginated": {
Expand Down Expand Up @@ -1098,6 +1137,20 @@
}
}
},
"KeyValue": {
"type": "object",
"properties": {
"key": {
"type": "string"
},
"value": {
"type": "string"
}
},
"required": [
"key"
]
},
"PaginationObject": {
"type": "object",
"properties": {
Expand Down
Loading