Skip to content

fix: rebuild push notifications on a single storefront push channel - #102

Merged
roncodes merged 2 commits into
release/v0.4.22from
fix/push-notification-layer
Sep 28, 2026
Merged

roncodes merged 2 commits into
release/v0.4.22from
fix/push-notification-layer

Conversation

@roncodes

@roncodes roncodes commented Sep 26, 2026 •

Copy link
Copy Markdown
Member

Summary

Push notifications were not reaching Android devices and some iOS devices. The cause was several separate bugs in how Storefront built push clients, picked credentials and registered devices. This PR replaces the per-notification APNs/FCM code with a single push channel.

Root causes fixed

# Problem Fix
A PushNotification::configureFcm put the entire Firebase service account JSON into credentials.private_key, on top of the platform-wide firebase.projects.app config, so a store's own FCM credentials never built a valid client. Android was broken. Push\FirebaseMessagingFactory decodes the channel's service account (and repairs escaped \n in the key). It builds a Messaging client straight from that account and never reads or changes firebase.projects.*.
B NotificationChannels\Fcm\FcmChannel needs the platform-wide Messaging in its constructor, which throws unless the instance also has platform-wide Firebase credentials. Storefront notifications no longer use FcmChannel/ApnChannel.
D Credentials were always taken from the order's storefront_id (the store), so marketplace (network) app users got no push, or pushes signed for the wrong app/project. PushCredentialResolver::storefrontsForOrder() prefers storefront_network_id and falls back to the store. If a provider reports a token as belonging to another app (SenderId mismatch / DeviceTokenNotForTopic), the send is retried on the next candidate channel.
E Each APNs channel had one production/sandbox flag, and ->first() picked a channel with no ordering. Tokens from development/sandbox builds were rejected with BadDeviceToken. APNs channels get environment: auto (the new default). The legacy production flag now only decides which environment is tried first, and a BadDeviceToken is retried in the other environment. If a device reports its APNs environment, only that environment is used. Channel order is deterministic.
F1 registerDevice used firstOrCreate(token, platform): a token was never moved to a newly logged-in customer, platform casing wasn't normalized (iOS matched nothing), and there was no validation. The token is unique per install and always moves to the latest customer; soft-deleted rows are restored and legacy duplicates removed. platform is validated and normalized. New POST customers/unregister-device endpoint for logout.
G Channel order was mail, database, apn, fcm, so an SMTP or toArray error (e.g. a null company) stopped the push. Storefront::autoAcceptOrder swallowed exceptions without logging. Push runs first and never throws. toArray is null-safe. The swallowed exception is now logged. Listeners guard against orders with no customer.
H No NotificationFailed handling: dead tokens were never removed and failures were never logged. Tokens reported Unregistered/NotFound/invalid, or BadDeviceToken in every environment, are marked invalid and soft-deleted. Every other failure is logged with [Storefront Push] context.
I FCM payloads had no android.priority: high (delayed in Doze mode), and APNs messages had no sound. PushMessage renders high priority, an optional android.channel_id, the apns-push-type/apns-priority headers, and sound/badge.

Other changes

  • The eight order notifications now extend StorefrontOrderNotification, removing about 1,100 duplicated lines. Database payload: message now always holds the human-readable body (it used to be the status code for most types). Added type, title, body, order, order_id, store_id and network_id; all existing keys are kept.
  • PromotionalPushNotification also writes to the database channel, so promotions appear in the customer inbox (see the follow-up inbox API). It tries the store's app first, then the apps of the networks the store belongs to.
  • New admin endpoint POST storefront/int/v1/notification-channels/{id}/test, plus a Send test button on channel lists (store settings and network page). It sends a test push to a pasted token and shows the raw provider result per environment, so misconfigured credentials can be diagnosed from the console.
  • Channel form: APNs gets an environment select (auto / production / sandbox). FCM drops the unused firebase_database_url/firebase_project_name fields and adds an optional android_channel_id. New env var STOREFRONT_PUSH_ANDROID_CHANNEL_ID.
  • Removed Support\PushNotification (no callers outside this package; fleetops uses core-api's own helper).

Related PRs

Related Issue

No tracking issue. Reported directly: push notifications not sending on Android and some iOS devices.

Type of Change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Test
  • Chore

Implementation Notes

  • Push\StorefrontPushChannel groups a customer's devices by platform (and by the app they registered from, when core-api#276 is deployed). It then walks the candidate channels: device's app → network → store, oldest first. For iOS it also walks the candidate APNs environments. Per-token outcomes come from Transports\FcmTransport / ApnTransport as PushOutcome (sent / dead / wrong_app / wrong_environment / error), which drives the retries and pruning.
  • The transports are the only code that talks to Google or Apple, so the retry/prune logic is unit tested with fake transports.
  • Notifications are still sent synchronously, as before. Queuing them is left as a follow-up to avoid requiring a worker in this PR.

Validation

  • Tests
  • Lint
  • Build
  • Manual validation (needs real FCM/APNs credentials: use the new Send test button against an Android token and iOS dev + App Store tokens)

Command output / summary:

composer test:unit   -> 477 tests, 3067 assertions, 0 failures (main: 440 tests)
composer test:lint   -> Found 0 of 261 files that can be fixed
pnpm lint            -> js/css/intl clean; lint:hbs: 2 errors that already exist on main
                        (addon/components/widget/customers.hbs:35, widget/orders.hbs:49)
pnpm build           -> production build succeeds
composer test:types  -> already broken on main: phpstan.neon.dist points at non-existent `src`

New or rewritten tests: server/tests/Unit/Push/PushLayerTest.php, Http/Controllers/CustomerDeviceRegistrationTest.php, Http/Controllers/NotificationChannelTestPushTest.php, and Notifications/NotificationContractsTest.php (the old version asserted the broken credentials.private_key shape).

Documentation Impact

  • No documentation changes needed
  • Documentation updated in fleetbase/fleetbase.io
  • Documentation needed but not included

API Reference Impact

  • No API reference changes needed
  • Updated fleetbase/postman
  • API reference updates required but not included

API reference notes:

  • New POST storefront/v1/customers/unregister-device (token).
  • POST storefront/v1/customers/register-device now requires token and platform/os (ios or android, case-insensitive), accepts an optional environment (production/sandbox), and returns an error for invalid input instead of storing null rows.
  • New internal POST storefront/int/v1/notification-channels/{id}/test (token, optional environment, title, body).

Documentation Notes

fleetbase.io, Storefront → notification channels / push setup:

  • APNs environment (auto / production / sandbox).
  • FCM: paste the full service account JSON; optional android_channel_id; new STOREFRONT_PUSH_ANDROID_CHANNEL_ID env var.
  • Marketplace apps should configure channels on the network.
  • The Send test troubleshooting flow.

Risk

  • Database payload of order notifications: message is now the readable body for every type (it used to be the status code for most). Existing keys are kept and new keys added.
  • Order status notifications are now also sent through the network's channels for marketplace orders. Stores that only configured channels on the store still work, because the store is the fallback.
  • APNs channels with production: true now retry sandbox after a BadDeviceToken: at most one extra request for tokens Apple rejects.
  • Dead tokens are soft-deleted (status = invalid); re-registering the same token restores it.
  • Ops: confirm the deployed version is ≥ v0.4.18 (b3a42fa), and that a queue worker is running for the queued order listeners.

Screenshots / Recordings

Console: a new Send test button on notification channel rows (store settings → notifications, and the network page), which opens a modal showing the provider result; and an APNs environment select in the channel form. Not captured.

Android pushes never worked with store-level FCM credentials and some iOS
devices were rejected, because of several stacked defects:

- configureFcm put the service account JSON into credentials.private_key,
  so no valid Firebase client was ever built from a store channel
- FcmChannel required the platform-wide Firebase project to be configured
- credentials were always resolved from the store, never the network app
- one APNs environment per channel rejected sandbox/dev build tokens
- registerDevice never reassigned a token to the latest customer and did
  not normalize platform casing
- mail/database errors ran before push and stopped it; failures and dead
  tokens were never handled or logged

Introduce Push\StorefrontPushChannel with isolated Firebase/APNs clients,
network-first credential resolution with wrong-app and wrong-environment
retries, dead token pruning, and high priority payloads. Order
notifications share a StorefrontOrderNotification base class. Add a
customers/unregister-device endpoint and an admin test push action.
@roncodes roncodes added needs-docs Requires documentation updates needs-api-spec Requires API specification updates needs-human-review Requires human review before proceeding type:bug Bug fix type:refactor Refactor labels Sep 26, 2026
@codecov

codecov Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (7f0ba99) to head (28fdf2d).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##                main      #102   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
- Complexity      1775      1867   +92     
===========================================
  Files            135       144    +9     
  Lines           7785      7755   -30     
===========================================
- Hits            7785      7755   -30     
Flag Coverage Δ
backend 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@roncodes roncodes mentioned this pull request Sep 26, 2026
6 of 16 tasks
Codecov requires full patch coverage. Cover the FCM and APNs transport
send paths, PushMessage setters, APNs environment short-circuit, explicit
push routes, and pruning/logging failure handling. Read device
attributes with data_get so devices returned by a custom push route do
not need to be Eloquent models, and drop two unreachable branches.
@roncodes
roncodes changed the base branch from main to release/v0.4.22 September 28, 2026 03:21
@roncodes
roncodes merged commit 9c039a5 into release/v0.4.22 Sep 28, 2026
11 checks passed
@roncodes
roncodes deleted the fix/push-notification-layer branch September 28, 2026 04:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-api-spec Requires API specification updates needs-docs Requires documentation updates needs-human-review Requires human review before proceeding type:bug Bug fix type:refactor Refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant