Add alerts to dashboard #1234

yaacov · 2017-05-03T10:05:28Z

Description

We are discussing adding alerts to the dashboard:

...
The current containers dashboard does not provide a summary of Alerts NOR ability to drill down to the Alerts area.
...
Mockup and comments make sense to me. Unless there are objections or other comments I think we could start working on this.

This PR is a POC for adding alerts to the dashboard.

Screenshots

System

One Provider

Current

Mockup

yaacov · 2017-05-03T10:06:28Z

@miq-bot add_label compute/containers

@simon3z @zeari @moolitayer @cben @dkorn @himdel please review

yaacov · 2017-05-03T10:10:24Z

@serenamarie125 @Loicavenel PTAL

yaacov · 2017-05-03T10:12:31Z

@miq-bot add_label enhancement

zeari · 2017-05-03T10:14:51Z

app/services/container_dashboard_service.rb

+      errors = @ems.miq_alert_statuses.where(:severity => "error").count
+    else
+      warnings = MiqAlertStatus.where(:severity => "warning").count
+      errors = MiqAlertStatus.where(:severity => "error").count


Are alert statuses exclusive to containers providers? Seems like there should be a filter here

👍 💯 Thanks ! working on that now ...

yaacov · 2017-05-03T10:36:08Z

@zeari Thanks fixed using @moolitayer's suggestion ems_id != null

yaacov · 2017-05-03T11:05:32Z

@himdel @cben codeclimate thinks that

  if (data) {
     statusObject.href = data.href;
     statusObject.count = data.count;
     statusObject.notifications = data.notifications;
   }

is similar to:

    if(type == "Add") {
      $scope.altText =  $scope.addBtnText;
      $scope.btnText = $scope.addBtnText;
      $scope.btnClick = $scope.addClicked;
    } else if(type == "Submit") {

with gravity 48 ! what is going on here ?

himdel · 2017-05-03T11:10:03Z

@yaacov Yeah, just ignore codeclimate here, aparently any 3 assignments in a condition are too similar :)

serenamarie125 · 2017-05-03T12:43:38Z

LGTM @yaacov!
What happens when there are no alerts, just a green check?

yaacov · 2017-05-03T12:47:17Z

What happens when there are no alerts, just a green check?

@serenamarie125

yaacov · 2017-05-03T12:57:56Z

@serenamarie125 looking at the way it looks with zero alerts, I changed it to:

yaacov · 2017-05-03T13:14:17Z

This is how it looks when only the number of errors is zero:

serenamarie125 · 2017-05-03T13:19:03Z

@yaacov I'd suggest that we never show 0.
So if there are only 15 errors, just show error icon with 15.

If there errors of any severity, just show a green check with no number.

See the standard card example here: http://www.patternfly.org/pattern-library/cards/aggregate-status-card/

yaacov · 2017-05-03T13:45:03Z

@serenamarie125 👍

Only warnings:

Only errors:

Warnings and errors:

Every thing is fine:
[EDIT: this has changed to not showing the zero]

yaacov · 2017-05-03T14:11:59Z

@serenamarie125 changed the no alerts to:

yaacov · 2017-05-03T16:04:54Z

@simon3z can we remove the POC from this ?

moolitayer · 2017-05-03T16:31:15Z

@simon3z can we remove the POC from this ?

Note the alerts feature isn't fully functional upstream (not in the cm-ops flow). So the counts might always be 0 for all our customers. We should how and when this can be available upstream
See also ManageIQ/manageiq#14976

martinpovolny · 2017-05-04T06:57:33Z

app/services/container_dashboard_service.rb

+    else
+      warnings = MiqAlertStatus.where.not(:ems_id => nil).where(:severity => "warning").count
+      errors = MiqAlertStatus.where.not(:ems_id => nil).where(:severity => "error").count
+    end


relation = @ems || MiqAlertStatus.where.not(:ems_id => nil) warnings, errors = relation.group(:severity).count.values_at(*%w(warning error))

something like this to cut the number of SQL queries (and also source lines) by 2 (3)?

@martinpovolny Nice, thanks 👍 fixed

serenamarie125 · 2017-08-17T13:01:44Z

@miq-bot add_label ux/review

zeari · 2017-08-21T13:06:50Z

app/services/container_dashboard_service.rb

@@ -96,6 +97,28 @@ def providers
    result.values
  end

+  def alerts
+    relation = @ems ? @ems.miq_alert_statuses : MiqAlertStatus.where.not(:ems_id => nil)


switch MiqAlertStatus.where.not(:ems_id => nil)
with something like MiqAlertStatus.where(:ems_id => ContainerManager.all.pluck(:id))

👍 right, thanks

yaacov · 2017-08-21T19:11:12Z

removing POC and WIP but this is still pending core.

miq-bot · 2017-08-22T06:38:47Z

Checked commits yaacov/manageiq-ui-classic@0cabfdb~...b25a37e with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks fine. ⭐

serenamarie125

LGTM @yaacov

serenamarie125 · 2017-08-22T20:12:36Z

@miq-bot add_label ux/approved
@miq-bot remove_label ux/review

martinpovolny · 2017-08-25T09:39:54Z

This might need to be converted to an (angular) component. Ping @h-kataria, please, take a look.

h-kataria · 2017-08-28T13:55:40Z

@yaacov changes look good in this PR. Container dashboards will need to be converted to use angular components once #1901 is completed and merged, I am close to getting Infrastructure providers dashboard converted.

miq-bot added the compute/containers label May 3, 2017

miq-bot added the enhancement label May 3, 2017

zeari reviewed May 3, 2017

View reviewed changes

yaacov force-pushed the add-alerts-to-dashboard branch from e99ed52 to ec1997c Compare May 3, 2017 10:34

yaacov force-pushed the add-alerts-to-dashboard branch from ec1997c to dd8a0bd Compare May 3, 2017 10:52

yaacov force-pushed the add-alerts-to-dashboard branch from 8d55807 to 735ecb3 Compare May 3, 2017 13:42

yaacov force-pushed the add-alerts-to-dashboard branch 2 times, most recently from a2d1ef1 to e816661 Compare May 3, 2017 14:10

yaacov force-pushed the add-alerts-to-dashboard branch from e816661 to 3897256 Compare May 3, 2017 14:13

dclarizio added the fine/no label May 3, 2017

dclarizio assigned martinpovolny May 3, 2017

yaacov force-pushed the add-alerts-to-dashboard branch from 11b6d0f to 69f0f8a Compare May 4, 2017 06:04

martinpovolny reviewed May 4, 2017

View reviewed changes

yaacov changed the title ~~[POC] Add alerts to dashboard~~ [WIP] [POC] Add alerts to dashboard May 11, 2017

miq-bot added the wip label May 11, 2017

miq-bot added the unmergeable label Aug 17, 2017

miq-bot added the ux/review label Aug 17, 2017

yaacov force-pushed the add-alerts-to-dashboard branch from 808b643 to 18d57de Compare August 21, 2017 12:18

miq-bot removed the unmergeable label Aug 21, 2017

zeari reviewed Aug 21, 2017

View reviewed changes

yaacov and others added 9 commits August 21, 2017 17:30

Add alerts to dashboard

0cabfdb

Fix spec fixtures

61af318

Change Icon if no alerts

2bd5468

Adjust the provider card height when no data is present

ff46678

streamline sql query (thanks martinpovolny)

82a2f9f

protect from null

a43f9c6

use args instead of array

2648cff

fix alerts scope (thanks Ari)

ea9e430

make codeclimate happe

f7e8d8a

yaacov force-pushed the add-alerts-to-dashboard branch from b3109f9 to f7e8d8a Compare August 21, 2017 14:48

yaacov changed the title ~~[WIP][POC] Add alerts to dashboard~~ Add alerts to dashboard Aug 21, 2017

miq-bot removed the wip label Aug 21, 2017

new version of single provider card

b25a37e

serenamarie125 approved these changes Aug 22, 2017

View reviewed changes

miq-bot added ux/approved and removed ux/review labels Aug 22, 2017

martinpovolny assigned h-kataria Aug 25, 2017

martinpovolny merged commit 0e2ef08 into ManageIQ:master Aug 31, 2017

martinpovolny added this to the Sprint 68 Ending Sep 4, 2017 milestone Aug 31, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add alerts to dashboard #1234

Add alerts to dashboard #1234

yaacov commented May 3, 2017 •

edited by himdel

Loading

yaacov commented May 3, 2017

yaacov commented May 3, 2017

yaacov commented May 3, 2017

zeari May 3, 2017

yaacov May 3, 2017

yaacov commented May 3, 2017

yaacov commented May 3, 2017

himdel commented May 3, 2017

serenamarie125 commented May 3, 2017

yaacov commented May 3, 2017

yaacov commented May 3, 2017

yaacov commented May 3, 2017

serenamarie125 commented May 3, 2017

yaacov commented May 3, 2017 •

edited

Loading

yaacov commented May 3, 2017

yaacov commented May 3, 2017

moolitayer commented May 3, 2017

martinpovolny May 4, 2017 •

edited

Loading

yaacov May 4, 2017

serenamarie125 commented Aug 17, 2017

zeari Aug 21, 2017 •

edited

Loading

yaacov Aug 21, 2017

yaacov commented Aug 21, 2017

miq-bot commented Aug 22, 2017

serenamarie125 left a comment

serenamarie125 commented Aug 22, 2017

martinpovolny commented Aug 25, 2017

h-kataria commented Aug 28, 2017

Add alerts to dashboard #1234

Add alerts to dashboard #1234

Conversation

yaacov commented May 3, 2017 • edited by himdel Loading

yaacov commented May 3, 2017

yaacov commented May 3, 2017

yaacov commented May 3, 2017

zeari May 3, 2017

Choose a reason for hiding this comment

yaacov May 3, 2017

Choose a reason for hiding this comment

yaacov commented May 3, 2017

yaacov commented May 3, 2017

himdel commented May 3, 2017

serenamarie125 commented May 3, 2017

yaacov commented May 3, 2017

yaacov commented May 3, 2017

yaacov commented May 3, 2017

serenamarie125 commented May 3, 2017

yaacov commented May 3, 2017 • edited Loading

yaacov commented May 3, 2017

yaacov commented May 3, 2017

moolitayer commented May 3, 2017

martinpovolny May 4, 2017 • edited Loading

Choose a reason for hiding this comment

yaacov May 4, 2017

Choose a reason for hiding this comment

serenamarie125 commented Aug 17, 2017

zeari Aug 21, 2017 • edited Loading

Choose a reason for hiding this comment

yaacov Aug 21, 2017

Choose a reason for hiding this comment

yaacov commented Aug 21, 2017

miq-bot commented Aug 22, 2017

serenamarie125 left a comment

Choose a reason for hiding this comment

serenamarie125 commented Aug 22, 2017

martinpovolny commented Aug 25, 2017

h-kataria commented Aug 28, 2017

yaacov commented May 3, 2017 •

edited by himdel

Loading

yaacov commented May 3, 2017 •

edited

Loading

martinpovolny May 4, 2017 •

edited

Loading

zeari Aug 21, 2017 •

edited

Loading