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.

Sign up for GitHub

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

Force ascending order #15559

Merged
merged 1 commit into from
Jul 13, 2017
Merged

Force ascending order #15559

merged 1 commit into from
Jul 13, 2017

Conversation

jntullo
Copy link

@jntullo jntullo commented Jul 12, 2017

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

When any sort_by is specified, the arel_attribute should be in ascending order by default. Currently, asc is not called either by default or when it is specified, causing an Arel::Attributes::Attribute to be returned, rather than Arel::Nodes::Ascending:, resulting in

{
  "error": {
    "kind": "internal_server_error",
    "message": "undefined method `to_sql' for #\u003cArel::Attributes::Attribute:0x007f98f3460288\u003e\nDid you mean?  to_s\n               to_set",
    "klass": "NoMethodError"
  }
}

The test added for this comes from the exact query mentioned in this BZ which allowed me to reproduce the bug.

@miq-bot add_label bz, api
cc: @AllenBW @imtayadeway

@jntullo jntullo changed the title Force ascending order if specified Force ascending order Jul 12, 2017
@miq-bot
Copy link
Member

miq-bot commented Jul 12, 2017

@jntullo Cannot apply the following label because they are not recognized: bz

@miq-bot miq-bot added the api label Jul 12, 2017
imtayadeway
imtayadeway approved these changes Jul 12, 2017
View reviewed changes
Copy link
Contributor

@imtayadeway imtayadeway left a comment

Choose a reason for hiding this comment

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

👍

@jntullo
Copy link
Author

jntullo commented Jul 12, 2017

@miq-bot add_label bug

@miq-bot miq-bot added the bug label Jul 12, 2017
@AllenBW
Copy link
Member

AllenBW commented Jul 12, 2017

🌮 💃 😻

@miq-bot
Copy link
Member

miq-bot commented Jul 13, 2017

Checked commit jntullo@291631d with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 🏆

gtanzillo
gtanzillo approved these changes Jul 13, 2017
View reviewed changes
Copy link
Member

@gtanzillo gtanzillo left a comment

Choose a reason for hiding this comment

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

👍

@gtanzillo gtanzillo added this to the Sprint 65 Ending Jul 24, 2017 milestone Jul 13, 2017
@gtanzillo gtanzillo merged commit 8ad46ce into ManageIQ:master Jul 13, 2017
@AllenBW AllenBW mentioned this pull request Jul 20, 2017
Merged
@jntullo jntullo deleted the bz/fix_asc_order branch November 28, 2017 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@imtayadeway imtayadeway imtayadeway approved these changes

@gtanzillo gtanzillo gtanzillo approved these changes

Assignees

@gtanzillo gtanzillo

Labels
api bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jntullo @miq-bot @AllenBW @gtanzillo @imtayadeway