refactor: Tested and refactored Stats component (#687)

Signed-off-by: Marcos Iglesias Valle <golodhros@gmail.com>

amundsen_application/static/js/components/ColumnList/ColumnStats/columnStats.story.tsx

+// Copyright Contributors to the Amundsen project.
+// SPDX-License-Identifier: Apache-2.0
+
+import React from 'react';
+import { storiesOf } from '@storybook/react';
+
+import StorySection from 'components/common/StorySection';
+import ColumnStats from '.';
+
+import TestDataBuilder from './testDataBuilder';
+
+const dataBuilder = new TestDataBuilder();
+
+const { stats: fourStats } = dataBuilder.withFourStats().build();
+const { stats: threeStats } = dataBuilder.withThreeStats().build();
+const { stats: eightStats } = dataBuilder.withEightStats().build();
+
+const stories = storiesOf('Components/ColumnStats', module);
+
+stories.add('Column Stats', () => (
+  <>
+    <StorySection title="with four stats">
+      <ColumnStats stats={fourStats} />
+    </StorySection>
+    <StorySection title="with three stats">
+      <ColumnStats stats={threeStats} />
+    </StorySection>
+    <StorySection title="with eight stats">
+      <ColumnStats stats={eightStats} />
+    </StorySection>
+  </>
+));

amundsen_application/static/js/components/ColumnList/ColumnStats/index.spec.tsx

@@ -2,126 +2,116 @@
 // SPDX-License-Identifier: Apache-2.0
 
 import * as React from 'react';
-import { shallow } from 'enzyme';
-import { ColumnStats, ColumnStatsProps } from '.';
+import { mount } from 'enzyme';
 
-describe('ColumnStats', () => {
-  const setup = (propOverrides?: Partial<ColumnStatsProps>) => {
-    const props = {
-      stats: [
-        {
-          end_epoch: 1571616000,
-          start_epoch: 1571616000,
-          stat_type: 'count',
-          stat_val: '12345',
-        },
-        {
-          end_epoch: 1571616000,
-          start_epoch: 1571616000,
-          stat_type: 'count_null',
-          stat_val: '123',
-        },
-        {
-          end_epoch: 1571616000,
-          start_epoch: 1571616000,
-          stat_type: 'count_distinct',
-          stat_val: '22',
-        },
-        {
-          end_epoch: 1571616000,
-          start_epoch: 1571616000,
-          stat_type: 'count_zero',
-          stat_val: '44',
-        },
-        {
-          end_epoch: 1571616000,
-          start_epoch: 1571616000,
-          stat_type: 'max',
-          stat_val: '1237466454',
-        },
-        {
-          end_epoch: 1571616000,
-          start_epoch: 1571616000,
-          stat_type: 'min',
-          stat_val: '856',
-        },
-        {
-          end_epoch: 1571616000,
-          start_epoch: 1571616000,
-          stat_type: 'avg',
-          stat_val: '2356575',
-        },
-        {
-          end_epoch: 1571616000,
-          start_epoch: 1571616000,
-          stat_type: 'stddev',
-          stat_val: '1234563',
-        },
-      ],
-      ...propOverrides,
-    };
-    const wrapper = shallow<ColumnStats>(<ColumnStats {...props} />);
-    return { props, wrapper };
+import ColumnStats, { ColumnStatsProps } from '.';
+import TestDataBuilder from './testDataBuilder';
+
+const dataBuilder = new TestDataBuilder();
+
+const setup = (propOverrides?: Partial<ColumnStatsProps>) => {
+  const props = {
+    stats: [],
+    ...propOverrides,
   };
+  const wrapper = mount<typeof ColumnStats>(<ColumnStats {...props} />);
 
-  const { wrapper, props } = setup();
-  const instance = wrapper.instance();
-
-  describe('getStatsInfoText', () => {
-    it('generates correct info text for a daily partition', () => {
-      const startEpoch = 1568160000;
-      const endEpoch = 1568160000;
-      const expectedInfoText = `Stats reflect data collected on Sep 11, 2019 only. (daily partition)`;
-      expect(instance.getStatsInfoText(startEpoch, endEpoch)).toBe(
-        expectedInfoText
-      );
-    });
+  return { props, wrapper };
+};
 
-    it('generates correct info text for a date range', () => {
-      const startEpoch = 1568160000;
-      const endEpoch = 1571616000;
-      const expectedInfoText = `Stats reflect data collected between Sep 11, 2019 and Oct 21, 2019.`;
-      expect(instance.getStatsInfoText(startEpoch, endEpoch)).toBe(
-        expectedInfoText
-      );
-    });
+describe('ColumnStats', () => {
+  describe('render', () => {
+    describe('when stats are empty', () => {
+      const { stats } = dataBuilder.withEmptyStats().build();
 
-    it('generates correct when no dates are given', () => {
-      const expectedInfoText = `Stats reflect data collected over a recent period of time.`;
+      it('does not render the component', () => {
+        const { wrapper } = setup({ stats });
+        const expected = stats.length;
+        const actual = wrapper.find('.column-stats').length;
 
-      expect(instance.getStatsInfoText()).toBe(expectedInfoText);
+        expect(actual).toEqual(expected);
+      });
     });
-  });
 
-  describe('renderColumnStat', () => {
-    it('renders a single column stat', () => {
-      const columnStat = {
-        end_epoch: 1571616000,
-        start_epoch: 1571616000,
-        stat_type: 'count',
-        stat_val: '12345',
-      };
-      const expectedStatType = columnStat.stat_type.toUpperCase();
-      const expectedStatValue = columnStat.stat_val;
-      const result = shallow(instance.renderColumnStat(columnStat));
-      expect(result.find('.stat-name').text()).toBe(expectedStatType);
-      expect(result.find('.stat-value').text()).toBe(expectedStatValue);
+    describe('when four stats are passed', () => {
+      const { stats } = dataBuilder.withFourStats().build();
+
+      it('renders the component', () => {
+        const { wrapper } = setup({ stats });
+        const expected = 1;
+        const actual = wrapper.find('.column-stats').length;
+
+        expect(actual).toEqual(expected);
+      });
+
+      it('renders two columns', () => {
+        const { wrapper } = setup({ stats });
+        const expected = 2;
+        const actual = wrapper.find('.column-stats-column').length;
+
+        expect(actual).toEqual(expected);
+      });
+
+      it('renders the stats info text', () => {
+        const { wrapper } = setup({ stats });
+        const expected = 1;
+        const actual = wrapper.find('.stat-collection-info').length;
+
+        expect(actual).toEqual(expected);
+      });
+
+      it('renders four stat rows', () => {
+        const { wrapper } = setup({ stats });
+        const expected = stats.length;
+        const actual = wrapper.find('.column-stat-row').length;
+
+        expect(actual).toEqual(expected);
+      });
     });
-  });
 
-  describe('render', () => {
-    it('calls the appropriate functions', () => {
-      const getStatsInfoTextSpy = jest.spyOn(instance, 'getStatsInfoText');
-      instance.render();
+    describe('when three stats are passed', () => {
+      const { stats } = dataBuilder.withThreeStats().build();
+
+      it('renders three stat rows', () => {
+        const { wrapper } = setup({ stats });
+        const expected = stats.length;
+        const actual = wrapper.find('.column-stat-row').length;
+
+        expect(actual).toEqual(expected);
+      });
+
+      it('renders two rows in the first column', () => {
+        const { wrapper } = setup({ stats });
+        const expected = 2;
+        const actual = wrapper
+          .find('.column-stats-column')
+          .first()
+          .find('.column-stat-row').length;
+
+        expect(actual).toEqual(expected);
+      });
 
-      expect(getStatsInfoTextSpy).toHaveBeenCalledWith(1571616000, 1571616000);
+      it('renders one row in the second column', () => {
+        const { wrapper } = setup({ stats });
+        const expected = 1;
+        const actual = wrapper
+          .find('.column-stats-column')
+          .last()
+          .find('.column-stat-row').length;
+
+        expect(actual).toEqual(expected);
+      });
     });
 
-    it('calls renderColumnStat with all of the stats', () => {
-      const renderColumnStatSpy = jest.spyOn(instance, 'renderColumnStat');
-      instance.render();
-      props.stats.forEach((stat) => {
-        expect(renderColumnStatSpy).toHaveBeenCalledWith(stat);
+    describe('when eight stats are passed', () => {
+      const { stats } = dataBuilder.withEightStats().build();
+
+      it('renders eight stat rows', () => {
+        const { wrapper } = setup({ stats });
+        const expected = stats.length;
+        const actual = wrapper.find('.column-stat-row').length;
+
+        expect(actual).toEqual(expected);
       });
     });
   });

amundsen_application/static/js/components/ColumnList/ColumnStats/index.tsx

@@ -4,7 +4,9 @@
 import * as React from 'react';
 
 import { TableColumnStats } from 'interfaces/index';
-import { formatDate } from 'utils/dateUtils';
+import { getStatsInfoText } from '../utils';
+
+import { COLUMN_STATS_TITLE } from '../constants';
 
 import './styles.scss';
 
@@ -12,70 +14,75 @@ export interface ColumnStatsProps {
   stats: TableColumnStats[];
 }
 
-export class ColumnStats extends React.Component<ColumnStatsProps> {
-  getStatsInfoText = (startEpoch?: number, endEpoch?: number) => {
-    const startDate = startEpoch
-      ? formatDate({ epochTimestamp: startEpoch })
-      : null;
-    const endDate = endEpoch ? formatDate({ epochTimestamp: endEpoch }) : null;
+type ColumnStatRowProps = {
+  stat_type: string;
+  stat_val: string;
+};
 
-    let infoText = 'Stats reflect data collected';
-    if (startDate && endDate) {
-      if (startDate === endDate) {
-        infoText = `${infoText} on ${startDate} only. (daily partition)`;
-      } else {
-        infoText = `${infoText} between ${startDate} and ${endDate}.`;
-      }
-    } else {
-      infoText = `${infoText} over a recent period of time.`;
-    }
-    return infoText;
-  };
+const ColumnStatRow: React.FC<ColumnStatRowProps> = ({
+  stat_type,
+  stat_val,
+}: ColumnStatRowProps) => {
+  return (
+    <div className="column-stat-row">
+      <div className="stat-name body-3">{stat_type.toUpperCase()}</div>
+      <div className="stat-value">{stat_val}</div>
+    </div>
+  );
+};
 
-  renderColumnStat = (entry: TableColumnStats) => {
-    return (
-      <div className="column-stat-row" key={entry.stat_type}>
-        <div className="stat-name body-3">{entry.stat_type.toUpperCase()}</div>
-        <div className="stat-value">{entry.stat_val}</div>
-      </div>
-    );
-  };
+const getStart = ({ start_epoch }) => start_epoch;
+const getEnd = ({ end_epoch }) => end_epoch;
 
-  render = () => {
-    const { stats } = this.props;
-    if (stats.length === 0) {
-      return null;
-    }
+const ColumnStats: React.FC<ColumnStatsProps> = ({
+  stats,
+}: ColumnStatsProps) => {
+  if (stats.length === 0) {
+    return null;
+  }
+  const startEpoch = Math.min(...stats.map(getStart));
+  const endEpoch = Math.max(...stats.map(getEnd));
 
-    // TODO - Move map statements to separate functions for better testing
-    const startEpoch = Math.min(...stats.map((s) => s.start_epoch));
-    const endEpoch = Math.max(...stats.map((s) => s.end_epoch));
+  return (
+    <article className="column-stats">
+      <div className="stat-collection-info">
+        <span className="stat-title">{COLUMN_STATS_TITLE} </span>
+        {getStatsInfoText(startEpoch, endEpoch)}
+      </div>
+      <div className="column-stats-table">
+        <div className="column-stats-column">
+          {stats.map((stat, index) => {
+            if (index % 2 === 0) {
+              return (
+                <ColumnStatRow
+                  key={stat.stat_type}
+                  stat_type={stat.stat_type}
+                  stat_val={stat.stat_val}
+                />
+              );
+            }
 
-    return (
-      <section className="column-stats">
-        <div className="stat-collection-info">
-          <span className="title-3">Column Statistics&nbsp;</span>
-          {this.getStatsInfoText(startEpoch, endEpoch)}
+            return null;
+          })}
         </div>
-        <div className="column-stats-table">
-          <div className="column-stats-column">
-            {stats.map((stat, index) => {
-              if (index % 2 === 0) {
-                return this.renderColumnStat(stat);
-              }
-            })}
-          </div>
-          <div className="column-stats-column">
-            {this.props.stats.map((stat, index) => {
-              if (index % 2 === 1) {
-                return this.renderColumnStat(stat);
-              }
-            })}
-          </div>
+        <div className="column-stats-column">
+          {stats.map((stat, index) => {
+            if (index % 2 === 1) {
+              return (
+                <ColumnStatRow
+                  key={stat.stat_type}
+                  stat_type={stat.stat_type}
+                  stat_val={stat.stat_val}
+                />
+              );
+            }
+
+            return null;
+          })}
         </div>
-      </section>
-    );
-  };
-}
+      </div>
+    </article>
+  );
+};
 
 export default ColumnStats;

amundsen_application/static/js/components/ColumnList/ColumnStats/styles.scss

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 @import 'variables';
+@import 'typography';
 
 .column-stats {
   .stat-collection-info {
@@ -48,4 +49,8 @@
       }
     }
   }
+
+  .stat-title {
+    @extend %text-title-w3;
+  }
 }

amundsen_application/static/js/components/ColumnList/ColumnStats/testDataBuilder.ts

+// Copyright Contributors to the Amundsen project.
+// SPDX-License-Identifier: Apache-2.0
+
+const defaultConfig = {
+  stats: [],
+};
+
+/**
+ * Generates test data for the table data
+ * @example
+ * let testData = new TestDataBuilder()
+ *                         .withEightStats()
+ *                         .build();
+ */
+function TestDataBuilder(config = {}) {
+  this.Klass = TestDataBuilder;
+  this.config = {
+    ...defaultConfig,
+    ...config,
+  };
+
+  this.withEightStats = () => {
+    const attr = {
+      stats: [
+        {
+          end_epoch: 1571616000,
+          start_epoch: 1571616000,
+          stat_type: 'count',
+          stat_val: '12345',
+        },
+        {
+          end_epoch: 1571616000,
+          start_epoch: 1571616000,
+          stat_type: 'count_null',
+          stat_val: '123',
+        },
+        {
+          end_epoch: 1571616000,
+          start_epoch: 1571616000,
+          stat_type: 'count_distinct',
+          stat_val: '22',
+        },
+        {
+          end_epoch: 1571616000,
+          start_epoch: 1571616000,
+          stat_type: 'count_zero',
+          stat_val: '44',
+        },
+        {
+          end_epoch: 1571616000,
+          start_epoch: 1571616000,
+          stat_type: 'max',
+          stat_val: '1237466454',
+        },
+        {
+          end_epoch: 1571616000,
+          start_epoch: 1571616000,
+          stat_type: 'min',
+          stat_val: '856',
+        },
+        {
+          end_epoch: 1571616000,
+          start_epoch: 1571616000,
+          stat_type: 'avg',
+          stat_val: '2356575',
+        },
+        {
+          end_epoch: 1571616000,
+          start_epoch: 1571616000,
+          stat_type: 'stddev',
+          stat_val: '1234563',
+        },
+      ],
+    };
+
+    return new this.Klass(attr);
+  };
+
+  this.withFourStats = () => {
+    const attr = {
+      stats: [
+        {
+          end_epoch: 1571616000,
+          start_epoch: 1571616000,
+          stat_type: 'count',
+          stat_val: '12345',
+        },
+        {
+          end_epoch: 1571616000,
+          start_epoch: 1571616000,
+          stat_type: 'count_null',
+          stat_val: '123',
+        },
+        {
+          end_epoch: 1571616000,
+          start_epoch: 1571616000,
+          stat_type: 'avg',
+          stat_val: '2356575',
+        },
+        {
+          end_epoch: 1571616000,
+          start_epoch: 1571616000,
+          stat_type: 'stddev',
+          stat_val: '1234563',
+        },
+      ],
+    };
+
+    return new this.Klass(attr);
+  };
+
+  this.withThreeStats = () => {
+    const attr = {
+      stats: [
+        {
+          end_epoch: 1571616000,
+          start_epoch: 1571616000,
+          stat_type: 'count',
+          stat_val: '12345',
+        },
+        {
+          end_epoch: 1571616000,
+          start_epoch: 1571616000,
+          stat_type: 'avg',
+          stat_val: '2356575',
+        },
+        {
+          end_epoch: 1571616000,
+          start_epoch: 1571616000,
+          stat_type: 'stddev',
+          stat_val: '1234563',
+        },
+      ],
+    };
+
+    return new this.Klass(attr);
+  };
+
+  this.withEmptyStats = () => {
+    const attr = { stats: [] };
+
+    return new this.Klass(attr);
+  };
+
+  this.build = () => this.config;
+}
+
+export default TestDataBuilder;

amundsen_application/static/js/components/ColumnList/index.tsx

@@ -12,12 +12,12 @@ import Table, {
 } from 'components/common/Table';
 
 import { logAction } from 'ducks/utilMethods';
-import { formatDate } from 'utils/dateUtils';
 import { notificationsEnabled, getMaxLength } from 'config/config-utils';
 import { TableColumn, RequestMetadataType } from 'interfaces';
 
 import ColumnType from './ColumnType';
 import ColumnDescEditableText from './ColumnDescEditableText';
+import { getStatsInfoText } from './utils';
 
 import {
   MORE_BUTTON_TEXT,
@@ -86,28 +86,6 @@ const handleRowExpand = (rowValues) => {
   });
 };
 
-// TODO: Move into utils
-const getStatsInfoText = (startEpoch: number, endEpoch: number) => {
-  const startDate = startEpoch
-    ? formatDate({ epochTimestamp: startEpoch })
-    : null;
-  const endDate = endEpoch ? formatDate({ epochTimestamp: endEpoch }) : null;
-
-  let infoText = 'Stats reflect data collected';
-
-  if (startDate && endDate) {
-    if (startDate === endDate) {
-      infoText = `${infoText} on ${startDate} only. (daily partition)`;
-    } else {
-      infoText = `${infoText} between ${startDate} and ${endDate}.`;
-    }
-  } else {
-    infoText = `${infoText} over a recent period of time.`;
-  }
-
-  return infoText;
-};
-
 // @ts-ignore
 const ExpandedRowComponent: React.FC<ExpandedRowProps> = (
   rowValue: FormattedDataType

amundsen_application/static/js/components/ColumnList/utils.spec.ts

+// Copyright Contributors to the Amundsen project.
+// SPDX-License-Identifier: Apache-2.0
+
+import { getStatsInfoText } from './utils';
+
+describe('ColumnList utils', () => {
+  describe('getStatsInfoText', () => {
+    it('generates correct info text for a daily partition', () => {
+      const startEpoch = 1568160000;
+      const endEpoch = 1568160000;
+      const expected = `Stats reflect data collected on Sep 11, 2019 only. (daily partition)`;
+      const actual = getStatsInfoText(startEpoch, endEpoch);
+
+      expect(actual).toEqual(expected);
+    });
+
+    it('generates correct info text for a date range', () => {
+      const startEpoch = 1568160000;
+      const endEpoch = 1571616000;
+      const expected = `Stats reflect data collected between Sep 11, 2019 and Oct 21, 2019.`;
+      const actual = getStatsInfoText(startEpoch, endEpoch);
+
+      expect(actual).toEqual(expected);
+    });
+
+    it('generates correct when no dates are given', () => {
+      const expected = `Stats reflect data collected over a recent period of time.`;
+      const actual = getStatsInfoText();
+
+      expect(actual).toEqual(expected);
+    });
+  });
+});

amundsen_application/static/js/components/ColumnList/utils.ts

+// Copyright Contributors to the Amundsen project.
+// SPDX-License-Identifier: Apache-2.0
+
+import { formatDate } from 'utils/dateUtils';
+
+export const getStatsInfoText = (startEpoch?: number, endEpoch?: number) => {
+  const startDate = startEpoch
+    ? formatDate({ epochTimestamp: startEpoch })
+    : null;
+  const endDate = endEpoch ? formatDate({ epochTimestamp: endEpoch }) : null;
+
+  let infoText = 'Stats reflect data collected';
+
+  if (startDate && endDate) {
+    if (startDate === endDate) {
+      infoText = `${infoText} on ${startDate} only. (daily partition)`;
+    } else {
+      infoText = `${infoText} between ${startDate} and ${endDate}.`;
+    }
+  } else {
+    infoText = `${infoText} over a recent period of time.`;
+  }
+
+  return infoText;
+};