[native] implement onPressSave keyserver button callback
ClosedPublic
Actions

Authored by ginsu on Oct 31 2023, 9:35 PM.

Details

Reviewers

inka
rohan
michal

Commits

rCOMM5ccad7e00e91: [native] implement onPressSave keyserver button callback

Summary

This diff implements onPressSave keyserver button. This callback will create the payload necessary for this action and dispatch the add keyserver redux action we just introduced with the payload we just created. For now we will just use the current user id as the keyserver admin user id until ENG-5282 is done and returns a userID

In a subsequent diff we will implement validation that will be used before we introduce the keyserver

Linear task: https://linear.app/comm/issue/ENG-5338/dispatch-addkeyserver-redux-action

Depends on D9664

Test Plan

Please see the demo video below

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ginsu created this revision.Oct 31 2023, 9:35 PM

Herald added subscribers: will, tomek, ashoat. · View Herald TranscriptOct 31 2023, 9:35 PM

ginsu edited the summary of this revision. (Show Details)Oct 31 2023, 9:38 PM

ginsu added reviewers: inka, rohan, michal.

ginsu mentioned this in D9590: [native] introduce Add keyserver screen UI.Oct 31 2023, 9:41 PM

ginsu edited the summary of this revision. (Show Details)Oct 31 2023, 9:49 PM

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

ginsu added a parent revision: D9664: [lib] add conditions for adding keyserver to the keyserver store.

ginsu attached a referenced file: F846550: Screen Recording 2023-11-01 at 12.47.43 AM.mov. (Show Details)

Harbormaster completed remote builds in B23633: Diff 32579.Oct 31 2023, 9:52 PM

ginsu requested review of this revision.Oct 31 2023, 9:52 PM

ginsu added a child revision: D9666: [lib/native] introduce useIsKeyserverURLValid hook.Oct 31 2023, 10:08 PM

inka added inline comments.Nov 7 2023, 2:06 AM

native/profile/add-keyserver.react.js
35–41 ↗	(On Diff #32579)	Wouldn't it make sense to fill in the default values in the reducer?

ashoat added inline comments.Nov 7 2023, 3:49 AM

native/profile/add-keyserver.react.js
35–41 ↗	(On Diff #32579)	Why?

inka added inline comments.Nov 7 2023, 4:28 AM

native/profile/add-keyserver.react.js
35–41 ↗	(On Diff #32579)	To reduce the amount of data in the payload. Just as a memory optimisation. Since I expect we will always be using the default values for those fields in `addKeyserver` action

ashoat added inline comments.Nov 7 2023, 5:18 AM

native/profile/add-keyserver.react.js
35–41 ↗	(On Diff #32579)	I don't think we need to be concerned about the memory usage of this object. It appears to be very small In general, we should focus on readability / simplicity unless there's a legitimate performance concern (eg. a performance impact that will affect the user) Overoptimizing for performance by reducing 5 params to 2 or something is not really productive in my opinion