zhangloo333 commented on a change in pull request #6345:
URL: https://github.com/apache/incubator-pinot/pull/6345#discussion_r543776067
##########
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'
Review comment:
what's the type of start and end? Please add @type in the explanation.
Also, when you transform as an object, do you expect startTime, endTime, and
duration as the type or other?
##########
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:
Please at least give the schema of the details
##########
File path:
thirdeye/thirdeye-frontend/app/styles/pods/custom/parent-anomalies-table.scss
##########
@@ -0,0 +1,20 @@
+.parent-anomalies-table {
+ width: 100%;
+ border: 1px solid #ccc;
+ tr {
+ border-bottom: 1px solid #ccc;
+ }
+ td, th {
+ padding: 10px 10px 10px 20px;
+
+ p {
Review comment:
To prevent issues in the later, please use a class instead of p element
selector directly, otherwise, any p tag will have this style.
##########
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:
Please make sure the data is not an empty array, otherwise d.startTime
will throw an error to block continue.
##########
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] });
Review comment:
it's better to omit the undefined value.
##########
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:
also, if this data is different from the inject data, the name of the
variable of the data is too general. it cannot express variable purpose and the
type of the inside element. Please give an extra explanation to increase
readability. Like the expected type and purpose. If this data is the inject
data from the caller, you don't need to create another object to store the
anomalies objects. data will a property 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;
+};
+
+/*
+ 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];
Review comment:
I suggest you using key-value pairs for feedbackOptions. It will reduce
the misspell issue.
----------------------------------------------------------------
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
