Page MenuHomePhabricator

[keyserver] Update `updateUserAvatarResponder` to return `CreateUpdatesResult` for new clients
ClosedPublic

Authored by atul on May 1 2023, 1:15 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 5, 2:11 AM
Unknown Object (File)
Fri, Oct 25, 7:22 AM
Unknown Object (File)
Oct 8 2024, 12:36 AM
Unknown Object (File)
Oct 3 2024, 2:57 AM
Unknown Object (File)
Oct 3 2024, 2:57 AM
Unknown Object (File)
Oct 3 2024, 2:57 AM
Unknown Object (File)
Oct 3 2024, 2:57 AM
Unknown Object (File)
Oct 3 2024, 2:57 AM
Subscribers

Details

Summary
Test Plan

Log CreateUpdatesResult on keyserver and ensure that it's as expected:

fb699a.png (526×2 px, 138 KB)

Will be tested implicitly by subsequent diff which includes necessary change on client (w/ codeVersion bumped to 215 for testing)

Diff Detail

Repository
rCOMM Comm
Branch
arcpatch-D7699 (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul published this revision for review.May 1 2023, 1:21 PM
ashoat added inline comments.
keyserver/src/updaters/account-updaters.js
195 ↗(On Diff #25974)

It's always better to return an object, so that it will be easier to change this in the future if we need to

This revision is now accepted and ready to land.May 1 2023, 1:59 PM
ashoat added inline comments.
keyserver/src/responders/user-responders.js
632–634 ↗(On Diff #25980)

We usually define these alongside the Result and Payload types (if any) – could you move this to lib/types/avatar-types.js?

This revision was landed with ongoing or failed builds.May 1 2023, 4:08 PM
This revision was automatically updated to reflect the committed changes.

Re-opening to address flow issues...

keyserver/src/responders/user-responders.js
632–634 ↗(On Diff #25980)

Yup, will do

This revision is now accepted and ready to land.May 1 2023, 7:03 PM

Okay so when I ran flow in keyserver, there were no issues. Then I ran it again AFTER yarn cleaninstall && killall flow and it gave me the three issues... so I guess got in some bad state? Hopefully a one-off issue

fix flow (will wait for buildkite CI to complete before landing)