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

Fix filter tab for chargeback report for all providers #1990

Merged
merged 2 commits into from
Aug 25, 2017
Merged

Fix filter tab for chargeback report for all providers #1990

merged 2 commits into from
Aug 25, 2017

Conversation

lpichler
Copy link
Contributor

@lpichler lpichler commented Aug 25, 2017 •
edited by simaishi
Loading

scenario:
http://g.recordit.co/JOKxoTDcTq.gif

Note: after creating the report, the error message is also thrown as well as on clicking on tab filter after re-editing.

error message


F, [2017-08-25T11:07:47.212860 #28711] FATAL -- : Error caught: [ActionView::Template::Error] Couldn't find ManageIQ::Providers::ContainerManager with 'id'=0 [WHERE "ext_management_systems"."type" IN ('ManageIQ::Providers::ContainerManager', 'ManageIQ::Providers::Kubernetes::ContainerManager', 'ManageIQ::Providers::Openshift::ContainerManager')]
/usr/local/opt/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activerecord-5.0.5/lib/active_record/relation/finder_methods.rb:353:in `raise_record_not_found_exception!'
/usr/local/opt/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activerecord-5.0.5/lib/active_record/relation/finder_methods.rb:479:in `find_one'
/usr/local/opt/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activerecord-5.0.5/lib/active_record/relation/finder_methods.rb:458:in `find_with_ids'
/usr/local/opt/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activerecord-5.0.5/lib/active_record/relation/finder_methods.rb:66:in `find'
/usr/local/opt/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activerecord-5.0.5/lib/active_record/querying.rb:3:in `find'
/usr/local/opt/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activerecord-5.0.5/lib/active_record/core.rb:151:in `find'
/Users/liborpichler/manageiq/manageiq-ui-classic/app/helpers/report_helper/editor.rb:3:in `cb_entities_by_provider_id'
/Users/liborpichler/manageiq/manageiq-ui-classic/app/views/report/_form_filter_chargeback.html.haml:119:in `___sers_liborpichler_manageiq_manageiq_ui_classic_app_views_report__form_filter_chargeback_html_haml___1162561060062711179_70220969285480'
/usr/local/opt/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/actionview-5.0.5/lib/action_view/template.rb:159:in `block in render'
/usr/local/opt/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-5.0.5/lib/active_support/notifications.rb:166:in `instrument'
/usr/local/opt/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/actionview-5.0.5/lib/action_view/template.rb:354:in `instrument'

@miq-bot assign @martinpovolny
cc @gtanzillo
can you review please @zeari ?

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

@lpichler
We selected All provider so we don't need list specific container_pro…
f0a584e 
…jects or container_images

there was a bug, because in @edit[:new][:cb_provider_id] was 'all' and
then is it was passes as interger ('all'.to_i == 0 ) to cb_entities_by_provider_id,
and there is method find which is failing it didn't find any record
@lpichler
Don't throw error when record is not found
38b382a
@miq-bot
Copy link
Member

miq-bot commented Aug 25, 2017

Checked commits lpichler/manageiq-ui-classic@f0a584e~...38b382a 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 Aug 25, 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.

This looks good. I ran into this issue yesterday and and have the same fix locally. 👍

@gtanzillo
Copy link
Member

This is the BZ for this issue - https://bugzilla.redhat.com/show_bug.cgi?id=1485006. We'll need this fix backported to fine and euwe.

@miq-bot add_label chargeback, bug, fine/yes, euwe/yes

@miq-bot
Copy link
Member

miq-bot commented Aug 25, 2017

@gtanzillo Cannot apply the following label because they are not recognized: chargeback

@martinpovolny martinpovolny merged commit 1c3311d into ManageIQ:master Aug 25, 2017
@martinpovolny martinpovolny added this to the Sprint 68 Ending Sep 4, 2017 milestone Aug 25, 2017
@lpichler lpichler deleted the fix_filter_tab_for_chargeback_report_for_all_providers branch August 25, 2017 12:40
@lpichler
Copy link
Contributor Author

@simaishi cherry-pick is clean for fine and euwe branch

@simaishi
Copy link
Contributor

simaishi commented Aug 31, 2017 •
edited
Loading

Euwe backport (to manageiq repo) details:

$ git log -1
commit 2ba97f4980f0da3b6ee2fcce17154161d42c59e9
Author: Martin Povolny <[email protected]>
Date:   Fri Aug 25 14:39:50 2017 +0200

    Merge pull request #1990 from lpichler/fix_filter_tab_for_chargeback_report_for_all_providers
    
    Fix filter tab for chargeback report for all providers
    (cherry picked from commit 1c3311d0bdf05d6ba2af3e772a9df04314217c28)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1487321

simaishi pushed a commit that referenced this pull request Aug 31, 2017
@martinpovolny @simaishi
Merge pull request #1990 from lpichler/fix_filter_tab_for_chargeback_…
ce3899a 
…report_for_all_providers

Fix filter tab for chargeback report for all providers
(cherry picked from commit 1c3311d)

https://bugzilla.redhat.com/show_bug.cgi?id=1487320
@simaishi
Copy link
Contributor

Fine backport details:

$ git log -1
commit ce3899a7cb4aa51798297b87a25ebb2aa37aa1e7
Author: Martin Povolny <[email protected]>
Date:   Fri Aug 25 14:39:50 2017 +0200

    Merge pull request #1990 from lpichler/fix_filter_tab_for_chargeback_report_for_all_providers
    
    Fix filter tab for chargeback report for all providers
    (cherry picked from commit 1c3311d0bdf05d6ba2af3e772a9df04314217c28)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1487320

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zeari zeari zeari approved these changes

@gtanzillo gtanzillo gtanzillo approved these changes

Assignees

@martinpovolny martinpovolny

Labels
bug euwe/backported fine/backported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@lpichler @miq-bot @gtanzillo @simaishi @zeari @martinpovolny