Details

Reviewers

inka
rohan
michal
ashoat

Commits

rCOMM232d032138c0: [lib/native] introduce useIsKeyserverURLValid hook

Summary

To validate that the keyserver url the user inputted into the add keyserver input exists we should call the getVersion endpoint. The logic for this actually already exists in the KeyserverSelection component in the new registation flow. This diff factors out the shared logic into a seperate hook called useIsKeyserverURLValid.

Linear task: https://linear.app/comm/issue/ENG-5337/factor-out-shared-validation-logic-in-the-onsubmit-callback-in

Depends on D9665

Test Plan

Followed the test plan in D8198 and made sure that there were no regressions

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

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

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

update

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

ginsu added reviewers: inka, rohan, michal.

ginsu added a parent revision: D9665: [native] implement onPressSave keyserver button callback.

ginsu added a child revision: D9667: [lib/native] add keyserver url validation logic and error handling to the add keyserver screen.Oct 31 2023, 10:14 PM

Harbormaster completed remote builds in B23637: Diff 32583.Oct 31 2023, 10:31 PM

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

Harbormaster failed remote builds in B23636: Diff 32582!Oct 31 2023, 10:36 PM

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

native/account/registration/keyserver-selection.react.js
94 ↗	(On Diff #32583)	Why was this change made?
126–128 ↗	(On Diff #32583)	This condition doesn't appear to have been "migrated" to the new function. The button should be disabled if `keyserverURL` isn't set, but it's still good to be safe in case there is a race condition where the input is cleared and then the button is pressed before the button state updates

ginsu added inline comments.Nov 2 2023, 12:57 AM

native/account/registration/keyserver-selection.react.js
94 ↗	(On Diff #32583)	I initially had the `keyserverURL` param in `useIsKeyserverURLValid` be required; however, based on other inline comment, I will make it optional and bring the `!keyserverURL` condition into `useIsKeyserverURLValid`