Skip to content

feat: CategoryHeader in Notifications screen#31817

Draft
kirillzyusko wants to merge 6 commits into
mainfrom
feat/category-header-in-notifications-tab
Draft

feat: CategoryHeader in Notifications screen#31817
kirillzyusko wants to merge 6 commits into
mainfrom
feat/category-header-in-notifications-tab

Conversation

@kirillzyusko

@kirillzyusko kirillzyusko commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Description

Use BE driven config for displaying Notification pages dynamically. Next time when new category gets introduced only Be should make changes to support it. No client code changes are required.

Changelog

CHANGELOG entry: use BE driven UI for notifications pages

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/GE-247 https://consensyssoftware.atlassian.net/browse/GE-332

Manual testing steps

Feature: BE driven Notifications UI

  Scenario: user opes the app
    Given user is landed on Notifications or Notifications settings screen

    When user opens these pages
    Then they should work as before (but should show skeletons if data is laoding from BE + BE should control amount of shown categories)

Screenshots/Recordings

Before

After

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
    • Ideally on a mid-range device; emulator is acceptable
  • I've tested with a power user scenario
    • Use these power-user SRPs to import wallets with many accounts and tokens
  • I've instrumented key operations with Sentry traces for production performance metrics

For performance guidelines and tooling, see the Performance Guide.

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@kirillzyusko kirillzyusko self-assigned this Jun 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@github-actions github-actions Bot added the pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. label Jun 16, 2026
@metamask-ci

metamask-ci Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

PR template — items to address before "Ready for review"

Warnings — informational, address before merging:

  • Screenshots/Recordings section is empty. Add an image/video for user-facing changes, logs/console output for non-user-facing changes, or write N/A if no evidence is applicable.

See docs/readme/ready-for-review.md for the full Definition of Ready for Review.

@kirillzyusko kirillzyusko force-pushed the feat/category-header-in-notifications-tab branch from 8ed5d26 to 9b4f484 Compare July 13, 2026 13:18
@github-actions github-actions Bot added size-XL and removed size-M labels Jul 13, 2026
@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

🧪 Flaky unit test detection

Run history flaky detection

View recent run history

Historical failure rate is a hint, not proof — review each suggestion in context. See the flaky-test-detection skill for the full pattern reference and manual audit workflow.

Failures / runs sampled per window:

File 7d 15d 30d
app/components/Views/Notifications/index.test.tsx 0/81 0/186 0/359
app/components/Views/SocialLeaderboard/TopTradersView/TopTradersView.test.tsx 0/81 0/186 0/359

AI-detected flaky patterns

