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 login command to show a list of shops #1023

Merged
merged 36 commits into from
Jun 28, 2023

Conversation

frandiox
Copy link
Contributor

Builds on top of #1022
Closes https://github.com/Shopify/core-issues/issues/56398
Blocked by: stable release of [email protected]

WHY are these changes introduced?

We want to show a list of shops available in the user account instead of asking them to type the shop name.

image image

WHAT is this pull request doing?

Uses the new business platform authentication to request the list of shops and renders a select prompt.
It also adds a script to generate business platform GraphQL schema for local development.

HOW to test your changes?

Try h2 logout and h2 login. You should be prompted with a list of shops instead of a text input.

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 11:49
@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.

const CurrentUserAccountQuery = `#graphql
query currentUserAccount {
currentUserAccount {
organizations(first: 100) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not handling pagination at the moment. Showing many shops might not look nice in the terminal... perhaps we should just add a select choice at the end for "Can't see my shop in this list", and then prompt a text input?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I just remembered there's a renderAutocompletePrompt that might be nice for this...

@frandiox frandiox mentioned this pull request Jun 22, 2023
5 tasks
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.

Worked great!

@@ -35,7 +35,7 @@
"@graphql-codegen/cli": "3.3.1",
"@oclif/core": "2.1.4",
"@remix-run/dev": "1.17.1",
"@shopify/cli-kit": "3.46.3",
"@shopify/cli-kit": "3.47.0-pre.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to ship with a -pre?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I mentioned Blocked by: stable release of [email protected] but tbh I think it wouldn't be a problem to release it like this. In any case, will ask the cli-kit folks about 3.47 stable first.

Comment on lines -35 to -39
shop = await renderTextPrompt({
message:
'Specify which Store you would like to use (e.g. {store}.myshopify.com)',
allowEmpty: false,
});
Copy link
Contributor

Choose a reason for hiding this comment

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

🔥

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

frandiox commented Jun 28, 2023

@graygilmore So I've modified a bit the local Shopify config file to store shopName and email. The reason is that we want to show this info in the success banner, and it will also be useful in the onboarding process later.

I've updated to [email protected] stable and I'm merging now to prevent more conflicts, but feel free to check the last changes.

@frandiox frandiox merged commit f6d8030 into 2023-04 Jun 28, 2023
@frandiox frandiox deleted the fd-login-command-improved branch June 28, 2023 15:08
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

* Merge main branch

* Fix tests

* Minor changes

* Update package-lock

* Apply suggestions from code review

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

* 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

* Update cli-kit version and its deps

* Cleanup

---------

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

* Merge main branch

* Fix tests

* Minor changes

* Update package-lock

* Apply suggestions from code review

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

* 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

* Update cli-kit version and its deps

* Cleanup

---------

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.

2 participants