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