Skip to content

Conversation

@chebizarro
Copy link
Contributor

@chebizarro chebizarro commented Jul 31, 2025

Issues Addressed
#160: Bug with key management creating a new user
#161: Bug when pressing the buy or sell button repeatedly

Changes Made
Submit Button Loading State

  • Converted to StatefulWidget: Added proper state management for button loading behavior
  • Loading Spinner: Submit button now shows a circular progress indicator when pressed
  • Button Disabled: Prevents multiple clicks by disabling button during submission
  • CantDo Reset: Button only re-enables when server responds with Action.cantDo message
  • Dialog Dismissal Fix: Properly resets loading state if user dismisses amount entry dialog

Key Features

  • Prevents Double Submissions: Button becomes unclickable after first press until server response
  • Visual Feedback: Clear loading indicator shows operation is in progress
  • Proper Error Handling: Button re-enables only on server-side errors (cantDo messages)
  • Range Order Support: Handles both fixed and range order dialogs correctly

Technical Implementation

  • Added _isSubmitting state to track button loading
  • Listens to mostroMessageStreamProvider for Action.cantDo messages
  • Maintains all existing order taking functionality (range orders, lightning addresses)
  • Fixed buggy Key Manager hasKey implementation
  • Clean state management without affecting other UI components

Impact

  • Issue Bug with key management creating a new user #160: Users can now take orders immediately after reset without app restart
  • Issue Bug when pressing the buy or sell button repeatedly #161: Eliminates accidental double submissions that cause trade index increment and pending_order_exists errors
  • UX Improvement: Clear feedback prevents user confusion about whether action was registered
  • This fix addresses the root cause of both issues by implementing proper button state management and preventing the rapid-fire submissions that were causing daemon-side conflicts.

Summary by CodeRabbit

  • New Features

    • The order-taking screen now provides improved user feedback by showing a loading indicator and disabling the "Take Order" button during submission.
    • Users are prompted to enter an amount within a specified range for applicable orders, with the ability to cancel the dialog and reset the loading state.
  • Bug Fixes

    • Prevented the creation of duplicate sessions for the same order.
  • Improvements

    • Enhanced reliability in detecting the existence of a master key.
    • Updated navigation for the "Close" button to use a more consistent method.

…for Bug when pressing the buy or sell button repeatedly #161
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 31, 2025

Walkthrough

This update introduces internal logic changes to key management and session handling, and refactors the order-taking screen to a stateful widget. The order-taking UI now manages submission state, provides improved user feedback, and prevents duplicate actions. No public API signatures were altered, but internal behaviors and widget structure were updated.

Changes

Cohort / File(s) Change Summary
Key Manager Logic
lib/features/key_manager/key_manager.dart
Simplified the hasMasterKey method by removing the in-memory cache check; now always reads from storage to determine key presence.
Order Screen Refactor & State Management
lib/features/order/screens/take_order_screen.dart
Refactored TakeOrderScreen from ConsumerWidget to ConsumerStatefulWidget, added submission/loading state, improved user feedback, handled message stream actions, and updated navigation and controller references.
Session Notifier Logic
lib/shared/notifiers/session_notifier.dart
Updated newSession to check for an existing session with the same orderId before creating a new one, preventing duplicates.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant TakeOrderScreen
    participant MessageStream
    participant SessionNotifier

    User->>TakeOrderScreen: Press "Take Order" button
    TakeOrderScreen->>TakeOrderScreen: Set _isSubmitting = true
    alt Order has amount range
        TakeOrderScreen->>User: Show amount entry dialog
        User-->>TakeOrderScreen: Enter amount or cancel
        alt User cancels
            TakeOrderScreen->>TakeOrderScreen: Set _isSubmitting = false
        else User enters amount
            TakeOrderScreen->>SessionNotifier: takeBuyOrder/takeSellOrder(amount)
        end
    else No amount range
        TakeOrderScreen->>SessionNotifier: takeBuyOrder/takeSellOrder(parsed amount)
    end
    MessageStream-->>TakeOrderScreen: Receive action (e.g., "cantDo")
    TakeOrderScreen->>TakeOrderScreen: If _isSubmitting, set _isSubmitting = false
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • AndreaDiazCorreia

Poem

A rabbit leaps through fields of code,
Stateful screens and logic flowed.
Keys and sessions checked with care,
No duplicate bugs hiding there!
With loading spinners spinning bright,
Orders hop along just right.
🐇✨

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 733ea90 and d60b845.

⛔ Files ignored due to path filters (1)
  • pubspec.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • lib/features/key_manager/key_manager.dart (0 hunks)
  • lib/features/order/screens/take_order_screen.dart (11 hunks)
  • lib/shared/notifiers/session_notifier.dart (1 hunks)
💤 Files with no reviewable changes (1)
  • lib/features/key_manager/key_manager.dart
🧰 Additional context used
📓 Path-based instructions (4)
lib/shared/**/*.dart

📄 CodeRabbit Inference Engine (CLAUDE.md)

Shared utilities and widgets must be placed in shared/

Files:

  • lib/shared/notifiers/session_notifier.dart
lib/**/*.dart

📄 CodeRabbit Inference Engine (CLAUDE.md)

lib/**/*.dart: Use S.of(context).yourKey for all user-facing strings
Always use localized strings instead of hardcoded text
Pass BuildContext to methods that need localization
Always use latest APIs (e.g., withValues() instead of withOpacity())
Always check mounted before using context after async operations

Files:

  • lib/shared/notifiers/session_notifier.dart
  • lib/features/order/screens/take_order_screen.dart
**/*.dart

📄 CodeRabbit Inference Engine (CLAUDE.md)

**/*.dart: Remove unused imports and dependencies
Use const constructors where possible

Files:

  • lib/shared/notifiers/session_notifier.dart
  • lib/features/order/screens/take_order_screen.dart
lib/features/*/{screens,providers,notifiers,widgets}/**/*.dart

📄 CodeRabbit Inference Engine (CLAUDE.md)

Feature-based organization: features/{feature}/{screens|providers|notifiers|widgets}/

Files:

  • lib/features/order/screens/take_order_screen.dart
🧠 Learnings (9)
📓 Common learnings
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: lib/features/trades/widgets/mostro_message_detail_widget.dart:134-141
Timestamp: 2025-06-08T23:54:09.987Z
Learning: In the OrderState refactor, public keys should be accessed via `tradeState.peer?.publicKey` from the OrderState instance rather than through `session?.peer?.publicKey`, as the OrderState encapsulates peer information directly.
📚 Learning: in the orderstate refactor, public keys should be accessed via `tradestate.peer?.publickey` from the...
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: lib/features/trades/widgets/mostro_message_detail_widget.dart:134-141
Timestamp: 2025-06-08T23:54:09.987Z
Learning: In the OrderState refactor, public keys should be accessed via `tradeState.peer?.publicKey` from the OrderState instance rather than through `session?.peer?.publicKey`, as the OrderState encapsulates peer information directly.

Applied to files:

  • lib/shared/notifiers/session_notifier.dart
📚 Learning: in abstractmostronotifier, state updates occur for all messages regardless of timestamp to hydrate t...
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.

Applied to files:

  • lib/shared/notifiers/session_notifier.dart
📚 Learning: applies to lib/core/mostro_fsm.dart : order state transitions must be managed by `core/mostro_fsm.da...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-30T14:10:43.895Z
Learning: Applies to lib/core/mostro_fsm.dart : Order state transitions must be managed by `core/mostro_fsm.dart`

Applied to files:

  • lib/features/order/screens/take_order_screen.dart
📚 Learning: mostroservice methods like takebuyorder() and takesellorder() return future and trigger side e...
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future<void> and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.

Applied to files:

  • lib/features/order/screens/take_order_screen.dart
📚 Learning: applies to lib/features/*/{screens,providers,notifiers,widgets}/**/*.dart : feature-based organizati...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-30T14:10:43.895Z
Learning: Applies to lib/features/*/{screens,providers,notifiers,widgets}/**/*.dart : Feature-based organization: `features/{feature}/{screens|providers|notifiers|widgets}/`

Applied to files:

  • lib/features/order/screens/take_order_screen.dart
📚 Learning: use riverpod for all state management...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-30T14:10:43.895Z
Learning: Use Riverpod for all state management

Applied to files:

  • lib/features/order/screens/take_order_screen.dart
📚 Learning: applies to lib/core/app_routes.dart : navigation must be configured using gorouter in `core/app_rout...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-30T14:10:43.895Z
Learning: Applies to lib/core/app_routes.dart : Navigation must be configured using GoRouter in `core/app_routes.dart`

Applied to files:

  • lib/features/order/screens/take_order_screen.dart
📚 Learning: applies to lib/**/*.dart : use `s.of(context).yourkey` for all user-facing strings...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-30T14:10:43.895Z
Learning: Applies to lib/**/*.dart : Use `S.of(context).yourKey` for all user-facing strings

Applied to files:

  • lib/features/order/screens/take_order_screen.dart
🔇 Additional comments (1)
lib/shared/notifiers/session_notifier.dart (1)

70-72: Good defensive programming to prevent duplicate sessions.

This change correctly prevents creating duplicate sessions for the same orderId, which aligns with the PR objective of fixing the key management bug (#160).

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chebizarro/issue160

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@chebizarro chebizarro requested a review from Catrya July 31, 2025 23:52
Copy link
Member

@Catrya Catrya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK

@Catrya Catrya merged commit b01f4f5 into main Aug 1, 2025
2 checks passed
@Catrya Catrya deleted the chebizarro/issue160 branch August 8, 2025 22:11
@coderabbitai coderabbitai bot mentioned this pull request Oct 30, 2025
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants