Skip to content

Commit

Permalink
fix: use npmFetch() instead of npmFetch.json() for team destroy comma…
Browse files Browse the repository at this point in the history
…nd (#6161)

The registry returns a 204 status and an empty body on success for the
team destroy command. Using npmFetch.json() makes the CLI error out on
completion even though the action was completed successfully in the
registry.
  • Loading branch information
heisantosh authored Feb 14, 2023
1 parent fef6ca0 commit 11e6cc9
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 17 deletions.
12 changes: 8 additions & 4 deletions workspaces/libnpmteam/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,17 @@ cmd.create = (entity, opts = {}) => {
})
}

cmd.destroy = (entity, opts = {}) => {
cmd.destroy = async (entity, opts = {}) => {
const { scope, team } = splitEntity(entity)
validate('SSO', [scope, team, opts])
const uri = `/-/team/${eu(scope)}/${eu(team)}`
return npmFetch.json(uri, {
await npmFetch(uri, {
...opts,
method: 'DELETE',
scope,
ignoreBody: true,
})
return true
}

cmd.add = (user, entity, opts = {}) => {
Expand All @@ -43,16 +45,18 @@ cmd.add = (user, entity, opts = {}) => {
})
}

cmd.rm = (user, entity, opts = {}) => {
cmd.rm = async (user, entity, opts = {}) => {
const { scope, team } = splitEntity(entity)
validate('SSO', [scope, team, opts])
const uri = `/-/team/${eu(scope)}/${eu(team)}/user`
return npmFetch.json(uri, {
await npmFetch(uri, {
...opts,
method: 'DELETE',
scope,
body: { user },
ignoreBody: true,
})
return true
}

cmd.lsTeams = (...args) => cmd.lsTeams.stream(...args).collect()
Expand Down
34 changes: 21 additions & 13 deletions workspaces/libnpmteam/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,28 +52,32 @@ test('create w/ description', async t => {
test('destroy', async t => {
tnock(t, REG).delete(
'/-/team/foo/cli'
).reply(204, {})
const ret = await team.destroy('@foo:cli', OPTS)
t.same(ret, {}, 'request succeeded')
).reply(204)
await t.resolves(
team.destroy('@foo:cli', OPTS),
'request succeeded'
)
})

test('destroy - no options', async t => {
// NOTE: mocking real url, because no opts variable means `registry` value
// will be defauled to real registry url in `npm-registry-fetch`
tnock(t, 'https://registry.npmjs.org')
.delete('/-/team/foo/cli')
.reply(204, {})
.reply(204)

const ret = await team.destroy('@foo:cli')
t.same(ret, {}, 'request succeeded')
await t.resolves(
team.destroy('@foo:cli'),
'request succeeded'
)
})

test('add', async t => {
tnock(t, REG).put(
'/-/team/foo/cli/user', { user: 'zkat' }
).reply(201, {})
const ret = await team.add('zkat', '@foo:cli', OPTS)
t.same(ret, {}, 'request succeeded')
t.ok(ret, 'request succeeded')
})

test('add - no options', async t => {
Expand All @@ -90,20 +94,24 @@ test('add - no options', async t => {
test('rm', async t => {
tnock(t, REG).delete(
'/-/team/foo/cli/user', { user: 'zkat' }
).reply(204, {})
const ret = await team.rm('zkat', '@foo:cli', OPTS)
t.same(ret, {}, 'request succeeded')
).reply(204)
await t.resolves(
team.rm('zkat', '@foo:cli', OPTS),
'request succeeded'
)
})

test('rm - no options', async t => {
// NOTE: mocking real url, because no opts variable means `registry` value
// will be defauled to real registry url in `npm-registry-fetch`
tnock(t, 'https://registry.npmjs.org')
.delete('/-/team/foo/cli/user', { user: 'zkat' })
.reply(204, {})
.reply(204)

const ret = await team.rm('zkat', '@foo:cli')
t.same(ret, {}, 'request succeeded')
await t.resolves(
team.rm('zkat', '@foo:cli'),
'request succeeded'
)
})

test('lsTeams', async t => {
Expand Down

0 comments on commit 11e6cc9

Please sign in to comment.