tejasajmera commented on a change in pull request #6333:
URL: https://github.com/apache/incubator-pinot/pull/6333#discussion_r538844285



##########
File path: 
thirdeye/thirdeye-frontend/app/pods/components/rootcause-metrics/component.js
##########
@@ -82,74 +91,91 @@ export default Component.extend({
    *  ]
    * }
    */
-  links: computed(
-    'entities',
-    function() {
-      const { entities } = getProperties(this, 'entities');
-      let metricUrlMapping = {};
-
-      filterPrefix(Object.keys(entities), 'thirdeye:metric:')
-        .forEach(urn => {
-          const attributes = entities[urn].attributes;
-          const { externalUrls = [] } = attributes;
-          let urlArr = [];
-
-          // Add the list of urls for each url type
-          externalUrls.forEach(urlLabel => {
-            urlArr.push({
-              [urlLabel]: attributes[urlLabel][0] // each type should only 
have 1 url
-            });
-          });
-
-          // Map all the url lists to a metric urn
-          metricUrlMapping[urn] = urlArr;
+  links: computed('entities', function () {
+    const { entities } = getProperties(this, 'entities');
+    let metricUrlMapping = {};
+
+    filterPrefix(Object.keys(entities), 'thirdeye:metric:').forEach((urn) => {
+      const attributes = entities[urn].attributes;
+      const { externalUrls = [] } = attributes;
+      let urlArr = [];
+
+      // Add the list of urls for each url type
+      externalUrls.forEach((urlLabel) => {
+        urlArr.push({
+          [urlLabel]: attributes[urlLabel][0] // each type should only have 1 
url
         });
+      });
 
-      return metricUrlMapping;
-    }
-  ),
+      // Map all the url lists to a metric urn
+      metricUrlMapping[urn] = urlArr;
+    });
+
+    return metricUrlMapping;
+  }),
 
   /**
    * Data for metrics table
    * @type Object[] - array of objects, each corresponding to a row in the 
table
    */
-  metricsTableData: computed(
-    'selectedUrns',
-    'entities',
-    'aggregates',
-    'scores',
-    'links',
-    function() {
-      const { selectedUrns, entities, aggregates, scores, links } =
-        getProperties(this, 'selectedUrns', 'entities', 'aggregates', 
'scores', 'links');
-
-      const rows = filterPrefix(Object.keys(entities), 'thirdeye:metric:')
-        .map(urn => {
-          return {
-            urn,
-            links: links[urn],
-            isSelected: selectedUrns.has(urn),
-            label: toMetricLabel(urn, entities),
-            dataset: toMetricDataset(urn, entities),
-            score: humanizeScore(scores[urn]),
-            current: this._makeRecord(urn, 'current', entities, aggregates),
-            baseline: this._makeRecord(urn, 'baseline', entities, aggregates),
-            wo1w: this._makeRecord(urn, 'wo1w', entities, aggregates),
-            wo2w: this._makeRecord(urn, 'wo2w', entities, aggregates),
-            sortable_current: this._makeChange(urn, 'current', aggregates),
-            sortable_baseline: this._makeChange(urn, 'baseline', aggregates),
-            sortable_wo1w: this._makeChange(urn, 'wo1w', aggregates),
-            sortable_wo2w: this._makeChange(urn, 'wo2w', aggregates),
-            isExclusionWarning: isExclusionWarning(urn, entities)
-          };
-        });
+  metricsTableData: computed('selectedUrns', 'entities', 'aggregates', 
'scores', 'links', 'compareMode', function () {
+    const { selectedUrns, entities, aggregates, scores, links, compareMode } = 
getProperties(
+      this,
+      'selectedUrns',
+      'entities',
+      'aggregates',
+      'scores',
+      'links',
+      'compareMode'
+    );
+    let rows;
 
-      return _.sortBy(rows, (row) => row.label);
+    if (compareMode === 'forecast') {
+      rows = filterPrefix(Object.keys(entities), 'thirdeye:metric:').map((urn) 
=> {
+        return {
+          urn,
+          isSelected: selectedUrns.has(urn),
+          label: toMetricLabel(urn, entities),
+          dataset: toMetricDataset(urn, entities),
+          score: humanizeScore(scores[urn]),
+          current: this._makeRecord(urn, 'current', entities, aggregates),
+          baseline: this._makeRecord(urn, 'baseline', entities, aggregates),
+          yoy: this._makeRecord(urn, 'yoy', entities, aggregates),
+          interval: this._makeIntervalString(urn, aggregates),
+          inInterval: this._isInInterval(urn, aggregates),
+          sortable_current: this._makeChange(urn, 'current', aggregates),
+          sortable_baseline: this._makeChange(urn, 'baseline', aggregates),
+          sortable_yoy: this._makeChange(urn, 'yoy', aggregates),
+          isExclusionWarning: isExclusionWarning(urn, entities)
+        };
+      });
+    } else {
+      rows = filterPrefix(Object.keys(entities), 'thirdeye:metric:').map((urn) 
=> {
+        return {
+          urn,
+          links: links[urn],
+          isSelected: selectedUrns.has(urn),
+          label: toMetricLabel(urn, entities),
+          dataset: toMetricDataset(urn, entities),
+          score: humanizeScore(scores[urn]),
+          current: this._makeRecord(urn, 'current', entities, aggregates),
+          baseline: this._makeRecord(urn, 'baseline', entities, aggregates),

Review comment:
       `urn`, `isSelected`, `label`, `dataset`, `score`, `current`, `baseline`, 
`isExclusionWarning` properties are same in both the `if` and `else` sections. 
Lets pull those out into a common object before the `if..else` and destructure 
it in that object in `return` in both `if..else` before proceeding to add 
unique properties in each.
   

##########
File path: 
thirdeye/thirdeye-frontend/app/pods/components/rootcause-metrics/component.js
##########
@@ -82,74 +91,91 @@ export default Component.extend({
    *  ]
    * }
    */
-  links: computed(
-    'entities',
-    function() {
-      const { entities } = getProperties(this, 'entities');
-      let metricUrlMapping = {};
-
-      filterPrefix(Object.keys(entities), 'thirdeye:metric:')
-        .forEach(urn => {
-          const attributes = entities[urn].attributes;
-          const { externalUrls = [] } = attributes;
-          let urlArr = [];
-
-          // Add the list of urls for each url type
-          externalUrls.forEach(urlLabel => {
-            urlArr.push({
-              [urlLabel]: attributes[urlLabel][0] // each type should only 
have 1 url
-            });
-          });
-
-          // Map all the url lists to a metric urn
-          metricUrlMapping[urn] = urlArr;
+  links: computed('entities', function () {
+    const { entities } = getProperties(this, 'entities');
+    let metricUrlMapping = {};
+
+    filterPrefix(Object.keys(entities), 'thirdeye:metric:').forEach((urn) => {
+      const attributes = entities[urn].attributes;
+      const { externalUrls = [] } = attributes;
+      let urlArr = [];
+
+      // Add the list of urls for each url type
+      externalUrls.forEach((urlLabel) => {
+        urlArr.push({
+          [urlLabel]: attributes[urlLabel][0] // each type should only have 1 
url
         });
+      });
 
-      return metricUrlMapping;
-    }
-  ),
+      // Map all the url lists to a metric urn
+      metricUrlMapping[urn] = urlArr;
+    });
+
+    return metricUrlMapping;
+  }),
 
   /**
    * Data for metrics table
    * @type Object[] - array of objects, each corresponding to a row in the 
table
    */
-  metricsTableData: computed(
-    'selectedUrns',
-    'entities',
-    'aggregates',
-    'scores',
-    'links',
-    function() {
-      const { selectedUrns, entities, aggregates, scores, links } =
-        getProperties(this, 'selectedUrns', 'entities', 'aggregates', 
'scores', 'links');
-
-      const rows = filterPrefix(Object.keys(entities), 'thirdeye:metric:')
-        .map(urn => {
-          return {
-            urn,
-            links: links[urn],
-            isSelected: selectedUrns.has(urn),
-            label: toMetricLabel(urn, entities),
-            dataset: toMetricDataset(urn, entities),
-            score: humanizeScore(scores[urn]),
-            current: this._makeRecord(urn, 'current', entities, aggregates),
-            baseline: this._makeRecord(urn, 'baseline', entities, aggregates),
-            wo1w: this._makeRecord(urn, 'wo1w', entities, aggregates),
-            wo2w: this._makeRecord(urn, 'wo2w', entities, aggregates),
-            sortable_current: this._makeChange(urn, 'current', aggregates),
-            sortable_baseline: this._makeChange(urn, 'baseline', aggregates),
-            sortable_wo1w: this._makeChange(urn, 'wo1w', aggregates),
-            sortable_wo2w: this._makeChange(urn, 'wo2w', aggregates),
-            isExclusionWarning: isExclusionWarning(urn, entities)
-          };
-        });
+  metricsTableData: computed('selectedUrns', 'entities', 'aggregates', 
'scores', 'links', 'compareMode', function () {
+    const { selectedUrns, entities, aggregates, scores, links, compareMode } = 
getProperties(
+      this,
+      'selectedUrns',
+      'entities',
+      'aggregates',
+      'scores',
+      'links',
+      'compareMode'
+    );
+    let rows;
 
-      return _.sortBy(rows, (row) => row.label);
+    if (compareMode === 'forecast') {

Review comment:
       This comparison is happening twice in this file; so it would be better 
to constantize `forecast` so any change to the string need to happen just once. 
   
   `const FORECAST_MODE = 'forecast'`




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to