Page MenuHomePhabricator

Create large notifications in thick threads
ClosedPublic

Authored by marcin on Sep 25 2024, 11:14 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 23, 1:16 PM
Unknown Object (File)
Fri, Nov 22, 10:53 AM
Unknown Object (File)
Fri, Nov 22, 10:48 AM
Unknown Object (File)
Fri, Nov 22, 10:37 AM
Unknown Object (File)
Fri, Nov 22, 5:44 AM
Unknown Object (File)
Wed, Nov 20, 9:59 AM
Unknown Object (File)
Sat, Nov 16, 2:44 AM
Unknown Object (File)
Fri, Nov 8, 3:02 AM
Subscribers

Details

Summary

This differential creates large notifications in thick threads

Test Plan

Tested in final diff

Diff Detail

Repository
rCOMM Comm
Branch
marcin/eng-8709
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

It's quite a large diff adding support for large notifs - I think it would be a lot easier to review it if it was split into multiple smaller ones. We don't have the time to split it, but overall the diff makes sense to me.

lib/push/send-utils.js
505

Can this be readonly?

793
816

Why is it inexact?

This revision is now accepted and ready to land.Sep 27 2024, 4:44 AM
lib/push/send-utils.js
505

I must have forgotten to update this diff. This code is no longer there

793

It is not going to work with eslint since largeNotifDataArray is already declared in the upper scope

816

buildNotifsForPlatform expects a function that returns

{
      +targetedNotificationsWithPlatform: $ReadOnlyArray<TargetedNotificationWithPlatform>,
      +largeNotifDataArray?: ...
}

but function that create badge updates and rescinds return only targetedNotificationsWithPlatform. Making the type here inexact solves flow complaint.

This revision was landed with ongoing or failed builds.Sep 27 2024, 7:48 AM
This revision was automatically updated to reflect the committed changes.