Milimetric has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/386774 )

Change subject: [WIP] Clean up UI and aggregations
......................................................................

[WIP] Clean up UI and aggregations

This cleans up a few things noticed by Erik in the linked task
It also refactors the aggregation a bit and corrects some calculation
mistakes.  The main thing remaining is to decide what to show on the
Dashboard because YoY numbers are not always going to match the widget
graph visually.

Bug: T178084
Change-Id: Id2bd700f9f72aea6ee56d68234426979d355e04d
---
M src/components/BottomFooter.vue
M src/components/dashboard/MetricWidget.vue
M src/components/detail/GraphPanel.vue
M src/lodash-custom-bundle.js
M src/models/GraphModel.js
A test/GraphModel.spec.js
6 files changed, 135 insertions(+), 32 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/analytics/wikistats2 
refs/changes/74/386774/1

diff --git a/src/components/BottomFooter.vue b/src/components/BottomFooter.vue
index bb1c2d0..7cacd8f 100644
--- a/src/components/BottomFooter.vue
+++ b/src/components/BottomFooter.vue
@@ -1,17 +1,20 @@
 <template>
-<section class="ui three column centered grid">
-    <div class="middle aligned column">
-        <p>Find out more <a 
href="https://wikitech.wikimedia.org/wiki/Analytics/Wikistats2.0";>about 
Wikistats</a> and the <a 
href="https://wikitech.wikimedia.org/wiki/Analytics#Datasets";>data that it 
uses</a></p>
+<section class="ui centered grid">
+    <div class="four wide middle aligned column">
+        Find out more <a target="_blank" 
href="https://wikitech.wikimedia.org/wiki/Analytics/Wikistats2.0";>about 
Wikistats</a><br/>
+        and the <a target="_blank" 
href="https://wikitech.wikimedia.org/wiki/Analytics#Datasets";>data it 
uses</a><br/>
     </div>
-    <div class="middle aligned column">
-        <p>Wikistats is written with <a target="_blank" 
href="//vuejs.org">Vue.js</a> and <a target="_blank" 
href="//semantic-ui.com">Semantic UI</a>.  If you'd like to contribute, see our 
<a target="_blank" 
href="//wikitech.wikimedia.org/wiki/Analytics/Wikistats2.0#Contributing_and_Deployment">contributing
 guide</a> or follow this link to <a target='_blank' 
href='https://phabricator.wikimedia.org/maniphest/task/edit/?title=Wikistats%20Bug&amp;projectPHIDs=Analytics-Wikistats,Analytics'>report
 a bug</a></p>
+    <div class="eight wide middle aligned column">
+        Wikistats is written with <a target="_blank" 
href="//vuejs.org">Vue.js</a> and <a target="_blank" 
href="//semantic-ui.com">Semantic UI</a>.<br/>
+        See our <a target="_blank" 
href="//wikitech.wikimedia.org/wiki/Analytics/Wikistats2.0#Contributing_and_Deployment">contributing
 guide</a> to help us build Wikistats.<br/>
+        Follow this link to <a target='_blank' 
href='https://phabricator.wikimedia.org/maniphest/task/edit/?title=Wikistats%20Bug&amp;projectPHIDs=Analytics-Wikistats,Analytics'>report
 a bug</a><br/>
     </div>
-    <div class="middle aligned column">
-        <p>Wikistats is designed by <a target="_blank" 
href="//aislinngrigas.com/about/">Aislinn Grigas</a>.</p>
+    <div class="four wide middle aligned column">
+        Designed by <a target="_blank" 
href="//aislinngrigas.com/about/">Aislinn Grigas</a>.<br/>
     </div>
     <div class="row">
-        <div class="eight wide middle aligned column">
-            <p>All data, charts, and other content is available under the <a 
href="https://creativecommons.org/publicdomain/zero/1.0/"; 
target="_new">Creative Commons CC0 dedication.</a></p>
+        <div class="sixteen wide middle aligned column">
+            All data, charts, and other content is available under the <a 
target="_blank" 
href="https://creativecommons.org/publicdomain/zero/1.0/";>Creative Commons CC0 
dedication.</a><br/>
         </div>
     </div>
 </section>
@@ -29,5 +32,6 @@
 </script>
 
 <style scoped>
