From 82d0de1d84288e4006c9fb1ab1bac3f258c68f6f Mon Sep 17 00:00:00 2001 From: Catrya <140891948+Catrya@users.noreply.github.com> Date: Fri, 18 Jul 2025 15:14:53 -0600 Subject: [PATCH 1/3] Improve the clock: Use the time published by the mostro instance 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. --- .../order/screens/take_order_screen.dart | 101 +++++-- .../trades/screens/trade_detail_screen.dart | 256 +++++++++++++++--- lib/shared/providers/time_provider.dart | 43 +++ test/shared/countdown_validation_test.dart | 122 +++++++++ test/shared/providers/time_provider_test.dart | 102 +++++++ 5 files changed, 556 insertions(+), 68 deletions(-) create mode 100644 test/shared/countdown_validation_test.dart create mode 100644 test/shared/providers/time_provider_test.dart diff --git a/lib/features/order/screens/take_order_screen.dart b/lib/features/order/screens/take_order_screen.dart index ce6423e8..2d9a2323 100644 --- a/lib/features/order/screens/take_order_screen.dart +++ b/lib/features/order/screens/take_order_screen.dart @@ -13,6 +13,8 @@ import 'package:mostro_mobile/shared/widgets/order_cards.dart'; import 'package:mostro_mobile/shared/providers/order_repository_provider.dart'; import 'package:mostro_mobile/shared/utils/currency_utils.dart'; import 'package:mostro_mobile/shared/widgets/custom_card.dart'; +import 'package:mostro_mobile/features/mostro/mostro_instance.dart'; +import 'package:mostro_mobile/shared/providers/time_provider.dart'; import 'package:mostro_mobile/generated/l10n.dart'; class TakeOrderScreen extends ConsumerWidget { @@ -49,7 +51,9 @@ class TakeOrderScreen extends ConsumerWidget { const SizedBox(height: 16), _buildCreatorReputation(order), const SizedBox(height: 24), - _buildCountDownTime(context, order.expirationDate), + _CountdownWidget( + expirationDate: order.expirationDate, + ), const SizedBox(height: 36), _buildActionButtons(context, ref, order), ], @@ -124,34 +128,6 @@ class TakeOrderScreen extends ConsumerWidget { ); } - Widget _buildCountDownTime(BuildContext context, DateTime expiration) { - Duration countdown = Duration(hours: 0); - final now = DateTime.now(); - - if (expiration.isAfter(now)) { - countdown = expiration.difference(now); - } - - final int maxOrderHours = 24; - final hoursLeft = countdown.inHours.clamp(0, maxOrderHours); - final minutesLeft = countdown.inMinutes % 60; - final secondsLeft = countdown.inSeconds % 60; - - final formattedTime = - '${hoursLeft.toString().padLeft(2, '0')}:${minutesLeft.toString().padLeft(2, '0')}:${secondsLeft.toString().padLeft(2, '0')}'; - - return Column( - children: [ - CircularCountdown( - countdownTotal: maxOrderHours, - countdownRemaining: hoursLeft, - ), - const SizedBox(height: 16), - Text(S.of(context)!.timeLeftLabel(formattedTime)), - ], - ); - } - Widget _buildPaymentMethod(BuildContext context, NostrEvent order) { final methods = order.paymentMethods.isNotEmpty ? order.paymentMethods.join(', ') @@ -334,3 +310,70 @@ class TakeOrderScreen extends ConsumerWidget { } } } + +/// Widget that displays a real-time countdown timer for pending orders +class _CountdownWidget extends ConsumerWidget { + final DateTime expirationDate; + + const _CountdownWidget({ + required this.expirationDate, + }); + + @override + Widget build(BuildContext context, WidgetRef ref) { + // Watch the countdown time provider for real-time updates + final timeAsync = ref.watch(countdownTimeProvider); + + return timeAsync.when( + data: (currentTime) { + return _buildCountDownTime(context, ref, expirationDate); + }, + loading: () => const CircularProgressIndicator(), + error: (error, stack) => const SizedBox.shrink(), + ); + } + + Widget _buildCountDownTime( + BuildContext context, WidgetRef ref, DateTime expiration) { + Duration countdown = Duration(hours: 0); + final now = DateTime.now(); + + // Handle edge case: expiration in the past + if (expiration.isBefore(now.subtract(const Duration(hours: 1)))) { + // If expiration is more than 1 hour in the past, likely invalid + return const SizedBox.shrink(); + } + + if (expiration.isAfter(now)) { + countdown = expiration.difference(now); + } + + // Get dynamic expiration hours from Mostro instance + final mostroInstance = ref.read(orderRepositoryProvider).mostroInstance; + final maxOrderHours = + mostroInstance?.expirationHours ?? 24; // fallback to 24 hours + + // Validate expiration hours + if (maxOrderHours <= 0 || maxOrderHours > 168) { // Max 1 week + return const SizedBox.shrink(); + } + + final hoursLeft = countdown.inHours.clamp(0, maxOrderHours); + final minutesLeft = countdown.inMinutes % 60; + final secondsLeft = countdown.inSeconds % 60; + + final formattedTime = + '${hoursLeft.toString().padLeft(2, '0')}:${minutesLeft.toString().padLeft(2, '0')}:${secondsLeft.toString().padLeft(2, '0')}'; + + return Column( + children: [ + CircularCountdown( + countdownTotal: maxOrderHours, + countdownRemaining: hoursLeft, + ), + const SizedBox(height: 16), + Text(S.of(context)!.timeLeftLabel(formattedTime)), + ], + ); + } +} diff --git a/lib/features/trades/screens/trade_detail_screen.dart b/lib/features/trades/screens/trade_detail_screen.dart index c4ef6615..6a41b073 100644 --- a/lib/features/trades/screens/trade_detail_screen.dart +++ b/lib/features/trades/screens/trade_detail_screen.dart @@ -13,9 +13,14 @@ import 'package:mostro_mobile/features/order/providers/order_notifier_provider.d import 'package:mostro_mobile/features/order/widgets/order_app_bar.dart'; import 'package:mostro_mobile/shared/widgets/order_cards.dart'; import 'package:mostro_mobile/features/trades/widgets/mostro_message_detail_widget.dart'; +import 'package:mostro_mobile/shared/providers/order_repository_provider.dart'; import 'package:mostro_mobile/shared/providers/session_notifier_provider.dart'; import 'package:mostro_mobile/shared/widgets/mostro_reactive_button.dart'; import 'package:mostro_mobile/data/models/session.dart'; +import 'package:mostro_mobile/features/mostro/mostro_instance.dart'; +import 'package:mostro_mobile/shared/providers/mostro_storage_provider.dart'; +import 'package:mostro_mobile/data/models/mostro_message.dart'; +import 'package:mostro_mobile/shared/providers/time_provider.dart'; import 'package:mostro_mobile/generated/l10n.dart'; class TradeDetailScreen extends ConsumerWidget { @@ -66,12 +71,14 @@ class TradeDetailScreen extends ConsumerWidget { MostroMessageDetail(orderId: orderId), ], const SizedBox(height: 24), - _buildCountDownTime( - context, - orderPayload.expiresAt != null - ? orderPayload.expiresAt! * 1000 - : null), - const SizedBox(height: 36), + // Show countdown timer only for specific statuses + _CountdownWidget( + orderId: orderId, + tradeState: tradeState, + expiresAtTimestamp: orderPayload.expiresAt != null + ? orderPayload.expiresAt! * 1000 + : null, + ), Wrap( alignment: WrapAlignment.center, spacing: 10, @@ -217,38 +224,6 @@ class TradeDetailScreen extends ConsumerWidget { ); } - /// Build a circular countdown to show how many hours are left until expiration. - Widget _buildCountDownTime(BuildContext context, int? expiresAtTimestamp) { - // Convert timestamp to DateTime - final now = DateTime.now(); - - final expiration = expiresAtTimestamp != null && expiresAtTimestamp > 0 - ? DateTime.fromMillisecondsSinceEpoch(expiresAtTimestamp) - : now.add(const Duration(hours: 24)); - - final Duration difference = - expiration.isAfter(now) ? expiration.difference(now) : const Duration(); - - final int maxOrderHours = 24; - final hoursLeft = difference.inHours.clamp(0, maxOrderHours); - final minutesLeft = difference.inMinutes % 60; - final secondsLeft = difference.inSeconds % 60; - - final formattedTime = - '${hoursLeft.toString().padLeft(2, '0')}:${minutesLeft.toString().padLeft(2, '0')}:${secondsLeft.toString().padLeft(2, '0')}'; - - return Column( - children: [ - CircularCountdown( - countdownTotal: maxOrderHours, - countdownRemaining: hoursLeft, - ), - const SizedBox(height: 16), - Text(S.of(context)!.timeLeftLabel(formattedTime)), - ], - ); - } - /// Main action button area, switching on `orderPayload.status`. /// Additional checks use `message.action` to refine which button to show. /// Following the Mostro protocol state machine for order flow. @@ -313,7 +288,7 @@ class TradeDetailScreen extends ConsumerWidget { ], ), ); - + // Reset loading state if dialog was cancelled if (result != true) { buttonController.resetLoading(); @@ -586,3 +561,206 @@ class TradeDetailScreen extends ConsumerWidget { } } } + +/// Widget that displays a real-time countdown timer that updates every second +class _CountdownWidget extends ConsumerWidget { + final String orderId; + final OrderState tradeState; + final int? expiresAtTimestamp; + + const _CountdownWidget({ + required this.orderId, + required this.tradeState, + this.expiresAtTimestamp, + }); + + @override + Widget build(BuildContext context, WidgetRef ref) { + // Watch the countdown time provider for real-time updates + final timeAsync = ref.watch(countdownTimeProvider); + final messagesAsync = ref.watch(mostroMessageHistoryProvider(orderId)); + + return timeAsync.when( + data: (currentTime) { + return messagesAsync.maybeWhen( + data: (messages) { + final countdownWidget = _buildCountDownTime( + context, + ref, + tradeState, + messages, + expiresAtTimestamp, + ); + + if (countdownWidget != null) { + return Column( + children: [ + countdownWidget, + const SizedBox(height: 36), + ], + ); + } else { + return const SizedBox(height: 12); + } + }, + orElse: () => const SizedBox(height: 12), + ); + }, + loading: () => const SizedBox(height: 12), + error: (error, stack) => const SizedBox(height: 12), + ); + } + + /// Build a circular countdown timer only for specific order statuses. + /// Shows countdown ONLY for: Pending, Waiting-buyer-invoice, Waiting-payment + /// - Pending: uses expirationHours from Mostro instance + /// - Waiting-buyer-invoice: countdown from message timestamp + expirationSeconds + /// - Waiting-payment: countdown from message timestamp + expirationSeconds + /// - All other states: no countdown timer + Widget? _buildCountDownTime( + BuildContext context, + WidgetRef ref, + OrderState tradeState, + List messages, + int? expiresAtTimestamp) { + final status = tradeState.status; + final now = DateTime.now(); + final mostroInstance = ref.read(orderRepositoryProvider).mostroInstance; + + // Show countdown ONLY for these 3 specific statuses + if (status == Status.pending) { + // Pending orders: use expirationHours + final expHours = + mostroInstance?.expirationHours ?? 24; // 24 hours fallback + final countdownDuration = Duration(hours: expHours); + + // Handle edge case: invalid timestamp + if (expiresAtTimestamp != null && expiresAtTimestamp <= 0) { + expiresAtTimestamp = null; + } + + final expiration = expiresAtTimestamp != null + ? DateTime.fromMillisecondsSinceEpoch(expiresAtTimestamp) + : now.add(countdownDuration); + + // Handle edge case: expiration in the past + if (expiration.isBefore(now.subtract(const Duration(hours: 1)))) { + // If expiration is more than 1 hour in the past, likely invalid + return null; + } + + final Duration difference = expiration.isAfter(now) + ? expiration.difference(now) + : const Duration(); + + final hoursLeft = difference.inHours.clamp(0, expHours); + final minutesLeft = difference.inMinutes % 60; + final secondsLeft = difference.inSeconds % 60; + + final formattedTime = + '${hoursLeft.toString().padLeft(2, '0')}:${minutesLeft.toString().padLeft(2, '0')}:${secondsLeft.toString().padLeft(2, '0')}'; + + return Column( + children: [ + CircularCountdown( + countdownTotal: expHours, + countdownRemaining: hoursLeft, + ), + const SizedBox(height: 16), + Text(S.of(context)!.timeLeftLabel(formattedTime)), + ], + ); + } else if (status == Status.waitingBuyerInvoice || + status == Status.waitingPayment) { + // Find the message that triggered this state + final stateMessage = _findMessageForState(messages, status); + if (stateMessage?.timestamp == null) { + // If no message found, don't show countdown + return null; + } + + final expSecs = + mostroInstance?.expirationSeconds ?? 900; // 15 minutes fallback + final expMinutes = (expSecs / 60).round(); + + // Validate timestamp + final messageTimestamp = stateMessage!.timestamp!; + if (messageTimestamp <= 0) { + return null; + } + + // Calculate expiration from when the message was received + final messageTime = DateTime.fromMillisecondsSinceEpoch(messageTimestamp); + + // Handle edge case: message timestamp in the future + if (messageTime.isAfter(now.add(const Duration(minutes: 5)))) { + // If message is more than 5 minutes in the future, likely invalid + return null; + } + + final expiration = messageTime.add(Duration(seconds: expSecs)); + + final Duration difference = expiration.isAfter(now) + ? expiration.difference(now) + : const Duration(); + + final minutesLeft = difference.inMinutes.clamp(0, expMinutes); + final secondsLeft = difference.inSeconds % 60; + + final formattedTime = + '${minutesLeft.toString().padLeft(2, '0')}:${secondsLeft.toString().padLeft(2, '0')}'; + + return Column( + children: [ + CircularCountdown( + countdownTotal: expMinutes, + countdownRemaining: minutesLeft, + ), + const SizedBox(height: 16), + Text(S.of(context)!.timeLeftLabel(formattedTime)), + ], + ); + } else { + // All other statuses: NO countdown timer + return null; + } + } + + /// Find the message that triggered the current state + /// Returns null if no valid message is found + MostroMessage? _findMessageForState( + List messages, Status status) { + // Filter out messages with invalid timestamps + final validMessages = messages + .where((m) => m.timestamp != null && m.timestamp! > 0) + .toList(); + + if (validMessages.isEmpty) { + return null; + } + + // Sort messages by timestamp (newest first) + final sortedMessages = List.from(validMessages) + ..sort((a, b) => (b.timestamp ?? 0).compareTo(a.timestamp ?? 0)); + + // Find the message that caused this state + for (final message in sortedMessages) { + // Additional validation: ensure timestamp is not in the future + final messageTime = DateTime.fromMillisecondsSinceEpoch(message.timestamp!); + if (messageTime.isAfter(DateTime.now().add(const Duration(minutes: 5)))) { + continue; // Skip messages with future timestamps + } + + if (status == Status.waitingBuyerInvoice && + (message.action == actions.Action.addInvoice || + message.action == actions.Action.waitingBuyerInvoice)) { + return message; + } else if (status == Status.waitingPayment && + (message.action == actions.Action.payInvoice || + message.action == actions.Action.waitingSellerToPay)) { + return message; + } + } + return null; + } +} diff --git a/lib/shared/providers/time_provider.dart b/lib/shared/providers/time_provider.dart index f61641c1..c617d471 100644 --- a/lib/shared/providers/time_provider.dart +++ b/lib/shared/providers/time_provider.dart @@ -8,3 +8,46 @@ final timeProvider = StreamProvider((ref) { (_) => DateTime.now(), ); }); + +/// Provides a more efficient countdown timer using Timer.periodic +/// with automatic cleanup and debouncing +final countdownTimeProvider = StreamProvider((ref) { + late StreamController controller; + Timer? timer; + DateTime? lastEmittedTime; + + controller = StreamController.broadcast( + onListen: () { + // Start timer when first listener subscribes + timer = Timer.periodic(const Duration(seconds: 1), (timer) { + final now = DateTime.now(); + // Debounce: only emit if seconds have actually changed + if (lastEmittedTime == null || + now.second != lastEmittedTime!.second || + now.minute != lastEmittedTime!.minute || + now.hour != lastEmittedTime!.hour) { + lastEmittedTime = now; + controller.add(now); + } + }); + // Emit initial value immediately + final now = DateTime.now(); + lastEmittedTime = now; + controller.add(now); + }, + onCancel: () { + // Cleanup timer when last listener unsubscribes + timer?.cancel(); + timer = null; + lastEmittedTime = null; + }, + ); + + // Ensure cleanup when provider is disposed + ref.onDispose(() { + timer?.cancel(); + controller.close(); + }); + + return controller.stream; +}); diff --git a/test/shared/countdown_validation_test.dart b/test/shared/countdown_validation_test.dart new file mode 100644 index 00000000..1b429c12 --- /dev/null +++ b/test/shared/countdown_validation_test.dart @@ -0,0 +1,122 @@ +import 'package:flutter_test/flutter_test.dart'; + +void main() { + group('Countdown Validation Tests', () { + + group('Timestamp Validation', () { + test('should reject negative timestamps', () { + final timestamp = -1000; + expect(timestamp <= 0, isTrue); + }); + + test('should reject zero timestamps', () { + final timestamp = 0; + expect(timestamp <= 0, isTrue); + }); + + test('should accept valid timestamps', () { + final timestamp = DateTime.now().millisecondsSinceEpoch; + expect(timestamp > 0, isTrue); + }); + + test('should reject future timestamps beyond threshold', () { + final now = DateTime.now(); + final futureTime = now.add(const Duration(minutes: 10)); + final threshold = now.add(const Duration(minutes: 5)); + + expect(futureTime.isAfter(threshold), isTrue); + }); + }); + + group('Expiration Validation', () { + test('should reject expiration times too far in the past', () { + final now = DateTime.now(); + final pastExpiration = now.subtract(const Duration(hours: 2)); + final threshold = now.subtract(const Duration(hours: 1)); + + expect(pastExpiration.isBefore(threshold), isTrue); + }); + + test('should accept recent expiration times', () { + final now = DateTime.now(); + final recentExpiration = now.subtract(const Duration(minutes: 30)); + final threshold = now.subtract(const Duration(hours: 1)); + + expect(recentExpiration.isAfter(threshold), isTrue); + }); + + test('should validate expiration hours range', () { + expect(0 <= 0, isTrue); // Invalid: zero hours + expect(24 > 0 && 24 <= 168, isTrue); // Valid: 24 hours + expect(200 > 168, isTrue); // Invalid: over 1 week + }); + }); + + group('Time Calculations', () { + test('should calculate hours correctly', () { + final duration = const Duration(hours: 5, minutes: 30, seconds: 45); + + final hours = duration.inHours; + final minutes = duration.inMinutes % 60; + final seconds = duration.inSeconds % 60; + + expect(hours, equals(5)); + expect(minutes, equals(30)); + expect(seconds, equals(45)); + }); + + test('should clamp hours to maximum', () { + final maxHours = 24; + final duration = const Duration(hours: 30); + + final hoursLeft = duration.inHours.clamp(0, maxHours); + expect(hoursLeft, equals(24)); + }); + + test('should handle zero duration', () { + final duration = const Duration(); + + final hours = duration.inHours; + final minutes = duration.inMinutes % 60; + final seconds = duration.inSeconds % 60; + + expect(hours, equals(0)); + expect(minutes, equals(0)); + expect(seconds, equals(0)); + }); + + test('should format time correctly', () { + final hours = 5; + final minutes = 7; + final seconds = 9; + + final formattedTime = '${hours.toString().padLeft(2, '0')}:${minutes.toString().padLeft(2, '0')}:${seconds.toString().padLeft(2, '0')}'; + + expect(formattedTime, equals('05:07:09')); + }); + }); + + group('Edge Cases', () { + test('should handle null expiration timestamp', () { + final int? timestamp = null; + final isValid = timestamp != null && timestamp > 0; + + expect(isValid, isFalse); + }); + + test('should convert seconds to minutes correctly', () { + final expirationSeconds = 900; // 15 minutes + final expectedMinutes = (expirationSeconds / 60).round(); + + expect(expectedMinutes, equals(15)); + }); + + test('should handle fractional minute conversions', () { + final expirationSeconds = 930; // 15.5 minutes + final expectedMinutes = (expirationSeconds / 60).round(); + + expect(expectedMinutes, equals(16)); // Rounded up + }); + }); + }); +} \ No newline at end of file diff --git a/test/shared/providers/time_provider_test.dart b/test/shared/providers/time_provider_test.dart new file mode 100644 index 00000000..db87fa2b --- /dev/null +++ b/test/shared/providers/time_provider_test.dart @@ -0,0 +1,102 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:mostro_mobile/shared/providers/time_provider.dart'; + +void main() { + group('TimeProvider Tests', () { + late ProviderContainer container; + + setUp(() { + container = ProviderContainer(); + }); + + tearDown(() { + container.dispose(); + }); + + test('timeProvider provides DateTime values', () async { + final provider = container.read(timeProvider); + + // Test that provider is AsyncValue + expect(provider, isA>()); + + // Test when it has data + provider.when( + data: (value) { + expect(value, isA()); + final now = DateTime.now(); + final difference = now.difference(value).abs(); + expect(difference.inSeconds, lessThan(60)); // Within 1 minute + }, + loading: () => expect(true, isTrue), // Loading is ok + error: (error, stack) => fail('Should not have error'), + ); + }); + + test('countdownTimeProvider provides DateTime values', () async { + final provider = container.read(countdownTimeProvider); + + // Test that provider is AsyncValue + expect(provider, isA>()); + + // Test when it has data + provider.when( + data: (value) { + expect(value, isA()); + final now = DateTime.now(); + final difference = now.difference(value).abs(); + expect(difference.inSeconds, lessThan(60)); // Within 1 minute + }, + loading: () => expect(true, isTrue), // Loading is ok + error: (error, stack) => fail('Should not have error'), + ); + }); + + test('countdownTimeProvider debouncing logic', () async { + // Test debouncing logic conceptually + final now = DateTime.now(); + final sameSecond = DateTime(now.year, now.month, now.day, now.hour, now.minute, now.second); + final nextSecond = sameSecond.add(const Duration(seconds: 1)); + + // Should be different seconds + expect(nextSecond.second != sameSecond.second, isTrue); + + // Test time comparison logic + final lastEmittedTime = DateTime.now(); + final currentTime = DateTime.now().add(const Duration(seconds: 1)); + + final shouldEmit = lastEmittedTime.second != currentTime.second || + lastEmittedTime.minute != currentTime.minute || + lastEmittedTime.hour != currentTime.hour; + + expect(shouldEmit, isTrue); + }); + + test('countdownTimeProvider cleanup works correctly', () async { + // Create new container to test cleanup + final testContainer = ProviderContainer(); + final provider = testContainer.read(countdownTimeProvider); + + // Should be able to get a provider + expect(provider, isA>()); + + // Dispose container (triggers cleanup) + testContainer.dispose(); + + // Test passed if no exceptions thrown + expect(true, isTrue); + }); + + test('providers are independent', () async { + final timeProvider1 = container.read(timeProvider); + final countdownProvider1 = container.read(countdownTimeProvider); + + // Both should be valid AsyncValue + expect(timeProvider1, isA>()); + expect(countdownProvider1, isA>()); + + // Test that they are different providers (not the same reference) + expect(identical(timeProvider1, countdownProvider1), isFalse); + }); + }); +} \ No newline at end of file From d43c657d26c720be1a24af989b7d2e5309ed4c81 Mon Sep 17 00:00:00 2001 From: Catrya <140891948+Catrya@users.noreply.github.com> Date: Fri, 18 Jul 2025 15:34:27 -0600 Subject: [PATCH 2/3] fix tests --- lib/features/trades/screens/trade_detail_screen.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/features/trades/screens/trade_detail_screen.dart b/lib/features/trades/screens/trade_detail_screen.dart index 6a41b073..99e2fea2 100644 --- a/lib/features/trades/screens/trade_detail_screen.dart +++ b/lib/features/trades/screens/trade_detail_screen.dart @@ -693,8 +693,8 @@ class _CountdownWidget extends ConsumerWidget { final messageTime = DateTime.fromMillisecondsSinceEpoch(messageTimestamp); // Handle edge case: message timestamp in the future - if (messageTime.isAfter(now.add(const Duration(minutes: 5)))) { - // If message is more than 5 minutes in the future, likely invalid + if (messageTime.isAfter(now.add(const Duration(hours: 1)))) { + // If message is more than 1 hour in the future, likely invalid return null; } @@ -747,7 +747,7 @@ class _CountdownWidget extends ConsumerWidget { for (final message in sortedMessages) { // Additional validation: ensure timestamp is not in the future final messageTime = DateTime.fromMillisecondsSinceEpoch(message.timestamp!); - if (messageTime.isAfter(DateTime.now().add(const Duration(minutes: 5)))) { + if (messageTime.isAfter(DateTime.now().add(const Duration(hours: 1)))) { continue; // Skip messages with future timestamps } From 92481acdbf87f0378cd5d68762076a7b712bb836 Mon Sep 17 00:00:00 2001 From: Catrya <140891948+Catrya@users.noreply.github.com> Date: Fri, 18 Jul 2025 15:41:29 -0600 Subject: [PATCH 3/3] implement proper validation functions in countdown tests --- test/shared/countdown_validation_test.dart | 139 +++++++++++++-------- 1 file changed, 87 insertions(+), 52 deletions(-) diff --git a/test/shared/countdown_validation_test.dart b/test/shared/countdown_validation_test.dart index 1b429c12..0dab48fd 100644 --- a/test/shared/countdown_validation_test.dart +++ b/test/shared/countdown_validation_test.dart @@ -1,22 +1,57 @@ import 'package:flutter_test/flutter_test.dart'; +// Validation functions to test real logic instead of constants +bool isValidTimestamp(int? timestamp) { + return timestamp != null && timestamp > 0; +} + +bool isValidExpirationHours(int hours) { + return hours > 0 && hours <= 168; // Between 1 hour and 1 week +} + +bool isTimestampInFuture(DateTime timestamp, DateTime threshold) { + return timestamp.isAfter(threshold); +} + +bool isExpirationTooFarInPast(DateTime expiration, DateTime threshold) { + return expiration.isBefore(threshold); +} + +Duration calculateDurationComponents(int hours, int minutes, int seconds) { + return Duration(hours: hours, minutes: minutes, seconds: seconds); +} + +int clampHours(int hours, int maxHours) { + return hours.clamp(0, maxHours); +} + +String formatTime(int hours, int minutes, int seconds) { + return '${hours.toString().padLeft(2, '0')}:${minutes.toString().padLeft(2, '0')}:${seconds.toString().padLeft(2, '0')}'; +} + +int convertSecondsToMinutes(int seconds) { + return (seconds / 60).round(); +} + void main() { group('Countdown Validation Tests', () { group('Timestamp Validation', () { test('should reject negative timestamps', () { - final timestamp = -1000; - expect(timestamp <= 0, isTrue); + expect(isValidTimestamp(-1000), isFalse); }); test('should reject zero timestamps', () { - final timestamp = 0; - expect(timestamp <= 0, isTrue); + expect(isValidTimestamp(0), isFalse); + }); + + test('should reject null timestamps', () { + expect(isValidTimestamp(null), isFalse); }); test('should accept valid timestamps', () { final timestamp = DateTime.now().millisecondsSinceEpoch; - expect(timestamp > 0, isTrue); + expect(isValidTimestamp(timestamp), isTrue); }); test('should reject future timestamps beyond threshold', () { @@ -24,7 +59,15 @@ void main() { final futureTime = now.add(const Duration(minutes: 10)); final threshold = now.add(const Duration(minutes: 5)); - expect(futureTime.isAfter(threshold), isTrue); + expect(isTimestampInFuture(futureTime, threshold), isTrue); + }); + + test('should accept timestamps within threshold', () { + final now = DateTime.now(); + final recentTime = now.add(const Duration(minutes: 2)); + final threshold = now.add(const Duration(minutes: 5)); + + expect(isTimestampInFuture(recentTime, threshold), isFalse); }); }); @@ -34,7 +77,7 @@ void main() { final pastExpiration = now.subtract(const Duration(hours: 2)); final threshold = now.subtract(const Duration(hours: 1)); - expect(pastExpiration.isBefore(threshold), isTrue); + expect(isExpirationTooFarInPast(pastExpiration, threshold), isTrue); }); test('should accept recent expiration times', () { @@ -42,80 +85,72 @@ void main() { final recentExpiration = now.subtract(const Duration(minutes: 30)); final threshold = now.subtract(const Duration(hours: 1)); - expect(recentExpiration.isAfter(threshold), isTrue); + expect(isExpirationTooFarInPast(recentExpiration, threshold), isFalse); }); test('should validate expiration hours range', () { - expect(0 <= 0, isTrue); // Invalid: zero hours - expect(24 > 0 && 24 <= 168, isTrue); // Valid: 24 hours - expect(200 > 168, isTrue); // Invalid: over 1 week + expect(isValidExpirationHours(0), isFalse); // Invalid: zero hours + expect(isValidExpirationHours(24), isTrue); // Valid: 24 hours + expect(isValidExpirationHours(168), isTrue); // Valid: 1 week (max) + expect(isValidExpirationHours(200), isFalse); // Invalid: over 1 week + expect(isValidExpirationHours(-5), isFalse); // Invalid: negative hours }); }); group('Time Calculations', () { - test('should calculate hours correctly', () { - final duration = const Duration(hours: 5, minutes: 30, seconds: 45); - - final hours = duration.inHours; - final minutes = duration.inMinutes % 60; - final seconds = duration.inSeconds % 60; + test('should calculate duration components correctly', () { + final duration = calculateDurationComponents(5, 30, 45); - expect(hours, equals(5)); - expect(minutes, equals(30)); - expect(seconds, equals(45)); + expect(duration.inHours, equals(5)); + expect(duration.inMinutes % 60, equals(30)); + expect(duration.inSeconds % 60, equals(45)); }); test('should clamp hours to maximum', () { - final maxHours = 24; - final duration = const Duration(hours: 30); - - final hoursLeft = duration.inHours.clamp(0, maxHours); - expect(hoursLeft, equals(24)); + expect(clampHours(30, 24), equals(24)); + expect(clampHours(10, 24), equals(10)); + expect(clampHours(-5, 24), equals(0)); + expect(clampHours(168, 24), equals(24)); }); test('should handle zero duration', () { - final duration = const Duration(); - - final hours = duration.inHours; - final minutes = duration.inMinutes % 60; - final seconds = duration.inSeconds % 60; + final duration = calculateDurationComponents(0, 0, 0); - expect(hours, equals(0)); - expect(minutes, equals(0)); - expect(seconds, equals(0)); + expect(duration.inHours, equals(0)); + expect(duration.inMinutes, equals(0)); + expect(duration.inSeconds, equals(0)); }); test('should format time correctly', () { - final hours = 5; - final minutes = 7; - final seconds = 9; - - final formattedTime = '${hours.toString().padLeft(2, '0')}:${minutes.toString().padLeft(2, '0')}:${seconds.toString().padLeft(2, '0')}'; - - expect(formattedTime, equals('05:07:09')); + expect(formatTime(5, 7, 9), equals('05:07:09')); + expect(formatTime(0, 0, 0), equals('00:00:00')); + expect(formatTime(23, 59, 59), equals('23:59:59')); + expect(formatTime(100, 5, 3), equals('100:05:03')); }); }); group('Edge Cases', () { test('should handle null expiration timestamp', () { - final int? timestamp = null; - final isValid = timestamp != null && timestamp > 0; - - expect(isValid, isFalse); + expect(isValidTimestamp(null), isFalse); }); test('should convert seconds to minutes correctly', () { - final expirationSeconds = 900; // 15 minutes - final expectedMinutes = (expirationSeconds / 60).round(); - - expect(expectedMinutes, equals(15)); + expect(convertSecondsToMinutes(900), equals(15)); // 15 minutes + expect(convertSecondsToMinutes(60), equals(1)); // 1 minute + expect(convertSecondsToMinutes(0), equals(0)); // 0 minutes + expect(convertSecondsToMinutes(3600), equals(60)); // 60 minutes }); test('should handle fractional minute conversions', () { - final expirationSeconds = 930; // 15.5 minutes - final expectedMinutes = (expirationSeconds / 60).round(); - - expect(expectedMinutes, equals(16)); // Rounded up + expect(convertSecondsToMinutes(930), equals(16)); // 15.5 minutes rounded up + expect(convertSecondsToMinutes(870), equals(15)); // 14.5 minutes rounded up + expect(convertSecondsToMinutes(450), equals(8)); // 7.5 minutes rounded up + }); + + test('should validate edge cases for expiration hours', () { + expect(isValidExpirationHours(1), isTrue); // Minimum valid + expect(isValidExpirationHours(168), isTrue); // Maximum valid + expect(isValidExpirationHours(169), isFalse); // Just over maximum }); }); });