Page MenuHomePhabricator

Synchronize platform details with identity on app start
ClosedPublic

Authored by marcin on Tue, Jun 4, 7:09 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Jun 30, 6:47 PM
Unknown Object (File)
Tue, Jun 25, 5:56 PM
Unknown Object (File)
Sun, Jun 23, 12:40 AM
Unknown Object (File)
Sat, Jun 22, 2:33 PM
Unknown Object (File)
Thu, Jun 20, 8:47 AM
Unknown Object (File)
Thu, Jun 20, 7:56 AM
Unknown Object (File)
Thu, Jun 20, 3:34 AM
Unknown Object (File)
Wed, Jun 19, 12:13 PM
Subscribers

Details

Summary

This differential ensures automatic platofrm details updates on both clients.

Test Plan

Repeat test plan for parent diff but but this time don't call anything from JS. It should be done automatically.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

marcin requested review of this revision.Tue, Jun 4, 7:26 AM

Check against usingCommServicesAccessToken before trying to sync platform details

bartek requested changes to this revision.EditedTue, Jun 4, 8:07 AM

I think we should make sure that this is being run only once, e.g. by introducing a hasRun ref, like in native/components/version-supported.react.js

By the way, do you think it makes sense to merge this with VersionSupportedChecker component? cc @varun

lib/components/platform-details-synchronizer.react.js
10 ↗(On Diff #40961)
This revision now requires changes to proceed.Tue, Jun 4, 8:07 AM

By the way, do you think it makes sense to merge this with VersionSupportedChecker component?

I was thinking about this but I couldn't find any compelling arguments against neither of those solutions. What made me finally decide to use separate component was that we have two different components to check version support on native on web. However this logic is exactly the same for both platforms so putting it inside version support checkers would result in code duplication.

EDIT:
In order to both avoid duplication and additional component we could extract this logic into a hook and use the hook in version support checkers. cc @varun , @ashoat what do you think?

lib/components/platform-details-synchronizer.react.js
10 ↗(On Diff #40961)

MinVersionHandler on web (VersionSupportChecker equivalent) is also typed like this. I will patch the fix into this diff.

  1. Fix typing
  2. Use hasRun ref.
  3. Fix typing of MinVersionHandler
This revision is now accepted and ready to land.Thu, Jun 6, 12:40 AM