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

Fixed validation while adding user with multiple groups. #427

Merged
merged 2 commits into from
Jul 24, 2018

Conversation

Hyperkid123
Copy link
Contributor

There was a buggy validation while adding new user with multiple user groups. Attribute name was required even if miq_groups was provided in data object.

@Hyperkid123
Copy link
Contributor Author

@skateman

req_attrs << "password" if ::Settings.authentication.mode == "database"
bad_attrs = []
req_attrs.each { |attr| bad_attrs << attr if data[attr].blank? }
bad_attrs << "group or miq_groups" if !data['group'] && !data['miq_groups']
Copy link
Member

Choose a reason for hiding this comment

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

@abellotti @gtanzillo what do you think about this?

Copy link
Member

Choose a reason for hiding this comment

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

@Hyperkid123 , @skateman - @abellotti and I looked at this and we are both ok with this change. We would just need some tests before merging.

req_attrs << "password" if ::Settings.authentication.mode == "database"
bad_attrs = []
req_attrs.each { |attr| bad_attrs << attr if data[attr].blank? }
bad_attrs << "group or miq_groups" if !data['group'] && !data['miq_groups']
Copy link
Member

Choose a reason for hiding this comment

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

@Hyperkid123 , @skateman - @abellotti and I looked at this and we are both ok with this change. We would just need some tests before merging.

@Hyperkid123
Copy link
Contributor Author

@gtanzillo i will add them in the morning :)

@miq-bot
Copy link
Member

miq-bot commented Jul 24, 2018

Checked commits Hyperkid123/manageiq-api@e479246~...2071fbc with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🏆

@gtanzillo gtanzillo self-assigned this Jul 24, 2018
@gtanzillo gtanzillo added this to the Sprint 91 Ending Jul 30, 2018 milestone Jul 24, 2018
@gtanzillo gtanzillo added the bug label Jul 24, 2018
@gtanzillo gtanzillo merged commit f9d4b5c into ManageIQ:master Jul 24, 2018
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.

4 participants