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:
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