feat(ui): role-aware waiting-phase trade status popups by arkanoider · Pull Request #73 · MostroP2P/mostrix

arkanoider · 2026-05-15T13:05:13Z

Summary

Invoice and payment popups were shown to both maker and taker even when only one party must act (e.g. buy maker seeing a Pay Invoice modal while waiting for the seller). This PR gates popups on who must act and adds a waiting-phase modal when the local user only needs to wait.

Add local_user_must_act_on_invoice_popup with buy/sell maker/taker rules for AddInvoice, PayInvoice, and PayBondInvoice
When the user must not act, show a Trade Status waiting popup (WaitingSellerToPay / WaitingBuyerInvoice) with phase-specific title and body text instead of an invoice input form
Suppress auto-popups in the notifications channel when the local user is not the acting party
Support Ok / Cancel Order on waiting-phase popups (same selection UX as bond/invoice modals)
Unit tests for role-based popup eligibility

Motivation

Early trade phases assign different actions to maker vs taker. Showing payment/invoice forms to the wrong party is confusing and can imply action is required when the user should only wait for the counterparty.

Summary by CodeRabbit

New Features
- Invoice and payment notifications now distinguish between actions the user must complete and scenarios where they're waiting for the other party.
- Added "Trade Status" notification popups that display waiting states for invoice and payment workflows with order details and action options.
- Enhanced keyboard navigation (left/right arrow keys) for managing invoice and payment notification action selections.

Show invoice/payment modals only when the local user must act; otherwise present a waiting-phase popup with phase-specific copy. Suppress auto-popups for non-acting parties and add unit tests for maker/taker role rules. Co-authored-by: Cursor <cursoragent@cursor.com>

coderabbitai · 2026-05-15T13:05:27Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 09009c51-817e-41bb-ae0b-989d42038fb4

📥 Commits

Reviewing files that changed from the base of the PR and between a671a7d and 8396046.

📒 Files selected for processing (8)

src/ui/key_handler/enter_handlers.rs
src/ui/key_handler/message_handlers.rs
src/ui/key_handler/mod.rs
src/ui/message_notification.rs
src/ui/orders.rs
src/util/dm_utils/mod.rs
src/util/dm_utils/notifications_ch_mng.rs
src/util/dm_utils/order_ch_mng.rs

Walkthrough

This PR introduces role-based invoice popup gating and waiting-phase UI rendering for WaitingSellerToPay and WaitingBuyerInvoice actions. New helper functions determine whether the local user must act on an invoice action, route non-acting parties to dedicated waiting-phase popups, and improve DM message role derivation to avoid incorrect state during early message arrivals.

Changes

Invoice Popup Role Gating and Waiting-Phase UI

Layer / File(s)	Summary
Notification model and role-based predicates `src/ui/orders.rs`, `src/util/dm_utils/order_ch_mng.rs`	`MessageNotification` gains optional `body` field for waiting-phase copy. New exported helpers `local_user_must_act_on_invoice_popup`, `waiting_phase_description`, `waiting_phase_short_label`, `waiting_popup_action_for_message`, and `order_message_to_waiting_notification` determine invoice action responsibility and build waiting-phase notifications. Comprehensive unit tests validate role gating across maker/taker scenarios and `None` role edge cases.
Waiting-phase popup rendering `src/ui/message_notification.rs`	New `render_waiting_phase_popup` displays order ID, phase preview, optional status message, "Ok"/"Cancel Order" buttons, and help text. Routes `WaitingSellerToPay` and `WaitingBuyerInvoice` actions to dedicated 90×16 popup with "📋 Trade Status" title.
Key handler integration for waiting actions `src/ui/key_handler/enter_handlers.rs`, `src/ui/key_handler/message_handlers.rs`, `src/ui/key_handler/mod.rs`	Enter handler branches on `local_user_must_act_on_invoice_popup`: acting party opens invoice action UI or shows `NewMessageNotification`; non-acting party transitions to waiting-phase popup via `order_message_to_waiting_notification`. Left/Right handlers extend action-selection cycling to `WaitingSellerToPay` and `WaitingBuyerInvoice`.
Robust role tracking in DM message handling `src/util/dm_utils/mod.rs`	New `effective_is_mine_for_trade_dm_message` helper computes maker/taker role using pre-upsert DB state, post-upsert hydration, and prior message role to avoid treating SQLite's default maker as authoritative when no DB row existed before upsert. Unit tests validate behavior across early-arrival and pre-existing-row scenarios.
Popup suppression gating in notification manager `src/util/dm_utils/notifications_ch_mng.rs`	Second-level gating via `local_user_must_act_on_invoice_popup` suppresses invoice modal when local user is not the responsible party, preventing unwanted popups in multi-role trade scenarios.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

MostroP2P/mostrix#61: Both modify Messages-tab invoice popup flow; this PR adds role-based gating via waiting-phase notifications, while the retrieved PR routes through preference-based popups.
MostroP2P/mostrix#54: Both change invoice notification and Enter-handling logic in src/ui/orders.rs and src/ui/key_handler/enter_handlers.rs, though with different branching strategies.
MostroP2P/mostrix#68: Both add role-aware branching and new popup rendering for invoice/payment actions and share invoice popup/action-routing pathways.

Suggested reviewers

AndreaDiazCorreia
Catrya
mostronatorcoder

Poem

🐰 A waiting phase blooms in the trading tale,
Role-aware popups now steer the sale,
When buyers and sellers both have their say,
The rabbit ensures none get in the way!
Let notifications wait with gentle grace,
Till the right actor steps into place. ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main feature: role-aware waiting-phase trade status popups, which directly matches the primary changes across the codebase.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feat/waiting-phase-invoice-popups

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

mostronatorcoder

I reviewed this strictly and I do see a blocker in the new role-aware popup gating.

Blocker

local_user_must_act_on_invoice_popup() now decides whether to show an actionable invoice/payment popup or a passive waiting popup based on msg.is_mine.

But in this codebase, is_mine is not always a strong/fully-hydrated signal yet:

timeline code already documents that takers often see is_mine: None until the local row hydrates
DM-only upserts can also preserve / infer role imperfectly before the local row is fully established

The new helper currently does:

let is_maker = msg.is_mine.unwrap_or(false);

So an unknown role (None) is silently treated as taker.

That was relatively harmless for timeline highlighting, but in this PR it directly controls whether the user:

gets the real actionable popup, or
gets a waiting-only popup / suppressed auto-popup instead

This creates a real risk that the correct acting party is misclassified during early hydration and does not get the action they are supposed to see.

In other words, this PR is using a field that is still known to be temporarily ambiguous as if it were authoritative enough to gate the invoice/payment UX.

I think this needs a safer fallback strategy for the unknown-role case, plus tests that explicitly cover is_mine: None for the invoice-popup decision path.

Because this can hide or downgrade an action-required popup for the actual local actor, I am marking this as request changes.

mostronatorcoder

Re-reviewed the latest changes.

The blocker I raised is addressed now.

The important fix is that OrderMessage.is_mine is no longer derived only from the pre-upsert snapshot in the trade-DM path. The listener now re-reads SQLite after upsert, but only trusts that post-upsert role when a local row already existed before the DM path. That avoids promoting the DM-upsert maker default to authoritative role information for the early taker race.

So the popup gating is now on much safer ground:

persisted maker/taker rows hydrate into Some(true/false) correctly
the brief unknown-role window still falls back to taker intentionally
tests now cover both the hydration helper and the is_mine: None popup behavior

With that fix in place, I do not see a remaining blocker in this PR.

arkanoider · 2026-05-15T14:43:16Z

@CodeRabbit review

coderabbitai · 2026-05-15T14:43:22Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

fix: cargo fmt

b33426e

mostronatorcoder Bot requested changes May 15, 2026

View reviewed changes

fix: fix(dm): hydrate OrderMessage.is_mine after trade-DM upsert

8396046

mostronatorcoder Bot approved these changes May 15, 2026

View reviewed changes

arkanoider merged commit aeee59f into main May 15, 2026
11 checks passed

arkanoider deleted the feat/waiting-phase-invoice-popups branch May 15, 2026 15:09

coderabbitai Bot mentioned this pull request Jun 8, 2026

feat(bond): handle maker PayBondInvoice on order creation (Phase 5) #85

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ui): role-aware waiting-phase trade status popups#73

feat(ui): role-aware waiting-phase trade status popups#73
arkanoider merged 3 commits into
mainfrom
feat/waiting-phase-invoice-popups

arkanoider commented May 15, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 15, 2026 •

edited

Loading

Uh oh!

mostronatorcoder Bot left a comment

Uh oh!

mostronatorcoder Bot left a comment

Uh oh!

arkanoider commented May 15, 2026

Uh oh!

coderabbitai Bot commented May 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

arkanoider commented May 15, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

mostronatorcoder Bot left a comment

Choose a reason for hiding this comment

Blocker

Uh oh!

mostronatorcoder Bot left a comment

Choose a reason for hiding this comment

Uh oh!

arkanoider commented May 15, 2026

Uh oh!

coderabbitai Bot commented May 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

arkanoider commented May 15, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 15, 2026 •

edited

Loading