diff --git a/app/selectors/post_list.js b/app/selectors/post_list.js index f98cc3b4c..2d2730a23 100644 --- a/app/selectors/post_list.js +++ b/app/selectors/post_list.js @@ -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(); } ); } diff --git a/test/app/selectors/post_list.test.js b/test/app/selectors/post_list.test.js index 01b527513..ae047e836 100644 --- a/test/app/selectors/post_list.test.js +++ b/test/app/selectors/post_list.test.js @@ -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)); +}