[PLA-1911] improve collection tracking check

[zlayine]

PR Type

Bug fix

---

Description

• Improved the logic for tracking collections by consolidating and enhancing account checks.
• Added user account and daemon account to the list of accounts to be checked for tracking.
• Ensured that only unique accounts are considered when determining if a collection is tracked.

---

Changes walkthrough 📝

	Relevant files
Bug fix	collection.ts Refactor and enhance collection tracking logic                      resources/js/factory/collection.ts Improved logic for tracking collections by consolidating account checks. Added user account and daemon account to the list of accounts to check. Ensured unique accounts are considered for tracking. +16/-13

---

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

[github-actions]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Possible Bug The accounts array is being modified by pushing new elements within conditionals, but it was initially declared as a constant. This might lead to unexpected behavior or errors. Consider declaring accounts with let if modification is intended. Code Clarity The logic for adding accounts and checking unique accounts could be simplified or broken down into smaller functions for better readability and maintainability.

[github-actions]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Add checks for collection.owner.account.publicKey before using it The current implementation of checking if an account is tracked might not handle cases where publicKeyToAddress(collection.owner.account.publicKey) returns undefined or an unexpected value. It's safer to explicitly check for the existence and type of collection.owner.account.publicKey before converting it. resources/js/factory/collection.ts [28] -tracked = !uniqueAccounts.find((account) => account === publicKeyToAddress(collection.owner.account.publicKey)); +const ownerPublicKey = collection.owner.account.publicKey; +tracked = ownerPublicKey && !uniqueAccounts.find((account) => account === publicKeyToAddress(ownerPublicKey)); Suggestion importance[1-10]: 9 Why: This suggestion adds a necessary check to prevent potential runtime errors if publicKeyToAddress returns undefined or an unexpected value, which is crucial for robustness.	9
Best practice	Use a new array for account aggregation to ensure immutability The accounts.push(...) operations inside the conditional blocks can lead to potential side effects if accounts is referenced elsewhere in the code after these modifications. To ensure immutability and prevent unintended side effects, consider using a new array to gather all accounts and then spread them into uniqueAccounts. resources/js/factory/collection.ts [12-22] +let allAccounts = [...accounts]; if (appStore.user?.account) { - accounts.push(publicKeyToAddress(appStore.user?.account)); + allAccounts.push(publicKeyToAddress(appStore.user?.account)); } ... if (connectionStore.accounts?.length) { const walletAccounts = connectionStore.accounts.map((account) => publicKeyToAddress(account.address)); - accounts.push(...walletAccounts); + allAccounts.push(...walletAccounts); } +const uniqueAccounts = [...new Set(allAccounts)]; Suggestion importance[1-10]: 8 Why: This suggestion ensures immutability and prevents unintended side effects, which is a good practice in functional programming. It addresses a potential issue where the original accounts array could be modified unintentionally.	8
Performance	Cache results of publicKeyToAddress to improve performance The use of publicKeyToAddress function multiple times for the same values suggests that caching the results could improve performance, especially if this function is computationally expensive or called frequently in loops. resources/js/factory/collection.ts [12-28] -if (appStore.user?.account) { - accounts.push(publicKeyToAddress(appStore.user?.account)); +const userAccountAddress = appStore.user?.account ? publicKeyToAddress(appStore.user?.account) : null; +if (userAccountAddress) { + accounts.push(userAccountAddress); } ... -tracked = !uniqueAccounts.find((account) => account === publicKeyToAddress(collection.owner.account.publicKey)); +const ownerAccountAddress = publicKeyToAddress(collection.owner.account.publicKey); +tracked = !uniqueAccounts.find((account) => account === ownerAccountAddress); Suggestion importance[1-10]: 7 Why: Caching the results of publicKeyToAddress can improve performance, especially if the function is computationally expensive. This is a useful optimization but not critical.	7
	Use concat() instead of the spread operator for potentially large arrays The use of spread operator ... in accounts.push(...walletAccounts) can lead to performance issues if walletAccounts is large. Consider using Array.prototype.concat() for better performance in handling large arrays. resources/js/factory/collection.ts [21-22] const walletAccounts = connectionStore.accounts.map((account) => publicKeyToAddress(account.address)); -accounts.push(...walletAccounts); +accounts = accounts.concat(walletAccounts); Suggestion importance[1-10]: 6 Why: Using concat() can be more performant than the spread operator for large arrays. This is a minor optimization that can be beneficial in specific scenarios.	6