app/components/Views/Notifications/index.test.tsx

  • J1 — Missing act() around async state updates (critical)
    • This test triggers a button press via fireEvent without wrapping it in act() (and the test is not marked async). Other tests in the same file correctly use 'await act(() => fireEvent.press(...))' for navigation/state updates. Matches J1 exactly; can cause intermittent failures from unbatched React updates. No historical flakiness (0 failures in flaky-history.json).
    • Suggested fix in app/components/Views/Notifications/index.test.tsx:
      -  it('shows enable prompt and opens settings when notifications are disabled', () => {
      -    const { getByText, getByTestId } = renderWithProvider(
      -      <NotificationsView navigation={navigationMock}>,
      -      { state: mockNotificationsDisabledState },
      -    );
      -
      -    expect(
      -      getByTestId(
      -        NotificationsViewSelectorsIDs.DISABLED_NOTIFICATIONS_CONTAINER,
      -      ),
      -    ).toBeOnTheScreen();
      -    expect(
      -      getByText(strings('notifications.disabled.title')),
      -    ).toBeOnTheScreen();
      -    expect(
      -      getByText(strings('notifications.disabled.message')),
      -    ).toBeOnTheScreen();
      -
      -    fireEvent.press(
      -      getByTestId(NotificationsViewSelectorsIDs.ENABLE_NOTIFICATIONS_BUTTON),
      -    );
      -
      -    expect(navigationMock.navigate).toHaveBeenCalledWith(
      -      Routes.SETTINGS.NOTIFICATIONS,
      -    );
      -  });
      +  it('shows enable prompt and opens settings when notifications are disabled', async () => {
      +    const { getByText, getByTestId } = renderWithProvider(
      +      <NotificationsView navigation={navigationMock}>,
      +      { state: mockNotificationsDisabledState },
      +    );
      +
      +    expect(
      +      getByTestId(
      +        NotificationsViewSelectorsIDs.DISABLED_NOTIFICATIONS_CONTAINER,
      +      ),
      +    ).toBeOnTheScreen();
      +    expect(
      +      getByText(strings('notifications.disabled.title')),
      +    ).toBeOnTheScreen();
      +    expect(
      +      getByText(strings('notifications.disabled.message')),
      +    ).toBeOnTheScreen();
      +
      +    await act(() =>
      +      fireEvent.press(
      +        getByTestId(NotificationsViewSelectorsIDs.ENABLE_NOTIFICATIONS_BUTTON),
      +      ),
      +    );
      +
      +    expect(navigationMock.navigate).toHaveBeenCalledWith(
      +      Routes.SETTINGS.NOTIFICATIONS,
      +    );
      +  });
  • J10 — jest.spyOn() without restoreAllMocks()/mockRestore() afterward (medium)
    • arrangeMocks() (used in the useMarkAsReadCallback tests) performs two jest.spyOn calls (on useMarkNotificationAsRead and setBadgeCount) but the describe only has beforeEach(clearAllMocks) with no afterEach(restoreAllMocks). Spies can leak across tests. Matches J10 (and J3 is already partially addressed by clearAllMocks). No historical flakiness.
    • Suggested fix in app/components/Views/Notifications/index.test.tsx:
      -  beforeEach(() => {
      -    jest.clearAllMocks();
      -  });
      +  beforeEach(() => {
      +    jest.clearAllMocks();
      +  });
      +
      +  afterEach(() => {
      +    jest.restoreAllMocks();
      +  });
  • J7 — Non-deterministic data (medium)
    • arrangeNotifications helper (used by useNotificationFilters tests) sets createdAt using live Date.now(), introducing non-determinism (J7). While not directly asserted in these tests, it matches the pattern of unstubbed Date.now() in test data. Other files had no such patterns; first and third test files properly reset module lets in beforeEach, use clearAllMocks, have no timers/waitFor/act issues or spies.
    • Suggested fix in app/components/Views/Notifications/index.test.tsx:
      -      notif.createdAt = Date.now().toString();
      +      notif.createdAt = '1704067200000';

