Page MenuHomePhabricator
Differential D6781

Initialize notifications olm account in CommCoreModule and expose notifications public keys to JS
ClosedPublic
Actions

Authored by marcin on Feb 20 2023, 6:17 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 19, 1:23 PM
Unknown Object (File)
Mon, Nov 4, 8:59 AM
Unknown Object (File)
Mon, Nov 4, 8:58 AM
Unknown Object (File)
Mon, Nov 4, 8:58 AM
Unknown Object (File)
Mon, Nov 4, 8:58 AM
Unknown Object (File)
Mon, Nov 4, 8:57 AM
Unknown Object (File)
Mon, Nov 4, 8:56 AM
Unknown Object (File)
Mon, Nov 4, 8:55 AM
View All Files
Subscribers
ashoat

Details

Reviewers
tomek
kamil
atul
bartek
Commits
rCOMM4e7a5cc55948: Initialize notifications olm account in CommCoreModule and expose notifications…
Summary

This differential uses NotificationsCryptoModule API to idempotently create notifications olm account and to expose notifications public keys in JS via JSI.

Test Plan

Modify one place in JS native code where we call getUserPublicKeys to log notifications public keys to the console. Example of such place is log-in panel. Launch the app and examine console.

Diff Detail

Repository
rCOMM Comm
Branch
marcin/eng-2901
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

marcin created this revision.Feb 20 2023, 6:17 AM
Herald added a subscriber: ashoat. · View Herald TranscriptFeb 20 2023, 6:17 AM
marcin added a parent revision: D6780: Refactor CommCoreModule.initializeCryptoAccount not to take userID as an argument..Feb 20 2023, 6:17 AM
marcin retitled this revision from Initialize noptifications olm account in CommCoreModule and expose notifications public keys to JS to Initialize notifications olm account in CommCoreModule and expose notifications public keys to JS.
marcin added 2 blocking reviewer(s): kamil, atul.
marcin published this revision for review.Feb 20 2023, 6:37 AM
Harbormaster failed remote builds in B16618: Diff 22724!Feb 20 2023, 8:47 AM
marcin updated this revision to Diff 22873.Feb 21 2023, 10:37 AM

Rebase

marcin added a child revision: D6821: Implement method to sign message.Feb 21 2023, 10:41 AM
Harbormaster completed remote builds in B16718: Diff 22873.Feb 21 2023, 10:53 AM
bartek accepted this revision.Feb 23 2023, 4:44 AM
ashoat added inline comments.Feb 23 2023, 11:35 AM
native/schema/CommCoreModuleSchema.js
28–31

Should we update this to have a nested structure like in my Linear comment? I'm not sure it's strictly necessary if this is a client-only type, but it might be good to be consistent

marcin updated this revision to Diff 23055.Feb 24 2023, 5:06 AM

Create nested notifications keys structure like on web.

Harbormaster completed remote builds in B16873: Diff 23055.Feb 24 2023, 5:21 AM
kamil accepted this revision.Feb 24 2023, 5:30 AM
ashoat added inline comments.Feb 24 2023, 3:01 PM
native/schema/CommCoreModuleSchema.js
27 ↗(On Diff #23055)

I think @atul defined this type somewhere else as well, after we land both of your diffs we should dedup and move to lib. Can you create a task and link it here before landing?

marcin added inline comments.Feb 27 2023, 2:22 AM
native/schema/CommCoreModuleSchema.js
27 ↗(On Diff #23055)

The task: https://linear.app/comm/issue/ENG-3080/refactor-web-and-native-primary-and-notification-identity-keys-data

atul accepted this revision.Feb 27 2023, 7:24 AM
This revision is now accepted and ready to land.Feb 27 2023, 7:24 AM
marcin updated this revision to Diff 23259.Feb 28 2023, 10:08 AM

Rebase before landing

Harbormaster failed remote builds in B17005: Diff 23259!Feb 28 2023, 10:34 AM
Harbormaster completed remote builds in B17005: Diff 23259.Feb 28 2023, 1:41 PM
marcin updated this revision to Diff 23313.Mar 1 2023, 1:23 AM

Rebase before landing

Harbormaster completed remote builds in B17030: Diff 23313.Mar 1 2023, 1:51 AM
Closed by commit rCOMM4e7a5cc55948: Initialize notifications olm account in CommCoreModule and expose notifications… (authored by marcin). · Explain WhyMar 1 2023, 2:04 AM
This revision was automatically updated to reflect the committed changes.
marcin added a commit: rCOMM4e7a5cc55948: Initialize notifications olm account in CommCoreModule and expose notifications….
ashoat added inline comments.Mar 6 2023, 11:07 AM
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
837 ↗(On Diff #23325)

I believe this call is causing a JNI error, see ENG-2787

We should perhaps avoid calling this from the crypto thread?

ashoat added inline comments.Mar 6 2023, 11:27 AM
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
837 ↗(On Diff #23325)

Ignore, this was incorrect

Revision Contents
Changeset List

PathSize
native/
cpp/
CommonCpp/
DatabaseManagers/
2 lines
NativeModules/
46 lines
schema/
2 lines

Diff 22873

View Options

native/cpp/CommonCpp/DatabaseManagers/DatabaseManager.cpp

Loading...
View Options

native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp

Loading...
View Options

native/schema/CommCoreModuleSchema.js

Loading...