Page MenuHomePhabricator
Differential D13053

[lib] Split change thread settings spec
ClosedPublic
Actions

Authored by tomek on Aug 12 2024, 7:27 AM.
Tags
None
Referenced Files
F5161210: D13053.id43313.diff
Fri, Apr 4, 10:28 AM
Unknown Object (File)
Fri, Apr 4, 9:43 AM
Unknown Object (File)
Fri, Apr 4, 7:23 AM
Unknown Object (File)
Fri, Apr 4, 7:10 AM
Unknown Object (File)
Fri, Apr 4, 5:23 AM
Unknown Object (File)
Thu, Apr 3, 7:53 AM
Unknown Object (File)
Fri, Mar 21, 6:43 AM
Unknown Object (File)
Mar 5 2025, 10:19 PM
View All 85 Files
Subscribers
ashoat

Details

Reviewers
kamil
inka
Commits
rCOMMf8120854b4f3: [lib] Split change thread settings spec
Summary

Avoid sending unnecessary data and simplify the spec logic.

https://linear.app/comm/issue/ENG-8929/split-add-members-operation

Depends on D13052

Test Plan

Checked two scenarios:

  1. Viewer creates a thread and adds a new user to it using change_thread_settings operation
  2. Viewer is added to a thread using change_thread_settings_and_add_viewer operation

In both cases the thread appeared in the store with correct membership array.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tomek created this revision.Aug 12 2024, 7:27 AM
Herald added a subscriber: ashoat. · View Herald TranscriptAug 12 2024, 7:27 AM
Harbormaster completed remote builds in B31049: Diff 43313.Aug 12 2024, 8:55 AM
tomek requested review of this revision.Aug 12 2024, 8:55 AM
tomek added a child revision: D13055: [lib] Allow sending messages to some users.Aug 12 2024, 9:06 AM
ashoat added inline comments.Aug 13 2024, 5:17 AM
lib/shared/dm-ops/add-members-spec.js
27–28 ↗(On Diff #43313)

What's the point of making the field read-only if the value isn't?

inka accepted this revision.Aug 19 2024, 5:56 AM
inka added inline comments.
lib/shared/dm-ops/change-thread-settings-and-add-viewer-spec.js
14 ↗(On Diff #43313)

Can we call it createAddViewerOperation? or createAddViewerToThreadOperation

33 ↗(On Diff #43313)

Can we merge this with the function above?

This revision is now accepted and ready to land.Aug 19 2024, 5:56 AM
tomek updated this revision to Diff 43458.Aug 19 2024, 7:28 AM

Rename, fix the types

tomek added inline comments.Aug 19 2024, 7:29 AM
lib/shared/dm-ops/add-members-spec.js
27–28 ↗(On Diff #43313)

Right, this isn't too useful.

lib/shared/dm-ops/change-thread-settings-and-add-viewer-spec.js
14 ↗(On Diff #43313)

Renamed to createAddViewerAndMembersOperation because there might be more users that get added.

33 ↗(On Diff #43313)

We can't because createAddMembersOperation is used in canBeProcessed.

Harbormaster completed remote builds in B31144: Diff 43458.Aug 19 2024, 7:44 AM
Closed by commit rCOMMf8120854b4f3: [lib] Split change thread settings spec (authored by tomek). · Explain WhyAug 20 2024, 9:33 AM
This revision was automatically updated to reflect the committed changes.
tomek added a commit: rCOMMf8120854b4f3: [lib] Split change thread settings spec.

Revision Contents
Changeset List

Diff 43519

View Options

lib/shared/dm-ops/add-members-spec.js

Show First 20 Lines • Show All 1,077 Lines • ▼ Show 20 Lines memberInfo:
| ServerMemberInfo, | ServerMemberInfo,
): boolean { ): boolean {
if (memberInfo.minimallyEncoded) { if (memberInfo.minimallyEncoded) {
return hasPermission(memberInfo.permissions, threadPermissions.CHANGE_ROLE); return hasPermission(memberInfo.permissions, threadPermissions.CHANGE_ROLE);
} }
return !!memberInfo.permissions[threadPermissions.CHANGE_ROLE]?.value; return !!memberInfo.permissions[threadPermissions.CHANGE_ROLE]?.value;
} }
function roleIsDefaultRole(
roleInfo: ?ClientLegacyRoleInfo | ?RoleInfo,
): boolean {
return !!(roleInfo && roleInfo.isDefault);
}
function roleIsAdminRole(roleInfo: ?ClientLegacyRoleInfo | ?RoleInfo): boolean { function roleIsAdminRole(roleInfo: ?ClientLegacyRoleInfo | ?RoleInfo): boolean {
return !!(roleInfo && !roleInfo.isDefault && roleInfo.name === 'Admins'); return !!(roleInfo && !roleInfo.isDefault && roleInfo.name === 'Admins');
} }
function threadHasAdminRole( function threadHasAdminRole(
threadInfo: ?( threadInfo: ?(
| LegacyRawThreadInfo | LegacyRawThreadInfo
| RawThreadInfo | RawThreadInfo
▲ Show 20 Lines • Show All 746 Lines • ▼ Show 20 Linesexport {
getMinimallyEncodedCurrentUser, getMinimallyEncodedCurrentUser,
threadFrozenDueToBlock, threadFrozenDueToBlock,
threadFrozenDueToViewerBlock, threadFrozenDueToViewerBlock,
rawThreadInfoFromServerThreadInfo, rawThreadInfoFromServerThreadInfo,
threadUIName, threadUIName,
threadInfoFromRawThreadInfo, threadInfoFromRawThreadInfo,
threadTypeDescriptions, threadTypeDescriptions,
memberHasAdminPowers, memberHasAdminPowers,
roleIsDefaultRole,
roleIsAdminRole, roleIsAdminRole,
threadHasAdminRole, threadHasAdminRole,
identifyInvalidatedThreads, identifyInvalidatedThreads,
permissionsDisabledByBlock, permissionsDisabledByBlock,
emptyItemText, emptyItemText,
threadNoun, threadNoun,
threadLabel, threadLabel,
useWatchThread, useWatchThread,
Show All 24 Lines
View Options

lib/shared/dm-ops/add-viewer-to-thread-members-spec.js

Show First 20 Lines • Show All 1,077 Lines • ▼ Show 20 Lines memberInfo:
| ServerMemberInfo, | ServerMemberInfo,
): boolean { ): boolean {
if (memberInfo.minimallyEncoded) { if (memberInfo.minimallyEncoded) {
return hasPermission(memberInfo.permissions, threadPermissions.CHANGE_ROLE); return hasPermission(memberInfo.permissions, threadPermissions.CHANGE_ROLE);
} }
return !!memberInfo.permissions[threadPermissions.CHANGE_ROLE]?.value; return !!memberInfo.permissions[threadPermissions.CHANGE_ROLE]?.value;
} }
function roleIsDefaultRole(
roleInfo: ?ClientLegacyRoleInfo | ?RoleInfo,
): boolean {
return !!(roleInfo && roleInfo.isDefault);
}
function roleIsAdminRole(roleInfo: ?ClientLegacyRoleInfo | ?RoleInfo): boolean { function roleIsAdminRole(roleInfo: ?ClientLegacyRoleInfo | ?RoleInfo): boolean {
return !!(roleInfo && !roleInfo.isDefault && roleInfo.name === 'Admins'); return !!(roleInfo && !roleInfo.isDefault && roleInfo.name === 'Admins');
} }
function threadHasAdminRole( function threadHasAdminRole(
threadInfo: ?( threadInfo: ?(
| LegacyRawThreadInfo | LegacyRawThreadInfo
| RawThreadInfo | RawThreadInfo
▲ Show 20 Lines • Show All 746 Lines • ▼ Show 20 Linesexport {
getMinimallyEncodedCurrentUser, getMinimallyEncodedCurrentUser,
threadFrozenDueToBlock, threadFrozenDueToBlock,
threadFrozenDueToViewerBlock, threadFrozenDueToViewerBlock,
rawThreadInfoFromServerThreadInfo, rawThreadInfoFromServerThreadInfo,
threadUIName, threadUIName,
threadInfoFromRawThreadInfo, threadInfoFromRawThreadInfo,
threadTypeDescriptions, threadTypeDescriptions,
memberHasAdminPowers, memberHasAdminPowers,
roleIsDefaultRole,
roleIsAdminRole, roleIsAdminRole,
threadHasAdminRole, threadHasAdminRole,
identifyInvalidatedThreads, identifyInvalidatedThreads,
permissionsDisabledByBlock, permissionsDisabledByBlock,
emptyItemText, emptyItemText,
threadNoun, threadNoun,
threadLabel, threadLabel,
useWatchThread, useWatchThread,
Show All 24 Lines
View Options

lib/shared/dm-ops/change-thread-settings-and-add-viewer-spec.js

Show First 20 Lines • Show All 1,077 Lines • ▼ Show 20 Lines memberInfo:
| ServerMemberInfo, | ServerMemberInfo,
): boolean { ): boolean {
if (memberInfo.minimallyEncoded) { if (memberInfo.minimallyEncoded) {
return hasPermission(memberInfo.permissions, threadPermissions.CHANGE_ROLE); return hasPermission(memberInfo.permissions, threadPermissions.CHANGE_ROLE);
} }
return !!memberInfo.permissions[threadPermissions.CHANGE_ROLE]?.value; return !!memberInfo.permissions[threadPermissions.CHANGE_ROLE]?.value;
} }
function roleIsDefaultRole(
roleInfo: ?ClientLegacyRoleInfo | ?RoleInfo,
): boolean {
return !!(roleInfo && roleInfo.isDefault);
}
function roleIsAdminRole(roleInfo: ?ClientLegacyRoleInfo | ?RoleInfo): boolean { function roleIsAdminRole(roleInfo: ?ClientLegacyRoleInfo | ?RoleInfo): boolean {
return !!(roleInfo && !roleInfo.isDefault && roleInfo.name === 'Admins'); return !!(roleInfo && !roleInfo.isDefault && roleInfo.name === 'Admins');
} }
function threadHasAdminRole( function threadHasAdminRole(
threadInfo: ?( threadInfo: ?(
| LegacyRawThreadInfo | LegacyRawThreadInfo
| RawThreadInfo | RawThreadInfo
▲ Show 20 Lines • Show All 746 Lines • ▼ Show 20 Linesexport {
getMinimallyEncodedCurrentUser, getMinimallyEncodedCurrentUser,
threadFrozenDueToBlock, threadFrozenDueToBlock,
threadFrozenDueToViewerBlock, threadFrozenDueToViewerBlock,
rawThreadInfoFromServerThreadInfo, rawThreadInfoFromServerThreadInfo,
threadUIName, threadUIName,
threadInfoFromRawThreadInfo, threadInfoFromRawThreadInfo,
threadTypeDescriptions, threadTypeDescriptions,
memberHasAdminPowers, memberHasAdminPowers,
roleIsDefaultRole,
roleIsAdminRole, roleIsAdminRole,
threadHasAdminRole, threadHasAdminRole,
identifyInvalidatedThreads, identifyInvalidatedThreads,
permissionsDisabledByBlock, permissionsDisabledByBlock,
emptyItemText, emptyItemText,
threadNoun, threadNoun,
threadLabel, threadLabel,
useWatchThread, useWatchThread,
Show All 24 Lines
View Options

lib/shared/dm-ops/change-thread-settings-spec.js

Show First 20 Lines • Show All 1,077 Lines • ▼ Show 20 Lines memberInfo:
| ServerMemberInfo, | ServerMemberInfo,
): boolean { ): boolean {
if (memberInfo.minimallyEncoded) { if (memberInfo.minimallyEncoded) {
return hasPermission(memberInfo.permissions, threadPermissions.CHANGE_ROLE); return hasPermission(memberInfo.permissions, threadPermissions.CHANGE_ROLE);
} }
return !!memberInfo.permissions[threadPermissions.CHANGE_ROLE]?.value; return !!memberInfo.permissions[threadPermissions.CHANGE_ROLE]?.value;
} }
function roleIsDefaultRole(
roleInfo: ?ClientLegacyRoleInfo | ?RoleInfo,
): boolean {
return !!(roleInfo && roleInfo.isDefault);
}
function roleIsAdminRole(roleInfo: ?ClientLegacyRoleInfo | ?RoleInfo): boolean { function roleIsAdminRole(roleInfo: ?ClientLegacyRoleInfo | ?RoleInfo): boolean {
return !!(roleInfo && !roleInfo.isDefault && roleInfo.name === 'Admins'); return !!(roleInfo && !roleInfo.isDefault && roleInfo.name === 'Admins');
} }
function threadHasAdminRole( function threadHasAdminRole(
threadInfo: ?( threadInfo: ?(
| LegacyRawThreadInfo | LegacyRawThreadInfo
| RawThreadInfo | RawThreadInfo
▲ Show 20 Lines • Show All 746 Lines • ▼ Show 20 Linesexport {
getMinimallyEncodedCurrentUser, getMinimallyEncodedCurrentUser,
threadFrozenDueToBlock, threadFrozenDueToBlock,
threadFrozenDueToViewerBlock, threadFrozenDueToViewerBlock,
rawThreadInfoFromServerThreadInfo, rawThreadInfoFromServerThreadInfo,
threadUIName, threadUIName,
threadInfoFromRawThreadInfo, threadInfoFromRawThreadInfo,
threadTypeDescriptions, threadTypeDescriptions,
memberHasAdminPowers, memberHasAdminPowers,
roleIsDefaultRole,
roleIsAdminRole, roleIsAdminRole,
threadHasAdminRole, threadHasAdminRole,
identifyInvalidatedThreads, identifyInvalidatedThreads,
permissionsDisabledByBlock, permissionsDisabledByBlock,
emptyItemText, emptyItemText,
threadNoun, threadNoun,
threadLabel, threadLabel,
useWatchThread, useWatchThread,
Show All 24 Lines
View Options

lib/shared/dm-ops/dm-op-specs.js

Loading...
View Options

lib/types/dm-ops.js

Loading...