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

Corrected loading record id by selected node #671

Merged
merged 1 commit into from
Mar 14, 2017

Conversation

romanblanco
Copy link
Member

Problem here is that @perf_record variable is getting nil, because controller takes folder selected in the tree as the @record.

 151   def perf_set_or_fix_dates(allow_interval_override = true)                                                                      
 152     start_date, end_date = @perf_record.first_and_last_capture('hourly')                                                         
 153     start_date, end_date = @perf_record.first_and_last_capture('realtime') if realtime = start_date.nil?

After clicking in chart context menu, the id sent in params is the id of folder.
There is alternative method x_node_right_cell, that returns the id of what's in right cell.

Reproducing bug:

  1. Compute -> Infrastructure -> Virtual Machines
  2. Select one VM, that has data with C&U
  3. Click "Utilization" button
  4. In rendered chart, click o anything in contextual menu:

screencast from 2017-03-13 19-10-46

@romanblanco
Copy link
Member Author

@miq-bot add_label bug

@miq-bot miq-bot added the bug label Mar 13, 2017
@romanblanco
Copy link
Member Author

@ZitaNemeckova @himdel @PanSpagetka could you review?

@miq-bot
Copy link
Member

miq-bot commented Mar 13, 2017

Checked commit romanblanco@fbc783b with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks good. 🍰

Copy link
Contributor

@PanSpagetka PanSpagetka left a comment

Choose a reason for hiding this comment

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

This change make sense and works in UI.

@himdel himdel merged commit 77bf9ac into ManageIQ:master Mar 14, 2017
@himdel himdel added this to the Sprint 57 Ending Mar 27, 2017 milestone Mar 14, 2017
@himdel
Copy link
Contributor

himdel commented Mar 14, 2017

Bug was introduced by ManageIQ/manageiq#11387 .. adding euwe/yes.

@romanblanco romanblanco deleted the correct_node branch March 14, 2017 13:36
@simaishi
Copy link
Contributor

@romanblanco Is there a BZ for this issue? Can you please create if it doesn't exist?

@romanblanco
Copy link
Member Author

@romanblanco
Copy link
Member Author

@miq-bot remove_label bugzilla needed

@simaishi
Copy link
Contributor

Euwe backport (to manageiq repo) details:

$ git log -1
commit b42dcf7ef88dde42f5a93e7375bd576d88569205
Author: Martin Hradil <[email protected]>
Date:   Tue Mar 14 12:18:14 2017 +0000

    Merge pull request #671 from romanblanco/correct_node
    
    Corrected loading record id by selected node
    (cherry picked from commit 77bf9ac8558f668fea2592191c677d682e876548)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1433486

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.

6 participants