Skip to content

Commit 277239d

Browse files
luis-almeidaluis-almeida
committed
fix: stop sending local preview URLs in themes:migrate payload
The migrate command has no bind/port flags, so getLocalServerBaseUrl was producing http://undefined:undefined/... for every file-type variable and asset URL sent to the migrations endpoint. Make flags optional in getVariables/getAssets and fall back to the basename when absent — the preview URLs are meaningless outside the preview server.
1 parent 2e4ec67 commit 277239d

7 files changed

Lines changed: 67 additions & 33 deletions

File tree

‎packages/zcli-themes/src/commands/themes/migrate.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ export default class Migrate extends Command {
1818
static strict = false
1919

2020
async run () {
21-
const { flags, argv: [themeDirectory] } = await this.parse(Migrate)
21+
const { argv: [themeDirectory] } = await this.parse(Migrate)
2222
const themePath = path.resolve(themeDirectory)
2323

24-
await migrate(themePath, flags)
24+
await migrate(themePath)
2525
}
2626
}

‎packages/zcli-themes/src/lib/getAssets.test.ts‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ describe('getAssets', () => {
2323
.withArgs('theme/path/assets')
2424
.returns(true)
2525

26-
readdirSyncStub.returns(['.gitkeep', 'foo.png', 'bar.png'] as any)
26+
readdirSyncStub.returns(['.gitkeep', 'foo.png', 'bar.png'] as unknown as fs.Dirent[])
2727

2828
const assets = getAssets('theme/path', flags)
2929

@@ -39,6 +39,30 @@ describe('getAssets', () => {
3939
])
4040
})
4141

42+
it('returns basenames as urls when flags are not provided', () => {
43+
const existsSyncStub = sinon.stub(fs, 'existsSync')
44+
const readdirSyncStub = sinon.stub(fs, 'readdirSync')
45+
46+
existsSyncStub
47+
.withArgs('theme/path/assets')
48+
.returns(true)
49+
50+
readdirSyncStub.returns(['foo.png', 'bar.png'] as unknown as fs.Dirent[])
51+
52+
const assets = getAssets('theme/path')
53+
54+
expect(assets).to.deep.equal([
55+
[
56+
{ base: 'foo.png', dir: '', ext: '.png', name: 'foo', root: '' },
57+
'foo.png'
58+
],
59+
[
60+
{ base: 'bar.png', dir: '', ext: '.png', name: 'bar', root: '' },
61+
'bar.png'
62+
]
63+
])
64+
})
65+
4266
it('throws an error when an asset has illegal characters in its name', () => {
4367
const existsSyncStub = sinon.stub(fs, 'existsSync')
4468
const readdirSyncStub = sinon.stub(fs, 'readdirSync')
@@ -47,7 +71,7 @@ describe('getAssets', () => {
4771
.withArgs('theme/path/assets')
4872
.returns(true)
4973

50-
readdirSyncStub.returns(['unsuported file name.png'] as any)
74+
readdirSyncStub.returns(['unsuported file name.png'] as unknown as fs.Dirent[])
5175

5276
expect(() => {
5377
getAssets('theme/path', flags)

‎packages/zcli-themes/src/lib/getAssets.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import * as fs from 'fs'
44
import * as path from 'path'
55
import { getLocalServerBaseUrl } from './getLocalServerBaseUrl'
66

7-
export default function getAssets (themePath: string, flags: Flags): [path.ParsedPath, string][] {
7+
export default function getAssets (themePath: string, flags?: Flags): [path.ParsedPath, string][] {
88
const assetsPath = `${themePath}/assets`
99
const filenames = fs.existsSync(assetsPath) ? fs.readdirSync(assetsPath) : []
1010
const assets: [path.ParsedPath, string][] = []
@@ -18,7 +18,8 @@ export default function getAssets (themePath: string, flags: Flags): [path.Parse
1818
)
1919
}
2020
if (!name.startsWith('.')) {
21-
assets.push([parsedPath, `${getLocalServerBaseUrl(flags)}/guide/assets/${filename}`])
21+
const url = flags ? `${getLocalServerBaseUrl(flags)}/guide/assets/${filename}` : filename
22+
assets.push([parsedPath, url])
2223
}
2324
})
2425

‎packages/zcli-themes/src/lib/getVariables.test.ts‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ describe('getVariables', () => {
3131
.withArgs('theme/path/settings')
3232
.returns(true)
3333

34-
readdirSyncStub.returns(['logo.png', 'favicon.png'] as any)
34+
readdirSyncStub.returns(['logo.png', 'favicon.png'] as unknown as fs.Dirent[])
3535

3636
expect(getVariables('theme/path', settings, flags)).to.deep.equal([
3737
{ identifier: 'color', type: 'color', value: '#999' },
@@ -40,6 +40,23 @@ describe('getVariables', () => {
4040
])
4141
})
4242

43+
it('uses the matched filename as the value when flags are not provided', () => {
44+
const existsSyncStub = sinon.stub(fs, 'existsSync')
45+
const readdirSyncStub = sinon.stub(fs, 'readdirSync')
46+
47+
existsSyncStub
48+
.withArgs('theme/path/settings')
49+
.returns(true)
50+
51+
readdirSyncStub.returns(['logo.png', 'favicon.png'] as unknown as fs.Dirent[])
52+
53+
expect(getVariables('theme/path', settings)).to.deep.equal([
54+
{ identifier: 'color', type: 'color', value: '#999' },
55+
{ identifier: 'logo', type: 'file', value: 'logo.png' },
56+
{ identifier: 'favicon', type: 'file', value: 'favicon.png' }
57+
])
58+
})
59+
4360
it('throws an error when it doesn\'t find an asset within the settings folder for a variable of type "file"', () => {
4461
const existsSyncStub = sinon.stub(fs, 'existsSync')
4562
const readdirSyncStub = sinon.stub(fs, 'readdirSync')
@@ -48,7 +65,7 @@ describe('getVariables', () => {
4865
.withArgs('theme/path/settings')
4966
.returns(true)
5067

51-
readdirSyncStub.returns(['logo.png'] as any)
68+
readdirSyncStub.returns(['logo.png'] as unknown as fs.Dirent[])
5269

5370
expect(() => {
5471
getVariables('theme/path', settings, flags)

‎packages/zcli-themes/src/lib/getVariables.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import * as path from 'path'
44
import { CLIError } from '@oclif/core/lib/errors'
55
import { getLocalServerBaseUrl } from './getLocalServerBaseUrl'
66

7-
export default function getVariables (themePath: string, settings: Setting[], flags: Flags): Variable[] {
7+
export default function getVariables (themePath: string, settings: Setting[], flags?: Flags): Variable[] {
88
const settingsPath = `${themePath}/settings`
99
const filenames = fs.existsSync(settingsPath) ? fs.readdirSync(settingsPath) : []
1010

@@ -18,7 +18,7 @@ export default function getVariables (themePath: string, settings: Setting[], fl
1818
`The setting "${variable.identifier}" of type "file" does not have a matching file within the "settings" folder`
1919
)
2020
}
21-
variable.value = file && `${getLocalServerBaseUrl(flags)}/guide/settings/${file}`
21+
variable.value = flags ? `${getLocalServerBaseUrl(flags)}/guide/settings/${file}` : file
2222
}
2323
return variable
2424
})

‎packages/zcli-themes/src/lib/migrate.test.ts‎

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,6 @@ const manifest = {
2727
]
2828
}
2929

30-
const flags = {
31-
bind: 'localhost',
32-
port: 1000,
33-
logs: true,
34-
livereload: false
35-
}
36-
3730
describe('migrate', () => {
3831
beforeEach(() => {
3932
sinon.restore()
@@ -56,16 +49,16 @@ describe('migrate', () => {
5649
'custom_pages/faq': '<h1>FAQ</h1>'
5750
})
5851

59-
getVariablesStub.withArgs('theme/path', manifest.settings, flags).returns([
52+
getVariablesStub.withArgs('theme/path', manifest.settings).returns([
6053
{ identifier: 'color', type: 'color', value: '#999' },
6154
{
6255
identifier: 'logo',
6356
type: 'file',
64-
value: 'http://localhost:1000/guide/settings/logo.png'
57+
value: 'logo.png'
6558
}
6659
])
6760

68-
getAssetsStub.withArgs('theme/path', flags).returns([
61+
getAssetsStub.withArgs('theme/path').returns([
6962
[
7063
{
7164
base: 'background.png',
@@ -74,7 +67,7 @@ describe('migrate', () => {
7467
name: 'background',
7568
root: ''
7669
},
77-
'http://localhost:1000/guide/assets/background.png'
70+
'background.png'
7871
]
7972
])
8073

@@ -98,7 +91,7 @@ describe('migrate', () => {
9891
}) as axios.AxiosPromise
9992
)
10093

101-
await migrate('theme/path', flags)
94+
await migrate('theme/path')
10295

10396
expect(
10497
requestStub.calledWith(
@@ -114,12 +107,11 @@ describe('migrate', () => {
114107
'article_pages/product_updates': '<h1>Product updates</h1>',
115108
'custom_pages/faq': '<h1>FAQ</h1>',
116109
assets: {
117-
'background.png':
118-
'http://localhost:1000/guide/assets/background.png'
110+
'background.png': 'background.png'
119111
},
120112
variables: {
121113
color: '#999',
122-
logo: 'http://localhost:1000/guide/settings/logo.png'
114+
logo: 'logo.png'
123115
},
124116
metadata: { api_version: 1 }
125117
}
@@ -178,7 +170,7 @@ describe('migrate', () => {
178170
const errorStub = sinon.stub(errors, 'error').callThrough()
179171

180172
try {
181-
await migrate('theme/path', flags)
173+
await migrate('theme/path')
182174
} catch {
183175
const [call] = errorStub.getCalls()
184176
const [error] = call.args
@@ -206,7 +198,7 @@ describe('migrate', () => {
206198
const errorStub = sinon.stub(errors, 'error').callThrough()
207199

208200
try {
209-
await migrate('theme/path', flags)
201+
await migrate('theme/path')
210202
} catch {
211203
const [call] = errorStub.getCalls()
212204
const [error] = call.args
@@ -230,7 +222,7 @@ describe('migrate', () => {
230222
const errorStub = sinon.stub(errors, 'error').callThrough()
231223

232224
try {
233-
await migrate('theme/path', flags)
225+
await migrate('theme/path')
234226
} catch {
235227
const [call] = errorStub.getCalls()
236228
const [error] = call.args
@@ -250,7 +242,7 @@ describe('migrate', () => {
250242
const errorStub = sinon.stub(errors, 'error').callThrough()
251243

252244
try {
253-
await migrate('theme/path', flags)
245+
await migrate('theme/path')
254246
} catch {
255247
const [call] = errorStub.getCalls()
256248
const [error] = call.args

‎packages/zcli-themes/src/lib/migrate.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { Flags, ValidationErrors } from '../types'
1+
import type { ValidationErrors } from '../types'
22
import getManifest from './getManifest'
33
import getTemplates from './getTemplates'
44
import getVariables from './getVariables'
@@ -14,11 +14,11 @@ import rewriteAssets from './rewriteAssets'
1414
import handleTemplateError from './handleTemplateError'
1515
import parseAxiosError from './parseAxiosError'
1616

17-
export default async function migrate (themePath: string, flags: Flags): Promise<string | void> {
17+
export default async function migrate (themePath: string): Promise<string | void> {
1818
const manifest = getManifest(themePath)
1919
const templates = getTemplates(themePath)
20-
const variables = getVariables(themePath, manifest.settings, flags)
21-
const assets = getAssets(themePath, flags)
20+
const variables = getVariables(themePath, manifest.settings)
21+
const assets = getAssets(themePath)
2222

2323
const variablesPayload = variables.reduce((payload, variable) => ({
2424
...payload,

0 commit comments

Comments
 (0)