Bug 1571404 - Set assignees for alert summaries

2019-10-02 14:50:12 +03:00 · 2019-10-02 14:50:12 +03:00 · 53c639c166
--- a/tests/ui/perfherder/alerts_test.jsx
+++ b/tests/ui/perfherder/alerts_test.jsx
@ -3,9 +3,9 @@ import {
  render,
  cleanup,
  fireEvent,
+  wait,
  waitForElement,
  waitForElementToBeRemoved,
-  wait,
 } from '@testing-library/react';

 import AlertsViewControls from '../../../ui/perfherder/alerts/AlertsViewControls';
@ -14,9 +14,9 @@ import { summaryStatusMap } from '../../../ui/perfherder/constants';
 import repos from '../mock/repositories';

 const testUser = {
-  username: 'test user',
+  username: 'mozilla-ldap/test_user@mozilla.com',
  is_superuser: false,
-  is_staff: true,
+  isStaff: true,
  email: 'test_user@mozilla.com',
 };

@ -95,6 +95,8 @@ const testAlertSummaries = [
    revision: '930f0f51b681aea2a5e915a2770f80a9914ed3df',
    push_timestamp: 1558111832,
    prev_push_revision: '76e3a842e496d78a80cd547b7bf94f041f9bc612',
+    assignee_username: null,
+    assignee_email: null,
  },
  {
    id: 20239,
@ -170,6 +172,8 @@ const testAlertSummaries = [
    revision: 'd4a9b4dd03ca5c3db2bd10e8097d9817435ba37d',
    push_timestamp: 1558583128,
    prev_push_revision: 'c8e9b6a81194dff2d37b4f67d23a419fd4587e49',
+    assignee_username: 'mozilla-ldap/test_user@mozilla.com',
+    assignee_email: 'test_user@mozilla.com',
  },
 ];

@ -213,6 +217,11 @@ const mockModifyAlert = {
  },
 };

