Merge pull request #4402 from cloud-gov/4379-explain-that-build-logs-…

…are-gone-after-180-days

fix: Add message explaining that build logs are deleted after 180 days.

frontend/components/site/CommitSummary.jsx

@@ -1,7 +1,6 @@
-import React, { useState, useEffect } from 'react';
+import React from 'react';
 import PropTypes from 'prop-types';
 
-import api from '../../util/federalistApi';
 import { IconBranch } from '../icons';
 import LoadingIndicator from '../LoadingIndicator';
 import { timeFrom, dateAndTime } from '../../util/datetime';
@@ -27,23 +26,8 @@ function buildShaLink(owner, repo, sha) {
   );
 }
 
-function CommitSummary({ buildId }) {
-  const [build, setBuild] = useState({
-    isLoading: true,
-    buildDetails: null,
-  });
-  const { isLoading, buildDetails } = build;
-
-  useEffect(() => {
-    if (!buildDetails) {
-      api.fetchBuild(buildId).then(data => setBuild({
-        isLoading: false,
-        buildDetails: data,
-      }));
-    }
-  }, [buildDetails]);
-
-  if (isLoading) {
+function CommitSummary({ buildDetails }) {
+  if (!buildDetails) {
     return <LoadingIndicator size="mini" text="Getting commit details..." />;
   }
 
@@ -74,7 +58,20 @@ function CommitSummary({ buildId }) {
   );
 }
 CommitSummary.propTypes = {
-  buildId: PropTypes.number.isRequired,
+  buildDetails: PropTypes.shape({
+    site: PropTypes.shape({
+      owner: PropTypes.string.isRequired,
+      repository: PropTypes.string.isRequired,
+    }).isRequired,
+    branch: PropTypes.string.isRequired,
+    username: PropTypes.string.isRequired,
+    clonedCommitSha: PropTypes.string.isRequired,
+    createdAt: PropTypes.instanceOf(Date).isRequired,
+  }),
+};
+
+CommitSummary.defaultProps = {
+  buildDetails: null,
 };
 
 export { CommitSummary };

frontend/components/site/siteBuildLogs.jsx

@@ -3,21 +3,42 @@
 import React from 'react';
 import { Link, useParams } from 'react-router-dom';
 
-import { useBuildLogs } from '../../hooks';
+import { useBuildLogs, useBuildDetails } from '../../hooks';
+import LoadingIndicator from '../LoadingIndicator';
 import SiteBuildLogTable from './siteBuildLogTable';
 import DownloadBuildLogsButton from './downloadBuildLogsButton';
 import CommitSummary from './CommitSummary';
 
 export const REFRESH_INTERVAL = 15 * 1000;
+const BUILD_LOG_RETENTION_LIMIT = 180 * 24 * 60 * 60 * 1000; // 180 days in milliseconds
+
+function buildTooOld(build) {
+  return (new Date() - new Date(build.completedAt)) > BUILD_LOG_RETENTION_LIMIT;
+}
+
+function getSiteBuildLogTable(buildDetails, logs, state) {
+  if (logs && logs?.length > 0) {
+    return <SiteBuildLogTable buildLogs={logs} buildState={state} />;
+  }
+  if (buildTooOld(buildDetails)) {
+    return <SiteBuildLogTable buildLogs={['Builds more than 180 days old are deleted according to platform policy.']} />;
+  }
+  return <SiteBuildLogTable buildLogs={['This build does not have any build logs.']} />;
+}
 
 const SiteBuildLogs = () => {
   const { buildId: buildIdStr } = useParams();
   const buildId = parseInt(buildIdStr, 10);
   const { logs, state } = useBuildLogs(buildId);
+  const { buildDetails, isLoading } = useBuildDetails(buildId);
+
+  if (isLoading || !logs) {
+    return <LoadingIndicator size="mini" text="Getting build log details..." />;
+  }
 
   return (
     <div>
-      <CommitSummary buildId={buildId} />
+      <CommitSummary buildDetails={buildDetails} />
       <div className="log-tools">
         <ul className="usa-unstyled-list">
           {(process.env.FEATURE_BUILD_TASKS === 'active') && (
@@ -26,14 +47,7 @@ const SiteBuildLogs = () => {
           <li><DownloadBuildLogsButton buildId={buildId} buildLogsData={logs} /></li>
         </ul>
       </div>
-      {(!logs || logs?.length === 0) && (
-      <div>
-        <SiteBuildLogTable buildLogs={['This build does not have any build logs.']} />
-      </div>
-      )}
-      {(logs && logs?.length > 0) && (
-      <SiteBuildLogTable buildLogs={logs} buildState={state} />
-      )}
+      { getSiteBuildLogTable(buildDetails, logs, state) }
     </div>
   );
 };

frontend/components/site/siteBuildTasks.jsx

@@ -7,6 +7,7 @@ import prettyBytes from 'pretty-bytes';
 import {
   IconCheckCircle, IconClock, IconExclamationCircle, IconSpinner,
 } from '../icons';
+import { useBuildDetails } from '../../hooks';
 import CommitSummary from './CommitSummary';
 import api from '../../util/federalistApi';
 
@@ -55,6 +56,8 @@ const SiteBuildTasks = () => {
   const buildId = parseInt(buildIdStr, 10);
   const [buildTasks, setBuildTasks] = useState([]);
 
+  const { buildDetails } = useBuildDetails(buildId);
+
   let intervalHandle;
   useEffect(() => {
     function fetchTasks(thisBuildId) {
@@ -76,7 +79,7 @@ const SiteBuildTasks = () => {
 
   return (
     <div>
-      <CommitSummary buildId={buildId} />
+      <CommitSummary buildDetails={buildDetails} />
       <div className="log-tools">
         <ul className="usa-unstyled-list">
           <li>

frontend/hooks/index.js

@@ -1,3 +1,4 @@
+export * from './useBuildDetails';
 export * from './useBuildLogs';
 export * from './useSiteBranchConfigs';
 export * from './useSiteDomains';

frontend/hooks/useBuildDetails.js

@@ -0,0 +1,23 @@
+/* eslint-disable import/prefer-default-export */
+import { useEffect, useState } from 'react';
+import api from '../util/federalistApi';
+
+const initResultsState = {
+  buildDetails: null,
+  isLoading: true,
+};
+
+export const useBuildDetails = (id) => {
+  const [results, setResults] = useState(initResultsState);
+
+  useEffect(() => {
+    if (!results.buildDetails) {
+      api.fetchBuild(id).then(data => setResults({
+        isLoading: false,
+        buildDetails: data,
+      }));
+    }
+  }, [results]);
+
+  return results;
+};

test/frontend/components/site/CommitSummary.test.js

@@ -1,79 +1,58 @@
 import React from 'react';
 import { expect, assert } from 'chai';
 import proxyquire from 'proxyquire';
-import sinon from 'sinon';
 import { mount } from 'enzyme';
 
-import api from '../../../../frontend/util/federalistApi';
 import LoadingIndicator from '../../../../frontend/components/LoadingIndicator';
 
-proxyquire.noCallThru();
-
-const fetchBuildMock = sinon.stub();
-const buildActions = {
-  fetchBuild: fetchBuildMock,
-};
 const CommitSummary = proxyquire(
   '../../../../frontend/components/site/CommitSummary',
   {
     '../icons': {
       IconBranch: () => <span />,
     },
-    '../../actions/buildActions': buildActions,
   }
 ).default;
 
 const defaultProps = {
-  buildId: 1,
+  buildDetails: {
+    site: {
+      owner: 'user',
+      repository: 'repo',
+    },
+    branch: 'branch',
+    username: 'username',
+    clonedCommitSha: 'sha4567890abcdef',
+    createdAt: new Date(),
+  }
 };
 
 describe('<CommitSummary />', () => {
-  afterEach(() => {
-    sinon.restore();
-  });
-
   it('should exist', () => {
     assert.isDefined(CommitSummary);
   });
 
-  it('renders a loading state whie loading', () => {
-    const stub = sinon.stub(api, 'fetchBuild');
-    stub.resolves([]);
-
-    const wrapper = mount(<CommitSummary {...defaultProps} />);
+  it('renders a loading state while loading', () => {
+    const wrapper = mount(<CommitSummary { ...{ buildDetails: null } } />);
     expect(wrapper.find(LoadingIndicator)).to.have.length(1);
   });
 
-  // no useEffect in tests
-  // it('requests build information once on load', () => {
-  //   const wrapper = mountStore(<CommitSummary {...defaultProps} />, defaultState);
-  //   const buildId = 1;
-  //   expect(fetchBuildMock.callCount).to.be.greaterThanOrEqual(1);
-  //   fetchBuildMock.resetHistory();
-  //   sinon.restore();
-  // });
-
-  // describe('after load', () => {
-  //   let wrapper;
-  //   let loadedState = lodashClonedeep(defaultState);
-  //   loadedState.build = {
-  //     isLoading: false,
-  //     data: { ...defaultBuildData }
-  //   };
-
-  //   it('renders the branch and github user name for the commit', () => {
-  //     wrapper = mountStore(<CommitSummary {...defaultProps} />, loadedState);
-  //     expect(wrapper.find('.commit-branch')).to.have.length(1);
-  //     expect(wrapper.find('.commit-branch').text()).to.contain(defaultBuildData.branch);
-  //     expect(wrapper.find('.commit-username')).to.have.length(1);
-  //     expect(wrapper.find('.commit-username').text()).to.equal(defaultBuildData.username);
-  //   });
-
-  //   it('formats a sha link correctly and limits to first 7 chars', () => {
-  //     wrapper = mountStore(<CommitSummary {...defaultProps} />, loadedState);
-  //     expect(wrapper.find('.sha-link')).to.have.length(1);
-  //     expect(defaultBuildData.clonedCommitSha).to.contain(wrapper.find('.sha-link').text());
-  //     expect(wrapper.find('.sha-link').text()).to.have.length(7);
-  //   });
-  // });
+  describe('after load', () => {
+    const build = defaultProps.buildDetails;
+
+    it('renders the branch and github user name for the commit', () => {
+      const wrapper = mount(<CommitSummary {...defaultProps} />);
+      expect(wrapper.find('.commit-branch')).to.have.length(1);
+      expect(wrapper.find('.commit-branch').text()).to.contain(build.branch);
+      expect(wrapper.find('.commit-username')).to.have.length(1);
+      expect(wrapper.find('.commit-username').text()).to.equal(build.username);
+    });
+
+    it('formats a sha link correctly and limits to first 7 chars', () => {
+      const wrapper = mount(<CommitSummary {...defaultProps} />);
+      expect(wrapper.find('.sha-link')).to.have.length(1);
+      expect(build.clonedCommitSha).to.contain(wrapper.find('.sha-link').text());
+      expect(wrapper.find('.sha-link').text()).to.have.length(7);
+    });
+  });
 });

test/frontend/components/site/siteBuildLogs.test.js

@@ -1,20 +1,50 @@
 import React from 'react';
-import { expect } from 'chai';
+import { expect, assert } from 'chai';
 import { shallow } from 'enzyme';
 import sinon from 'sinon';
 
+import api from '../../../../frontend/util/federalistApi';
 import { SiteBuildLogs } from '../../../../frontend/components/site/siteBuildLogs';
 import LoadingIndicator from '../../../../frontend/components/LoadingIndicator';
 
 let props;
 
+// Initial work for when the containing .skip can be removed
+const defaultBuild = {
+  site: {
+    owner: 'user',
+    repository: 'repo',
+  },
+  branch: 'branch',
+  username: 'username',
+  clonedCommitSha: 'sha4567890abcdef',
+  createdAt: new Date(),
+};
+
 describe.skip('<SiteBuildLogs/>', () => {
   beforeEach(() => {
     props = {
-      buildId: '123'
+      buildId: '123',
+      buildDetails: defaultBuild,
+      isLoading: false,
+      logs: [],
     };
   });
 
+  afterEach(() => {
+    sinon.restore();
+  });
+
+  // For when the containing .skip can be removed
+  it('should exist', () => {
+    assert.isDefined(SiteBuildLogs);
+  });
+
+  // Placeholder for when the containing .skip can be removed
+  it('requests build information once on load', () => {
+    // Verify that fetchBuild is called once
+  });
+
   it('should render a button to download logs if builds exist', () => {
     const wrapper = shallow(<SiteBuildLogs {...props} />);
     expect(wrapper.find('SiteBuildLogTable')).to.have.length(1);
@@ -32,6 +62,21 @@ describe.skip('<SiteBuildLogs/>', () => {
     expect(wrapper.find(LoadingIndicator)).to.have.length(1);
   });
 
+  // Placeholder for when the containing .skip can be removed
+  it('should render an explanatory message if the build is over 180 days old', () => {
+    const stubBuild = { completedAt: new Date(Date.now() - (181 * 86400000)) }; // 181 days ago
+    const stub = sinon.stub(api, 'fetchBuild');
+    stub.resolves(stubBuild);
+
+    props.buildLogs.isLoading = false;
+    props.buildLogs.data = [];
+
+    const wrapper = shallow(<SiteBuildLogs {...props} />);
+    expect(wrapper.find('SiteBuildLogTable')).to.have.length(0);
+    expect(wrapper.find('p').contains('Builds more than 180 days old are deleted according to platform policy.')).to.be.true;
+    expect(wrapper.find('DownloadBuildLogsButton')).to.have.length(0);
+  });
+
   it('should render an empty state if there are no builds', () => {
     props.buildLogs.isLoading = false;
     props.buildLogs.data = [];
@@ -42,7 +87,7 @@ describe.skip('<SiteBuildLogs/>', () => {
     expect(wrapper.find('DownloadBuildLogsButton')).to.have.length(0);
   });
 
-  it('should fetch the builds on mount', () => {
+  it('should fetch the build logs on mount', () => {
     const spy = sinon.spy();
 
     props.actions = { fetchBuildLogs: spy };