Skip to content

Commit 8facede

Browse files
svenaassvenaas
committed
fix: Add message explaining that build logs are deleted after 180 days. (#4379)
1 parent 352c9b3 commit 8facede

File tree

7 files changed

+147
-85
lines changed

7 files changed

+147
-85
lines changed

‎frontend/components/site/CommitSummary.jsx‎

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
1-
import React, { useState, useEffect } from 'react';
1+
import React from 'react';
22
import PropTypes from 'prop-types';
33

4-
import api from '../../util/federalistApi';
54
import { IconBranch } from '../icons';
65
import LoadingIndicator from '../LoadingIndicator';
76
import { timeFrom, dateAndTime } from '../../util/datetime';
@@ -27,23 +26,8 @@ function buildShaLink(owner, repo, sha) {
2726
);
2827
}
2928

30-
function CommitSummary({ buildId }) {
31-
const [build, setBuild] = useState({
32-
isLoading: true,
33-
buildDetails: null,
34-
});
35-
const { isLoading, buildDetails } = build;
36-
37-
useEffect(() => {
38-
if (!buildDetails) {
39-
api.fetchBuild(buildId).then(data => setBuild({
40-
isLoading: false,
41-
buildDetails: data,
42-
}));
43-
}
44-
}, [buildDetails]);
45-
46-
if (isLoading) {
29+
function CommitSummary({ buildDetails }) {
30+
if (!buildDetails) {
4731
return <LoadingIndicator size="mini" text="Getting commit details..." />;
4832
}
4933

@@ -74,7 +58,20 @@ function CommitSummary({ buildId }) {
7458
);
7559
}
7660
CommitSummary.propTypes = {
77-
buildId: PropTypes.number.isRequired,
61+
buildDetails: PropTypes.shape({
62+
site: PropTypes.shape({
63+
owner: PropTypes.string.isRequired,
64+
repository: PropTypes.string.isRequired,
65+
}).isRequired,
66+
branch: PropTypes.string.isRequired,
67+
username: PropTypes.string.isRequired,
68+
clonedCommitSha: PropTypes.string.isRequired,
69+
createdAt: PropTypes.instanceOf(Date).isRequired,
70+
}),
71+
};
72+
73+
CommitSummary.defaultProps = {
74+
buildDetails: null,
7875
};
7976

8077
export { CommitSummary };

‎frontend/components/site/siteBuildLogs.jsx‎

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,42 @@
33
import React from 'react';
44
import { Link, useParams } from 'react-router-dom';
55

6-
import { useBuildLogs } from '../../hooks';
6+
import { useBuildLogs, useBuildDetails } from '../../hooks';
7+
import LoadingIndicator from '../LoadingIndicator';
78
import SiteBuildLogTable from './siteBuildLogTable';
89
import DownloadBuildLogsButton from './downloadBuildLogsButton';
910
import CommitSummary from './CommitSummary';
1011

1112
export const REFRESH_INTERVAL = 15 * 1000;
13+
const BUILD_LOG_RETENTION_LIMIT = 180 * 24 * 60 * 60 * 1000; // 180 days in milliseconds
14+
15+
function buildTooOld(build) {
16+
return (new Date() - new Date(build.completedAt)) > BUILD_LOG_RETENTION_LIMIT;
17+
}
18+
19+
function getSiteBuildLogTable(buildDetails, logs, state) {
20+
if (logs && logs?.length > 0) {
21+
return <SiteBuildLogTable buildLogs={logs} buildState={state} />;
22+
}
23+
if (buildTooOld(buildDetails)) {
24+
return <SiteBuildLogTable buildLogs={['Builds more than 180 days old are deleted according to platform policy.']} />;
25+
}
26+
return <SiteBuildLogTable buildLogs={['This build does not have any build logs.']} />;
27+
}
1228

1329
const SiteBuildLogs = () => {
1430
const { buildId: buildIdStr } = useParams();
1531
const buildId = parseInt(buildIdStr, 10);
1632
const { logs, state } = useBuildLogs(buildId);
33+
const { buildDetails, isLoading } = useBuildDetails(buildId);
34+
35+
if (isLoading || !logs) {
36+
return <LoadingIndicator size="mini" text="Getting build log details..." />;
37+
}
1738

1839
return (
1940
<div>
20-
<CommitSummary buildId={buildId} />
41+
<CommitSummary buildDetails={buildDetails} />
2142
<div className="log-tools">
2243
<ul className="usa-unstyled-list">
2344
{(process.env.FEATURE_BUILD_TASKS === 'active') && (
@@ -26,14 +47,7 @@ const SiteBuildLogs = () => {
2647
<li><DownloadBuildLogsButton buildId={buildId} buildLogsData={logs} /></li>
2748
</ul>
2849
</div>
29-
{(!logs || logs?.length === 0) && (
30-
<div>
31-
<SiteBuildLogTable buildLogs={['This build does not have any build logs.']} />
32-
</div>
33-
)}
34-
{(logs && logs?.length > 0) && (
35-
<SiteBuildLogTable buildLogs={logs} buildState={state} />
36-
)}
50+
{ getSiteBuildLogTable(buildDetails, logs, state) }
3751
</div>
3852
);
3953
};

‎frontend/components/site/siteBuildTasks.jsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import prettyBytes from 'pretty-bytes';
77
import {
88
IconCheckCircle, IconClock, IconExclamationCircle, IconSpinner,
99
} from '../icons';
10+
import { useBuildDetails } from '../../hooks';
1011
import CommitSummary from './CommitSummary';
1112
import api from '../../util/federalistApi';
1213

@@ -55,6 +56,8 @@ const SiteBuildTasks = () => {
5556
const buildId = parseInt(buildIdStr, 10);
5657
const [buildTasks, setBuildTasks] = useState([]);
5758

59+
const { buildDetails } = useBuildDetails(buildId);
60+
5861
let intervalHandle;
5962
useEffect(() => {
6063
function fetchTasks(thisBuildId) {
@@ -76,7 +79,7 @@ const SiteBuildTasks = () => {
7679

7780
return (
7881
<div>
79-
<CommitSummary buildId={buildId} />
82+
<CommitSummary buildDetails={buildDetails} />
8083
<div className="log-tools">
8184
<ul className="usa-unstyled-list">
8285
<li>

‎frontend/hooks/index.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
export * from './useBuildDetails';
12
export * from './useBuildLogs';
23
export * from './useSiteBranchConfigs';
34
export * from './useSiteDomains';

‎frontend/hooks/useBuildDetails.js‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/* eslint-disable import/prefer-default-export */
2+
import { useEffect, useState } from 'react';
3+
import api from '../util/federalistApi';
4+
5+
const initResultsState = {
6+
buildDetails: null,
7+
isLoading: true,
8+
};
9+
10+
export const useBuildDetails = (id) => {
11+
const [results, setResults] = useState(initResultsState);
12+
13+
useEffect(() => {
14+
if (!results.buildDetails) {
15+
api.fetchBuild(id).then(data => setResults({
16+
isLoading: false,
17+
buildDetails: data,
18+
}));
19+
}
20+
}, [results]);
21+
22+
return results;
23+
};

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

Lines changed: 30 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,79 +1,58 @@
11
import React from 'react';
22
import { expect, assert } from 'chai';
33
import proxyquire from 'proxyquire';
4-
import sinon from 'sinon';
54
import { mount } from 'enzyme';
65

7-
import api from '../../../../frontend/util/federalistApi';
86
import LoadingIndicator from '../../../../frontend/components/LoadingIndicator';
97

10-
proxyquire.noCallThru();
11-
12-
const fetchBuildMock = sinon.stub();
13-
const buildActions = {
14-
fetchBuild: fetchBuildMock,
15-
};
168
const CommitSummary = proxyquire(
179
'../../../../frontend/components/site/CommitSummary',
1810
{
1911
'../icons': {
2012
IconBranch: () => <span />,
2113
},
22-
'../../actions/buildActions': buildActions,
2314
}
2415
).default;
2516

2617
const defaultProps = {
27-
buildId: 1,
18+
buildDetails: {
19+
site: {
20+
owner: 'user',
21+
repository: 'repo',
22+
},
23+
branch: 'branch',
24+
username: 'username',
25+
clonedCommitSha: 'sha4567890abcdef',
26+
createdAt: new Date(),
27+
}
2828
};
2929

3030
describe('<CommitSummary />', () => {
31-
afterEach(() => {
32-
sinon.restore();
33-
});
34-
3531
it('should exist', () => {
3632
assert.isDefined(CommitSummary);
3733
});
3834

39-
it('renders a loading state whie loading', () => {
40-
const stub = sinon.stub(api, 'fetchBuild');
41-
stub.resolves([]);
42-
43-
const wrapper = mount(<CommitSummary {...defaultProps} />);
35+
it('renders a loading state while loading', () => {
36+
const wrapper = mount(<CommitSummary { ...{ buildDetails: null } } />);
4437
expect(wrapper.find(LoadingIndicator)).to.have.length(1);
4538
});
4639

47-
// no useEffect in tests
48-
// it('requests build information once on load', () => {
49-
// const wrapper = mountStore(<CommitSummary {...defaultProps} />, defaultState);
50-
// const buildId = 1;
51-
// expect(fetchBuildMock.callCount).to.be.greaterThanOrEqual(1);
52-
// fetchBuildMock.resetHistory();
53-
// sinon.restore();
54-
// });
55-
56-
// describe('after load', () => {
57-
// let wrapper;
58-
// let loadedState = lodashClonedeep(defaultState);
59-
// loadedState.build = {
60-
// isLoading: false,
61-
// data: { ...defaultBuildData }
62-
// };
63-
64-
// it('renders the branch and github user name for the commit', () => {
65-
// wrapper = mountStore(<CommitSummary {...defaultProps} />, loadedState);
66-
// expect(wrapper.find('.commit-branch')).to.have.length(1);
67-
// expect(wrapper.find('.commit-branch').text()).to.contain(defaultBuildData.branch);
68-
// expect(wrapper.find('.commit-username')).to.have.length(1);
69-
// expect(wrapper.find('.commit-username').text()).to.equal(defaultBuildData.username);
70-
// });
71-
72-
// it('formats a sha link correctly and limits to first 7 chars', () => {
73-
// wrapper = mountStore(<CommitSummary {...defaultProps} />, loadedState);
74-
// expect(wrapper.find('.sha-link')).to.have.length(1);
75-
// expect(defaultBuildData.clonedCommitSha).to.contain(wrapper.find('.sha-link').text());
76-
// expect(wrapper.find('.sha-link').text()).to.have.length(7);
77-
// });
78-
// });
40+
describe('after load', () => {
41+
const build = defaultProps.buildDetails;
42+
43+
it('renders the branch and github user name for the commit', () => {
44+
const wrapper = mount(<CommitSummary {...defaultProps} />);
45+
expect(wrapper.find('.commit-branch')).to.have.length(1);
46+
expect(wrapper.find('.commit-branch').text()).to.contain(build.branch);
47+
expect(wrapper.find('.commit-username')).to.have.length(1);
48+
expect(wrapper.find('.commit-username').text()).to.equal(build.username);
49+
});
50+
51+
it('formats a sha link correctly and limits to first 7 chars', () => {
52+
const wrapper = mount(<CommitSummary {...defaultProps} />);
53+
expect(wrapper.find('.sha-link')).to.have.length(1);
54+
expect(build.clonedCommitSha).to.contain(wrapper.find('.sha-link').text());
55+
expect(wrapper.find('.sha-link').text()).to.have.length(7);
56+
});
57+
});
7958
});

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

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,50 @@
11
import React from 'react';
2-
import { expect } from 'chai';
2+
import { expect, assert } from 'chai';
33
import { shallow } from 'enzyme';
44
import sinon from 'sinon';
55

6+
import api from '../../../../frontend/util/federalistApi';
67
import { SiteBuildLogs } from '../../../../frontend/components/site/siteBuildLogs';
78
import LoadingIndicator from '../../../../frontend/components/LoadingIndicator';
89

910
let props;
1011

12+
// Initial work for when the containing .skip can be removed
13+
const defaultBuild = {
14+
site: {
15+
owner: 'user',
16+
repository: 'repo',
17+
},
18+
branch: 'branch',
19+
username: 'username',
20+
clonedCommitSha: 'sha4567890abcdef',
21+
createdAt: new Date(),
22+
};
23+
1124
describe.skip('<SiteBuildLogs/>', () => {
1225
beforeEach(() => {
1326
props = {
14-
buildId: '123'
27+
buildId: '123',
28+
buildDetails: defaultBuild,
29+
isLoading: false,
30+
logs: [],
1531
};
1632
});
1733

34+
afterEach(() => {
35+
sinon.restore();
36+
});
37+
38+
// For when the containing .skip can be removed
39+
it('should exist', () => {
40+
assert.isDefined(SiteBuildLogs);
41+
});
42+
43+
// Placeholder for when the containing .skip can be removed
44+
it('requests build information once on load', () => {
45+
// Verify that fetchBuild is called once
46+
});
47+
1848
it('should render a button to download logs if builds exist', () => {
1949
const wrapper = shallow(<SiteBuildLogs {...props} />);
2050
expect(wrapper.find('SiteBuildLogTable')).to.have.length(1);
@@ -32,6 +62,21 @@ describe.skip('<SiteBuildLogs/>', () => {
3262
expect(wrapper.find(LoadingIndicator)).to.have.length(1);
3363
});
3464

65+
// Placeholder for when the containing .skip can be removed
66+
it('should render an explanatory message if the build is over 180 days old', () => {
67+
const stubBuild = { completedAt: new Date(Date.now() - (181 * 86400000)) }; // 181 days ago
68+
const stub = sinon.stub(api, 'fetchBuild');
69+
stub.resolves(stubBuild);
70+
71+
props.buildLogs.isLoading = false;
72+
props.buildLogs.data = [];
73+
74+
const wrapper = shallow(<SiteBuildLogs {...props} />);
75+
expect(wrapper.find('SiteBuildLogTable')).to.have.length(0);
76+
expect(wrapper.find('p').contains('Builds more than 180 days old are deleted according to platform policy.')).to.be.true;
77+
expect(wrapper.find('DownloadBuildLogsButton')).to.have.length(0);
78+
});
79+
3580
it('should render an empty state if there are no builds', () => {
3681
props.buildLogs.isLoading = false;
3782
props.buildLogs.data = [];
@@ -42,7 +87,7 @@ describe.skip('<SiteBuildLogs/>', () => {
4287
expect(wrapper.find('DownloadBuildLogsButton')).to.have.length(0);
4388
});
4489

45-
it('should fetch the builds on mount', () => {
90+
it('should fetch the build logs on mount', () => {
4691
const spy = sinon.spy();
4792

4893
props.actions = { fetchBuildLogs: spy };

0 commit comments

Comments
 (0)