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

feat: prompt for login any time a user omits API key #636

Merged

Conversation

kanadgupta
Copy link
Member

@kanadgupta kanadgupta commented Oct 12, 2022

🚥 Fix RM-5540

🧰 Changes

This updates all of our commands so that when they're run locally and the user omits the --key parameter (and they're running the command from a non-CI environment), we now prompt the user to log in, rather than just erroring out. Most of this diff is just a super tiny refactor to all commands and then refactoring a bunch of tests to account for this change.

Here's what it looks like when a user omits the --key flag and attempts to run a command that requires authentication:

Screen.Recording.2022-10-12.at.6.05.13.PM.mov

Also did a tiny bit of test housekeeping:

  • Added some nock cleanup logic to our docs:edit tests to be consistent with every other test suite 🧹
  • Stricter login command nocks 💪🏽

🧬 QA & Testing

Our testbed covers this, but if you want to test this out, you can check out this branch and run this flow of commands:

# Set up rdme
npm ci && npm run build
# Just in case...
bin/rdme logout
# Now try running an authenticated command and follow the prompts
bin/rdme openapi

@kanadgupta kanadgupta added this to the v8 milestone Oct 12, 2022
@kanadgupta kanadgupta added enhancement New feature or request command:docs Issues pertaining to the `docs`, `changelogs`, or `custompages` commands command:openapi Issues pertaining to the `openapi`, `validate`, `reduce`, or `swagger` commands command:versions Issues pertaining to the `versions` commands command:categories Issues pertaining to the `categories` commands command:other Issues pertaining to miscellaneous commands (e.g. `open`, `logout`) or requests for new commands labels Oct 12, 2022
@kanadgupta kanadgupta marked this pull request as ready for review October 12, 2022 23:12
@kanadgupta kanadgupta requested review from erunion, a team, dannobytes, trishaprile and darrenyong and removed request for a team October 12, 2022 23:12
Copy link
Member

@erunion erunion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well shit, you made this look easy. we should've had these commands do this this whole time.

src/lib/baseCommand.ts Outdated Show resolved Hide resolved
src/lib/baseCommand.ts Outdated Show resolved Hide resolved
src/lib/baseCommand.ts Outdated Show resolved Hide resolved
__tests__/cmds/categories/index.test.ts Show resolved Hide resolved
__tests__/cmds/login.test.ts Show resolved Hide resolved
kanadgupta and others added 3 commits October 13, 2022 09:40
TS handles this now
... and refactor console.info statements to use that instead
@kanadgupta kanadgupta merged commit 542307b into main Oct 13, 2022
@kanadgupta kanadgupta deleted the kanad/rm-5540-if-user-omits-key-prompt-them-for-login branch October 13, 2022 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
command:categories Issues pertaining to the `categories` commands command:docs Issues pertaining to the `docs`, `changelogs`, or `custompages` commands command:openapi Issues pertaining to the `openapi`, `validate`, `reduce`, or `swagger` commands command:other Issues pertaining to miscellaneous commands (e.g. `open`, `logout`) or requests for new commands command:versions Issues pertaining to the `versions` commands enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants