Page MenuHomePhabricator
Differential D8246

[web] Refresh invite links
ClosedPublic
Actions

Authored by tomek on Jun 19 2023, 8:42 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Mar 15, 5:59 PM
Unknown Object (File)
Thu, Mar 13, 6:57 AM
Unknown Object (File)
Mon, Mar 10, 9:52 AM
Unknown Object (File)
Thu, Mar 6, 5:07 PM
Unknown Object (File)
Thu, Mar 6, 5:07 PM
Unknown Object (File)
Thu, Mar 6, 5:07 PM
Unknown Object (File)
Thu, Mar 6, 5:07 PM
Unknown Object (File)
Thu, Mar 6, 5:00 PM
View All Files
Subscribers
ashoat

Details

Reviewers
bartek
kamil
inka
Commits
rCOMMbd5f5b2f701e: [web] Refresh invite links
Summary

Every time a window becomes active, fetch invite links so that we can display the most recent version without requesting too often. Ultimately, we would like to have a mechanism where every invite link change results in updates being sent to interested clients.

Depends on D8239

Test Plan

Check if actions are dispatched when the window becomes active.
Modify website responder so that an empty invite links store is returned. Check if the links are updated after successful result is returned from the server when a window becomes active.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

tomek created this revision.Jun 19 2023, 8:42 AM
Herald added a subscriber: ashoat. · View Herald TranscriptJun 19 2023, 8:42 AM
Harbormaster completed remote builds in B20323: Diff 27858.Jun 19 2023, 8:59 AM
tomek requested review of this revision.Jun 19 2023, 8:59 AM
kamil accepted this revision.Jun 22 2023, 3:29 AM
kamil added inline comments.
web/invite-links/invite-links-refresher.react.js
22 ↗(On Diff #27858)

shouldn't we also check whether we have logged in user to avoid not necessary endpoint calls?

This revision is now accepted and ready to land.Jun 22 2023, 3:29 AM
tomek added inline comments.Jun 22 2023, 7:28 AM
web/invite-links/invite-links-refresher.react.js
22 ↗(On Diff #27858)

Ok, makes sense!

tomek updated this revision to Diff 28005.Jun 22 2023, 8:11 AM

Fetch links only when user is logged in and clear the store after logout

Harbormaster failed remote builds in B20431: Diff 28005!Jun 22 2023, 8:33 AM
tomek added a child revision: D8294: [web] Update button label after successful copy.Jun 22 2023, 8:55 AM
Harbormaster completed remote builds in B20431: Diff 28005.Jun 26 2023, 4:13 AM
Closed by commit rCOMMbd5f5b2f701e: [web] Refresh invite links (authored by tomek). · Explain WhyJun 26 2023, 6:04 AM
This revision was automatically updated to reflect the committed changes.
tomek added a commit: rCOMMbd5f5b2f701e: [web] Refresh invite links.

Revision Contents
Changeset List

PathSize
lib/
reducers/
14 lines
web/
2 lines
invite-links/
36 lines

Diff 28005

View Options

lib/reducers/invite-links-reducer.js

// @flow // @flow
import invariant from 'invariant'; import invariant from 'invariant';
import { import {
pushTypes, pushTypes,
type CreateMessageInfoParams, type CreateMessageInfoParams,
type MessageSpec, type MessageSpec,
type RobotextParams, type RobotextParams,
} from './message-spec.js'; } from './message-spec.js';
import { joinResult } from './utils.js'; import { joinResult } from './utils.js';
import type { PlatformDetails } from '../../types/device-types.js';
import { messageTypes } from '../../types/message-types-enum.js'; import { messageTypes } from '../../types/message-types-enum.js';
import type { import type {
MessageInfo, MessageInfo,
ClientDBMessageInfo, ClientDBMessageInfo,
} from '../../types/message-types.js'; } from '../../types/message-types.js';
import { import {
type ChangeRoleMessageData, type ChangeRoleMessageData,
type ChangeRoleMessageInfo, type ChangeRoleMessageInfo,
type RawChangeRoleMessageInfo, type RawChangeRoleMessageInfo,
rawChangeRoleMessageInfoValidator, rawChangeRoleMessageInfoValidator,
} from '../../types/messages/change-role.js'; } from '../../types/messages/change-role.js';
import type { RawUnsupportedMessageInfo } from '../../types/messages/unsupported';
import type { NotifTexts } from '../../types/notif-types.js'; import type { NotifTexts } from '../../types/notif-types.js';
import type { ThreadInfo } from '../../types/thread-types.js'; import type { ThreadInfo } from '../../types/thread-types.js';
import type { RelativeUserInfo } from '../../types/user-types.js'; import type { RelativeUserInfo } from '../../types/user-types.js';
import { import {
ET, ET,
type EntityText, type EntityText,
pluralizeEntityText, pluralizeEntityText,
} from '../../utils/entity-text.js'; } from '../../utils/entity-text.js';
import { values } from '../../utils/objects.js'; import { values } from '../../utils/objects.js';
import { notifRobotextForMessageInfo } from '../notif-utils.js'; import { notifRobotextForMessageInfo } from '../notif-utils.js';
import { NEXT_CODE_VERSION, hasMinCodeVersion } from '../version-utils.js';
export const changeRoleMessageSpec: MessageSpec< export const changeRoleMessageSpec: MessageSpec<
ChangeRoleMessageData, ChangeRoleMessageData,
RawChangeRoleMessageInfo, RawChangeRoleMessageInfo,
ChangeRoleMessageInfo, ChangeRoleMessageInfo,
> = Object.freeze({ > = Object.freeze({
messageContentForServerDB( messageContentForServerDB(
data: ChangeRoleMessageData | RawChangeRoleMessageInfo, data: ChangeRoleMessageData | RawChangeRoleMessageInfo,
): string { ): string {
return JSON.stringify({ return JSON.stringify({
userIDs: data.userIDs, userIDs: data.userIDs,
newRole: data.newRole, newRole: data.newRole,
roleName: data.roleName,
}); });
}, },
messageContentForClientDB(data: RawChangeRoleMessageInfo): string { messageContentForClientDB(data: RawChangeRoleMessageInfo): string {
return this.messageContentForServerDB(data); return this.messageContentForServerDB(data);
}, },
rawMessageInfoFromServerDBRow(row: Object): RawChangeRoleMessageInfo { rawMessageInfoFromServerDBRow(row: Object): RawChangeRoleMessageInfo {
const content = JSON.parse(row.content); const content = JSON.parse(row.content);
return { return {
type: messageTypes.CHANGE_ROLE, type: messageTypes.CHANGE_ROLE,
id: row.id.toString(), id: row.id.toString(),
threadID: row.threadID.toString(), threadID: row.threadID.toString(),
time: row.time, time: row.time,
creatorID: row.creatorID.toString(), creatorID: row.creatorID.toString(),
userIDs: content.userIDs, userIDs: content.userIDs,
newRole: content.newRole, newRole: content.newRole,
roleName: content.roleName,
}; };
}, },
rawMessageInfoFromClientDB( rawMessageInfoFromClientDB(
clientDBMessageInfo: ClientDBMessageInfo, clientDBMessageInfo: ClientDBMessageInfo,
): RawChangeRoleMessageInfo { ): RawChangeRoleMessageInfo {
invariant( invariant(
clientDBMessageInfo.content !== undefined && clientDBMessageInfo.content !== undefined &&
clientDBMessageInfo.content !== null, clientDBMessageInfo.content !== null,
'content must be defined for ChangeRole', 'content must be defined for ChangeRole',
); );
const content = JSON.parse(clientDBMessageInfo.content); const content = JSON.parse(clientDBMessageInfo.content);
const rawChangeRoleMessageInfo: RawChangeRoleMessageInfo = { const rawChangeRoleMessageInfo: RawChangeRoleMessageInfo = {
type: messageTypes.CHANGE_ROLE, type: messageTypes.CHANGE_ROLE,
id: clientDBMessageInfo.id, id: clientDBMessageInfo.id,
threadID: clientDBMessageInfo.thread, threadID: clientDBMessageInfo.thread,
time: parseInt(clientDBMessageInfo.time), time: parseInt(clientDBMessageInfo.time),
creatorID: clientDBMessageInfo.user, creatorID: clientDBMessageInfo.user,
userIDs: content.userIDs, userIDs: content.userIDs,
newRole: content.newRole, newRole: content.newRole,
roleName: content.roleName,
}; };
return rawChangeRoleMessageInfo; return rawChangeRoleMessageInfo;
}, },
createMessageInfo( createMessageInfo(
rawMessageInfo: RawChangeRoleMessageInfo, rawMessageInfo: RawChangeRoleMessageInfo,
creator: RelativeUserInfo, creator: RelativeUserInfo,
params: CreateMessageInfoParams, params: CreateMessageInfoParams,
): ChangeRoleMessageInfo { ): ChangeRoleMessageInfo {
const members = params.createRelativeUserInfos(rawMessageInfo.userIDs); const members = params.createRelativeUserInfos(rawMessageInfo.userIDs);
return { return {
type: messageTypes.CHANGE_ROLE, type: messageTypes.CHANGE_ROLE,
id: rawMessageInfo.id, id: rawMessageInfo.id,
threadID: rawMessageInfo.threadID, threadID: rawMessageInfo.threadID,
creator, creator,
time: rawMessageInfo.time, time: rawMessageInfo.time,
members, members,
newRole: rawMessageInfo.newRole, newRole: rawMessageInfo.newRole,
roleName: rawMessageInfo.roleName,
}; };
}, },
rawMessageInfoFromMessageData( rawMessageInfoFromMessageData(
messageData: ChangeRoleMessageData, messageData: ChangeRoleMessageData,
id: ?string, id: ?string,
): RawChangeRoleMessageInfo { ): RawChangeRoleMessageInfo {
invariant(id, 'RawChangeRoleMessageInfo needs id'); invariant(id, 'RawChangeRoleMessageInfo needs id');
Show All 9 Lines ): EntityText {
const creator = ET.user({ userInfo: messageInfo.creator }); const creator = ET.user({ userInfo: messageInfo.creator });
const affectedUsers = pluralizeEntityText( const affectedUsers = pluralizeEntityText(
users.map(user => ET`${ET.user({ userInfo: user })}`), users.map(user => ET`${ET.user({ userInfo: user })}`),
); );
const { threadInfo } = params; const { threadInfo } = params;
invariant(threadInfo, 'ThreadInfo should be set for CHANGE_ROLE message'); invariant(threadInfo, 'ThreadInfo should be set for CHANGE_ROLE message');
const roleName = threadInfo.roles[messageInfo.newRole].name;
const threadRoleName = threadInfo.roles[messageInfo.newRole]?.name || null;
ashoatUnsubmitted
Not Done
Inline Actions

What's the || null for? Typically ?? null is preferred is you really want to coerce to null, but I'm not clear on why you want to do that

ashoat: What's the `|| null` for? Typically `?? null` is preferred is you really want to coerce to…
const messageInfoRoleName = messageInfo.roleName;
const roleName = threadRoleName || messageInfoRoleName;
ashoatUnsubmitted
Not Done
Inline Actions

?? instead of ||

ashoat: `??` instead of `||`
invariant(
roleName,
'roleName should either be derived from threadInfo or messageInfo',
);
return ET`${creator} assigned ${affectedUsers} the \"${roleName}\" role`; return ET`${creator} assigned ${affectedUsers} the \"${roleName}\" role`;
}, },
async notificationTexts( async notificationTexts(
messageInfos: $ReadOnlyArray<MessageInfo>, messageInfos: $ReadOnlyArray<MessageInfo>,
threadInfo: ThreadInfo, threadInfo: ThreadInfo,
): Promise<NotifTexts> { ): Promise<NotifTexts> {
Show All 23 Lines ): Promise<NotifTexts> {
})}`; })}`;
return { return {
merged, merged,
title: threadInfo.uiName, title: threadInfo.uiName,
body: robotext, body: robotext,
}; };
}, },
shimUnsupportedMessageInfo(
rawMessageInfo: RawChangeRoleMessageInfo,
platformDetails: ?PlatformDetails,
): RawChangeRoleMessageInfo | RawUnsupportedMessageInfo {
if (hasMinCodeVersion(platformDetails, { native: NEXT_CODE_VERSION })) {
rohanAuthorUnsubmitted
Done
Inline Actions

I'm not sure if this is the right constant to use here / what to do prior to landing.

Before landing, should I replace it like usual with current code version + 2 to include public and staff releases Or is that part of the release process now and I should leave it as is?

rohan: I'm not sure if this is the right constant to use here / what to do prior to landing. Before…
ashoatUnsubmitted
Not Done
Inline Actions

Or is that part of the release process now and I should leave it as is?

That's the idea, but it helps to ping @atul to give him a heads-up

ashoat: > Or is that part of the release process now and I should leave it as is? That's the idea, but…
return rawMessageInfo;
}
const { id } = rawMessageInfo;
invariant(id !== null && id !== undefined, 'id should be set on server');
return {
type: messageTypes.UNSUPPORTED,
id,
threadID: rawMessageInfo.threadID,
creatorID: rawMessageInfo.creatorID,
time: rawMessageInfo.time,
robotext: 'changed the role of some members',
ashoatUnsubmitted
Not Done
Inline Actions

Should we make this look more like the result of the robotext function? Is it possible to factor out the assigned ${affectedUsers} the \"${roleName}\" role part of the robotext method and call it here?

Worth noting that there are some nuances here for converting from`EntityText` to a string since it requires ENS resolution. Some ideas:

  1. Call entityTextToRawString and accept that ENS resolution would not occur
  2. Use "some members" or "a member" instead of affectedUsers for this case, so that ENS resolution doesn't matter. Then you can call entityTextToRawString without worrying
  3. (Best but hardest) call getEntityTextAsString and do ENS resolution, but first you'll need to convert shimUnsupportedMessageInfo and shimUnsupportedRawMessageInfos into async functions
ashoat: Should we make this look more like the result of the `robotext` function? Is it possible to…
rohanAuthorUnsubmitted
Done
Inline Actions

I thought the idea was that older clients won't have access to the new roleName field in the first place, so we'd have to shim the message. The original way we're getting the roleName is by accessing threadInfo.roles[messageInfo.newRole].name, but I took a look at the message-spec and it doesn't look like there's a way we can access params to get the threadInfo.

TLDR of that is I don't think we can access the role name for older clients with just the messageInfo.

Unless maybe I'm misundestanding the problem with 'older clients', and accessing messageInfo.roleName here won't actually be an issue

rohan: I thought the idea was that older clients won't have access to the new `roleName` field in the…
ashoatUnsubmitted
Not Done
Inline Actions

I think shimUnsupportedMessageInfo gets called on the keyserver, not on clients

ashoat: I think `shimUnsupportedMessageInfo` gets called on the keyserver, not on clients
rohanAuthorUnsubmitted
Done
Inline Actions

Ah yeah good point

rohan: Ah yeah good point
unsupportedMessageInfo: rawMessageInfo,
};
},
unshimMessageInfo(
unwrapped: RawChangeRoleMessageInfo,
): RawChangeRoleMessageInfo {
return unwrapped;
},
notificationCollapseKey(rawMessageInfo: RawChangeRoleMessageInfo): string { notificationCollapseKey(rawMessageInfo: RawChangeRoleMessageInfo): string {
return joinResult( return joinResult(
rawMessageInfo.type, rawMessageInfo.type,
rawMessageInfo.threadID, rawMessageInfo.threadID,
rawMessageInfo.creatorID, rawMessageInfo.creatorID,
rawMessageInfo.newRole, rawMessageInfo.newRole,
); );
}, },
generatesNotifs: async () => pushTypes.NOTIF, generatesNotifs: async () => pushTypes.NOTIF,
validator: rawChangeRoleMessageInfoValidator, validator: rawChangeRoleMessageInfoValidator,
}); });
View Options

