Timeline: repair oldest/newest metrics if we fetch nothing

This commit is contained in:
Scott Nonnenberg 2020-12-04 12:41:40 -08:00 committed by GitHub
parent 56ae4a41eb
commit 6832b8acca
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
47 changed files with 579 additions and 173 deletions

View file

@ -150,7 +150,7 @@ module.exports = {
rules,
},
{
files: ['**/*.stories.tsx', 'ts/build/**', 'ts/test/**'],
files: ['**/*.stories.tsx', 'ts/build/**', 'ts/test-*/**'],
rules: {
...rules,
'import/no-extraneous-dependencies': 'off',

View file

@ -29,8 +29,8 @@
"publish-to-apt": "NAME=$npm_package_name VERSION=$npm_package_version ./aptly.sh",
"test": "yarn test-node && yarn test-electron",
"test-electron": "yarn grunt test",
"test-node": "electron-mocha --recursive test/app test/modules ts/test",
"test-node-coverage": "nyc --reporter=lcov --reporter=text mocha --recursive test/app test/modules ts/test",
"test-node": "electron-mocha --recursive test/app test/modules ts/test-node ts/test-both",
"test-node-coverage": "nyc --reporter=lcov --reporter=text mocha --recursive test/app test/modules ts/test-node ts/test-both",
"eslint": "eslint .",
"lint": "yarn format --list-different && yarn eslint",
"lint-deps": "node ts/util/lint/linter.js",

View file

@ -563,9 +563,12 @@ try {
};
/* eslint-disable global-require, import/no-extraneous-dependencies */
require('./ts/test-electron/models/messages_test');
require('./ts/test-both/state/ducks/conversations_test');
require('./ts/test-both/state/selectors/conversations_test');
require('./ts/test-both/state/selectors/items_test');
require('./ts/test-electron/linkPreviews/linkPreviewFetch_test');
require('./ts/test-electron/state/ducks/conversations_test');
require('./ts/test-electron/models/messages_test');
require('./ts/test-electron/state/ducks/calling_test');
require('./ts/test-electron/state/selectors/calling_test');

View file

@ -13,6 +13,8 @@ import {
values,
without,
} from 'lodash';
import { getOwn } from '../../util/getOwn';
import { trigger } from '../../shims/events';
import { NoopActionType } from './noop';
import { AttachmentType } from '../../types/Attachment';
@ -281,6 +283,19 @@ export type MessagesAddedActionType = {
isActive: boolean;
};
};
export type RepairNewestMessageActionType = {
type: 'REPAIR_NEWEST_MESSAGE';
payload: {
conversationId: string;
};
};
export type RepairOldestMessageActionType = {
type: 'REPAIR_OLDEST_MESSAGE';
payload: {
conversationId: string;
};
};
export type MessagesResetActionType = {
type: 'MESSAGES_RESET';
payload: {
@ -367,6 +382,8 @@ export type ConversationActionType =
| MessageChangedActionType
| MessageDeletedActionType
| MessagesAddedActionType
| RepairNewestMessageActionType
| RepairOldestMessageActionType
| MessagesResetActionType
| SetMessagesLoadingActionType
| SetIsNearBottomActionType
@ -407,6 +424,8 @@ export const actions = {
openConversationExternal,
showInbox,
showArchivedConversations,
repairNewestMessage,
repairOldestMessage,
};
function conversationAdded(
@ -511,6 +530,28 @@ function messagesAdded(
},
};
}
function repairNewestMessage(
conversationId: string
): RepairNewestMessageActionType {
return {
type: 'REPAIR_NEWEST_MESSAGE',
payload: {
conversationId,
},
};
}
function repairOldestMessage(
conversationId: string
): RepairOldestMessageActionType {
return {
type: 'REPAIR_OLDEST_MESSAGE',
payload: {
conversationId,
},
};
}
function messagesReset(
conversationId: string,
messages: Array<MessageType>,
@ -1119,6 +1160,68 @@ export function reducer(
},
};
}
if (action.type === 'REPAIR_NEWEST_MESSAGE') {
const { conversationId } = action.payload;
const { messagesByConversation, messagesLookup } = state;
const existingConversation = getOwn(messagesByConversation, conversationId);
if (!existingConversation) {
return state;
}
const { messageIds } = existingConversation;
const lastId =
messageIds && messageIds.length
? messageIds[messageIds.length - 1]
: undefined;
const last = lastId ? getOwn(messagesLookup, lastId) : undefined;
const newest = last ? pick(last, ['id', 'received_at']) : undefined;
return {
...state,
messagesByConversation: {
...messagesByConversation,
[conversationId]: {
...existingConversation,
metrics: {
...existingConversation.metrics,
newest,
},
},
},
};
}
if (action.type === 'REPAIR_OLDEST_MESSAGE') {
const { conversationId } = action.payload;
const { messagesByConversation, messagesLookup } = state;
const existingConversation = getOwn(messagesByConversation, conversationId);
if (!existingConversation) {
return state;
}
const { messageIds } = existingConversation;
const firstId = messageIds && messageIds.length ? messageIds[0] : undefined;
const first = firstId ? getOwn(messagesLookup, firstId) : undefined;
const oldest = first ? pick(first, ['id', 'received_at']) : undefined;
return {
...state,
messagesByConversation: {
...messagesByConversation,
[conversationId]: {
...existingConversation,
metrics: {
...existingConversation.metrics,
oldest,
},
},
},
};
}
if (action.type === 'MESSAGES_ADDED') {
const { conversationId, isActive, isNewMessage, messages } = action.payload;
const { messagesByConversation, messagesLookup } = state;

View file

@ -15,6 +15,7 @@ import {
MessagesByConversationType,
MessageType,
} from '../ducks/conversations';
import { getBubbleProps } from '../../shims/Whisper';
import { PropsDataType as TimelinePropsType } from '../../components/conversation/Timeline';
import { TimelineItemType } from '../../components/conversation/TimelineItem';
@ -26,6 +27,7 @@ import {
getUserConversationId,
getUserNumber,
} from './user';
import { getPinnedConversationIds } from './items';
export const getConversations = (state: StateType): ConversationsStateType =>
state.conversations;
@ -127,7 +129,8 @@ export const getConversationComparator = createSelector(
export const _getLeftPaneLists = (
lookup: ConversationLookupType,
comparator: (left: ConversationType, right: ConversationType) => number,
selectedConversation?: string
selectedConversation?: string,
pinnedConversationIds?: Array<string>
): {
conversations: Array<ConversationType>;
archivedConversations: Array<ConversationType>;
@ -162,13 +165,10 @@ export const _getLeftPaneLists = (
conversations.sort(comparator);
archivedConversations.sort(comparator);
const pinnedConversationIds = window.storage.get<Array<string>>(
'pinnedConversationIds',
[]
);
pinnedConversations.sort(
(a, b) =>
pinnedConversationIds.indexOf(a.id) - pinnedConversationIds.indexOf(b.id)
(pinnedConversationIds || []).indexOf(a.id) -
(pinnedConversationIds || []).indexOf(b.id)
);
return { conversations, archivedConversations, pinnedConversations };
@ -178,6 +178,7 @@ export const getLeftPaneLists = createSelector(
getConversationLookup,
getConversationComparator,
getSelectedConversation,
getPinnedConversationIds,
_getLeftPaneLists
);

View file

@ -0,0 +1,20 @@
// Copyright 2019-2020 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
import { createSelector } from 'reselect';
import { StateType } from '../reducer';
import { ItemsStateType } from '../ducks/items';
export const getItems = (state: StateType): ItemsStateType => state.items;
export const getUserAgent = createSelector(
getItems,
(state: ItemsStateType): string => state.userAgent as string
);
export const getPinnedConversationIds = createSelector(
getItems,
(state: ItemsStateType): Array<string> =>
(state.pinnedConversationIds || []) as Array<string>
);

View file

@ -24,7 +24,8 @@ import {
} from '../../components/SearchResults';
import { PropsDataType as MessageSearchResultPropsDataType } from '../../components/MessageSearchResult';
import { getRegionCode, getUserAgent, getUserNumber } from './user';
import { getRegionCode, getUserNumber } from './user';
import { getUserAgent } from './items';
import {
GetConversationByIdType,
getConversationLookup,

View file

@ -7,12 +7,9 @@ import { LocalizerType } from '../../types/Util';
import { StateType } from '../reducer';
import { UserStateType } from '../ducks/user';
import { ItemsStateType } from '../ducks/items';
export const getUser = (state: StateType): UserStateType => state.user;
export const getItems = (state: StateType): ItemsStateType => state.items;
export const getUserNumber = createSelector(
getUser,
(state: UserStateType): string => state.ourNumber
@ -33,11 +30,6 @@ export const getUserUuid = createSelector(
(state: UserStateType): string => state.ourUuid
);
export const getUserAgent = createSelector(
getItems,
(state: ItemsStateType): string => state.userAgent as string
);
export const getIntl = createSelector(
getUser,
(state: UserStateType): LocalizerType => state.i18n

View file

@ -0,0 +1,384 @@
// Copyright 2020 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
import { assert } from 'chai';
import {
actions,
ConversationMessageType,
ConversationsStateType,
ConversationType,
getConversationCallMode,
MessageType,
reducer,
} from '../../../state/ducks/conversations';
import { CallMode } from '../../../types/Calling';
const { repairNewestMessage, repairOldestMessage } = actions;
describe('both/state/ducks/conversations', () => {
describe('helpers', () => {
describe('getConversationCallMode', () => {
const fakeConversation: ConversationType = {
id: 'id1',
e164: '+18005551111',
activeAt: Date.now(),
name: 'No timestamp',
timestamp: 0,
inboxPosition: 0,
phoneNumber: 'notused',
isArchived: false,
markedUnread: false,
type: 'direct',
isMe: false,
lastUpdated: Date.now(),
title: 'No timestamp',
unreadCount: 1,
isSelected: false,
typingContact: {
name: 'Someone There',
color: 'blue',
phoneNumber: '+18005551111',
},
acceptedMessageRequest: true,
};
it("returns CallMode.None if you've left the conversation", () => {
assert.strictEqual(
getConversationCallMode({
...fakeConversation,
left: true,
}),
CallMode.None
);
});
it("returns CallMode.None if you've blocked the other person", () => {
assert.strictEqual(
getConversationCallMode({
...fakeConversation,
isBlocked: true,
}),
CallMode.None
);
});
it("returns CallMode.None if you haven't accepted message requests", () => {
assert.strictEqual(
getConversationCallMode({
...fakeConversation,
acceptedMessageRequest: false,
}),
CallMode.None
);
});
it('returns CallMode.None if the conversation is Note to Self', () => {
assert.strictEqual(
getConversationCallMode({
...fakeConversation,
isMe: true,
}),
CallMode.None
);
});
it('returns CallMode.None for v1 groups', () => {
assert.strictEqual(
getConversationCallMode({
...fakeConversation,
type: 'group',
groupVersion: 1,
}),
CallMode.None
);
assert.strictEqual(
getConversationCallMode({
...fakeConversation,
type: 'group',
}),
CallMode.None
);
});
it('returns CallMode.Direct if the conversation is a normal direct conversation', () => {
assert.strictEqual(
getConversationCallMode(fakeConversation),
CallMode.Direct
);
});
it('returns CallMode.Group if the conversation is a v2 group', () => {
assert.strictEqual(
getConversationCallMode({
...fakeConversation,
type: 'group',
groupVersion: 2,
}),
CallMode.Group
);
});
});
});
describe('reducer', () => {
const time = Date.now();
const conversationId = 'conversation-guid-1';
const messageId = 'message-guid-1';
const messageIdTwo = 'message-guid-2';
const messageIdThree = 'message-guid-3';
function getDefaultState(): ConversationsStateType {
return {
conversationLookup: {},
selectedMessageCounter: 0,
selectedConversationPanelDepth: 0,
showArchived: false,
messagesLookup: {},
messagesByConversation: {},
};
}
function getDefaultMessage(id: string): MessageType {
return {
id,
conversationId: 'conversationId',
source: 'source',
type: 'incoming' as const,
received_at: Date.now(),
attachments: [],
sticker: {},
unread: false,
};
}
function getDefaultConversationMessage(): ConversationMessageType {
return {
heightChangeMessageIds: [],
isLoadingMessages: false,
messageIds: [],
metrics: {
totalUnread: 0,
},
resetCounter: 0,
scrollToMessageCounter: 0,
};
}
describe('REPAIR_NEWEST_MESSAGE', () => {
it('updates newest', () => {
const action = repairNewestMessage(conversationId);
const state: ConversationsStateType = {
...getDefaultState(),
messagesLookup: {
[messageId]: {
...getDefaultMessage(messageId),
received_at: time,
},
},
messagesByConversation: {
[conversationId]: {
...getDefaultConversationMessage(),
messageIds: [messageIdThree, messageIdTwo, messageId],
metrics: {
totalUnread: 0,
},
},
},
};
const expected: ConversationsStateType = {
...getDefaultState(),
messagesLookup: {
[messageId]: {
...getDefaultMessage(messageId),
received_at: time,
},
},
messagesByConversation: {
[conversationId]: {
...getDefaultConversationMessage(),
messageIds: [messageIdThree, messageIdTwo, messageId],
metrics: {
totalUnread: 0,
newest: {
id: messageId,
received_at: time,
},
},
},
},
};
const actual = reducer(state, action);
assert.deepEqual(actual, expected);
});
it('clears newest', () => {
const action = repairNewestMessage(conversationId);
const state: ConversationsStateType = {
...getDefaultState(),
messagesLookup: {
[messageId]: {
...getDefaultMessage(messageId),
received_at: time,
},
},
messagesByConversation: {
[conversationId]: {
...getDefaultConversationMessage(),
messageIds: [],
metrics: {
totalUnread: 0,
newest: {
id: messageId,
received_at: time,
},
},
},
},
};
const expected: ConversationsStateType = {
...getDefaultState(),
messagesLookup: {
[messageId]: {
...getDefaultMessage(messageId),
received_at: time,
},
},
messagesByConversation: {
[conversationId]: {
...getDefaultConversationMessage(),
messageIds: [],
metrics: {
newest: undefined,
totalUnread: 0,
},
},
},
};
const actual = reducer(state, action);
assert.deepEqual(actual, expected);
});
it('returns state if conversation not present', () => {
const action = repairNewestMessage(conversationId);
const state: ConversationsStateType = getDefaultState();
const actual = reducer(state, action);
assert.equal(actual, state);
});
});
describe('REPAIR_OLDEST_MESSAGE', () => {
it('updates oldest', () => {
const action = repairOldestMessage(conversationId);
const state: ConversationsStateType = {
...getDefaultState(),
messagesLookup: {
[messageId]: {
...getDefaultMessage(messageId),
received_at: time,
},
},
messagesByConversation: {
[conversationId]: {
...getDefaultConversationMessage(),
messageIds: [messageId, messageIdTwo, messageIdThree],
metrics: {
totalUnread: 0,
},
},
},
};
const expected: ConversationsStateType = {
...getDefaultState(),
messagesLookup: {
[messageId]: {
...getDefaultMessage(messageId),
received_at: time,
},
},
messagesByConversation: {
[conversationId]: {
...getDefaultConversationMessage(),
messageIds: [messageId, messageIdTwo, messageIdThree],
metrics: {
totalUnread: 0,
oldest: {
id: messageId,
received_at: time,
},
},
},
},
};
const actual = reducer(state, action);
assert.deepEqual(actual, expected);
});
it('clears oldest', () => {
const action = repairOldestMessage(conversationId);
const state: ConversationsStateType = {
...getDefaultState(),
messagesLookup: {
[messageId]: {
...getDefaultMessage(messageId),
received_at: time,
},
},
messagesByConversation: {
[conversationId]: {
...getDefaultConversationMessage(),
messageIds: [],
metrics: {
totalUnread: 0,
oldest: {
id: messageId,
received_at: time,
},
},
},
},
};
const expected: ConversationsStateType = {
...getDefaultState(),
messagesLookup: {
[messageId]: {
...getDefaultMessage(messageId),
received_at: time,
},
},
messagesByConversation: {
[conversationId]: {
...getDefaultConversationMessage(),
messageIds: [],
metrics: {
oldest: undefined,
totalUnread: 0,
},
},
},
};
const actual = reducer(state, action);
assert.deepEqual(actual, expected);
});
it('returns state if conversation not present', () => {
const action = repairOldestMessage(conversationId);
const state: ConversationsStateType = getDefaultState();
const actual = reducer(state, action);
assert.equal(actual, state);
});
});
});
});

View file

@ -2,7 +2,6 @@
// SPDX-License-Identifier: AGPL-3.0-only
import { assert } from 'chai';
import * as sinon from 'sinon';
import { ConversationLookupType } from '../../../state/ducks/conversations';
import {
@ -10,28 +9,7 @@ import {
_getLeftPaneLists,
} from '../../../state/selectors/conversations';
describe('state/selectors/conversations', () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const globalAsAny = global as any;
beforeEach(function beforeEach() {
this.oldWindow = globalAsAny.window;
globalAsAny.window = {};
window.storage = {
get: sinon.stub().returns([]),
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any;
});
afterEach(function afterEach() {
if (this.oldWindow === undefined) {
delete globalAsAny.window;
} else {
globalAsAny.window = this.oldWindow;
}
});
describe('both/state/selectors/conversations', () => {
describe('#getLeftPaneList', () => {
it('sorts conversations based on timestamp then by intl-friendly title', () => {
const data: ConversationLookupType = {
@ -172,14 +150,6 @@ describe('state/selectors/conversations', () => {
});
describe('given pinned conversations', () => {
beforeEach(() => {
(window.storage.get as sinon.SinonStub).returns([
'pin1',
'pin2',
'pin3',
]);
});
it('sorts pinned conversations based on order in storage', () => {
const data: ConversationLookupType = {
pin2: {
@ -262,8 +232,14 @@ describe('state/selectors/conversations', () => {
},
};
const pinnedConversationIds = ['pin1', 'pin2', 'pin3'];
const comparator = _getConversationComparator();
const { pinnedConversations } = _getLeftPaneLists(data, comparator);
const { pinnedConversations } = _getLeftPaneLists(
data,
comparator,
undefined,
pinnedConversationIds
);
assert.strictEqual(pinnedConversations[0].name, 'Pin One');
assert.strictEqual(pinnedConversations[1].name, 'Pin Two');

View file

@ -0,0 +1,40 @@
// Copyright 2020 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
import { assert } from 'chai';
import { getPinnedConversationIds } from '../../../state/selectors/items';
import type { StateType } from '../../../state/reducer';
describe('both/state/selectors/items', () => {
describe('#getPinnedConversationIds', () => {
// Note: we would like to use the full reducer here, to get a real empty state object
// but we cannot load the full reducer inside of electron-mocha.
function getDefaultStateType(): StateType {
return {
items: {},
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any;
}
it('returns pinnedConversationIds key from items', () => {
const expected = ['one', 'two'];
const state: StateType = {
...getDefaultStateType(),
items: {
pinnedConversationIds: expected,
},
};
const actual = getPinnedConversationIds(state);
assert.deepEqual(actual, expected);
});
it('returns empty array if no saved data', () => {
const expected: Array<string> = [];
const state = getDefaultStateType();
const actual = getPinnedConversationIds(state);
assert.deepEqual(actual, expected);
});
});
});

View file

@ -1,118 +0,0 @@
// Copyright 2020 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
import { assert } from 'chai';
import {
getConversationCallMode,
ConversationType,
} from '../../../state/ducks/conversations';
import { CallMode } from '../../../types/Calling';
describe('conversations duck', () => {
describe('helpers', () => {
describe('getConversationCallMode', () => {
const fakeConversation: ConversationType = {
id: 'id1',
e164: '+18005551111',
activeAt: Date.now(),
name: 'No timestamp',
timestamp: 0,
inboxPosition: 0,
phoneNumber: 'notused',
isArchived: false,
markedUnread: false,
type: 'direct',
isMe: false,
lastUpdated: Date.now(),
title: 'No timestamp',
unreadCount: 1,
isSelected: false,
typingContact: {
name: 'Someone There',
color: 'blue',
phoneNumber: '+18005551111',
},
acceptedMessageRequest: true,
};
it("returns CallMode.None if you've left the conversation", () => {
assert.strictEqual(
getConversationCallMode({
...fakeConversation,
left: true,
}),
CallMode.None
);
});
it("returns CallMode.None if you've blocked the other person", () => {
assert.strictEqual(
getConversationCallMode({
...fakeConversation,
isBlocked: true,
}),
CallMode.None
);
});
it("returns CallMode.None if you haven't accepted message requests", () => {
assert.strictEqual(
getConversationCallMode({
...fakeConversation,
acceptedMessageRequest: false,
}),
CallMode.None
);
});
it('returns CallMode.None if the conversation is Note to Self', () => {
assert.strictEqual(
getConversationCallMode({
...fakeConversation,
isMe: true,
}),
CallMode.None
);
});
it('returns CallMode.None for v1 groups', () => {
assert.strictEqual(
getConversationCallMode({
...fakeConversation,
type: 'group',
groupVersion: 1,
}),
CallMode.None
);
assert.strictEqual(
getConversationCallMode({
...fakeConversation,
type: 'group',
}),
CallMode.None
);
});
it('returns CallMode.Direct if the conversation is a normal direct conversation', () => {
assert.strictEqual(
getConversationCallMode(fakeConversation),
CallMode.Direct
);
});
it('returns CallMode.Group if the conversation is a v2 group', () => {
assert.strictEqual(
getConversationCallMode({
...fakeConversation,
type: 'group',
groupVersion: 2,
}),
CallMode.Group
);
});
});
});
});

View file

@ -15007,7 +15007,7 @@
},
{
"rule": "jQuery-before(",
"path": "ts/test/util/windowsZoneIdentifier_test.js",
"path": "ts/test-node/util/windowsZoneIdentifier_test.js",
"line": " before(function thisNeeded() {",
"lineNumber": 21,
"reasonCategory": "testCode",
@ -15016,7 +15016,7 @@
},
{
"rule": "jQuery-before(",
"path": "ts/test/util/windowsZoneIdentifier_test.ts",
"path": "ts/test-node/util/windowsZoneIdentifier_test.ts",
"line": " before(function thisNeeded() {",
"lineNumber": 15,
"reasonCategory": "testCode",
@ -15167,4 +15167,4 @@
"reasonCategory": "falseMatch",
"updated": "2020-09-08T23:07:22.682Z"
}
]
]

View file

@ -823,6 +823,7 @@ Whisper.ConversationView = Whisper.View.extend({
const {
messagesAdded,
setMessagesLoading,
repairOldestMessage,
} = window.reduxActions.conversations;
const conversationId = this.model.id;
@ -851,6 +852,7 @@ Whisper.ConversationView = Whisper.View.extend({
window.log.warn(
'loadOlderMessages: requested, but loaded no messages'
);
repairOldestMessage(conversationId);
return;
}
@ -875,6 +877,7 @@ Whisper.ConversationView = Whisper.View.extend({
const {
messagesAdded,
setMessagesLoading,
repairNewestMessage,
} = window.reduxActions.conversations;
const conversationId = this.model.id;
@ -902,6 +905,7 @@ Whisper.ConversationView = Whisper.View.extend({
window.log.warn(
'loadNewerMessages: requested, but loaded no messages'
);
repairNewestMessage(conversationId);
return;
}