Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: cleanup star/unstar #4868

Merged
merged 1 commit into from
May 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions docs/content/commands/npm-star.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,20 @@ false, it uses ascii characters instead of unicode glyphs.
<!-- automatically generated, do not edit manually -->
<!-- see lib/utils/config/definitions.js -->

#### `otp`

* Default: null
* Type: null or String

This is a one-time password from a two-factor authenticator. It's needed
when publishing or changing package permissions with `npm access`.

If not set, and a registry response fails with a challenge for a one-time
password, npm will prompt on the command line for one.

<!-- automatically generated, do not edit manually -->
<!-- see lib/utils/config/definitions.js -->

<!-- AUTOGENERATED CONFIG DESCRIPTIONS END -->

### See Also
Expand Down
27 changes: 11 additions & 16 deletions lib/commands/star.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ class Star extends BaseCommand {
static params = [
'registry',
'unicode',
'otp',
]

static ignoreImplicitWorkspace = false
Expand All @@ -23,34 +24,28 @@ class Star extends BaseCommand {
// if we're unstarring, then show an empty star image
// otherwise, show the full star image
const unicode = this.npm.config.get('unicode')
const unstar = this.npm.config.get('star.unstar')
const full = unicode ? '\u2605 ' : '(*)'
const empty = unicode ? '\u2606 ' : '( )'
const show = unstar ? empty : full
const show = this.name === 'star' ? full : empty

const pkgs = args.map(npa)
for (const pkg of pkgs) {
const [username, fullData] = await Promise.all([
getIdentity(this.npm, { ...this.npm.flatOptions }),
fetch.json(pkg.escapedName, {
...this.npm.flatOptions,
spec: pkg,
query: { write: true },
preferOnline: true,
}),
])
const username = await getIdentity(this.npm, this.npm.flatOptions)

if (!username) {
throw new Error('You need to be logged in!')
}
for (const pkg of pkgs) {
const fullData = await fetch.json(pkg.escapedName, {
...this.npm.flatOptions,
spec: pkg,
query: { write: true },
preferOnline: true,
})

const body = {
_id: fullData._id,
_rev: fullData._rev,
users: fullData.users || {},
}

if (!unstar) {
if (this.name === 'star') {
log.info('star', 'starring', body._id)
body.users[username] = true
log.verbose('star', 'starring', body)
Expand Down
10 changes: 0 additions & 10 deletions lib/commands/unstar.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,5 @@ const Star = require('./star.js')
class Unstar extends Star {
static description = 'Remove an item from your favorite packages'
static name = 'unstar'
static params = [
'registry',
'unicode',
'otp',
]

async exec (args) {
this.npm.config.set('star.unstar', true)
return super.exec(args)
}
}
module.exports = Unstar
2 changes: 1 addition & 1 deletion tap-snapshots/test/lib/load-all-commands.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -828,7 +828,7 @@ Usage:
npm star [<pkg>...]

Options:
[--registry <registry>] [--unicode]
[--registry <registry>] [--unicode] [--otp <otp>]

Run "npm help star" for more info
`
Expand Down
2 changes: 1 addition & 1 deletion tap-snapshots/test/lib/utils/npm-usage.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -862,7 +862,7 @@ All commands:
npm star [<pkg>...]

Options:
[--registry <registry>] [--unicode]
[--registry <registry>] [--unicode] [--otp <otp>]

Run "npm help star" for more info

Expand Down
14 changes: 13 additions & 1 deletion test/fixtures/mock-registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,15 @@ class MockRegistry {
}
}

star (manifest, users) {
const spec = npa(manifest.name)
this.nock = this.nock.put(`/${spec.escapedName}`, {
_id: manifest._id,
_rev: manifest._rev,
users,
}).reply(200, { ...manifest, users })
}

async package ({ manifest, times = 1, query, tarballs }) {
let nock = this.nock
const spec = npa(manifest.name)
Expand All @@ -206,7 +215,7 @@ class MockRegistry {
// either pass in packuments if you need to set specific attributes besides version,
// or an array of versions
// the last packument in the packuments or versions array will be tagged latest
manifest ({ name = 'test-package', packuments, versions } = {}) {
manifest ({ name = 'test-package', users, packuments, versions } = {}) {
packuments = this.packuments(packuments, name)
const latest = packuments.slice(-1)[0]
const manifest = {
Expand All @@ -220,6 +229,9 @@ class MockRegistry {
'dist-tags': { latest: latest.version },
...latest,
}
if (users) {
manifest.users = users
}
if (versions) {
packuments = versions.map(version => ({ version }))
}
Expand Down
6 changes: 3 additions & 3 deletions test/lib/commands/deprecate.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ t.test('undeprecate', async t => {
name: 'foo',
versions,
})
registry.package({ manifest, query: { write: true } })
await registry.package({ manifest, query: { write: true } })
registry.nock.put('/foo', body => {
for (const version of versions) {
if (body.versions[version].deprecated !== '') {
Expand All @@ -110,7 +110,7 @@ t.test('deprecates given range', async t => {
name: 'foo',
versions,
})
registry.package({ manifest, query: { write: true } })
await registry.package({ manifest, query: { write: true } })
const message = 'test deprecation message'
registry.nock.put('/foo', body => {
if (body.versions['1.0.1'].deprecated) {
Expand All @@ -136,7 +136,7 @@ t.test('deprecates all versions when no range is specified', async t => {
name: 'foo',
versions,
})
registry.package({ manifest, query: { write: true } })
await registry.package({ manifest, query: { write: true } })
const message = 'test deprecation message'
registry.nock.put('/foo', body => {
for (const version of versions) {
Expand Down
44 changes: 22 additions & 22 deletions test/lib/commands/owner.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ function registryPackage (t, registry, name) {
name,
packuments: [{ maintainers, version: '1.0.0' }],
})
mockRegistry.package({ manifest })
return mockRegistry.package({ manifest })
}

t.test('owner no args', async t => {
Expand All @@ -73,7 +73,7 @@ t.test('owner ls no args', async t => {
name: packageName,
packuments: [{ maintainers, version: '1.0.0' }],
})
registry.package({ manifest })
await registry.package({ manifest })

await npm.exec('owner', ['ls'])
t.match(joinedOutput(), maintainers.map(m => `${m.name} <${m.email}>`).join('\n'))
Expand Down Expand Up @@ -137,7 +137,7 @@ t.test('owner ls <pkg>', async t => {
name: packageName,
packuments: [{ maintainers, version: '1.0.0' }],
})
registry.package({ manifest })
await registry.package({ manifest })

await npm.exec('owner', ['ls', packageName])
t.match(joinedOutput(), maintainers.map(m => `${m.name} <${m.email}>`).join('\n'))
Expand All @@ -153,7 +153,7 @@ t.test('owner ls <pkg> no maintainers', async t => {
name: packageName,
versions: ['1.0.0'],
})
registry.package({ manifest })
await registry.package({ manifest })

await npm.exec('owner', ['ls', packageName])
t.equal(joinedOutput(), 'no admin found')
Expand All @@ -173,7 +173,7 @@ t.test('owner add <user> <pkg>', async t => {
packuments: [{ maintainers, version: '1.0.0' }],
})
registry.couchuser({ username })
registry.package({ manifest })
await registry.package({ manifest })
registry.nock.put(`/${spec.escapedName}/-rev/${manifest._rev}`, body => {
t.match(body, {
_id: manifest._id,
Expand Down Expand Up @@ -206,7 +206,7 @@ t.test('owner add <user> cwd package', async t => {
packuments: [{ maintainers, version: '1.0.0' }],
})
registry.couchuser({ username })
registry.package({ manifest })
await registry.package({ manifest })
registry.nock.put(`/${spec.escapedName}/-rev/${manifest._rev}`, body => {
t.match(body, {
_id: manifest._id,
Expand Down Expand Up @@ -236,7 +236,7 @@ t.test('owner add <user> <pkg> already an owner', async t => {
packuments: [{ maintainers, version: '1.0.0' }],
})
registry.couchuser({ username })
registry.package({ manifest })
await registry.package({ manifest })
await npm.exec('owner', ['add', username, packageName])
t.equal(joinedOutput(), '')
t.match(
Expand Down Expand Up @@ -273,7 +273,7 @@ t.test('owner add <user> <pkg> fails to PUT updates', async t => {
packuments: [{ maintainers, version: '1.0.0' }],
})
registry.couchuser({ username })
registry.package({ manifest })
await registry.package({ manifest })
registry.nock.put(`/${spec.escapedName}/-rev/${manifest._rev}`).reply(404, {})
await t.rejects(
npm.exec('owner', ['add', username, packageName]),
Expand All @@ -295,7 +295,7 @@ t.test('owner add <user> <pkg> no previous maintainers property from server', as
packuments: [{ maintainers: undefined, version: '1.0.0' }],
})
registry.couchuser({ username })
registry.package({ manifest })
await registry.package({ manifest })
registry.nock.put(`/${spec.escapedName}/-rev/${manifest._rev}`, body => {
t.match(body, {
_id: manifest._id,
Expand Down Expand Up @@ -351,7 +351,7 @@ t.test('owner rm <user> <pkg>', async t => {
packuments: [{ maintainers, version: '1.0.0' }],
})
registry.couchuser({ username })
registry.package({ manifest })
await registry.package({ manifest })
registry.nock.put(`/${spec.escapedName}/-rev/${manifest._rev}`, body => {
t.match(body, {
_id: manifest._id,
Expand All @@ -378,7 +378,7 @@ t.test('owner rm <user> <pkg> not a current owner', async t => {
packuments: [{ maintainers, version: '1.0.0' }],
})
registry.couchuser({ username })
registry.package({ manifest })
await registry.package({ manifest })
await npm.exec('owner', ['rm', username, packageName])
t.match(logs.info, [['owner rm', `Not a package owner: ${username}`]])
})
Expand All @@ -400,7 +400,7 @@ t.test('owner rm <user> cwd package', async t => {
packuments: [{ maintainers, version: '1.0.0' }],
})
registry.couchuser({ username })
registry.package({ manifest })
await registry.package({ manifest })
registry.nock.put(`/${spec.escapedName}/-rev/${manifest._rev}`, body => {
t.match(body, {
_id: manifest._id,
Expand Down Expand Up @@ -430,7 +430,7 @@ t.test('owner rm <user> only user', async t => {
packuments: [{ maintainers: maintainers.slice(0, 1), version: '1.0.0' }],
})
registry.couchuser({ username })
registry.package({ manifest })
await registry.package({ manifest })
await t.rejects(
npm.exec('owner', ['rm', username]),
{
Expand Down Expand Up @@ -486,7 +486,7 @@ t.test('workspaces', async t => {
'process.cwd': () => path.join(prefix, 'workspace-a'),
}),
})
registryPackage(t, npm.config.get('registry'), 'workspace-a')
await registryPackage(t, npm.config.get('registry'), 'workspace-a')
await npm.exec('owner', ['ls'])
t.match(joinedOutput(), maintainers.map(m => `${m.name} <${m.email}>`).join('\n'))
})
Expand All @@ -499,7 +499,7 @@ t.test('workspaces', async t => {
}),
})
npm.config.set('workspace', ['workspace-a'])
registryPackage(t, npm.config.get('registry'), 'workspace-a')
await registryPackage(t, npm.config.get('registry'), 'workspace-a')
await npm.exec('owner', ['ls'])
t.match(joinedOutput(), maintainers.map(m => `${m.name} <${m.email}>`).join('\n'))
})
Expand All @@ -511,7 +511,7 @@ t.test('workspaces', async t => {
'process.cwd': () => path.join(prefix, 'workspace-a'),
}),
})
registryPackage(t, npm.config.get('registry'), packageName)
await registryPackage(t, npm.config.get('registry'), packageName)
await npm.exec('owner', ['ls', packageName])
t.match(joinedOutput(), maintainers.map(m => `${m.name} <${m.email}>`).join('\n'))
})
Expand All @@ -524,7 +524,7 @@ t.test('workspaces', async t => {
}),
})
npm.config.set('workspace', ['workspace-a'])
registryPackage(t, npm.config.get('registry'), packageName)
await registryPackage(t, npm.config.get('registry'), packageName)
await npm.exec('owner', ['ls', packageName])
t.match(joinedOutput(), maintainers.map(m => `${m.name} <${m.email}>`).join('\n'))
})
Expand All @@ -543,7 +543,7 @@ t.test('workspaces', async t => {
name: 'workspace-a',
packuments: [{ maintainers, version: '1.0.0' }],
})
registry.package({ manifest })
await registry.package({ manifest })
registry.couchuser({ username })
registry.nock.put(`/workspace-a/-rev/${manifest._rev}`, body => {
t.match(body, {
Expand Down Expand Up @@ -572,7 +572,7 @@ t.test('workspaces', async t => {
name: 'workspace-a',
packuments: [{ maintainers, version: '1.0.0' }],
})
registry.package({ manifest })
await registry.package({ manifest })
registry.couchuser({ username })
registry.nock.put(`/workspace-a/-rev/${manifest._rev}`, body => {
t.match(body, {
Expand Down Expand Up @@ -603,7 +603,7 @@ t.test('workspaces', async t => {
name: 'workspace-a',
packuments: [{ maintainers, version: '1.0.0' }],
})
registry.package({ manifest })
await registry.package({ manifest })
registry.couchuser({ username })
registry.nock.put(`/workspace-a/-rev/${manifest._rev}`, body => {
t.match(body, {
Expand Down Expand Up @@ -649,7 +649,7 @@ t.test('completion', async t => {
name: packageName,
packuments: [{ maintainers, version: '1.0.0' }],
})
registry.package({ manifest })
await registry.package({ manifest })
const res = await owner.completion({ conf: { argv: { remain: ['npm', 'owner', 'rm'] } } })
t.strictSame(res, maintainers.map(m => m.name), 'should return list of current owners')
})
Expand Down Expand Up @@ -683,7 +683,7 @@ t.test('completion', async t => {
name: packageName,
packuments: [{ maintainers: [], version: '1.0.0' }],
})
registry.package({ manifest })
await registry.package({ manifest })

const res = await owner.completion({ conf: { argv: { remain: ['npm', 'owner', 'rm'] } } })
t.strictSame(res, [], 'should return no owners if not found')
Expand Down
Loading