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 tracking with daemon account #151

Merged
merged 1 commit into from
Aug 9, 2024

Conversation

zlayine
Copy link
Contributor

@zlayine zlayine commented Aug 9, 2024

PR Type

Bug fix


Description

  • Added a condition to check appStore.isMultiTenant before pushing the daemon account in DTOCollectionFactory.
  • This fix ensures that the daemon account is not tracked in multi-tenant setups.

Changes walkthrough 📝

Relevant files
Bug fix
collection.ts
Fix daemon account tracking for multi-tenant setups           

resources/js/factory/collection.ts

  • Added a condition to check appStore.isMultiTenant before pushing the
    daemon account.
  • +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

    github-actions bot commented Aug 9, 2024

    PR Reviewer Guide 🔍

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

    Logic Check
    Ensure that the condition !appStore.isMultiTenant correctly handles all expected scenarios for multi-tenant and single-tenant configurations. This is crucial as it affects the tracking of daemon accounts.

    Copy link

    github-actions bot commented Aug 9, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Add a check to ensure appStore.config.daemon is valid before using it

    Consider checking for the existence of appStore.config.daemon before pushing its
    address to the accounts array. This ensures that the publicKeyToAddress function
    does not receive an undefined or null value, which could lead to runtime errors.

    resources/js/factory/collection.ts [16-17]

    -if (appStore.config.daemon && !appStore.isMultiTenant) {
    +if (appStore.config.daemon && publicKeyToAddress(appStore.config.daemon) && !appStore.isMultiTenant) {
         accounts.push(publicKeyToAddress(appStore.config.daemon));
     }
     
    Suggestion importance[1-10]: 9

    Why: The suggestion correctly identifies a potential runtime error by ensuring appStore.config.daemon is valid before using it. This is a crucial check that improves code robustness and prevents possible bugs.

    9

    @zlayine zlayine merged commit 20d4c8a into master Aug 9, 2024
    3 checks passed
    @zlayine zlayine deleted the hotfix/pla-1911/tracking-daemon branch August 9, 2024 10:39
    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