Page MenuHomePhabricator

Create large notifications in thick threads
ClosedPublic

Authored by marcin on Wed, Sep 25, 11:14 AM.
Tags
None
Referenced Files
F2838999: D13469.diff
Sun, Sep 29, 12:25 AM
F2836699: D13469.id44642.diff
Sat, Sep 28, 5:08 PM
F2834346: D13469.id44645.diff
Sat, Sep 28, 10:59 AM
Unknown Object (File)
Fri, Sep 27, 11:07 PM
Unknown Object (File)
Fri, Sep 27, 7:59 AM
Unknown Object (File)
Thu, Sep 26, 7:05 PM
Unknown Object (File)
Thu, Sep 26, 5:19 PM
Unknown Object (File)
Wed, Sep 25, 12:22 PM
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.Fri, Sep 27, 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.Fri, Sep 27, 7:48 AM
This revision was automatically updated to reflect the committed changes.