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

[PLA-1911] fix collection tracked #141

Merged
merged 1 commit into from
Jul 29, 2024

Conversation

zlayine
Copy link
Contributor

@zlayine zlayine commented Jul 26, 2024

PR Type

Bug fix


Description

  • Fixed the logic for tracking collections by changing the method from some to find for account comparison.
  • Added conversion of public key to address using publicKeyToAddress function to ensure proper comparison.

Changes walkthrough 📝

Relevant files
Bug fix
collection.ts
Fix account tracking logic in collection factory                 

resources/js/factory/collection.ts

  • Changed some method to find method for account comparison.
  • Utilized publicKeyToAddress function for public key conversion.
  • +1/-1     

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

    Copy link

    PR Reviewer Guide 🔍

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

    Logic Change Verification
    The change from some to find method alters the logic significantly. some returns a boolean if any elements match the condition, whereas find returns the first matching element or undefined. Ensure that the new logic correctly interprets these return values for the intended functionality.

    Copy link

    github-actions bot commented Jul 26, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Use the some method for a more appropriate and efficient boolean existence check

    Replace the find method with some for checking the existence of an element. The some
    method is more appropriate for boolean checks as it returns a boolean directly and
    stops iterating once it finds an element that satisfies the condition.

    resources/js/factory/collection.ts [10]

    -tracked = !accounts.find((account) => account === publicKeyToAddress(collection.owner.account.publicKey));
    +tracked = !accounts.some((account) => account === publicKeyToAddress(collection.owner.account.publicKey));
     
    Suggestion importance[1-10]: 10

    Why: The suggestion is correct and improves the code by using the some method, which is more appropriate for boolean checks and stops iterating once it finds a matching element, making the code more efficient.

    10

    @zlayine zlayine merged commit f0cedde into master Jul 29, 2024
    3 checks passed
    @zlayine zlayine deleted the bugfix/pla-1911/collection-tracking branch July 29, 2024 06:06
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Development

    Successfully merging this pull request may close these issues.

    2 participants