Don't show name collisions for system contacts

This commit is contained in:
Evan Hahn 2021-06-02 12:24:22 -05:00 committed by GitHub
parent 84be8288e9
commit 6c6eed0b1e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 121 additions and 17 deletions

View File

@ -11,6 +11,7 @@ import { InContactsIcon } from './InContactsIcon';
import { LocalizerType } from '../types/Util';
import { sortByTitle } from '../util/sortByTitle';
import { ConversationType } from '../state/ducks/conversations';
import { isInSystemContacts } from '../util/isInSystemContacts';
type ParticipantType = ConversationType & {
hasRemoteAudio?: boolean;
@ -118,7 +119,7 @@ export const CallingParticipantsList = React.memo(
module="module-calling-participants-list__name"
title={participant.title}
/>
{participant.name ? (
{isInSystemContacts(participant) ? (
<span>
{' '}
<InContactsIcon

View File

@ -11,6 +11,7 @@ import { InContactsIcon } from './InContactsIcon';
import { LocalizerType } from '../types/Util';
import { ConversationType } from '../state/ducks/conversations';
import { isInSystemContacts } from '../util/isInSystemContacts';
type Props = {
i18n: LocalizerType;
@ -69,10 +70,18 @@ export class ContactListItem extends React.Component<Props> {
}
public render(): JSX.Element {
const { about, i18n, isAdmin, isMe, name, onClick, title } = this.props;
const {
about,
i18n,
isAdmin,
isMe,
name,
onClick,
title,
type,
} = this.props;
const displayName = isMe ? i18n('you') : title;
const shouldShowIcon = Boolean(name);
return (
<button
@ -88,7 +97,7 @@ export class ContactListItem extends React.Component<Props> {
<div className="module-contact-list-item__left">
<div className="module-contact-list-item__text__name">
<Emojify text={displayName} />
{shouldShowIcon ? (
{isInSystemContacts({ name, type }) ? (
<span>
{' '}
<InContactsIcon i18n={i18n} />

View File

@ -11,6 +11,7 @@ import { Modal } from './Modal';
import { ConversationType } from '../state/ducks/conversations';
import { LocalizerType } from '../types/Util';
import { isInSystemContacts } from '../util/isInSystemContacts';
export type SafetyNumberProps = {
contactID: string;
@ -103,7 +104,7 @@ export const SafetyNumberChangeDialog = ({
<div className="module-SafetyNumberChangeDialog__contact--wrapper">
<div className="module-SafetyNumberChangeDialog__contact--name">
{contact.title}
{contact.name ? (
{isInSystemContacts(contact) ? (
<span>
{' '}
<InContactsIcon i18n={i18n} />

View File

@ -25,6 +25,7 @@ import { Intl } from '../Intl';
import { Emojify } from './Emojify';
import { assert } from '../../util/assert';
import { missingCaseError } from '../../util/missingCaseError';
import { isInSystemContacts } from '../../util/isInSystemContacts';
type PropsType = {
i18n: LocalizerType;
@ -271,7 +272,7 @@ export const ContactSpoofingReviewDialog: FunctionComponent<PropsType> = props =
{i18n('MessageRequests--unblock')}
</Button>
);
} else if (!conversationInfo.conversation.name) {
} else if (!isInSystemContacts(conversationInfo.conversation)) {
button = (
<Button
variant={ButtonVariant.SecondaryDestructive}

View File

@ -23,6 +23,7 @@ import { MuteOption, getMuteOptions } from '../../util/getMuteOptions';
import * as expirationTimer from '../../util/expirationTimer';
import { isMuted } from '../../util/isMuted';
import { missingCaseError } from '../../util/missingCaseError';
import { isInSystemContacts } from '../../util/isInSystemContacts';
export enum OutgoingCallButtonStyle {
None,
@ -155,12 +156,10 @@ export class ConversationHeader extends React.Component<PropsType, StateType> {
);
}
const shouldShowIcon = Boolean(name && type === 'direct');
return (
<div className="module-ConversationHeader__header__info__title">
<Emojify text={title} />
{shouldShowIcon ? (
{isInSystemContacts({ name, type }) ? (
<InContactsIcon
className="module-ConversationHeader__header__info__title__in-contacts-icon"
i18n={i18n}

View File

@ -2,7 +2,7 @@
// SPDX-License-Identifier: AGPL-3.0-only
import memoizee from 'memoizee';
import { fromPairs, isNumber, isString } from 'lodash';
import { fromPairs, isNumber } from 'lodash';
import { createSelector } from 'reselect';
import { StateType } from '../reducer';
@ -38,6 +38,7 @@ import {
getUserNumber,
} from './user';
import { getPinnedConversationIds } from './items';
import { isInSystemContacts } from '../../util/isInSystemContacts';
let placeholderContact: ConversationType;
export const getPlaceholderContact = (): ConversationType => {
@ -366,7 +367,7 @@ function isTrusted(conversation: ConversationType): boolean {
}
return Boolean(
isString(conversation.name) ||
isInSystemContacts(conversation) ||
conversation.profileSharing ||
conversation.isMe
);

View File

@ -46,6 +46,7 @@ describe('group member name collision utilities', () => {
title: 'Bob',
});
const bob2 = getDefaultConversation({
name: 'Bob In Your Contacts',
profileName: 'Bob',
title: 'Bob',
});
@ -53,19 +54,44 @@ describe('group member name collision utilities', () => {
profileName: 'Bob',
title: 'Bob',
});
// Ignored, because Bob is not in your contacts (lacks `name`), has no profile name,
// and has no E164.
const ignoredBob = getDefaultConversation({
e164: undefined,
title: 'Bob',
});
// Ignored, because there's only one Charlie.
const charlie = getDefaultConversation({
profileName: 'Charlie',
title: 'Charlie',
});
// Ignored, because all Donnas are in your contacts (they have a `name`).
const donna1 = getDefaultConversation({
name: 'Donna One',
profileName: 'Donna',
title: 'Donna',
});
const donna2 = getDefaultConversation({
name: 'Donna Two',
profileName: 'Donna',
title: 'Donna',
});
const donna3 = getDefaultConversation({
name: 'Donna Three',
profileName: 'Donna',
title: 'Donna',
});
// Ignored, because you're not included.
const me = getDefaultConversation({
isMe: true,
profileName: 'Alice',
title: 'Alice',
});
const memberships = [
alice1,
alice2,
@ -74,6 +100,9 @@ describe('group member name collision utilities', () => {
bob3,
ignoredBob,
charlie,
donna1,
donna2,
donna3,
me,
].map(member => ({ member }));

View File

@ -0,0 +1,46 @@
// Copyright 2021 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
import { assert } from 'chai';
import { isInSystemContacts } from '../../util/isInSystemContacts';
describe('isInSystemContacts', () => {
it('returns true for direct conversations that have a `name` property', () => {
assert.isTrue(
isInSystemContacts({
type: 'direct',
name: 'Jane Doe',
})
);
assert.isTrue(
isInSystemContacts({
type: 'private',
name: 'Jane Doe',
})
);
});
it('returns true for direct conversations that have an empty string `name`', () => {
assert.isTrue(
isInSystemContacts({
type: 'direct',
name: '',
})
);
});
it('returns false for direct conversations that lack a `name` property', () => {
assert.isFalse(isInSystemContacts({ type: 'direct' }));
});
it('returns false for group conversations', () => {
assert.isFalse(isInSystemContacts({ type: 'group' }));
assert.isFalse(
isInSystemContacts({
type: 'group',
name: 'Tahoe Trip',
})
);
});
});

View File

@ -6,6 +6,7 @@ import { SendMetadataType, SendOptionsType } from '../textsecure/SendMessage';
import { arrayBufferToBase64, getRandomBytes } from '../Crypto';
import { getConversationMembers } from './getConversationMembers';
import { isDirectConversation, isMe } from './whatTypeOfConversation';
import { isInSystemContacts } from './isInSystemContacts';
import { missingCaseError } from './missingCaseError';
import { senderCertificateService } from '../services/senderCertificate';
import {
@ -117,13 +118,11 @@ function getSenderCertificateForDirectConversation(
case PhoneNumberSharingMode.Everybody:
certificateMode = SenderCertificateMode.WithE164;
break;
case PhoneNumberSharingMode.ContactsOnly: {
const isInSystemContacts = Boolean(conversationAttrs.name);
certificateMode = isInSystemContacts
case PhoneNumberSharingMode.ContactsOnly:
certificateMode = isInSystemContacts(conversationAttrs)
? SenderCertificateMode.WithE164
: SenderCertificateMode.WithoutE164;
break;
}
case PhoneNumberSharingMode.Nobody:
certificateMode = SenderCertificateMode.WithoutE164;
break;

View File

@ -6,6 +6,7 @@ import { groupBy, map, filter } from './iterables';
import { getOwn } from './getOwn';
import { ConversationType } from '../state/ducks/conversations';
import { isConversationNameKnown } from './isConversationNameKnown';
import { isInSystemContacts } from './isInSystemContacts';
export type GroupNameCollisionsWithIdsByTitle = Record<string, Array<string>>;
export type GroupNameCollisionsWithConversationsByTitle = Record<
@ -37,7 +38,8 @@ export function getCollisionsFromMemberships(
// [0]: https://www.typescriptlang.org/play?#code/C4TwDgpgBAYg9nKBeKAFAhgJ2AS3QGwB4AlCAYzkwBNCBnYTHAOwHMAaKJgVwFsAjCJgB8QgNwAoCk3pQAZgC5YCZFADeUABY5FAVigBfCeNCQoAISwrSFanQbN2nXgOESpMvoouYVs0UA
return (pickBy(
groupedByTitle,
group => group.length >= 2
group =>
group.length >= 2 && !group.every(person => isInSystemContacts(person))
) as unknown) as GroupNameCollisionsWithConversationsByTitle;
}

View File

@ -3,6 +3,7 @@
import { ConversationAttributesType } from '../model-types.d';
import { isDirectConversation, isMe } from './whatTypeOfConversation';
import { isInSystemContacts } from './isInSystemContacts';
/**
* Determine if this conversation should be considered "accepted" in terms
@ -63,7 +64,10 @@ function isFromOrAddedByTrustedContact(
conversationAttrs: ConversationAttributesType
): boolean {
if (isDirectConversation(conversationAttrs)) {
return Boolean(conversationAttrs.name || conversationAttrs.profileSharing);
return (
isInSystemContacts(conversationAttrs) ||
Boolean(conversationAttrs.profileSharing)
);
}
const { addedBy } = conversationAttrs;

View File

@ -0,0 +1,12 @@
// Copyright 2021 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
export const isInSystemContacts = ({
type,
name,
}: Readonly<{
type?: string;
name?: string;
}>): boolean =>
// `direct` for redux, `private` for models and the database
(type === 'direct' || type === 'private') && typeof name === 'string';