Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions config/config.exs
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,8 @@ config :logger, :console,
config :phoenix, :json_library, Jason

# Filter FCM tokens from logs
config :phoenix, :filter_parameters, ["fcm_token"]
config :logster, :filter_parameters, ["fcm_token"]
config :phoenix, :filter_parameters, ["fcm_token", "fcm_installation_id"]
config :logster, :filter_parameters, ["fcm_token", "fcm_installation_id"]

# Use Req for making HTTP requests
config :mobile_app_backend, MobileAppBackend.HTTP, Req
Expand Down
2 changes: 1 addition & 1 deletion lib/mix/tasks/generate_alert_summary_table.ex
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,7 @@ if Mix.env() == :test do
if is_nil(Repo.get(MobileAppBackend.User, user_id)) do
Repo.insert!(%MobileAppBackend.User{
id: user_id,
fcm_token: "not-a-real-token-#{user_id}",
fcm_installation_id: "not-a-real-installation-id-#{user_id}",
fcm_last_verified: ~U[2000-01-01 00:00:00Z]
})
end
Expand Down
12 changes: 11 additions & 1 deletion lib/mobile_app_backend/notifications/deliverer.ex
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,15 @@ defmodule MobileAppBackend.Notifications.Deliverer do
_ -> alert_id
end

{installation_id, token} =
case user do
%User{fcm_installation_id: installation_id} when not is_nil(installation_id) ->
{installation_id, nil}

%User{fcm_installation_id: nil, fcm_token: token} ->

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

question(non-blocking): Do we ~always have to support this path since users may not upgrade their app? Or is there a point where the old token just won't work anymore, and users might stop getting notifications without knowing why?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

One day, Google will probably shut off the old notification delivery path, and our users on old versions of the app will stop getting notifications. That will probably not happen anytime soon (even Google probably won’t do it in less than a year), and we should have plenty of notice ahead of time, so we could probably send out a targeted “please upgrade your app, you are on an old version and notifications will break soon” notification if we needed to.

{nil, token}
end

