[arimu1]

Description

Fixes #3113

When a push notification was sent to a USER or REALM target, console assets were only included in target mapping if they already had a non-empty FCM token in CONSOLE_PROVIDERS. Consoles without a token were dropped during mapping, which led to an empty target list and the misleading MISSING_TARGETS / "Notification targets must be set" error.

Direct ASSET console targets already kept consoles without a token so PushNotificationHandler.sendMessage could record No FCM token found for console: …. USER/REALM expansion now uses the same behaviour by not requiring a token in the asset query.

Checklist

• 1. Acceptance criteria of the linked issue(s) are met
• 2. Tests are written and all tests pass
• 3. Changes are manually tested by you and the reviewer
• 4. Documentation is written or updated

Test plan

• ./gradlew :manager:compileJava :test:compileTestGroovy (JDK 21)
• NotificationTest integration spec (requires docker compose -f profile/dev-testing.yml -p openremote up -d --no-build; Docker stack was not running in the implementator environment)
• Manual: send push to a user whose linked console has no FCM token; notifications UI shows No FCM token found for console instead of targets must be set

[CLAassistant]

All committers have signed the CLA.

[MartinaeyNL]

First of all, thank you for creating a pull request! 🥳

You need to sign the contributor license agreement as stated by the bot above 😉
I'm happy to take a look after that's been resolved.

[arimu1]

CLA is signed (license/cla is green).

The missing-FCM-token check now runs before the dev-mode skip. In the last integration run, NotificationTest expected a 400 for a user target with no token, but the skip returned success because FCM is not configured in that job. Tip 403996ef3 also includes current master.

The two GatewayTest failures in that run are outside this change.

[Ekhorn]

This is an initial AI-assisted review.

The new step for a user whose consoles all lack a token covers the scenario from #3113, and checking the token before the dev-mode skip makes dev mode record the same failure as production.

Removing the token predicate from target expansion goes beyond the issue, though: browser and anonymous consoles, which never have an FCM token, become USER and REALM targets, so ordinary sends to those targets return 400 and store a failed notification per browser console. Details inline.

[Ekhorn]

Dropping the token predicate widens USER, REALM and linked-user ASSET expansion to every console asset, not just FCM consoles that lost their token:

• Every browser that loads the manager registers a console whose push provider has version web and never carries a token (ui/component/core/src/console.ts:486-500), and manager/src/main/java/org/openremote/manager/asset/console/ConsoleResourceImpl.java:177-183 links it to the logged-in user. Anonymous browser consoles and consoles without any push provider are included too.
• Each of these consoles fails with "No FCM token found for console", and one failed target makes the whole exchange return false (manager/src/main/java/org/openremote/manager/notification/NotificationService.java:359-362), so sendNotification answers 400 even when the user's phone received the push. The rewritten step at test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy:398-408 shows this: testuser3Console2 still has a token, yet the send that used to succeed now expects 400.
• A REALM broadcast now stores one failed SentNotification for every browser console in the realm on each send, and runs the per-console user query for each of them.

To keep the fix scoped to #3113, the token-less consoles could be mapped only when no console with a token is found for a target, so MISSING_TARGETS is replaced by a recorded failure without changing the result of normal sends. Limiting expansion to consoles whose push provider is fcm, the rule isLinkedToFcmProvider (manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java:602-611) applies in start(), would also keep browser consoles out. A REALM step with a token-less console in the spec would pin down whichever behavior is chosen.

[Ekhorn]

The handler's token cache is updated by an asynchronous route on PERSISTENCE_TOPIC (a SEDA endpoint), so this send can still read the old token for testuser3Console2. When it does, that console is sent without an error and the console2Notifications assertion below times out. The step that clears the token on testuser3Console1 already waits for the cache before sending; the same wait here removes the race:

Suggested change

	assetStorageService.merge(testUser3Console2Asset)
	assetStorageService.merge(testUser3Console2Asset)
	conditions.eventually {
	assert pushNotificationHandler.consoleFCMTokenMap.get(testUser3Console2Asset.id) == null
	}

[arimu1]

Addressed the review on tip d8a5deb1b (includes current main).

• A user, realm, or linked-asset send still expands only to consoles that have an FCM token, so a browser console does not turn a normal send into a 400.
• When that expansion is empty, FCM consoles (push version fcm) are mapped instead, and the send records "No FCM token found" rather than a missing-targets error. That is the Misleading 'targets must be set' error when sending to a user target without a valid FCM token #3113 case.
• The all-missing step waits until consoleFCMTokenMap drops testuser3Console2 before sending.
• :manager:compileJava and :test:compileGroovy passed. I did not rerun NotificationTest here (it needs the full manager stack).

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud