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

Jump to bottom

Update generic API Create to allow arrays in POST if set on object #4356

Merged
merged 6 commits into from Feb 18, 2020
Merged

Update generic API Create to allow arrays in POST if set on object #4356

merged 6 commits into from Feb 18, 2020

Conversation

mattjackson220
Copy link
Contributor

@mattjackson220 mattjackson220 commented Jan 30, 2020 •
edited by ocket8888

What does this PR (Pull Request) do?

This PR updates the generic API Create function to allow arrays as part of the POST if the struct passed in has it enabled.

Which Traffic Control components are affected by this PR?

  • Documentation
  • Traffic Control Client Go
  • Traffic Ops

What is the best way to verify this PR?

  • run TO locally and verify that POSTs to the /profileparameters and /parameters endpoints allow a single object, an array with only 1 object, and an array with multiple objects and that all get correctly inserted into the database.
  • verify that logs get added in the log table for each added parameter
  • run the same POSTs again with AllowMultipleCreates() set to false and verify that the array format does not work
  • run the same POSTs again with AllowMultipleCreates() completely removed from from the object and verify that the array format does not work

If this is a bug fix, what versions of Traffic Control are affected?

The following criteria are ALL met by this PR

  • This PR includes tests
  • This PR includes documentation
  • This PR includes an update to CHANGELOG.md
  • This PR includes any and all required license headers
  • This PR does not include a database migration
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY (see the Apache Software Foundation's security guidelines for details)

Additional Information

@ocket8888 ocket8888 added improvement The functionality exists but it could be improved in some way. Traffic Ops related to Traffic Ops bug something isn't working as intended regression bug a bug in existing functionality introduced by a new version labels Jan 30, 2020
@ocket8888
Copy link
Contributor

This is an improvement in general, but it also fixes regression bugs in endpoints which used to accept either objects in the request body or arrays thereof.

@zrhoffman
Copy link
Member

  • docs build without warnings
  • go fmt comes up clean
  • unit tests pass
  • API tests pass

Looks good to me!

mhoppa
mhoppa reviewed Feb 14, 2020
View reviewed changes
Copy link
Contributor

@mhoppa mhoppa left a comment

Choose a reason for hiding this comment

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

some very very small comments

traffic_ops/client/parameter.go Outdated Show resolved Hide resolved
traffic_ops/client/parameter.go Outdated Show resolved Hide resolved
traffic_ops/client/profile_parameter.go Show resolved Hide resolved
traffic_ops/client/profile_parameter.go Show resolved Hide resolved
traffic_ops/testing/api/v1/profile_parameters_test.go Outdated Show resolved Hide resolved
traffic_ops/testing/api/v1/profile_parameters_test.go Outdated Show resolved Hide resolved
traffic_ops/testing/api/v1/profile_parameters_test.go Outdated Show resolved Hide resolved
traffic_ops/testing/api/v1/profile_parameters_test.go Outdated Show resolved Hide resolved
mhoppa
mhoppa approved these changes Feb 18, 2020
View reviewed changes
Copy link
Contributor

@mhoppa mhoppa left a comment

Choose a reason for hiding this comment

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

some minor nit/non-blocking comments

@ocket8888 ocket8888 merged commit b084b66 into apache:master Feb 18, 2020
@ocket8888 ocket8888 mentioned this pull request Feb 21, 2020
Closed
@mitchell852 mitchell852 removed the bug something isn't working as intended label Aug 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zrhoffman zrhoffman zrhoffman requested changes

@mhoppa mhoppa mhoppa approved these changes

Assignees
No one assigned
Labels
improvement The functionality exists but it could be improved in some way. regression bug a bug in existing functionality introduced by a new version Traffic Ops related to Traffic Ops
Projects
None yet
Milestone
No milestone
5 participants
@mattjackson220 @ocket8888 @zrhoffman @mhoppa @mitchell852