Page MenuHomePhabricator

[lib] Allow overriding a keyserver in a function returned from useKeyserverCall hook
AbandonedPublic

Authored by tomek on Dec 12 2023, 4:12 AM.
Tags
None
Referenced Files
F3527415: D10311.id36530.diff
Tue, Dec 24, 5:04 AM
F3527348: D10311.id34530.diff
Tue, Dec 24, 4:45 AM
F3527194: D10311.id34565.diff
Tue, Dec 24, 4:12 AM
F3525251: D10311.diff
Mon, Dec 23, 4:11 PM
Unknown Object (File)
Mon, Dec 23, 5:39 AM
Unknown Object (File)
Mon, Dec 23, 5:39 AM
Unknown Object (File)
Mon, Dec 23, 5:39 AM
Unknown Object (File)
Mon, Dec 23, 5:39 AM
Subscribers

Details

Summary

Modifying the keyserver by setting a parameter of a hook appeared to be really complicated when we don't know the keyserver when rendering a component. This diff allows overriding when calling a function.

Depends on D10308

Test Plan

Check if calling link verification works with default and overridden keyserver.

Diff Detail

Repository
rCOMM Comm
Branch
linki
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

tomek requested review of this revision.Dec 12 2023, 5:04 AM
tomek added inline comments.
lib/utils/keyserver-call.js
103–106

There's an issue with memoization here, when we provide a next argument - this requires changing the solution

Probably fix the override. This still requires a lot of testing and thinking about
memoization, but it isn't broken in an obvious way.

Makes sense to me but I might not have enough context to be sure

This diff still has an issue: when calling a new keyserver, it returns sessionChange which causes setNewSession to be dispatched - this results in the keyserver being added to a store in an incorrect way and breaks more things down the line.

This diff still has an issue: when calling a new keyserver, it returns sessionChange which causes setNewSession to be dispatched - this results in the keyserver being added to a store in an incorrect way and breaks more things down the line.

I'm seeing the same thing with the getVersion call in the new registration flow. Created ENG-6097 to track.

Rebased the diff but I'm not sure whether it works and is the right approach

tomek planned changes to this revision.Feb 1 2024, 7:57 AM

With the new approach we no longer need this