Paths

Table of Contentst

Differential D11291

[native][web] Expose Secondary device registration RPC to JS
ClosedPublic
Actions

Authored by bartek on Mar 11 2024, 6:16 AM.

Tags

None

Referenced Files

	Unknown Object (File)
	Thu, Apr 25, 7:23 AM

	Unknown Object (File)
	Apr 3 2024, 12:44 PM

	Unknown Object (File)
	Mar 31 2024, 8:37 PM

	Unknown Object (File)
	Mar 30 2024, 1:31 AM

	Unknown Object (File)
	Mar 23 2024, 9:33 PM

	Unknown Object (File)
	Mar 23 2024, 1:37 AM

	Unknown Object (File)
	Mar 22 2024, 6:13 PM

	Unknown Object (File)
	Mar 21 2024, 10:54 PM

View All 20 Files

Subscribers

Details

Reviewers

kamil
michal

Commits

rCOMM8962994db2af: [native][web] Expose Secondary device registration RPC to JS

Summary

Exposed this RPC via Identity context

Depends on D11290

Test Plan

This is tested later in further diffs. Now just ensured it's callable on both native and web

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bartek created this revision.Mar 11 2024, 6:16 AM

bartek held this revision as a draft.

Herald added subscribers: tomek, ashoat. · View Herald TranscriptMar 11 2024, 6:16 AM

bartek added a child revision: D11292: [lib] Expose isAuthorized property in Tunnelbroker context.Mar 11 2024, 6:17 AM

Harbormaster completed remote builds in B27421: Diff 37969.Mar 11 2024, 6:31 AM

Rebase

bartek edited the summary of this revision. (Show Details)Mar 11 2024, 7:03 AM

bartek edited the test plan for this revision. (Show Details)

Harbormaster completed remote builds in B27434: Diff 37982.Mar 11 2024, 7:23 AM

bartek published this revision for review.Mar 11 2024, 7:27 AM

Looks good but when landing you could comment in ENG-7198 to use olmAPI here - or if the olmAPI is ready at the time of landing, use it here.

native/identity-service/identity-service-context-provider.react.js
483 ↗	(On Diff #37982)	could you at least add a comment explaining why `username` is an empty string here?
web/grpc/identity-service-client-wrapper.js
433 ↗	(On Diff #37982)

This revision is now accepted and ready to land.Mar 12 2024, 9:16 AM

ashoat added inline comments.Mar 12 2024, 3:12 PM

native/identity-service/identity-service-context-provider.react.js
481 ↗	(On Diff #37982)	Nit: why assign to `token` instead of just letting it be `accessToken`?
483 ↗	(On Diff #37982)	Is there a task to address the username being an empty string?
web/grpc/identity-service-client-wrapper.js
422 ↗	(On Diff #37982)	Not sure why this newline was removed

bartek added inline comments.Mar 14 2024, 6:11 AM

native/identity-service/identity-service-context-provider.react.js
481 ↗	(On Diff #37982)	eslint complains about name shadowing. It's also done for other methods in this file
483 ↗	(On Diff #37982)	It's https://linear.app/comm/issue/ENG-7215/retrieve-username-when-doing-qr-code-auth

Address comments

bartek added inline comments.Mar 14 2024, 6:14 AM

lib/types/identity-service-types.js
155 ↗	(On Diff #38062)	unintended, will fix before landing

Harbormaster completed remote builds in B27505: Diff 38062.Mar 14 2024, 6:26 AM

Closed by commit rCOMM8962994db2af: [native][web] Expose Secondary device registration RPC to JS (authored by bartek). · Explain WhyMar 19 2024, 4:18 AM

This revision was automatically updated to reflect the committed changes.

bartek added a commit: rCOMM8962994db2af: [native][web] Expose Secondary device registration RPC to JS.

Revision Contents
Changeset List

Path

Size

lib/

types/

identity-service-types.js

13 lines

native/

identity-service/

identity-service-context-provider.react.js

45 lines

services/

identity/

src/

database/

2 lines

web/

grpc/

identity-service-client-proxy.js

8 lines

identity-service-client-wrapper.js

34 lines

Diff 38166

lib/types/identity-service-types.js

Loading...

native/identity-service/identity-service-context-provider.react.js

Loading...

services/identity/src/database/device_list.rs

Loading...

web/grpc/identity-service-client-proxy.js

Loading...

web/grpc/identity-service-client-wrapper.js

Loading...