Custom http methods for API documentation with relevant examples #801

abradburne · 2024-03-19T03:28:02Z

This allows for different parameters to be used for create and update methods, both for the parameters reference and the example request bodies.

This can be done either through the automatic_components_for call, or by using a different strong parameters method for updates.

e.g.

  automatic_components_for Courses::Enrollment,
    parameters: {
      update: {
        remove: [:contact_id],
        add: {email: {type: :string, required: true, example: "fred@example.com"}
      },
      create: {
        add: {: {type: :string, required: true, example: "fred@example.com"}
      }
    }

Alternatively, in the API controller, add an additional <model>_update_params strong parameters method and use those strong params in the def update method. The automatic_components_for will automatically look for the <model>_update_params method and use those for the update documentation.

This PR also changes the automatic_paths_for to reference the example request bodies that are generated by the automatic_components_for rather than generating an additional example request. This also means that the customisations made in the automatic_components_for call don't have to be repeated for the automatic_paths_for example generation.

To achieve this, automatic_components_for has to create different schemas for <model>ParametersCreate and <model>ParametersUpdate since the OpenAPI 3.1 spec does not allow for schemas to be altered for different requests.

This does not change the attribute definitions per-method.

Suggested Release note:
The included view template files for OpenAPI schema documentation generation have been updated to allow custom parameters to be set for different HTTP methods. If you have copied these templates (i.e /app/views/api/v1/open_api/shared/_paths.yaml.erb, users/_paths.yaml.erb and team/_paths.yaml.erb) into your project and manage them yourself, they will need to be updated manually.

jagthedrummer

@abradburne I have one question that I left inline. And I've asked @newstler to review this since he's our resident BT-API expert.

Do you expect that this will be backwards compatible for all apps? Even highly customized ones with lots of ejected files? For instance, would there be concerns if an app has an ejected copy of bullet_train-api/app/views/api/v1/open_api/teams/_paths.yaml.erb that still says just TeamParameters instead of TeamParametersUpdate?

(There's also a merge conflict on this one after I merged #633.)

jagthedrummer · 2024-03-29T16:14:19Z

bullet_train-api/app/helpers/api/open_api_helper.rb

-        if strong_parameter_keys.last.is_a?(Hash)
-          strong_parameter_keys += strong_parameter_keys.pop.keys
+        strong_params_module_name = "Api::#{@version.upcase}::#{model.name.pluralize}Controller::StrongParameters".constantize
+        strong_params_module = BulletTrain::Api::StrongParametersReporter.new(model, strong_params_module_name)


So I went looking to see if these two variables are used anywhere else, just to make sure that it's safe change their semantic meaning like this, and I came across this other block of code that is very similar to the original code in this block.

bullet_train-core/bullet_train-api/lib/bullet_train/api/example_bot.rb

Lines 45 to 61 in 567d4ad

if has_strong_parameters?("::Api::#{version.upcase}::#{class_name.pluralize}Controller".constantize)

strong_params_module = "::Api::#{version.upcase}::#{class_name.pluralize}Controller::StrongParameters".constantize

strong_parameter_keys = BulletTrain::Api::StrongParametersReporter.new(class_name.constantize, strong_params_module).report

if strong_parameter_keys.last.is_a?(Hash)

strong_parameter_keys += strong_parameter_keys.pop.keys

end

output = _json_output(template, version, class_name, var_name, values)

parameters_output = JSON.parse(output)

parameters_output&.select! { |key| strong_parameter_keys.include?(key.to_sym) }

# Wrapping the example as parameters should be wrapped with the model name:

parameters_output = {model.to_s => parameters_output}

return indent(parameters_output.to_yaml.delete_prefix("---\n"), 6).html_safe

end

Do we need to also update that one to match this one?

If not we may want to at least reconsider changing the semantic meaning of these variables in this block. I think it would be nice for both blocks to have variables that mean more or less the same thing so that there's less context switching required to work on them.

Here we should probably get different strong_parameter_keys based on method as well. Feels like this functionality should be extracted to a separate method to avoid code duplicating.

jagthedrummer · 2024-03-29T16:37:27Z

Does this PR resolve #755?

newstler

Feels like removing original components might break backwards compatibility. Maybe no need to rename create parameters, and if update parameters are different, those could be added optionally? Or, based on the customisation method, we'd have same old parameters everywhere, and if we need customisation, we just create custom parameters?

Also please don't forget to add information on how to use this to bullet_train-api/README.md.

newstler · 2024-03-29T16:44:52Z

bullet_train-api/app/helpers/api/open_api_helper.rb

@@ -79,7 +79,7 @@ def automatic_components_for(model, **options)
      attributes_output = JSON.parse(schema_json)

      # Allow customization of Attributes
-      customize_component!(attributes_output, options[:attributes]) if options[:attributes]
+      customize_component!(attributes_output, options[:attributes], model.name.underscore) if options[:attributes]


I'd suggest using model.model_name.collection for more predictable results.

newstler · 2024-03-29T16:50:11Z

bullet_train-api/app/helpers/api/open_api_helper.rb

+      # Allow customization of Parameters
+      parameters_custom = options[:parameters][method_type] if options[:parameters].is_a?(Hash) && options[:parameters].key?(method_type)
+      parameters_custom ||= options[:parameters]
+      customize_component!(parameters_output, parameters_custom, strong_params_module.model_name.underscore, method_type) if parameters_custom


Doesn't this line fail on calling underscore on an instance of ActiveModel::Name?

newstler · 2024-03-29T16:56:02Z

bullet_train-api/app/helpers/api/open_api_helper.rb

+
+      # We need to wrap the example parameters with the model name as expected by the API controllers
+      if parameters_output["example"]
+        parameters_output["example"] = {strong_params_module.model_name.underscore.tr("/", "_") => parameters_output["example"]}


Same here, for underscored model name it should be .model_name.param_key

the model_name is coming from def model_name method added to the strong_parameters_reporter, but I will change it to just expose model then it will have consistent naming and can use strong_params_module.model.name.underscoreandstrong_params_module.model.param_key`

Ah, I didn't notice it was declared, I thought it is this one: https://api.rubyonrails.org/classes/ActiveModel/Naming.html

newstler · 2024-03-29T17:03:43Z

bullet_train-api/app/helpers/api/open_api_helper.rb

-        if strong_parameter_keys.last.is_a?(Hash)
-          strong_parameter_keys += strong_parameter_keys.pop.keys
+        strong_params_module_name = "Api::#{@version.upcase}::#{model.name.pluralize}Controller::StrongParameters".constantize
+        strong_params_module = BulletTrain::Api::StrongParametersReporter.new(model, strong_params_module_name)


Here we should probably get different strong_parameter_keys based on method as well. Feels like this functionality should be extracted to a separate method to avoid code duplicating.

newstler · 2024-03-29T17:11:52Z

Does this PR resolve #755?

I think so!

# Conflicts: # bullet_train-api/lib/bullet_train/api/strong_parameters_reporter.rb

This switches to <model_name>Parameters for the 'Create params', to ensure back-compatibility. Removed duplicated code by abstracting a 'strong_parameters_for' method

abradburne · 2024-04-02T11:14:59Z

@newstler @jagthedrummer these update should address all of the points raised. I'm writing up the changes for the readme

jagthedrummer · 2024-04-05T19:26:21Z

@abradburne just curious about the updates the README. Are you wanting to do that in a separate PR? (Having it as part of this one would generally be preferable, unless there are reasons not to.)

…ain-core into custom_http_methods

abradburne · 2024-05-30T08:37:13Z

@jagthedrummer I've added docs for the feature.

Thinking about it, one thing that may be an issue is if a BT user has manually copied and maintains their own version of that shared/_paths.yaml.erb file in their project (or the , then Teams or Users too), then those would need to be updated too - but difficult to know if anyone has done this. Would a release note cover this?

jagthedrummer · 2024-05-30T17:29:41Z

Thanks, @abradburne! Yeah, I think a release note should be good enough to point people in the right direction. Can you add that to the primary description of this PR so that it's easy for people to find? A link to this PR will automatically be included in the release notes, but I can also copy your instructions into the body of the release to make it a little more prominent.

abradburne · 2024-06-05T08:14:18Z

@jagthedrummer I've added a suggested note to the end of the description. Let me know if you think it should give more info.

jagthedrummer · 2024-06-05T16:12:30Z

Thanks, @abradburne!

abradburne added 9 commits March 15, 2024 14:31

wip

f9fa83b

wip

ab5644d

wip

585058d

tidy

6234f5d

improve example

77a59bb

standardrb

cee2298

fix scaffolding path for API doc

931c70f

another one

04a2aeb

Don't include openapi schema for non-present paths in standard bt

6e531e8

jagthedrummer requested a review from newstler March 29, 2024 16:21

jagthedrummer reviewed Mar 29, 2024

View reviewed changes

newstler reviewed Mar 29, 2024

View reviewed changes

gazayas mentioned this pull request Mar 30, 2024

Update field partials for correct use of options variables #633

Merged

9 tasks

abradburne added 2 commits April 2, 2024 12:49

Merge branch 'main' into custom_http_methods

614af68

# Conflicts: # bullet_train-api/lib/bullet_train/api/strong_parameters_reporter.rb

Reduce dupe code, use standard 'Parameters' for create

9cc4fb2

This switches to <model_name>Parameters for the 'Create params', to ensure back-compatibility. Removed duplicated code by abstracting a 'strong_parameters_for' method

newstler approved these changes Apr 2, 2024

View reviewed changes

abradburne added 6 commits May 28, 2024 12:55

Merge branch 'bullet-train-co:main' into custom_http_methods

f8b8c7e

fix customization call

473762b

add documentation

15b0bf6

add doc index link

6442c55

Merge branch 'custom_http_methods' of github.com:abradburne/bullet_tr…

165519c

…ain-core into custom_http_methods

remove unused method param

bfe4511

jagthedrummer merged commit ecce692 into bullet-train-co:main Jun 5, 2024
30 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom http methods for API documentation with relevant examples #801

Custom http methods for API documentation with relevant examples #801

abradburne commented Mar 19, 2024 •

edited

jagthedrummer left a comment

jagthedrummer Mar 29, 2024

newstler Mar 29, 2024

jagthedrummer commented Mar 29, 2024

newstler left a comment

newstler Mar 29, 2024

newstler Mar 29, 2024

newstler Mar 29, 2024

abradburne Apr 2, 2024

newstler Apr 2, 2024

newstler Mar 29, 2024

newstler commented Mar 29, 2024

abradburne commented Apr 2, 2024

jagthedrummer commented Apr 5, 2024 •

edited

abradburne commented May 30, 2024

jagthedrummer commented May 30, 2024

abradburne commented Jun 5, 2024

jagthedrummer commented Jun 5, 2024

	if has_strong_parameters?("::Api::#{version.upcase}::#{class_name.pluralize}Controller".constantize)
	strong_params_module = "::Api::#{version.upcase}::#{class_name.pluralize}Controller::StrongParameters".constantize
	strong_parameter_keys = BulletTrain::Api::StrongParametersReporter.new(class_name.constantize, strong_params_module).report
	if strong_parameter_keys.last.is_a?(Hash)
	strong_parameter_keys += strong_parameter_keys.pop.keys
	end

	output = _json_output(template, version, class_name, var_name, values)

	parameters_output = JSON.parse(output)
	parameters_output&.select! { \|key\| strong_parameter_keys.include?(key.to_sym) }

	# Wrapping the example as parameters should be wrapped with the model name:
	parameters_output = {model.to_s => parameters_output}

	return indent(parameters_output.to_yaml.delete_prefix("---\n"), 6).html_safe
	end

Custom http methods for API documentation with relevant examples #801

Custom http methods for API documentation with relevant examples #801

Conversation

abradburne commented Mar 19, 2024 • edited

jagthedrummer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jagthedrummer commented Mar 29, 2024

newstler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

newstler commented Mar 29, 2024

abradburne commented Apr 2, 2024

jagthedrummer commented Apr 5, 2024 • edited

abradburne commented May 30, 2024

jagthedrummer commented May 30, 2024

abradburne commented Jun 5, 2024

jagthedrummer commented Jun 5, 2024

abradburne commented Mar 19, 2024 •

edited

jagthedrummer commented Apr 5, 2024 •

edited