[lib/native] add keyserver url validation logic and error handling to the add keyserver screen
ClosedPublic
Actions

Authored by ginsu on Oct 31 2023, 10:12 PM.

Details

Reviewers

inka
rohan
michal

Commits

rCOMMe250e5ccff7b: [lib/native] add keyserver url validation logic and error handling to the add…

Summary

This diff adds keyserver url validation logic to the onPressSave callback in the add keyserver screen. We also now display an error message if for any reason the keyserver url inputed by the user is not valid

Linear task: https://linear.app/comm/issue/ENG-4917/update-onpress-of-goadd-button-in-customservermodal-to-add-keyserver

Depends on D9666

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, 10:12 PM

Herald added subscribers: will, tomek, ashoat. · View Herald TranscriptOct 31 2023, 10:12 PM

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

ginsu added a parent revision: D9666: [lib/native] introduce useIsKeyserverURLValid hook.

ginsu added reviewers: inka, rohan, michal.

ginsu edited the test plan for this revision. (Show Details)Oct 31 2023, 10:25 PM

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

ginsu added inline comments.Oct 31 2023, 10:32 PM

lib/shared/keyserver-utils.js
18	We need to add this to the `serverCallParamOverride` otherwise when we attempt to call the get version endpoint from the add keyserver screen we will enter this if condition in the `callServerEndpoint` function and the get version endpoint will not correctly validate if the new keyserver url is correct or not. https://github.com/CommE2E/comm/blob/master/lib/utils/call-server-endpoint.js#L105-L125 When we call the get version endpoint from the keyserver selection in the registration flow the `connectionStatus` is `connecting` as well

Harbormaster completed remote builds in B23638: Diff 32584.Oct 31 2023, 10:43 PM

ginsu requested review of this revision.Oct 31 2023, 10:43 PM

ashoat added inline comments.Nov 1 2023, 10:39 AM

lib/shared/keyserver-utils.js
18	This is a legitimate issue you've discovered, but you're papering over it here. When we're overriding `urlPrefix`, we almost certainly never want to use an established socket, since that socket won't respect the overriden `urlPrefix`. Instead of making this change, can you submit a separate diff that skips over the condition you linked when `urlPrefix` is being overriden?