FIX: improves number/percent support in reports

app/assets/javascripts/admin/components/admin-report-table-cell.js.es6

@@ -0,0 +1,18 @@
+import computed from "ember-addons/ember-computed-decorators";
+
+export default Ember.Component.extend({
+  tagName: "td",
+  classNames: ["admin-report-table-cell"],
+  classNameBindings: ["type", "property"],
+  options: null,
+
+  @computed("label", "data", "options")
+  computedLabel(label, data, options) {
+    return label.compute(data, options || {});
+  },
+
+  type: Ember.computed.alias("label.type"),
+  property: Ember.computed.alias("label.mainProperty"),
+  formatedValue: Ember.computed.alias("computedLabel.formatedValue"),
+  value: Ember.computed.alias("computedLabel.value")
+});

app/assets/javascripts/admin/components/admin-report-table-header.js.es6

@@ -3,7 +3,7 @@ import computed from "ember-addons/ember-computed-decorators";
 export default Ember.Component.extend({
   tagName: "th",
   classNames: ["admin-report-table-header"],
-  classNameBindings: ["label.mainProperty", "isCurrentSort"],
+  classNameBindings: ["label.mainProperty", "label.type", "isCurrentSort"],
   attributeBindings: ["label.title:title"],
 
   @computed("currentSortLabel.sortProperty", "label.sortProperty")

app/assets/javascripts/admin/components/admin-report-table-row.js.es6

@@ -1,13 +1,5 @@
-import computed from "ember-addons/ember-computed-decorators";
-
 export default Ember.Component.extend({
   tagName: "tr",
   classNames: ["admin-report-table-row"],
-
-  @computed("data", "labels")
-  cells(row, labels) {
-    return labels.map(label => {
-      return label.compute(row);
-    });
-  }
+  options: null
 });

app/assets/javascripts/admin/components/admin-report-table.js.es6

@@ -1,6 +1,5 @@
 import computed from "ember-addons/ember-computed-decorators";
 import { registerTooltip, unregisterTooltip } from "discourse/lib/tooltip";
-import { isNumeric } from "discourse/lib/utilities";
 
 const PAGES_LIMIT = 8;
 
@@ -67,14 +66,16 @@ export default Ember.Component.extend({
         const computedLabel = label.compute(row);
         const value = computedLabel.value;
 
-        if (!computedLabel.countable || !value || !isNumeric(value)) {
-          return undefined;
+        if (!["seconds", "number", "percent"].includes(label.type)) {
+          return;
         } else {
-          return sum + value;
+          return sum + Math.round(value || 0);
         }
       };
 
-      totalsRow[label.mainProperty] = rows.reduce(reducer, 0);
+      const total = rows.reduce(reducer, 0);
+      totalsRow[label.mainProperty] =
+        label.type === "percent" ? Math.round(total / rows.length) : total;
     });
 
     return totalsRow;

app/assets/javascripts/admin/components/admin-report.js.es6

@@ -9,7 +9,8 @@ import { registerTooltip, unregisterTooltip } from "discourse/lib/tooltip";
 const TABLE_OPTIONS = {
   perPage: 8,
   total: true,
-  limit: 20
+  limit: 20,
+  formatNumbers: true
 };
 
 const CHART_OPTIONS = {};
@@ -347,12 +348,12 @@ export default Ember.Component.extend({
     if (mode === "table") {
       const tableOptions = JSON.parse(JSON.stringify(TABLE_OPTIONS));
       return Ember.Object.create(
-        _.assign(tableOptions, this.get("reportOptions.table") || {})
+        Object.assign(tableOptions, this.get("reportOptions.table") || {})
       );
     } else {
       const chartOptions = JSON.parse(JSON.stringify(CHART_OPTIONS));
       return Ember.Object.create(
-        _.assign(chartOptions, this.get("reportOptions.chart") || {})
+        Object.assign(chartOptions, this.get("reportOptions.chart") || {})
       );
     }
   },

app/assets/javascripts/admin/controllers/admin-reports-show.js.es6

@@ -5,7 +5,7 @@ export default Ember.Controller.extend({
 
   @computed("model.type")
   reportOptions(type) {
-    let options = { table: { perPage: 50, limit: 50 } };
+    let options = { table: { perPage: 50, limit: 50, formatNumbers: false } };
 
     if (type === "top_referred_topics") {
       options.table.limit = 10;

app/assets/javascripts/admin/models/report.js.es6

@@ -1,11 +1,7 @@
 import { escapeExpression } from "discourse/lib/utilities";
 import { ajax } from "discourse/lib/ajax";
 import round from "discourse/lib/round";
-import {
-  fillMissingDates,
-  isNumeric,
-  formatUsername
-} from "discourse/lib/utilities";
+import { fillMissingDates, formatUsername } from "discourse/lib/utilities";
 import computed from "ember-addons/ember-computed-decorators";
 import { number, durationTiny } from "discourse/lib/formatter";
 import { renderAvatar } from "discourse/helpers/user-avatar";
@@ -252,7 +248,7 @@ const Report = Discourse.Model.extend({
   @computed("labels")
   computedLabels(labels) {
     return labels.map(label => {
-      const type = label.type;
+      const type = label.type || "string";
 
       let mainProperty;
       if (label.property) mainProperty = label.property;
@@ -266,70 +262,63 @@ const Report = Discourse.Model.extend({
         title: label.title,
         sortProperty: label.sort_property || mainProperty,
         mainProperty,
-        compute: row => {
+        type,
+        compute: (row, opts = {}) => {
           const value = row[mainProperty];
 
           if (type === "user") return this._userLabel(label.properties, row);
           if (type === "post") return this._postLabel(label.properties, row);
           if (type === "topic") return this._topicLabel(label.properties, row);
-          if (type === "seconds")
-            return this._secondsLabel(mainProperty, value);
+          if (type === "seconds") return this._secondsLabel(value);
           if (type === "link") return this._linkLabel(label.properties, row);
-          if (type === "percent")
-            return this._percentLabel(mainProperty, value);
-          if (type === "number" || isNumeric(value)) {
-            return this._numberLabel(mainProperty, value);
+          if (type === "percent") return this._percentLabel(value);
+          if (type === "number") {
+            return this._numberLabel(value, opts);
           }
           if (type === "date") {
             const date = moment(value, "YYYY-MM-DD");
-            if (date.isValid())
-              return this._dateLabel(mainProperty, value, date);
+            if (date.isValid()) return this._dateLabel(value, date);
           }
-          if (type === "text") return this._textLabel(mainProperty, value);
-          if (!value) return this._undefinedLabel();
+          if (type === "text") return this._textLabel(value);
 
           return {
-            property: mainProperty,
             value,
-            type: type || "string",
-            formatedValue: escapeExpression(value)
+            type,
+            property: mainProperty,
+            formatedValue: value ? escapeExpression(value) : "-"
           };
         }
       };
     });
   },
 
-  _undefinedLabel() {
-    return {
-      value: null,
-      formatedValue: "-",
-      type: "undefined"
-    };
-  },
-
   _userLabel(properties, row) {
     const username = row[properties.username];
 
-    if (!username) return this._undefinedLabel();
+    const formatedValue = () => {
+      const userId = row[properties.id];
 
-    const user = Ember.Object.create({
-      username,
-      name: formatUsername(username),
-      avatar_template: row[properties.avatar]
-    });
+      const user = Ember.Object.create({
+        username,
+        name: formatUsername(username),
+        avatar_template: row[properties.avatar]
+      });
 
-    const avatarImg = renderAvatar(user, {
-      imageSize: "tiny",
-      ignoreTitle: true
-    });
+      const href = `/admin/users/${userId}/${username}`;
 
-    const href = `/admin/users/${row[properties.id]}/${username}`;
+      const avatarImg = renderAvatar(user, {
+        imageSize: "tiny",
+        ignoreTitle: true
+      });
+
+      return `<a href='${href}'>${avatarImg}<span class='username'>${
+        user.name
+      }</span></a>`;
+    };
 
     return {
-      type: "user",
-      property: properties.username,
       value: username,
-      formatedValue: `<a href='${href}'>${avatarImg}<span class='username'>${username}</span></a>`
+      formatedValue: username ? formatedValue(username) : "-"
     };
   },
 
@@ -339,8 +328,6 @@ const Report = Discourse.Model.extend({
     const href = `/t/-/${topicId}`;
 
     return {
-      type: "topic",
-      property: properties.title,
       value: topicTitle,
       formatedValue: `<a href='${href}'>${topicTitle}</a>`
     };
@@ -353,72 +340,67 @@ const Report = Discourse.Model.extend({
     const href = `/t/-/${topicId}/${postNumber}`;
 
     return {
-      type: "post",
       property: properties.title,
       value: postTitle,
       formatedValue: `<a href='${href}'>${postTitle}</a>`
     };
   },
 
-  _secondsLabel(property, value) {
+  _secondsLabel(value) {
     return {
       value,
-      property,
-      countable: true,
-      type: "seconds",
       formatedValue: durationTiny(value)
     };
   },
 
-  _percentLabel(property, value) {
+  _percentLabel(value) {
     return {
-      type: "percent",
-      property,
       value,
-      formatedValue: `${value}%`
+      formatedValue: value ? `${value}%` : "-"
     };
   },
 
-  _numberLabel(property, value) {
+  _numberLabel(value, options = {}) {
+    const formatNumbers = Ember.isEmpty(options.formatNumbers)
+      ? true
+      : options.formatNumbers;
+
+    const formatedValue = () => (formatNumbers ? number(value) : value);
+
     return {
-      type: "number",
-      countable: true,
-      property,
       value,
-      formatedValue: number(value)
+      formatedValue: value ? formatedValue() : "-"
     };
   },
 
-  _dateLabel(property, value, date) {
+  _dateLabel(value, date) {
     return {
-      type: "date",
-      property,
       value,
-      formatedValue: date.format("LL")
+      formatedValue: value ? date.format("LL") : "-"
     };
   },
 
-  _textLabel(property, value) {
+  _textLabel(value) {
     const escaped = escapeExpression(value);
 
     return {
-      type: "text",
-      property,
       value,
-      formatedValue: escaped
+      formatedValue: value ? escaped : "-"
     };
   },
 
   _linkLabel(properties, row) {
     const property = properties[0];
     const value = row[property];
+    const formatedValue = (href, anchor) => {
+      return `<a href="${escapeExpression(href)}">${escapeExpression(
+        anchor
+      )}</a>`;
+    };
+
     return {
-      type: "link",
-      property,
       value,
-      formatedValue: `<a href="${escapeExpression(
-        row[properties[1]]
-      )}">${escapeExpression(value)}</a>`
+      formatedValue: value ? formatedValue(value, row[properties[1]]) : "-"
     };
   },
 

app/assets/javascripts/admin/templates/components/admin-report-table-cell.hbs

@@ -0,0 +1 @@
+{{{formatedValue}}}

app/assets/javascripts/admin/templates/components/admin-report-table-row.hbs

@@ -1,5 +1,3 @@
-{{#each cells as |cell|}}
-  <td class="{{cell.type}} {{cell.property}}" title="{{cell.tooltip}}">
-    {{{cell.formatedValue}}}
-  </td>
+{{#each labels as |label|}}
+  {{admin-report-table-cell label=label data=data options=options}}
 {{/each}}

app/assets/javascripts/admin/templates/components/admin-report-table.hbs

@@ -19,7 +19,7 @@
   </thead>
   <tbody>
     {{#each paginatedData as |data|}}
-      {{admin-report-table-row data=data labels=model.computedLabels}}
+      {{admin-report-table-row data=data labels=model.computedLabels options=options}}
     {{/each}}
 
     {{#if showTotalForSample}}
@@ -30,7 +30,7 @@
       </tr>
       <tr class="admin-report-table-row">
         {{#each totalsForSample as |total|}}
-          <td class="admin-report-table-row {{total.type}} {{total.property}}">
+          <td class="{{total.type}} {{total.property}}">
             {{total.formatedValue}}
           </td>
         {{/each}}

app/models/report.rb

@@ -296,6 +296,19 @@ class Report
   end
 
   def self.report_dau_by_mau(report)
+    report.labels = [
+      {
+        type: :date,
+        property: :x,
+        title: I18n.t("reports.default.labels.day")
+      },
+      {
+        type: :percent,
+        property: :y,
+        title: I18n.t("reports.default.labels.percent")
+      },
+    ]
+
     report.average = true
     report.percent = true
 
@@ -441,6 +454,7 @@ class Report
       },
       {
         property: :y,
+        type: :number,
         title: I18n.t("reports.default.labels.count")
       }
     ]
@@ -537,6 +551,7 @@ class Report
       },
       {
         property: :count,
+        type: :number,
         title: I18n.t("reports.web_crawlers.labels.page_views")
       }
     ]
@@ -562,6 +577,7 @@ class Report
       },
       {
         property: :y,
+        type: :number,
         title: I18n.t("reports.default.labels.count")
       }
     ]
@@ -596,6 +612,7 @@ class Report
       },
       {
         property: :num_clicks,
+        type: :number,
         title: I18n.t("reports.top_referred_topics.labels.num_clicks")
       }
     ]
@@ -616,10 +633,12 @@ class Report
       },
       {
         property: :num_clicks,
+        type: :number,
         title: I18n.t("reports.top_traffic_sources.labels.num_clicks")
       },
       {
         property: :num_topics,
+        type: :number,
         title: I18n.t("reports.top_traffic_sources.labels.num_topics")
       }
     ]
@@ -638,6 +657,7 @@ class Report
       },
       {
         property: :unique_searches,
+        type: :number,
         title: I18n.t("reports.trending_search.labels.searches")
       },
       {
@@ -696,6 +716,7 @@ class Report
       },
       {
         property: :flag_count,
+        type: :number,
         title: I18n.t("reports.moderators_activity.labels.flag_count")
       },
       {
@@ -705,18 +726,22 @@ class Report
       },
       {
         property: :topic_count,
+        type: :number,
         title: I18n.t("reports.moderators_activity.labels.topic_count")
       },
       {
         property: :pm_count,
+        type: :number,
         title: I18n.t("reports.moderators_activity.labels.pm_count")
       },
       {
         property: :post_count,
+        type: :number,
         title: I18n.t("reports.moderators_activity.labels.post_count")
       },
       {
         property: :revision_count,
+        type: :number,
         title: I18n.t("reports.moderators_activity.labels.revision_count")
       }
     ]

config/locales/server.en.yml

@@ -850,6 +850,7 @@ en:
     default:
       labels:
         count: Count
+        percent: Percent
         day: Day
     post_edits:
       title: "Post edits"

test/javascripts/models/report-test.js.es6

@@ -418,7 +418,7 @@ QUnit.test("computed labels", assert => {
       },
       title: "Moderator"
     },
-    { property: "flag_count", title: "Flag count" },
+    { type: "number", property: "flag_count", title: "Flag count" },
     { type: "seconds", property: "time_read", title: "Time read" },
     { type: "text", property: "note", title: "Note" },
     {
@@ -453,62 +453,66 @@ QUnit.test("computed labels", assert => {
   assert.equal(usernameLabel.mainProperty, "username");
   assert.equal(usernameLabel.sortProperty, "username");
   assert.equal(usernameLabel.title, "Moderator");
+  assert.equal(usernameLabel.type, "user");
   const computedUsernameLabel = usernameLabel.compute(row);
   assert.equal(
     computedUsernameLabel.formatedValue,
     "<a href='/admin/users/1/joffrey'><img alt='' width='20' height='20' src='/' class='avatar' title='joffrey'><span class='username'>joffrey</span></a>"
   );
-  assert.equal(computedUsernameLabel.type, "user");
   assert.equal(computedUsernameLabel.value, "joffrey");
 
   const flagCountLabel = computedLabels[1];
   assert.equal(flagCountLabel.mainProperty, "flag_count");
   assert.equal(flagCountLabel.sortProperty, "flag_count");
   assert.equal(flagCountLabel.title, "Flag count");
-  const computedFlagCountLabel = flagCountLabel.compute(row);
+  assert.equal(flagCountLabel.type, "number");
+  let computedFlagCountLabel = flagCountLabel.compute(row);
   assert.equal(computedFlagCountLabel.formatedValue, "1.9k");
-  assert.equal(computedFlagCountLabel.type, "number");
   assert.equal(computedFlagCountLabel.value, 1876);
+  computedFlagCountLabel = flagCountLabel.compute(row, {
+    formatNumbers: false
+  });
+  assert.equal(computedFlagCountLabel.formatedValue, 1876);
 
   const timeReadLabel = computedLabels[2];
   assert.equal(timeReadLabel.mainProperty, "time_read");
   assert.equal(timeReadLabel.sortProperty, "time_read");
   assert.equal(timeReadLabel.title, "Time read");
+  assert.equal(timeReadLabel.type, "seconds");
   const computedTimeReadLabel = timeReadLabel.compute(row);
   assert.equal(computedTimeReadLabel.formatedValue, "3d");
-  assert.equal(computedTimeReadLabel.type, "seconds");
   assert.equal(computedTimeReadLabel.value, 287362);
 
   const noteLabel = computedLabels[3];
   assert.equal(noteLabel.mainProperty, "note");
   assert.equal(noteLabel.sortProperty, "note");
   assert.equal(noteLabel.title, "Note");
+  assert.equal(noteLabel.type, "text");
   const computedNoteLabel = noteLabel.compute(row);
   assert.equal(computedNoteLabel.formatedValue, "This is a long note");
-  assert.equal(computedNoteLabel.type, "text");
   assert.equal(computedNoteLabel.value, "This is a long note");
 
   const topicLabel = computedLabels[4];
   assert.equal(topicLabel.mainProperty, "topic_title");
   assert.equal(topicLabel.sortProperty, "topic_title");
   assert.equal(topicLabel.title, "Topic");
+  assert.equal(topicLabel.type, "topic");
   const computedTopicLabel = topicLabel.compute(row);
   assert.equal(
     computedTopicLabel.formatedValue,
     "<a href='/t/-/2'>Test topic</a>"
   );
-  assert.equal(computedTopicLabel.type, "topic");
   assert.equal(computedTopicLabel.value, "Test topic");
 
   const postLabel = computedLabels[5];
   assert.equal(postLabel.mainProperty, "post_raw");
   assert.equal(postLabel.sortProperty, "post_raw");
   assert.equal(postLabel.title, "Post");
+  assert.equal(postLabel.type, "post");
   const computedPostLabel = postLabel.compute(row);
   assert.equal(
     computedPostLabel.formatedValue,
     "<a href='/t/-/2/3'>This is the beginning of</a>"
   );
-  assert.equal(computedPostLabel.type, "post");
   assert.equal(computedPostLabel.value, "This is the beginning of");
 });