Add In Progress status#96
Conversation
WalkthroughThis change updates the order status functionality by adding an Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant OrderStatus
Client->>OrderStatus: Call fmt() with Status::InProgress
OrderStatus-->>Client: Return "in-progress"
sequenceDiagram
participant Client
participant OrderStatus
Client->>OrderStatus: Call from_str("in-progress")
OrderStatus-->>Client: Return Status::InProgress
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/order.rs (1)
48-64: Consider documenting the new statusWhile the implementation is correct, the purpose and context of when the "InProgress" status should be used isn't documented. Consider adding a brief comment describing when an order should be marked as "in-progress" vs. other similar statuses.
pub enum Status { Active, Canceled, CanceledByAdmin, SettledByAdmin, CompletedByAdmin, Dispute, Expired, FiatSent, SettledHoldInvoice, Pending, Success, WaitingBuyerInvoice, WaitingPayment, CooperativelyCanceled, + /// Order is currently being processed according to nip69 specification InProgress, }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/order.rs(3 hunks)
🔇 Additional comments (4)
src/order.rs (4)
63-63: LGTM: Adding InProgress status to the Status enumThis change adds a new enum variant to represent an "in-progress" state for orders, aligning with nip69 as mentioned in the PR description.
83-83: LGTM: Display implementation for InProgressThe Display trait implementation is properly updated to format the new Status::InProgress variant as "in-progress", consistent with the kebab-case format used for other statuses.
107-107: LGTM: FromStr implementation for InProgressThe FromStr trait implementation correctly handles parsing "in-progress" strings into the Status::InProgress variant, maintaining the bidirectional conversion between string representations and enum variants.
48-64:Details
✅ Verification successful
Verify completeness of status transition validations
The code appears to have validation logic for status transitions in methods like
check_status(). Ensure that any code paths that rely on status checks are updated to handle the new InProgress status appropriately.
🏁 Script executed:
#!/bin/bash # Look for any status-checking logic or status transitions in the codebase rg -A 5 "check_status|Status::" --type rust | grep -v "FromStr\|Display"Length of output: 5049
Status transition update verified:
We confirmed that the newInProgressstatus is now fully integrated across the codebase. In particular:
In src/order.rs:
- The
InProgressvariant is defined in theStatusenum with the correct string representation ("in-progress").- Methods such as
check_status()and status conversions properly handle all enum variants includingInProgress.In src/dispute.rs:
- The
InProgressstatus is also managed correctly within status formatting logic.All status-related validations and transitions have been updated to include
InProgressconsistently. No additional changes are required from what was initially flagged.
Catrya
left a comment
There was a problem hiding this comment.
Hi @arkanoider great job!
The only thing that doesn't work well yet is that when an order is taken, it goes to in-progress (that's fine), but when the taker puts the invoice if they're a buyer, or pays the hold invoice if they're a seller, order event it's published again with the status in-progress.
It should only be published when the taker takes it, not also when it moves to the next status in the DB.
Everything else I told you yesterday already works perfectly.
uh yes probably i have to refine that edge case! |
Sorry, I just realized I put the review here in mostro-core instead of mostrod 😅 |
Don't worry it's clear... |
@grunch @Catrya
added in progress status for align behaviour to nip69.
Summary by CodeRabbit