Paths

Table of Contentst

Differential D11290

[native][web] Expose Device List RPCs to JS
ClosedPublic
Actions

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

Tags

None

Referenced Files

	F3386856: D11290.id37981.diff
	Fri, Nov 29, 6:31 AM

	Unknown Object (File)
	Sun, Nov 10, 2:03 PM

	Unknown Object (File)
	Oct 27 2024, 8:20 AM

	Unknown Object (File)
	Oct 23 2024, 12:18 PM

	Unknown Object (File)
	Oct 23 2024, 10:32 AM

	Unknown Object (File)
	Oct 23 2024, 10:20 AM

	Unknown Object (File)
	Oct 23 2024, 2:20 AM

	Unknown Object (File)
	Oct 23 2024, 2:15 AM

View All 62 Files

Subscribers

Details

Reviewers

kamil
michal

Commits

rCOMM8105e6af9a3e: [native][web] Expose Device List RPCs to JS

Summary

These RPCs were exposed to JS but not added to the Identity context.

Test Plan

Called them manually on both native and web and verified they're callable as expected.

Diff Detail

Repository

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

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

bartek held this revision as a draft.

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

bartek added a child revision: D11291: [native][web] Expose Secondary device registration RPC to JS.Mar 11 2024, 6:16 AM

Harbormaster failed remote builds in B27420: Diff 37968!Mar 11 2024, 6:29 AM

Rebase

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

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

Harbormaster completed remote builds in B27433: Diff 37981.Mar 11 2024, 7:19 AM

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

bartek added inline comments.

lib/types/identity-service-types.js
135 ↗	(On Diff #37981)	This RPC should be available only on native (primary device)

ashoat added inline comments.Mar 11 2024, 7:00 PM

lib/types/identity-service-types.js
135 ↗	(On Diff #37981)	Can you add a comment explaining why it's optional, like there is on `publishWebPrekeys`?
web/grpc/identity-service-client-wrapper.js
476 ↗	(On Diff #37981)	Can we undo this change?

Looks good, just requesting changes to address my and @ashoat's comments

lib/types/identity-service-types.js
174–177 ↗	(On Diff #37981)	I know it's trivial but can we add a validator here and when returning result return this using `assertWithValidator`? This will maintain convention we have for `IdentityServiceClient` to validate each Identity response

This revision now requires changes to proceed.Mar 12 2024, 8:53 AM

@michal is moving the Identity client to worker now so should review this code as well

Added validator, added comment and fixed accidental newlines

michal accepted this revision.Mar 14 2024, 6:06 AM

Harbormaster completed remote builds in B27503: Diff 38060.Mar 14 2024, 6:19 AM

kamil accepted this revision.Mar 14 2024, 6:37 AM

This revision is now accepted and ready to land.Mar 14 2024, 6:37 AM

Closed by commit rCOMM8105e6af9a3e: [native][web] Expose Device List RPCs 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: rCOMM8105e6af9a3e: [native][web] Expose Device List RPCs to JS.

Revision Contents
Changeset List

Path

Size

lib/

types/

identity-service-types.js

17 lines

native/

identity-service/

identity-service-context-provider.react.js

37 lines

web/

grpc/

identity-service-client-wrapper.js

27 lines

Diff 37968

lib/types/identity-service-types.js

Loading...

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

Loading...

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

Loading...