Page MenuHomePhabricator

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

Authored by ginsu on Oct 31 2023, 10:12 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Oct 28, 12:14 AM
Unknown Object (File)
Oct 12 2024, 2:53 PM
Unknown Object (File)
Oct 9 2024, 3:34 AM
Unknown Object (File)
Sep 29 2024, 10:39 AM
Unknown Object (File)
Sep 28 2024, 3:22 AM
Unknown Object (File)
Sep 28 2024, 3:22 AM
Unknown Object (File)
Sep 28 2024, 3:22 AM
Unknown Object (File)
Sep 28 2024, 3:22 AM
Subscribers

Details

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
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

lib/shared/keyserver-utils.js
18 ↗(On Diff #32584)

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

lib/shared/keyserver-utils.js
18 ↗(On Diff #32584)

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?

lib/shared/keyserver-utils.js
18 ↗(On Diff #32584)

Accepted! After landing that diff, can you rebase this diff on it and confirm that this change isn't necessary anymore?

This revision is now accepted and ready to land.Nov 7 2023, 4:38 AM
This revision was landed with ongoing or failed builds.Dec 4 2023, 10:31 PM
This revision was automatically updated to reflect the committed changes.