Merge pull request #10334 from mabashian/outdated-revision

Adds ability to refresh project revision on sync'd rows in the project list

SUMMARY
This PR also adds the revision to the project details view as well as handles updating the revision on the project details view when the project is done syncing.
Since the project revision is not included in the websocket payload, when a project is done syncing the displayed revision may be out of date.  As such, we wanted to expose that information to the user and give them the ability to "refresh" and fetch the new revision.
Here's what that flow looks like:

When a particular row finishes syncing the user should see this in place of the revision:

Clicking on that refresh button goes out and fetches the updated project (and with it the potentially updated revision).
We don't do this automatically on the projects list (and force the user to click on the refresh button) is due to issues we've had in the past with the UI triggering API calls based on websocket events.
The flow when a user is on the project details view is a little different because I wasn't as worried about spamming the API with requests.
When a project finishes syncing and the user is viewing the details I do go ahead and automatically refresh the project data (and with it, the revision).  Here's what that looks like:

A few other notes:

@tiagodread @akus062381 @one-t I'm almost certain this is going to break some tests because I removed the ClipboardCopyButton in favor of PF's ClipboardCopy component.  This component looks and behaves slightly differently from our home grown solution.  Additionally, the PF ClipboardCopy button does not expose the ouiaId prop so I had to add a data-cy on that component.  You'll likely have to use that identifier to then grab the button inside in order to test out the clipboard copy functionality.
Source Control Revision is a net-new detail in the project details.  I think this was simply missed on our initial build of this page.

Here are the identifiers on the various bits:



Note that the identifiers on the project rows have the id of the project appended to them for uniqueness.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME

UI

Reviewed-by: Jake McDermott <yo@jakemcdermott.me>
Reviewed-by: Michael Abashian <None>
Reviewed-by: Sarah Akus <sarah.akus@gmail.com>
This commit is contained in:
softwarefactory-project-zuul[bot] 2021-06-08 14:14:54 +00:00 committed by GitHub
commit ac5b53b13c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 300 additions and 207 deletions

View File

@ -1,90 +0,0 @@
import React from 'react';
import PropTypes from 'prop-types';
import { Button, Tooltip } from '@patternfly/react-core';
import { CopyIcon } from '@patternfly/react-icons';
export const clipboardCopyFunc = (event, text) => {
const clipboard = event.currentTarget.parentElement;
const el = document.createElement('input');
el.value = text;
clipboard.appendChild(el);
el.select();
document.execCommand('copy');
clipboard.removeChild(el);
};
class ClipboardCopyButton extends React.Component {
constructor(props) {
super(props);
this.state = {
copied: false,
};
this.handleCopyClick = this.handleCopyClick.bind(this);
}
handleCopyClick = event => {
const { stringToCopy, switchDelay } = this.props;
if (this.timer) {
window.clearTimeout(this.timer);
this.setState({ copied: false });
}
clipboardCopyFunc(event, stringToCopy);
this.setState({ copied: true }, () => {
this.timer = window.setTimeout(() => {
this.setState({ copied: false });
this.timer = null;
}, switchDelay);
});
};
render() {
const {
copyTip,
entryDelay,
exitDelay,
copiedSuccessTip,
isDisabled,
ouiaId,
} = this.props;
const { copied } = this.state;
return (
<Tooltip
entryDelay={entryDelay}
exitDelay={exitDelay}
trigger="mouseenter focus click"
content={copied ? copiedSuccessTip : copyTip}
>
<Button
ouiaId={ouiaId}
isDisabled={isDisabled}
variant="plain"
onClick={this.handleCopyClick}
aria-label={copyTip}
>
<CopyIcon />
</Button>
</Tooltip>
);
}
}
ClipboardCopyButton.propTypes = {
copyTip: PropTypes.string.isRequired,
entryDelay: PropTypes.number,
exitDelay: PropTypes.number,
copiedSuccessTip: PropTypes.string.isRequired,
stringToCopy: PropTypes.string.isRequired,
switchDelay: PropTypes.number,
isDisabled: PropTypes.bool.isRequired,
};
ClipboardCopyButton.defaultProps = {
entryDelay: 100,
exitDelay: 1600,
switchDelay: 2000,
};
export default ClipboardCopyButton;

View File

@ -1,61 +0,0 @@
import React from 'react';
import { mountWithContexts } from '../../../testUtils/enzymeHelpers';
import ClipboardCopyButton from './ClipboardCopyButton';
describe('ClipboardCopyButton', () => {
beforeEach(() => {
document.execCommand = jest.fn();
});
afterEach(() => {
jest.clearAllMocks();
});
test('renders the expected content', () => {
const wrapper = mountWithContexts(
<ClipboardCopyButton
clickTip="foo"
hoverTip="bar"
copyTip="baz"
copiedSuccessTip="qux"
stringToCopy="foobar!"
isDisabled={false}
/>
);
expect(wrapper).toHaveLength(1);
});
test('clicking button calls execCommand to copy to clipboard', async () => {
const mockDelay = 1;
const wrapper = mountWithContexts(
<ClipboardCopyButton
clickTip="foo"
hoverTip="bar"
copyTip="baz"
copiedSuccessTip="qux"
stringToCopy="foobar!"
isDisabled={false}
switchDelay={mockDelay}
/>
).find('ClipboardCopyButton');
expect(wrapper.state('copied')).toBe(false);
wrapper.find('Button').simulate('click');
expect(document.execCommand).toBeCalledWith('copy');
expect(wrapper.state('copied')).toBe(true);
await new Promise(resolve => setTimeout(resolve, mockDelay));
wrapper.update();
expect(wrapper.state('copied')).toBe(false);
});
test('should render disabled button', () => {
const wrapper = mountWithContexts(
<ClipboardCopyButton
clickTip="foo"
hoverTip="bar"
copyTip="baz"
copiedSuccessTip="qux"
stringToCopy="foobar!"
isDisabled
/>
);
expect(wrapper.find('Button').prop('isDisabled')).toBe(true);
});
});

View File

@ -1 +0,0 @@
export { default } from './ClipboardCopyButton';

View File

@ -1,8 +1,6 @@
import React, { useEffect } from 'react';
import { t } from '@lingui/macro';
import PropTypes from 'prop-types';
import { Button } from '@patternfly/react-core';
import { CopyIcon } from '@patternfly/react-icons';
import useRequest, { useDismissableError } from '../../util/useRequest';
@ -16,7 +14,6 @@ function CopyButton({
onCopyStart,
onCopyFinish,
errorMessage,
ouiaId,
}) {
const { isLoading, error: copyError, request: copyItemToAPI } = useRequest(
@ -44,16 +41,18 @@ function CopyButton({
>
<CopyIcon />
</Button>
<AlertModal
aria-label={t`Copy Error`}
isOpen={error}
variant="error"
title={t`Error!`}
onClose={dismissError}
>
{errorMessage}
<ErrorDetail error={error} />
</AlertModal>
{error && (
<AlertModal
aria-label={t`Copy Error`}
isOpen={error}
variant="error"
title={t`Error!`}
onClose={dismissError}
>
{errorMessage}
<ErrorDetail error={error} />
</AlertModal>
)}
</>
);
}

View File

@ -1,11 +1,16 @@
import React, { Fragment, useCallback } from 'react';
import { Link, useHistory } from 'react-router-dom';
import { t } from '@lingui/macro';
import { Button, List, ListItem, Tooltip } from '@patternfly/react-core';
import styled from 'styled-components';
import {
Button,
ClipboardCopy,
List,
ListItem,
Tooltip,
} from '@patternfly/react-core';
import { Project } from '../../../types';
import { Config } from '../../../contexts/Config';
import AlertModal from '../../../components/AlertModal';
import { CardBody, CardActionsRow } from '../../../components/Card';
import DeleteButton from '../../../components/DeleteButton';
@ -27,6 +32,10 @@ import StatusLabel from '../../../components/StatusLabel';
import { formatDateString } from '../../../util/dates';
import useWsProject from './useWsProject';
const Label = styled.span`
color: var(--pf-global--disabled-color--100);
`;
function ProjectDetail({ project }) {
const {
allow_override,
@ -42,6 +51,7 @@ function ProjectDetail({ project }) {
scm_delete_on_update,
scm_track_submodules,
scm_refspec,
scm_revision,
scm_type,
scm_update_on_launch,
scm_update_cache_timeout,
@ -146,6 +156,31 @@ function ProjectDetail({ project }) {
label={t`Source Control Type`}
value={scm_type === '' ? t`Manual` : toTitleCase(project.scm_type)}
/>
<Detail
label={t`Source Control Revision`}
value={
scm_revision ? (
<ClipboardCopy
data-cy="project-copy-revision"
variant="inline-compact"
clickTip={t`Successfully copied to clipboard!`}
hoverTip={t`Copy full revision to clipboard.`}
onCopy={() =>
navigator.clipboard.writeText(scm_revision.toString())
}
>
{scm_revision.substring(0, 7)}
</ClipboardCopy>
) : (
<Label
aria-label={t`The project must be synced before a revision is available.`}
>
{t`Sync for revision`}
</Label>
)
}
alwaysVisible
/>
<Detail label={t`Source Control URL`} value={scm_url} />
<Detail label={t`Source Control Branch`} value={scm_branch} />
<Detail label={t`Source Control Refspec`} value={scm_refspec} />

View File

@ -1,5 +1,6 @@
import { useState, useEffect } from 'react';
import useWebsocket from '../../../util/useWebsocket';
import { ProjectsAPI } from '../../../api';
export default function useWsProjects(initialProject) {
const [project, setProject] = useState(initialProject);
@ -8,6 +9,11 @@ export default function useWsProjects(initialProject) {
control: ['limit_reached_1'],
});
const refreshProject = async () => {
const { data } = await ProjectsAPI.readDetail(project.id);
setProject(data);
};
useEffect(() => {
setProject(initialProject);
}, [initialProject]);
@ -22,6 +28,11 @@ export default function useWsProjects(initialProject) {
return;
}
if (lastMessage.finished) {
refreshProject();
return;
}
const updatedProject = {
...project,
summary_fields: {

View File

@ -2,8 +2,11 @@ import React from 'react';
import { act } from 'react-dom/test-utils';
import WS from 'jest-websocket-mock';
import { mountWithContexts } from '../../../../testUtils/enzymeHelpers';
import { ProjectsAPI } from '../../../api';
import useWsProject from './useWsProject';
jest.mock('../../../api/models/Projects');
function TestInner() {
return <div />;
}
@ -15,13 +18,30 @@ function Test({ project }) {
describe('useWsProject', () => {
let debug;
let wrapper;
beforeEach(() => {
debug = global.console.debug; // eslint-disable-line prefer-destructuring
global.console.debug = () => {};
ProjectsAPI.readDetail.mockResolvedValue({
data: {
id: 1,
summary_fields: {
last_job: {
id: 19,
name: 'Test Project',
description: '',
finished: '2021-06-01T18:43:53.332201Z',
status: 'successful',
failed: false,
},
},
},
});
});
afterEach(() => {
global.console.debug = debug;
jest.clearAllMocks();
});
test('should return project detail', async () => {
@ -127,14 +147,20 @@ describe('useWsProject', () => {
})
);
});
wrapper.update();
expect(ProjectsAPI.readDetail).toHaveBeenCalledTimes(1);
expect(
wrapper.find('TestInner').prop('project').summary_fields.current_job
wrapper.find('TestInner').prop('project').summary_fields.last_job
).toEqual({
id: 2,
id: 19,
name: 'Test Project',
description: '',
finished: '2021-06-01T18:43:53.332201Z',
status: 'successful',
finished: '2020-07-02T16:28:31.839071Z',
failed: false,
});
WS.clean();
});

View File

@ -2,9 +2,11 @@ import React, { useEffect, useCallback } from 'react';
import { useLocation, useRouteMatch } from 'react-router-dom';
import { t, Plural } from '@lingui/macro';
import { Card, PageSection } from '@patternfly/react-core';
import { ProjectsAPI } from '../../../api';
import useRequest, { useDeleteItems } from '../../../util/useRequest';
import useRequest, {
useDeleteItems,
useDismissableError,
} from '../../../util/useRequest';
import AlertModal from '../../../components/AlertModal';
import DataListToolbar from '../../../components/DataListToolbar';
import ErrorDetail from '../../../components/ErrorDetail';
@ -33,6 +35,21 @@ function ProjectList() {
const location = useLocation();
const match = useRouteMatch();
const {
request: fetchUpdatedProject,
error: fetchUpdatedProjectError,
result: updatedProject,
} = useRequest(
useCallback(async projectId => {
if (!projectId) {
return {};
}
const { data } = await ProjectsAPI.readDetail(projectId);
return data;
}, []),
null
);
const {
result: {
results,
@ -44,6 +61,7 @@ function ProjectList() {
error: contentError,
isLoading,
request: fetchProjects,
setValue: setProjects,
} = useRequest(
useCallback(async () => {
const params = parseQueryString(QS_CONFIG, location.search);
@ -115,6 +133,27 @@ function ProjectList() {
selected[0]
);
useEffect(() => {
if (updatedProject) {
const updatedProjects = projects.map(project =>
project.id === updatedProject.id ? updatedProject : project
);
setProjects({
results: updatedProjects,
itemCount,
actions,
relatedSearchableKeys,
searchableKeys,
});
}
/* eslint-disable-next-line react-hooks/exhaustive-deps */
}, [updatedProject]);
const {
error: projectError,
dismissError: dismissProjectError,
} = useDismissableError(fetchUpdatedProjectError);
return (
<>
<PageSection>
@ -214,6 +253,7 @@ function ProjectList() {
isSelected={selected.some(row => row.id === project.id)}
onSelect={() => handleSelect(project)}
rowIndex={index}
onRefreshRow={projectId => fetchUpdatedProject(projectId)}
/>
)}
emptyStateControls={
@ -224,16 +264,30 @@ function ProjectList() {
/>
</Card>
</PageSection>
<AlertModal
isOpen={deletionError}
variant="error"
aria-label={t`Deletion Error`}
title={t`Error!`}
onClose={clearDeletionError}
>
{t`Failed to delete one or more projects.`}
<ErrorDetail error={deletionError} />
</AlertModal>
{deletionError && (
<AlertModal
isOpen={deletionError}
variant="error"
aria-label={t`Deletion Error`}
title={t`Error!`}
onClose={clearDeletionError}
>
{t`Failed to delete one or more projects.`}
<ErrorDetail error={deletionError} />
</AlertModal>
)}
{projectError && (
<AlertModal
isOpen={projectError}
variant="error"
aria-label={t`Error fetching updated project`}
title={t`Error!`}
onClose={dismissProjectError}
>
{t`Failed to fetch the updated project data.`}
<ErrorDetail error={projectError} />
</AlertModal>
)}
</>
);
}

View File

@ -1,20 +1,19 @@
import 'styled-components/macro';
import React, { Fragment, useState, useCallback } from 'react';
import { string, bool, func } from 'prop-types';
import { Button, Tooltip } from '@patternfly/react-core';
import { Button, ClipboardCopy, Tooltip } from '@patternfly/react-core';
import { Tr, Td, ExpandableRowContent } from '@patternfly/react-table';
import { t } from '@lingui/macro';
import { Link } from 'react-router-dom';
import {
PencilAltIcon,
ExclamationTriangleIcon as PFExclamationTriangleIcon,
UndoIcon,
} from '@patternfly/react-icons';
import styled from 'styled-components';
import { ActionsTd, ActionItem } from '../../../components/PaginatedTable';
import { formatDateString, timeOfDay } from '../../../util/dates';
import { ProjectsAPI } from '../../../api';
import ClipboardCopyButton from '../../../components/ClipboardCopyButton';
import {
DetailList,
Detail,
@ -23,6 +22,7 @@ import {
import ExecutionEnvironmentDetail from '../../../components/ExecutionEnvironmentDetail';
import StatusLabel from '../../../components/StatusLabel';
import { toTitleCase } from '../../../util/strings';
import { isJobRunning } from '../../../util/jobs';
import CopyButton from '../../../components/CopyButton';
import ProjectSyncButton from '../shared/ProjectSyncButton';
import { Project } from '../../../types';
@ -44,6 +44,7 @@ function ProjectListItem({
detailUrl,
fetchProjects,
rowIndex,
onRefreshRow,
}) {
const [isExpanded, setIsExpanded] = useState(false);
const [isDisabled, setIsDisabled] = useState(false);
@ -88,6 +89,62 @@ function ProjectListItem({
setIsDisabled(false);
}, []);
const renderRevision = () => {
if (!project.summary_fields?.current_job || project.scm_revision) {
return project.scm_revision ? (
<ClipboardCopy
data-cy={`project-copy-revision-${project.id}`}
variant="inline-compact"
clickTip={t`Successfully copied to clipboard!`}
hoverTip={t`Copy full revision to clipboard.`}
onCopy={() =>
navigator.clipboard.writeText(project.scm_revision.toString())
}
>
{project.scm_revision.substring(0, 7)}
</ClipboardCopy>
) : (
<Label
aria-label={t`The project must be synced before a revision is available.`}
>
{t`Sync for revision`}
</Label>
);
}
if (
isJobRunning(project.summary_fields.current_job.status) &&
!project.scm_revision
) {
return (
<Label
aria-label={t`The project is currently syncing and the revision will be available after the sync is complete.`}
>
{t`Syncing`}
</Label>
);
}
return (
<>
<Label
aria-label={t`The project revision is currently out of date. Please refresh to fetch the most recent revision.`}
>
{t`Refresh for revision`}
</Label>
<Tooltip content={t`Refresh project revision`}>
<Button
ouiaId={`project-refresh-revision-${project.id}`}
variant="plain"
onClick={() => onRefreshRow(project.id)}
>
<UndoIcon />
</Button>
</Tooltip>
</>
);
};
const labelId = `check-action-${project.id}`;
const missingExecutionEnvironment =
@ -153,21 +210,7 @@ function ProjectListItem({
<Td dataLabel={t`Type`}>
{project.scm_type === '' ? t`Manual` : toTitleCase(project.scm_type)}
</Td>
<Td dataLabel={t`Revision`}>
{project.scm_revision.substring(0, 7)}
{!project.scm_revision && (
<Label aria-label={t`copy to clipboard disabled`}>
{t`Sync for revision`}
</Label>
)}
<ClipboardCopyButton
isDisabled={!project.scm_revision}
stringToCopy={project.scm_revision}
copyTip={t`Copy full revision to clipboard.`}
copiedSuccessTip={t`Successfully copied to clipboard!`}
ouiaId="copy-revision-button"
/>
</Td>
<Td dataLabel={t`Revision`}>{renderRevision()}</Td>
<ActionsTd dataLabel={t`Actions`}>
{['running', 'pending', 'waiting'].includes(job?.status) ? (
<ActionItem

View File

@ -215,7 +215,7 @@ describe('<ProjectsListItem />', () => {
});
test('should render proper alert modal on copy error', async () => {
ProjectsAPI.copy.mockRejectedValue(new Error());
ProjectsAPI.copy.mockRejectedValue(new Error('This is an error'));
const wrapper = mountWithContexts(
<table>
@ -285,7 +285,7 @@ describe('<ProjectsListItem />', () => {
);
expect(wrapper.find('CopyButton').length).toBe(0);
});
test('should render disabled copy to clipboard button', () => {
test('should render proper revision text when project has not been synced', () => {
const wrapper = mountWithContexts(
<table>
<tbody>
@ -314,10 +314,82 @@ describe('<ProjectsListItem />', () => {
</tbody>
</table>
);
expect(
wrapper.find('span[aria-label="copy to clipboard disabled"]').text()
).toBe('Sync for revision');
expect(wrapper.find('ClipboardCopyButton').prop('isDisabled')).toBe(true);
expect(wrapper.find('ClipboardCopy').length).toBe(0);
expect(wrapper.find('td[data-label="Revision"]').text()).toBe(
'Sync for revision'
);
});
test('should render the clipboard copy with the right text when scm revision available', () => {
const wrapper = mountWithContexts(
<table>
<tbody>
<ProjectsListItem
isSelected={false}
detailUrl="/project/1"
onSelect={() => {}}
project={{
id: 1,
name: 'Project 1',
url: '/api/v2/projects/1',
type: 'project',
scm_type: 'git',
scm_revision: 'osofej904r09a9sf0udfsajogsdfbh4e23489adf',
summary_fields: {
last_job: {
id: 9000,
status: 'successful',
},
user_capabilities: {
edit: true,
},
},
}}
/>
</tbody>
</table>
);
expect(wrapper.find('ClipboardCopy').length).toBe(1);
expect(wrapper.find('ClipboardCopy').text()).toBe('osofej9');
});
test('should indicate that the revision needs to be refreshed when project sync is done', () => {
const wrapper = mountWithContexts(
<table>
<tbody>
<ProjectsListItem
isSelected={false}
detailUrl="/project/1"
onSelect={() => {}}
project={{
id: 1,
name: 'Project 1',
url: '/api/v2/projects/1',
type: 'project',
scm_type: 'git',
scm_revision: null,
summary_fields: {
current_job: {
id: 9001,
status: 'successful',
finished: '2021-06-01T18:43:53.332201Z',
},
last_job: {
id: 9000,
status: 'successful',
},
user_capabilities: {
edit: true,
},
},
}}
/>
</tbody>
</table>
);
expect(wrapper.find('ClipboardCopy').length).toBe(0);
expect(wrapper.find('td[data-label="Revision"]').text()).toBe(
'Refresh for revision'
);
expect(wrapper.find('UndoIcon').length).toBe(1);
});
test('should render expected details in expanded section', async () => {
const wrapper = mountWithContexts(

View File

@ -33,6 +33,11 @@ export default function useWsProjects(initialProjects) {
},
},
};
if (lastMessage.finished) {
updatedProject.scm_revision = null;
}
setProjects([
...projects.slice(0, index),
updatedProject,