Fixes #12597: remove extraneous method definition in the DNS API tests

[sodabrew]

http://projects.theforeman.org/issues/12597

[sodabrew]

@witlessbird Could you take a look at the test failure? I removed the extra method definition, which seems to be blocking this test before, and now that it is executing it fails. Was originally added in 42b9581

   def test_delete_returns_error_if_value_is_missing
     delete '/'
     assert_equal 400, last_response.status
   end

Smart Proxy is returning 404 in this case. That seems correct to me?

[domcleal]

Oops, sorry about the failure, I should've noticed that in the review and also re-run tests with the new rubocop.

Either 404 or 400 seems reasonable to me. Returning 400 as dns_api.rb intends would require changing the route to /:value?, but since the value isn't actually optional (that's the point of the nil check) then returning 404 seems OK as the route DELETE / isn't valid. I'd say removing the check and expecting 404 is probably OK.

[dmitri-d]

I'm not sure why it's returning 404 (this implies parameter validator isn't catching that the required parameter is missing), let me look into this. I think I favour returning a 400, but 404 seems reasonable too.

[domcleal]

I'm not sure why it's returning 404 (this implies parameter validator isn't catching that the required parameter is missing)

The route doesn't match. It's defined as delete '/:value' which implies :value must be present. Adding a ? would make the value optional, which would trigger the validator.

[domcleal]

Adding a ? would make the value optional, which would trigger the validator.

A thought - it looks like all other delete routes are the same, so we probably shouldn't change just this one.

[dmitri-d]

Of course, always forget this bit. Agree that the behaviour in this case should match other 'delete' calls.

[theforeman-bot]

There were the following issues with the commit message:

• 0914c05 must be in the format fixes #redmine_number - brief description.
• af4b48b must be in the format fixes #redmine_number - brief description.

If you don't have a ticket number, please create an issue in Redmine, selecting the appropriate project.

More guidelines are available on the Foreman wiki.

---

This message was auto-generated by Foreman's prprocessor

[sodabrew]

@witlessbird I merged in your further change - thank you! I will be offline a lot this week for the US Thanksgiving holiday. Please let me know if I need to squash on this branch, if it it'll get taken care of in a squash-cherry-pick to develop?

[dmitri-d]

@sodabrew: could you squash the commits please?

[sodabrew]

ok, got me while I'm still sitting at the desk!

[dmitri-d]

merged in bef9bee. Thanks, @sodabrew!