change payment_method type to string array#128
Conversation
WalkthroughThis update refactors payment method handling across several modules, changing it from a single string to a vector of strings. It updates the function signatures, parsing, serialization, and display logic to support multiple payment methods. Additionally, it improves error handling in tag parsing and updates the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant DB
participant PrettyTable
User->>CLI: Provide payment methods as comma-separated string
CLI->>CLI: Split and trim payment methods into Vec<String>
CLI->>DB: Pass Vec<String> as payment_method
DB->>DB: Serialize payment_method Vec<String> to JSON
DB-->>CLI: Order created with serialized payment methods
CLI->>PrettyTable: Request to display order(s)
PrettyTable->>PrettyTable: Join payment methods as comma-separated string
PrettyTable-->>User: Show order(s) with payment methods listed
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![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
🔭 Outside diff range comments (1)
src/nip33.rs (1)
62-91: 🛠️ Refactor suggestionInconsistent error handling:
dispute_from_tagsstill uses unsafe patterns.While
order_from_tagswas improved with robust error handling,dispute_from_tagsstill usesunwrap()calls that can panic. For consistency and reliability, apply similar safety improvements here.Consider applying similar error handling patterns:
pub fn dispute_from_tags(tags: Tags) -> Result<Dispute> { let mut dispute = Dispute::default(); for tag in tags { let t = tag.to_vec(); - let v = t.get(1).unwrap().as_str(); - match t.first().unwrap().as_str() { + if t.is_empty() { + continue; + } + let v = t.get(1).map(|s| s.as_str()).unwrap_or_default(); + match t.first().map(|s| s.as_str()).unwrap_or_default() { "d" => { - let id = t.get(1).unwrap().as_str().parse::<Uuid>(); + let id_str = t.get(1).map(|s| s.as_str()).unwrap_or_default(); + let id = Uuid::parse_str(id_str); let id = match id { core::result::Result::Ok(id) => id, Err(_) => return Err(anyhow::anyhow!("Invalid dispute id")), }; dispute.id = id; }
🧹 Nitpick comments (1)
src/nip33.rs (1)
36-36: Improved error handling withunwrap_or(0)but consider the appropriateness of zero defaults.The change from
unwrap()tounwrap_or(0)prevents panics, which is good. However, defaulting to 0 for amounts and premium might not always be semantically correct, especially for required fields likeamount.Consider whether 0 is an appropriate default for these financial fields, or if parsing failures should be handled differently (e.g., logging warnings or using Option types).
Also applies to: 44-44, 46-46, 53-53
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
Cargo.toml(1 hunks)src/cli/new_order.rs(2 hunks)src/db.rs(2 hunks)src/nip33.rs(1 hunks)src/pretty_table.rs(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/pretty_table.rs (2)
src/db.rs (2)
new(84-105)new(225-292)src/cli/new_order.rs (1)
payment_method(60-63)
src/db.rs (1)
src/cli/new_order.rs (1)
payment_method(60-63)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (10)
Cargo.toml (1)
42-42: LGTM: Dependency update aligns with payment method refactor.The
mostro-coreupdate to version 0.6.41 supports the changes from single payment method string to multiple payment methods as a vector.src/pretty_table.rs (2)
71-71: LGTM: Proper display of multiple payment methods.The change from
.to_string()to.join(", ")correctly handles the newVec<String>payment method format, displaying multiple payment methods as a readable comma-separated list.
176-177: LGTM: Consistent payment method display logic.The join operation properly formats the payment method vector for table display, maintaining consistency with the preview function.
src/cli/new_order.rs (2)
22-22: LGTM: Improved parameter type.The change from
&Stringto&strfollows Rust best practices and is more flexible for callers.
60-63: LGTM: Well-implemented multiple payment method parsing.The logic properly splits comma-separated payment methods, trims whitespace, and collects into the expected
Vec<String>format. This handles user input variations gracefully.src/db.rs (1)
236-237: LGTM: Proper JSON serialization with error handling.The serialization of the payment method vector to JSON string is the correct approach for database storage. The
unwrap_or_default()provides reasonable fallback behavior if serialization fails.src/nip33.rs (4)
2-2: LGTM: Added necessary import for Kind enum.The import is correctly added to support the
Kind::from_strusage in the updated parsing logic.
12-21: Excellent improvement in error handling and tag parsing robustness.The refactored tag parsing logic significantly improves safety by:
- Adding empty tag checks to prevent index out-of-bounds errors
- Using safe array access with bounds checking
- Eliminating potential panics from direct indexing
This is a substantial improvement over the previous implementation.
24-24: Good use of.ok()to prevent panics on parsing failures.Converting the
unwrap()calls to.ok()properly handles parsing failures without crashing, allowing the order to be created even if some fields fail to parse.Also applies to: 27-27
50-50: LGTM: Successfully implements payment_method as vector of strings.The change correctly collects all payment method values into a vector, supporting the new requirement for multiple payment methods per order.
grunch
changed the title
1 participant
Summary by CodeRabbit