[LWM] feat(notifications): make prompt QA screen readable (LIVE-35955) - #21450
Draft
sarneijim wants to merge 4 commits into
Draft
[LWM] feat(notifications): make prompt QA screen readable (LIVE-35955)#21450sarneijim wants to merge 4 commits into
sarneijim wants to merge 4 commits into
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
live-github-bot
Bot
changed the title
Contributor
Web Tools Build Status
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The QA screen introduces at least one misleading action label and an inspector raw-value mismatch that can confuse QA and should be corrected before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR improves the Ledger Live Mobile Notifications Prompt QA/debug screen so QA can reliably understand and reproduce prompt eligibility outcomes (verdict + reason), apply deterministic scenarios, and preview the drawer UI without impacting production prompt state.
Changes:
- Reworked the Notifications Prompt QA screen UI into “Scenarios” and “Inspect” tabs with plain-English verdicts and readable labels.
- Added deterministic QA scenarios + utilities to map engine decisions to user-facing expectations and reason labels.
- Extracted the drawer content into a reusable
NotificationsPromptDrawerViewand exposed scheduler controls (cancelPendingDrawer, etc.) viaNotificationsPromptProviderso the QA screen can safely reuse the single scheduler.
File summaries
| File | Description |
|---|---|
| apps/ledger-live-mobile/src/screens/Settings/Debug/NotificationsPromptQA/utils.ts | Adds QA scenario definitions, verdict/reason labeling, and helpers to map engine decisions to QA expectations. |
| apps/ledger-live-mobile/src/screens/Settings/Debug/NotificationsPromptQA/index.tsx | Rebuilds the QA screen UX (verdict card, scenarios tab, inspect tab) and adds local drawer preview + reset/apply actions. |
| apps/ledger-live-mobile/src/screens/Settings/Debug/NotificationsPromptQA/tests/utils.test.ts | Adds scenario-based tests ensuring engine outcomes match advertised QA expectations and labels remain readable. |
| apps/ledger-live-mobile/src/mvvm/features/NotificationsPrompt/screens/NotificationsPromptDrawerView.tsx | Extracts reusable drawer UI content so it can be mounted both in production and in the QA preview. |
| apps/ledger-live-mobile/src/mvvm/features/NotificationsPrompt/screens/NotificationsPromptDrawer.tsx | Refactors production drawer to render NotificationsPromptDrawerView. |
| apps/ledger-live-mobile/src/mvvm/features/NotificationsPrompt/new/NotificationsPromptProvider.tsx | Exposes scheduler functions (open/isPending/cancel) through the notifications prompt context. |
| apps/ledger-live-mobile/src/mvvm/features/NotificationsPrompt/new/hooks/useNotificationsPromptTriggers.ts | Plumbs cancelPendingDrawer (and scheduler helpers) up from the scheduler hook. |
| apps/ledger-live-mobile/src/mvvm/features/NotificationsPrompt/new/hooks/useNotificationsPromptDrawerScheduler.ts | Adds cancelPendingDrawer and reuses it from openDrawer and the cleanup effect. |
| .changeset/notifications-prompt-qa-screen.md | Adds a patch changeset describing the QA-screen improvements. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+701
to
+703
| <Button appearance="base" onPress={onForceOpenDrawer}> | ||
| Force open global drawer — bypass rules | ||
| </Button> |
Comment on lines
+755
to
+760
| field={{ | ||
| label: "Drawer target", | ||
| value: resolvedPromptTarget, | ||
| raw: `dismissedCount: ${decision.dismissedCount}`, | ||
| status: { label: "Resolved", tone: "success" }, | ||
| }} |
Contributor
Rsdoctor Bundle Diff AnalysisFound 7 projects in monorepo, 1 project with changes. 📊 Quick Summary
📋 Detailed Reports (Click to expand)📁 mobilePath:
📦 Download Diff Report: mobile Bundle Diff Generated by Rsdoctor GitHub Action |
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new hook test suite mocks the feature-flag hook instead of exercising overrides via the feature-flags store, which conflicts with established repo testing patterns and should be aligned for consistency.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 13/13 changed files
- Comments generated: 1
- Review effort level: Lite
Comment on lines
+67
to
+69
| jest.mock("@features/platform-feature-flags", () => ({ | ||
| useFeature: () => qaState.brazePushNotifications, | ||
| })); |
|
Sonar flagged 5.3% duplication on new code: the QA inspector row was a verbatim copy of the Analytics Consent QA one, and the scenario list repeated the same default user state five times. Both QA screens now use a single QaInspectorRow component, and scenarios only spell out what makes them different. Tests drop the feature-flag hook mock in favour of the real store, so overrides resolve through useFeature as they do in production, and render the real Lumen and native-ui components instead of stand-ins. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new QA ViewModel can apply scenarios before the baseline snapshot is captured (permission status starts undefined), which can make “Reset all” restore the wrong state.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 16/16 changed files
- Comments generated: 1
- Review effort level: Lite
Comment on lines
+154
to
+156
| (scenario: NotificationsQaScenario) => { | ||
| const now = Date.now(); | ||
| const userData = buildNotificationsQaScenarioUserData(scenario, { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 Description
Problem: the LWM notifications prompt debug screen (Settings → Debug → Notifications prompt QA) could not be used by QA without an engineer. Field labels exposed implementation names (
isOptIn,dismissedCount, the raw OS permission enum), feature flag params rendered as JSON, actions used engineer verbs, and the screen never stated whether the drawer would show or why.Solution: the screen is reshaped after the Analytics consent QA screen, and every verdict is computed from
notificationsPromptEnginerather than duplicating eligibility rules.Show drawer,Skip,Blocked) and the reason behind it.brazePushNotificationsconfiguration.NotificationsPromptDrawerView, which the QA screen mounts in its own local drawer so previewing never touches production prompt state.Reset allrestores the state captured when the screen was opened and clears the feature flag override.Production behaviour is unchanged. The drawer scheduler is now shared through
NotificationsPromptProvider(which also exposescancelPendingDrawer) so the QA screen reuses the single scheduler instead of instantiating a second one, and skip reasons map to labels the same way analytics consent mapsREASON_LABEL.🔗 Context
✅ Checks
pnpm --filter live-mobile exec jest src/screens/Settings/Debug/NotificationsPromptQA→ 26 passed, including one case per QA scenario asserting the engine decision matches the advertised outcome.pnpm --filter live-mobile exec oxlinton the changed paths → warnings only, all pre-existing.src/mvvm/features/NotificationsPromptthat fail on@shared/i18n/@ledgerhq/live-common/families/hedera/...resolution fail the same way ondevelop(unbuilt workspace packages), unrelated to this change.🧪 QA focus
Reset all, then confirm the inspector returns to the values seen when the screen was opened.Made with Cursor