Merge pull request #9931 from seiwailai/issue-9872-sync-feedback

project: Add last job status as for project sync feedback feature.

SUMMARY
Fixes #9872

Add last job status as for the project sync feedback feature. When users clicked the sync button on the project page, last job status will eventually update the status to Pending, Waiting, Running, and final result (Successful, Cancelled, Failed, Error). The implementation requires the WebSocket connection to ensure synchronous status update.
In particular, the last job status is similar to the functionality of status in the project list (status in ProjectListItem.jsx). More specifically, the last job status accompanied by a job link that allowing the user to navigate to the job output page. Besides, there is also the tooltip that allowing the user to view the information related to the most recent sync, covering information like JOB ID, STATUS, FINISHED.
The rationale of having the last job status instead of redirection or toast notification is that:

User has choices on whether to redirect to the job output. If the user wishes to navigate to job output, he/she can click the link. Besides, the user might have other projects to be synced right after the current project and he/she may want to proceed back to the project list page instead of the job output page. If we implement force redirection, it would take a longer time to navigate to the project list page.
The status update on last job status is fundamentally similar to toast notification where the user can immediately be notified if he/she already clicked the sync button to launch the job.

Nevertheless, this PR requires further discussion. Any comments are welcomed!

ISSUE TYPE


Feature Pull Request

COMPONENT NAME


UI


awx/ui_next/src/screens/Project/Project.jsx
awx/ui_next/src/screens/Project/useWsProject.js - Added websocket implementation
awx/ui_next/src/screens/Project/ProjectDetail/ProjectDetail.jsx

AWX VERSION

awx: 19.0.0

ADDITIONAL INFORMATION

In case if users spam the sync button, we will need to ensure the fluent UI on the most recent sync tooltip and last job status. Thus, we would not want to update our last job status to Pending if there is a current running job.
For instance, we clicked sync for a particular project twice.

For the first sync, our last job status should immediately change to Pending, then Waiting, then Running, then result (which are Successful, Failed, Error, Cancelled).
For the second sync, if we have a running job, we should not update our UI to Pending, otherwise our most recent sync tooltip UI will lose our current running job and we cannot navigate to the job link through the link provided by last job status tooltip.

Issue of sync button click spam
Ideally, we should prevent any spamming on the sync button using backend logic to reduce overload on the server as we already have a similar running project. Together with backend logic, we can disable the sync button right after we start to sync a project.
However, if we only disable sync through the frontend, this seems insecure as people with bad intentions might able to change the button disable attribute.

After

Reviewed-by: Alex Corey <Alex.swansboro@gmail.com>
Reviewed-by: Sei Wai Lai <None>
Reviewed-by: Tiago Góes <tiago.goes2009@gmail.com>

awx/ui_next/src/screens/Project/ProjectDetail/ProjectDetail.jsx

@@ -1,8 +1,8 @@
-import React, { useCallback } from 'react';
+import React, { Fragment, useCallback } from 'react';
 import { Link, useHistory } from 'react-router-dom';
 import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
-import { Button, List, ListItem } from '@patternfly/react-core';
+import { Button, List, ListItem, Tooltip } from '@patternfly/react-core';
 import { Project } from '../../../types';
 import { Config } from '../../../contexts/Config';
 
@@ -22,6 +22,9 @@ import { toTitleCase } from '../../../util/strings';
 import useRequest, { useDismissableError } from '../../../util/useRequest';
 import { relatedResourceDeleteRequests } from '../../../util/getRelatedResourceDeleteDetails';
 import ProjectSyncButton from '../shared/ProjectSyncButton';
+import StatusLabel from '../../../components/StatusLabel';
+import { formatDateString } from '../../../util/dates';
+import useWsProject from './useWsProject';
 
 function ProjectDetail({ project, i18n }) {
   const {
@@ -43,7 +46,7 @@ function ProjectDetail({ project, i18n }) {
     scm_update_cache_timeout,
     scm_url,
     summary_fields,
-  } = project;
+  } = useWsProject(project);
   const history = useHistory();
 
   const { request: deleteProject, isLoading, error: deleteError } = useRequest(
@@ -84,9 +87,52 @@ function ProjectDetail({ project, i18n }) {
     );
   }
 
+  const generateLastJobTooltip = job => {
+    return (
+      <Fragment>
+        <div>{i18n._(t`MOST RECENT SYNC`)}</div>
+        <div>
+          {i18n._(t`JOB ID:`)} {job.id}
+        </div>
+        <div>
+          {i18n._(t`STATUS:`)} {job.status.toUpperCase()}
+        </div>
+        {job.finished && (
+          <div>
+            {i18n._(t`FINISHED:`)} {formatDateString(job.finished)}
+          </div>
+        )}
+      </Fragment>
+    );
+  };
+
+  let job = null;
+
+  if (summary_fields?.current_job) {
+    job = summary_fields.current_job;
+  } else if (summary_fields?.last_job) {
+    job = summary_fields.last_job;
+  }
+
   return (
     <CardBody>
       <DetailList gutter="sm">
+        <Detail
+          label={i18n._(t`Last Job Status`)}
+          value={
+            job && (
+              <Tooltip
+                position="top"
+                content={generateLastJobTooltip(job)}
+                key={job.id}
+              >
+                <Link to={`/jobs/project/${job.id}`}>
+                  <StatusLabel status={job.status} />
+                </Link>
+              </Tooltip>
+            )
+          }
+        />
         <Detail
           label={i18n._(t`Name`)}
           value={name}
@@ -171,7 +217,10 @@ function ProjectDetail({ project, i18n }) {
           </Button>
         )}
         {summary_fields.user_capabilities?.start && (
-          <ProjectSyncButton projectId={project.id} />
+          <ProjectSyncButton
+            projectId={project.id}
+            lastJobStatus={job && job.status}
+          />
         )}
         {summary_fields.user_capabilities?.delete && (
           <DeleteButton

awx/ui_next/src/screens/Project/ProjectDetail/useWsProject.js

@@ -0,0 +1,42 @@
+import { useState, useEffect } from 'react';
+import useWebsocket from '../../../util/useWebsocket';
+
+export default function useWsProjects(initialProject) {
+  const [project, setProject] = useState(initialProject);
+  const lastMessage = useWebsocket({
+    jobs: ['status_changed'],
+    control: ['limit_reached_1'],
+  });
+
+  useEffect(() => {
+    setProject(initialProject);
+  }, [initialProject]);
+
+  useEffect(
+    () => {
+      if (
+        !project ||
+        !lastMessage?.unified_job_id ||
+        lastMessage.type !== 'project_update'
+      ) {
+        return;
+      }
+
+      const updatedProject = {
+        ...project,
+        summary_fields: {
+          ...project.summary_fields,
+          current_job: {
+            id: lastMessage.unified_job_id,
+            status: lastMessage.status,
+            finished: lastMessage.finished,
+          },
+        },
+      };
+      setProject(updatedProject);
+    },
+    [lastMessage] // eslint-disable-line react-hooks/exhaustive-deps
+  );
+
+  return project;
+}

awx/ui_next/src/screens/Project/ProjectDetail/useWsProject.test.jsx

@@ -0,0 +1,141 @@
+import React from 'react';
+import { act } from 'react-dom/test-utils';
+import WS from 'jest-websocket-mock';
+import { mountWithContexts } from '../../../../testUtils/enzymeHelpers';
+import useWsProject from './useWsProject';
+
+function TestInner() {
+  return <div />;
+}
+function Test({ project }) {
+  const synced = useWsProject(project);
+  return <TestInner project={synced} />;
+}
+
+describe('useWsProject', () => {
+  let debug;
+  let wrapper;
+  beforeEach(() => {
+    debug = global.console.debug; // eslint-disable-line prefer-destructuring
+    global.console.debug = () => {};
+  });
+
+  afterEach(() => {
+    global.console.debug = debug;
+  });
+
+  test('should return project detail', async () => {
+    const project = { id: 1 };
+    await act(async () => {
+      wrapper = await mountWithContexts(<Test project={project} />);
+    });
+
+    expect(wrapper.find('TestInner').prop('project')).toEqual(project);
+    WS.clean();
+  });
+
+  test('should establish websocket connection', async () => {
+    global.document.cookie = 'csrftoken=abc123';
+    const mockServer = new WS('ws://localhost/websocket/');
+
+    const project = { id: 1 };
+    await act(async () => {
+      wrapper = await mountWithContexts(<Test project={project} />);
+    });
+
+    await mockServer.connected;
+    await expect(mockServer).toReceiveMessage(
+      JSON.stringify({
+        xrftoken: 'abc123',
+        groups: {
+          jobs: ['status_changed'],
+          control: ['limit_reached_1'],
+        },
+      })
+    );
+    WS.clean();
+  });
+
+  test('should update project status', async () => {
+    global.document.cookie = 'csrftoken=abc123';
+    const mockServer = new WS('ws://localhost/websocket/');
+
+    const project = {
+      id: 1,
+      summary_fields: {
+        last_job: {
+          id: 1,
+          status: 'successful',
+          finished: '2020-07-02T16:25:31.839071Z',
+        },
+      },
+    };
+
+    await act(async () => {
+      wrapper = await mountWithContexts(<Test project={project} />);
+    });
+
+    await mockServer.connected;
+    await expect(mockServer).toReceiveMessage(
+      JSON.stringify({
+        xrftoken: 'abc123',
+        groups: {
+          jobs: ['status_changed'],
+          control: ['limit_reached_1'],
+        },
+      })
+    );
+    expect(
+      wrapper.find('TestInner').prop('project').summary_fields.current_job
+    ).toBeUndefined();
+    expect(
+      wrapper.find('TestInner').prop('project').summary_fields.last_job.status
+    ).toEqual('successful');
+
+    await act(async () => {
+      mockServer.send(
+        JSON.stringify({
+          group_name: 'jobs',
+          project_id: 1,
+          status: 'running',
+          type: 'project_update',
+          unified_job_id: 2,
+          unified_job_template_id: 1,
+        })
+      );
+    });
+    wrapper.update();
+
+    expect(
+      wrapper.find('TestInner').prop('project').summary_fields.current_job
+    ).toEqual({
+      id: 2,
+      status: 'running',
+      finished: undefined,
+    });
+
+    await act(async () => {
+      mockServer.send(
+        JSON.stringify({
+          group_name: 'jobs',
+          project_id: 1,
+          status: 'successful',
+          type: 'project_update',
+          unified_job_id: 2,
+          unified_job_template_id: 1,
+          finished: '2020-07-02T16:28:31.839071Z',
+        })
+      );
+    });
+    wrapper.update();
+
+    expect(
+      wrapper.find('TestInner').prop('project').summary_fields.current_job
+    ).toEqual({
+      id: 2,
+      status: 'successful',
+      finished: '2020-07-02T16:28:31.839071Z',
+    });
+    WS.clean();
+  });
+});

awx/ui_next/src/screens/Project/ProjectList/ProjectListItem.jsx

@@ -93,6 +93,14 @@ function ProjectListItem({
   const missingExecutionEnvironment =
     project.custom_virtualenv && !project.default_environment;
 
+  let job = null;
+
+  if (project.summary_fields?.current_job) {
+    job = project.summary_fields.current_job;
+  } else if (project.summary_fields?.last_job) {
+    job = project.summary_fields.last_job;
+  }
+
   return (
     <>
       <Tr id={`${project.id}`}>
@@ -132,14 +140,14 @@ function ProjectListItem({
           )}
         </Td>
         <Td dataLabel={i18n._(t`Status`)}>
-          {project.summary_fields.last_job && (
+          {job && (
             <Tooltip
               position="top"
-              content={generateLastJobTooltip(project.summary_fields.last_job)}
-              key={project.summary_fields.last_job.id}
+              content={generateLastJobTooltip(job)}
+              key={job.id}
             >
-              <Link to={`/jobs/project/${project.summary_fields.last_job.id}`}>
-                <StatusLabel status={project.summary_fields.last_job.status} />
+              <Link to={`/jobs/project/${job.id}`}>
+                <StatusLabel status={job.status} />
               </Link>
             </Tooltip>
           )}
@@ -169,7 +177,10 @@ function ProjectListItem({
             visible={project.summary_fields.user_capabilities.start}
             tooltip={i18n._(t`Sync Project`)}
           >
-            <ProjectSyncButton projectId={project.id} />
+            <ProjectSyncButton
+              projectId={project.id}
+              lastJobStatus={job && job.status}
+            />
           </ActionItem>
           <ActionItem
             visible={project.summary_fields.user_capabilities.edit}

awx/ui_next/src/screens/Project/ProjectList/ProjectListItem.test.jsx

@@ -342,6 +342,10 @@ describe('<ProjectsListItem />', () => {
                   description: '',
                   name: 'Mock org',
                 },
+                last_job: {
+                  id: 9000,
+                  status: 'successful',
+                },
                 user_capabilities: {
                   start: true,
                 },

awx/ui_next/src/screens/Project/ProjectList/useWsProjects.js

@@ -26,7 +26,7 @@ export default function useWsProjects(initialProjects) {
       ...project,
       summary_fields: {
         ...project.summary_fields,
-        last_job: {
+        current_job: {
           id: lastMessage.unified_job_id,
           status: lastMessage.status,
           finished: lastMessage.finished,

awx/ui_next/src/screens/Project/ProjectList/useWsProjects.test.jsx

@@ -64,7 +64,7 @@ describe('useWsProjects', () => {
       {
         id: 1,
         summary_fields: {
-          last_job: {
+          current_job: {
             id: 1,
             status: 'running',
             finished: null,
@@ -87,7 +87,7 @@ describe('useWsProjects', () => {
       })
     );
     expect(
-      wrapper.find('TestInner').prop('projects')[0].summary_fields.last_job
+      wrapper.find('TestInner').prop('projects')[0].summary_fields.current_job
         .status
     ).toEqual('running');
     await act(async () => {
@@ -104,7 +104,7 @@ describe('useWsProjects', () => {
     wrapper.update();
 
     expect(
-      wrapper.find('TestInner').prop('projects')[0].summary_fields.last_job
+      wrapper.find('TestInner').prop('projects')[0].summary_fields.current_job
     ).toEqual({
       id: 12,
       status: 'successful',

awx/ui_next/src/screens/Project/shared/ProjectSyncButton.jsx

@@ -1,6 +1,6 @@
 import React, { useCallback } from 'react';
 import { useRouteMatch } from 'react-router-dom';
-import { Button } from '@patternfly/react-core';
+import { Button, Tooltip } from '@patternfly/react-core';
 import { SyncIcon } from '@patternfly/react-icons';
 
 import { number } from 'prop-types';
@@ -12,7 +12,7 @@ import AlertModal from '../../../components/AlertModal';
 import ErrorDetail from '../../../components/ErrorDetail';
 import { ProjectsAPI } from '../../../api';
 
-function ProjectSyncButton({ i18n, projectId }) {
+function ProjectSyncButton({ i18n, projectId, lastJobStatus = null }) {
   const match = useRouteMatch();
 
   const { request: handleSync, error: syncError } = useRequest(
@@ -24,16 +24,39 @@ function ProjectSyncButton({ i18n, projectId }) {
 
   const { error, dismissError } = useDismissableError(syncError);
   const isDetailsView = match.url.endsWith('/details');
+  const isDisabled = ['pending', 'waiting', 'running'].includes(lastJobStatus);
+
   return (
     <>
-      <Button
-        ouiaId={`${projectId}-sync-button`}
-        aria-label={i18n._(t`Sync Project`)}
-        variant={isDetailsView ? 'secondary' : 'plain'}
-        onClick={handleSync}
-      >
-        {match.url.endsWith('/details') ? i18n._(t`Sync`) : <SyncIcon />}
-      </Button>
+      {isDisabled ? (
+        <Tooltip
+          content={i18n._(
+            t`This project is currently on sync and cannot be clicked until sync process completed`
+          )}
+          position="top"
+        >
+          <div>
+            <Button
+              ouiaId={`${projectId}-sync-button`}
+              aria-label={i18n._(t`Sync Project`)}
+              variant={isDetailsView ? 'secondary' : 'plain'}
+              isDisabled={isDisabled}
+            >
+              {match.url.endsWith('/details') ? i18n._(t`Sync`) : <SyncIcon />}
+            </Button>
+          </div>
+        </Tooltip>
+      ) : (
+        <Button
+          ouiaId={`${projectId}-sync-button`}
+          aria-label={i18n._(t`Sync Project`)}
+          variant={isDetailsView ? 'secondary' : 'plain'}
+          isDisabled={isDisabled}
+          onClick={handleSync}
+        >
+          {match.url.endsWith('/details') ? i18n._(t`Sync`) : <SyncIcon />}
+        </Button>
+      )}
       {error && (
         <AlertModal
           isOpen={error}

awx/ui_next/src/screens/Project/shared/ProjectSyncButton.test.jsx

@@ -41,6 +41,32 @@ describe('ProjectSyncButton', () => {
 
     expect(ProjectsAPI.sync).toHaveBeenCalledWith(1);
   });
+  test('disable button and set onClick to undefined on sync', async () => {
+    await act(async () => {
+      wrapper = mountWithContexts(
+        <ProjectSyncButton projectId={1} lastJobStatus="running">
+          {children}
+        </ProjectSyncButton>
+      );
+    });
+
+    expect(wrapper.find('Button').prop('isDisabled')).toBe(true);
+    expect(wrapper.find('Button').prop('onClick')).toBe(undefined);
+  });
+  test('should render tooltip on sync', async () => {
+    await act(async () => {
+      wrapper = mountWithContexts(
+        <ProjectSyncButton projectId={1} lastJobStatus="running">
+          {children}
+        </ProjectSyncButton>
+      );
+    });
+
+    expect(wrapper.find('Tooltip')).toHaveLength(1);
+    expect(wrapper.find('Tooltip').prop('content')).toEqual(
+      'This project is currently on sync and cannot be clicked until sync process completed'
+    );
+  });
   test('displays error modal after unsuccessful sync', async () => {
     ProjectsAPI.sync.mockRejectedValue(
       new Error({