request_body = %{
message: %FCM.Message{
notification: %FCM.Notification{
Expand All @@ -63,7 +72,8 @@ defmodule MobileAppBackend.Notifications.Deliverer do
fcm_options: %FCM.FcmOptions{
analytics_label: analytics_label
},
token: user.fcm_token
fid: installation_id,
token: token
}
}

Expand Down
9 changes: 5 additions & 4 deletions lib/mobile_app_backend/notifications/write_payload.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ defmodule MobileAppBackend.Notifications.WritePayload do
alias Ecto.Changeset
alias MobileAppBackend.Notifications
alias MobileAppBackend.User
alias Util.FCMTarget

defmodule Window do
@type t :: %__MODULE__{
Expand Down Expand Up @@ -100,21 +101,21 @@ defmodule MobileAppBackend.Notifications.WritePayload do
end

@type t :: %__MODULE__{
fcm_token: String.t(),
fcm_target: FCMTarget.t(),
subscriptions: MapSet.t(Subscription.t()),
locale: Gettext.locale() | nil
}
defstruct [:fcm_token, :subscriptions, :locale]
defstruct [:fcm_target, :subscriptions, :locale]

def parse(payload) do
{:ok, parse!(payload)}
rescue
_ -> :error
end

def parse!(%{"fcm_token" => fcm_token, "subscriptions" => subscriptions} = payload) do
def parse!(%{"subscriptions" => subscriptions} = payload) do
%__MODULE__{
fcm_token: fcm_token,
fcm_target: FCMTarget.parse!(payload),
subscriptions: MapSet.new(subscriptions, &Subscription.parse!/1),
locale: payload["locale"]
}
Expand Down
3 changes: 2 additions & 1 deletion lib/mobile_app_backend/user.ex
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ defmodule MobileAppBackend.User do
"""

typed_schema "users" do
field(:fcm_token, :string, null: false, redact: true)
field(:fcm_token, :string, null: true, redact: true)
field(:fcm_installation_id, :string, null: true, redact: true)
field(:fcm_last_verified, :utc_datetime, null: false)
# should be a BCP 47 language tag like iOS’s
field(:locale, :string, null: true) :: Gettext.locale() | nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,31 @@ defmodule MobileAppBackendWeb.NotificationSubscriptionsController do
alias MobileAppBackend.Notifications.WritePayload
alias MobileAppBackend.Repo
alias MobileAppBackend.User
alias Util.FCMTarget

def set_include_accessibility(conn, params) do
status =
with {:ok, fcm_token} <- Map.fetch(params, "fcm_token"),
with {:ok, fcm_target} <- FCMTarget.parse(params),
{:ok, include_accessibility} <- Map.fetch(params, "include_accessibility") do
locale = params["locale"]

now = Map.get_lazy(conn.private, :mobile_app_backend_now, &DateTime.utc_now/0)

user_where = FCMTarget.user_where(fcm_target)

Repo.update_all(
from(u in User,
where: u.fcm_token == ^fcm_token,
where: ^user_where,
update: [set: [fcm_last_verified: ^now, locale: coalesce(^locale, u.locale)]]
),
[]
)

Repo.update_all(
from(ns in Subscription, join: u in assoc(ns, :user), where: u.fcm_token == ^fcm_token),
from(ns in Subscription,
join: u in subquery(from u in User, where: ^user_where),
on: ns.user_id == u.id
),
set: [include_accessibility: include_accessibility]
)

Expand Down Expand Up @@ -67,17 +73,17 @@ defmodule MobileAppBackendWeb.NotificationSubscriptionsController do

@spec perform_write(WritePayload.t(), DateTime.t()) :: {:ok, :ok} | {:error, term()}
defp perform_write(payload, now) do
fcm_token = payload.fcm_token
user_where = FCMTarget.user_where(payload.fcm_target)

Repo.transact(fn ->
user =
Repo.one(
from u in User,
where: u.fcm_token == ^fcm_token,
where: ^user_where,
preload: [notification_subscriptions: :windows]
)
|> case do
nil -> %User{fcm_token: fcm_token}
nil -> FCMTarget.new_user(payload.fcm_target)
user -> user
end

Expand Down
37 changes: 37 additions & 0 deletions lib/util/fcm_target.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
defmodule Util.FCMTarget do

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

👍 I like this approach

alias MobileAppBackend.User

@type t :: {:installation_id, String.t()} | {:token, String.t()}

@spec parse!(%{String.t() => String.t()}) :: t()
def parse!(payload) do
{:ok, target} = parse(payload)
target
end

@spec parse(%{String.t() => String.t()}) :: {:ok, t()} | :error
def parse(payload) do
case payload do
%{"fcm_installation_id" => installation_id} when not is_nil(installation_id) ->
{:ok, {:installation_id, installation_id}}

%{"fcm_token" => token} when not is_nil(token) ->
{:ok, {:token, token}}

_ ->
:error
end
end

def user_where(target)
def user_where({:installation_id, installation_id}), do: [fcm_installation_id: installation_id]
def user_where({:token, token}), do: [fcm_token: token]

def new_user(target)

def new_user({:installation_id, installation_id}) do
%User{fcm_installation_id: installation_id}
end

def new_user({:token, token}), do: %User{fcm_token: token}
end
5 changes: 3 additions & 2 deletions lib/util/gcp/fcm.ex
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,11 @@ defmodule Util.GCP.FCM do
android: AndroidConfig.t() | nil,
apns: ApnsConfig.t() | nil,
fcm_options: FcmOptions.t() | nil,
token: String.t() | nil
token: String.t() | nil,
fid: String.t() | nil
}
@derive Jason.Encoder
defstruct [:data, :notification, :android, :apns, :fcm_options, :token]
defstruct [:data, :notification, :android, :apns, :fcm_options, :token, :fid]
end

@doc "https://firebase.google.com/docs/reference/fcm/rest/v1/projects.messages/send"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
defmodule MobileAppBackend.Repo.Migrations.AddUsersFcmInstallationId do
use Ecto.Migration

def change do
alter table(:users) do
modify :fcm_token, :string, null: true, from: {:string, null: false}
add :fcm_installation_id, :string, null: true
end

create unique_index(:users, [:fcm_installation_id])

create constraint(:users, :some_fcm_target,
check: "COALESCE(fcm_installation_id, fcm_token) IS NOT NULL"
)
end
end
85 changes: 84 additions & 1 deletion test/mobile_app_backend/notifications/deliverer_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,92 @@ defmodule MobileAppBackend.Notifications.DelivererTest do
setup :verify_on_exit!
setup {Req.Test, :verify_on_exit!}

test "delivers notification via FCM and marks notification as delivered" do
test "delivers notification via FCM with new installation ID and marks notification as delivered" do
start_link_supervised!(Alerts)
user = NotificationsFactory.insert(:user)
user_id = user.id
fcm_installation_id = user.fcm_installation_id
alert = Factory.build(:alert)
Alerts.process_reset([alert], [])
alert_id = alert.id
upstream_timestamp = DateTime.utc_now(:second)
type = :notification

title = "Notification title"
body = "Notification body"
deep_link_path = "/a/#{alert_id}/r/1/s/1"
analytics_label = "route=1;effect=delay;type=notification"

reassign_persistent_term(GCPToken.default_key(), %GCPToken.StoredToken{
token: "gcp_token",
expires: ~U[9999-12-31 23:59:59Z]
})

Req.Test.expect(Util.GCP, fn conn ->
assert conn.method == "POST"

assert Plug.Conn.request_url(conn) ==
"https://fcm.googleapis.com/v1/projects/mbta-app-c574d/messages:send"

assert [
{"accept", "application/json"},
{"authorization", "Bearer gcp_token"},
{"content-type", "application/json"},
{"user-agent", "req/" <> _}
] = Enum.sort(conn.req_headers)

assert conn.body_params == %{
"message" => %{
"token" => nil,
"fid" => fcm_installation_id,
"notification" => %{"title" => title, "body" => body},
"data" => %{
"deep_link_path" => deep_link_path,
"analytics_label" => analytics_label
},
"android" => %{
"notification" => %{
"tag" => alert_id,
"sound" => "default",
"visibility" => "public"
}
},
"apns" => %{"payload" => %{"aps" => %{"sound" => "default"}}},
"fcm_options" => %{"analytics_label" => analytics_label}
}
}

Req.Test.json(conn, %{})
end)

:ok =
perform_job(Notifications.Deliverer, %{
user_id: user_id,
alert_id: alert_id,
title: title,
body: body,
deep_link_path: deep_link_path,
upstream_timestamp: upstream_timestamp,
type: type,
analytics_label: analytics_label
})

assert [
%DeliveredNotification{
user_id: ^user_id,
alert_id: ^alert_id,
upstream_timestamp: ^upstream_timestamp,
type: ^type
}
] = Repo.all(DeliveredNotification)

assert DateTime.before?(user.fcm_last_verified, Repo.reload!(user).fcm_last_verified)
end

test "delivers notification via FCM with old token and marks notification as delivered" do
start_link_supervised!(Alerts)
user = NotificationsFactory.insert(:user_with_old_token)
user_id = user.id
fcm_token = user.fcm_token
alert = Factory.build(:alert)
Alerts.process_reset([alert], [])
Expand Down Expand Up @@ -53,6 +135,7 @@ defmodule MobileAppBackend.Notifications.DelivererTest do
assert conn.body_params == %{
"message" => %{
"token" => fcm_token,
"fid" => nil,
"notification" => %{"title" => title, "body" => body},
"data" => %{
"deep_link_path" => deep_link_path,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ defmodule MobileAppBackend.Notifications.SubscriptionTest do
test "can insert subscription for user" do
%{id: user_id} =
MobileAppBackend.Repo.insert!(%User{
fcm_token: "fake",
fcm_installation_id: "fake",
fcm_last_verified: ~U[2025-09-10 00:00:00Z]
})

Expand Down
2 changes: 1 addition & 1 deletion test/mobile_app_backend/notifications/window_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ defmodule MobileAppBackend.Notifications.WindowTest do
test "can insert windows for subscription" do
%{id: user_id} =
MobileAppBackend.Repo.insert!(%User{
fcm_token: "fake",
fcm_installation_id: "fake",
fcm_last_verified: ~U[2025-09-10 00:00:00Z]
})

Expand Down
17 changes: 16 additions & 1 deletion test/mobile_app_backend/user_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,25 @@ defmodule MobileAppBackend.UserTest do
use MobileAppBackend.DataCase
alias MobileAppBackend.User

test "can insert record for user" do
test "can insert record for user with new installation id" do
MobileAppBackend.Repo.insert!(%User{
fcm_installation_id: "fake_installation_id",
fcm_last_verified: ~U[2025-09-10 00:00:00Z]
})
end

test "can insert record for user with old token" do
MobileAppBackend.Repo.insert!(%User{
fcm_token: "fake_token",
fcm_last_verified: ~U[2025-09-10 00:00:00Z]
})
end

test "cannot insert record for user with neither token nor installation id" do
assert_raise Ecto.ConstraintError, fn ->
MobileAppBackend.Repo.insert!(%User{
fcm_last_verified: ~U[2025-09-10 00:00:00Z]
})
end
end
end
Loading