From 36c47e5533800c795d13f19170bbc04754832ad4 Mon Sep 17 00:00:00 2001 From: Kanad Gupta Date: Fri, 18 Aug 2023 13:45:12 -0500 Subject: [PATCH 1/7] fix(versions): general refactors to handle flag edge cases --- __tests__/cmds/versions/create.test.ts | 47 ++++++- __tests__/cmds/versions/update.test.ts | 173 ++++++++++++++++++++++++- __tests__/lib/prompts.test.ts | 9 +- src/cmds/versions/create.ts | 22 ++-- src/cmds/versions/update.ts | 26 +++- src/lib/castStringOptToBool.ts | 19 +++ src/lib/prompts.ts | 30 +++-- 7 files changed, 291 insertions(+), 35 deletions(-) create mode 100644 src/lib/castStringOptToBool.ts diff --git a/__tests__/cmds/versions/create.test.ts b/__tests__/cmds/versions/create.test.ts index 879c03a26..c3346d440 100644 --- a/__tests__/cmds/versions/create.test.ts +++ b/__tests__/cmds/versions/create.test.ts @@ -52,7 +52,6 @@ describe('rdme versions:create', () => { .reply(200, [{ version }, { version: '1.1.0' }]) .post('/api/v1/version', { version: newVersion, - codename: '', is_stable: false, is_beta: true, from: '1.0.0', @@ -75,7 +74,9 @@ describe('rdme versions:create', () => { version: newVersion, codename: 'test', from: '1.0.0', + is_beta: false, is_hidden: false, + is_stable: false, }) .basicAuth({ user: key }) .reply(201, { version: newVersion }); @@ -108,4 +109,48 @@ describe('rdme versions:create', () => { await expect(createVersion.run({ key, version, fork: '0.0.5' })).rejects.toStrictEqual(new APIError(errorResponse)); mockRequest.done(); }); + + describe('bad flag values', () => { + it('should throw if non-boolean `beta` flag is passed', async () => { + const newVersion = '1.0.1'; + + await expect( + createVersion.run({ + key, + version: newVersion, + fork: version, + // @ts-expect-error deliberately passing a bad value here + beta: 'test', + }) + ).rejects.toStrictEqual(new Error("Invalid option passed for 'beta'. Must be 'true' or 'false'.")); + }); + + it('should throw if non-boolean `isPublic` flag is passed', async () => { + const newVersion = '1.0.1'; + + await expect( + createVersion.run({ + key, + version: newVersion, + fork: version, + // @ts-expect-error deliberately passing a bad value here + isPublic: 'test', + }) + ).rejects.toStrictEqual(new Error("Invalid option passed for 'isPublic'. Must be 'true' or 'false'.")); + }); + + it('should throw if non-boolean `main` flag is passed', async () => { + const newVersion = '1.0.1'; + + await expect( + createVersion.run({ + key, + version: newVersion, + fork: version, + // @ts-expect-error deliberately passing a bad value here + main: 'test', + }) + ).rejects.toStrictEqual(new Error("Invalid option passed for 'main'. Must be 'true' or 'false'.")); + }); + }); }); diff --git a/__tests__/cmds/versions/update.test.ts b/__tests__/cmds/versions/update.test.ts index 0848e9504..70fc4ac35 100644 --- a/__tests__/cmds/versions/update.test.ts +++ b/__tests__/cmds/versions/update.test.ts @@ -36,7 +36,6 @@ describe('rdme versions:update', () => { prompts.inject([versionToChange, renamedVersion, false, true, true, false]); const updatedVersionObject = { - codename: '', version: renamedVersion, is_stable: false, is_beta: true, @@ -67,7 +66,9 @@ describe('rdme versions:update', () => { codename: 'updated-test', version: renamedVersion, is_beta: true, + is_deprecated: true, is_hidden: false, + is_stable: false, }; const mockRequest = getAPIMock() @@ -86,6 +87,7 @@ describe('rdme versions:update', () => { key, version: versionToChange, newVersion: renamedVersion, + deprecated: 'true', beta: 'true', main: 'false', codename: 'updated-test', @@ -95,6 +97,83 @@ describe('rdme versions:update', () => { mockRequest.done(); }); + it("should update a specific version object using flags that contain the string 'false'", async () => { + const versionToChange = '1.1.0'; + const renamedVersion = '1.1.0-update'; + + const updatedVersionObject = { + codename: 'updated-test', + version: renamedVersion, + is_beta: false, + is_deprecated: false, + is_hidden: false, + is_stable: false, + }; + + const mockRequest = getAPIMock() + .get(`/api/v1/version/${versionToChange}`) + .basicAuth({ user: key }) + .reply(200, { version: versionToChange }) + .get(`/api/v1/version/${versionToChange}`) + .basicAuth({ user: key }) + .reply(200, { version: versionToChange }) + .put(`/api/v1/version/${versionToChange}`, updatedVersionObject) + .basicAuth({ user: key }) + .reply(201, updatedVersionObject); + + await expect( + updateVersion.run({ + key, + version: versionToChange, + newVersion: renamedVersion, + beta: 'false', + deprecated: 'false', + main: 'false', + codename: 'updated-test', + isPublic: 'true', + }) + ).resolves.toBe(`Version ${versionToChange} updated successfully.`); + mockRequest.done(); + }); + + it("should update a specific version object using flags that contain the string 'false' and a prompt", async () => { + const versionToChange = '1.1.0'; + const renamedVersion = '1.1.0-update'; + // prompt for beta flag + prompts.inject([false]); + + const updatedVersionObject = { + codename: 'updated-test', + version: renamedVersion, + is_beta: false, + is_hidden: false, + is_stable: false, + }; + + const mockRequest = getAPIMock() + .get(`/api/v1/version/${versionToChange}`) + .basicAuth({ user: key }) + .reply(200, { version: versionToChange }) + .get(`/api/v1/version/${versionToChange}`) + .basicAuth({ user: key }) + .reply(200, { version: versionToChange }) + .put(`/api/v1/version/${versionToChange}`, updatedVersionObject) + .basicAuth({ user: key }) + .reply(201, updatedVersionObject); + + await expect( + updateVersion.run({ + key, + version: versionToChange, + newVersion: renamedVersion, + main: 'false', + codename: 'updated-test', + isPublic: 'true', + }) + ).resolves.toBe(`Version ${versionToChange} updated successfully.`); + mockRequest.done(); + }); + // Note: this test is a bit bizarre since the flag management // in our version commands is really confusing to follow. // I'm not sure if it's technically possible to demote a stable version @@ -104,10 +183,10 @@ describe('rdme versions:update', () => { const renamedVersion = '1.0.0-update'; const updatedVersionObject = { - codename: '', version: renamedVersion, is_beta: true, is_hidden: true, + is_stable: false, }; prompts.inject([renamedVersion, true]); @@ -133,4 +212,94 @@ describe('rdme versions:update', () => { await expect(updateVersion.run({ key, version, main: 'false' })).rejects.toStrictEqual(new APIError(errorResponse)); mockRequest.done(); }); + + describe('bad flag values', () => { + it('should throw if non-boolean `beta` flag is passed', async () => { + const versionToChange = '1.1.0'; + + const mockRequest = getAPIMock() + .get(`/api/v1/version/${versionToChange}`) + .basicAuth({ user: key }) + .reply(200, { version: versionToChange }) + .get(`/api/v1/version/${versionToChange}`) + .basicAuth({ user: key }) + .reply(200, { version: versionToChange }); + + await expect( + updateVersion.run({ + key, + version: versionToChange, + // @ts-expect-error deliberately passing a bad value here + beta: 'hi', + }) + ).rejects.toStrictEqual(new Error("Invalid option passed for 'beta'. Must be 'true' or 'false'.")); + mockRequest.done(); + }); + + it('should throw if non-boolean `deprecated` flag is passed', async () => { + const versionToChange = '1.1.0'; + + const mockRequest = getAPIMock() + .get(`/api/v1/version/${versionToChange}`) + .basicAuth({ user: key }) + .reply(200, { version: versionToChange }) + .get(`/api/v1/version/${versionToChange}`) + .basicAuth({ user: key }) + .reply(200, { version: versionToChange }); + + await expect( + updateVersion.run({ + key, + version: versionToChange, + // @ts-expect-error deliberately passing a bad value here + deprecated: 'hi', + }) + ).rejects.toStrictEqual(new Error("Invalid option passed for 'deprecated'. Must be 'true' or 'false'.")); + mockRequest.done(); + }); + + it('should throw if non-boolean `isPublic` flag is passed', async () => { + const versionToChange = '1.1.0'; + + const mockRequest = getAPIMock() + .get(`/api/v1/version/${versionToChange}`) + .basicAuth({ user: key }) + .reply(200, { version: versionToChange }) + .get(`/api/v1/version/${versionToChange}`) + .basicAuth({ user: key }) + .reply(200, { version: versionToChange }); + + await expect( + updateVersion.run({ + key, + version: versionToChange, + // @ts-expect-error deliberately passing a bad value here + isPublic: 'hi', + }) + ).rejects.toStrictEqual(new Error("Invalid option passed for 'isPublic'. Must be 'true' or 'false'.")); + mockRequest.done(); + }); + + it('should throw if non-boolean `main` flag is passed', async () => { + const versionToChange = '1.1.0'; + + const mockRequest = getAPIMock() + .get(`/api/v1/version/${versionToChange}`) + .basicAuth({ user: key }) + .reply(200, { version: versionToChange }) + .get(`/api/v1/version/${versionToChange}`) + .basicAuth({ user: key }) + .reply(200, { version: versionToChange }); + + await expect( + updateVersion.run({ + key, + version: versionToChange, + // @ts-expect-error deliberately passing a bad value here + main: 'hi', + }) + ).rejects.toStrictEqual(new Error("Invalid option passed for 'main'. Must be 'true' or 'false'.")); + mockRequest.done(); + }); + }); }); diff --git a/__tests__/lib/prompts.test.ts b/__tests__/lib/prompts.test.ts index 32bdeb689..8fce2d6a4 100644 --- a/__tests__/lib/prompts.test.ts +++ b/__tests__/lib/prompts.test.ts @@ -1,4 +1,3 @@ -import type { Options as VersionUpdateOptions } from '../../src/cmds/versions/update'; import type { Response } from 'node-fetch'; import prompts from 'prompts'; @@ -83,23 +82,21 @@ describe('prompt test bed', () => { describe('createVersionPrompt()', () => { it('should allow user to choose a fork if flag is not passed (creating version)', async () => { - const opts: VersionUpdateOptions = { newVersion: '1.2.1' }; - prompts.inject(['1', true, true]); - const answer = await promptTerminal(promptHandler.createVersionPrompt(versionlist, opts)); + const answer = await promptTerminal(promptHandler.createVersionPrompt(versionlist)); expect(answer).toStrictEqual({ from: '1', is_stable: true, is_beta: true }); }); it('should skip fork prompt if value passed (updating version)', async () => { prompts.inject(['1.2.1', false, true, true, false]); - const answer = await promptTerminal(promptHandler.createVersionPrompt(versionlist, {}, { is_stable: false })); + const answer = await promptTerminal(promptHandler.createVersionPrompt(versionlist, { is_stable: false })); expect(answer).toStrictEqual({ newVersion: '1.2.1', is_stable: false, is_beta: true, - is_hidden: true, + is_public: true, is_deprecated: false, }); }); diff --git a/src/cmds/versions/create.ts b/src/cmds/versions/create.ts index 84a5757ea..db872b35c 100644 --- a/src/cmds/versions/create.ts +++ b/src/cmds/versions/create.ts @@ -3,9 +3,11 @@ import type { CommandOptions } from '../../lib/baseCommand'; import config from 'config'; import { Headers } from 'node-fetch'; +import prompts from 'prompts'; import semver from 'semver'; import Command, { CommandCategories } from '../../lib/baseCommand'; +import castStringOptToBool from '../../lib/castStringOptToBool'; import * as promptHandler from '../../lib/prompts'; import promptTerminal from '../../lib/promptWrapper'; import readmeAPIFetch, { cleanHeaders, handleRes } from '../../lib/readmeAPIFetch'; @@ -66,20 +68,24 @@ export default class CreateVersionCommand extends Command { }).then(handleRes); } - const versionPrompt = promptHandler.createVersionPrompt(versionList || [], { - newVersion: version, - ...opts, + const versionPrompt = promptHandler.createVersionPrompt(versionList || []); + + prompts.override({ + from: fork, + is_beta: castStringOptToBool(beta, 'beta'), + is_public: castStringOptToBool(isPublic, 'isPublic'), + is_stable: castStringOptToBool(main, 'main'), }); const promptResponse = await promptTerminal(versionPrompt); const body: Version = { + codename, version, - codename: codename || '', - is_stable: main === 'true' || promptResponse.is_stable, - is_beta: beta === 'true' || promptResponse.is_beta, - from: fork || promptResponse.from, - is_hidden: promptResponse.is_stable ? false : !(isPublic === 'true' || promptResponse.is_hidden), + from: promptResponse.from, + is_beta: promptResponse.is_beta, + is_hidden: !promptResponse.is_public, + is_stable: promptResponse.is_stable, }; return readmeAPIFetch('/api/v1/version', { diff --git a/src/cmds/versions/update.ts b/src/cmds/versions/update.ts index 4293c739e..2c803bf36 100644 --- a/src/cmds/versions/update.ts +++ b/src/cmds/versions/update.ts @@ -3,8 +3,10 @@ import type { CommonOptions } from './create'; import type { CommandOptions } from '../../lib/baseCommand'; import { Headers } from 'node-fetch'; +import prompts from 'prompts'; import Command, { CommandCategories } from '../../lib/baseCommand'; +import castStringOptToBool from '../../lib/castStringOptToBool'; import * as promptHandler from '../../lib/prompts'; import promptTerminal from '../../lib/promptWrapper'; import readmeAPIFetch, { cleanHeaders, handleRes } from '../../lib/readmeAPIFetch'; @@ -55,20 +57,30 @@ export default class UpdateVersionCommand extends Command { Command.debug(`selectedVersion: ${selectedVersion}`); + // TODO: I think this is fetch here is unnecessary but + // it will require a bigger refactor of getProjectVersion const foundVersion = await readmeAPIFetch(`/api/v1/version/${selectedVersion}`, { method: 'get', headers: cleanHeaders(key), }).then(handleRes); - const promptResponse = await promptTerminal(promptHandler.createVersionPrompt([], opts, foundVersion)); + prompts.override({ + is_beta: castStringOptToBool(beta, 'beta'), + is_deprecated: castStringOptToBool(deprecated, 'deprecated'), + is_public: castStringOptToBool(isPublic, 'isPublic'), + is_stable: castStringOptToBool(main, 'main'), + newVersion, + }); + + const promptResponse = await promptTerminal(promptHandler.createVersionPrompt([], foundVersion)); const body: Version = { - codename: codename || '', - version: newVersion || promptResponse.newVersion, - is_stable: foundVersion.is_stable || main === 'true' || promptResponse.is_stable, - is_beta: beta === 'true' || promptResponse.is_beta, - is_deprecated: deprecated === 'true' || promptResponse.is_deprecated, - is_hidden: promptResponse.is_stable ? false : !(isPublic === 'true' || promptResponse.is_hidden), + codename, + version: promptResponse.newVersion, + is_beta: promptResponse.is_beta, + is_deprecated: promptResponse.is_deprecated, + is_hidden: !promptResponse.is_public, + is_stable: promptResponse.is_stable, }; return readmeAPIFetch(`/api/v1/version/${selectedVersion}`, { diff --git a/src/lib/castStringOptToBool.ts b/src/lib/castStringOptToBool.ts new file mode 100644 index 000000000..591f3ac65 --- /dev/null +++ b/src/lib/castStringOptToBool.ts @@ -0,0 +1,19 @@ +import type { Options as CreateOptions } from '../cmds/versions/create'; +import type { Options as UpdateOptions } from '../cmds/versions/update'; + +/** + * Takes a CLI flag that is expected to be a 'true' or 'false' string + * and casts it to a boolean. + */ +export default function castStringOptToBool(opt: 'true' | 'false', optName: keyof CreateOptions | keyof UpdateOptions) { + if (!opt) { + return undefined; + } + if (opt === 'true') { + return true; + } + if (opt === 'false') { + return false; + } + throw new Error(`Invalid option passed for '${optName}'. Must be 'true' or 'false'.`); +} diff --git a/src/lib/prompts.ts b/src/lib/prompts.ts index b56be1482..02cb94a11 100644 --- a/src/lib/prompts.ts +++ b/src/lib/prompts.ts @@ -1,6 +1,4 @@ import type { Version } from '../cmds/versions'; -import type { Options as VersionCreateOptions } from 'cmds/versions/create'; -import type { Options as VersionUpdateOptions } from 'cmds/versions/update'; import type { Response } from 'node-fetch'; import type { Choice, PromptObject } from 'prompts'; @@ -131,15 +129,17 @@ export function createOasPrompt( } export function createVersionPrompt( + /** list of versions, used for prompt about which version to fork */ versionList: Version[], - opts: VersionCreateOptions & VersionUpdateOptions, + /** existing version if we're performing an update */ isUpdate?: { is_stable: boolean; } ): PromptObject[] { return [ { - type: opts.fork || isUpdate ? null : 'select', + // only runs for versions:create command + type: isUpdate ? null : 'select', name: 'from', message: 'Which version would you like to fork from?', choices: versionList.map(v => { @@ -150,35 +150,43 @@ export function createVersionPrompt( }), }, { - type: opts.newVersion || !isUpdate ? null : 'text', + // only runs for versions:update command + type: !isUpdate ? null : 'text', name: 'newVersion', message: 'What should the version be renamed to?', - initial: opts.newVersion || false, hint: '1.0.0', validate(val: string) { + // allow empty string, in which case the version won't be renamed + if (!val) return true; return semver.valid(semver.coerce(val)) ? true : 'Please specify a semantic version.'; }, }, { - type: opts.main || isUpdate?.is_stable ? null : 'confirm', + // if the existing version being updated is already the main version, + // we can't switch that so we skip this question + type: isUpdate?.is_stable ? null : 'confirm', name: 'is_stable', message: 'Would you like to make this version the main version for this project?', }, { - type: opts.beta ? null : 'confirm', + type: 'confirm', name: 'is_beta', message: 'Should this version be in beta?', }, { type: (prev, values) => { - return opts.isPublic || opts.main || values.is_stable ? null : 'confirm'; + // if user previously wanted this version to be the main version + // it can't also be hidden. + return values.is_stable ? null : 'confirm'; }, - name: 'is_hidden', + name: 'is_public', message: 'Would you like to make this version public?', }, { type: (prev, values) => { - return opts.deprecated || opts.main || !isUpdate || values.is_stable ? null : 'confirm'; + // if user previously wanted this version to be the main version + // it can't also be deprecated. + return values.is_stable ? null : 'confirm'; }, name: 'is_deprecated', message: 'Would you like to deprecate this version?', From a469d0f7358d9f25af815ead702f53a1211b44eb Mon Sep 17 00:00:00 2001 From: Kanad Gupta Date: Fri, 18 Aug 2023 13:47:14 -0500 Subject: [PATCH 2/7] refactor: rename function, add JSDoc --- __tests__/lib/prompts.test.ts | 6 +++--- src/cmds/versions/create.ts | 2 +- src/cmds/versions/update.ts | 2 +- src/lib/prompts.ts | 6 +++++- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/__tests__/lib/prompts.test.ts b/__tests__/lib/prompts.test.ts index 8fce2d6a4..fb0b06d95 100644 --- a/__tests__/lib/prompts.test.ts +++ b/__tests__/lib/prompts.test.ts @@ -80,18 +80,18 @@ describe('prompt test bed', () => { }); }); - describe('createVersionPrompt()', () => { + describe('versionPrompt()', () => { it('should allow user to choose a fork if flag is not passed (creating version)', async () => { prompts.inject(['1', true, true]); - const answer = await promptTerminal(promptHandler.createVersionPrompt(versionlist)); + const answer = await promptTerminal(promptHandler.versionPrompt(versionlist)); expect(answer).toStrictEqual({ from: '1', is_stable: true, is_beta: true }); }); it('should skip fork prompt if value passed (updating version)', async () => { prompts.inject(['1.2.1', false, true, true, false]); - const answer = await promptTerminal(promptHandler.createVersionPrompt(versionlist, { is_stable: false })); + const answer = await promptTerminal(promptHandler.versionPrompt(versionlist, { is_stable: false })); expect(answer).toStrictEqual({ newVersion: '1.2.1', is_stable: false, diff --git a/src/cmds/versions/create.ts b/src/cmds/versions/create.ts index db872b35c..ea855ec82 100644 --- a/src/cmds/versions/create.ts +++ b/src/cmds/versions/create.ts @@ -68,7 +68,7 @@ export default class CreateVersionCommand extends Command { }).then(handleRes); } - const versionPrompt = promptHandler.createVersionPrompt(versionList || []); + const versionPrompt = promptHandler.versionPrompt(versionList || []); prompts.override({ from: fork, diff --git a/src/cmds/versions/update.ts b/src/cmds/versions/update.ts index 2c803bf36..c70be96d7 100644 --- a/src/cmds/versions/update.ts +++ b/src/cmds/versions/update.ts @@ -72,7 +72,7 @@ export default class UpdateVersionCommand extends Command { newVersion, }); - const promptResponse = await promptTerminal(promptHandler.createVersionPrompt([], foundVersion)); + const promptResponse = await promptTerminal(promptHandler.versionPrompt([], foundVersion)); const body: Version = { codename, diff --git a/src/lib/prompts.ts b/src/lib/prompts.ts index 02cb94a11..27fe1377d 100644 --- a/src/lib/prompts.ts +++ b/src/lib/prompts.ts @@ -128,7 +128,11 @@ export function createOasPrompt( ]; } -export function createVersionPrompt( +/** + * Series of prompts to construct a version object, + * used in our `versions:create` and `versions:update` commands + */ +export function versionPrompt( /** list of versions, used for prompt about which version to fork */ versionList: Version[], /** existing version if we're performing an update */ From 2c00d336a8452932d4b9df0c0a4672e1d2a0738c Mon Sep 17 00:00:00 2001 From: Kanad Gupta Date: Fri, 18 Aug 2023 14:21:04 -0500 Subject: [PATCH 3/7] fix: don't prompt for deprecated flag in create command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit major bummer that prompts.inject doesn't play nicely with the `type` values in the prompt definitions. figured this out by just playing around locally 😞 --- src/lib/prompts.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/lib/prompts.ts b/src/lib/prompts.ts index 27fe1377d..6736b9df5 100644 --- a/src/lib/prompts.ts +++ b/src/lib/prompts.ts @@ -190,7 +190,11 @@ export function versionPrompt( type: (prev, values) => { // if user previously wanted this version to be the main version // it can't also be deprecated. - return values.is_stable ? null : 'confirm'; + // + // we also only allow the setting of this flag in the versions:update command + // it technically could run in versions:create but that might be considered + // a breaking change + return values.is_stable || !isUpdate ? null : 'confirm'; }, name: 'is_deprecated', message: 'Would you like to deprecate this version?', From c866734fffabc17808f69f68ac2f61bb03766dc9 Mon Sep 17 00:00:00 2001 From: Kanad Gupta Date: Fri, 18 Aug 2023 14:33:52 -0500 Subject: [PATCH 4/7] fix: actually screw it y'all get deprecated too --- __tests__/cmds/versions/create.test.ts | 28 ++++++++++++++++++++------ src/cmds/versions/create.ts | 5 ++++- src/cmds/versions/update.ts | 1 - src/lib/prompts.ts | 6 +----- 4 files changed, 27 insertions(+), 13 deletions(-) diff --git a/__tests__/cmds/versions/create.test.ts b/__tests__/cmds/versions/create.test.ts index c3346d440..4c6e0ab47 100644 --- a/__tests__/cmds/versions/create.test.ts +++ b/__tests__/cmds/versions/create.test.ts @@ -75,6 +75,7 @@ describe('rdme versions:create', () => { codename: 'test', from: '1.0.0', is_beta: false, + is_deprecated: false, is_hidden: false, is_stable: false, }) @@ -87,6 +88,7 @@ describe('rdme versions:create', () => { version: newVersion, fork: version, beta: 'false', + deprecated: 'false', main: 'false', codename: 'test', isPublic: 'true', @@ -111,10 +113,10 @@ describe('rdme versions:create', () => { }); describe('bad flag values', () => { - it('should throw if non-boolean `beta` flag is passed', async () => { + it('should throw if non-boolean `beta` flag is passed', () => { const newVersion = '1.0.1'; - await expect( + return expect( createVersion.run({ key, version: newVersion, @@ -125,10 +127,24 @@ describe('rdme versions:create', () => { ).rejects.toStrictEqual(new Error("Invalid option passed for 'beta'. Must be 'true' or 'false'.")); }); - it('should throw if non-boolean `isPublic` flag is passed', async () => { + it('should throw if non-boolean `deprecated` flag is passed', () => { const newVersion = '1.0.1'; - await expect( + return expect( + createVersion.run({ + key, + version: newVersion, + fork: version, + // @ts-expect-error deliberately passing a bad value here + deprecated: 'test', + }) + ).rejects.toStrictEqual(new Error("Invalid option passed for 'deprecated'. Must be 'true' or 'false'.")); + }); + + it('should throw if non-boolean `isPublic` flag is passed', () => { + const newVersion = '1.0.1'; + + return expect( createVersion.run({ key, version: newVersion, @@ -139,10 +155,10 @@ describe('rdme versions:create', () => { ).rejects.toStrictEqual(new Error("Invalid option passed for 'isPublic'. Must be 'true' or 'false'.")); }); - it('should throw if non-boolean `main` flag is passed', async () => { + it('should throw if non-boolean `main` flag is passed', () => { const newVersion = '1.0.1'; - await expect( + return expect( createVersion.run({ key, version: newVersion, diff --git a/src/cmds/versions/create.ts b/src/cmds/versions/create.ts index ea855ec82..b081864c8 100644 --- a/src/cmds/versions/create.ts +++ b/src/cmds/versions/create.ts @@ -19,6 +19,7 @@ export interface Options extends CommonOptions { export interface CommonOptions { beta?: 'true' | 'false'; codename?: string; + deprecated?: 'true' | 'false'; isPublic?: 'true' | 'false'; main?: 'true' | 'false'; } @@ -53,7 +54,7 @@ export default class CreateVersionCommand extends Command { await super.run(opts); let versionList; - const { key, version, fork, codename, main, beta, isPublic } = opts; + const { key, version, fork, codename, main, beta, deprecated, isPublic } = opts; if (!version || !semver.valid(semver.coerce(version))) { return Promise.reject( @@ -73,6 +74,7 @@ export default class CreateVersionCommand extends Command { prompts.override({ from: fork, is_beta: castStringOptToBool(beta, 'beta'), + is_deprecated: castStringOptToBool(deprecated, 'deprecated'), is_public: castStringOptToBool(isPublic, 'isPublic'), is_stable: castStringOptToBool(main, 'main'), }); @@ -84,6 +86,7 @@ export default class CreateVersionCommand extends Command { version, from: promptResponse.from, is_beta: promptResponse.is_beta, + is_deprecated: promptResponse.is_deprecated, is_hidden: !promptResponse.is_public, is_stable: promptResponse.is_stable, }; diff --git a/src/cmds/versions/update.ts b/src/cmds/versions/update.ts index c70be96d7..0fa734d92 100644 --- a/src/cmds/versions/update.ts +++ b/src/cmds/versions/update.ts @@ -13,7 +13,6 @@ import readmeAPIFetch, { cleanHeaders, handleRes } from '../../lib/readmeAPIFetc import { getProjectVersion } from '../../lib/versionSelect'; export interface Options extends CommonOptions { - deprecated?: 'true' | 'false'; newVersion?: string; } diff --git a/src/lib/prompts.ts b/src/lib/prompts.ts index 6736b9df5..27fe1377d 100644 --- a/src/lib/prompts.ts +++ b/src/lib/prompts.ts @@ -190,11 +190,7 @@ export function versionPrompt( type: (prev, values) => { // if user previously wanted this version to be the main version // it can't also be deprecated. - // - // we also only allow the setting of this flag in the versions:update command - // it technically could run in versions:create but that might be considered - // a breaking change - return values.is_stable || !isUpdate ? null : 'confirm'; + return values.is_stable ? null : 'confirm'; }, name: 'is_deprecated', message: 'Would you like to deprecate this version?', From 1b625d1a51afff7b26c11c8ccc26fd06eef1ac43 Mon Sep 17 00:00:00 2001 From: Kanad Gupta Date: Fri, 18 Aug 2023 14:42:04 -0500 Subject: [PATCH 5/7] fix: consolidate option definitions --- src/cmds/versions/create.ts | 5 ----- src/cmds/versions/update.ts | 10 ---------- src/lib/baseCommand.ts | 10 ++++++++++ 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/cmds/versions/create.ts b/src/cmds/versions/create.ts index b081864c8..850d7cb35 100644 --- a/src/cmds/versions/create.ts +++ b/src/cmds/versions/create.ts @@ -36,11 +36,6 @@ export default class CreateVersionCommand extends Command { this.hiddenArgs = ['version']; this.args = [ this.getKeyArg(), - { - name: 'version', - type: String, - defaultOption: true, - }, { name: 'fork', type: String, diff --git a/src/cmds/versions/update.ts b/src/cmds/versions/update.ts index 0fa734d92..d455c0c72 100644 --- a/src/cmds/versions/update.ts +++ b/src/cmds/versions/update.ts @@ -28,22 +28,12 @@ export default class UpdateVersionCommand extends Command { this.hiddenArgs = ['version']; this.args = [ this.getKeyArg(), - { - name: 'version', - type: String, - defaultOption: true, - }, { name: 'newVersion', type: String, description: 'What should the version be renamed to?', }, ...this.getVersionOpts(), - { - name: 'deprecated', - type: String, - description: "Would you like to deprecate this version? (Must be 'true' or 'false')", - }, ]; } diff --git a/src/lib/baseCommand.ts b/src/lib/baseCommand.ts index cece69a42..d554919de 100644 --- a/src/lib/baseCommand.ts +++ b/src/lib/baseCommand.ts @@ -172,6 +172,11 @@ export default class Command { */ getVersionOpts(): OptionDefinition[] { return [ + { + name: 'version', + type: String, + defaultOption: true, + }, { name: 'codename', type: String, @@ -188,6 +193,11 @@ export default class Command { type: String, description: "Is this version in beta? (Must be 'true' or 'false')", }, + { + name: 'deprecated', + type: String, + description: "Would you like to deprecate this version? (Must be 'true' or 'false')", + }, { name: 'isPublic', type: String, From 0a657ff7facb5cd98ba22f2cc93c04f9502592e8 Mon Sep 17 00:00:00 2001 From: Kanad Gupta Date: Fri, 18 Aug 2023 16:44:16 -0500 Subject: [PATCH 6/7] fix: fallback to current version if user doesn't enter one kind of weird, but our PUT /version endpoint requires a version as both the path param and as a body param, even if you aren't changing the name of the version. and passing in an empty string for the latter will result in the endpoint shitting the bed. that's API v1 for ya, folks! --- __tests__/cmds/versions/update.test.ts | 37 ++++++++++++++++++++++++++ src/cmds/versions/update.ts | 3 ++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/__tests__/cmds/versions/update.test.ts b/__tests__/cmds/versions/update.test.ts index 70fc4ac35..4e5e646a5 100644 --- a/__tests__/cmds/versions/update.test.ts +++ b/__tests__/cmds/versions/update.test.ts @@ -174,6 +174,43 @@ describe('rdme versions:update', () => { mockRequest.done(); }); + it('should update a specific version object even if user bypasses prompt for new version name', async () => { + const versionToChange = '1.1.0'; + // simulating user entering nothing for the prompt to enter a new version name + prompts.inject(['']); + + const updatedVersionObject = { + codename: 'updated-test', + is_beta: false, + is_hidden: false, + is_stable: false, + version: versionToChange, + }; + + const mockRequest = getAPIMock() + .get(`/api/v1/version/${versionToChange}`) + .basicAuth({ user: key }) + .reply(200, { version: versionToChange }) + .get(`/api/v1/version/${versionToChange}`) + .basicAuth({ user: key }) + .reply(200, { version: versionToChange }) + .put(`/api/v1/version/${versionToChange}`, updatedVersionObject) + .basicAuth({ user: key }) + .reply(201, updatedVersionObject); + + await expect( + updateVersion.run({ + key, + version: versionToChange, + beta: 'false', + main: 'false', + codename: 'updated-test', + isPublic: 'true', + }) + ).resolves.toBe(`Version ${versionToChange} updated successfully.`); + mockRequest.done(); + }); + // Note: this test is a bit bizarre since the flag management // in our version commands is really confusing to follow. // I'm not sure if it's technically possible to demote a stable version diff --git a/src/cmds/versions/update.ts b/src/cmds/versions/update.ts index d455c0c72..bebc35755 100644 --- a/src/cmds/versions/update.ts +++ b/src/cmds/versions/update.ts @@ -65,7 +65,8 @@ export default class UpdateVersionCommand extends Command { const body: Version = { codename, - version: promptResponse.newVersion, + // fall back to current version if user didn't enter one + version: promptResponse.newVersion || version, is_beta: promptResponse.is_beta, is_deprecated: promptResponse.is_deprecated, is_hidden: !promptResponse.is_public, From cb648e02354c4c0b5e3d788c6c3a8565161c1bea Mon Sep 17 00:00:00 2001 From: Kanad Gupta Date: Fri, 18 Aug 2023 16:49:30 -0500 Subject: [PATCH 7/7] chore: comment typo --- src/cmds/versions/update.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmds/versions/update.ts b/src/cmds/versions/update.ts index bebc35755..fe484933e 100644 --- a/src/cmds/versions/update.ts +++ b/src/cmds/versions/update.ts @@ -46,7 +46,7 @@ export default class UpdateVersionCommand extends Command { Command.debug(`selectedVersion: ${selectedVersion}`); - // TODO: I think this is fetch here is unnecessary but + // TODO: I think this fetch here is unnecessary but // it will require a bigger refactor of getProjectVersion const foundVersion = await readmeAPIFetch(`/api/v1/version/${selectedVersion}`, { method: 'get',