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 callbackUrl for bitte wallet #535

Merged
merged 1 commit into from
Sep 2, 2024
Merged

fix callbackUrl for bitte wallet #535

merged 1 commit into from
Sep 2, 2024

Conversation

rubenmarcus
Copy link
Member

@rubenmarcus rubenmarcus commented Sep 2, 2024

PR Type

Bug fix


Description

  • Added support for bitte-wallet in the callback URL handling logic.
  • Fixed minor formatting issues in the callback URL check conditions.

This adds support for the extra checks using execute method from @mintbase-js/sdk, on bitte-wallet too. it adds back again signMeta params with nfts metadata on it.


Changes walkthrough 📝

Relevant files
Bug fix
checkCallback.ts
Fix callback URL handling for `bitte-wallet`                         

packages/sdk/src/execute/checkCallback.ts

  • Added condition to check for bitte-wallet in addition to
    mintbase-wallet
  • Fixed formatting issues with callback URL checks
  • +3/-3     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @mintbase-codium-pr-agent
    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Logical Error
    The new condition for shouldGetFromMbjs might lead to unexpected behavior as it checks for callbackUrl?.length < 1 or callbackUrl === undefined without proper grouping of conditions. This could result in shouldGetFromMbjs being true in unintended scenarios.

    @rubenmarcus rubenmarcus self-assigned this Sep 2, 2024
    @rubenmarcus rubenmarcus merged commit 0ab4956 into beta Sep 2, 2024
    2 checks passed
    @mintbase-codium-pr-agent
    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Add parentheses to ensure correct evaluation order in logical conditions

    The logical condition in line 23 might not work as expected due to missing
    parentheses around the || condition. This could lead to unexpected behavior when
    checking callbackUrl properties. It's important to ensure that the entire condition
    for callbackUrl being undefined or having a length less than 1 is evaluated
    together.

    packages/sdk/src/execute/checkCallback.ts [23]

    -const shouldGetFromMbjs = callbackUrl?.length < 1 || callbackUrl === undefined &&
    +const shouldGetFromMbjs = (callbackUrl?.length < 1 || callbackUrl === undefined) &&
       window?.['mbjs']?.callbackUrl && window?.['mbjs']?.callbackUrl.length > 0;
     
    Suggestion importance[1-10]: 10

    Why: The suggestion correctly identifies a potential logical error due to operator precedence and provides a fix that ensures the condition is evaluated as intended, preventing unexpected behavior.

    10
    Maintainability
    Simplify the condition by directly checking the property length

    The condition window?.['mbjs']?.callbackUrl && window?.['mbjs']?.callbackUrl.length
    > 0 can be simplified by directly checking the length. This avoids redundancy and
    makes the code more readable.

    packages/sdk/src/execute/checkCallback.ts [24]

    -window?.['mbjs']?.callbackUrl && window?.['mbjs']?.callbackUrl.length > 0;
    +window?.['mbjs']?.callbackUrl?.length > 0;
     
    Suggestion importance[1-10]: 9

    Why: This suggestion improves code readability and maintainability by removing redundancy in the condition check, making it more concise and easier to understand.

    9
    Best practice
    Use strict equality for comparison to avoid type coercion

    The condition checking for the wallet ID should use strict equality (===) instead of
    loose equality (==) to avoid potential type coercion issues which might lead to
    unexpected behavior.

    packages/sdk/src/execute/checkCallback.ts [26]

    -if (wallet?.id == 'mintbase-wallet' || wallet?.id == 'bitte-wallet') {
    +if (wallet?.id === 'mintbase-wallet' || wallet?.id === 'bitte-wallet') {
     
    Suggestion importance[1-10]: 8

    Why: Using strict equality is a best practice to avoid type coercion issues, which can lead to bugs. This suggestion improves code reliability.

    8
    Enhancement
    Simplify the check for undefined or null values

    The condition callbackUrl === undefined can be simplified to use !callbackUrl, which
    checks for both null and undefined, making the code cleaner and potentially catching
    more edge cases where callbackUrl might not be properly initialized.

    packages/sdk/src/execute/checkCallback.ts [27]

    -if (callbackUrl?.length < 1 || callbackUrl === undefined) {
    +if (!callbackUrl || callbackUrl?.length < 1) {
     
    Suggestion importance[1-10]: 7

    Why: The suggestion simplifies the condition, making the code cleaner and potentially more robust by catching more edge cases. However, it slightly changes the logic order, which might not be necessary.

    7

    @rubenmarcus rubenmarcus deleted the callback-url-checker branch September 2, 2024 16:36
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    3 participants