Page MenuHomePhabricator
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
F6236157: D7699.diff
Thu, Apr 24, 3:07 PM
Unknown Object (File)
Thu, Apr 17, 12:31 AM
Unknown Object (File)
Tue, Apr 8, 5:18 PM
Unknown Object (File)
Mar 11 2025, 6:30 PM
Unknown Object (File)
Mar 2 2025, 12:28 AM
Unknown Object (File)
Mar 2 2025, 12:28 AM
Unknown Object (File)
Mar 2 2025, 12:28 AM
Unknown Object (File)
Mar 2 2025, 12:28 AM
View All Files
Subscribers
tomek

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:

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
Lint
No Lint Coverage
Unit
No Test Coverage

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

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
atul updated this revision to Diff 25980.May 1 2023, 2:09 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?

atul updated this revision to Diff 25987.May 1 2023, 4:04 PM

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
atul reopened this revision.May 1 2023, 7:03 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
atul updated this revision to Diff 25990.May 1 2023, 7:03 PM

re-opening....

atul added a comment.EditedMay 1 2023, 7:05 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

atul updated this revision to Diff 25991.May 1 2023, 7:06 PM

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

PathSize
keyserver/
src/
responders/
3 lines
updaters/
23 lines

Diff 25974

View Options

keyserver/src/responders/user-responders.js

