From 76662b779432026a50f97d3e2d31fd83bbbb9be9 Mon Sep 17 00:00:00 2001 From: Armen Zambrano G Date: Fri, 14 Dec 2018 10:23:26 -0500 Subject: [PATCH] Refactor: Move data fetch into MainContainer & generic metrics Also make fetching metrics for a component generic in order to add a full summary of various metrics in a future detailed view for a specific Bugzilla component. --- package.json | 1 + src/components/BugzillaComponent/index.jsx | 27 ++ src/components/MainView/index.jsx | 48 ++++ src/components/Reportees/index.jsx | 25 ++ .../BugzillaComponentSummary/index.jsx | 59 ----- src/containers/MainContainer/index.jsx | 93 +++---- src/utils/bugzilla/getBugsCountAndLink.js | 26 ++ src/utils/bugzilla/getLinkToComponent.js | 8 - src/utils/bugzilla/getUntriagedBugsCount.js | 23 -- src/utils/bugzilla/metrics.js | 13 + test/components/MainView.test.jsx | 72 ++++++ .../__snapshots__/MainView.test.jsx.snap | 241 ++++++++++++++++++ test/simple_test.js | 5 - yarn.lock | 12 +- 14 files changed, 504 insertions(+), 149 deletions(-) create mode 100644 src/components/BugzillaComponent/index.jsx create mode 100644 src/components/MainView/index.jsx create mode 100644 src/components/Reportees/index.jsx delete mode 100644 src/containers/BugzillaComponentSummary/index.jsx create mode 100644 src/utils/bugzilla/getBugsCountAndLink.js delete mode 100644 src/utils/bugzilla/getLinkToComponent.js delete mode 100644 src/utils/bugzilla/getUntriagedBugsCount.js create mode 100644 src/utils/bugzilla/metrics.js create mode 100644 test/components/MainView.test.jsx create mode 100644 test/components/__snapshots__/MainView.test.jsx.snap delete mode 100644 test/simple_test.js diff --git a/package.json b/package.json index 0f8bdaf..bed2524 100644 --- a/package.json +++ b/package.json @@ -45,6 +45,7 @@ "lint-staged": "^8.1.0", "neutrino": "^9.0.0-beta.1", "node-fetch": "^2.3.0", + "react-test-renderer": "^16.6.3", "webpack": "^4", "webpack-cli": "^3", "webpack-dev-server": "^3" diff --git a/src/components/BugzillaComponent/index.jsx b/src/components/BugzillaComponent/index.jsx new file mode 100644 index 0000000..773faa8 --- /dev/null +++ b/src/components/BugzillaComponent/index.jsx @@ -0,0 +1,27 @@ +import React from 'react'; +import PropTypes from 'prop-types'; + +const linkToQuery = (key, metrics, alt) => ( + {metrics[key].count} +); + +const BugzillaComponent = ({ product, component, metrics }) => ( +
+ {`${product}::${component}`} + {metrics.untriaged + && linkToQuery('untriaged', metrics, 'Number of untriaged bugs') + } +
+); + +BugzillaComponent.propTypes = { + product: PropTypes.string.isRequired, + component: PropTypes.string.isRequired, + metrics: PropTypes.shape({}), +}; + +BugzillaComponent.defaultProps = { + metrics: {}, +}; + +export default BugzillaComponent; diff --git a/src/components/MainView/index.jsx b/src/components/MainView/index.jsx new file mode 100644 index 0000000..a9e7c5c --- /dev/null +++ b/src/components/MainView/index.jsx @@ -0,0 +1,48 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import BugzillaComponent from '../BugzillaComponent'; +import Reportees from '../Reportees'; + +const sortByComponentName = (a, b) => { + let result = (a.product <= b.product); + if (a.product === b.product) { + result = a.component <= b.component; + } + return result ? -1 : 1; +}; + +const MainView = ({ ldapEmail, partialOrg, bugzillaComponents }) => ( +
+

{partialOrg[ldapEmail].cn}

+
+ + {Object.values(bugzillaComponents).length > 0 && ( +
+

Components

+ {Object.values(bugzillaComponents) + .sort(sortByComponentName) + .map(({ component, product, metrics }) => ( + + ))} +
+ )} +
+
+); + +MainView.propTypes = { + ldapEmail: PropTypes.string.isRequired, + partialOrg: PropTypes.shape({}).isRequired, + bugzillaComponents: PropTypes.shape({}), +}; + +MainView.defaultProps = { + bugzillaComponents: {}, +}; + +export default MainView; diff --git a/src/components/Reportees/index.jsx b/src/components/Reportees/index.jsx new file mode 100644 index 0000000..ccc0b49 --- /dev/null +++ b/src/components/Reportees/index.jsx @@ -0,0 +1,25 @@ +import React from 'react'; +import PropTypes from 'prop-types'; + +const sortByPersonName = (a, b) => (a.cn <= b.cn ? -1 : 1); + +const Reportees = ({ ldapEmail, partialOrg }) => ( +
+

Reportees

+ {Object.values(partialOrg) + .filter(({ cn }) => cn !== ldapEmail) + .sort(sortByPersonName) + .map(({ cn, mail }) => ( +
+ {`${cn} `} +
+ ))} +
+); + +Reportees.propTypes = { + ldapEmail: PropTypes.string.isRequired, + partialOrg: PropTypes.shape({}).isRequired, +}; + +export default Reportees; diff --git a/src/containers/BugzillaComponentSummary/index.jsx b/src/containers/BugzillaComponentSummary/index.jsx deleted file mode 100644 index 6318cfb..0000000 --- a/src/containers/BugzillaComponentSummary/index.jsx +++ /dev/null @@ -1,59 +0,0 @@ -import React from 'react'; -import PropTypes from 'prop-types'; -import OpenInNew from '@material-ui/icons/OpenInNew'; -import { withStyles } from '@material-ui/core/styles'; -import getLinkToComponent from '../../utils/bugzilla/getLinkToComponent'; -import getUntriagedBugsCount from '../../utils/bugzilla/getUntriagedBugsCount'; - -const styles = theme => ({ - root: { - color: theme.palette.text.primary, - }, - icon: { - margin: 0, - fontSize: '1rem', - verticalAlign: 'text-top', - }, -}); - -class BugzillaComponentSummary extends React.Component { - state = { - untriaged: undefined, - }; - - static propTypes = { - classes: PropTypes.shape({}).isRequired, - product: PropTypes.string.isRequired, - component: PropTypes.string.isRequired, - }; - - async componentDidMount() { - this.fetchData(this.props); - } - - async fetchData({ product, component }) { - const untriaged = await getUntriagedBugsCount(product, component); - this.setState({ untriaged }); - } - - render() { - const { classes, product, component } = this.props; - const { untriaged } = this.state; - return ( -
- {`${product}::${component}`} - - - - {!!untriaged && {untriaged}} -
- ); - } -} - -export default withStyles(styles)(BugzillaComponentSummary); diff --git a/src/containers/MainContainer/index.jsx b/src/containers/MainContainer/index.jsx index 0850a87..d2f0e40 100644 --- a/src/containers/MainContainer/index.jsx +++ b/src/containers/MainContainer/index.jsx @@ -1,23 +1,14 @@ import React, { Component } from 'react'; import PropTypes from 'prop-types'; -import BugzillaComponentSummary from '../BugzillaComponentSummary'; +import MainView from '../../components/MainView'; import getAllReportees from '../../utils/getAllReportees'; import getBugzillaOwners from '../../utils/getBugzillaOwners'; - -const sortByPersonName = (a, b) => (a.cn <= b.cn ? -1 : 1); - -const sortByComponentName = (a, b) => { - let result = (a.product <= b.product); - if (a.product === b.product) { - result = a.component <= b.component; - } - return result ? -1 : 1; -}; +import getBugsCountAndLink from '../../utils/bugzilla/getBugsCountAndLink'; class MainContainer extends Component { state = { ldapEmail: '', - reporteesComponents: undefined, + bugzillaComponents: undefined, partialOrg: undefined, }; @@ -50,24 +41,45 @@ class MainContainer extends Component { return partialOrg; } - async reporteeComponents(bzOwners, partialOrg) { + async bugzillaComponents(bzOwners, partialOrg) { // bzOwners uses the bugzilla email address as the key // while partialOrg uses the LDAP email address - const reporteesComponents = Object.values(partialOrg) + /* eslint-disable no-param-reassign */ + const bugzillaComponents = Object.values(partialOrg) .reduce((result, { bugzillaEmail, mail }) => { const componentsOwned = bzOwners[bugzillaEmail] || bzOwners[mail]; if (componentsOwned) { componentsOwned.forEach(({ product, component }) => { - result.push({ + if (!result[`${product}::${component}`]) { + result[`${product}::${component}`] = {}; + } + result[`${product}::${component}`] = { bugzillaEmail: bugzillaEmail || mail, product, component, - }); + metrics: {}, + }; }); } return result; - }, []); - this.setState({ reporteesComponents }); + }, {}); + /* eslint-enable no-param-reassign */ + // This will list the components but will not show metrics + this.setState({ bugzillaComponents }); + + // Let's fetch the metrics for each component + Object.values(bugzillaComponents) + .map(async ({ product, component }) => { + const metric = 'untriaged'; + const { count, link } = await getBugsCountAndLink(product, component, metric); + bugzillaComponents[`${product}::${component}`].metrics = { + [metric]: { + count, + link, + }, + }; + this.setState({ bugzillaComponents }); + }); } async retrieveData(ldapEmail) { @@ -75,13 +87,13 @@ class MainContainer extends Component { getBugzillaOwners(), this.getReportees(ldapEmail), ]); - this.reporteeComponents(bzOwners, partialOrg); + this.bugzillaComponents(bzOwners, partialOrg); } handleChange(event) { this.setState({ ldapEmail: event.target.value, - reporteesComponents: undefined, + bugzillaComponents: undefined, partialOrg: undefined, }); } @@ -93,43 +105,18 @@ class MainContainer extends Component { } render() { - const { ldapEmail, reporteesComponents, partialOrg } = this.state; + const { + ldapEmail, bugzillaComponents, partialOrg, + } = this.state; return (
{partialOrg && ( -
-

{partialOrg[ldapEmail].cn}

-
- {partialOrg && ( -
-

Reportees

- {Object.values(partialOrg) - .filter(({ cn }) => cn !== ldapEmail) - .sort(sortByPersonName) - .map(({ cn, mail }) => ( -
- {`${cn} `} -
- ))} -
- )} - {reporteesComponents && reporteesComponents.length > 0 && ( -
-

Components

- {reporteesComponents - .sort(sortByComponentName) - .map(({ product, component }) => ( - - ))} -
- )} -
-
+ )}
); diff --git a/src/utils/bugzilla/getBugsCountAndLink.js b/src/utils/bugzilla/getBugsCountAndLink.js new file mode 100644 index 0000000..bd2efb3 --- /dev/null +++ b/src/utils/bugzilla/getBugsCountAndLink.js @@ -0,0 +1,26 @@ +import { stringify } from 'query-string'; +import fetchJson from '../fetchJson'; +import settings from './settings'; +import METRICS from './metrics'; + +const queryBugzilla = async queryParameters => ( + fetchJson(`${settings.BZ_HOST}/rest/bug?${queryParameters}`)); + +const getBugzillaComponentLink = queryParameters => ( + `${settings.BZ_HOST}/buglist.cgi?${stringify(queryParameters)}`); + +/* eslint-disable camelcase */ +const getBugsCountAndLink = async (product, component, metric) => { + const baseParams = { + product, + component, + ...METRICS[metric], + }; + const link = getBugzillaComponentLink(baseParams); + const { bug_count = 0 } = await queryBugzilla( + stringify({ ...baseParams, count_only: 1 }), + ); + return { count: bug_count, link }; +}; + +export default getBugsCountAndLink; diff --git a/src/utils/bugzilla/getLinkToComponent.js b/src/utils/bugzilla/getLinkToComponent.js deleted file mode 100644 index dc9e89c..0000000 --- a/src/utils/bugzilla/getLinkToComponent.js +++ /dev/null @@ -1,8 +0,0 @@ -import { stringify } from 'query-string'; -import settings from './settings'; -import untriagedParameters from './untriagedParameters'; - -const getBugzillaComponentLink = (product, component) => ( - `${settings.BZ_HOST}/buglist.cgi?${stringify({ product, component, ...untriagedParameters() })}`); - -export default getBugzillaComponentLink; diff --git a/src/utils/bugzilla/getUntriagedBugsCount.js b/src/utils/bugzilla/getUntriagedBugsCount.js deleted file mode 100644 index 219157c..0000000 --- a/src/utils/bugzilla/getUntriagedBugsCount.js +++ /dev/null @@ -1,23 +0,0 @@ -import { stringify } from 'query-string'; -import untriagedParameters from './untriagedParameters'; -import fetchJson from '../fetchJson'; -import settings from './settings'; - -const queryBugzilla = async queryParameters => ( - fetchJson(`${settings.BZ_HOST}/rest/bug?${queryParameters}`)); - -const getUntriagedBugsCount = async (product, component) => { - // eslint-disable-next-line camelcase - const { bug_count = 0 } = await queryBugzilla( - stringify({ - product, - component, - ...untriagedParameters(), - count_only: 1, // This only returns the bug count - }), - ); - // eslint-disable-next-line camelcase - return bug_count; -}; - -export default getUntriagedBugsCount; diff --git a/src/utils/bugzilla/metrics.js b/src/utils/bugzilla/metrics.js new file mode 100644 index 0000000..8e0c3c7 --- /dev/null +++ b/src/utils/bugzilla/metrics.js @@ -0,0 +1,13 @@ +/* eslint-disable indent */ +/* eslint-disable object-property-newline */ +/* eslint-disable no-multi-spaces */ +const METRICS = { + untriaged: { + f1: 'bug_severity', o1: 'notequals', v1: 'enhancement', + f2: 'keywords', o2: 'notsubstring', v2: 'meta', + f3: 'resolution', o3: 'isempty', + limit: 0, + }, +}; + +export default METRICS; diff --git a/test/components/MainView.test.jsx b/test/components/MainView.test.jsx new file mode 100644 index 0000000..c6e22d8 --- /dev/null +++ b/test/components/MainView.test.jsx @@ -0,0 +1,72 @@ +import React from 'react'; +import renderer from 'react-test-renderer'; +import MainView from '../../src/components/MainView'; + +const partialOrg = { + 'someone@mozilla.com': { + cn: 'Someone', + mail: 'someone@mozilla.com', + manager: { + dn: 'mail=manager@mozilla.com,o=com,dc=mozilla', + }, + }, + 'manager@mozilla.com': { + cn: 'Manager', + mail: 'manager@mozilla.com', + manager: null, + }, +}; +const bugzillaComponents = { + 'Core::DOM: IndexedDB': { + bugzillaEmail: 'someone@mozilla.com', + component: 'DOM: IndexedDB', + product: 'Core', + }, + 'Core::JavaScript Engine': { + bugzillaEmail: 'someone@mozilla.com', + component: 'JavaScript Engine', + product: 'Core', + }, + 'Core::DOM: Core & HTML': { + bugzillaEmail: 'someone@mozilla.com', + component: 'DOM: Core & HTML', + product: 'Core', + metrics: { + untriaged: { + count: 944, + link: 'https://bugzilla.mozilla.org/buglist.cgi?component=DOM%3A%20Core%20%26%20HTML&f1=bug_severity&f2=keywords&f3=resolution&limit=0&o1=notequals&o2=notsubstring&o3=isempty&product=Core&v1=enhancement&v2=meta', + }, + }, + }, + 'Toolkit::Async Tooling': { + bugzillaEmail: 'manager@mozilla.com', + component: 'Async Tooling', + product: 'Toolkit', + }, +}; + +it('renders Someone with no reportees', () => { + const tree = renderer + .create(( + + )) + .toJSON(); + expect(tree).toMatchSnapshot(); +}); + +it('renders Manager who has reportees', () => { + const tree = renderer + .create(( + + )) + .toJSON(); + expect(tree).toMatchSnapshot(); +}); diff --git a/test/components/__snapshots__/MainView.test.jsx.snap b/test/components/__snapshots__/MainView.test.jsx.snap new file mode 100644 index 0000000..30ef59f --- /dev/null +++ b/test/components/__snapshots__/MainView.test.jsx.snap @@ -0,0 +1,241 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`renders Manager who has reportees 1`] = ` +
+

+ Manager +

+
+
+

+ Reportees +

+
+ + Manager + +
+
+ + Someone + +
+
+
+

+ Components +

+
+ + Core::DOM: Core & HTML + + + 944 + +
+
+ + Core::DOM: IndexedDB + +
+
+ + Core::JavaScript Engine + +
+
+ + Toolkit::Async Tooling + +
+
+
+
+`; + +exports[`renders Someone with no reportees 1`] = ` +
+

+ Someone +

+
+
+

+ Reportees +

+
+ + Manager + +
+
+ + Someone + +
+
+
+

+ Components +

+
+ + Core::DOM: Core & HTML + + + 944 + +
+
+ + Core::DOM: IndexedDB + +
+
+ + Core::JavaScript Engine + +
+
+ + Toolkit::Async Tooling + +
+
+
+
+`; diff --git a/test/simple_test.js b/test/simple_test.js deleted file mode 100644 index b184d36..0000000 --- a/test/simple_test.js +++ /dev/null @@ -1,5 +0,0 @@ -describe('simple', () => { - it('should be sane', () => { - expect(false).not.toBe(true); - }); -}); diff --git a/yarn.lock b/yarn.lock index b46c72d..98e2ba8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7055,7 +7055,7 @@ react-hot-loader@^4: react-lifecycles-compat "^3.0.4" shallowequal "^1.0.2" -react-is@^16.3.2: +react-is@^16.3.2, react-is@^16.6.3: version "16.6.3" resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.6.3.tgz#d2d7462fcfcbe6ec0da56ad69047e47e56e7eac0" integrity sha512-u7FDWtthB4rWibG/+mFbVd5FvdI20yde86qKGx4lVUTWmPlSWQ4QxbBIrrs+HnXGbxOUlUzTAP/VDmvCwaP2yA== @@ -7090,6 +7090,16 @@ react-router@^4.3.1: prop-types "^15.6.1" warning "^4.0.1" +react-test-renderer@^16.6.3: + version "16.6.3" + resolved "https://registry.yarnpkg.com/react-test-renderer/-/react-test-renderer-16.6.3.tgz#5f3a1a7d5c3379d46f7052b848b4b72e47c89f38" + integrity sha512-B5bCer+qymrQz/wN03lT0LppbZUDRq6AMfzMKrovzkGzfO81a9T+PWQW6MzkWknbwODQH/qpJno/yFQLX5IWrQ== + dependencies: + object-assign "^4.1.1" + prop-types "^15.6.2" + react-is "^16.6.3" + scheduler "^0.11.2" + react-transition-group@^2.2.1: version "2.5.0" resolved "https://registry.yarnpkg.com/react-transition-group/-/react-transition-group-2.5.0.tgz#70bca0e3546102c4dc5cf3f5f57f73447cce6874"