Page MenuHomePhabricator

[lib][web][native] Refactor setDeviceToken action
ClosedPublic

Authored by inka on Oct 30 2023, 6:45 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Nov 25, 4:29 PM
Unknown Object (File)
Sat, Nov 23, 10:52 PM
Unknown Object (File)
Sat, Nov 23, 10:29 PM
Unknown Object (File)
Sat, Nov 23, 9:00 PM
Unknown Object (File)
Sat, Nov 23, 5:57 PM
Unknown Object (File)
Thu, Nov 21, 12:57 AM
Unknown Object (File)
Wed, Nov 20, 3:16 PM
Unknown Object (File)
Tue, Nov 19, 9:52 PM
Subscribers

Details

Summary

issue: https://linear.app/comm/issue/ENG-5284/refactor-setdevicetoken-action
I split the action into two actions to spare the calling code having to select all keyservers when it simply wants to call all of them

Test Plan

tested that both action work, tested that

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

web/chat/reaction-message-utils.js
63–72 ↗(On Diff #32493)

Same as below

Could not decide which case to select, since  case 112 [1] may work but if it doesn't  case 127 [1] looks promising too. To fix add a type annotation  to `result.interface` [2],  to `result.time` [3], or  to `result.server.id` [4].Flow(speculation-ambiguous)
action-utils.js(117, 20): [1] case 112
action-utils.js(117, 20): [1] case 127
reaction-message-utils.js(72, 24): [2] to `result.interface`
reaction-message-utils.js(71, 19): [3] to `result.time`
reaction-message-utils.js(69, 23): [4] to `result.server.id`
72 ↗(On Diff #32493)

Interface is a special keyword, cannot create a variable named this way

web/modals/history/history-modal.react.js
221–222 ↗(On Diff #32493)

Flow was showing some errors about not being able to tell the type...

Could not decide which case to select, since  case 112 [1] may work but if it doesn't  case 156 [1] looks promising too. To fix add a type annotation  to `result[0].deleted` [2] or  to `result[0].text` [3].Flow(speculation-ambiguous)
action-utils.js(117, 20): [1] case 112
action-utils.js(117, 20): [1] case 156
history-modal.react.js(226, 16): [2] to `result[0].deleted`
history-modal.react.js(225, 13): [3] to `result[0].text`
web/modals/history/history-modal.react.js
221–222 ↗(On Diff #32493)

I remember seeing an error like this about a year ago, but I don't remember what it was about. @tomek do you remember what that was about?
It appears when I change the palyoad type of SET_DEVICE_TOKEN_SUCCESS to DeviceTokens.

Harbormaster returned this revision to the author for changes because remote builds failed.Oct 30 2023, 6:57 AM
Harbormaster failed remote builds in B23563: Diff 32493!
Harbormaster returned this revision to the author for changes because remote builds failed.Oct 30 2023, 7:14 AM
Harbormaster failed remote builds in B23565: Diff 32495!
inka requested review of this revision.Oct 30 2023, 1:29 PM
inka retitled this revision from [lib][web][native] Refactor deviceToken action to [lib][web][native] Refactor setDeviceToken action.Oct 31 2023, 12:58 AM

I forgot to create hooks, I'll fix that

web/modals/history/history-modal.react.js
221–222 ↗(On Diff #32493)

This error means that Flow can't decide what is the type, and fixing it is different for each case. In this case, it seems to be caused by the fact that a payload of action is DeviceTokens which is an object mapping from any string to a string, so it is a supertype of some other payloads. I think we can solve the issue by modifying SET_DEVICE_TOKEN_SUCCESS action payload to be a new type, e.g. SetDeviceTokensPayload = { +deviceTokes: DeviceTokens }

web/modals/history/history-modal.react.js
221–222 ↗(On Diff #32493)

Thank you!!

Fix flow types, add hooks

inka planned changes to this revision.Nov 2 2023, 11:54 AM
inka added inline comments.
native/push/push-handler.react.js
443

Will fix

This revision is now accepted and ready to land.Nov 3 2023, 7:39 AM
lib/actions/device-actions.js
29 ↗(On Diff #32881)

There's a type error here. DeviceTokens points to a value that is nullable, but DeviceTokenUpdateRequest requires deviceToken to be specified

The validator for the type (deviceTokenUpdateRequestInputValidator) appears to allow for a nullable deviceToken, so maybe the type (DeviceTokenUpdateRequest) should be updated so that deviceToken is nullable there too?

lib/actions/device-actions.js
29 ↗(On Diff #32881)

Fixed in D9751