// @flow // @flow
import _isEqual from 'lodash/fp/isEqual.js'; import _isEqual from 'lodash/fp/isEqual.js';
import { createSelector } from 'reselect'; import { createSelector } from 'reselect';
import type { StateSyncSpec, BoundStateSyncSpec } from './state-sync-spec.js'; import type { StateSyncSpec, BoundStateSyncSpec } from './state-sync-spec.js';
import type { AppState } from '../../types/redux-types.js'; import type { AppState } from '../../types/redux-types.js';
import { import {
reportTypes, reportTypes,
type ClientThreadInconsistencyReportCreationRequest, type ClientThreadInconsistencyReportCreationRequest,
} from '../../types/report-types.js'; } from '../../types/report-types.js';
import type { ProcessServerRequestAction } from '../../types/request-types.js'; import type { ProcessServerRequestAction } from '../../types/request-types.js';
import { import {
type LegacyRawThreadInfos, type RawThreadInfo,
type LegacyRawThreadInfo, type RawThreadInfos,
} from '../../types/thread-types.js'; } from '../../types/thread-types.js';
import { actionLogger } from '../../utils/action-logger.js'; import { actionLogger } from '../../utils/action-logger.js';
import { getConfig } from '../../utils/config.js'; import { getConfig } from '../../utils/config.js';
import { combineUnorderedHashes, values } from '../../utils/objects.js'; import { combineUnorderedHashes, values } from '../../utils/objects.js';
import { generateReportID } from '../../utils/report-utils.js'; import { generateReportID } from '../../utils/report-utils.js';
import { sanitizeActionSecrets } from '../../utils/sanitization.js'; import { sanitizeActionSecrets } from '../../utils/sanitization.js';
import { ashoatKeyserverID } from '../../utils/validation-utils.js'; import { ashoatKeyserverID } from '../../utils/validation-utils.js';
const selector: ( const selector: (
state: AppState, state: AppState,
) => BoundStateSyncSpec< ) => BoundStateSyncSpec<
LegacyRawThreadInfos, RawThreadInfos,
LegacyRawThreadInfo, RawThreadInfo,
$ReadOnlyArray<ClientThreadInconsistencyReportCreationRequest>, $ReadOnlyArray<ClientThreadInconsistencyReportCreationRequest>,
> = createSelector( > = createSelector(
(state: AppState) => state.integrityStore.threadHashes, (state: AppState) => state.integrityStore.threadHashes,
(state: AppState) => state.integrityStore.threadHashingStatus === 'completed', (state: AppState) => state.integrityStore.threadHashingStatus === 'completed',
(threadHashes: { +[string]: number }, threadHashingComplete: boolean) => ({ (threadHashes: { +[string]: number }, threadHashingComplete: boolean) => ({
...threadsStateSyncSpec, ...threadsStateSyncSpec,
getInfoHash: (id: string) => threadHashes[`${ashoatKeyserverID}|${id}`], getInfoHash: (id: string) => threadHashes[`${ashoatKeyserverID}|${id}`],
getAllInfosHash: threadHashingComplete getAllInfosHash: threadHashingComplete
? () => combineUnorderedHashes(values(threadHashes)) ? () => combineUnorderedHashes(values(threadHashes))
: () => null, : () => null,
getIDs: threadHashingComplete getIDs: threadHashingComplete
? () => Object.keys(threadHashes).map(id => id.split('|')[1]) ? () => Object.keys(threadHashes).map(id => id.split('|')[1])
: () => null, : () => null,
}), }),
); );
export const threadsStateSyncSpec: StateSyncSpec< export const threadsStateSyncSpec: StateSyncSpec<
LegacyRawThreadInfos, RawThreadInfos,
LegacyRawThreadInfo, RawThreadInfo,
$ReadOnlyArray<ClientThreadInconsistencyReportCreationRequest>, $ReadOnlyArray<ClientThreadInconsistencyReportCreationRequest>,
> = Object.freeze({ > = Object.freeze({
hashKey: 'threadInfos', hashKey: 'threadInfos',
innerHashSpec: { innerHashSpec: {
hashKey: 'threadInfo', hashKey: 'threadInfo',
deleteKey: 'deleteThreadIDs', deleteKey: 'deleteThreadIDs',
rawInfosKey: 'rawThreadInfos', rawInfosKey: 'rawThreadInfos',
}, },
findStoreInconsistencies( findStoreInconsistencies(
action: ProcessServerRequestAction, action: ProcessServerRequestAction,
beforeStateCheck: LegacyRawThreadInfos, beforeStateCheck: RawThreadInfos,
afterStateCheck: LegacyRawThreadInfos, afterStateCheck: RawThreadInfos,
) { ) {
if (_isEqual(beforeStateCheck)(afterStateCheck)) { if (_isEqual(beforeStateCheck)(afterStateCheck)) {
return emptyArray; return emptyArray;
} }
return [ return [
{ {
type: reportTypes.THREAD_INCONSISTENCY, type: reportTypes.THREAD_INCONSISTENCY,
platformDetails: getConfig().platformDetails, platformDetails: getConfig().platformDetails,
Show All 14 Lines
View Options

keyserver/src/updaters/account-updaters.js

// @flow // @flow
import _isEqual from 'lodash/fp/isEqual.js'; import _isEqual from 'lodash/fp/isEqual.js';
import { createSelector } from 'reselect'; import { createSelector } from 'reselect';
import type { StateSyncSpec, BoundStateSyncSpec } from './state-sync-spec.js'; import type { StateSyncSpec, BoundStateSyncSpec } from './state-sync-spec.js';
import type { AppState } from '../../types/redux-types.js'; import type { AppState } from '../../types/redux-types.js';
import { import {
reportTypes, reportTypes,
type ClientThreadInconsistencyReportCreationRequest, type ClientThreadInconsistencyReportCreationRequest,
} from '../../types/report-types.js'; } from '../../types/report-types.js';
import type { ProcessServerRequestAction } from '../../types/request-types.js'; import type { ProcessServerRequestAction } from '../../types/request-types.js';
import { import {
type LegacyRawThreadInfos, type RawThreadInfo,
type LegacyRawThreadInfo, type RawThreadInfos,
} from '../../types/thread-types.js'; } from '../../types/thread-types.js';
import { actionLogger } from '../../utils/action-logger.js'; import { actionLogger } from '../../utils/action-logger.js';
import { getConfig } from '../../utils/config.js'; import { getConfig } from '../../utils/config.js';
import { combineUnorderedHashes, values } from '../../utils/objects.js'; import { combineUnorderedHashes, values } from '../../utils/objects.js';
import { generateReportID } from '../../utils/report-utils.js'; import { generateReportID } from '../../utils/report-utils.js';
import { sanitizeActionSecrets } from '../../utils/sanitization.js'; import { sanitizeActionSecrets } from '../../utils/sanitization.js';
import { ashoatKeyserverID } from '../../utils/validation-utils.js'; import { ashoatKeyserverID } from '../../utils/validation-utils.js';
const selector: ( const selector: (
state: AppState, state: AppState,
) => BoundStateSyncSpec< ) => BoundStateSyncSpec<
LegacyRawThreadInfos, RawThreadInfos,
LegacyRawThreadInfo, RawThreadInfo,
$ReadOnlyArray<ClientThreadInconsistencyReportCreationRequest>, $ReadOnlyArray<ClientThreadInconsistencyReportCreationRequest>,
> = createSelector( > = createSelector(
(state: AppState) => state.integrityStore.threadHashes, (state: AppState) => state.integrityStore.threadHashes,
(state: AppState) => state.integrityStore.threadHashingStatus === 'completed', (state: AppState) => state.integrityStore.threadHashingStatus === 'completed',
(threadHashes: { +[string]: number }, threadHashingComplete: boolean) => ({ (threadHashes: { +[string]: number }, threadHashingComplete: boolean) => ({
...threadsStateSyncSpec, ...threadsStateSyncSpec,
getInfoHash: (id: string) => threadHashes[`${ashoatKeyserverID}|${id}`], getInfoHash: (id: string) => threadHashes[`${ashoatKeyserverID}|${id}`],
getAllInfosHash: threadHashingComplete getAllInfosHash: threadHashingComplete
? () => combineUnorderedHashes(values(threadHashes)) ? () => combineUnorderedHashes(values(threadHashes))
: () => null, : () => null,
getIDs: threadHashingComplete getIDs: threadHashingComplete
? () => Object.keys(threadHashes).map(id => id.split('|')[1]) ? () => Object.keys(threadHashes).map(id => id.split('|')[1])
: () => null, : () => null,
}), }),
); );
export const threadsStateSyncSpec: StateSyncSpec< export const threadsStateSyncSpec: StateSyncSpec<
LegacyRawThreadInfos, RawThreadInfos,
LegacyRawThreadInfo, RawThreadInfo,
$ReadOnlyArray<ClientThreadInconsistencyReportCreationRequest>, $ReadOnlyArray<ClientThreadInconsistencyReportCreationRequest>,
> = Object.freeze({ > = Object.freeze({
hashKey: 'threadInfos', hashKey: 'threadInfos',
innerHashSpec: { innerHashSpec: {
hashKey: 'threadInfo', hashKey: 'threadInfo',
deleteKey: 'deleteThreadIDs', deleteKey: 'deleteThreadIDs',
rawInfosKey: 'rawThreadInfos', rawInfosKey: 'rawThreadInfos',
}, },
findStoreInconsistencies( findStoreInconsistencies(
action: ProcessServerRequestAction, action: ProcessServerRequestAction,
beforeStateCheck: LegacyRawThreadInfos, beforeStateCheck: RawThreadInfos,
afterStateCheck: LegacyRawThreadInfos, afterStateCheck: RawThreadInfos,
) { ) {
if (_isEqual(beforeStateCheck)(afterStateCheck)) { if (_isEqual(beforeStateCheck)(afterStateCheck)) {
return emptyArray; return emptyArray;
} }
return [ return [
{ {
type: reportTypes.THREAD_INCONSISTENCY, type: reportTypes.THREAD_INCONSISTENCY,
platformDetails: getConfig().platformDetails, platformDetails: getConfig().platformDetails,
Show All 14 Lines