FIX: ensure we dont collapse data multiple times (#13399)

Note that this commit will also disable daily grouping for datasets with more than 30 data points. This will also smartly do the grouping by month when grouping a full year.

app/assets/javascripts/admin/addon/components/admin-report-chart.js

@@ -1,3 +1,4 @@
+import Report from "admin/models/report";
 import Component from "@ember/component";
 import discourseDebounce from "discourse-common/lib/debounce";
 import loadScript from "discourse/lib/load-script";
@@ -157,7 +158,7 @@ export default Component.extend({
               gridLines: { display: false },
               type: "time",
               time: {
-                unit: this._unitForGrouping(options),
+                unit: Report.unitForGrouping(options.chartGrouping),
               },
               ticks: {
                 sampleSize: 5,
@@ -179,62 +180,6 @@ export default Component.extend({
   },
 
   _applyChartGrouping(model, data, options) {
-    if (!options.chartGrouping || options.chartGrouping === "daily") {
+    return Report.collapse(model, data, options.chartGrouping);
-      return data;
-    }
-
-    if (
-      options.chartGrouping === "weekly" ||
-      options.chartGrouping === "monthly"
-    ) {
-      const isoKind = options.chartGrouping === "weekly" ? "isoWeek" : "month";
-      const kind = options.chartGrouping === "weekly" ? "week" : "month";
-      const startMoment = moment(model.start_date, "YYYY-MM-DD");
-
-      let currentIndex = 0;
-      let currentStart = startMoment.clone().startOf(isoKind);
-      let currentEnd = startMoment.clone().endOf(isoKind);
-      const transformedData = [
-        {
-          x: currentStart.format("YYYY-MM-DD"),
-          y: 0,
-        },
-      ];
-
-      data.forEach((d) => {
-        let date = moment(d.x, "YYYY-MM-DD");
-
-        if (!date.isBetween(currentStart, currentEnd)) {
-          currentIndex += 1;
-          currentStart = currentStart.add(1, kind).startOf(isoKind);
-          currentEnd = currentEnd.add(1, kind).endOf(isoKind);
-        }
-
-        if (transformedData[currentIndex]) {
-          transformedData[currentIndex].y += d.y;
-        } else {
-          transformedData[currentIndex] = {
-            x: d.x,
-            y: d.y,
-          };
-        }
-      });
-
-      return transformedData;
-    }
-
-    // ensure we return something if grouping is unknown
-    return data;
-  },
-
-  _unitForGrouping(options) {
-    switch (options.chartGrouping) {
-      case "monthly":
-        return "month";
-      case "weekly":
-        return "week";
-      default:
-        return "day";
-    }
   },
 });

app/assets/javascripts/admin/addon/components/admin-report-stacked-chart.js

@@ -1,3 +1,4 @@
+import Report from "admin/models/report";
 import Component from "@ember/component";
 import discourseDebounce from "discourse-common/lib/debounce";
 import loadScript from "discourse/lib/load-script";
@@ -63,7 +64,7 @@ export default Component.extend({
         return {
           label: cd.label,
           stack: "pageviews-stack",
-          data: cd.data.map((d) => Math.round(parseFloat(d.y))),
+          data: Report.collapse(model, cd.data),
           backgroundColor: cd.color,
         };
       }),
@@ -129,15 +130,14 @@ export default Component.extend({
               },
             },
           ],
+
           xAxes: [
             {
               display: true,
               gridLines: { display: false },
               type: "time",
-              offset: true,
               time: {
-                parser: "YYYY-MM-DD",
+                unit: Report.unitForDatapoints(data.labels.length),
-                minUnit: "day",
               },
               ticks: {
                 sampleSize: 5,

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

@@ -1,5 +1,5 @@
 import EmberObject, { action, computed } from "@ember/object";
-import Report, { SCHEMA_VERSION } from "admin/models/report";
+import Report, { DAILY_LIMIT_DAYS, SCHEMA_VERSION } from "admin/models/report";
 import { alias, and, equal, notEmpty, or } from "@ember/object/computed";
 import Component from "@ember/component";
 import I18n from "I18n";
@@ -21,26 +21,6 @@ const TABLE_OPTIONS = {
 
 const CHART_OPTIONS = {};
 
-function collapseWeekly(data, average) {
-  let aggregate = [];
-  let bucket, i;
-  let offset = data.length % 7;
-  for (i = offset; i < data.length; i++) {
-    if (bucket && i % 7 === offset) {
-      if (average) {
-        bucket.y = parseFloat((bucket.y / 7.0).toFixed(2));
-      }
-      aggregate.push(bucket);
-      bucket = null;
-    }
-
-    bucket = bucket || { x: data[i].x, y: 0 };
-    bucket.y += data[i].y;
-  }
-
-  return aggregate;
-}
-
 export default Component.extend({
   classNameBindings: [
     "isHidden:hidden",
@@ -99,6 +79,10 @@ export default Component.extend({
     }
     this.set("endDate", endDate);
 
+    if (this.filters) {
+      this.set("currentMode", this.filters.mode);
+    }
+
     if (this.report) {
       this._renderReport(this.report, this.forcedModes, this.currentMode);
     } else if (this.dataSourceName) {
@@ -147,7 +131,7 @@ export default Component.extend({
 
     return makeArray(modes).map((mode) => {
       const base = `btn-default mode-btn ${mode}`;
-      const cssClass = currentMode === mode ? `${base} is-current` : base;
+      const cssClass = currentMode === mode ? `${base} btn-primary` : base;
 
       return {
         mode,
@@ -196,15 +180,16 @@ export default Component.extend({
     return reportKey;
   },
 
-  @discourseComputed("reportOptions.chartGrouping")
+  @discourseComputed("options.chartGrouping", "model.chartData.length")
-  chartGroupings(chartGrouping) {
+  chartGroupings(grouping, count) {
-    chartGrouping = chartGrouping || "daily";
+    const options = ["daily", "weekly", "monthly"];
 
-    return ["daily", "weekly", "monthly"].map((id) => {
+    return options.map((id) => {
       return {
         id,
+        disabled: id === "daily" && count >= DAILY_LIMIT_DAYS,
         label: `admin.dashboard.reports.${id}`,
-        class: `chart-grouping ${chartGrouping === id ? "active" : "inactive"}`,
+        class: `chart-grouping ${grouping === id ? "active" : "inactive"}`,
       };
     });
   },
@@ -240,6 +225,7 @@ export default Component.extend({
 
     this.attrs.onRefresh({
       type: this.get("model.type"),
+      mode: this.currentMode,
       chartGrouping: options.chartGrouping,
       startDate:
         typeof options.startDate === "undefined"
@@ -271,7 +257,7 @@ export default Component.extend({
   },
 
   @action
-  changeMode(mode) {
+  onChangeMode(mode) {
     this.set("currentMode", mode);
 
     this.send("refreshReport", {
@@ -329,7 +315,7 @@ export default Component.extend({
     this.setProperties({
       model: report,
       currentMode,
-      options: this._buildOptions(currentMode),
+      options: this._buildOptions(currentMode, report),
     });
   },
 
@@ -391,7 +377,7 @@ export default Component.extend({
     return payload;
   },
 
-  _buildOptions(mode) {
+  _buildOptions(mode, report) {
     if (mode === "table") {
       const tableOptions = JSON.parse(JSON.stringify(TABLE_OPTIONS));
       return EmberObject.create(
@@ -401,7 +387,9 @@ export default Component.extend({
       const chartOptions = JSON.parse(JSON.stringify(CHART_OPTIONS));
       return EmberObject.create(
         Object.assign(chartOptions, this.get("reportOptions.chart") || {}, {
-          chartGrouping: this.get("reportOptions.chartGrouping"),
+          chartGrouping:
+            this.get("reportOptions.chartGrouping") ||
+            Report.groupingForDatapoints(report.chartData.length),
         })
       );
     }
@@ -414,7 +402,7 @@ export default Component.extend({
       jsonReport.chartData = jsonReport.chartData.map((chartData) => {
         if (chartData.length > 40) {
           return {
-            data: collapseWeekly(chartData.data),
+            data: chartData.data,
             req: chartData.req,
             label: chartData.label,
             color: chartData.color,
@@ -423,11 +411,6 @@ export default Component.extend({
           return chartData;
         }
       });
-    } else if (jsonReport.chartData && jsonReport.chartData.length > 40) {
-      jsonReport.chartData = collapseWeekly(
-        jsonReport.chartData,
-        jsonReport.average
-      );
     }
 
     if (jsonReport.prev_data) {
@@ -437,13 +420,6 @@ export default Component.extend({
         starDate: jsonReport.prev_startDate,
         endDate: jsonReport.prev_endDate,
       });
-
-      if (jsonReport.prevChartData && jsonReport.prevChartData.length > 40) {
-        jsonReport.prevChartData = collapseWeekly(
-          jsonReport.prevChartData,
-          jsonReport.average
-        );
-      }
     }
 
     return Report.create(jsonReport);

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

@@ -2,7 +2,7 @@ import Controller from "@ember/controller";
 import discourseComputed from "discourse-common/utils/decorators";
 
 export default Controller.extend({
-  queryParams: ["start_date", "end_date", "filters", "chart_grouping"],
+  queryParams: ["start_date", "end_date", "filters", "chart_grouping", "mode"],
   start_date: null,
   end_date: null,
   filters: null,

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

@@ -503,7 +503,94 @@ const Report = EmberObject.extend({
   },
 });
 
+export const WEEKLY_LIMIT_DAYS = 365;
+export const DAILY_LIMIT_DAYS = 30;
+
 Report.reopenClass({
+  groupingForDatapoints(count) {
+    if (count < DAILY_LIMIT_DAYS) {
+      return "daily";
+    }
+
+    if (count >= DAILY_LIMIT_DAYS && count < WEEKLY_LIMIT_DAYS) {
+      return "weekly";
+    }
+
+    if (count >= WEEKLY_LIMIT_DAYS) {
+      return "monthly";
+    }
+  },
+
+  unitForDatapoints(count) {
+    if (count >= DAILY_LIMIT_DAYS && count < WEEKLY_LIMIT_DAYS) {
+      return "week";
+    } else if (count >= WEEKLY_LIMIT_DAYS) {
+      return "month";
+    } else {
+      return "day";
+    }
+  },
+
+  unitForGrouping(grouping) {
+    switch (grouping) {
+      case "monthly":
+        return "month";
+      case "weekly":
+        return "week";
+      default:
+        return "day";
+    }
+  },
+
+  collapse(model, data, grouping) {
+    grouping = grouping || Report.groupingForDatapoints(data.length);
+
+    if (grouping === "daily") {
+      return data;
+    } else if (grouping === "weekly" || grouping === "monthly") {
+      const isoKind = grouping === "weekly" ? "isoWeek" : "month";
+      const kind = grouping === "weekly" ? "week" : "month";
+      const startMoment = moment(model.start_date, "YYYY-MM-DD");
+
+      let currentIndex = 0;
+      let currentStart = startMoment.clone().startOf(isoKind);
+      let currentEnd = startMoment.clone().endOf(isoKind);
+      const transformedData = [
+        {
+          x: currentStart.format("YYYY-MM-DD"),
+          y: 0,
+        },
+      ];
+
+      data.forEach((d) => {
+        const date = moment(d.x, "YYYY-MM-DD");
+
+        if (
+          !date.isSame(currentStart) &&
+          !date.isBetween(currentStart, currentEnd)
+        ) {
+          currentIndex += 1;
+          currentStart = currentStart.add(1, kind).startOf(isoKind);
+          currentEnd = currentEnd.add(1, kind).endOf(isoKind);
+        }
+
+        if (transformedData[currentIndex]) {
+          transformedData[currentIndex].y += d.y;
+        } else {
+          transformedData[currentIndex] = {
+            x: d.x,
+            y: d.y,
+          };
+        }
+      });
+
+      return transformedData;
+    }
+
+    // ensure we return something if grouping is unknown
+    return data;
+  },
+
   fillMissingDates(report, options = {}) {
     const dataField = options.dataField || "data";
     const filledField = options.filledField || "data";

app/assets/javascripts/admin/addon/routes/admin-reports-show.js

@@ -6,6 +6,7 @@ export default DiscourseRoute.extend({
     end_date: { refreshModel: true },
     filters: { refreshModel: true },
     chart_grouping: { refreshModel: true },
+    mode: { refreshModel: true },
   },
 
   model(params) {
@@ -28,6 +29,8 @@ export default DiscourseRoute.extend({
     params.chartGrouping = params.chart_grouping || "daily";
     delete params.chart_grouping;
 
+    params.mode = params.mode || "table";
+
     return params;
   },
 
@@ -55,6 +58,7 @@ export default DiscourseRoute.extend({
     onParamsChange(params) {
       const queryParams = {
         type: params.type,
+        mode: params.mode,
         start_date: params.startDate
           ? params.startDate.toISOString(true).split("T")[0]
           : null,

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

@@ -122,7 +122,7 @@
               <div class="modes">
                 {{#each displayedModes as |displayedMode|}}
                   {{d-button
-                    action=(action "changeMode")
+                    action=(action "onChangeMode")
                     actionParam=displayedMode.mode
                     class=displayedMode.cssClass
                     icon=displayedMode.icon}}
@@ -137,6 +137,7 @@
                     label=chartGrouping.label
                     action=(action "changeGrouping" chartGrouping.id)
                     class=chartGrouping.class
+                    disabled=chartGrouping.disabled
                   }}
                 {{/each}}
               </div>