app/components/Views/SocialLeaderboard/TopTradersView/TopTradersView.test.tsx

  • J9 — Module-level mutable let binding not reset in beforeEach (high)
    • The module-level let mockNotificationPreferences is mutated directly in several tests inside the trading signals setup intercept describe block (e.g. mockNotificationPreferences = channelsDisabledPreferences). The beforeEach at the top of the outer describe('TopTradersView') calls jest.clearAllMocks() and resetTabResults(), but it does NOT reset mockNotificationPreferences back to its initial value. If a test that sets mockNotificationPreferences = channelsDisabledPreferences runs before a test that expects the default preferences, the latter will see the wrong value and fail intermittently depending on test execution order.
    • Suggested fix in app/components/Views/SocialLeaderboard/TopTradersView/TopTradersView.test.tsx:48:
      -let mockNotificationPreferences = {
      -  ...DEFAULT_SOCIAL_AI_PREFERENCES,
      -  mutedTraderProfileIds: [
      -    ...DEFAULT_SOCIAL_AI_PREFERENCES.mutedTraderProfileIds,
      -    // ...
      +const defaultNotificationPreferences = {
      +  ...DEFAULT_SOCIAL_AI_PREFERENCES,
      +  mutedTraderProfileIds: [
      +    ...DEFAULT_SOCIAL_AI_PREFERENCES.mutedTraderProfileIds,
      +  ],
      +};
      +let mockNotificationPreferences = { ...defaultNotificationPreferences };
      +
      +// Inside the outer describe('TopTradersView') beforeEach:
      +beforeEach(() => {
      +  jest.clearAllMocks();
      +  resetTabResults();
      +  mockNotificationPreferences = { ...defaultNotificationPreferences }; // ← add this line
      +  mockUseTopTradersHook.mockImplementation(
      +    (options?: UseTopTradersHookOptions) =>
      +      // ...
      +  );
      +});
  • J8 — jest.useFakeTimers() combined with waitFor / missing cleanup (high)
    • jest.useFakeTimers() is called inside a single test in the performance describe block, but there is no afterEach(() => { jest.useRealTimers(); }) in that describe block (or the outer one). Fake timers installed in one test leak into all subsequent tests in the file. Any test that runs after this one and relies on real timer behavior (e.g. waitFor polling, Promise microtask flushing, or setTimeout-based debouncing in the component) will hang or time out intermittently, depending on test execution order.
    • Suggested fix in app/components/Views/SocialLeaderboard/TopTradersView/TopTradersView.test.tsx:793:
      -    it('keeps a stable renderItem reference across a parent re-render with unchanged trader props', async () => {
      -      jest.useFakeTimers();
      -      mockRefresh.mockResolvedValue(undefined);
      -      const { UNSAFE_getByType } = renderWithProvider(<TopTradersView />);
      -      const renderItemBefore = UNSAFE_getByType(FlatList).props.renderItem;
      -      let refreshPromise: Promise<void> | undefined;
      -      act(() => {
      -        refreshPromise = screen
      -          .getByTestId(TopTradersViewSelectorsIDs.TRADER_LIST)
      -    // ...
      -      const renderItemDuringRefresh =
      -        UNSAFE_getByType(FlatList).props.renderItem;
      -      expect(typeof renderItemBefore).toBe('function');
      -      expect(renderItemDuringRefresh).toBe(renderItemBefore);
      -      await act(async () => {
      -    // ...
      +  describe('performance', () => {
      +    afterEach(() => {
      +      jest.useRealTimers();
      +    });
      +
      +    it('limits the initial trader row render batch during screen mount', () => {
      +      // ... unchanged
      +    });
      +
      +    it('keeps a stable renderItem reference across a parent re-render with unchanged trader props', async () => {
      +      jest.useFakeTimers();
      +      // ... rest of test unchanged
      +    });
      +  });

This check is informational only and does not block merging.

@kirillzyusko kirillzyusko removed the pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. label Jul 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeNetworkAbstractions
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: medium
  • AI Confidence: 80%
click to see 🤖 AI reasoning details

E2E Test Selection:
The PR introduces a new NotificationsCategory filter component in the Notifications view and refactors the NotificationsSettings screen to use a dynamic, API-driven category system. Key changes include:

  1. Notifications View: A new NotificationsCategory horizontal filter bar is added above the notification list. This changes the UI layout of the notifications screen, which could affect the existing notification E2E test that navigates to and interacts with the notifications view.

  2. Navigation param change: NotificationSettingsSection now receives categoryId + ausKeys instead of type. The TopTradersView was updated accordingly. Any E2E test navigating to the notification settings section screen would need to use the new params.

  3. The existing notification smoke test (tests/smoke/notifications/enable-notifications-after-onboarding.spec.ts) is tagged with SmokeNetworkAbstractions and tests the notifications view. The UI changes (new category filter bar) could affect element visibility/scrolling in this test.

  4. Locales: Added all_title key across all language files — cosmetic/additive, no functional impact on tests.

  5. No other smoke tags are directly impacted. The changes are scoped to the notifications feature area. No confirmations, swap, stake, browser, snaps, or other flows are affected.

SmokeNetworkAbstractions is selected because the existing notification E2E test uses this tag and the UI changes to the notifications view could affect test behavior.

Performance Test Selection:
The changes are limited to the Notifications view and Settings screens. No performance-sensitive flows (app launch, login, onboarding, swaps, asset loading, account list, perps, predictions) are affected. The new category filter uses a mocked API with a 100ms timeout, but this is not measured by any performance test scenario. No performance test tags are warranted.

View GitHub Actions results

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
7.1% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant