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

Use Rails path helpers for all routes #40

Merged
merged 1 commit into from
Sep 7, 2017

Conversation

imtayadeway
Copy link
Contributor

Follow up to #23

This enables us to:

  • delete the custom metaprogramming in api_helper.rb which offered a
    similar but limited implementation.
  • take advantage of the default behavior when passed an object which
    is to form a URL from the object's id, meaning the id can be omitted
    for terseness.
  • avoid having to do fuzzy matching when comparing hrefs, because
    where our methods provided only the path, Rails' gives us the full
    URL
  • avoid doing our own string concatenation for subcollection routes,
    e.g.:
    # before
    "#{blueprints_url(blueprint.id)}/tags/#{tag.id}"
    # after
    api_blueprint_tag_url(nil, blueprint, tag)
  • possibly leverage some of these helpers on the production side

This won't stay fresh for long, so ask me to rebase prior to merging.

@miq-bot add-label test, technical debt
@miq-bot assign @abellotti

@miq-bot
Copy link
Member

miq-bot commented Sep 7, 2017

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

@abellotti
Copy link
Member

@imtayadeway ❤️ this PR. Can you resolve the conflicts/repush ? Thanks.

This enables us to:

* delete the custom metaprogramming in api_helper.rb which offered a
  similar but limited implementation.
* take advantage of the default behavior when passed an object which
  is to form a URL from the object's id, meaning the id can be omitted
  for terseness.
* avoid having to do fuzzy matching when comparing hrefs, because
  where our methods provided only the path, Rails' gives us the full
  URL
* avoid doing our own string concatenation for subcollection routes,
  e.g.:
  ```ruby
  # before
  "#{blueprints_url(blueprint.id)}/tags/#{tag.id}"
  # after
  api_blueprint_tag_url(nil, blueprint, tag)
* possibly leverage some of these helpers on the production side
@miq-bot
Copy link
Member

miq-bot commented Sep 7, 2017

Checked commit imtayadeway@2c1bdc4 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
71 files checked, 0 offenses detected
Everything looks fine. ⭐

@abellotti
Copy link
Member

Looks good to me, Thanks @imtayadeway for this refactoring !! 🎼

@abellotti abellotti merged commit c0ce5d9 into ManageIQ:master Sep 7, 2017
@abellotti abellotti added this to the Sprint 69 Ending Sep 18, 2017 milestone Sep 7, 2017
imtayadeway added a commit to imtayadeway/manageiq-api that referenced this pull request Sep 7, 2017
This went red with the collision of ManageIQ#38 and ManageIQ#40 - updated to use the
new helper.
alexander-demicev pushed a commit to alexander-demicev/manageiq-api that referenced this pull request Oct 5, 2017
This went red with the collision of ManageIQ#38 and ManageIQ#40 - updated to use the
new helper.
@imtayadeway imtayadeway deleted the url-helpers-ii branch January 12, 2018 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants