Skip to content

fix(notifications): resolve notification settings from the notified company - #274

Merged
roncodes merged 1 commit into
release/v1.6.65from
fix/notification-settings-queue
Sep 28, 2026
Merged

roncodes merged 1 commit into
release/v1.6.65from
fix/notification-settings-queue

Conversation

@roncodes

Copy link
Copy Markdown
Member

Fixes #262.

Why

NotificationRegistry::notify() read the company's notification settings with Setting::lookupCompany(), which returns nothing when there is no session('company'). Its callers have no session:

  • Fleetbase\FleetOps\Listeners\NotifyOrderEvent is ShouldQueue, so it runs in the queue worker.
  • Fleet-Ops' ProcessOperationalAlerts is a console command.

So whenever the queue runs as its own process (including the documented docker-compose setup), everything configured in Settings → Notifications was silently ignored.

notifyUsingDefinitionName() (used by Flespi) had a second problem: it read the global notification_settings key, but the console only ever writes company.<uuid>.notification_settings, so it never found any settings, even with a session.

What changed

  • Both methods resolve settings through a new resolveNotificationSettings(). It takes the company from the notification parameters: a Company, or the first model with a company_uuid. It falls back to the session only when no parameter carries a company.
  • New Setting::lookupForCompany($companyUuid, $key, $default) for reading a company setting without a session. lookupFromCompany() now delegates to it.

Verification

  • New tests:
    • no session, subject with a company: notifications are sent (the queue case);
    • the subject's company wins over a different session company;
    • a Company parameter is resolved;
    • the session fallback still works;
    • with no company anywhere, nobody is notified;
    • the global key is no longer read.
  • The existing notifyUsingDefinitionName tests move from the global key to the company-scoped one. These tests and the new ones fail without the fix.
  • Checked on the dev stack with a real order, inside a rolled-back transaction and with no session: the old lookup returned [], and the new one returned the company's settings.

…ompany

NotificationRegistry::notify() read company settings from the session, but
it is called from queued listeners and console commands, which have no
session. Settings -> Notifications was therefore ignored whenever the queue
ran in its own worker. notifyUsingDefinitionName() read the global
notification_settings key, which the console never writes.

Both now take the company from the notification parameters (a company, or a
model with a company_uuid) and fall back to the session. Adds
Setting::lookupForCompany() for looking up a company setting without a
session.

Closes #262

(cherry picked from commit 9c24673e7677c53d8d6adf01eb6b5ae98c40fb2c)
@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 (3965998) to head (17462b1).

Additional details and impacted files
@@             Coverage Diff             @@
##                main      #274   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
- Complexity      7499      7506    +7     
===========================================
  Files            430       430           
  Lines          24488     24502   +14     
===========================================
+ Hits           24488     24502   +14     
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.

@roncodes roncodes mentioned this pull request Sep 28, 2026
@roncodes
roncodes changed the base branch from main to release/v1.6.65 September 28, 2026 03:28
@roncodes
roncodes merged commit e6e3788 into release/v1.6.65 Sep 28, 2026
7 checks passed
@roncodes
roncodes deleted the fix/notification-settings-queue branch September 28, 2026 03:34
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.

NotificationRegistry::notify() reads company settings from the session, but its callers are queued listeners

1 participant