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

Metric vis should display a ? or 0 for empty sets #5532

Merged
merged 18 commits into from
Dec 16, 2015
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/plugins/metric_vis/public/metric_vis.html
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<div ng-controller="KbnMetricVisController" class="metric-vis">
<div class="metric-container" ng-repeat="metric in metrics">
<div class="metric-value" ng-style="{'font-size': vis.params.fontSize+'pt'}">{{metric.value}}</div>
<div>{{metric.label}}</div>
</div>
<div class="metric-container" ng-repeat="metric in metrics">
<div class="metric-value" ng-style="{'font-size': vis.params.fontSize+'pt'}">{{metric.value}}</div>
<div>{{metric.label}}</div>
</div>
</div>
1 change: 1 addition & 0 deletions src/plugins/metric_vis/public/metric_vis.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ define(function (require) {
template: require('plugins/metric_vis/metric_vis.html'),
params: {
defaults: {
handleNoResults: true,
fontSize: 60
},
editor: require('plugins/metric_vis/metric_vis_params.html')
Expand Down
11 changes: 10 additions & 1 deletion src/plugins/metric_vis/public/metric_vis_controller.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
define(function (require) {
var _ = require('lodash');
// get the kibana/metric_vis module, and make sure that it requires the "kibana" module if it
// didn't already
var module = require('ui/modules').get('kibana/metric_vis', ['kibana']);
Expand All @@ -8,13 +9,21 @@ define(function (require) {

var metrics = $scope.metrics = [];

function isInvalid(val) {
return _.isUndefined(val) || _.isNull(val) || _.isNaN(val);
}

$scope.processTableGroups = function (tableGroups) {
tableGroups.tables.forEach(function (table) {
table.columns.forEach(function (column, i) {
var fieldFormatter = table.aggConfig(column).fieldFormatter();
var value = table.rows[0][i];

value = isInvalid(value) ? '?' : fieldFormatter(value);

metrics.push({
label: column.title,
value: fieldFormatter(table.rows[0][i])
value: value
});
});
});
Expand Down
11 changes: 9 additions & 2 deletions src/ui/public/agg_types/metrics/MetricAggType.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,17 @@ define(function (require) {
/**
* Read the values for this metric from the
* @param {[type]} bucket [description]
* @return {[type]} [description]
* @return {*} [description]
*/
MetricAggType.prototype.getValue = function (agg, bucket) {
return bucket[agg.id].value;
// Metric types where an empty set equals `zero`
var isSettableToZero = ['cardinality', 'sum'].indexOf(agg.__type.name) !== -1;

// Return proper values when no buckets are present
// `Count` handles empty sets properly
if (!bucket[agg.id] && isSettableToZero) return 0;

return bucket[agg.id] && bucket[agg.id].value;
};

/**
Expand Down
2 changes: 1 addition & 1 deletion src/ui/public/agg_types/metrics/percentile_ranks.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ define(function (require) {
getValue: function (agg, bucket) {
// values for 1, 5, and 10 will come back as 1.0, 5.0, and 10.0 so we
// parse the keys and respond with the value that matches
return _.find(bucket[agg.parentId].values, function (value, key) {
return _.find(bucket[agg.parentId] && bucket[agg.parentId].values, function (value, key) {
return agg.key === parseFloat(key);
}) / 100;
}
Expand Down
2 changes: 1 addition & 1 deletion src/ui/public/agg_types/metrics/percentiles.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ define(function (require) {
getValue: function (agg, bucket) {
// percentiles for 1, 5, and 10 will come back as 1.0, 5.0, and 10.0 so we
// parse the keys and respond with the value that matches
return _.find(bucket[agg.parentId].values, function (value, key) {
return _.find(bucket[agg.parentId] && bucket[agg.parentId].values, function (value, key) {
return agg.key === parseFloat(key);
});
}
Expand Down
8 changes: 2 additions & 6 deletions src/ui/public/visualize/visualize.html
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
<div
ng-if="vis.type.requiresSearch && esResp.hits.total === 0"
class="text-center visualize-error visualize-chart">
<div ng-if="showNoResultsMessage()" class="text-center visualize-error visualize-chart">
<div class="item top"></div>
<div class="item">
<h2 aria-hidden="true"><i aria-hidden="true" class="fa fa-meh-o"></i></h2>
<h4>No results found</h4>
</div>
<div class="item bottom"></div>
</div>
<div
ng-hide="vis.type.requiresSearch && esResp.hits.total === 0"
class="vis-container">
<div div ng-hide="showNoResultsMessage()" class="vis-container">
<div
ng-style="loadingStyle"
ng-class="{ loading: vis.type.requiresSearch && searchSource.activeFetchCount > 0 }"
Expand Down
13 changes: 13 additions & 0 deletions src/ui/public/visualize/visualize.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,19 @@ define(function (require) {
var getVisEl = getter('.visualize-chart');
var getVisContainer = getter('.vis-container');

// Show no results message when hasZeroHits is true and it requires search
$scope.showNoResultsMessage = function () {
var requiresSearch = _.get($scope, 'vis.type.requiresSearch');
Copy link
Member

Choose a reason for hiding this comment

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

can _.has be used for requireSearch and shouldNotShowMessage?

Copy link
Member

Choose a reason for hiding this comment

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

my bad, I didn't notice these were both already booleans. it looks like the has check works fine but probably wasn't needed

var isZeroHits = _.get($scope,'esResp.hits.total') === 0;
var shouldNotShowMessage = _.get($scope, 'vis.params.handleNoResults');

if (requiresSearch && isZeroHits) {
// if the vis type handles instances when there are no results,
// do not show a no results message
return shouldNotShowMessage ? false : true;
Copy link
Member

Choose a reason for hiding this comment

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

this block might be a easier to read if we did this in the positive. maybe something like return Boolean(requiresSearch & isZeroHits & shouldShowMessage).

}
};

$scope.fullScreenSpy = false;
$scope.spy = {};
$scope.spy.mode = ($scope.uiState) ? $scope.uiState.get('spy.mode', {}) : {};
Expand Down