-a, a:visited { color: #777; }
+a, a:visited { color: #888; }
+.ui.centered { font-size: 1.1em; }
 </style>
diff --git a/src/components/dashboard/MetricWidget.vue 
b/src/components/dashboard/MetricWidget.vue
index 2856ebc..4f025d2 100644
--- a/src/components/dashboard/MetricWidget.vue
+++ b/src/components/dashboard/MetricWidget.vue
@@ -34,7 +34,7 @@
                 </metric-line-widget>
                 <div class="ui horizontal small statistic">
                     <div class="value">
-                        {{lastYearAggregation.total | kmb}}
+                        {{lastYearAggregation | kmb}}
                     </div>
                     <div class="change label">
                         <arrow-icon :value="changeYoY"/>
@@ -131,15 +131,7 @@
                 return this.graphData[_.indexOf(this.graphData, 
this.lastMonth) - 12];
             },
             lastYearAggregation: function () {
-                if (this.metricData.additive) {
-                    return {
-                        total: 
_.sumBy(this.graphData.slice(_.indexOf(this.graphData, this.lastMonth) - 12), 
month => month.total)
-                    };
-                } else {
-                    return {
-                        total: 
_.sumBy(this.graphData.slice(_.indexOf(this.graphData, this.lastMonth) - 12), 
month => month.total) / 12
-                    };
-                }
+                return this.graphModel.getAggregate(12);
             },
             lastMonth: function () {
                 return _.last(this.graphData);
@@ -154,6 +146,8 @@
                 return ((diff / prev.total) * 100).toFixed(2);
             },
             changeYoY: function () {
+                // TODO: We're showing more than the last year, but reporting 
YoY.  This can be confusing because the YoY might not match up visually with 
the graph (like for Unique Devices in Achinese).
+
                 const diff = this.lastMonth.total - this.monthOneYearAgo.total;
                 return ((diff / this.monthOneYearAgo.total) * 100).toFixed(2);
             },
@@ -161,7 +155,7 @@
                 return !this.metricData.global && this.$store.state.project 
=== 'all-projects';
             },
             aggregationType: function () {
-                return !this.metricData.additive? 'Average': 'Total';
+                return this.graphModel.getAggregateLabel();
             }
         }
     ),
diff --git a/src/components/detail/GraphPanel.vue 
b/src/components/detail/GraphPanel.vue
index e84feec..38c22cb 100644
--- a/src/components/detail/GraphPanel.vue
+++ b/src/components/detail/GraphPanel.vue
@@ -43,14 +43,17 @@
             <div class="ui center aligned basic segment" v-if="graphModel && 
metricData.type !== 'list'">
                 <time-range-selector 
v-on:changeTimeRange='changeTimeRange'></time-range-selector>
                 <h5>
-                    Total: {{total | kmb}} {{metricData.fullName}} <arrow-icon 
:value="changeOverRange"></arrow-icon> {{((changeOverRange / total) * 
100).toFixed(2)}}% over this time range.
+                    {{graphModel.getAggregateLabel()}}:
+                    {{aggregate | kmb}} {{metricData.fullName}}
+                    <arrow-icon :value="changeOverRange"></arrow-icon>
+                    {{changeOverRange}}% over this time range.
                 </h5>
             </div>
             <div class="ui center aligned subdued basic segment">
                 <p>{{metricData.description}}. <a class='metric link' 
:href="metricData.info_url" target="_blank">More info about this metric.</a></p>
 
             </div>
-            <div class="ui right floated icon button" 
@click="toggleFullscreen">
+            <div class="ui right floated icon button" title="Toggle full-width 
graph" @click="toggleFullscreen">
                 <i class="ui icon" :class="{expand: !fullscreen, compress: 
fullscreen}"/>
             </div>
         </div>
@@ -97,12 +100,12 @@
             let chartTypes = this.getChartTypes();
             return (chartTypes[0].chart || 'empty') + '-chart';
         },
