UI improvements#69
Conversation
CantDo messages should not drive order row upserts; treat them like NewOrder and return early from handle_trade_dm_for_order. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThis PR adds synthetic placeholder messages when trade orders arrive before corresponding DM rows exist, refactors timeline step enums for both buy and sell listings, and updates message display logic to use compact action labels with status-dependent popup state. ChangesOrder Placeholder Messages and Timeline Display
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
![mostronatorcoder[bot]](https://avatars.githubusercontent.com/in/2922838?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this strictly and I found a blocker in the new timeline-state mapping.
Blocker
listing_step_from_status() now regresses terminal-status handling in two ways.
In src/ui/orders.rs:
Status::Canceled
| Status::CanceledByAdmin
| Status::CooperativelyCanceled
| Status::Expired
| Status::Dispute
| Status::SettledByAdmin
| Status::CompletedByAdmin => {
Some(FlowStep::BuyFlowStep(StepLabelsBuy::StepPendingOrder))
}and in the Kind::Sell arm the same terminal statuses now return:
Some(FlowStep::BuyFlowStep(StepLabelsBuy::StepPendingOrder))That creates two separate problems:
- Canceled / expired / dispute / admin-settled trades now render as if they were still in the initial pending phase, instead of yielding no active progress step.
- In the sell branch, terminal statuses return a
BuyFlowStepinstead of aSellFlowStep, which is plainly inconsistent with the kind-specific timeline model.
Before this change, those terminal non-success statuses returned None, which was much safer and semantically correct for a timeline that represents active trade progression.
So this PR currently makes terminal failed/canceled states look like fresh pending trades, and also crosses buy/sell flow types in the sell branch.
Because this affects user-visible state interpretation in the trade timeline, I am marking this as request changes.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/ui/orders.rs`:
- Around line 724-726: The sell-status mapping incorrectly returns a buy flow
step: in the Kind::Sell branch where Status::CompletedByAdmin currently maps to
FlowStep::BuyFlowStep(StepLabelsBuy::StepPendingOrder), change it to return the
corresponding sell variant—i.e., FlowStep::SellFlowStep(...) using the
appropriate StepLabelsSell::StepPendingOrder (or the correct sell label) so sell
terminal statuses use the SellFlowStep path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fafd2138-a426-45b1-8d20-b13d5e136419
📒 Files selected for processing (6)
src/main.rssrc/ui/helpers/startup.rssrc/ui/key_handler/enter_handlers.rssrc/ui/orders.rssrc/util/dm_utils/mod.rssrc/util/dm_utils/order_ch_mng.rs
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Thanks, the sell-branch But the main semantic blocker is still present: terminal non-success statuses are still being mapped to That means states like:
still render like an initial pending trade instead of yielding no active progression step. So the type mismatch is gone, but the timeline regression itself is still there. I still think those terminal non-success states should not map to the pending column. |
There was a problem hiding this comment.
Re-reviewed with the clarified UI semantics in mind.
Given that the terminal non-success outcome is already surfaced explicitly through the persisted-status warning/header path, I no longer consider the StepPendingOrder mapping here a blocker by itself. In that full UI context, the timeline is acting as a neutral “no active progress completed” baseline rather than the sole carrier of the final state.
The earlier sell-branch BuyFlowStep bug was fixed, checks are green, and I do not see another blocker in the current version.
So with that interpretation, I am comfortable approving this PR.
1 participant
Summary
Improves how My Trades and order timelines behave when success arrives before DMs, tightens flow step labels (including pending / bond phases), adjusts enter-handler behavior, and avoids CantDo trade DMs from driving order upserts.
Motivation
Successlands before a real DM-backedOrderMessage.Changes (vs
main)OrderMessagefromOrderSuccess— synthetic row for the sidebar without faketake-buy/take-sellactions that break Messages-tab Enter (src/ui/orders.rs).Pending, pending-order step enums for buy/sell flows (src/ui/orders.rs).src/main.rs,src/ui/helpers/startup.rs,src/util/dm_utils/order_ch_mng.rs).src/ui/key_handler/enter_handlers.rs).CantDolikeNewOrderinhandle_trade_dm_for_order(src/util/dm_utils/mod.rs).Summary by CodeRabbit
New Features
Bug Fixes