Skip to content

feat: add PushStateReporter for push reachability reports - #40

Merged
anilcancakir merged 2 commits into
masterfrom
feature/push-state-report
Sep 27, 2026
Merged

anilcancakir merged 2 commits into
masterfrom
feature/push-state-report

Conversation

@anilcancakir

Copy link
Copy Markdown
Member

Summary

  • PushStateReporter, reached as Notify.pushState. The device tells its backend whether a push can reach it (subscription id, permission, opt-in) and withdraws that on sign-out. Moved here from the one app that had written it.
  • Off by default, with no default path. notifications.push_state.report_path and release_path are null in the install stub, so an app whose backend has no such route sends nothing; external_id_prefix defaults to user_, matching magic-starter-laravel. isConfigured tells the wiring code whether to attach; release is idempotent and skips a device with no subscription.

The matching backend routes are in the magic-starter-laravel PR of this set.

Verification

flutter analyze clean, flutter test 735 passing, format clean.

Part of a coordinated set

This PR is one of eight that move reusable, app-agnostic pieces out of Uptizm into the magic framework and its plugins, so any magic app can use them the Laravel way.

Merge order:

  1. fluttersdk/magic first: every plugin PR compiles against its new API (SessionScope, Repository, MagicAction, Event.listenAny, ReportsBreadcrumb).
  2. Then magic_deeplink, magic_notifications, magic_payments and magic_sentry, in any order.
  3. Then magic_starter, which uses magic core's SessionScope, magic_payments' StoreIdentitySync and magic_deeplink's gate.
  4. magic-starter-laravel is independent of the Flutter side.
  5. anilcancakir/uptizm last.

Until magic merges and ships, CI on the plugin PRs resolves the published magic and is expected to be red. Each branch was verified locally against the sibling working trees (analyze, the full test suite, format check).

No version bump, publish or tag is included; a release is a separate step.

@kodizm

kodizm Bot commented Sep 26, 2026

Copy link
Copy Markdown

Note

Kodizm (AI-generated). May contain mistakes; verify before acting.

The reporter's logic looks correct and well tested, but the package does not yet declare that it needs the newer magic API it now calls.

Major

pubspec.yaml:30: push_state_reporter.dart:153 calls Event.listenAny, which the PR description says is new in the unmerged magic PR. The constraint is still magic: ^0.0.16, so once magic ships, a solver can still resolve 0.0.16 and this package will fail to compile for that adopter (correctness). The comments above this constraint say the floor is raised whenever a new magic API is required (see the Echo.connection note), so the floor should go up to the magic release that ships listenAny before this is published.

Minor

lib/src/support/push_state_reporter.dart:116: isConfigured only checks report_path, but the class doc (line 22) and the stub say reporting stays off until both endpoints are named. If an app sets only report_path, watch() reports and release() silently does nothing, so the device keeps receiving pages after sign-out, which is the problem this class exists to prevent. Either require both paths in isConfigured, or log when report_path is set without release_path.
README.md: Notify.pushState is new public API, but the README does not mention it. CLAUDE.md's Post-Change Checklist asks for the README to be updated when the API changes (the doc/ pages were updated).

Tests

test/support/push_state_reporter_test.dart covers configuration, the pass filter, wire shape, the memo across sign-out, the driver-less path, late driver attach and release single-flight and retry. Its assertions were not read in detail, and CI did not confirm they pass.

CI

  • Lint & Test: failure (the only annotation is "Process completed with exit code 1", so the cause is in the job log, which I can't read. The PR says red is expected until magic publishes the new API).
  • Auto-merge low-risk Dependabot PRs: skipped.

…ent Notify.pushState in the README, raise the magic floor to ^0.0.22
@anilcancakir

Copy link
Copy Markdown
Member Author

Fixed in 32ebbd1.

Major, pubspec.yaml:30 (floor): magic moves ^0.0.16 to ^0.0.22 (pubspec.yaml:31). magic#205 ships Event.listenAny and releases as magic 0.0.22. The pubspec comment names the reason, and CHANGELOG.md gains a ### Changed line under [Unreleased]. doc/architecture/notification-manager.md:232 said "magic ^0.0.6 is the floor". That has been stale since 0.0.7, so it now says the floor cannot sit below 0.0.6.

Minor, lib/src/support/push_state_reporter.dart:116 (isConfigured): I took the "require both" option.

  • isConfigured is now _reportPath != null && _releasePath != null.
  • watch() returns without arming anything when only one path is named, and logs an error through NotificationLog: "Push state reporting stays off: set both notifications.push_state.report_path and notifications.push_state.release_path, or neither".
  • Why not just log a warning and keep reporting: a device that no sign-out can withdraw would keep vouching for whoever left it last, and silence is safer than that.
  • Tests, red before the fix (+38 -2):
    • an absent release path reads as not configured.
    • a report path without a release path reports nothing, and says so. It asserts no push-state request at all, and exactly one log entry that names release_path.
  • The old an absent release path releases nothing expected one report, which was the half-configured behaviour. It is replaced by the second test above.

Minor, README missing Notify.pushState: new section "Tell Your Backend Whether the Device Can Be Paged". It gives the config keys and watch(), and links to the backend contract and the shipping-push page. The "Clean Up on Logout" sample now releases before logoutPush().

Backend cross-check against magic-starter-laravel#45 (origin/feature/push-devices):

  • src/routes/api.php registers POST devices/push-state and POST devices/push-state/release, the documented paths.
  • StorePushDeviceStateRequest requires external_id / subscription_id (present, nullable), reachability (in REACHABILITY_VALUES) and captured_at (date). These are exactly the four keys PushDeliverySnapshot.toMap() sends.

Gates, run locally against magic#205's head (89c4daa) through a temporary pubspec_overrides.yaml:

Formatted 110 files (0 changed)
flutter analyze --no-fatal-infos: No issues found!
flutter test: +736 ~1: All tests passed!

CI: Lint & Test stays red until magic 0.0.22 is on pub.dev. CI resolves the hosted magic, and Event.listenAny is not published yet. That is expected; CI will be re-run once the release is live.

@kodizm please re-review.

@kodizm

kodizm Bot commented Sep 27, 2026

Copy link
Copy Markdown

Note

Kodizm (AI-generated). May contain mistakes; verify before acting.

All three earlier findings are fixed in 32ebbd1, and I found nothing new. The only thing left is the CI failure, which the PR expects until magic 0.0.22 is published.

What changed since my last review:

  • The magic floor is now ^0.0.22 (pubspec.yaml:31). The pubspec comment and a ### Changed entry in the CHANGELOG explain why.
  • isConfigured now needs both paths (push_state_reporter.dart:120). If only one path is set, watch() does nothing and logs an error.
  • release() still reads _releasePath on its own, so nothing changes for an app that has configured both paths.
  • The README has a new Notify.pushState section, and the logout sample now calls release before logoutPush(). Its links point to anchors that exist in shipping-push.md:138 and laravel-backend-setup.md:387.

Tests

test/support/push_state_reporter_test.dart adds two cases, "an absent release path reads as not configured" and "a report path without a release path reports nothing, and says so". Together they cover the half-configured path. CI has not confirmed that they pass.

CI

  • Lint & Test: failure. The only annotation is "Process completed with exit code 1", so the cause is in the job log, which I can't read. The PR says CI stays red until magic 0.0.22, which ships Event.listenAny, is on pub.dev. Re-run it once that release is live, and don't call this green before then.
  • Auto-merge low-risk Dependabot PRs: skipped.

@codecov

codecov Bot commented Sep 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.91525% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
lib/src/support/push_state_reporter.dart 94.78% 6 Missing ⚠️

📢 Thoughts on this report? Let us know!

@anilcancakir
anilcancakir merged commit d4e719c into master Sep 27, 2026
4 of 5 checks passed
@anilcancakir
anilcancakir deleted the feature/push-state-report branch September 27, 2026 15:47
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.

1 participant