From 1d3859a4e2f9b49e78b1110fdc1bb4c5049a5cba Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Sat, 14 Dec 2019 12:04:36 -0800 Subject: [PATCH] fix: fix duplicates in threads (#1672) fixes #943 --- src/routes/_actions/timeline.js | 5 +- src/routes/_components/list/List.html | 10 ++++ tests/spec/122-replies-in-thread.js | 80 ++++++++++++++++++++++++++- tests/utils.js | 21 +++++++ 4 files changed, 113 insertions(+), 3 deletions(-) diff --git a/src/routes/_actions/timeline.js b/src/routes/_actions/timeline.js index 5d4237ef..4722e234 100644 --- a/src/routes/_actions/timeline.js +++ b/src/routes/_actions/timeline.js @@ -10,6 +10,9 @@ import { getStatus, getStatusContext } from '../_api/statuses' import { emit } from '../_utils/eventBus' import { TIMELINE_BATCH_SIZE } from '../_static/timelines' import { timelineItemToSummary } from '../_utils/timelineItemToSummary' +import uniqBy from 'lodash-es/uniqBy' + +const byId = _ => _.id async function storeFreshTimelineItemsInDatabase (instanceName, timelineName, items) { await database.insertTimelineItems(instanceName, timelineName, items) @@ -70,7 +73,7 @@ export async function addTimelineItemSummaries (instanceName, timelineName, newS const oldSummaries = store.getForTimeline(instanceName, timelineName, 'timelineItemSummaries') || [] const oldStale = store.getForTimeline(instanceName, timelineName, 'timelineItemSummariesAreStale') - const mergedSummaries = mergeArrays(oldSummaries, newSummaries, compareTimelineItemSummaries) + const mergedSummaries = uniqBy(mergeArrays(oldSummaries, newSummaries, compareTimelineItemSummaries), byId) if (!isEqual(oldSummaries, mergedSummaries)) { store.setForTimeline(instanceName, timelineName, { timelineItemSummaries: mergedSummaries }) diff --git a/src/routes/_components/list/List.html b/src/routes/_components/list/List.html index 1e4eff47..e423917f 100644 --- a/src/routes/_components/list/List.html +++ b/src/routes/_components/list/List.html @@ -19,6 +19,7 @@ import ListLazyItem from './ListLazyItem.html' import { listStore } from './listStore' import { getScrollContainer } from '../../_utils/scrollContainer' + import { observe } from 'svelte-extras' function getScrollTopOffset () { const style = getComputedStyle(document.documentElement) @@ -30,11 +31,20 @@ oncreate () { const { realm } = this.get() this.store.setCurrentRealm(realm) + + if (process.env.NODE_ENV !== 'production') { + this.observe('safeItems', safeItems => { + if (new Set(safeItems).size !== safeItems.length) { + console.error('list of items is not unique:', safeItems) + } + }) + } }, ondestroy () { this.store.setCurrentRealm(null) }, methods: { + observe, itemInitialized () { let { initializedCount, length } = this.get() initializedCount++ diff --git a/tests/spec/122-replies-in-thread.js b/tests/spec/122-replies-in-thread.js index fbac9064..2fd1dd02 100644 --- a/tests/spec/122-replies-in-thread.js +++ b/tests/spec/122-replies-in-thread.js @@ -3,9 +3,17 @@ import { getNthStatus, getNthStatusContent, getUrl, - getNthReplyButton, getNthComposeReplyInput, sleep, + getNthReplyButton, + getNthComposeReplyInput, + sleep, getAriaSetSize, - getNthComposeReplyButton, goBack, goForward + getNthComposeReplyButton, + goBack, + goForward, + composeInput, + composeButton, + getNthStatusId, + getStatusContents } from '../utils' import { postAs, postReplyAs } from '../serverActions' @@ -106,3 +114,71 @@ test('reply to non-focused grandchild status in a thread', async t => { .click(getNthComposeReplyButton(3)) await verifyAriaSetSize(t, '4') }) + +async function replyToNthStatus (t, n, replyText, expectedN) { + await t + .click(getNthReplyButton(n)) + .typeText(getNthComposeReplyInput(n), replyText, { paste: true }) + .click(getNthComposeReplyButton(n)) + .expect(getNthStatusContent(expectedN).innerText).contains(replyText) +} + +test('no duplicates in threads', async t => { + await loginAsFoobar(t) + await t + .hover(composeInput) + .typeText(composeInput, 'this is my thread 1', { paste: true }) + .click(composeButton) + .expect(getNthStatusContent(1).innerText).contains('this is my thread 1') + .click(getNthStatus(1)) + .expect(getUrl()).contains('status') + await replyToNthStatus(t, 1, 'this is my thread 2', 2) + const [id1, id2] = await Promise.all([getNthStatusId(1)(), getNthStatusId(2)()]) + await t + .click(getNthStatus(2)) + .expect(getUrl()).contains(id2) + await replyToNthStatus(t, 2, 'this is my thread 3', 3) + const id3 = await getNthStatusId(3)() + await postReplyAs('quux', 'hey i am replying to 1', id1) + await postReplyAs('admin', 'hey i am replying to 3', id3) + await t + .expect(getNthStatusContent(4).innerText).contains('hey i am replying to 3', { timeout: 20000 }) + const idReplyTo3 = await getNthStatusId(4)() + await t + .click(getNthStatus(4)) + .expect(getUrl()).contains(idReplyTo3) + await postReplyAs('quux', 'hey check this reply', id1) + await t + .click(getNthStatus(2)) + .expect(getUrl()).contains(id2) + .click(getNthStatus(3)) + .expect(getUrl()).contains(id3) + await replyToNthStatus(t, 3, 'this is my thread 4', 5) + await replyToNthStatus(t, 5, 'this is my thread 5', 6) + const id5 = await getNthStatusId(6)() + await postReplyAs('admin', 'hey i am replying to 1 again', id1) + await t + .click(getNthStatus(6)) + .expect(getUrl()).contains(id5) + .click(getNthStatus(1)) + .expect(getUrl()).contains(id1) + + await t + .click(getNthStatus(6)) + .expect(getUrl()).contains(id5) + await replyToNthStatus(t, 5, 'this is my thread 6', 6) + await t + .click(getNthStatus(1)) + .expect(getUrl()).contains(id1) + + const contents = await getStatusContents() + const contentsToCounts = new Map() + for (const content of contents) { + contentsToCounts.set(content, 1 + (contentsToCounts.get(content) || 0)) + } + const duplicates = [...contentsToCounts.entries()].filter(arr => arr[1] > 1).map(arr => arr[0]) + + // there should be no duplicates + await t + .expect(duplicates).eql([]) +}) diff --git a/tests/utils.js b/tests/utils.js index fa822185..2081eb6a 100644 --- a/tests/utils.js +++ b/tests/utils.js @@ -167,6 +167,27 @@ export const getActiveElementInsideNthStatus = exec(() => { return '' }) +export const getNthStatusId = n => exec(() => { + return document.querySelector(getNthStatusSelector(n)) + .getAttribute('id') + .split('/') + .slice(-1)[0] +}, { + dependencies: { + getNthStatusSelector, + n + } +}) + +export const getStatusContents = exec(() => { + const res = [] + const elements = document.querySelectorAll('.list-item > article .status-content') + for (let i = 0; i < elements.length; i++) { + res.push(elements[i].innerText) + } + return res +}) + export const getTitleText = exec(() => document.head.querySelector('title') && document.head.querySelector('title').innerHTML) export const goBack = exec(() => window.history.back())