search

Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.33k stars 2.76k forks source link

Android/iOS: Push notifications from large rooms are not styled #47820

Closed arosiclair closed 3 days ago

arosiclair commented 3 weeks ago

Version Number: Latest Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Email or phone of affected tester (no customers): Everyone Logs: Log search for al's message Expensify/Expensify Issue URL: N/A Issue reported by: @arosiclair Slack conversation: https://expensify.slack.com/archives/C05LX9D6E07/p1724268766246389

Action Performed:

  1. Join a room with 30+ people
  2. Receive an iOS/Android push notification for a short message from the room

Expected Result:

The message should be styled with the sender's name and the avatar

Actual Result:

The message is not styled

Workaround:

None

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

IMG_4BF62B86C078-1

View all open jobs on GitHub

melvin-bot[bot] commented 3 weeks ago

Triggered auto assignment to @slafortune (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

arosiclair commented 3 weeks ago

Slack discussion

We started including all report participant's notification preferences in the onyx updates for new actions in this PR. participants turns out to be about ~2 KB just for 30 users so that causes the onyxData to be removed for most push notifications since the size limit is only ~3.5 KB.

The fix is to queue the report.participants separately so they're not included in the push notification payload. We'll queue them first, so the notification preference is set in Onyx before the new action is merged.

melvin-bot[bot] commented 2 weeks ago

@arosiclair, @slafortune Huh... This is 4 days overdue. Who can take care of this?

arosiclair commented 2 weeks ago

I posted these PRs for review:

melvin-bot[bot] commented 1 week ago

@arosiclair, @slafortune Whoops! This issue is 2 days overdue. Let's get this updated quick!

arosiclair commented 1 week ago

Not overdue this is on its way to prod

melvin-bot[bot] commented 3 days ago

@arosiclair, @slafortune Whoops! This issue is 2 days overdue. Let's get this updated quick!

arosiclair commented 3 days ago

https://github.com/Expensify/Auth/pull/12180 and https://github.com/Expensify/Web-Expensify/pull/43259 are deployed so this is all set!