Improve the clock: Use the time published by the mostro instance #206
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Use expiration_hours for the pending orders counter, and expiration_seconds for orders in waiting-buyer-invoice and waiting-payment. It also displays a more efficient countdown.
Contributor
WalkthroughThe countdown timer logic in order and trade screens was refactored into modular, real-time updating widgets using a new Riverpod Changes
Sequence Diagram(s)sequenceDiagram
participant UI as TakeOrderScreen/TradeDetailScreen
participant CountdownWidget
participant Riverpod
participant countdownTimeProvider
UI->>CountdownWidget: Build with expiration/order info
CountdownWidget->>Riverpod: Subscribe to countdownTimeProvider
Riverpod-->>CountdownWidget: Stream<DateTime> updates every second
CountdownWidget->>CountdownWidget: Compute remaining time, handle edge cases
CountdownWidget-->>UI: Render countdown or hide if expired/invalid
Suggested reviewers
Poem
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
lib/features/trades/screens/trade_detail_screen.dart (1)
541-562: Consider extracting formatDateTime to a shared utilityThis
formatDateTimemethod is duplicated in bothtake_order_screen.dartandtrade_detail_screen.dart.Consider extracting this to a shared date formatting utility:
// In lib/shared/utils/date_utils.dart class DateUtils { static String formatDateTime(DateTime dt, [BuildContext? context]) { if (context != null) { final dateFormatter = DateFormat.yMMMd(Localizations.localeOf(context).languageCode); final timeFormatter = DateFormat.Hm(Localizations.localeOf(context).languageCode); final formattedDate = dateFormatter.format(dt); final formattedTime = timeFormatter.format(dt); return S.of(context)!.createdOnDate('$formattedDate $formattedTime'); } else { final dateFormatter = DateFormat('EEE, MMM dd yyyy'); final timeFormatter = DateFormat('HH:mm'); final formattedDate = dateFormatter.format(dt); final formattedTime = timeFormatter.format(dt); return '$formattedDate at $formattedTime'; } } }Then import and use it in both screens.
🧹 Nitpick comments (3)
lib/shared/providers/time_provider.dart (1)
12-13: Update comment to be more specific about the provider's purposeThe comment could be more descriptive about the specific use case.
-/// Provides a more efficient countdown timer using Timer.periodic -/// with automatic cleanup and debouncing +/// Provides a real-time countdown timer that emits DateTime updates every second +/// with automatic cleanup and debouncing for efficient UI updatestest/shared/providers/time_provider_test.dart (1)
5-102: Comprehensive test coverage with room for enhancementThe test suite provides good coverage of the basic functionality. The cleanup test and provider independence tests are particularly well done.
Consider adding a test to verify the broadcast stream behavior with multiple listeners:
test('countdownTimeProvider supports multiple listeners', () async { final provider1 = container.listen(countdownTimeProvider, (_, __) {}); final provider2 = container.listen(countdownTimeProvider, (_, __) {}); // Both listeners should receive values expect(provider1.read(), isA<AsyncValue<DateTime>>()); expect(provider2.read(), isA<AsyncValue<DateTime>>()); // Close one listener provider1.close(); // Second listener should still work expect(provider2.read(), isA<AsyncValue<DateTime>>()); provider2.close(); });lib/features/trades/screens/trade_detail_screen.dart (1)
514-527: Address TODO: Implement creator rating data extractionThe TODO comment indicates missing functionality for extracting creator rating data.
Would you like me to help implement the extraction of creator rating data from the order state or create an issue to track this task?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
lib/features/order/screens/take_order_screen.dart(3 hunks)lib/features/trades/screens/trade_detail_screen.dart(4 hunks)lib/shared/providers/time_provider.dart(1 hunks)test/shared/countdown_validation_test.dart(1 hunks)test/shared/providers/time_provider_test.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.dart
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
test/**/*.dart
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
lib/features/**/{screens,widgets}/*.dart
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to lib/features/**/*.{dart} : Use Riverpod for all state management
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to lib/features/**/{screens,widgets}/*.dart : Use post-frame callbacks for side effects like SnackBars/dialogs
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to lib/features/**/{screens,widgets}/*.dart : Keep UI code declarative and side-effect free
test/shared/providers/time_provider_test.dart (9)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to test/**/*.dart : Unit tests should be placed in the `test/` directory
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to lib/features/**/*.{dart} : Use Riverpod for all state management
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `@Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to integration_test/**/*.dart : Integration tests should be placed in the `integration_test/` directory
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to test/mocks.dart : Mocks should be generated using Mockito in `test/mocks.dart`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to lib/main.dart : Locales for timeago package are configured in `main.dart` with `timeago.setLocaleMessages()`
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.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to lib/features/**/{screens,widgets}/*.dart : Always check `mounted` before using context after async operations
lib/features/trades/screens/trade_detail_screen.dart (11)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to lib/features/**/{screens,widgets}/*.dart : Use post-frame callbacks for side effects like SnackBars/dialogs
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/features/trades/models/trade_state.dart:1-15
Timestamp: 2025-05-08T16:06:33.665Z
Learning: In the context of the Mostro Mobile app, the `TradeState` class is specifically constructed using the `tradeStateProvider`. While some fields are nullable (`lastAction` and `orderPayload`), they are still marked as required parameters to ensure they are explicitly considered during state construction.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to lib/features/**/{screens,widgets}/*.dart : Keep UI code declarative and side-effect free
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to lib/features/**/{screens,widgets}/*.dart : Pass BuildContext to methods that need localization
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.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
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.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to lib/features/**/*.{dart} : Access data only through repository classes
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `@Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to lib/features/**/{screens,widgets}/*.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-18T20:24:34.836Z
Learning: Applies to lib/features/**/{screens,widgets}/*.dart : Always check `mounted` before using context after async operations
lib/features/order/screens/take_order_screen.dart (10)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to lib/features/**/{screens,widgets}/*.dart : Pass BuildContext to methods that need localization
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to lib/features/**/{screens,widgets}/*.dart : Keep UI code declarative and side-effect free
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to lib/features/**/{screens,widgets}/*.dart : Use post-frame callbacks for side effects like SnackBars/dialogs
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.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to lib/features/**/{screens,widgets}/*.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-18T20:24:34.836Z
Learning: Applies to lib/features/**/{screens,widgets}/*.dart : Always check `mounted` before using context after async operations
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to lib/features/**/{screens,widgets}/*.dart : Always use localized strings: Replace hardcoded text with `S.of(context)!.keyName`
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `@Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to lib/data/models/nostr_event.dart : Custom `timeAgoWithLocale()` method is implemented in NostrEvent extension
test/shared/countdown_validation_test.dart (4)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to test/**/*.dart : Unit tests should be placed in the `test/` directory
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to integration_test/**/*.dart : Integration tests should be placed in the `integration_test/` directory
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to test/mocks.dart : Mocks should be generated using Mockito in `test/mocks.dart`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: All changes should pass `flutter analyze` before commit
🔇 Additional comments (6)
lib/shared/providers/time_provider.dart (1)
14-53: Well-implemented countdown timer provider with proper lifecycle managementThe implementation demonstrates excellent practices:
- Broadcast stream allows multiple widgets to subscribe
- Debouncing prevents unnecessary rebuilds within the same second
- Proper cleanup in both
onCancelandonDisposeprevents memory leaks- Immediate emission of initial value ensures UI has data right away
test/shared/countdown_validation_test.dart (1)
55-97: Well-structured time calculation testsThe time calculation tests properly verify duration breakdown, clamping behavior, zero duration handling, and time formatting.
lib/features/order/screens/take_order_screen.dart (2)
314-379: Well-implemented countdown widget with proper edge case handlingThe
_CountdownWidgetimplementation demonstrates good practices:
- Proper subscription to
countdownTimeProviderfor real-time updates- Comprehensive edge case handling for past expiration dates
- Dynamic expiration hours from Mostro instance with validation
- Correct use of localized strings
352-354: Null safety already handled with fallback
You’re already using a null-aware access (mostroInstance?.expirationHours) combined with?? 24, so there’s no risk of a null-pointer here. No further changes are needed.lib/features/trades/screens/trade_detail_screen.dart (2)
290-295: Good UX consideration for cancel dialogNice attention to detail in resetting the loading state when the dialog is cancelled. This prevents the button from staying in a loading state if the user backs out.
565-766: Comprehensive countdown implementation with excellent edge case handlingThe
_CountdownWidgetimplementation is well-designed:
- Status-specific countdown logic is clear and follows business requirements
- Extensive timestamp validation prevents issues with invalid data
- The
_findMessageForStatehelper method is well-implemented with proper sorting and filtering- Edge cases like future timestamps and past expirations are handled appropriately
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
test/shared/countdown_validation_test.dart (2)
3-34: Consider moving utility functions to production code.These validation and utility functions appear to be reusable business logic that could benefit the main application, not just tests. Consider moving them to a shared utilities module in the main codebase (e.g.,
lib/shared/utils/countdown_utils.dart) and importing them in tests.If these functions are meant to remain test-only, add a comment explaining why they're not part of the production codebase.
132-155: LGTM! Comprehensive edge case coverage with minor suggestion.The edge case tests provide excellent coverage of boundary conditions and conversion scenarios.
Consider clarifying the rounding behavior in the comments (lines 145-147) - Dart's
.round()uses "round half away from zero" for positive numbers, not strictly "rounded up".
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/shared/countdown_validation_test.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.dart
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
test/**/*.dart
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to lib/features/**/*.{dart} : Use Riverpod for all state management
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to lib/features/**/{screens,widgets}/*.dart : Use post-frame callbacks for side effects like SnackBars/dialogs
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to lib/features/**/{screens,widgets}/*.dart : Keep UI code declarative and side-effect free
test/shared/countdown_validation_test.dart (4)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to test/**/*.dart : Unit tests should be placed in the `test/` directory
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to integration_test/**/*.dart : Integration tests should be placed in the `integration_test/` directory
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: Applies to test/mocks.dart : Mocks should be generated using Mockito in `test/mocks.dart`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T20:24:34.836Z
Learning: All changes should pass `flutter analyze` before commit
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
test/shared/countdown_validation_test.dart (4)
39-72: LGTM! Comprehensive timestamp validation tests.This test group effectively covers edge cases and realistic scenarios for timestamp validation. The tests properly validate function behavior rather than constants, addressing previous review feedback.
74-98: LGTM! Expiration validation tests properly implemented.The expiration hours validation test (lines 91-97) now correctly calls the actual validation function
isValidExpirationHours()with various inputs, addressing the previous review feedback about testing constants. The boundary value testing is thorough and appropriate.
100-130: LGTM! Thorough time calculation tests.The time calculation tests provide excellent coverage of duration handling, clamping logic, and formatting functionality. The edge case testing (zero values, boundary conditions) is particularly well done.
1-157: Excellent test suite that aligns with PR objectives and coding guidelines.This comprehensive test file:
- ✅ Correctly placed in
test/directory per guidelines- ✅ Addresses previous review feedback by testing actual validation functions
- ✅ Provides thorough coverage of countdown timer validation logic
- ✅ Supports the PR's goal of improving countdown timer accuracy
- ✅ Follows Flutter testing conventions with proper test organization
The test suite effectively validates the countdown functionality that's been refactored across the application.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fix #155
Use expiration_hours for the pending orders counter, and expiration_seconds for orders in waiting-buyer-invoice and waiting-payment. It also displays a more efficient countdown.
Summary by CodeRabbit