You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2020/12/14 21:12:05 UTC
[GitHub] [incubator-pinot] tejasajmera commented on a change in pull request #6345: [TE]frontend - THIRDEYE-3772 - create a new 'composite-anomalies' com…
tejasajmera commented on a change in pull request #6345:
URL: https://github.com/apache/incubator-pinot/pull/6345#discussion_r542686576
##########
File path: thirdeye/thirdeye-frontend/app/pods/components/composite-anomalies/parent-anomalies/template.hbs
##########
@@ -0,0 +1,20 @@
+<div class='panel-group'>
+ <div class='panel panel-default'>
+ <div class='panel-heading'>
+ <h4 class="panel-title">{{title}}</h4>
+ </div>
+ {{#if tableData.length}}
+ <div class='panel-body'>
+ {{models-table
+ data=tableData
+ columns=tableColumns
+ showGlobalFilter=false
+ showColumnsDropdown=false
+ customClasses=customClasses
+ }}
+ </div>
+ {{else}}
+ <p class="composite-anomalies-no-records">{{noRecords}}</p>
+ {{/if}}
+ </div>
+</div>
Review comment:
The formatting looks a bit off here. Also there is a mixed usage of single quotes and double quotes. I would suggest using https://marketplace.visualstudio.com/items?itemName=EmberTooling.prettier-for-handlebars-vscode extension for addressing those issues. You can then either change the settings to auto-format on save or format manually with selecting everything on the file and then shift+option+f .
##########
File path: thirdeye/thirdeye-frontend/app/pods/components/composite-anomalies/parent-anomalies/component.js
##########
@@ -0,0 +1,120 @@
+/*
+ * Parent Anomalies Component
+ *
+ * Display a table which contains composite anomalies
+ * @module composite-anomalies/parent-anomalies
+ * @property {string} title - Heading to use on the table
+ * @property {object[]} data - [required] array of composite anomalies objects
+ *
+ * @example
+ * {{composite-anomalies/parent-anomalies title=<title> data=<data>}}
+ *
+ * @exports composite-anomalies/parent-anomalies
+ */
+
+import Component from '@ember/component';
+import { computed } from '@ember/object';
+
+import moment from 'moment';
+import config from 'thirdeye-frontend/config/environment';
+
+let data = [];
+
+/*
+ * convert animaly 'start' and 'end' into an object to be used by template: 'custom/composite-animalies-table/start-duration'
+ */
+const getAnomaliesStartDuration = (start, end) => {
+ return {
+ startTime: start,
+ endTime: end,
+ duration: moment.duration(end - start).asHours() + ' hours'
+ };
+};
+
+/*
+ * convert list of anonalies object in to a objects
+ */
+const getAnomaliesDetails = (details) => {
+ const detailsObject = [];
+ Object.entries(details).forEach((detail) => {
+ detailsObject.push({ name: detail[0], count: detail[1] });
+ });
+
+ return detailsObject;
+};
+
+/*
+ map feedback onto a feedbackObject to be use in the feedback dropdown
+ This code assumes that `feedback` is either null when it has not be set, or an integer that can then be mapped to feedbackOptions.
+*/
+const getFeedback = (feedback = 0) => {
+ const feedbackOptions = [
+ 'Not reviewed yet',
+ 'Yes - unexpected',
+ 'Expected temporary change',
+ 'Expected permanent change',
+ 'No change observed'
+ ];
+ const selectedFeedback = feedback ? feedbackOptions[feedback] : feedbackOptions[0];
+
+ const feedbackObject = {
+ options: feedbackOptions,
+ selected: selectedFeedback
+ };
+
+ return feedbackObject;
+};
+
+export default Component.extend({
+ didReceiveAttrs() {
+ data = this.data;
+ },
+ tagName: 'section',
+ customClasses: {
+ table: 'parent-anomalies-table'
+ },
+ title: 'Composite Anomalies', // Default Header if no title is passed in.
+ noRecords: 'No Composite Anomalies found',
+ tableData: computed('data', () => {
+ let computedTableData = [];
+
+ if (data && data.length > 0) {
+ data.map((d) => {
+ const row = {
+ startDuration: getAnomaliesStartDuration(d.startTime, d.endTime),
+ anomalies: getAnomaliesDetails(d.details),
+ feedback: getFeedback(d.feedback)
+ };
+ computedTableData.push(row);
+ });
+ }
+
+ return computedTableData;
+ }),
+ tableColumns: computed(function () {
Review comment:
Not sure why this is a computed function? Computed function depends on being triggered on a property change. But this is not looking like it is depending on any property. You can instead constantize the tableColumns array outside the component.
Outside the component
```suggestion
const TABLE_COLUMNS = [
{
template: 'custom/composite-animalies-table/start-duration',
propertyName: 'startDuration',
title: `Start / Duration (${moment.tz([2012, 5], config.timeZone).format('z')})`
},
{
template: 'custom/composite-animalies-table/anomalies-list',
propertyName: 'details',
title: 'Anomalies details'
},
{
component: 'custom/composite-animalies-table/resolution',
propertyName: 'feedback',
title: 'Feedback '
}
];
```
Now within the component you can reference it,
`tableColumns: TABLE_COLUMNS`
##########
File path: thirdeye/thirdeye-frontend/app/pods/custom/composite-animalies-table/resolution/template.hbs
##########
@@ -0,0 +1,12 @@
+
Review comment:
The empty line can be removed.
##########
File path: thirdeye/thirdeye-frontend/app/pods/components/composite-anomalies/parent-anomalies/component.js
##########
@@ -0,0 +1,120 @@
+/*
+ * Parent Anomalies Component
+ *
+ * Display a table which contains composite anomalies
+ * @module composite-anomalies/parent-anomalies
+ * @property {string} title - Heading to use on the table
+ * @property {object[]} data - [required] array of composite anomalies objects
+ *
+ * @example
+ * {{composite-anomalies/parent-anomalies title=<title> data=<data>}}
+ *
+ * @exports composite-anomalies/parent-anomalies
+ */
+
+import Component from '@ember/component';
+import { computed } from '@ember/object';
+
+import moment from 'moment';
+import config from 'thirdeye-frontend/config/environment';
+
+let data = [];
+
+/*
+ * convert animaly 'start' and 'end' into an object to be used by template: 'custom/composite-animalies-table/start-duration'
+ */
Review comment:
```suggestion
I would suggest using the following format for comments explaining the function
/**
* convert anomaly 'start' and 'end' into an object to be used by template: 'custom/composite-anomalies-table/start-duration'
*
*@param {Number} start
* The start time in milliseconds.
*@param {Number} end
* The end time in milliseconds
*
*@returns {Object}
* Description of the object.
*/
```
##########
File path: thirdeye/thirdeye-frontend/app/pods/components/composite-anomalies/parent-anomalies/component.js
##########
@@ -0,0 +1,120 @@
+/*
+ * Parent Anomalies Component
+ *
+ * Display a table which contains composite anomalies
+ * @module composite-anomalies/parent-anomalies
+ * @property {string} title - Heading to use on the table
+ * @property {object[]} data - [required] array of composite anomalies objects
+ *
+ * @example
+ * {{composite-anomalies/parent-anomalies title=<title> data=<data>}}
+ *
+ * @exports composite-anomalies/parent-anomalies
+ */
+
+import Component from '@ember/component';
+import { computed } from '@ember/object';
+
+import moment from 'moment';
+import config from 'thirdeye-frontend/config/environment';
+
+let data = [];
+
+/*
+ * convert animaly 'start' and 'end' into an object to be used by template: 'custom/composite-animalies-table/start-duration'
+ */
+const getAnomaliesStartDuration = (start, end) => {
+ return {
+ startTime: start,
+ endTime: end,
+ duration: moment.duration(end - start).asHours() + ' hours'
+ };
+};
+
+/*
+ * convert list of anonalies object in to a objects
+ */
+const getAnomaliesDetails = (details) => {
+ const detailsObject = [];
+ Object.entries(details).forEach((detail) => {
+ detailsObject.push({ name: detail[0], count: detail[1] });
+ });
+
+ return detailsObject;
+};
+
+/*
+ map feedback onto a feedbackObject to be use in the feedback dropdown
+ This code assumes that `feedback` is either null when it has not be set, or an integer that can then be mapped to feedbackOptions.
+*/
+const getFeedback = (feedback = 0) => {
+ const feedbackOptions = [
+ 'Not reviewed yet',
+ 'Yes - unexpected',
+ 'Expected temporary change',
+ 'Expected permanent change',
+ 'No change observed'
+ ];
+ const selectedFeedback = feedback ? feedbackOptions[feedback] : feedbackOptions[0];
+
+ const feedbackObject = {
+ options: feedbackOptions,
+ selected: selectedFeedback
+ };
+
+ return feedbackObject;
+};
+
+export default Component.extend({
+ didReceiveAttrs() {
+ data = this.data;
+ },
+ tagName: 'section',
+ customClasses: {
+ table: 'parent-anomalies-table'
+ },
+ title: 'Composite Anomalies', // Default Header if no title is passed in.
+ noRecords: 'No Composite Anomalies found',
+ tableData: computed('data', () => {
+ let computedTableData = [];
+
+ if (data && data.length > 0) {
+ data.map((d) => {
+ const row = {
+ startDuration: getAnomaliesStartDuration(d.startTime, d.endTime),
+ anomalies: getAnomaliesDetails(d.details),
Review comment:
I'd suggest changing the property name to `anomalyDetails` to make it more representative. Also should these property names be matching `propertyName` in `tableColumns` below? I noticed the 2nd column is being referenced with `propertyName: 'details'` which is different from `anomalies` property name here. Lets try to keep these in sync to prevent any confusion.
##########
File path: thirdeye/thirdeye-frontend/app/pods/components/composite-anomalies/parent-anomalies/component.js
##########
@@ -0,0 +1,120 @@
+/*
+ * Parent Anomalies Component
+ *
+ * Display a table which contains composite anomalies
+ * @module composite-anomalies/parent-anomalies
+ * @property {string} title - Heading to use on the table
+ * @property {object[]} data - [required] array of composite anomalies objects
+ *
+ * @example
+ * {{composite-anomalies/parent-anomalies title=<title> data=<data>}}
+ *
+ * @exports composite-anomalies/parent-anomalies
+ */
+
+import Component from '@ember/component';
+import { computed } from '@ember/object';
+
+import moment from 'moment';
+import config from 'thirdeye-frontend/config/environment';
+
+let data = [];
+
+/*
+ * convert animaly 'start' and 'end' into an object to be used by template: 'custom/composite-animalies-table/start-duration'
+ */
+const getAnomaliesStartDuration = (start, end) => {
+ return {
+ startTime: start,
+ endTime: end,
+ duration: moment.duration(end - start).asHours() + ' hours'
+ };
+};
+
+/*
+ * convert list of anonalies object in to a objects
+ */
+const getAnomaliesDetails = (details) => {
+ const detailsObject = [];
+ Object.entries(details).forEach((detail) => {
+ detailsObject.push({ name: detail[0], count: detail[1] });
+ });
+
+ return detailsObject;
+};
+
+/*
+ map feedback onto a feedbackObject to be use in the feedback dropdown
+ This code assumes that `feedback` is either null when it has not be set, or an integer that can then be mapped to feedbackOptions.
+*/
+const getFeedback = (feedback = 0) => {
Review comment:
As above, would suggest moving all the component specific functions inside the component.
##########
File path: thirdeye/thirdeye-frontend/app/pods/custom/composite-animalies-table/resolution/component.js
##########
@@ -0,0 +1,18 @@
+import Component from "@ember/component";
+import { set } from '@ember/object';
+
+export default Component.extend({
+ didReceiveAttrs() {
Review comment:
This override is not needed as you are not doing any additional changes in this function.
##########
File path: thirdeye/thirdeye-frontend/app/pods/components/composite-anomalies/parent-anomalies/component.js
##########
@@ -0,0 +1,120 @@
+/*
+ * Parent Anomalies Component
+ *
+ * Display a table which contains composite anomalies
+ * @module composite-anomalies/parent-anomalies
+ * @property {string} title - Heading to use on the table
+ * @property {object[]} data - [required] array of composite anomalies objects
+ *
+ * @example
+ * {{composite-anomalies/parent-anomalies title=<title> data=<data>}}
+ *
+ * @exports composite-anomalies/parent-anomalies
+ */
+
+import Component from '@ember/component';
+import { computed } from '@ember/object';
+
+import moment from 'moment';
+import config from 'thirdeye-frontend/config/environment';
+
+let data = [];
Review comment:
You need not maintain a global state for `data`. Here is what I would do-
```suggestion
import { A as EmberArray } from '@ember/array';
Within the component,
data: EmberArray(),
```
##########
File path: thirdeye/thirdeye-frontend/app/pods/custom/composite-animalies-table/start-duration/template.hbs
##########
@@ -0,0 +1,10 @@
+
+<label class="te-label te-label--small te-anomaly-table__link">
+ <span class='start-time'>{{moment-format record.startDuration.startTime 'MMM Do, h:mm z'}}</span>
+ {{#tooltip-on-element}}
+ Start: {{moment-format record.startDuration.startTime 'MMM Do, h:mm z'}}<br />
+ End: {{moment-format record.startDuration.endTime 'MMM Do, h:mm z'}}
+ {{/tooltip-on-element}}
+</label>
+
+<p class='duration'>{{record.startDuration.duration}}</p>
Review comment:
The empty line at the top can be removed and mixed usage of quotes can be taken care of by the prettier for handlebars extension highlighted above.
##########
File path: thirdeye/thirdeye-frontend/app/pods/components/composite-anomalies/parent-anomalies/component.js
##########
@@ -0,0 +1,120 @@
+/*
+ * Parent Anomalies Component
+ *
+ * Display a table which contains composite anomalies
+ * @module composite-anomalies/parent-anomalies
+ * @property {string} title - Heading to use on the table
+ * @property {object[]} data - [required] array of composite anomalies objects
+ *
+ * @example
+ * {{composite-anomalies/parent-anomalies title=<title> data=<data>}}
+ *
+ * @exports composite-anomalies/parent-anomalies
+ */
+
+import Component from '@ember/component';
+import { computed } from '@ember/object';
+
+import moment from 'moment';
+import config from 'thirdeye-frontend/config/environment';
+
+let data = [];
+
+/*
+ * convert animaly 'start' and 'end' into an object to be used by template: 'custom/composite-animalies-table/start-duration'
+ */
+const getAnomaliesStartDuration = (start, end) => {
+ return {
+ startTime: start,
+ endTime: end,
+ duration: moment.duration(end - start).asHours() + ' hours'
+ };
+};
+
+/*
+ * convert list of anonalies object in to a objects
+ */
+const getAnomaliesDetails = (details) => {
+ const detailsObject = [];
+ Object.entries(details).forEach((detail) => {
+ detailsObject.push({ name: detail[0], count: detail[1] });
+ });
+
+ return detailsObject;
+};
+
+/*
+ map feedback onto a feedbackObject to be use in the feedback dropdown
+ This code assumes that `feedback` is either null when it has not be set, or an integer that can then be mapped to feedbackOptions.
+*/
+const getFeedback = (feedback = 0) => {
+ const feedbackOptions = [
+ 'Not reviewed yet',
+ 'Yes - unexpected',
+ 'Expected temporary change',
+ 'Expected permanent change',
+ 'No change observed'
+ ];
+ const selectedFeedback = feedback ? feedbackOptions[feedback] : feedbackOptions[0];
+
+ const feedbackObject = {
+ options: feedbackOptions,
+ selected: selectedFeedback
+ };
+
+ return feedbackObject;
+};
+
+export default Component.extend({
+ didReceiveAttrs() {
+ data = this.data;
+ },
+ tagName: 'section',
+ customClasses: {
+ table: 'parent-anomalies-table'
Review comment:
Let's use a more generic name `composite-anomalies-table` as all the different tables for composite anomalies viz. parent anomalies table, group constituents table and entity metrics table should be sharing the same styling.
##########
File path: thirdeye/thirdeye-frontend/app/styles/pods/custom/parent-anomalies-table.scss
##########
@@ -0,0 +1,20 @@
+.parent-anomalies-table {
Review comment:
Lets use `.composite-anomalies-table` as explained above.
##########
File path: thirdeye/thirdeye-frontend/app/pods/custom/composite-animalies-table/anomalies-list/template.hbs
##########
@@ -0,0 +1,4 @@
+
Review comment:
Please remove the extra empty line here. The prettier for handlebars will be able to take care of all these minor bits :)
Also, there is a typo in the folder name. `animalies -> anomalies`
##########
File path: thirdeye/thirdeye-frontend/tests/integration/pods/components/composite-anomalies/parent-anomalies/component-test.js
##########
@@ -0,0 +1,74 @@
+import $ from 'jquery';
Review comment:
I am not sure you need to explicitly import jquery. You should be able to do `this.$(any-selector)`. Please refer a sample example [here](https://github.com/apache/incubator-pinot/blob/master/thirdeye/thirdeye-frontend/tests/integration/pods/components/detection-yaml/component-test.js#L40).
##########
File path: thirdeye/thirdeye-frontend/app/pods/components/composite-anomalies/parent-anomalies/component.js
##########
@@ -0,0 +1,120 @@
+/*
+ * Parent Anomalies Component
+ *
+ * Display a table which contains composite anomalies
+ * @module composite-anomalies/parent-anomalies
+ * @property {string} title - Heading to use on the table
+ * @property {object[]} data - [required] array of composite anomalies objects
+ *
+ * @example
+ * {{composite-anomalies/parent-anomalies title=<title> data=<data>}}
+ *
+ * @exports composite-anomalies/parent-anomalies
+ */
+
+import Component from '@ember/component';
+import { computed } from '@ember/object';
+
+import moment from 'moment';
+import config from 'thirdeye-frontend/config/environment';
+
+let data = [];
+
+/*
+ * convert animaly 'start' and 'end' into an object to be used by template: 'custom/composite-animalies-table/start-duration'
+ */
+const getAnomaliesStartDuration = (start, end) => {
+ return {
+ startTime: start,
+ endTime: end,
+ duration: moment.duration(end - start).asHours() + ' hours'
+ };
+};
+
+/*
+ * convert list of anonalies object in to a objects
+ */
+const getAnomaliesDetails = (details) => {
Review comment:
As above, would suggest moving all the component specific functions inside the component.
##########
File path: thirdeye/thirdeye-frontend/app/pods/components/composite-anomalies/parent-anomalies/component.js
##########
@@ -0,0 +1,120 @@
+/*
+ * Parent Anomalies Component
+ *
+ * Display a table which contains composite anomalies
+ * @module composite-anomalies/parent-anomalies
+ * @property {string} title - Heading to use on the table
+ * @property {object[]} data - [required] array of composite anomalies objects
+ *
+ * @example
+ * {{composite-anomalies/parent-anomalies title=<title> data=<data>}}
+ *
+ * @exports composite-anomalies/parent-anomalies
+ */
+
+import Component from '@ember/component';
+import { computed } from '@ember/object';
+
+import moment from 'moment';
+import config from 'thirdeye-frontend/config/environment';
+
+let data = [];
+
+/*
+ * convert animaly 'start' and 'end' into an object to be used by template: 'custom/composite-animalies-table/start-duration'
+ */
+const getAnomaliesStartDuration = (start, end) => {
Review comment:
I would suggest moving all the component specific functions inside the component.
##########
File path: thirdeye/thirdeye-frontend/app/pods/components/composite-anomalies/parent-anomalies/component.js
##########
@@ -0,0 +1,120 @@
+/*
+ * Parent Anomalies Component
+ *
+ * Display a table which contains composite anomalies
+ * @module composite-anomalies/parent-anomalies
+ * @property {string} title - Heading to use on the table
+ * @property {object[]} data - [required] array of composite anomalies objects
+ *
+ * @example
+ * {{composite-anomalies/parent-anomalies title=<title> data=<data>}}
+ *
+ * @exports composite-anomalies/parent-anomalies
+ */
+
+import Component from '@ember/component';
+import { computed } from '@ember/object';
+
+import moment from 'moment';
+import config from 'thirdeye-frontend/config/environment';
+
+let data = [];
+
+/*
+ * convert animaly 'start' and 'end' into an object to be used by template: 'custom/composite-animalies-table/start-duration'
+ */
+const getAnomaliesStartDuration = (start, end) => {
+ return {
+ startTime: start,
+ endTime: end,
+ duration: moment.duration(end - start).asHours() + ' hours'
+ };
+};
+
+/*
+ * convert list of anonalies object in to a objects
+ */
+const getAnomaliesDetails = (details) => {
+ const detailsObject = [];
+ Object.entries(details).forEach((detail) => {
+ detailsObject.push({ name: detail[0], count: detail[1] });
+ });
+
+ return detailsObject;
Review comment:
You can rather use `reduce` function to accomplish this. The advantage is it will prevent any mutation of the return object.
```suggestion
const getAnomaliesDetails = (details) => {
return Object.entries(details).reduce((memo, detail) => {
memo.push({ name: detail[0], count: detail[1]});
return memo;
}, []);
```
##########
File path: thirdeye/thirdeye-frontend/app/pods/components/composite-anomalies/parent-anomalies/component.js
##########
@@ -0,0 +1,120 @@
+/*
+ * Parent Anomalies Component
+ *
+ * Display a table which contains composite anomalies
+ * @module composite-anomalies/parent-anomalies
+ * @property {string} title - Heading to use on the table
+ * @property {object[]} data - [required] array of composite anomalies objects
+ *
+ * @example
+ * {{composite-anomalies/parent-anomalies title=<title> data=<data>}}
+ *
+ * @exports composite-anomalies/parent-anomalies
+ */
+
+import Component from '@ember/component';
+import { computed } from '@ember/object';
+
+import moment from 'moment';
+import config from 'thirdeye-frontend/config/environment';
+
+let data = [];
+
+/*
+ * convert animaly 'start' and 'end' into an object to be used by template: 'custom/composite-animalies-table/start-duration'
+ */
+const getAnomaliesStartDuration = (start, end) => {
+ return {
+ startTime: start,
+ endTime: end,
+ duration: moment.duration(end - start).asHours() + ' hours'
+ };
+};
+
+/*
+ * convert list of anonalies object in to a objects
+ */
+const getAnomaliesDetails = (details) => {
+ const detailsObject = [];
+ Object.entries(details).forEach((detail) => {
+ detailsObject.push({ name: detail[0], count: detail[1] });
+ });
+
+ return detailsObject;
+};
+
+/*
+ map feedback onto a feedbackObject to be use in the feedback dropdown
+ This code assumes that `feedback` is either null when it has not be set, or an integer that can then be mapped to feedbackOptions.
+*/
+const getFeedback = (feedback = 0) => {
+ const feedbackOptions = [
Review comment:
Also, this mapping can live outside the function as well as the component as it is a constant. Defining mappings inside the function means there will be memory allocation to initialize it on each invocation of the function.
##########
File path: thirdeye/thirdeye-frontend/app/pods/components/composite-anomalies/parent-anomalies/component.js
##########
@@ -0,0 +1,120 @@
+/*
+ * Parent Anomalies Component
+ *
+ * Display a table which contains composite anomalies
+ * @module composite-anomalies/parent-anomalies
+ * @property {string} title - Heading to use on the table
+ * @property {object[]} data - [required] array of composite anomalies objects
+ *
+ * @example
+ * {{composite-anomalies/parent-anomalies title=<title> data=<data>}}
+ *
+ * @exports composite-anomalies/parent-anomalies
+ */
+
+import Component from '@ember/component';
+import { computed } from '@ember/object';
+
+import moment from 'moment';
+import config from 'thirdeye-frontend/config/environment';
+
+let data = [];
+
+/*
+ * convert animaly 'start' and 'end' into an object to be used by template: 'custom/composite-animalies-table/start-duration'
+ */
+const getAnomaliesStartDuration = (start, end) => {
+ return {
+ startTime: start,
+ endTime: end,
+ duration: moment.duration(end - start).asHours() + ' hours'
+ };
+};
+
+/*
+ * convert list of anonalies object in to a objects
+ */
+const getAnomaliesDetails = (details) => {
+ const detailsObject = [];
+ Object.entries(details).forEach((detail) => {
+ detailsObject.push({ name: detail[0], count: detail[1] });
+ });
+
+ return detailsObject;
+};
+
+/*
+ map feedback onto a feedbackObject to be use in the feedback dropdown
+ This code assumes that `feedback` is either null when it has not be set, or an integer that can then be mapped to feedbackOptions.
+*/
+const getFeedback = (feedback = 0) => {
+ const feedbackOptions = [
+ 'Not reviewed yet',
+ 'Yes - unexpected',
+ 'Expected temporary change',
+ 'Expected permanent change',
+ 'No change observed'
+ ];
+ const selectedFeedback = feedback ? feedbackOptions[feedback] : feedbackOptions[0];
+
+ const feedbackObject = {
+ options: feedbackOptions,
+ selected: selectedFeedback
+ };
+
+ return feedbackObject;
+};
+
+export default Component.extend({
+ didReceiveAttrs() {
+ data = this.data;
+ },
+ tagName: 'section',
+ customClasses: {
+ table: 'parent-anomalies-table'
+ },
+ title: 'Composite Anomalies', // Default Header if no title is passed in.
+ noRecords: 'No Composite Anomalies found',
+ tableData: computed('data', () => {
+ let computedTableData = [];
+
+ if (data && data.length > 0) {
+ data.map((d) => {
+ const row = {
+ startDuration: getAnomaliesStartDuration(d.startTime, d.endTime),
Review comment:
A property name of `anomalyDuration` would be more representative. Likewise I'd suggest function to be named `getAnomalyDuration` as it is giving out duration for each anomaly.
##########
File path: thirdeye/thirdeye-frontend/app/pods/custom/composite-animalies-table/resolution/component.js
##########
@@ -0,0 +1,18 @@
+import Component from "@ember/component";
+import { set } from '@ember/object';
+
+export default Component.extend({
+ didReceiveAttrs() {
+ this._super(...arguments);
+ },
+
+ actions: {
+ onChangeAnomalyResponse: (anomalyObject, selection, options ) => {
Review comment:
I am not sure I fully understand the function. This function when being referenced from the template is passing in just 1 parameter - `record` which I assume is the first argument in this function. Where do `selection` and `options` come from? Is it automatically added by `power-select`? Also, can you please add comments for this action in the format mentioned in one of my comments above?
##########
File path: thirdeye/thirdeye-frontend/app/pods/custom/composite-animalies-table/resolution/component.js
##########
@@ -0,0 +1,18 @@
+import Component from "@ember/component";
+import { set } from '@ember/object';
+
+export default Component.extend({
+ didReceiveAttrs() {
+ this._super(...arguments);
+ },
+
+ actions: {
+ onChangeAnomalyResponse: (anomalyObject, selection, options ) => {
+ set(options, 'selected', selection); // set selected option to user's selection
+ set(anomalyObject.feedback, 'selected', selection); // set anomalies feedback to user's selection
+ /*
+ TODO: add API call to update anomalies on the BackEnd
+ */
+ },
+ }
+})
Review comment:
The formatting looks a bit off here. Would suggest using https://marketplace.visualstudio.com/items?itemName=esbenp.prettier-vscode extension for addressing it.
##########
File path: thirdeye/thirdeye-frontend/app/pods/components/composite-anomalies/parent-anomalies/component.js
##########
@@ -0,0 +1,120 @@
+/*
+ * Parent Anomalies Component
+ *
+ * Display a table which contains composite anomalies
+ * @module composite-anomalies/parent-anomalies
+ * @property {string} title - Heading to use on the table
+ * @property {object[]} data - [required] array of composite anomalies objects
+ *
+ * @example
+ * {{composite-anomalies/parent-anomalies title=<title> data=<data>}}
+ *
+ * @exports composite-anomalies/parent-anomalies
+ */
+
+import Component from '@ember/component';
+import { computed } from '@ember/object';
+
+import moment from 'moment';
+import config from 'thirdeye-frontend/config/environment';
+
+let data = [];
+
+/*
+ * convert animaly 'start' and 'end' into an object to be used by template: 'custom/composite-animalies-table/start-duration'
+ */
+const getAnomaliesStartDuration = (start, end) => {
+ return {
+ startTime: start,
+ endTime: end,
+ duration: moment.duration(end - start).asHours() + ' hours'
+ };
+};
+
+/*
+ * convert list of anonalies object in to a objects
+ */
+const getAnomaliesDetails = (details) => {
+ const detailsObject = [];
+ Object.entries(details).forEach((detail) => {
+ detailsObject.push({ name: detail[0], count: detail[1] });
+ });
+
+ return detailsObject;
+};
+
+/*
+ map feedback onto a feedbackObject to be use in the feedback dropdown
+ This code assumes that `feedback` is either null when it has not be set, or an integer that can then be mapped to feedbackOptions.
+*/
+const getFeedback = (feedback = 0) => {
+ const feedbackOptions = [
Review comment:
The mapping seems incorrect. We are not going to pass in an integer value for feedback. It will be a string. It will be one of the following `"ANOMALY", "ANOMALY_EXPECTED", "ANOMALY_NEW_TREND", "NOT_ANOMALY", "NO_FEEDBACK"`. These in-turn will be mapped to entries in your array below - specifically they will be mapped as shown [here](https://github.com/apache/incubator-pinot/blob/master/thirdeye/thirdeye-frontend/app/pods/components/rootcause-anomaly/component.js#L30). The default case you can put in your function parameter would be `feedback="NO_FEEDBACK"` which should take effect if `null` is being passed in from backend. Because of this reason you'd need to refactor this function to instead use an object mapping instead of an array.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org