Paths

Table of Contentst

Differential D7699

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

Authored by atul on May 1 2023, 1:15 PM.

Tags

None

Referenced Files

	Unknown Object (File)
	Sun, Sep 15, 8:56 AM

	Unknown Object (File)
	Sat, Sep 14, 12:26 PM

	Unknown Object (File)
	Fri, Sep 13, 6:46 PM

	Unknown Object (File)
	Sat, Aug 31, 3:26 AM

	Unknown Object (File)
	Fri, Aug 30, 1:45 PM

	Unknown Object (File)
	Mon, Aug 26, 1:55 PM

	Unknown Object (File)
	Sun, Aug 25, 9:37 PM

	Unknown Object (File)
	Sun, Aug 25, 6:20 PM

Subscribers

Details

Reviewers

ashoat
ginsu

Commits

rCOMMd9fa30eeedd5: [keyserver] Update `updateUserAvatarResponder` to return `CreateUpdatesResult`…
rCOMMfa6d8384b74f: [keyserver] Update `updateUserAvatarResponder` to return `CreateUpdatesResult`…

Summary

For new clients (>215), we return CreateUpdatesResult instead of ClientAvatar.

Context can be found:

ENG-3808 : updateUserAvatar should create UPDATE_USER and UPDATE_CURRENT_USER updates

https://linear.app/comm/issue/ENG-3809/add-code-to-reduceuserinfos-to-handle-user-setting-their-own-avatar

Test Plan

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

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

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

atul created this revision.May 1 2023, 1:15 PM

Herald added a subscriber: tomek. · View Herald TranscriptMay 1 2023, 1:15 PM

atul edited the test plan for this revision. (Show Details)May 1 2023, 1:15 PM

atul added a child revision: D7700: [lib] Update `reduceCurrentUserInfo` to handle `CurrentUserUpdate` payload instead of `ClientAvatar`.

atul published this revision for review.May 1 2023, 1:21 PM

Harbormaster completed remote builds in B18962: Diff 25974.May 1 2023, 1:34 PM

ashoat accepted this revision.May 1 2023, 1:59 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

address feedback

Harbormaster completed remote builds in B18965: Diff 25980.May 1 2023, 2:29 PM

ashoat accepted this revision.May 1 2023, 3:21 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`?

address feedback

This revision was landed with ongoing or failed builds.May 1 2023, 4:08 PM

Closed by commit rCOMMfa6d8384b74f: [keyserver] Update `updateUserAvatarResponder` to return `CreateUpdatesResult`… (authored by atul). · Explain Why

This revision was automatically updated to reflect the committed changes.

atul added a commit: rCOMMfa6d8384b74f: [keyserver] Update `updateUserAvatarResponder` to return `CreateUpdatesResult`….

Harbormaster failed remote builds in B18972: Diff 25987!May 1 2023, 4:19 PM

atul added a reverting change: rCOMMcf1cf9bd3776: Revert "[keyserver] Update `updateUserAvatarResponder` to return….May 1 2023, 7:01 PM

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

re-opening....

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)

Harbormaster failed remote builds in B18974: Diff 25990!May 1 2023, 7:19 PM

Harbormaster completed remote builds in B18975: Diff 25991.May 1 2023, 7:24 PM

Closed by commit rCOMMd9fa30eeedd5: [keyserver] Update `updateUserAvatarResponder` to return `CreateUpdatesResult`… (authored by atul). · Explain WhyMay 1 2023, 7:42 PM

This revision was automatically updated to reflect the committed changes.

atul added a commit: rCOMMd9fa30eeedd5: [keyserver] Update `updateUserAvatarResponder` to return `CreateUpdatesResult`….

Revision Contents
Changeset List

Path

Size

keyserver/

src/

responders/

user-responders.js

3 lines

updaters/

account-updaters.js

27 lines

lib/

types/

avatar-types.js

6 lines

Diff 25992

keyserver/src/responders/user-responders.js

Loading...

keyserver/src/updaters/account-updaters.js

Loading...

lib/types/avatar-types.js

Loading...