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

Improve logs in the login command #1028

Merged
merged 47 commits into from
Jun 30, 2023
Merged

Improve logs in the login command #1028

merged 47 commits into from
Jun 30, 2023

Conversation

frandiox
Copy link
Contributor

@frandiox frandiox commented Jun 22, 2023

Builds on top of #1023

Cli-kit does not provide a way to change the logs of the authentication features, and it probably would take a while to refactor that since the logs are added in very deep functions.
For now, this should do the job. It's replacing the default logs with renderTasks and so on.

🎩 :

  • Try h2 logout; h2 login and do the normal flow.
  • Try h2 logout; h2login again but this time open the browser with a keypress and don't login. Return to the terminal and wait for 10 seconds until the timeout happens. Click the Open link.

Gotchas:

  • We can't show the link at the beginning because it's not exposed from cli-kit until the timeout.
  • We can't move the "Open" link after the timeout inline, it wraps to the next line due to some kind of bug (I think?).

First screen:

image

After pressing a key:

image

After timeout:

image

After successful login and selecting 1 shop:

image

Checklist

  • I've read the Contributing Guidelines
  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've added a changeset if this PR contains user-facing or noteworthy changes
  • I've added tests to cover my changes
  • I've added or updated the documentation

@frandiox frandiox requested review from graygilmore and a team June 22, 2023 15:47
@github-actions
Copy link
Contributor

We detected some changes in packages/*/package.json or packages/*/src, and there are no updates in the .changeset.
If the changes are user-facing and should cause a version bump, run npm run changeset add to track your changes and include them in the next release CHANGELOG.
If you are making simple updates to examples or documentation, you do not need to add a changeset.

@frandiox
Copy link
Contributor Author

@graygilmore @blittle I've updated the OP with new images. Now we only show the loader after pressing a key, so the "press a key" message should be more visible.

Base automatically changed from fd-login-command-improved to 2023-04 June 28, 2023 15:08
@frandiox
Copy link
Contributor Author

Updated to stable cli-kit. Please review 🙌

Copy link
Contributor

@graygilmore graygilmore left a comment

Choose a reason for hiding this comment

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

👍🏻 auth logs looked good in my testing!

@frandiox frandiox merged commit e11fcff into 2023-04 Jun 30, 2023
@frandiox frandiox deleted the fd-login-logs branch June 30, 2023 01:45
juanpprieto pushed a commit that referenced this pull request Jul 10, 2023
* Add login and logout commands

* Refactor graphql client and requests, add unit tests

* Update shopify-config methods and tests

* Update renderErrors to use cli Command

* Rework env-pull to call login and remove silent param

* Rework link command to call login and remove --shop flag

* Update list command to call login and remove --shop flag

* Update env-list command to call login and remove --shop flag

* Oclif manifest

* Cleanup

* Rework log replacer

* Mute cli-kit auth logs

* Fix mock

* Update cli-kit version

* Add GraphQL schema generator for local development

* Use business platform login to render a list of shops

* Improve login logs

* Merge main branch

* Fix tests

* Minor changes

* Update package-lock

* Apply suggestions from code review

Co-authored-by: Gray Gilmore <[email protected]>

* Change link color

* Skip writing config when no directory is passed to login

* Improve unit tests

* Changesets

* Refactor to getUserAccount and add tests

* Store shopName and email in local config

* Show shopName and email after login

* Force prompt during login when not passing --shop flag

* Fix tests

* Only show loader after pressing a key

* Show short version of link in terminal

* Update cli-kit version and its deps

* Cleanup

* Update package-lock

* Remove duplicated code chunk

---------

Co-authored-by: Gray Gilmore <[email protected]>
FrcPpe pushed a commit to FrcPpe/hydrogen that referenced this pull request Aug 13, 2023
* Add login and logout commands

* Refactor graphql client and requests, add unit tests

* Update shopify-config methods and tests

* Update renderErrors to use cli Command

* Rework env-pull to call login and remove silent param

* Rework link command to call login and remove --shop flag

* Update list command to call login and remove --shop flag

* Update env-list command to call login and remove --shop flag

* Oclif manifest

* Cleanup

* Rework log replacer

* Mute cli-kit auth logs

* Fix mock

* Update cli-kit version

* Add GraphQL schema generator for local development

* Use business platform login to render a list of shops

* Improve login logs

* Merge main branch

* Fix tests

* Minor changes

* Update package-lock

* Apply suggestions from code review

Co-authored-by: Gray Gilmore <[email protected]>

* Change link color

* Skip writing config when no directory is passed to login

* Improve unit tests

* Changesets

* Refactor to getUserAccount and add tests

* Store shopName and email in local config

* Show shopName and email after login

* Force prompt during login when not passing --shop flag

* Fix tests

* Only show loader after pressing a key

* Show short version of link in terminal

* Update cli-kit version and its deps

* Cleanup

* Update package-lock

* Remove duplicated code chunk

---------

Co-authored-by: Gray Gilmore <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants