Page MenuHomePhabricator
Differential D4433

[android] Fix android CI + build with CMake changes
AbandonedPublicDraft
Actions

Authored by • jon on Jul 3 2022, 10:41 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Mar 25, 6:08 PM
Unknown Object (File)
Tue, Mar 18, 7:27 AM
Unknown Object (File)
Sun, Mar 16, 3:11 PM
Unknown Object (File)
Sun, Mar 16, 8:38 AM
Unknown Object (File)
Sun, Mar 16, 8:38 AM
Unknown Object (File)
Tue, Mar 11, 4:20 PM
Unknown Object (File)
Mon, Mar 10, 7:29 PM
Unknown Object (File)
Sat, Mar 8, 11:38 PM
View All Files
Subscribers
• abosh
• adrian
ashoat
atul
tomek

Details

Reviewers
ashoat
• max
• karol
Summary

Update android build to re-use comm cmake projects

Depends on D4432

Test Plan

cd native/android && ./gradlew bundleRelease

Diff Detail

Repository
rCOMM Comm
Branch
joringer/add-nix-clean (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

• jon created this revision.Jul 3 2022, 10:41 AM
Herald added subscribers: • abosh, atul, • adrian and 2 others. · View Herald TranscriptJul 3 2022, 10:41 AM
• jon edited the summary of this revision. (Show Details)Jul 3 2022, 10:42 AM
• jon added a parent revision: D4432: [nix] Add fbjni.
• jon edited the test plan for this revision. (Show Details)
• jon updated this revision to Diff 14125.Jul 3 2022, 10:44 AM
• jon edited the summary of this revision. (Show Details)

Remove androidDevEnv from dev shell

There's currently no way to accept the ndk and patcher
licenses which makes it unusable for now.

• jon updated this revision to Diff 14126.Jul 3 2022, 10:46 AM

Remove explicit includes directories

Should be available through CMake targets

• jon added reviewers: ashoat, • max, • karol.Jul 3 2022, 10:50 AM
Harbormaster failed remote builds in B10235: Diff 14124!Jul 3 2022, 10:51 AM
Harbormaster failed remote builds in B10236: Diff 14125!Jul 3 2022, 10:54 AM
Harbormaster returned this revision to the author for changes because remote builds failed.Jul 3 2022, 10:56 AM
Harbormaster failed remote builds in B10237: Diff 14126!
• jon updated this revision to Diff 14195.Jul 5 2022, 9:25 AM

Stash attempt to do canonical cmake build

Harbormaster returned this revision to the author for changes because remote builds failed.Jul 5 2022, 9:32 AM
Harbormaster failed remote builds in B10297: Diff 14195!
• jon mentioned this in D4448: Update android build with cmake changes.Jul 5 2022, 11:58 AM
• jon abandoned this revision.Jul 5 2022, 12:03 PM

Going to abandon to avoid it polluting the backlog.

Unresolved issues:

  • Demand to target cross compilation requires the sources files to be present, so we can't really use native pre-built nix packages
  • Nixpkgs' android-sdk.composeSdk doesn't really support agreeing to the ndk and patch licenses.
  • Android uses a patched version of cmake 3.10 which makes importing sibling projects impossible

Future work:

  • Support nixpkgs to auto-generate the ndk license in the expected location
  • Use cmake 3.21+ which supports android toolchain files natively
• jon mentioned this in D4483: [Nix] Support building Android through gradle on Linux.Jul 7 2022, 4:15 PM
• jon mentioned this in D3640: Add Android to Nix.Jul 13 2022, 12:31 PM
• jon mentioned this in rCOMM0cb68e57850f: Update android build with cmake changes.Jul 20 2022, 3:18 PM
• jon mentioned this in rCOMM7726e8c8e6d1: [Nix] Support building Android through gradle on Linux.Jul 29 2022, 10:03 AM

Revision Contents
Changeset List

PathSize
1 line
native/
android/
app/
23 lines
nix/
33 lines
17 lines
1 line
9 lines
54 lines

Diff 14125

View Options

flake.nix

// @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

native/android/app/CMakeLists.txt

// @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

nix/android-dev-env.nix

// @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

nix/dev-shell.nix

// @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

nix/fbjni.nix

Loading...
View Options

nix/overlay.nix

Loading...
View Options

nix/tunnelbroker.nix

Loading...