Skip to content

Fix order creation time display bug #386

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Catrya wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix order creation time display bug #386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3ec8d98
use created at
Catrya Dec 11, 2025
27c6558
Remove buggy _timeAgo() method and expiration property
Catrya Dec 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 6 additions & 17 deletions lib/data/models/nostr_event.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,8 @@ extension NostrEventExtensions on NostrEvent {
String? get name => _getTagValue('name') ?? 'Anon';
String? get geohash => _getTagValue('g');
String? get bond => _getTagValue('bond');
String? get expiration => _timeAgo(_getTagValue('expiration'));
String? timeAgoWithLocale(String? locale) =>
_timeAgo(_getTagValue('expiration'), locale);
_timeAgoFromCreated(locale);
DateTime get expirationDate => _getTimeStamp(_getTagValue('expiration')!);
String? get expiresAt => _getTagValue('expires_at');
String? get platform => _getTagValue('y');
Expand All @@ -61,21 +60,11 @@ extension NostrEventExtensions on NostrEvent {
.subtract(Duration(hours: 12));
}

String _timeAgo(String? ts, [String? locale]) {
if (ts == null) return "invalid date";
final timestamp = int.tryParse(ts);
if (timestamp != null && timestamp > 0) {
final DateTime eventTime =
DateTime.fromMillisecondsSinceEpoch(timestamp * 1000)
.subtract(Duration(hours: 48));

// Use provided locale or fallback to Spanish
final effectiveLocale = locale ?? 'es';
return timeago.format(eventTime,
allowFromNow: true, locale: effectiveLocale);
} else {
return "invalid date";
}

String _timeAgoFromCreated([String? locale]) {
if (createdAt == null) return "invalid date";
final effectiveLocale = locale ?? 'es';
return timeago.format(createdAt!, allowFromNow: true, locale: effectiveLocale);
}

Future<NostrEvent> unWrap(String privateKey) async {
Expand Down
7 changes: 2 additions & 5 deletions lib/features/trades/widgets/mostro_message_detail_widget.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import 'package:flutter/material.dart';
import 'package:flutter_riverpod/flutter_riverpod.dart';
import 'package:mostro_mobile/data/enums.dart';
import 'package:mostro_mobile/data/models/nostr_event.dart';
import 'package:mostro_mobile/data/models/session.dart';
import 'package:mostro_mobile/features/order/models/order_state.dart';
import 'package:mostro_mobile/features/order/providers/order_notifier_provider.dart';
Expand Down Expand Up @@ -60,10 +59,8 @@ class MostroMessageDetail extends ConsumerWidget {
final orderPayload = tradeState.order;
switch (action) {
case actions.Action.newOrder:
final expHrs =
ref.read(orderRepositoryProvider).mostroInstance?.expiration ??
'24';
return S.of(context)!.newOrder(int.tryParse(expHrs) ?? 24);
final expHrs = 24; // TODO: use mostroInstance.expirationHours when available
return S.of(context)!.newOrder(expHrs);
Comment on lines +62 to +63
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Temporary workaround: hardcoded expiration hours may not match actual Mostrod configuration.

The hardcoded value of 24 hours decouples the displayed message from the actual expiration_hours setting in Mostrod. If the instance uses a different value (e.g., 40 hours as mentioned in issue #358), the message will be inconsistent with the actual order expiration.

While this is preferable to the previous bug (showing future-relative times), please ensure the TODO is tracked. Do you want me to help verify when mostroInstance.expirationHours will be available, or open an issue to track this technical debt?

Note: Other actions in this file successfully use mostroInstance?.expirationSeconds (lines 67-71, 80-84, etc.), suggesting the issue is specifically with the availability of an expirationHours field rather than access to mostroInstance itself.

🤖 Prompt for AI Agents 
lib/features/trades/widgets/mostro_message_detail_widget.dart lines 62-63:
replace the hardcoded expHrs=24 by deriving the value from the available
mostroInstance fields — if mostroInstance.expirationHours exists use it,
otherwise compute hours = (mostroInstance?.expirationSeconds ??
defaultSeconds)/3600 and use that as the displayed expiration; update the TODO
to track the technical debt (create an issue or add a clear TODO comment
referencing issue #358) and ensure null-safe access (use the nullable
mostroInstance) and a sensible default fallback so the displayed message always
reflects the instance configuration.

Copy link
Member Author

@Catrya Catrya Dec 11, 2025

Choose a reason for hiding this comment

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

The hardcoded 24 hours maintains the existing behavior - before this PR, the buggy code was already always displaying 24 hours regardless of the actual mostroInstance configuration due to parsing expiration as a relative time string instead of using the proper expirationSeconds field.

This PR only fixes the critical "in X days" timestamp bug. The proper mostroInstance.expirationSeconds integration will be handled in a separate PR to avoid mixing concerns.

Copy link
Contributor

@coderabbitai coderabbitai bot Dec 11, 2025

Choose a reason for hiding this comment

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

@Catrya, thank you for the clarification! That makes sense—if the previous code was already displaying 24 hours regardless of configuration, then maintaining that behavior while fixing the critical timestamp bug is a reasonable approach. Separating the timestamp fix from the expiration hours integration keeps the PRs focused and easier to review.

The TODO comment in the code will serve as a reminder for the follow-up work. If you'd like, I can help track this by opening a GitHub issue to capture the technical debt (integrating mostroInstance.expirationSeconds for the newOrder action), but that's entirely optional.

Thanks for the context on the existing behavior! 👍

🧠 Learnings used 
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/shared/widgets/dynamic_countdown_widget.dart : Use exact `order_expires_at` timestamps from Mostro protocol for DynamicCountdownWidget precision and localize display with `S.of(context)!.timeLeftLabel()`

Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/auth/notifiers/abstract_mostro_notifier.dart : Use `startSessionTimeoutCleanupForRequestId()` for order creation timeout protection and cancel timer automatically when any Mostro response received

Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/orders/notifiers/add_order_notifier.dart : Start session timeout cleanup in `submitOrder()` method to prevent orphan sessions when Mostro doesn't respond within 10 seconds

Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/core/mostro_fsm.dart : Use MostroFSM for managing order state transitions - all state changes must go through FSM state methods

Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/auth/notifiers/abstract_mostro_notifier.dart : Start 10-second cleanup timer automatically when taking orders via `startSessionTimeoutCleanup()` to prevent orphan sessions

Learnt from: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:157-182
Timestamp: 2025-10-21T21:47:03.451Z
Learning: In MostroP2P/mobile, for Action.canceled handling in abstract_mostro_notifier.dart (Riverpod StateNotifier), do not add mounted checks after async sessionNotifier.deleteSession(orderId) as they break order state synchronization during app restart. The Action.canceled flow contains critical business logic that must complete fully; Riverpod handles provider disposal automatically. Mounted checks should only protect UI operations, not business logic in StateNotifiers.

Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:141-154
Timestamp: 2025-10-14T21:12:06.887Z
Learning: In the MostroP2P mobile codebase, the notification system uses a two-layer localization pattern: providers/notifiers (without BuildContext access) call `showCustomMessage()` with string keys (e.g., 'orderTimeoutMaker', 'orderCanceled'), and the UI layer's `NotificationListenerWidget` has a switch statement that maps these keys to localized strings using `S.of(context)`. This architectural pattern properly separates concerns while maintaining full localization support for all user-facing messages.

Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.

case actions.Action.canceled:
return S.of(context)!.canceled(orderPayload?.id ?? '');
case actions.Action.payInvoice:
Expand Down