-        total: function () {
-            return this.graphModel && this.graphModel.getTotal();
+        aggregate: function () {
+            return this.graphModel && this.graphModel.getAggregate();
         },
         changeOverRange: function () {
             const data = this.graphModel.getAggregatedValues();
-            return data[data.length - 1] - data[0];
+            return ((data[data.length - 1] - data[0]) / data[0] * 
100).toFixed(2);
         }
     },
     data () {
diff --git a/src/lodash-custom-bundle.js b/src/lodash-custom-bundle.js
index 9f1de23..b790524 100644
--- a/src/lodash-custom-bundle.js
+++ b/src/lodash-custom-bundle.js
@@ -13,6 +13,7 @@
 import map from 'lodash/map';
 import pickBy from 'lodash/pickBy';
 import replace from 'lodash/replace';
+import round from 'lodash/round';
 import sortBy from 'lodash/sortBy';
 import startsWith from 'lodash/startsWith';
 import sum from 'lodash/sum';
@@ -38,6 +39,7 @@
     map,
     pickBy,
     replace,
+    round,
     sortBy,
     startsWith,
     sum,
diff --git a/src/models/GraphModel.js b/src/models/GraphModel.js
index 28fe3af..e9b6c27 100644
--- a/src/models/GraphModel.js
+++ b/src/models/GraphModel.js
@@ -59,28 +59,43 @@
     getDarkColor () {
         return this.metricData.darkColor;
     }
-    getTotal () {
-        return _.sum(this.getAggregatedValues());
-    }
     getActiveBreakdown () {
         if (!this.breakdowns) return null;
         return this.breakdowns.filter((breakdown) => {
             return breakdown.on;
         })[0];
     }
-    getAggregatedValues () {
+
+    getAggregateLabel () {
+        return this.metricData.additive ? 'Total' : 'Average';
+    }
+
+    getAggregate (limitToLastN) {
+        const values = this.getAggregatedValues(limitToLastN);
+        const total = _.sum(values);
+        const average = _.round(total / values.length, 1);
+
+        return this.metricData.additive ? total : average;
+    }
+
+    getAggregatedValues (limitToLastN) {
         const data = this.getGraphData();
+        let values;
+
         if (typeof data[0].total === 'number') {
-            return data.map((c) => {
+            values = data.map((c) => {
                 return c.total;
             });
         } else {
-            return data.map((r) => {
+            values = data.map((r) => {
                 return _.sum(_.map(r.total, (breakdownValue, key) => {
                     return this.getActiveBreakdown().values.find(v => v.key 
=== key).on? breakdownValue: 0;
                 }));
             });
         }
+        console.log( _.take(values, Math.min(limitToLastN || values.length, 
values.length)));
+
+        return _.take(values, Math.min(limitToLastN || values.length, 
values.length));
     }
     topXByY (x, y) {
         this.dimensionalData.measure(x);
diff --git a/test/GraphModel.spec.js b/test/GraphModel.spec.js
new file mode 100644
index 0000000..8efcc1a
--- /dev/null
+++ b/test/GraphModel.spec.js
@@ -0,0 +1,85 @@
+import DimensionalData from '../src/models/DimensionalData'
+import GraphModel from '../src/models/GraphModel'
+
+import config from '../src/config'
+import uniques from './mocks/uniques'
+
+const metric = {
+    type: 'lines',
+    value: 'devices',
+    area: 'reading',
+    breakdowns: [{
+        on: false,
+        name: 'Access site',
+        breakdownName: 'access-site',
+        values: [
+            { name: 'Mobile Site', on: true, key: 'mobile-site' },
+            { name: 'Desktop Site', on: true, key: 'desktop-site' }
+        ]
+    }],
+};
+
+let dimensionalData = new DimensionalData(uniques.desktop.items);
+dimensionalData.merge(uniques.mobile.items);
+
+describe('GraphModel', function () {
+    it('should reflect basic properties', function () {
+        let graphModel = new GraphModel(metric, dimensionalData);
+
+        expect(graphModel.getArea()).toEqual(metric.area);
+        
expect(graphModel.getBreakdowns()[0].name).toEqual(metric.breakdowns[0].name);
+    });
+
+    it('should aggregate total when metric is additive', function () {
+        metric.additive = true;
+
+        let graphModel = new GraphModel(metric, dimensionalData);
+        expect(graphModel.getAggregateLabel()).toEqual('Total');
+        expect(graphModel.getAggregate()).toEqual(1449174299);
+
+        expect(graphModel.getAggregate(3)).toEqual(355950563);
+        expect(graphModel.getAggregate(300)).toEqual(1449174299);
+    });
+
+    it('should aggregate average when metric is not additive', function () {
+        metric.additive = false;
+
+        let graphModel = new GraphModel(metric, dimensionalData);
+        expect(graphModel.getAggregateLabel()).toEqual('Average');
+        expect(graphModel.getAggregate()).toEqual(120764524.9);
+    });
+
+    it('should total properly when breaking down', function () {
+        metric.additive = true;
+
+        metric.breakdowns[0].on = true;
+        metric.breakdowns[0].values[0].on = true;
+        metric.breakdowns[0].values[1].on = false;
+
+        let graphModel = new GraphModel(metric, dimensionalData);
+        expect(graphModel.getAggregate()).toEqual(882978744);
+
+        metric.breakdowns[0].values[0].on = false;
+        metric.breakdowns[0].values[1].on = true;
+
+        graphModel = new GraphModel(metric, dimensionalData);
+        expect(graphModel.getAggregate()).toEqual(566195555);
+    });
+
+    it('should average properly when breaking down', function () {
+        metric.additive = false;
+
+        metric.breakdowns[0].on = true;
+        metric.breakdowns[0].values[0].on = true;
+        metric.breakdowns[0].values[1].on = false;
+
+        let graphModel = new GraphModel(metric, dimensionalData);
+        expect(graphModel.getAggregate()).toEqual(73581562);
+
+        metric.breakdowns[0].values[0].on = false;
+        metric.breakdowns[0].values[1].on = true;
+
+        graphModel = new GraphModel(metric, dimensionalData);
+        expect(graphModel.getAggregate()).toEqual(47182962.9);
+    });
+});

-- 
To view, visit https://gerrit.wikimedia.org/r/386774
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id2bd700f9f72aea6ee56d68234426979d355e04d
Gerrit-PatchSet: 1
Gerrit-Project: analytics/wikistats2
Gerrit-Branch: master
Gerrit-Owner: Milimetric <dandree...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to