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 proper multi select condition #15436

Merged
merged 3 commits into from
Jul 13, 2017
Merged

Conversation

Ladas
Copy link
Contributor

@Ladas Ladas commented Jun 23, 2017

Right now we are using this for loading from the db:
"col1 IN [a,b] AND col2 IN [e,f]"
which does give incorrect results since we need to load exact
combination:
"(col1, col2) IN [(a,e), (b,f), (b,e)]

The exact combination is especially needed when we are
determining if we should create/delete/update the records.

Unifying the compare and loading from the db to the correct
multiselect query. And using it on appropriate places.

Ladas added 3 commits June 23, 2017 12:48
Right now we are using this for loading from the db:
"col1 IN [a,b] AND col2 IN [e,f]"
which does give incorrect results since we need to load exact
combination:
"(col1, col2) IN [(a,e), (b,f), (b,e)]

The exact combination is especially needed when we are
determining if we should create/delete/update the records.

Unifying the comparation and loading from the db to the correct
multiselect query.
Fix custom_db_finder for hardwares and networks, the finder
for networks was not defined and the query was built badly.
Use build_multi_selection_condition in sql helper
@Ladas
Copy link
Contributor Author

Ladas commented Jun 23, 2017

@miq-bot assign @agrare

@Ladas
Copy link
Contributor Author

Ladas commented Jun 23, 2017

@miq-bot add_label enhancement

@miq-bot
Copy link
Member

miq-bot commented Jun 23, 2017

Checked commits Ladas/manageiq@78558b3~...4a706ac with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 1 offense detected

app/models/manager_refresh/inventory_collection.rb

@agrare agrare merged commit 310b842 into ManageIQ:master Jul 13, 2017
@agrare agrare added this to the Sprint 65 Ending Jul 24, 2017 milestone Jul 13, 2017
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