Fix new message indicator (#1053)

* Fix new message indicator

* Revert "Fix new message indicator"

This reverts commit 3732d2e53149f2e29b671cb6fd68e359d64aab36.

* Fixed new messages indicator and added unit tests for it
This commit is contained in:
enahum 2017-10-24 19:42:50 -03:00 committed by GitHub
parent f50b1c6ae9
commit c8a0753289
2 changed files with 76 additions and 93 deletions

View file

@ -28,6 +28,10 @@ export function makePreparePostIdsForPostList() {
getCurrentUserId,
shouldShowJoinLeaveMessages,
(posts, lastViewedAt, currentUserId, showJoinLeave) => {
if (posts.length === 0) {
return [];
}
const out = [];
let lastDate = null;
@ -35,8 +39,10 @@ export function makePreparePostIdsForPostList() {
const filterOptions = {showJoinLeave};
// Remember that we're iterating through the posts from newest to oldest
for (const post of posts) {
// Iterating through the posts from oldest to newest
for (let i = posts.length - 1; i >= 0; i--) {
const post = posts[i];
if (post.state === Posts.POST_DELETED && post.user_id === currentUserId) {
continue;
}
@ -46,34 +52,27 @@ export function makePreparePostIdsForPostList() {
continue;
}
// Only add the new messages line if a lastViewedAt time is set
if (lastViewedAt && !addedNewMessagesIndicator) {
const postIsUnread = post.create_at > lastViewedAt;
if (postIsUnread) {
out.push(START_OF_NEW_MESSAGES);
addedNewMessagesIndicator = true;
}
}
// Push on a date header if the last post was on a different day than the current one
const postDate = new Date(post.create_at);
if (lastDate && lastDate.toDateString() !== postDate.toDateString()) {
out.push(DATE_LINE + lastDate.toDateString());
if (!lastDate || lastDate.toDateString() !== postDate.toDateString()) {
out.push(DATE_LINE + postDate.toDateString());
lastDate = postDate;
}
lastDate = postDate;
// Only add the new messages line if a lastViewedAt time is set
const postIsUnread = post.create_at > lastViewedAt;
if (lastViewedAt != null && !addedNewMessagesIndicator && postIsUnread) {
out.push(START_OF_NEW_MESSAGES);
addedNewMessagesIndicator = true;
}
out.push(post.id);
}
// Push on the date header for the oldest post
if (lastDate) {
out.push(DATE_LINE + lastDate.toDateString());
}
return out;
// Flip it back to newest to oldest
return out.reverse();
}
);
}

View file

@ -34,12 +34,11 @@ describe('Selectors.PostList', () => {
}
};
const lastViewedAt = Number.POSITIVE_INFINITY;
const postIds = ['1001', '1002'];
const postIds = ['1002', '1001'];
// Defaults to show post
let now = preparePostIdsForPostList(state, {postIds, lastViewedAt});
assert.ok(now.indexOf('1001') !== -1);
assert.ok(now.indexOf('1002') !== -1);
assert.deepEqual(removeDateLines(now), ['1002', '1001']);
// Show join/leave posts
state = {
@ -61,8 +60,7 @@ describe('Selectors.PostList', () => {
};
now = preparePostIdsForPostList(state, {postIds, lastViewedAt});
assert.ok(now.indexOf('1001') !== -1);
assert.ok(now.indexOf('1002') !== -1);
assert.deepEqual(removeDateLines(now), ['1002', '1001']);
// Hide join/leave posts
state = {
@ -84,8 +82,43 @@ describe('Selectors.PostList', () => {
};
now = preparePostIdsForPostList(state, {postIds, lastViewedAt});
assert.ok(now.indexOf('1001') !== -1);
assert.ok(now.indexOf('1002') === -1);
assert.deepEqual(removeDateLines(now), ['1001']);
});
it('new messages indicator', () => {
const preparePostIdsForPostList = makePreparePostIdsForPostList();
const state = {
entities: {
posts: {
posts: {
1000: {id: '1000', create_at: 1000},
1005: {id: '1005', create_at: 1005},
1010: {id: '1010', create_at: 1010}
}
},
preferences: {
myPreferences: {}
},
users: {
currentUserId: '1234'
}
}
};
const postIds = ['1010', '1005', '1000']; // Remember that we list the posts backwards
// Show new messages indicator before all posts
let now = preparePostIdsForPostList(state, {postIds, lastViewedAt: 0});
assert.deepEqual(removeDateLines(now), ['1010', '1005', '1000', START_OF_NEW_MESSAGES]);
// Show indicator between posts
now = preparePostIdsForPostList(state, {postIds, lastViewedAt: 1003});
assert.deepEqual(removeDateLines(now), ['1010', '1005', START_OF_NEW_MESSAGES, '1000']);
// Don't show indicator when all posts are read
now = preparePostIdsForPostList(state, {postIds, lastViewedAt: 1020});
assert.deepEqual(removeDateLines(now), ['1010', '1005', '1000']);
});
it('memoization', () => {
@ -120,31 +153,17 @@ describe('Selectors.PostList', () => {
}
};
let postIds = ['1001', '1003', '1004', '1006'];
let postIds = ['1006', '1004', '1003', '1001'];
let lastViewedAt = initialPosts['1001'].create_at + 1;
let now = preparePostIdsForPostList(state, {postIds, lastViewedAt});
assert.ok(now.indexOf('1001') !== -1);
assert.ok(now.indexOf('1002') === -1);
assert.ok(now.indexOf('1003') !== -1);
assert.ok(now.indexOf('1004') !== -1);
assert.ok(now.indexOf('1005') === -1);
assert.ok(now.indexOf('1006') !== -1);
assert.ok(now.findIndex((id) => id.startsWith(DATE_LINE)) !== -1);
assert.ok(now.indexOf(START_OF_NEW_MESSAGES) !== -1);
assert.deepEqual(removeDateLines(now), ['1006', '1004', '1003', START_OF_NEW_MESSAGES, '1001']);
// No changes
let prev = now;
now = preparePostIdsForPostList(state, {postIds, lastViewedAt});
assert.equal(now, prev);
assert.ok(now.indexOf('1001') !== -1);
assert.ok(now.indexOf('1002') === -1);
assert.ok(now.indexOf('1003') !== -1);
assert.ok(now.indexOf('1004') !== -1);
assert.ok(now.indexOf('1005') === -1);
assert.ok(now.indexOf('1006') !== -1);
assert.ok(now.findIndex((id) => id.startsWith(DATE_LINE)) !== -1);
assert.ok(now.indexOf(START_OF_NEW_MESSAGES) !== -1);
assert.deepEqual(removeDateLines(now), ['1006', '1004', '1003', START_OF_NEW_MESSAGES, '1001']);
// lastViewedAt changed slightly
lastViewedAt = initialPosts['1001'].create_at + 2;
@ -152,14 +171,7 @@ describe('Selectors.PostList', () => {
prev = now;
now = preparePostIdsForPostList(state, {postIds, lastViewedAt});
assert.equal(now, prev);
assert.ok(now.indexOf('1001') !== -1);
assert.ok(now.indexOf('1002') === -1);
assert.ok(now.indexOf('1003') !== -1);
assert.ok(now.indexOf('1004') !== -1);
assert.ok(now.indexOf('1005') === -1);
assert.ok(now.indexOf('1006') !== -1);
assert.ok(now.findIndex((id) => id.startsWith(DATE_LINE)) !== -1);
assert.ok(now.indexOf(START_OF_NEW_MESSAGES) !== -1);
assert.deepEqual(removeDateLines(now), ['1006', '1004', '1003', START_OF_NEW_MESSAGES, '1001']);
// lastViewedAt changed a lot
lastViewedAt += initialPosts['1003'].create_at + 1;
@ -167,18 +179,12 @@ describe('Selectors.PostList', () => {
prev = now;
now = preparePostIdsForPostList(state, {postIds, lastViewedAt});
assert.notEqual(now, prev);
assert.ok(now.indexOf('1001') !== -1);
assert.ok(now.indexOf('1002') === -1);
assert.ok(now.indexOf('1003') !== -1);
assert.ok(now.indexOf('1004') !== -1);
assert.ok(now.indexOf('1005') === -1);
assert.ok(now.indexOf('1006') !== -1);
assert.ok(now.findIndex((id) => id.startsWith(DATE_LINE)) !== -1);
assert.ok(now.indexOf(START_OF_NEW_MESSAGES) !== -1);
assert.deepEqual(removeDateLines(now), ['1006', '1004', START_OF_NEW_MESSAGES, '1003', '1001']);
prev = now;
now = preparePostIdsForPostList(state, {postIds, lastViewedAt});
assert.equal(now, prev);
assert.deepEqual(removeDateLines(now), ['1006', '1004', START_OF_NEW_MESSAGES, '1003', '1001']);
// postIds changed, but still shallowly equal
postIds = [...postIds];
@ -186,14 +192,7 @@ describe('Selectors.PostList', () => {
prev = now;
now = preparePostIdsForPostList(state, {postIds, lastViewedAt});
assert.equal(now, prev);
assert.ok(now.indexOf('1001') !== -1);
assert.ok(now.indexOf('1002') === -1);
assert.ok(now.indexOf('1003') !== -1);
assert.ok(now.indexOf('1004') !== -1);
assert.ok(now.indexOf('1005') === -1);
assert.ok(now.indexOf('1006') !== -1);
assert.ok(now.findIndex((id) => id.startsWith(DATE_LINE)) !== -1);
assert.ok(now.indexOf(START_OF_NEW_MESSAGES) !== -1);
assert.deepEqual(removeDateLines(now), ['1006', '1004', START_OF_NEW_MESSAGES, '1003', '1001']);
// Post changed, not in postIds
state = {
@ -213,14 +212,7 @@ describe('Selectors.PostList', () => {
prev = now;
now = preparePostIdsForPostList(state, {postIds, lastViewedAt});
assert.equal(now, prev);
assert.ok(now.indexOf('1001') !== -1);
assert.ok(now.indexOf('1002') === -1);
assert.ok(now.indexOf('1003') !== -1);
assert.ok(now.indexOf('1004') !== -1);
assert.ok(now.indexOf('1005') === -1);
assert.ok(now.indexOf('1006') !== -1);
assert.ok(now.findIndex((id) => id.startsWith(DATE_LINE)) !== -1);
assert.ok(now.indexOf(START_OF_NEW_MESSAGES) !== -1);
assert.deepEqual(removeDateLines(now), ['1006', '1004', START_OF_NEW_MESSAGES, '1003', '1001']);
// Post changed, in postIds
state = {
@ -240,14 +232,7 @@ describe('Selectors.PostList', () => {
prev = now;
now = preparePostIdsForPostList(state, {postIds, lastViewedAt});
assert.equal(now, prev);
assert.ok(now.indexOf('1001') !== -1);
assert.ok(now.indexOf('1002') === -1);
assert.ok(now.indexOf('1003') !== -1);
assert.ok(now.indexOf('1004') !== -1);
assert.ok(now.indexOf('1005') === -1);
assert.ok(now.indexOf('1006') !== -1);
assert.ok(now.findIndex((id) => id.startsWith(DATE_LINE)) !== -1);
assert.ok(now.indexOf(START_OF_NEW_MESSAGES) !== -1);
assert.deepEqual(removeDateLines(now), ['1006', '1004', START_OF_NEW_MESSAGES, '1003', '1001']);
// Filter changed
state = {
@ -271,18 +256,17 @@ describe('Selectors.PostList', () => {
prev = now;
now = preparePostIdsForPostList(state, {postIds, lastViewedAt});
assert.notEqual(now, prev);
assert.ok(now.indexOf('1001') !== -1);
assert.ok(now.indexOf('1002') === -1);
assert.ok(now.indexOf('1003') !== -1);
assert.ok(now.indexOf('1004') !== -1);
assert.ok(now.indexOf('1005') === -1);
assert.ok(now.indexOf('1006') === -1);
assert.ok(now.findIndex((id) => id.startsWith(DATE_LINE)) !== -1);
assert.ok(now.indexOf(START_OF_NEW_MESSAGES) !== -1);
assert.deepEqual(removeDateLines(now), ['1004', START_OF_NEW_MESSAGES, '1003', '1001']);
prev = now;
now = preparePostIdsForPostList(state, {postIds, lastViewedAt});
assert.equal(now, prev);
assert.deepEqual(removeDateLines(now), ['1004', START_OF_NEW_MESSAGES, '1003', '1001']);
});
});
});
// Remove date lines when checking equality since those depend on time-zone of the computer running the tests
function removeDateLines(list) {
return list.filter((item) => !item.startsWith(DATE_LINE));
}