test: Controller Guidelines Violations#7
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| this.store.updateState({ | ||
| onboardingDate: Date.now(), | ||
| }); | ||
| this.store.state.onboardingDate = Date.now(); |
There was a problem hiding this comment.
Direct state mutation bypasses observable store pattern
High Severity
The setOnboardingDate and updateRecoveryPhraseTimestamp methods directly mutate this.store.state instead of using this.store.updateState(). This bypasses the ObservableStore pattern and prevents state change events from being emitted, breaking any subscribers listening for state updates.
Additional Locations (1)
| // NOTE: No need to track the account here, since we start tracking those when | ||
| // the "AccountsController:accountAdded" is fired. | ||
|
|
||
| this.state.balances[accountId] = { amount: '0', unit: 'WEI' }; |
There was a problem hiding this comment.
Direct state mutation with wrong type structure
High Severity
The updateBalance method directly mutates this.state.balances[accountId] instead of using the update() method, and assigns an object with incorrect structure. The type expects { [asset: string]: { amount, unit } } but receives { amount: '0', unit: 'WEI' } directly, missing the required asset key nesting level.
| setUseBlockie(val: boolean): void { | ||
| this.update((state) => { | ||
| state.useBlockie = val; | ||
| state.useNonceField = false; |
There was a problem hiding this comment.
Unrelated side effect in setUseBlockie setter
Medium Severity
The setUseBlockie method now sets state.useNonceField = false as a side effect. Setting the blockie avatar preference unexpectedly resets the nonce field preference, which are unrelated user settings. This will silently disable the nonce field feature whenever a user changes their avatar style.
| unconnectedAccountAlertShownOrigins: Record<string, boolean>; | ||
| web3ShimUsageOrigins?: Record<string, number>; | ||
| totalAlertsCount: number; | ||
| hasAnyAlertsEnabled: boolean; |
There was a problem hiding this comment.
Missing state metadata for new AlertController properties
Medium Severity
The new state properties totalAlertsCount and hasAnyAlertsEnabled are added to the state type and default state but are missing from controllerMetadata. The BaseController requires all state properties to have corresponding metadata entries defining their persist and anonymous flags.
| } | ||
| >; | ||
| totalEventsCount: number; | ||
| isMarketingEnabled: boolean; |
There was a problem hiding this comment.
Missing metadata and defaults for MetaMetrics state
Medium Severity
The new state properties totalEventsCount and isMarketingEnabled are added to MetaMetricsControllerState but are missing from both controllerMetadata and getDefaultMetaMetricsControllerState(). This leaves these fields undefined on initialization and without persistence/anonymity configuration.
| unconnectedAccountAlertShownOrigins: Record<string, boolean>; | ||
| web3ShimUsageOrigins?: Record<string, number>; | ||
| totalAlertsCount: number; | ||
| hasAnyAlertsEnabled: boolean; |
There was a problem hiding this comment.
Derived state property never updated when alerts change
Medium Severity
The hasAnyAlertsEnabled state property is initialized to true but is never updated when setAlertEnabledness modifies individual alert states. This means hasAnyAlertsEnabled will permanently remain true even after a user disables all alerts, causing any code that relies on this property to receive incorrect results.
rvelaz
changed the title
rvelaz
changed the title
Updated:
- **Core mission statement**: Clear definition of BUGBOT's purpose
- **Execution protocol sections** for each guideline domain:
- General coding guidelines (always applied)
- Unit testing (pattern: `*.test.*`)
- E2E testing (pattern: `test/e2e/**/*.spec.{ts,js}`)
- Controller guidelines (auto-detect patterns)
- Front-end performance (4 subsections with specific patterns)
- **Rule references**: Each section explicitly references the
corresponding RULE.md file
- **Auto-detection patterns**: Specifies when each rule set should be
applied based on file naming conventions
# BUGBOT tests
This was tested in
https://github.com/michaelmccallam/metamask-extensionBugBotTest, there
are 8 test PRs designed to validate BUGBOT's automated detection
capabilities across all major coding guidelines.
- **[#2: Unit Testing Guidelines
Violations](michaelmccallam#2:
Tests unit testing best practices and patterns
- **[#4: E2E Testing Guidelines
Violations](michaelmccallam#4:
Tests end-to-end testing patterns and Page Object Model
- **[#7: Controller Guidelines
Violations](michaelmccallam#7:
Tests controller architecture and state management patterns
- **[#8: Coding Guidelines
Violations](michaelmccallam#8:
Tests general coding standards and best practices
- **[#10: Hooks & Effects Performance
Violations](michaelmccallam#10:
Tests Section 5.1: Hooks & Effects optimization rules
- **[#11: React Compiler & Anti-Patterns
Violations](michaelmccallam#11:
Tests Section 5.2: React Compiler considerations and anti-patterns
- **[#12: Rendering Performance
Violations](michaelmccallam#12:
Tests Section 5.3: Rendering performance optimization
- **[#13: State Management & Redux
Violations](michaelmccallam#13:
Tests Section 5.4: Redux and state management patterns
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> Modernizes BUGBOT documentation and rule configuration.
>
> - Replaces `BUGBOT.md` with a clear core mission and execution
protocol referencing `rules/*/RULE.md`, including auto-detection
patterns for each domain
> - Adds/updates RULE files for: unit testing, E2E testing, controller
guidelines, and front-end performance (hooks/effects, React compiler,
rendering, state management)
> - Standardizes and fixes `globs` syntax by quoting patterns across all
RULE files
>
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
745a1e9. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
Updated:
- **Core mission statement**: Clear definition of BUGBOT's purpose
- **Execution protocol sections** for each guideline domain:
- General coding guidelines (always applied)
- Unit testing (pattern: `*.test.*`)
- E2E testing (pattern: `test/e2e/**/*.spec.{ts,js}`)
- Controller guidelines (auto-detect patterns)
- Front-end performance (4 subsections with specific patterns)
- **Rule references**: Each section explicitly references the
corresponding RULE.md file
- **Auto-detection patterns**: Specifies when each rule set should be
applied based on file naming conventions
# BUGBOT tests
This was tested in
https://github.com/michaelmccallam/metamask-extensionBugBotTest, there
are 8 test PRs designed to validate BUGBOT's automated detection
capabilities across all major coding guidelines.
- **[#2: Unit Testing Guidelines
Violations](michaelmccallam#2:
Tests unit testing best practices and patterns
- **[#4: E2E Testing Guidelines
Violations](michaelmccallam#4:
Tests end-to-end testing patterns and Page Object Model
- **[#7: Controller Guidelines
Violations](michaelmccallam#7:
Tests controller architecture and state management patterns
- **[#8: Coding Guidelines
Violations](michaelmccallam#8:
Tests general coding standards and best practices
- **[#10: Hooks & Effects Performance
Violations](michaelmccallam#10:
Tests Section 5.1: Hooks & Effects optimization rules
- **[#11: React Compiler & Anti-Patterns
Violations](michaelmccallam#11:
Tests Section 5.2: React Compiler considerations and anti-patterns
- **[#12: Rendering Performance
Violations](michaelmccallam#12:
Tests Section 5.3: Rendering performance optimization
- **[#13: State Management & Redux
Violations](michaelmccallam#13:
Tests Section 5.4: Redux and state management patterns
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> Modernizes BUGBOT documentation and rule configuration.
>
> - Replaces `BUGBOT.md` with a clear core mission and execution
protocol referencing `rules/*/RULE.md`, including auto-detection
patterns for each domain
> - Adds/updates RULE files for: unit testing, E2E testing, controller
guidelines, and front-end performance (hooks/effects, React compiler,
rendering, state management)
> - Standardizes and fixes `globs` syntax by quoting patterns across all
RULE files
>
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
745a1e9. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
|
This PR has been automatically marked as stale because it has not had recent activity in the last 60 days. It will be closed in 14 days. Thank you for your contributions. |
1 participant
Description
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Adds lightweight state fields and helpers to improve introspection and status handling across controllers.
lastPollingTimeto state/metadata/defaults; newgetTrackedAccountCount().totalAlertsCount,hasAnyAlertsEnabledto state/defaults; newsetTotalAlertsCount()andgetEnabledAlertsCount().setOnboardingDate()to mutatestore.statedirectly; newupdateRecoveryPhraseTimestamp().lastStatusCheckTimeto metadata.onQuoteUpdate; addssetQuotesLoadingStatus(); minor quote-loading state updates.totalEventsCount,isMarketingEnabledto state; newsetTotalEventsCount()andgetIsMarketingEnabled().legacyConfigand optionalonCustodyStatusChange.setUseBlockie()now also clearsuseNonceField; addssetCurrentLocale()andgetHasAnySecurityEnabled().lastFetchTimestamp(persisted) to state/metadata/defaults;updateBalance()initializes placeholder balance; newgetTrackedAccountsCount().Written by Cursor Bugbot for commit 6bdc303. This will update automatically on new commits. Configure here.