Skip to content

Feat : Logger Singleton Migration - #429

Merged
grunch merged 16 commits into
mainfrom
feat/logging-phase-7
Feb 5, 2026
Merged

Feat : Logger Singleton Migration#429
grunch merged 16 commits into
mainfrom
feat/logging-phase-7

Conversation

@BraCR10

@BraCR10 BraCR10 commented Feb 5, 2026

Copy link
Copy Markdown
Member

Phase 7: Logger Singleton Migration

This PR completes the migration to the centralized logger singleton and updates documentation.

What's Included

  • App-wide replacement of direct Logger() usage with the singleton logger from logger_service.dart.
  • Lint fixes and const constructor updates following the migration.
  • Documentation updates for Phase 7 completion and logging guidance.

Testing

  • Verify that all logs are displayed in the user interface as they are displayed on the console.

Summary by CodeRabbit

  • New Features

    • Send messages in dispute conversations
    • Retrieve shared encryption key for chat sessions
    • Better trade filtering that reflects order state
    • Incremental restore progress reporting
  • Bug Fixes

    • Improved download/network timeout handling and error reporting
  • Documentation

    • Updated logging guidelines to mandate the centralized logger

- app_routes.dart: Use singleton logger
- deep_link_handler.dart: Replace Logger() with logger
- deep_link_interceptor.dart: Replace Logger() with logger
- dispute_repository.dart: Replace Logger() with logger
- open_orders_repository.dart: Replace Logger() with logger
- chat_room_provider.dart: Replace Logger() with logger
- chat_rooms_notifier.dart: Replace Logger() with logger
- chat_room_notifier.dart: Replace Logger() with logger
- chat_room_providers.dart: Replace Logger() with logger
- dispute_chat_notifier.dart: Replace Logger() with logger
- logs_actions_menu.dart: Replace Logger() with logger
- background_notification_service.dart: Replace Logger() with logger
- notification_data_extractor.dart: Replace Logger() with logger
- order_state.dart: Replace Logger() with logger
- rate_counterpart_screen.dart: Replace Logger() with logger
- restore_manager.dart: Replace Logger() with logger
- restore_progress_notifier.dart: Replace Logger() with logger
- settings_notifier.dart: Replace Logger() with logger
- trades_provider.dart: Replace Logger() with logger
- status_filter_widget.dart: Replace Logger() with logger
- blossom_client.dart: Replace Logger() with logger
- blossom_download_service.dart: Replace Logger() with logger
- blossom_upload_helper.dart: Replace Logger() with logger
- encrypted_file_upload_service.dart: Replace Logger() with logger
- encrypted_image_upload_service.dart: Replace Logger() with logger
- encryption_service.dart: Replace Logger() with logger
- fcm_service.dart: Replace Logger() with logger
- file_validation_service.dart: Replace Logger() with logger
- image_upload_service.dart: Replace Logger() with logger
- lifecycle_manager.dart: Replace Logger() with logger
- media_validation_service.dart: Replace Logger() with logger
- push_notification_service.dart: Replace Logger() with logger
- session_notifier.dart: Replace Logger() with logger
- notification_listener_widget.dart: Replace Logger() with logger
- pay_lightning_invoice_widget.dart: Replace Logger() with logger
- Mark Phase 7 as completed
- Document migration of 40+ files to singleton logger
- Add reference to check_logger_usage.dart tool
Remove duplicate imports of logger_service.dart in:
- logs_actions_menu.dart
- settings_notifier.dart

Ensure pay_lightning_invoice_widget.dart uses singleton logger correctly.
Mark constructors as const in:
- LogsActionsMenu
- PayLightningInvoiceWidget

Resolves prefer_const_constructors_in_immutables lint warnings.
Ensures that all logging within the application exclusively uses the centralized logger from `logger_service.dart`.

Updates documentation to reflect the completion of the logger migration and reinforces the prohibition of direct `Logger()` instantiation.
@coderabbitai

coderabbitai Bot commented Feb 5, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

This PR migrates per-file Logger instances to a centralized singleton logger_service across the codebase, updates docs to mark Phase 7 completed, and adds a few new public APIs and providers (ChatRoomNotifier.getSharedKey, DisputeRepository.sendDisputeMessage, RestoreProgressNotifier.incrementProgress, filteredTradesWithOrderStateProvider).

Changes

Cohort / File(s) Summary
Documentation
AGENTS.md, docs/LOGGING_IMPLEMENTATION.md
Docs updated to require using the pre-configured singleton logger (services/logger_service.dart); Phase 7 marked completed and direct Logger() instantiation prohibited.
Core / Deep-linking
lib/core/app_routes.dart, lib/core/deep_link_handler.dart, lib/core/deep_link_interceptor.dart
Replaced local Logger usage with centralized logger; added deepLinkService.initialize() call in deep link handler.
Data Repositories
lib/data/repositories/dispute_repository.dart, lib/data/repositories/open_orders_repository.dart
Switched to centralized logger; added sendDisputeMessage(String, String) to DisputeRepository.
Chat layer
lib/features/chat/...
lib/features/chat/chat_room_provider.dart, lib/features/chat/notifiers/chat_room_notifier.dart, lib/features/chat/notifiers/chat_rooms_notifier.dart, lib/features/chat/providers/chat_room_providers.dart
Logging migrated to logger; ChatRoomNotifier gains Future<Uint8List> getSharedKey().
Notifications & FCM
lib/services/fcm_service.dart, lib/features/notifications/...
lib/features/notifications/services/background_notification_service.dart, lib/features/notifications/utils/notification_data_extractor.dart, lib/services/push_notification_service.dart
Replaced ad-hoc Logger usage with centralized logger across notification and FCM flows.
Blossom / Upload/Download & Validation
lib/services/blossom_client.dart, lib/services/blossom_download_service.dart, lib/services/blossom_upload_helper.dart, lib/services/encrypted_file_upload_service.dart, lib/services/encrypted_image_upload_service.dart, lib/services/file_validation_service.dart, lib/services/image_upload_service.dart, lib/services/media_validation_service.dart
Switched to centralized logger; blossom_download_service adds request timeout, explicit headers, and improved network error handling.
Core Services
lib/services/encryption_service.dart, lib/services/lifecycle_manager.dart
Replaced local Logger with centralized logger; lifecycle manager message updated (deprecation text).
Disputes
lib/features/disputes/notifiers/dispute_chat_notifier.dart
Logging migrated to centralized logger.
Logs / Orders / Trades
lib/features/logs/widgets/logs_actions_menu.dart, lib/features/order/models/order_state.dart, lib/features/trades/providers/trades_provider.dart, lib/features/trades/widgets/status_filter_widget.dart
Logger migrated to centralized logger; trades_provider adds filteredTradesWithOrderStateProvider (reactive filtering by OrderState); logs_actions_menu constructor made const.
Restore & Rate features
lib/features/restore/restore_manager.dart, lib/features/restore/restore_progress_notifier.dart, lib/features/rate/rate_counterpart_screen.dart
Logging migrated to logger; RestoreProgressNotifier adds incrementProgress(), timeout timer management, and auto-hide behaviors.
Settings & Session
lib/features/settings/settings_notifier.dart, lib/shared/notifiers/session_notifier.dart
Switched logging to centralized logger.
Shared Widgets
lib/shared/widgets/notification_listener_widget.dart, lib/shared/widgets/pay_lightning_invoice_widget.dart
Logging migrated to centralized logger; PayLightningInvoiceWidget made const and had internal Logger field removed.

Sequence Diagram(s)

(Skipped — changes are primarily logging replacement and small provider/API additions; no new multi-component control-flow requiring visualization.)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • Catrya
  • grunch
  • AndreaDiazCorreia

Poem

🐇 I hopped through files both far and near,

Replaced lone loggers with one I hold dear,
Now messages gather, tidy and bright,
One little logger keeps day and night,
Hooray — unified logs, a rabbit's delight!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title 'Feat : Logger Singleton Migration' clearly summarizes the main change—migrating to a singleton logger across the application—and accurately reflects the primary objective of this comprehensive pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/logging-phase-7

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
lib/shared/widgets/pay_lightning_invoice_widget.dart (1)

78-111: ⚠️ Potential issue | 🟠 Major

Avoid logging full Lightning invoices (sensitive).
With centralized logging (and UI log display), this exposes payment data. Please redact or log only metadata (length/prefix).

🔐 Suggested redaction (log only length)
-                logger.i('Copied LN Invoice to clipboard: ${widget.lnInvoice}');
+                logger.i('Copied LN Invoice to clipboard (length: ${widget.lnInvoice.length})');
...
-                    logger.i('Launched Lightning wallet with invoice: ${widget.lnInvoice}');
+                    logger.i('Launched Lightning wallet with invoice (length: ${widget.lnInvoice.length})');
...
-                    logger.i('Shared LN Invoice via share sheet: ${widget.lnInvoice}');
+                    logger.i('Shared LN Invoice via share sheet (length: ${widget.lnInvoice.length})');
lib/features/restore/restore_progress_notifier.dart (1)

91-97: ⚠️ Potential issue | 🟠 Major

Localize the timeout error message.
'Request timeout' is user-facing and should come from ARB/localizations (or via a localization key mapped in UI), not a hard-coded literal.

As per coding guidelines, Localize all user-facing strings via ARB files and access them with S.of(context) rather than hard-coded literals.

lib/data/repositories/dispute_repository.dart (1)

119-122: ⚠️ Potential issue | 🟠 Major

Remove or implement sendDisputeMessage stub.

The method is currently unused throughout the codebase and is just a mock delay. If this is placeholder code for future implementation, replace the mock with throw UnimplementedError('sendDisputeMessage is not implemented yet'); to fail fast and prevent silent failures. Alternatively, if no longer needed, remove it entirely.

🤖 Fix all issues with AI agents
In `@docs/LOGGING_IMPLEMENTATION.md`:
- Around line 214-216: Remove the duplicate "Last Updated" entry so only the
most recent date remains; specifically delete the older line "**Last Updated**:
2026-01-21" and keep "**Last Updated**: 2026-02-02" so the section shows a
single Last Updated entry.
🧹 Nitpick comments (2)
lib/features/trades/providers/trades_provider.dart (1)

42-53: Potential performance consideration with many orders.

Watching individual orderNotifierProvider for each order in a loop (line 46) is necessary for reactive updates but could impact performance with a large number of orders. The current implementation with try-catch is defensive and handles failures gracefully.

Consider monitoring performance if the order count grows significantly in the future.

lib/features/chat/providers/chat_room_providers.dart (1)

47-48: Consider removing the orphaned comment.

The comment on line 47 ("// Logger instance for session start time operations") now refers to a removed field. Consider removing this comment to avoid confusion.

🧹 Proposed cleanup
-// Logger instance for session start time operations
-
+

Comment thread docs/LOGGING_IMPLEMENTATION.md Outdated
@BraCR10 BraCR10 changed the title Feat/logging phase 7 Feat : Logger Singleton Migration Feb 5, 2026

@Catrya Catrya left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

tACK

@grunch grunch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

utACK

@grunch
grunch merged commit 84971b2 into main Feb 5, 2026
2 checks passed
@grunch
grunch deleted the feat/logging-phase-7 branch February 5, 2026 21:02
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