Skip to content

Conversation

@AndreaDiazCorreia
Copy link
Member

@AndreaDiazCorreia AndreaDiazCorreia commented Dec 5, 2025

Fixes background notifications that were working in debug mode but failing silently in release builds. The notification icon resource was not being found at runtime in release builds. Changed to use @drawable/ic_bg_service_small which is guaranteed to exist in all build modes. Fixes #377

Summary by CodeRabbit

  • Chores
    • Added Flutter version management config and excluded its cache from version control.
  • Improvements
    • Added a dedicated notification icon metadata for consistent local notifications.
    • Minor robustness and null-safety refinements in background notification handling.
  • New Features
    • Notifications now include fiat currency code and amount when a fiat payment is marked sent.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 5, 2025

Walkthrough

Added FVM config and .gitignore entry; introduced Android notification icon metadata; tightened null-safety and early-return handling in background notification code; extended notification data extraction to handle an Action.fiatSent payload.

Changes

Cohort / File(s) Change Summary
FVM config
\.fvmrc
Added Flutter version spec locking Flutter to 3.35.7 ({ "flutter": "3.35.7" }).
VCS ignore
\.gitignore
Added .fvm/ entry to ignore FVM version cache.
Android manifest metadata
android/app/src/main/AndroidManifest.xml
Added metadata entry com.app.notification_icon pointing to @drawable/ic_notification.
Background notification logic
lib/features/notifications/services/background_notification_service.dart, lib/background/background.dart
Reformatted several single-line conditionals to block form with explicit early returns and added null/error early-return branches; no behavioral changes on success paths.
Notification data extraction
lib/features/notifications/utils/notification_data_extractor.dart
Added handling for Action.fiatSent to extract fiat_code and fiat_amount from an Order payload when present.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Check AndroidManifest metadata vs. actual drawable resources and usage in notification code.
  • Verify null-safety/error-return changes in background notification service preserve intended behavior.
  • Validate assumptions about Order payload structure in notification_data_extractor.dart.

Possibly related PRs

Suggested reviewers

  • Catrya
  • grunch

Poem

🐰
I hopped through files with nimble paws,
Pinned flutter steps and nudged the laws.
Icons whispered, payloads found,
Early returns kept things sound.
A little hop — deploy applause!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly addresses the main fix: background notifications failing in release builds, which is the primary objective of this PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/backgroun-notifications

📜 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 d93efbe and cc190f6.

📒 Files selected for processing (2)
  • lib/background/background.dart (1 hunks)
  • lib/features/notifications/services/background_notification_service.dart (4 hunks)
✅ Files skipped from review due to trivial changes (1)
  • lib/background/background.dart
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/features/notifications/services/background_notification_service.dart
⏰ 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

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.

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.

@AndreaDiazCorreia las notificaciones en background funcionan, pero la UI no se actualiza, por ejemplo:

  • pongo orden de venta
  • la toman y recibo notificacion de Esperando invoice (aun no abro la app)
  • el comprador pone la invoice y me sale la notificacion de Esperando pago
  • al abrir la app me sale la factura con el QR para pagar y hasta ahi bien
  • al regresar a Mi Trades la orden sale como Esperando factura (el status anterior) y en order details igual, y claro no sale el QR de nuevo para seguir

si la orden es de compra pasa al reves, pero igual se queda en el status anterior la ui

@Catrya
Copy link
Member

Catrya commented Dec 5, 2025

Volvi a probar, desinstalando todo y volviendo a instalar y ya se actualizan los status que antes quedaban trabados en ordenes de venta, en las de compra no me sale la notificacion de Esperando factura
Sin embargo las notificaciones en background de status Active y Fiatsent no me salen

Changes the local notification icon from `ic_bg_service_small` to `ic_notification` and adds metadata to preserve the dedicated notification icon in AndroidManifest.xml
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.

ya funciona correctamente la notificación de fiat-sent, pero aun no llega la de Active al creador de un oferta de venta, en los demás casos si llega la notificación de Active, pero en ese no, seria la acción buyer-took-order.

Para reproducirlo:

  • crear orden de venta
  • cuando la tomen y toque pagar la factura, ir a la wallet y pagar, ahi deberia llegar la notificacion pero no llega nunca

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

Copy link
Member

@grunch grunch left a comment

Choose a reason for hiding this comment

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

LGTM

@grunch grunch merged commit e0ea3b9 into main Dec 10, 2025
2 checks passed
@grunch grunch deleted the fix/backgroun-notifications branch December 10, 2025 14:05
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.

Background notifications not showing in release builds

4 participants