+// eslint-disable-next-line no-unused-vars
+const mockUpdateAlertSummary = (alertSummaryId, params) => ({
+  failureStatus: null,
+});
+
 const alertsViewControls = () =>
  render(
    <AlertsViewControls
@ -230,6 +239,9 @@ const alertsViewControls = () =>
      updateViewState={() => {}}
      user={testUser}
      modifyAlert={(alert, params) => mockModifyAlert.update(alert, params)}
+      updateAlertSummary={() =>
+        Promise.resolve({ failureStatus: false, data: 'alert summary data' })
+      }
      projects={repos}
      location={{
        pathname: '/alerts',
@ -429,5 +441,107 @@ test('selecting the alert summary checkbox then deselecting one alert only updat
  modifyAlertSpy.mockClear();
 });

+test("display of alert summaries's assignee badge", async () => {
+  const { getAllByTitle, getAllByText } = alertsViewControls();
+
+  const ownershipBadges = getAllByTitle('Click to change assignee');
+  const takeButtons = getAllByText('Take');
+
+  // summary with no assignee defaults to "Unassigned" badge &
+  // displays the 'Take' button
+  expect(ownershipBadges[0]).toHaveTextContent('Unassigned');
+  expect(takeButtons).toHaveLength(1);
+
+  // summary with assignee displays username extracted from email &
+  // hides the 'Take' button
+  expect(ownershipBadges[1]).toHaveTextContent('test_user');
+});
+
+test("'Take' button hides when clicking on 'Unassigned' badge", async () => {
+  const {
+    getByText,
+    queryByText,
+    queryByPlaceholderText,
+  } = alertsViewControls();
+
+  const unassignedBadge = await waitForElement(() => getByText('Unassigned'));
+
+  await fireEvent.click(unassignedBadge);
+  expect(queryByText('Take')).not.toBeInTheDocument();
+  // and the placeholder nicely shows up
+  expect(queryByPlaceholderText('nobody@mozilla.org')).toBeInTheDocument();
+});
+
+test('setting an assignee on unassigned alert summary updates the badge accordingly', async () => {
+  const { getByText, getByPlaceholderText } = alertsViewControls();
+
+  const unassignedBadge = await waitForElement(() => getByText('Unassigned'));
+
+  fireEvent.click(unassignedBadge);
+  const inputField = await waitForElement(() =>
+    getByPlaceholderText('nobody@mozilla.org'),
+  );
+  fireEvent.change(inputField, {
+    target: { value: 'mozilla-ldap/test_assignee@mozilla.com' },
+  });
+  // pressing 'Enter' has some issues on react-testing-library;
+  // found workaround on https://github.com/testing-library/react-testing-library/issues/269
+  fireEvent.keyPress(inputField, { key: 'Enter', keyCode: 13 });
+
+  // ensure this updated the assignee
+  await waitForElement(() => getByText('test_assignee'));
+});
+
+test('setting an assignee on an already assigned summary is possible', async () => {
+  const { getByText, getByDisplayValue } = alertsViewControls();
+
+  const unassignedBadge = await waitForElement(() => getByText('test_user'));
+
+  fireEvent.click(unassignedBadge);
+  const inputField = await waitForElement(() =>
+    getByDisplayValue('mozilla-ldap/test_user@mozilla.com'),
+  );
+  fireEvent.change(inputField, {
+    target: { value: 'mozilla-ldap/test_another_user@mozilla.com' },
+  });
+  // pressing 'Enter' has some issues on react-testing-library;
+  // found workaround on https://github.com/testing-library/react-testing-library/issues/269
+  fireEvent.keyPress(inputField, { key: 'Enter', keyCode: 13 });
+
+  // ensure this updated the assignee
+  await waitForElement(() => getByText('test_another_user'));
+});
+
+test("'Escape' from partially editted assignee does not update original assignee", async () => {
+  const { getByText, getByDisplayValue } = alertsViewControls();
+
+  const unassignedBadge = await waitForElement(() => getByText('test_user'));
+
+  fireEvent.click(unassignedBadge);
+  const inputField = await waitForElement(() =>
+    getByDisplayValue('mozilla-ldap/test_user@mozilla.com'),
+  );
+  fireEvent.change(inputField, {
+    target: { value: 'mozilla-ldap/test_another_' },
+  });
+  fireEvent.keyDown(inputField, { key: 'Escape' });
+
+  // ensure assignee wasn't updated
+  await waitForElement(() => getByText('test_user'));
+});
+
+test("Clicking on 'Take' prefills with logged in user", async () => {
+  const { getByText, getByDisplayValue } = alertsViewControls();
+
+  const takeButton = getByText('Take');
+
+  fireEvent.click(takeButton);
+
+  // ensure it preffiled input field
+  await waitForElement(() =>
+    getByDisplayValue('mozilla-ldap/test_user@mozilla.com'),
+  );
+});
+
 // TODO should write tests for alert summary dropdown menu actions performed in StatusDropdown
 // (adding notes or marking as 'fixed', etc)
--- a/ui/perfherder/alerts/AlertHeader.jsx
+++ b/ui/perfherder/alerts/AlertHeader.jsx
@ -5,6 +5,9 @@ import {
  DropdownMenu,
  DropdownItem,
  DropdownToggle,
+  Container,
+  Row,
+  Col,
 } from 'reactstrap';
 import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
 import { faExternalLinkAlt } from '@fortawesome/free-solid-svg-icons';
@ -13,7 +16,15 @@ import moment from 'moment';
 import { getTitle } from '../helpers';
 import { getJobsUrl } from '../../helpers/url';

-const AlertHeader = ({ alertSummary, repoModel, issueTrackers }) => {
+import Assignee from './Assignee';
+
+const AlertHeader = ({
+  alertSummary,
+  repoModel,
+  issueTrackers,
+  user,
+  updateAssignee,
+}) => {
  const getIssueTrackerUrl = () => {
    const { issueTrackerUrl } = issueTrackers.find(
      tracker => tracker.id === alertSummary.issue_tracker,
@ -25,62 +36,65 @@ const AlertHeader = ({ alertSummary, repoModel, issueTrackers }) => {
    : '';

  return (
-    <div className="pl-2">
-      <a
-        className="text-dark font-weight-bold align-middle"
-        href={`#/alerts?id=${alertSummary.id}`}
-        id={`alert summary ${alertSummary.id.toString()} title`}
-        data-testid={`alert summary ${alertSummary.id.toString()} title`}
-      >
-        Alert #{alertSummary.id} - {alertSummary.repository} -{' '}
-        {getTitle(alertSummary)}{' '}
-        <FontAwesomeIcon
-          icon={faExternalLinkAlt}
-          className="icon-superscript"
-        />
-      </a>
-      <br />
-      <span className="font-weight-normal">
-        <span className="align-middle">{`${moment(
+    <Container>
+      <Row>
+        <a
+          className="text-dark font-weight-bold align-middle"
+          href={`#/alerts?id=${alertSummary.id}`}
+          id={`alert summary ${alertSummary.id.toString()} title`}
+          data-testid={`alert summary ${alertSummary.id.toString()} title`}
+        >
+          Alert #{alertSummary.id} - {alertSummary.repository} -{' '}
+          {getTitle(alertSummary)}{' '}
+          <FontAwesomeIcon
+            icon={faExternalLinkAlt}
+            className="icon-superscript"
+          />
+        </a>
+      </Row>
+      <Row className="font-weight-normal">
+        <Col className="p-0" xs="auto">{`${moment(
          alertSummary.push_timestamp * 1000,
-        ).format('ddd MMM D, HH:mm:ss')} · `}</span>
-        <UncontrolledDropdown tag="span">
-          <DropdownToggle
-            className="btn-link text-info p-0"
-            color="transparent"
-            caret
-          >
-            {alertSummary.revision.slice(0, 12)}
-          </DropdownToggle>
-          <DropdownMenu>
-            <a
-              className="text-dark"
-              href={getJobsUrl({
-                repo: alertSummary.repository,
-                fromchange: alertSummary.prev_push_revision,
-                tochange: alertSummary.revision,
-              })}
-              target="_blank"
-              rel="noopener noreferrer"
+        ).format('ddd MMM D, HH:mm:ss')} ·`}</Col>
+        <Col className="p-0" xs="auto">
+          <UncontrolledDropdown tag="span">
+            <DropdownToggle
+              className="btn-link text-info p-0"
+              color="transparent"
+              caret
            >
-              <DropdownItem>Jobs</DropdownItem>
-            </a>
-            <a
-              className="text-dark"
-              href={repoModel.getPushLogRangeHref({
-                fromchange: alertSummary.prev_push_revision,
-                tochange: alertSummary.revision,
-              })}
-              target="_blank"
-              rel="noopener noreferrer"
-            >
-              <DropdownItem>Pushlog</DropdownItem>
-            </a>
-          </DropdownMenu>
-        </UncontrolledDropdown>
+              {alertSummary.revision.slice(0, 12)}
+            </DropdownToggle>
+            <DropdownMenu>
+              <a
+                className="text-dark"
+                href={getJobsUrl({
+                  repo: alertSummary.repository,
+                  fromchange: alertSummary.prev_push_revision,
+                  tochange: alertSummary.revision,
+                })}
+                target="_blank"
+                rel="noopener noreferrer"
+              >
+                <DropdownItem>Jobs</DropdownItem>
+              </a>
+              <a
+                className="text-dark"
+                href={repoModel.getPushLogRangeHref({
+                  fromchange: alertSummary.prev_push_revision,
+                  tochange: alertSummary.revision,
+                })}
+                target="_blank"
+                rel="noopener noreferrer"
+              >
+                <DropdownItem>Pushlog</DropdownItem>
+              </a>
+            </DropdownMenu>
+          </UncontrolledDropdown>
+          <span>·</span>
+        </Col>
        {bugNumber && (
-          <span>
-            <span className="align-middle"> · </span>
+          <Col className="p-0" xs="auto">
            {alertSummary.issue_tracker && issueTrackers.length > 0 ? (
              <a
                className="text-info align-middle"
@ -93,16 +107,25 @@ const AlertHeader = ({ alertSummary, repoModel, issueTrackers }) => {
            ) : (
              { bugNumber }
            )}
-          </span>
+            <span>·</span>
+          </Col>
        )}
-      </span>
-    </div>
+        <Col className="p-0" xs="auto">
+          <Assignee
+            assigneeUsername={alertSummary.assignee_username}
+            updateAssignee={updateAssignee}
+            user={user}
+          />
+        </Col>
+      </Row>
+    </Container>
  );
 };

 AlertHeader.propTypes = {
  alertSummary: PropTypes.shape({}).isRequired,
  repoModel: PropTypes.shape({}).isRequired,
+  user: PropTypes.shape({}).isRequired,
  issueTrackers: PropTypes.arrayOf(PropTypes.shape({})),
 };

--- a/ui/perfherder/alerts/AlertTable.jsx
+++ b/ui/perfherder/alerts/AlertTable.jsx
@ -9,7 +9,11 @@ import {
  errorMessageClass,
 } from '../../helpers/constants';
 import RepositoryModel from '../../models/repository';
-import { getInitializedAlerts, containsText } from '../helpers';
+import {
+  getInitializedAlerts,
+  containsText,
+  updateAlertSummary,
+} from '../helpers';
 import TruncatedText from '../../shared/TruncatedText';
 import ErrorBoundary from '../../shared/ErrorBoundary';

@ -119,6 +123,32 @@ export default class AlertTable extends React.Component {
    this.setState({ filteredAlerts });
  };

+  updateAssignee = async newAssigneeUsername => {
+    const {
+      updateAlertSummary,
+      updateViewState,
+      fetchAlertSummaries,
+    } = this.props;
+    const { alertSummary } = this.state;
+
+    const { data, failureStatus } = await updateAlertSummary(alertSummary.id, {
+      assignee_username: newAssigneeUsername,
+    });
+
+    if (!failureStatus) {
+      // now refresh UI, by syncing with backend
+      fetchAlertSummaries(alertSummary.id);
+    } else {
+      updateViewState({
+        errorMessages: [
+          `Failed to set new assignee "${newAssigneeUsername}". (${data})`,
+        ],
+      });
+    }
+
+    return { failureStatus };
+  };
+
  render() {
    const {
      user,
@ -180,6 +210,8 @@ export default class AlertTable extends React.Component {
                            alertSummary={alertSummary}
                            repoModel={repoModel}
                            issueTrackers={issueTrackers}
+                            user={user}
+                            updateAssignee={this.updateAssignee}
                          />
                        </Label>
                      </FormGroup>
@ -280,7 +312,7 @@ export default class AlertTable extends React.Component {

 AlertTable.propTypes = {
  alertSummary: PropTypes.shape({}),
-  user: PropTypes.shape({}),
+  user: PropTypes.shape({}).isRequired,
  alertSummaries: PropTypes.arrayOf(PropTypes.shape({})).isRequired,
  issueTrackers: PropTypes.arrayOf(PropTypes.shape({})),
  optionCollectionMap: PropTypes.shape({}).isRequired,
@ -293,13 +325,16 @@ AlertTable.propTypes = {
  updateViewState: PropTypes.func.isRequired,
  bugTemplate: PropTypes.shape({}),
  modifyAlert: PropTypes.func,
+  updateAlertSummary: PropTypes.func,
  projects: PropTypes.arrayOf(PropTypes.shape({})).isRequired,
 };

 AlertTable.defaultProps = {
  alertSummary: null,
-  user: null,
  issueTrackers: [],
  bugTemplate: null,
  modifyAlert: undefined,
+  // leverage dependency injection
+  // to improve code testability
+  updateAlertSummary,
 };
--- a/ui/perfherder/alerts/AlertsViewControls.jsx
+++ b/ui/perfherder/alerts/AlertsViewControls.jsx
@ -48,8 +48,14 @@ export default class AlertsViewControls extends React.Component {
  };

  render() {
+    const {
+      alertSummaries,
+      dropdownOptions,
+      fetchAlertSummaries,
+      user,
+    } = this.props;
    const { hideImprovements, hideDownstream } = this.state;
-    const { dropdownOptions, alertSummaries } = this.props;
+
    const alertFilters = [
      {
        text: 'Hide improvements',
@ -78,7 +84,9 @@ export default class AlertsViewControls extends React.Component {
              filters={this.state}
              key={alertSummary.id}
              alertSummary={alertSummary}
+              fetchAlertSummaries={fetchAlertSummaries}
              {...this.props}
+              user={user}
            />
          ))}
      </React.Fragment>
@ -91,7 +99,9 @@ AlertsViewControls.propTypes = {
    updateParams: PropTypes.func,
  }).isRequired,
  dropdownOptions: PropTypes.arrayOf(PropTypes.shape({})),
+  fetchAlertSummaries: PropTypes.func.isRequired,
  alertSummaries: PropTypes.arrayOf(PropTypes.shape({})),
+  user: PropTypes.shape({}).isRequired,
 };

 AlertsViewControls.defaultProps = {
--- a/ui/perfherder/alerts/Assignee.jsx
+++ b/ui/perfherder/alerts/Assignee.jsx
@ -0,0 +1,145 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+import { Button, Input, InputGroup } from 'reactstrap';
+
+export default class Assignee extends React.Component {
+  constructor(props) {
+    super(props);
+    const { assigneeUsername } = props;
+
+    this.state = {
+      assigneeUsername,
+      inEditMode: false,
+      newAssigneeUsername: null,
+    };
+  }
+
+  componentDidMount() {
+    // React's onKeyPress isn't able to listen to 'Escape'.
+    // This is a workaround.
+    document.addEventListener('keydown', this.keydownListener);
+  }
+
+  componentWillUnmount() {
+    document.removeEventListener('keydown', this.keydownListener);
+  }
+
+  goToEditMode = () => {
+    const { user } = this.props;
+    const { assigneeUsername } = this.state;
+
+    if (user.isStaff) {
+      this.setState({
+        inEditMode: true,
+        // input prefills with this field, so
+        // we must have it prepared
+        newAssigneeUsername: assigneeUsername,
+      });
+    }
+  };
+
+  editUsername = newAssigneeUsername => {
+    this.setState({ newAssigneeUsername });
+  };
+
+  pressedEnter = async event => {
+    if (event.key === 'Enter') {
+      const { updateAssignee } = this.props;
+      const newAssigneeUsername = event.target.value;
+
+      const { failureStatus } = await updateAssignee(newAssigneeUsername);
+
+      if (!failureStatus) {
+        this.setState({
+          assigneeUsername: newAssigneeUsername,
+          inEditMode: false,
+        });
+      }
+    }
+  };
+
+  prefillWithLoggedInUsername = () => {
+    const { user } = this.props;
+
+    this.setState({
+      newAssigneeUsername: user.username,
+      inEditMode: true,
+    });
+  };
+
+  keydownListener = event => {
+    if (event.key === 'Escape') {
+      this.setState({ inEditMode: false });
+    }
+  };
+
+  extractNicknameAndPlaceholder = assigneeUsername => {
+    let nickname = 'Unassigned';
+    const placeholder = 'nobody@mozilla.org';
+
+    if (!assigneeUsername) {
+      return { nickname, placeholder };
+    }
+
+    const nicknameRegex = /\/(\w+)@/g;
+    // eslint-disable-next-line prefer-destructuring
+    nickname = nicknameRegex.exec(assigneeUsername)[1];
+
+    return { nickname, placeholder };
+  };
+
+  render() {
+    const { user } = this.props;
+    const { assigneeUsername, newAssigneeUsername, inEditMode } = this.state;
+
+    const { nickname, placeholder } = this.extractNicknameAndPlaceholder(
+      assigneeUsername,
+    );
+
+    return !inEditMode ? (
+      <React.Fragment>
+        <Button
+          className="ml-1"
+          color="outline-info"
+          size="xs"
+          onClick={this.goToEditMode}
+          title="Click to change assignee"
+        >
+          {nickname}
+        </Button>
+        {!assigneeUsername && (
+          <Button
+            className="ml-1"
+            size="xs"
+            disabled={!user.isStaff}
+            onClick={this.prefillWithLoggedInUsername}
+          >
+            Take
+          </Button>
+        )}
+      </React.Fragment>
+    ) : (
+      <InputGroup size="sm">
+        <Input
+          disabled={!user.isStaff}
+          placeholder={placeholder}
+          value={newAssigneeUsername}
+          aria-label="Set assignee"
+          onChange={event => this.editUsername(event.target.value)}
+          onKeyPress={event => this.pressedEnter(event)}
+          autoFocus
+        />
+      </InputGroup>
+    );
+  }
+}
+
+Assignee.propTypes = {
+  updateAssignee: PropTypes.func.isRequired,
+  user: PropTypes.shape({}).isRequired,
+  assigneeUsername: PropTypes.string,
+};
+
+Assignee.defaultProps = {
+  assigneeUsername: null,
+};
--- a/ui/perfherder/alerts/StatusDropdown.jsx
+++ b/ui/perfherder/alerts/StatusDropdown.jsx
@ -12,10 +12,15 @@ import moment from 'moment';
 import template from 'lodash/template';
 import templateSettings from 'lodash/templateSettings';

-import { getTextualSummary, getTitle, getStatus } from '../helpers';
-import { getData, update } from '../../helpers/http';
+import {
+  getTextualSummary,
+  getTitle,
+  getStatus,
+  updateAlertSummary,
+} from '../helpers';
+import { getData } from '../../helpers/http';
 import { getApiUrl, bzBaseUrl, createQueryParams } from '../../helpers/url';
-import { endpoints, summaryStatusMap } from '../constants';
+import { summaryStatusMap } from '../constants';
 import DropdownMenuItems from '../../shared/DropdownMenuItems';

 import AlertModal from './AlertModal';
@ -111,10 +116,11 @@ export default class StatusDropdown extends React.Component {
  changeAlertSummary = async params => {
    const { alertSummary, updateState, updateViewState } = this.props;

-    const { data, failureStatus } = await update(
-      getApiUrl(`${endpoints.alertSummary}${alertSummary.id}/`),
+    const { data, failureStatus } = await updateAlertSummary(
+      alertSummary.id,
      params,
    );
+
    if (failureStatus) {
      return updateViewState({
        errorMessages: [
--- a/ui/perfherder/helpers.js
+++ b/ui/perfherder/helpers.js
@ -527,6 +527,9 @@ export const getTitle = alertSummary => {
  return title;
 };

+export const updateAlertSummary = async (alertSummaryId, params) =>
+  update(getApiUrl(`${endpoints.alertSummary}${alertSummaryId}/`), params);
+
 export const convertParams = (params, value) =>
  Boolean(params[value] !== undefined && parseInt(params[value], 10));