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: improve login/connect flow #616

Merged
merged 2 commits into from
Jan 14, 2025
Merged

fix: improve login/connect flow #616

merged 2 commits into from
Jan 14, 2025

Conversation

janb87
Copy link
Contributor

@janb87 janb87 commented Jan 14, 2025

Summary by Sourcery

Enhancements:

  • Use a consistent default instance across login and connect prompts.

Verified

This commit was signed with the committer’s verified signature.
janb87 Jan Bevers
@janb87 janb87 requested a review from roderik January 14, 2025 12:41
Copy link
Contributor

sourcery-ai bot commented Jan 14, 2025

Reviewer's Guide by Sourcery

This pull request improves the login/connect flow by prioritizing the instance used for login when prompting the user for an instance URL.

Sequence diagram for improved login/connect flow

Loading
sequenceDiagram
    actor User
    participant CLI
    participant InstancePrompt
    participant EnvLoader

    User->>CLI: Initiates login/connect
    CLI->>EnvLoader: loadEnv()
    CLI->>InstancePrompt: instancePrompt()
    InstancePrompt->>InstancePrompt: getDefaultInstance()
    alt Has SETTLEMINT_INSTANCE env var
        InstancePrompt-->>CLI: Return env instance
    else No env var but has default login instance
        InstancePrompt-->>CLI: Return default login instance
    else No defaults
        InstancePrompt->>User: Prompt for instance URL
        User->>InstancePrompt: Provide URL
        InstancePrompt-->>CLI: Return provided URL
    end
    CLI-->>User: Continue with login process

Flow diagram for instance URL resolution

Loading
flowchart TD
    A[Start] --> B{Has SETTLEMINT_INSTANCE?}
    B -->|Yes| C[Use env instance]
    B -->|No| D{Has default login instance?}
    D -->|Yes| E[Use default login instance]
    D -->|No| F{Free text input enabled?}
    F -->|Yes| G[Show text input prompt]
    F -->|No| H[Show instance selection list]
    G --> I[Validate URL]
    H --> I
    I --> J[Return instance URL]

File-Level Changes

Change Details Files
Prioritize login instance for default prompt value
  • Use the login instance as the default value when prompting for the SettleMint instance URL.
  • Remove validation of the instance URL in the login command since it is now handled in the instance prompt.
  • Use a default instance URL if available from environment variables or known instances, or from the login process as a fallback.
  • Update the instance prompt to use the default instance URL if available, or "https://console.settlemint.com" as a last resort.
sdk/cli/src/commands/connect/instance.prompt.ts
sdk/cli/src/commands/login.ts

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@github-actions github-actions bot added the fix label Jan 14, 2025
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @janb87 - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟡 Review instructions: 2 issues found
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Verified

This commit was signed with the committer’s verified signature.
janb87 Jan Bevers
@janb87 janb87 enabled auto-merge (squash) January 14, 2025 13:10
Copy link

📦 Packages

Package NPM Docker
SDK Cli @settlemint/sdk-cli@1.0.0-pr895881b7
SDK The Graph @settlemint/sdk-thegraph@1.0.0-pr895881b7
SDK Portal @settlemint/sdk-portal@1.0.0-pr895881b7
SDK Hasura @settlemint/sdk-hasura@1.0.0-pr895881b7
SDK JS @settlemint/sdk-js@1.0.0-pr895881b7
SDK Utils @settlemint/sdk-utils@1.0.0-pr895881b7
SDK Next @settlemint/sdk-next@1.0.0-pr895881b7
SDK Minio @settlemint/sdk-minio@1.0.0-pr895881b7
SDK IPFS @settlemint/sdk-ipfs@1.0.0-pr895881b7
SDK Blockscout @settlemint/sdk-blockscout@1.0.0-pr895881b7

@janb87 janb87 merged commit fa7f92b into main Jan 14, 2025
2 checks passed
@janb87 janb87 deleted the jan/improve-login-flow branch January 14, 2025 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants