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

Return correct version href #87

Merged
merged 1 commit into from Oct 4, 2017
Merged

Return correct version href #87

merged 1 commit into from Oct 4, 2017

Conversation

jntullo
Copy link

@jntullo jntullo commented Sep 28, 2017

BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1485424

Currently, when GET /api/:version is called, the href that is returned for the version in the response is in the format /api/:version/:version. This ensures only /api/:version is returned.

@miq-bot add_label bug
@miq-bot assign @abellotti

@@ -26,7 +26,7 @@ def entrypoint_versions
ApiConfig.version.definitions.select(&:ident).collect do |version_specification|
{
:name => version_specification[:name],
:href => "#{@req.api_prefix}/#{version_specification[:ident]}"
:href => @req.api_prefix.include?(version_specification[:ident]) ? @req.api_prefix : "#{@req.api_prefix}/#{version_specification[:ident]}"
Copy link
Member

@abellotti abellotti Oct 2, 2017

Choose a reason for hiding this comment

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

can you double check the code in parser/request_adapter.rb ? I thought api_prefix always includes the version so we may not want to use it.

issue remaining, I've added 2.4.0 as a version def in the api.yml and got this while fetching GET /api/v2.4.0

  "version": "3.0.0-pre",
  "versions": [
    {
      "name": "3.0.0-pre",
      "href": "http://localhost:3000/api/v2.4.0/v3.0.0-pre"
    },
    {
      "name": "2.4.0",
      "href": "http://localhost:3000/api/v2.4.0"
    }
  ],

@Fryguy
Copy link
Member

Fryguy commented Oct 2, 2017

Is this still a thing with #88 ?

@abellotti
Copy link
Member

abellotti commented Oct 2, 2017

yep, have tried this PR with #88, still an issue.

@abellotti
Copy link
Member

ping @jntullo can you double check the failures. thanks.

@jntullo
Copy link
Author

jntullo commented Oct 3, 2017

@abellotti seeing the same failures on other PRs. seems external, trying to figure it out

@miq-bot
Copy link
Member

miq-bot commented Oct 3, 2017

This pull request is not mergeable. Please rebase and repush.

@abellotti abellotti added this to the Sprint 71 Ending Oct 16, 2017 milestone Oct 4, 2017
@miq-bot
Copy link
Member

miq-bot commented Oct 4, 2017

Checked commit jntullo@0ba0e61 with ruby 2.3.1, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 0 offenses detected
Everything looks fine. 🍪

@abellotti
Copy link
Member

LGTM!! Thanks @jntullo for fixing this.

@abellotti abellotti merged commit 5e983b2 into ManageIQ:master Oct 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants