@@ -105,4 +105,4 @@
overflow: auto;
box-shadow: inset 0 0 3px gray;
}
-
\ No newline at end of file
+
diff --git a/api/models/build.js b/api/models/build.js
index a61c5d83a..6b3967d13 100644
--- a/api/models/build.js
+++ b/api/models/build.js
@@ -133,6 +133,12 @@ const beforeValidate = (build) => {
const sanitizeCompleteJobErrorMessage = (message) =>
message.replace(/\/\/(.*)@github/g, '//[token_redacted]@github');
+async function beforeCreate(build) {
+ const { Site, SiteBranchConfig, Domain } = this.sequelize.models;
+ const site = await Site.findByPk(build.site, { include: [SiteBranchConfig, Domain] });
+ build.url = buildUrl(build, site);
+}
+
const jobErrorMessage = (message = 'An unknown error occurred') =>
sanitizeCompleteJobErrorMessage(message);
@@ -153,10 +159,6 @@ const jobStateUpdate = (buildStatus, build, site, timestamp) => {
atts.completedAt = timestamp;
}
- if (buildStatus.status === States.Success) {
- atts.url = buildUrl(build, site);
- }
-
if (build.canStart(buildStatus.status)) {
atts.startedAt = timestamp;
}
@@ -318,6 +320,7 @@ module.exports = (sequelize, DataTypes) => {
tableName: 'build',
hooks: {
beforeValidate,
+ beforeCreate,
},
paranoid: true,
},
diff --git a/api/serializers/build.js b/api/serializers/build.js
index 653e0fc5f..4ba8e18ea 100644
--- a/api/serializers/build.js
+++ b/api/serializers/build.js
@@ -7,7 +7,6 @@ const {
BuildTask,
BuildTaskType,
} = require('../models');
-const { buildViewLink } = require('../utils/build');
const siteSerializer = require('./site');
const userSerializer = require('./user');
@@ -25,17 +24,12 @@ const toJSON = (build) => {
object.startedAt = object.startedAt.toISOString();
}
- if (build.Site) {
- object.viewLink = buildViewLink(build, build.Site);
- }
-
Object.keys(object).forEach((key) => {
if (object[key] === null) {
delete object[key];
}
});
delete object.token;
- delete object.url;
delete object.logsS3Key;
// only return first 80 chars in case it's long
if (object.error) {
diff --git a/api/services/GithubBuildHelper.js b/api/services/GithubBuildHelper.js
index 3dc1ec8ea..98b2b96a0 100644
--- a/api/services/GithubBuildHelper.js
+++ b/api/services/GithubBuildHelper.js
@@ -1,7 +1,6 @@
const url = require('url');
const GitHub = require('./GitHub');
const config = require('../../config');
-const { buildViewLink } = require('../utils/build');
// Loops through supplied list of users, until it
// finds a user with a valid access token
@@ -109,7 +108,7 @@ const reportBuildStatus = async (build) => {
options.description = 'The build is running.';
} else if (build.state === 'success') {
options.state = 'success';
- options.target_url = buildViewLink(build, site);
+ options.target_url = build.url;
options.description = 'The build is complete!';
} else if (build.state === 'error') {
options.state = 'error';
diff --git a/api/services/SiteBuildQueue.js b/api/services/SiteBuildQueue.js
index dfe95bf5c..85534d5c1 100644
--- a/api/services/SiteBuildQueue.js
+++ b/api/services/SiteBuildQueue.js
@@ -9,7 +9,7 @@ const {
} = require('../models');
const config = require('../../config');
const CloudFoundryAPIClient = require('../utils/cfApiClient');
-const { buildViewLink, buildUrl } = require('../utils/build');
+const { sitePrefix, buildUrl } = require('../utils/build');
const GithubBuildHelper = require('./GithubBuildHelper');
const S3Helper = require('./S3Helper');
@@ -24,13 +24,8 @@ const siteConfig = (build, siteBranchConfigs = []) => {
return configRecord?.config || {};
};
-const baseURLForDomain = (rawDomain) => url.parse(rawDomain).path.replace(/(\/)+$/, '');
-
-const sitePrefixForBuild = (rawDomain) =>
- baseURLForDomain(rawDomain).replace(/^(\/)+/, '');
-
const baseURLForBuild = (build) => {
- const link = buildViewLink(build, build.Site);
+ const link = buildUrl(build, build.Site);
const urlObject = new URL(link);
return urlObject.pathname.replace(/(\/)+$/, '');
};
@@ -61,7 +56,7 @@ const generateDefaultCredentials = async (build) => {
CONFIG: JSON.stringify(siteConfig(build, SiteBranchConfigs)),
REPOSITORY: repository,
OWNER: owner,
- SITE_PREFIX: sitePrefixForBuild(buildUrl(build, build.Site)),
+ SITE_PREFIX: sitePrefix(build, build.Site),
GITHUB_TOKEN: (build.User || {}).githubAccessToken, // temp hot-fix
GENERATOR: engine,
BUILD_ID: build.id,
diff --git a/api/utils/build.js b/api/utils/build.js
index 97dbbce41..8607b260d 100644
--- a/api/utils/build.js
+++ b/api/utils/build.js
@@ -1,40 +1,29 @@
const config = require('../../config');
-function buildPath(build, site) {
- const sbc = site?.SiteBranchConfigs?.find((c) => c.branch === build.branch);
-
- if (sbc) {
- return sbc.s3Key;
- }
-
+function previewPath(build, site) {
return `/preview/${site.owner}/${site.repository}/${build.branch}`;
}
-function buildUrl(build, site) {
+function proxyUrl(path, site) {
const { proxyDomain } = config.app;
- const path = buildPath(build, site);
const url = new URL(path, `https://${site.awsBucketName}.${proxyDomain}`);
-
- return url.href;
+ // use a trailing slash to prevent the browser from interpreting
+ // branch names with dots as file extensions
+ return url.href + '/';
}
-function buildViewLink(build, site) {
+/* canonical url for a build:
+ - if we don't have a matching site branch config, use the preview url
+ - if we don't have a matching domain, use the site branch config s3key path
+ - use the custom domain
+*/
+function buildUrl(build, site) {
const { SiteBranchConfigs, Domains } = site;
- if (build.url) {
- const regex = /(http|https):\/\/+/;
-
- if (regex.test(build.url)) {
- return `${build.url.replace(regex, 'https://')}/`;
- }
-
- return `https://${build.url}/`;
- }
-
const siteBranchConfig = SiteBranchConfigs.find((sbc) => sbc.branch === build.branch);
if (!siteBranchConfig) {
- return `${buildUrl(build, site)}/`;
+ return proxyUrl(previewPath(build, site), site);
}
const domain = Domains.find(
@@ -42,7 +31,7 @@ function buildViewLink(build, site) {
);
if (!domain) {
- return `${buildUrl(build, site)}/`;
+ return proxyUrl(siteBranchConfig.s3Key, site);
}
const domainName = domain.names.split(',')[0];
@@ -50,7 +39,18 @@ function buildViewLink(build, site) {
return `https://${domainName.replace(/\/+$/, '')}/`;
}
-module.exports = {
- buildViewLink,
- buildUrl,
-};
+/* generate SITE_PREFIX for builds
+ uses similar but slightly different logic to buildUrl
+(ignores domains, no proxyUrl, no leading slashes)
+*/
+function sitePrefix(build, site) {
+ const { SiteBranchConfigs } = site;
+
+ const siteBranchConfig = SiteBranchConfigs.find((sbc) => sbc.branch === build.branch);
+
+ const path = siteBranchConfig ? siteBranchConfig.s3Key : previewPath(build, site);
+
+ return path.replace(/^(\/)+/, '');
+}
+
+module.exports = { buildUrl, sitePrefix };
diff --git a/api/workers/jobProcessors/buildTaskRunner.js b/api/workers/jobProcessors/buildTaskRunner.js
index d3b1848de..4ce3e4abf 100644
--- a/api/workers/jobProcessors/buildTaskRunner.js
+++ b/api/workers/jobProcessors/buildTaskRunner.js
@@ -21,28 +21,12 @@ async function buildTaskRunner(job, { sleepNumber = 15000, totalAttempts = 240 }
let cfResponse;
- // TODO: rewrite; some tasks rely on scanning the final build url:
- // for non-production sites, this is buildTaskId.Build.url
- // for production sites, we overwrite that value with the production domain
- const siteBranchConfig = site.SiteBranchConfigs.find(
- (sbc) => sbc.branch === buildTask.Build.branch,
- );
-
if (taskTypeRunner === BuildTaskType.Runners.Cf_task) {
try {
logger.log(
`Starting ${taskTypeRunner} for ${owner}/${repository} on branch ${branch}`,
);
-
- const rawTask = buildTask.get({
- plain: true,
- });
-
- if (siteBranchConfig?.Domains?.length) {
- const domain = siteBranchConfig.Domains[0]; // TODO: always the first domain
- rawTask.Build.url = `https://${domain.names.split(',')[0]}`;
- }
-
+ const rawTask = buildTask.get({ plain: true });
cfResponse = await apiClient.startBuildTask(rawTask, { data }, { sleepInterval });
if (cfResponse.state === 'FAILED') {
@@ -76,9 +60,7 @@ async function buildTaskRunner(job, { sleepNumber = 15000, totalAttempts = 240 }
BuildTask.Statuses.Created,
].includes(failedTask.status)
) {
- await failedTask.update({
- status: BuildTask.Statuses.Error,
- });
+ await failedTask.update({ status: BuildTask.Statuses.Error });
}
}
@@ -103,10 +85,7 @@ async function buildTaskRunner(job, { sleepNumber = 15000, totalAttempts = 240 }
} catch (err) {
logger.log(`An error occured: ${err?.message}`);
const errorTask = await BuildTask.findByPk(buildTaskId);
- await errorTask.update({
- status: BuildTask.Statuses.Error,
- message: err?.message,
- });
+ await errorTask.update({ status: BuildTask.Statuses.Error, message: err?.message });
throw err;
}
}
diff --git a/frontend/pages/sites/$siteId/builds/Build.jsx b/frontend/pages/sites/$siteId/builds/Build.jsx
index 4d3feadff..8e7e17a8a 100644
--- a/frontend/pages/sites/$siteId/builds/Build.jsx
+++ b/frontend/pages/sites/$siteId/builds/Build.jsx
@@ -8,7 +8,7 @@ import buildActions from '@actions/buildActions';
import GithubBuildBranchLink from '@shared/GithubBuildBranchLink';
import GithubBuildShaLink from '@shared/GithubBuildShaLink';
-import BranchViewLink from '@shared/BranchViewLink';
+import { IconView } from '@shared/icons';
import {
IconCheckCircle,
IconClock,
@@ -207,12 +207,15 @@ const Build = ({ build, latestForBranch, showBuildTasks = false, site }) => {
|
{latestForBranch && build.state === 'success' && (
-
+
+
+ View site preview
+
)}
{latestForBranch && ['error', 'success'].includes(build.state) && (
diff --git a/frontend/pages/sites/$siteId/published/BranchRow.jsx b/frontend/pages/sites/$siteId/published/BranchRow.jsx
index b8dce47ee..7004ea8db 100644
--- a/frontend/pages/sites/$siteId/published/BranchRow.jsx
+++ b/frontend/pages/sites/$siteId/published/BranchRow.jsx
@@ -2,7 +2,7 @@ import React from 'react';
import PropTypes from 'prop-types';
import { Link } from 'react-router-dom';
-import BranchViewLink from '@shared/BranchViewLink';
+import BranchViewLink from './BranchViewLink';
import { SITE } from '@propTypes';
diff --git a/frontend/shared/BranchViewLink.jsx b/frontend/pages/sites/$siteId/published/BranchViewLink.jsx
similarity index 73%
rename from frontend/shared/BranchViewLink.jsx
rename to frontend/pages/sites/$siteId/published/BranchViewLink.jsx
index 28a07580f..8ff80b3f7 100644
--- a/frontend/shared/BranchViewLink.jsx
+++ b/frontend/pages/sites/$siteId/published/BranchViewLink.jsx
@@ -1,9 +1,7 @@
import React from 'react';
import PropTypes from 'prop-types';
-import { connect } from 'react-redux';
-import { SITE } from '../propTypes';
-import { IconView } from './icons';
-import globals from '../globals';
+import { SITE } from '@propTypes';
+import globals from '@globals';
// Based on the site's related site branch configs and domains
// This function determines the link for a built branch
@@ -32,17 +30,11 @@ const getViewLink = (branch, site) => {
return `https://${domain.names.split(',')[0]}`;
};
-export const BranchViewLink = ({ branchName, site, showIcon }) => {
+export const BranchViewLink = ({ branchName, site }) => {
const domain = getViewLink(branchName, site);
return (
-
- {showIcon && }
+
View site preview
);
@@ -51,11 +43,5 @@ export const BranchViewLink = ({ branchName, site, showIcon }) => {
BranchViewLink.propTypes = {
branchName: PropTypes.string.isRequired,
site: SITE.isRequired,
- showIcon: PropTypes.bool,
};
-
-BranchViewLink.defaultProps = {
- showIcon: false,
-};
-
-export default connect()(BranchViewLink);
+export default BranchViewLink;
diff --git a/test/api/unit/models/build.test.js b/test/api/unit/models/build.test.js
index 1f7aa94d8..91a2f89f2 100644
--- a/test/api/unit/models/build.test.js
+++ b/test/api/unit/models/build.test.js
@@ -296,7 +296,7 @@ describe('Build model', () => {
expect(build.completedAt.getTime()).to.eql(site.publishedAt.getTime());
const url = [
`https://${site.awsBucketName}.${config.app.proxyDomain}`,
- `/preview/${site.owner}/${site.repository}/${build.branch}`,
+ `/preview/${site.owner}/${site.repository}/${build.branch}/`,
].join('');
expect(build.url).to.eql(url);
});
diff --git a/test/api/unit/services/GithubBuildHelper.test.js b/test/api/unit/services/GithubBuildHelper.test.js
index 7ab6e679f..e74ac196e 100644
--- a/test/api/unit/services/GithubBuildHelper.test.js
+++ b/test/api/unit/services/GithubBuildHelper.test.js
@@ -5,7 +5,7 @@ const config = require('../../../../config');
const { Domain, Site, SiteBranchConfig, User } = require('../../../../api/models');
const factory = require('../../support/factory');
const githubAPINocks = require('../../support/githubAPINocks');
-const { buildViewLink } = require('../../../../api/utils/build');
+const { buildUrl } = require('../../../../api/utils/build');
const GitHub = require('../../../../api/services/GitHub');
const GithubBuildHelper = require('../../../../api/services/GithubBuildHelper');
@@ -770,6 +770,14 @@ describe('GithubBuildHelper', () => {
await build.update({
branch: 'preview-branch',
});
+
+ await build.reload();
+
+ // this depends on branch so we update twice
+ await build.update({
+ url: buildUrl(build, site),
+ });
+
const repoNock = githubAPINocks.repo({
accessToken: user.githubAccessToken,
owner: site.owner,
@@ -780,7 +788,7 @@ describe('GithubBuildHelper', () => {
owner: site.owner,
repo: site.repository,
sha: clonedCommitSha,
- targetURL: buildViewLink(build, site),
+ targetURL: buildUrl(build, site),
});
await build.reload({
@@ -791,6 +799,7 @@ describe('GithubBuildHelper', () => {
},
],
});
+
await GithubBuildHelper.reportBuildStatus(build);
expect(repoNock.isDone()).to.be.true;
expect(statusNock.isDone()).to.be.true;
diff --git a/test/api/unit/utils/build.test.js b/test/api/unit/utils/build.test.js
index de78ca888..712962b7a 100644
--- a/test/api/unit/utils/build.test.js
+++ b/test/api/unit/utils/build.test.js
@@ -1,95 +1,12 @@
const expect = require('chai').expect;
const { Domain, Site, SiteBranchConfig } = require('../../../../api/models');
-const { buildViewLink, buildUrl } = require('../../../../api/utils/build');
+const { buildUrl } = require('../../../../api/utils/build');
const factory = require('../../support/factory');
const config = require('../../../../config');
const { proxyDomain } = config.app;
describe('build utils', () => {
- describe('buildUrl', () => {
- let site;
- before(async () => {
- const { id } = await factory.site({
- defaultBranch: 'main',
- demoBranch: 'staging',
- });
- site = await Site.findByPk(id, {
- include: [SiteBranchConfig, Domain],
- });
- });
-
- it('default branch url start with site', async () => {
- let build = await factory.build({
- branch: site.defaultBranch,
- site,
- });
- const url = [
- `https://${site.awsBucketName}.${proxyDomain}`,
- `/site/${site.owner}/${site.repository}`,
- ].join('');
- expect(buildUrl(build, site)).to.eql(url);
- });
-
- it('demo branch url start with demo', async () => {
- const build = await factory.build({
- branch: site.demoBranch,
- site,
- });
- const url = [
- `https://${site.awsBucketName}.${proxyDomain}`,
- `/demo/${site.owner}/${site.repository}`,
- ].join('');
- expect(buildUrl(build, site)).to.eql(url);
- });
-
- it('non-default/demo branch url start with preview', async () => {
- const build = await factory.build({
- branch: 'other',
- site,
- });
- const url = [
- `https://${site.awsBucketName}.${proxyDomain}`,
- `/preview/${site.owner}/${site.repository}/other`,
- ].join('');
- expect(buildUrl(build, site)).to.eql(url);
- });
- });
-
- describe('buildUrl with other site branch context and s3Key', () => {
- let site;
- const branch = 'other-branch';
- const context = 'other';
- const s3Key = 'test/other';
-
- before(async () => {
- const interimSite = await factory.site(
- {},
- {
- noSiteBranchConfig: true,
- },
- );
- await SiteBranchConfig.create({
- siteId: interimSite.id,
- context,
- branch,
- s3Key,
- });
- site = await Site.findByPk(interimSite.id, {
- include: [SiteBranchConfig],
- });
- });
-
- it('branch url to have other s3Key', async () => {
- let build = await factory.build({
- branch,
- site,
- });
- const url = [`https://${site.awsBucketName}.${proxyDomain}`, `/${s3Key}`].join('');
- expect(buildUrl(build, site)).to.eql(url);
- });
- });
-
describe('viewLink', () => {
let site;
const defaultBranch = 'main';
@@ -106,7 +23,7 @@ describe('build utils', () => {
});
site = await Site.findByPk(id, {
- include: [SiteBranchConfig],
+ include: [SiteBranchConfig, Domain],
});
});
@@ -123,7 +40,7 @@ describe('build utils', () => {
]),
);
- it('default branch url start with site', async () => {
+ it('domained sites have the domain', async () => {
const siteUrl = new URL(domain);
const sbc = site.SiteBranchConfigs.find((c) => c.branch === defaultBranch);
await factory.domain.create({
@@ -139,10 +56,10 @@ describe('build utils', () => {
const updatedSite = await Site.findByPk(site.id, {
include: [Domain, SiteBranchConfig],
});
- expect(buildViewLink(build, updatedSite)).to.eql('https://www.main.com/');
+ expect(buildUrl(build, updatedSite)).to.eql('https://www.main.com/');
});
- it('should show preview url when site domain is not provisioned', async () => {
+ it('should show site url when site domain is not provisioned', async () => {
const siteUrl = new URL(domain);
const sbc = site.SiteBranchConfigs.find((c) => c.branch === defaultBranch);
await factory.domain.create({
@@ -158,47 +75,28 @@ describe('build utils', () => {
const updatedSite = await Site.findByPk(site.id, {
include: [Domain, SiteBranchConfig],
});
- expect(buildViewLink(build, updatedSite)).to.eql(
- `https://${site.awsBucketName}.${proxyDomain}/site/${site.owner}/${site.repository}/`,
+
+ const host = `${site.awsBucketName}.${proxyDomain}`;
+
+ expect(buildUrl(build, updatedSite)).to.eql(
+ `https://${host}/site/${site.owner}/${site.repository}/`,
);
+
+ expect(buildUrl(build, updatedSite)).to.eql(`https://${host}${sbc.s3Key}/`);
});
- it('demo branch url start with demo', async () => {
- const siteUrl = new URL(demoDomain);
- const sbc = site.SiteBranchConfigs.find((c) => c.branch === demoBranch);
- await factory.domain.create({
- siteId: site.id,
- siteBranchConfigId: sbc.id,
- names: siteUrl.host,
- state: 'provisioned',
- });
- const updatedSite = await Site.findByPk(site.id, {
- include: [Domain, SiteBranchConfig],
- });
+ it('should show preview url without a branch configuration', async () => {
+ const branch = 'anotherbranch';
const build = await factory.build({
- branch: site.demoBranch,
+ branch,
site,
});
- expect(buildViewLink(build, updatedSite)).to.eql(`${demoDomain}/`);
- });
-
- describe('should return configured domain', () => {
- it('build.url does not exist', async () => {
- const build = await factory.build({
- branch: 'other',
- site,
- });
- expect(buildViewLink(build, site)).to.equal(`${buildUrl(build, site)}/`);
- });
-
- it('build.url does exist', async () => {
- const build = await factory.build({
- branch: 'other',
- site,
- url: 'https://the.url',
- });
- expect(buildViewLink(build, site)).to.equal(`${build.url}/`);
+ const updatedSite = await Site.findByPk(site.id, {
+ include: [Domain, SiteBranchConfig],
});
+ expect(buildUrl(build, updatedSite)).to.eql(
+ `https://${site.awsBucketName}.${proxyDomain}/preview/${site.owner}/${site.repository}/${branch}/`,
+ );
});
});
});
diff --git a/test/api/workers/jobProcessors/buildTaskRunner.test.js b/test/api/workers/jobProcessors/buildTaskRunner.test.js
index f3aac1129..137db7095 100644
--- a/test/api/workers/jobProcessors/buildTaskRunner.test.js
+++ b/test/api/workers/jobProcessors/buildTaskRunner.test.js
@@ -250,7 +250,7 @@ describe('buildTaskRunner', () => {
branch: siteBranchConfig.branch,
});
const task = await factory.buildTask({ build });
- const domain = await factory.domain.create({
+ await factory.domain.create({
siteId: site.id,
siteBranchConfigId: siteBranchConfig.id,
state: 'provisioned',
@@ -259,7 +259,6 @@ describe('buildTaskRunner', () => {
const rawTask = taskWithIncludes.get({
plain: true,
});
- rawTask.Build.url = `https://${domain.names.split(',')[0]}`;
sinon.stub(BuildTaskQueue, 'setupTaskEnv').resolves({
buildTask: taskWithIncludes,
...jobData,
|