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

Fix applying filter from Advanced Search in My Services #4645

Conversation

hstastna
Copy link

@hstastna hstastna commented Sep 11, 2018

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1627078

There is a problem in My Services page while applying filter from Advanced Search: error occurs and the filter is not applied.

Steps to reproduce:

  1. Go to Services > My Services page
  2. Open Advanced Search and create some new expression, commit it
  3. Apply the expression, no need to save it. If you save the filter before applying it, apply the filter using Advanced Search, too.
    => error:
[----] F, [2018-09-11T18:00:16.007675 #29073:2b18a138e138] FATAL -- : Error caught: [RuntimeError] operator 'token' is not supported
/home/hstastna/manageiq/manageiq/lib/miq_expression.rb:1435:in `to_arel'
/home/hstastna/manageiq/manageiq/lib/miq_expression.rb:289:in `to_sql'
/home/hstastna/manageiq/manageiq/lib/rbac/filterer.rb:242:in `search'
/home/hstastna/manageiq/manageiq/lib/rbac/filterer.rb:131:in `search'
/home/hstastna/manageiq/manageiq/lib/rbac.rb:3:in `search'
/home/hstastna/manageiq/manageiq/app/models/miq_report/search.rb:110:in `paged_view_search'
/home/hstastna/manageiq/manageiq-ui-classic/app/controllers/application_controller.rb:1469:in `get_view'
/home/hstastna/manageiq/manageiq-ui-classic/app/controllers/application_controller.rb:434:in `report_data'

How reproducible:
not always, I have figured out that it also depends on which fields we choose in expression editor in Adv Search or how the expression looks like (sometimes we have more options to choose or more fields to set up, sometimes we choose only true/false for some specific fields)


Before:
filter_before

After:
filter_after

Note:
The problem happens here https://github.com/ManageIQ/manageiq-ui-classic/blob/master/app/controllers/application_controller.rb#L1517 where we set the filter which is about to be applied, and @edit[:adv_search_applied][:exp] contains :token key which is unnecessary here and it only causes problems here: https://github.com/ManageIQ/manageiq/blob/master/app/models/miq_report/search.rb#L108 because options contain the expression with the :token key (and its value). The solution is to set @edit[:adv_search_applied][:exp] properly by using copy_hash to make a proper copy of @edit[:new][@expkey], otherwise if @edit[:new][@expkey] changes (token is added in exp_build_table method), @edit[:adv_search_applied][:exp] changes, too, so that it also contains unnecessary token.

@hstastna hstastna changed the title Fix applying filter from Advanced Search in My Services [WIP] Fix applying filter from Advanced Search in My Services Sep 11, 2018
@miq-bot miq-bot added the wip label Sep 11, 2018
@hstastna
Copy link
Author

@miq-bot add_label bug, gaprindashvili/yes

@hstastna hstastna force-pushed the Selecting_advanced_search_My_Services_broken branch from d5c1ba2 to d295480 Compare September 12, 2018 10:31
@miq-bot
Copy link
Member

miq-bot commented Sep 12, 2018

Checked commit hstastna@d295480 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🏆

@hstastna hstastna changed the title [WIP] Fix applying filter from Advanced Search in My Services Fix applying filter from Advanced Search in My Services Sep 12, 2018
@miq-bot miq-bot removed the wip label Sep 12, 2018
@mzazrivec mzazrivec self-assigned this Sep 13, 2018
@mzazrivec mzazrivec added this to the Sprint 95 Ending Sep 24, 2018 milestone Sep 13, 2018
@mzazrivec mzazrivec merged commit be9832a into ManageIQ:master Sep 13, 2018
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