web/app.react.js

// @flow // @flow
import invariant from 'invariant'; import invariant from 'invariant';
import { import {
pushTypes, pushTypes,
type CreateMessageInfoParams, type CreateMessageInfoParams,
type MessageSpec, type MessageSpec,
type RobotextParams, type RobotextParams,
} from './message-spec.js'; } from './message-spec.js';
import { joinResult } from './utils.js'; import { joinResult } from './utils.js';
import type { PlatformDetails } from '../../types/device-types.js';
import { messageTypes } from '../../types/message-types-enum.js'; import { messageTypes } from '../../types/message-types-enum.js';
import type { import type {
MessageInfo, MessageInfo,
ClientDBMessageInfo, ClientDBMessageInfo,
} from '../../types/message-types.js'; } from '../../types/message-types.js';
import { import {
type ChangeRoleMessageData, type ChangeRoleMessageData,
type ChangeRoleMessageInfo, type ChangeRoleMessageInfo,
type RawChangeRoleMessageInfo, type RawChangeRoleMessageInfo,
rawChangeRoleMessageInfoValidator, rawChangeRoleMessageInfoValidator,
} from '../../types/messages/change-role.js'; } from '../../types/messages/change-role.js';
import type { RawUnsupportedMessageInfo } from '../../types/messages/unsupported';
import type { NotifTexts } from '../../types/notif-types.js'; import type { NotifTexts } from '../../types/notif-types.js';
import type { ThreadInfo } from '../../types/thread-types.js'; import type { ThreadInfo } from '../../types/thread-types.js';
import type { RelativeUserInfo } from '../../types/user-types.js'; import type { RelativeUserInfo } from '../../types/user-types.js';
import { import {
ET, ET,
type EntityText, type EntityText,
pluralizeEntityText, pluralizeEntityText,
} from '../../utils/entity-text.js'; } from '../../utils/entity-text.js';
import { values } from '../../utils/objects.js'; import { values } from '../../utils/objects.js';
import { notifRobotextForMessageInfo } from '../notif-utils.js'; import { notifRobotextForMessageInfo } from '../notif-utils.js';
import { NEXT_CODE_VERSION, hasMinCodeVersion } from '../version-utils.js';
export const changeRoleMessageSpec: MessageSpec< export const changeRoleMessageSpec: MessageSpec<
ChangeRoleMessageData, ChangeRoleMessageData,
RawChangeRoleMessageInfo, RawChangeRoleMessageInfo,
ChangeRoleMessageInfo, ChangeRoleMessageInfo,
> = Object.freeze({ > = Object.freeze({
messageContentForServerDB( messageContentForServerDB(
data: ChangeRoleMessageData | RawChangeRoleMessageInfo, data: ChangeRoleMessageData | RawChangeRoleMessageInfo,
): string { ): string {
return JSON.stringify({ return JSON.stringify({
userIDs: data.userIDs, userIDs: data.userIDs,
newRole: data.newRole, newRole: data.newRole,
roleName: data.roleName,
}); });
}, },
messageContentForClientDB(data: RawChangeRoleMessageInfo): string { messageContentForClientDB(data: RawChangeRoleMessageInfo): string {
return this.messageContentForServerDB(data); return this.messageContentForServerDB(data);
}, },
rawMessageInfoFromServerDBRow(row: Object): RawChangeRoleMessageInfo { rawMessageInfoFromServerDBRow(row: Object): RawChangeRoleMessageInfo {
const content = JSON.parse(row.content); const content = JSON.parse(row.content);
return { return {
type: messageTypes.CHANGE_ROLE, type: messageTypes.CHANGE_ROLE,
id: row.id.toString(), id: row.id.toString(),
threadID: row.threadID.toString(), threadID: row.threadID.toString(),
time: row.time, time: row.time,
creatorID: row.creatorID.toString(), creatorID: row.creatorID.toString(),
userIDs: content.userIDs, userIDs: content.userIDs,
newRole: content.newRole, newRole: content.newRole,
roleName: content.roleName,
}; };
}, },
rawMessageInfoFromClientDB( rawMessageInfoFromClientDB(
clientDBMessageInfo: ClientDBMessageInfo, clientDBMessageInfo: ClientDBMessageInfo,
): RawChangeRoleMessageInfo { ): RawChangeRoleMessageInfo {
invariant( invariant(
clientDBMessageInfo.content !== undefined && clientDBMessageInfo.content !== undefined &&
clientDBMessageInfo.content !== null, clientDBMessageInfo.content !== null,
'content must be defined for ChangeRole', 'content must be defined for ChangeRole',
); );
const content = JSON.parse(clientDBMessageInfo.content); const content = JSON.parse(clientDBMessageInfo.content);
const rawChangeRoleMessageInfo: RawChangeRoleMessageInfo = { const rawChangeRoleMessageInfo: RawChangeRoleMessageInfo = {
type: messageTypes.CHANGE_ROLE, type: messageTypes.CHANGE_ROLE,
id: clientDBMessageInfo.id, id: clientDBMessageInfo.id,
threadID: clientDBMessageInfo.thread, threadID: clientDBMessageInfo.thread,
time: parseInt(clientDBMessageInfo.time), time: parseInt(clientDBMessageInfo.time),
creatorID: clientDBMessageInfo.user, creatorID: clientDBMessageInfo.user,
userIDs: content.userIDs, userIDs: content.userIDs,
newRole: content.newRole, newRole: content.newRole,
roleName: content.roleName,
}; };
return rawChangeRoleMessageInfo; return rawChangeRoleMessageInfo;
}, },
createMessageInfo( createMessageInfo(
rawMessageInfo: RawChangeRoleMessageInfo, rawMessageInfo: RawChangeRoleMessageInfo,
creator: RelativeUserInfo, creator: RelativeUserInfo,
params: CreateMessageInfoParams, params: CreateMessageInfoParams,
): ChangeRoleMessageInfo { ): ChangeRoleMessageInfo {
const members = params.createRelativeUserInfos(rawMessageInfo.userIDs); const members = params.createRelativeUserInfos(rawMessageInfo.userIDs);
return { return {
type: messageTypes.CHANGE_ROLE, type: messageTypes.CHANGE_ROLE,
id: rawMessageInfo.id, id: rawMessageInfo.id,
threadID: rawMessageInfo.threadID, threadID: rawMessageInfo.threadID,
creator, creator,
time: rawMessageInfo.time, time: rawMessageInfo.time,
members, members,
newRole: rawMessageInfo.newRole, newRole: rawMessageInfo.newRole,
roleName: rawMessageInfo.roleName,
}; };
}, },
rawMessageInfoFromMessageData( rawMessageInfoFromMessageData(
messageData: ChangeRoleMessageData, messageData: ChangeRoleMessageData,
id: ?string, id: ?string,
): RawChangeRoleMessageInfo { ): RawChangeRoleMessageInfo {
invariant(id, 'RawChangeRoleMessageInfo needs id'); invariant(id, 'RawChangeRoleMessageInfo needs id');
Show All 9 Lines ): EntityText {
const creator = ET.user({ userInfo: messageInfo.creator }); const creator = ET.user({ userInfo: messageInfo.creator });
const affectedUsers = pluralizeEntityText( const affectedUsers = pluralizeEntityText(
users.map(user => ET`${ET.user({ userInfo: user })}`), users.map(user => ET`${ET.user({ userInfo: user })}`),
); );
const { threadInfo } = params; const { threadInfo } = params;
invariant(threadInfo, 'ThreadInfo should be set for CHANGE_ROLE message'); invariant(threadInfo, 'ThreadInfo should be set for CHANGE_ROLE message');
const roleName = threadInfo.roles[messageInfo.newRole].name;
const threadRoleName = threadInfo.roles[messageInfo.newRole]?.name || null;
ashoatUnsubmitted
Not Done
Inline Actions

What's the || null for? Typically ?? null is preferred is you really want to coerce to null, but I'm not clear on why you want to do that

ashoat: What's the `|| null` for? Typically `?? null` is preferred is you really want to coerce to…
const messageInfoRoleName = messageInfo.roleName;
const roleName = threadRoleName || messageInfoRoleName;
ashoatUnsubmitted
Not Done
Inline Actions

?? instead of ||

ashoat: `??` instead of `||`
invariant(
roleName,
'roleName should either be derived from threadInfo or messageInfo',
);
return ET`${creator} assigned ${affectedUsers} the \"${roleName}\" role`; return ET`${creator} assigned ${affectedUsers} the \"${roleName}\" role`;
}, },
async notificationTexts( async notificationTexts(
messageInfos: $ReadOnlyArray<MessageInfo>, messageInfos: $ReadOnlyArray<MessageInfo>,
threadInfo: ThreadInfo, threadInfo: ThreadInfo,
): Promise<NotifTexts> { ): Promise<NotifTexts> {
Show All 23 Lines ): Promise<NotifTexts> {
})}`; })}`;
return { return {
merged, merged,
title: threadInfo.uiName, title: threadInfo.uiName,
body: robotext, body: robotext,
}; };
}, },
shimUnsupportedMessageInfo(
rawMessageInfo: RawChangeRoleMessageInfo,
platformDetails: ?PlatformDetails,
): RawChangeRoleMessageInfo | RawUnsupportedMessageInfo {
if (hasMinCodeVersion(platformDetails, { native: NEXT_CODE_VERSION })) {
rohanAuthorUnsubmitted
Done
Inline Actions

I'm not sure if this is the right constant to use here / what to do prior to landing.

Before landing, should I replace it like usual with current code version + 2 to include public and staff releases Or is that part of the release process now and I should leave it as is?

rohan: I'm not sure if this is the right constant to use here / what to do prior to landing. Before…
ashoatUnsubmitted
Not Done
Inline Actions

Or is that part of the release process now and I should leave it as is?

That's the idea, but it helps to ping @atul to give him a heads-up

ashoat: > Or is that part of the release process now and I should leave it as is? That's the idea, but…
return rawMessageInfo;
}
const { id } = rawMessageInfo;
invariant(id !== null && id !== undefined, 'id should be set on server');
return {
type: messageTypes.UNSUPPORTED,
id,
threadID: rawMessageInfo.threadID,
creatorID: rawMessageInfo.creatorID,
time: rawMessageInfo.time,
robotext: 'changed the role of some members',
ashoatUnsubmitted
Not Done
Inline Actions

Should we make this look more like the result of the robotext function? Is it possible to factor out the assigned ${affectedUsers} the \"${roleName}\" role part of the robotext method and call it here?

Worth noting that there are some nuances here for converting from`EntityText` to a string since it requires ENS resolution. Some ideas:

  1. Call entityTextToRawString and accept that ENS resolution would not occur
  2. Use "some members" or "a member" instead of affectedUsers for this case, so that ENS resolution doesn't matter. Then you can call entityTextToRawString without worrying
  3. (Best but hardest) call getEntityTextAsString and do ENS resolution, but first you'll need to convert shimUnsupportedMessageInfo and shimUnsupportedRawMessageInfos into async functions
ashoat: Should we make this look more like the result of the `robotext` function? Is it possible to…
rohanAuthorUnsubmitted
Done
Inline Actions

I thought the idea was that older clients won't have access to the new roleName field in the first place, so we'd have to shim the message. The original way we're getting the roleName is by accessing threadInfo.roles[messageInfo.newRole].name, but I took a look at the message-spec and it doesn't look like there's a way we can access params to get the threadInfo.

TLDR of that is I don't think we can access the role name for older clients with just the messageInfo.

Unless maybe I'm misundestanding the problem with 'older clients', and accessing messageInfo.roleName here won't actually be an issue

rohan: I thought the idea was that older clients won't have access to the new `roleName` field in the…
ashoatUnsubmitted
Not Done
Inline Actions

I think shimUnsupportedMessageInfo gets called on the keyserver, not on clients

ashoat: I think `shimUnsupportedMessageInfo` gets called on the keyserver, not on clients
rohanAuthorUnsubmitted
Done
Inline Actions

Ah yeah good point

rohan: Ah yeah good point
unsupportedMessageInfo: rawMessageInfo,
};
},
unshimMessageInfo(
unwrapped: RawChangeRoleMessageInfo,
): RawChangeRoleMessageInfo {
return unwrapped;
},
notificationCollapseKey(rawMessageInfo: RawChangeRoleMessageInfo): string { notificationCollapseKey(rawMessageInfo: RawChangeRoleMessageInfo): string {
return joinResult( return joinResult(
rawMessageInfo.type, rawMessageInfo.type,
rawMessageInfo.threadID, rawMessageInfo.threadID,
rawMessageInfo.creatorID, rawMessageInfo.creatorID,
rawMessageInfo.newRole, rawMessageInfo.newRole,
); );
}, },
generatesNotifs: async () => pushTypes.NOTIF, generatesNotifs: async () => pushTypes.NOTIF,
validator: rawChangeRoleMessageInfoValidator, validator: rawChangeRoleMessageInfoValidator,
}); });
View Options

web/invite-links/invite-links-refresher.react.js

// @flow // @flow
import invariant from 'invariant'; import invariant from 'invariant';
import { import {
pushTypes, pushTypes,
type CreateMessageInfoParams, type CreateMessageInfoParams,
type MessageSpec, type MessageSpec,
type RobotextParams, type RobotextParams,
} from './message-spec.js'; } from './message-spec.js';
import { joinResult } from './utils.js'; import { joinResult } from './utils.js';
import type { PlatformDetails } from '../../types/device-types.js';
import { messageTypes } from '../../types/message-types-enum.js'; import { messageTypes } from '../../types/message-types-enum.js';
import type { import type {
MessageInfo, MessageInfo,
ClientDBMessageInfo, ClientDBMessageInfo,
} from '../../types/message-types.js'; } from '../../types/message-types.js';
import { import {
type ChangeRoleMessageData, type ChangeRoleMessageData,
type ChangeRoleMessageInfo, type ChangeRoleMessageInfo,
type RawChangeRoleMessageInfo, type RawChangeRoleMessageInfo,
rawChangeRoleMessageInfoValidator, rawChangeRoleMessageInfoValidator,
} from '../../types/messages/change-role.js'; } from '../../types/messages/change-role.js';
import type { RawUnsupportedMessageInfo } from '../../types/messages/unsupported';
import type { NotifTexts } from '../../types/notif-types.js'; import type { NotifTexts } from '../../types/notif-types.js';
import type { ThreadInfo } from '../../types/thread-types.js'; import type { ThreadInfo } from '../../types/thread-types.js';
import type { RelativeUserInfo } from '../../types/user-types.js'; import type { RelativeUserInfo } from '../../types/user-types.js';
import { import {
ET, ET,
type EntityText, type EntityText,
pluralizeEntityText, pluralizeEntityText,
} from '../../utils/entity-text.js'; } from '../../utils/entity-text.js';
import { values } from '../../utils/objects.js'; import { values } from '../../utils/objects.js';
import { notifRobotextForMessageInfo } from '../notif-utils.js'; import { notifRobotextForMessageInfo } from '../notif-utils.js';
import { NEXT_CODE_VERSION, hasMinCodeVersion } from '../version-utils.js';
export const changeRoleMessageSpec: MessageSpec< export const changeRoleMessageSpec: MessageSpec<
ChangeRoleMessageData, ChangeRoleMessageData,
RawChangeRoleMessageInfo, RawChangeRoleMessageInfo,
ChangeRoleMessageInfo, ChangeRoleMessageInfo,
> = Object.freeze({ > = Object.freeze({
messageContentForServerDB( messageContentForServerDB(
data: ChangeRoleMessageData | RawChangeRoleMessageInfo, data: ChangeRoleMessageData | RawChangeRoleMessageInfo,
): string { ): string {
return JSON.stringify({ return JSON.stringify({
userIDs: data.userIDs, userIDs: data.userIDs,
newRole: data.newRole, newRole: data.newRole,
roleName: data.roleName,
}); });
}, },
messageContentForClientDB(data: RawChangeRoleMessageInfo): string { messageContentForClientDB(data: RawChangeRoleMessageInfo): string {
return this.messageContentForServerDB(data); return this.messageContentForServerDB(data);
}, },
rawMessageInfoFromServerDBRow(row: Object): RawChangeRoleMessageInfo { rawMessageInfoFromServerDBRow(row: Object): RawChangeRoleMessageInfo {
const content = JSON.parse(row.content); const content = JSON.parse(row.content);
return { return {
type: messageTypes.CHANGE_ROLE, type: messageTypes.CHANGE_ROLE,
id: row.id.toString(), id: row.id.toString(),
threadID: row.threadID.toString(), threadID: row.threadID.toString(),
time: row.time, time: row.time,
creatorID: row.creatorID.toString(), creatorID: row.creatorID.toString(),
userIDs: content.userIDs, userIDs: content.userIDs,
newRole: content.newRole, newRole: content.newRole,
roleName: content.roleName,
}; };
}, },
rawMessageInfoFromClientDB( rawMessageInfoFromClientDB(
clientDBMessageInfo: ClientDBMessageInfo, clientDBMessageInfo: ClientDBMessageInfo,
): RawChangeRoleMessageInfo { ): RawChangeRoleMessageInfo {
invariant( invariant(
clientDBMessageInfo.content !== undefined && clientDBMessageInfo.content !== undefined &&
clientDBMessageInfo.content !== null, clientDBMessageInfo.content !== null,
'content must be defined for ChangeRole', 'content must be defined for ChangeRole',
); );
const content = JSON.parse(clientDBMessageInfo.content); const content = JSON.parse(clientDBMessageInfo.content);
const rawChangeRoleMessageInfo: RawChangeRoleMessageInfo = { const rawChangeRoleMessageInfo: RawChangeRoleMessageInfo = {
type: messageTypes.CHANGE_ROLE, type: messageTypes.CHANGE_ROLE,
id: clientDBMessageInfo.id, id: clientDBMessageInfo.id,
threadID: clientDBMessageInfo.thread, threadID: clientDBMessageInfo.thread,
time: parseInt(clientDBMessageInfo.time), time: parseInt(clientDBMessageInfo.time),
creatorID: clientDBMessageInfo.user, creatorID: clientDBMessageInfo.user,
userIDs: content.userIDs, userIDs: content.userIDs,
newRole: content.newRole, newRole: content.newRole,
roleName: content.roleName,
}; };
return rawChangeRoleMessageInfo; return rawChangeRoleMessageInfo;
}, },
createMessageInfo( createMessageInfo(
rawMessageInfo: RawChangeRoleMessageInfo, rawMessageInfo: RawChangeRoleMessageInfo,
creator: RelativeUserInfo, creator: RelativeUserInfo,
params: CreateMessageInfoParams, params: CreateMessageInfoParams,
): ChangeRoleMessageInfo { ): ChangeRoleMessageInfo {
const members = params.createRelativeUserInfos(rawMessageInfo.userIDs); const members = params.createRelativeUserInfos(rawMessageInfo.userIDs);
return { return {
type: messageTypes.CHANGE_ROLE, type: messageTypes.CHANGE_ROLE,
id: rawMessageInfo.id, id: rawMessageInfo.id,
threadID: rawMessageInfo.threadID, threadID: rawMessageInfo.threadID,
creator, creator,
time: rawMessageInfo.time, time: rawMessageInfo.time,
members, members,
newRole: rawMessageInfo.newRole, newRole: rawMessageInfo.newRole,
roleName: rawMessageInfo.roleName,
}; };
}, },
rawMessageInfoFromMessageData( rawMessageInfoFromMessageData(
messageData: ChangeRoleMessageData, messageData: ChangeRoleMessageData,
id: ?string, id: ?string,
): RawChangeRoleMessageInfo { ): RawChangeRoleMessageInfo {
invariant(id, 'RawChangeRoleMessageInfo needs id'); invariant(id, 'RawChangeRoleMessageInfo needs id');
Show All 9 Lines ): EntityText {
const creator = ET.user({ userInfo: messageInfo.creator }); const creator = ET.user({ userInfo: messageInfo.creator });
const affectedUsers = pluralizeEntityText( const affectedUsers = pluralizeEntityText(
users.map(user => ET`${ET.user({ userInfo: user })}`), users.map(user => ET`${ET.user({ userInfo: user })}`),
); );
const { threadInfo } = params; const { threadInfo } = params;
invariant(threadInfo, 'ThreadInfo should be set for CHANGE_ROLE message'); invariant(threadInfo, 'ThreadInfo should be set for CHANGE_ROLE message');
const roleName = threadInfo.roles[messageInfo.newRole].name;
const threadRoleName = threadInfo.roles[messageInfo.newRole]?.name || null;
ashoatUnsubmitted
Not Done
Inline Actions

What's the || null for? Typically ?? null is preferred is you really want to coerce to null, but I'm not clear on why you want to do that

ashoat: What's the `|| null` for? Typically `?? null` is preferred is you really want to coerce to…
const messageInfoRoleName = messageInfo.roleName;
const roleName = threadRoleName || messageInfoRoleName;
ashoatUnsubmitted
Not Done
Inline Actions

?? instead of ||

ashoat: `??` instead of `||`
invariant(
roleName,
'roleName should either be derived from threadInfo or messageInfo',
);
return ET`${creator} assigned ${affectedUsers} the \"${roleName}\" role`; return ET`${creator} assigned ${affectedUsers} the \"${roleName}\" role`;
}, },
async notificationTexts( async notificationTexts(
messageInfos: $ReadOnlyArray<MessageInfo>, messageInfos: $ReadOnlyArray<MessageInfo>,
threadInfo: ThreadInfo, threadInfo: ThreadInfo,
): Promise<NotifTexts> { ): Promise<NotifTexts> {
Show All 23 Lines ): Promise<NotifTexts> {
})}`; })}`;
return { return {
merged, merged,
title: threadInfo.uiName, title: threadInfo.uiName,
body: robotext, body: robotext,
}; };
}, },
shimUnsupportedMessageInfo(
rawMessageInfo: RawChangeRoleMessageInfo,
platformDetails: ?PlatformDetails,
): RawChangeRoleMessageInfo | RawUnsupportedMessageInfo {
if (hasMinCodeVersion(platformDetails, { native: NEXT_CODE_VERSION })) {
rohanAuthorUnsubmitted
Done
Inline Actions

I'm not sure if this is the right constant to use here / what to do prior to landing.

Before landing, should I replace it like usual with current code version + 2 to include public and staff releases Or is that part of the release process now and I should leave it as is?

rohan: I'm not sure if this is the right constant to use here / what to do prior to landing. Before…
ashoatUnsubmitted
Not Done
Inline Actions

Or is that part of the release process now and I should leave it as is?

That's the idea, but it helps to ping @atul to give him a heads-up

ashoat: > Or is that part of the release process now and I should leave it as is? That's the idea, but…
return rawMessageInfo;
}
const { id } = rawMessageInfo;
invariant(id !== null && id !== undefined, 'id should be set on server');
return {
type: messageTypes.UNSUPPORTED,
id,
threadID: rawMessageInfo.threadID,
creatorID: rawMessageInfo.creatorID,
time: rawMessageInfo.time,
robotext: 'changed the role of some members',
ashoatUnsubmitted
Not Done
Inline Actions

Should we make this look more like the result of the robotext function? Is it possible to factor out the assigned ${affectedUsers} the \"${roleName}\" role part of the robotext method and call it here?

Worth noting that there are some nuances here for converting from`EntityText` to a string since it requires ENS resolution. Some ideas:

  1. Call entityTextToRawString and accept that ENS resolution would not occur
  2. Use "some members" or "a member" instead of affectedUsers for this case, so that ENS resolution doesn't matter. Then you can call entityTextToRawString without worrying
  3. (Best but hardest) call getEntityTextAsString and do ENS resolution, but first you'll need to convert shimUnsupportedMessageInfo and shimUnsupportedRawMessageInfos into async functions
ashoat: Should we make this look more like the result of the `robotext` function? Is it possible to…
rohanAuthorUnsubmitted
Done
Inline Actions

I thought the idea was that older clients won't have access to the new roleName field in the first place, so we'd have to shim the message. The original way we're getting the roleName is by accessing threadInfo.roles[messageInfo.newRole].name, but I took a look at the message-spec and it doesn't look like there's a way we can access params to get the threadInfo.

TLDR of that is I don't think we can access the role name for older clients with just the messageInfo.

Unless maybe I'm misundestanding the problem with 'older clients', and accessing messageInfo.roleName here won't actually be an issue

rohan: I thought the idea was that older clients won't have access to the new `roleName` field in the…
ashoatUnsubmitted
Not Done
Inline Actions

I think shimUnsupportedMessageInfo gets called on the keyserver, not on clients

ashoat: I think `shimUnsupportedMessageInfo` gets called on the keyserver, not on clients
rohanAuthorUnsubmitted
Done
Inline Actions

Ah yeah good point

rohan: Ah yeah good point
unsupportedMessageInfo: rawMessageInfo,
};
},
unshimMessageInfo(
unwrapped: RawChangeRoleMessageInfo,
): RawChangeRoleMessageInfo {
return unwrapped;
},
notificationCollapseKey(rawMessageInfo: RawChangeRoleMessageInfo): string { notificationCollapseKey(rawMessageInfo: RawChangeRoleMessageInfo): string {
return joinResult( return joinResult(
rawMessageInfo.type, rawMessageInfo.type,
rawMessageInfo.threadID, rawMessageInfo.threadID,
rawMessageInfo.creatorID, rawMessageInfo.creatorID,
rawMessageInfo.newRole, rawMessageInfo.newRole,
); );
}, },
generatesNotifs: async () => pushTypes.NOTIF, generatesNotifs: async () => pushTypes.NOTIF,
validator: rawChangeRoleMessageInfoValidator, validator: rawChangeRoleMessageInfoValidator,
}); });