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

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.

diff --git a/app/assets/javascripts/admin/addon/components/admin-report-chart.js b/app/assets/javascripts/admin/addon/components/admin-report-chart.js
index 8a894b9..9e2d382 100644
--- a/app/assets/javascripts/admin/addon/components/admin-report-chart.js
+++ b/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 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";
-    }
+    return Report.collapse(model, data, options.chartGrouping);
   },
 });
diff --git a/app/assets/javascripts/admin/addon/components/admin-report-stacked-chart.js b/app/assets/javascripts/admin/addon/components/admin-report-stacked-chart.js
index 4cac3b1..4c3a9bb 100644
--- a/app/assets/javascripts/admin/addon/components/admin-report-stacked-chart.js
+++ b/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",
-                minUnit: "day",
+                unit: Report.unitForDatapoints(data.labels.length),
               },
               ticks: {
                 sampleSize: 5,
diff --git a/app/assets/javascripts/admin/addon/components/admin-report.js b/app/assets/javascripts/admin/addon/components/admin-report.js
index 2fe0ab6..5fade62 100644
--- a/app/assets/javascripts/admin/addon/components/admin-report.js
+++ b/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")
-  chartGroupings(chartGrouping) {
-    chartGrouping = chartGrouping || "daily";
+  @discourseComputed("options.chartGrouping", "model.chartData.length")
+  chartGroupings(grouping, count) {
+    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(

[... diff too long, it was truncated ...]

GitHub sha: 4c3d2267b4893fc36e38e53de93b7ad59ca18463

This commit appears in #13399 which was approved by eviltrout. It was merged by jjaffeux.