From 5e348dc14927737fc1fceed4e66537c715997126 Mon Sep 17 00:00:00 2001 From: Roger Braun Date: Sun, 13 Nov 2016 17:59:35 +0100 Subject: [PATCH 1/9] Add RT status adding specs. --- test/unit/specs/modules/statuses.spec.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/unit/specs/modules/statuses.spec.js b/test/unit/specs/modules/statuses.spec.js index fe8ed0b8ce..27936fb1db 100644 --- a/test/unit/specs/modules/statuses.spec.js +++ b/test/unit/specs/modules/statuses.spec.js @@ -70,6 +70,25 @@ describe('The Statuses module', () => { expect(state.allStatuses[0]).to.equal(modStatus) }) + it('replaces existing statuses with the same id, coming from a retweet', () => { + const state = cloneDeep(defaultState) + const status = makeMockStatus({id: 1}) + const modStatus = makeMockStatus({id: 1, text: 'something else'}) + const retweet = makeMockStatus({id: 2}) + retweet.retweeted_status = modStatus + + // Add original status + mutations.addNewStatuses(state, { statuses: [status], showImmediately: true, timeline: 'public' }) + expect(state.timelines.public.visibleStatuses).to.have.length(1) + expect(state.allStatuses).to.have.length(1) + + // Add new version of status + mutations.addNewStatuses(state, { statuses: [retweet], showImmediately: false, timeline: 'public' }) + expect(state.timelines.public.visibleStatuses).to.have.length(1) + expect(state.allStatuses).to.have.length(2) + expect(state.allStatuses[0]).to.equal(modStatus) + }) + it('handles favorite actions', () => { const state = cloneDeep(defaultState) const status = makeMockStatus({id: 1}) From 5888697c0dccb02eb3fdefba28bc359c32ec264e Mon Sep 17 00:00:00 2001 From: Roger Braun Date: Sun, 13 Nov 2016 22:09:27 +0100 Subject: [PATCH 2/9] Better maxId calculation. --- src/modules/statuses.js | 16 +++++++++++++--- test/unit/specs/modules/statuses.spec.js | 20 ++++++++++++++++++-- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/src/modules/statuses.js b/src/modules/statuses.js index 8238644d41..6fe2558c8c 100644 --- a/src/modules/statuses.js +++ b/src/modules/statuses.js @@ -1,4 +1,4 @@ -import { reduce, map, slice, last, intersectionBy, sortBy, unionBy, toInteger, groupBy, differenceBy, each, find } from 'lodash' +import { reduce, map, slice, last, intersectionBy, sortBy, unionBy, toInteger, groupBy, differenceBy, each, find, flatten, maxBy } from 'lodash' import moment from 'moment' import apiService from '../services/api/api.service.js' import parse from '../services/status_parser/status_parser.js' @@ -41,7 +41,7 @@ const statusType = (status) => { return !status.is_post_verb && status.uri.match(/fave/) ? 'fave' : 'status' } -const addStatusesToTimeline = (addedStatuses, showImmediately, { statuses, visibleStatuses, newStatusCount, faves, loading }) => { +const addStatusesToTimeline = (addedStatuses, showImmediately, { statuses, visibleStatuses, newStatusCount, faves, loading, maxId }) => { const statusesAndFaves = groupBy(addedStatuses, statusType) const addedFaves = statusesAndFaves['fave'] || [] const unseenFaves = differenceBy(addedFaves, faves, 'id') @@ -92,9 +92,9 @@ const addStatusesToTimeline = (addedStatuses, showImmediately, { statuses, visib statuses: newStatuses, visibleStatuses: newVisibleStatuses, newStatusCount: newNewStatusCount, - maxId: newStatuses[0].id, minVisibleId: (last(newVisibleStatuses) || { id: undefined }).id, faves: unionBy(faves, addedFaves, 'id'), + maxId, loading } } @@ -109,8 +109,18 @@ const updateTimestampsInStatuses = (statuses) => { }) } + +export const findMaxId = (...args) => { + return (maxBy(flatten(args), 'id') || {}).id +} + export const mutations = { addNewStatuses (state, { statuses, showImmediately = false, timeline }) { + const timelineObject = state.timelines[timeline] + + // Set new maxId + const maxId = findMaxId(statuses, timelineObject.statuses) + timelineObject.maxId = maxId state.timelines[timeline] = addStatusesToTimeline(statuses, showImmediately, state.timelines[timeline]) state.allStatuses = unionBy(state.timelines[timeline].statuses, state.allStatuses, 'id') diff --git a/test/unit/specs/modules/statuses.spec.js b/test/unit/specs/modules/statuses.spec.js index 27936fb1db..372ab3ca48 100644 --- a/test/unit/specs/modules/statuses.spec.js +++ b/test/unit/specs/modules/statuses.spec.js @@ -1,5 +1,5 @@ import { cloneDeep } from 'lodash' -import { defaultState, mutations } from '../../../../src/modules/statuses.js' +import { defaultState, mutations, findMaxId } from '../../../../src/modules/statuses.js' const makeMockStatus = ({id, text}) => { return { @@ -11,6 +11,21 @@ const makeMockStatus = ({id, text}) => { } } +describe('findMaxId', () => { + it('returns the largest id in any of the given arrays', () => { + const statusesOne = [{ id: 100 }, { id: 2 }] + const statusesTwo = [{ id: 3 }] + + const maxId = findMaxId(statusesOne, statusesTwo) + expect(maxId).to.eq(100) + }) + + it('returns undefined for empty arrays', () => { + const maxId = findMaxId([], []) + expect(maxId).to.eq(undefined) + }) +}) + describe('The Statuses module', () => { it('adds the status to allStatuses and to the given timeline', () => { const state = cloneDeep(defaultState) @@ -96,7 +111,7 @@ describe('The Statuses module', () => { const favorite = { id: 2, is_post_verb: false, - in_reply_to_status_id: 1, // The API uses strings here... + in_reply_to_status_id: '1', // The API uses strings here... uri: 'tag:shitposter.club,2016-08-21:fave:3895:note:773501:2016-08-21T16:52:15+00:00', text: 'a favorited something by b' } @@ -106,6 +121,7 @@ describe('The Statuses module', () => { expect(state.timelines.public.visibleStatuses.length).to.eql(1) expect(state.timelines.public.visibleStatuses[0].fave_num).to.eql(1) + expect(state.timelines.public.maxId).to.eq(favorite.id) // Adding again shouldn't change anything mutations.addNewStatuses(state, { statuses: [favorite], showImmediately: true, timeline: 'public' }) From 59647798b97615d43710dde8f6c52894f39b8c4a Mon Sep 17 00:00:00 2001 From: Roger Braun Date: Sun, 13 Nov 2016 22:40:33 +0100 Subject: [PATCH 3/9] prepareStatus: nsfw tag parsing. --- src/modules/statuses.js | 9 +++++++++ test/unit/specs/modules/statuses.spec.js | 21 +++++++++++++++++++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/modules/statuses.js b/src/modules/statuses.js index 6fe2558c8c..5e3a373a56 100644 --- a/src/modules/statuses.js +++ b/src/modules/statuses.js @@ -114,6 +114,15 @@ export const findMaxId = (...args) => { return (maxBy(flatten(args), 'id') || {}).id } +export const prepareStatus = (status) => { + if (status.nsfw === undefined) { + const nsfwRegex = /#nsfw/i + status.nsfw = !!status.text.match(nsfwRegex) + } + + return status +} + export const mutations = { addNewStatuses (state, { statuses, showImmediately = false, timeline }) { const timelineObject = state.timelines[timeline] diff --git a/test/unit/specs/modules/statuses.spec.js b/test/unit/specs/modules/statuses.spec.js index 372ab3ca48..1de15e6ab2 100644 --- a/test/unit/specs/modules/statuses.spec.js +++ b/test/unit/specs/modules/statuses.spec.js @@ -1,5 +1,5 @@ import { cloneDeep } from 'lodash' -import { defaultState, mutations, findMaxId } from '../../../../src/modules/statuses.js' +import { defaultState, mutations, findMaxId, prepareStatus } from '../../../../src/modules/statuses.js' const makeMockStatus = ({id, text}) => { return { @@ -11,7 +11,24 @@ const makeMockStatus = ({id, text}) => { } } -describe('findMaxId', () => { +describe('Statuses.prepareStatus', () => { + it('sets nsfw for statuses with the #nsfw tag', () => { + const safe = makeMockStatus({id: 1, text: 'Hello oniichan'}) + const nsfw = makeMockStatus({id: 1, text: 'Hello oniichan #nsfw'}) + + expect(prepareStatus(safe).nsfw).to.eq(false) + expect(prepareStatus(nsfw).nsfw).to.eq(true) + }) + + it('leaves existing nsfw settings alone', () => { + const nsfw = makeMockStatus({id: 1, text: 'Hello oniichan #nsfw'}) + nsfw.nsfw = false + + expect(prepareStatus(nsfw).nsfw).to.eq(false) + }) +}) + +describe('Statuses.findMaxId', () => { it('returns the largest id in any of the given arrays', () => { const statusesOne = [{ id: 100 }, { id: 2 }] const statusesTwo = [{ id: 3 }] From aeb8868b82c767e6c8bf19aa8e6355f71941fea5 Mon Sep 17 00:00:00 2001 From: Roger Braun Date: Sun, 13 Nov 2016 22:54:49 +0100 Subject: [PATCH 4/9] prepareStatus: created_at_parsed. --- src/modules/statuses.js | 4 ++++ test/unit/specs/modules/statuses.spec.js | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/src/modules/statuses.js b/src/modules/statuses.js index 5e3a373a56..86995b0303 100644 --- a/src/modules/statuses.js +++ b/src/modules/statuses.js @@ -115,11 +115,15 @@ export const findMaxId = (...args) => { } export const prepareStatus = (status) => { + // Parse nsfw tags if (status.nsfw === undefined) { const nsfwRegex = /#nsfw/i status.nsfw = !!status.text.match(nsfwRegex) } + // Set created_at_parsed to initial value + status.created_at_parsed = status.created_at + return status } diff --git a/test/unit/specs/modules/statuses.spec.js b/test/unit/specs/modules/statuses.spec.js index 1de15e6ab2..28ecbdfbf2 100644 --- a/test/unit/specs/modules/statuses.spec.js +++ b/test/unit/specs/modules/statuses.spec.js @@ -26,6 +26,15 @@ describe('Statuses.prepareStatus', () => { expect(prepareStatus(nsfw).nsfw).to.eq(false) }) + + it('sets the created_at_parsed property', () => { + const status = makeMockStatus({id: 1}) + status.created_at = '' + expect(status.created_at_parsed).to.eq(undefined) + + const prepared = prepareStatus(status) + expect(prepared.created_at_parsed).to.not.eq(undefined) + }) }) describe('Statuses.findMaxId', () => { From d10a58f26ab9b22fd9dba499a9f097ac177e1cae Mon Sep 17 00:00:00 2001 From: Roger Braun Date: Tue, 15 Nov 2016 10:35:16 +0100 Subject: [PATCH 5/9] Some reducer changes for statuses. --- src/modules/statuses.js | 57 ++++++++++++++++++++++++++--------------- 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/src/modules/statuses.js b/src/modules/statuses.js index 86995b0303..d7607bf1b3 100644 --- a/src/modules/statuses.js +++ b/src/modules/statuses.js @@ -1,7 +1,7 @@ import { reduce, map, slice, last, intersectionBy, sortBy, unionBy, toInteger, groupBy, differenceBy, each, find, flatten, maxBy } from 'lodash' import moment from 'moment' import apiService from '../services/api/api.service.js' -import parse from '../services/status_parser/status_parser.js' +// import parse from '../services/status_parser/status_parser.js' export const defaultState = { allStatuses: [], @@ -41,6 +41,19 @@ const statusType = (status) => { return !status.is_post_verb && status.uri.match(/fave/) ? 'fave' : 'status' } +export const prepareStatus = (status) => { + // Parse nsfw tags + if (status.nsfw === undefined) { + const nsfwRegex = /#nsfw/i + status.nsfw = !!status.text.match(nsfwRegex) + } + + // Set created_at_parsed to initial value + status.created_at_parsed = status.created_at + + return status +} + const addStatusesToTimeline = (addedStatuses, showImmediately, { statuses, visibleStatuses, newStatusCount, faves, loading, maxId }) => { const statusesAndFaves = groupBy(addedStatuses, statusType) const addedFaves = statusesAndFaves['fave'] || [] @@ -60,13 +73,7 @@ const addStatusesToTimeline = (addedStatuses, showImmediately, { statuses, visib addedStatuses = map(addedStatuses, (status) => { const statusoid = status.retweeted_status || status - statusoid.created_at_parsed = statusoid.created_at - statusoid.statusnet_html = parse(statusoid.statusnet_html) - - if (statusoid.nsfw === undefined) { - const nsfwRegex = /#nsfw/i - statusoid.nsfw = statusoid.text.match(nsfwRegex) - } + prepareStatus(statusoid) return status }) @@ -109,24 +116,28 @@ const updateTimestampsInStatuses = (statuses) => { }) } +// const groupStatusesByType = (statuses) => { +// return groupBy(statuses, (status) => { +// if (status.is_post_verb) { +// return 'status' +// } + +// if (status.retweeted_status) { +// return 'retweet' +// } + +// if (typeof status.uri === 'string' && status.uri.match(/fave/)) { +// return 'favorite' +// } + +// return 'unknown' +// }) +// } export const findMaxId = (...args) => { return (maxBy(flatten(args), 'id') || {}).id } -export const prepareStatus = (status) => { - // Parse nsfw tags - if (status.nsfw === undefined) { - const nsfwRegex = /#nsfw/i - status.nsfw = !!status.text.match(nsfwRegex) - } - - // Set created_at_parsed to initial value - status.created_at_parsed = status.created_at - - return status -} - export const mutations = { addNewStatuses (state, { statuses, showImmediately = false, timeline }) { const timelineObject = state.timelines[timeline] @@ -134,6 +145,10 @@ export const mutations = { // Set new maxId const maxId = findMaxId(statuses, timelineObject.statuses) timelineObject.maxId = maxId + + // Split statuses by type + // const statusesByType = groupStatusesByType(statuses) + state.timelines[timeline] = addStatusesToTimeline(statuses, showImmediately, state.timelines[timeline]) state.allStatuses = unionBy(state.timelines[timeline].statuses, state.allStatuses, 'id') From 4fcb60487cbe034d12d42852354c7522033dc23c Mon Sep 17 00:00:00 2001 From: Roger Braun Date: Thu, 17 Nov 2016 18:31:26 +0100 Subject: [PATCH 6/9] Do more mutation-y status reducing. --- src/modules/statuses.js | 17 +++++++++++++++-- test/unit/specs/modules/statuses.spec.js | 4 ++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/modules/statuses.js b/src/modules/statuses.js index d7607bf1b3..7fa8a7b24f 100644 --- a/src/modules/statuses.js +++ b/src/modules/statuses.js @@ -1,4 +1,4 @@ -import { reduce, map, slice, last, intersectionBy, sortBy, unionBy, toInteger, groupBy, differenceBy, each, find, flatten, maxBy } from 'lodash' +import { reduce, map, slice, last, intersectionBy, sortBy, unionBy, toInteger, groupBy, differenceBy, each, find, flatten, maxBy, merge } from 'lodash' import moment from 'moment' import apiService from '../services/api/api.service.js' // import parse from '../services/status_parser/status_parser.js' @@ -54,6 +54,19 @@ export const prepareStatus = (status) => { return status } +// Merges old and new status collections. +const mergeStatuses = (oldStatuses, newStatuses) => { + each(newStatuses, (status) => { + let oldStatus = find(oldStatuses, { id: status.id }) + if (oldStatus) { + merge(oldStatus, status) + } else { + oldStatuses.push(status) + } + }) + return oldStatuses +} + const addStatusesToTimeline = (addedStatuses, showImmediately, { statuses, visibleStatuses, newStatusCount, faves, loading, maxId }) => { const statusesAndFaves = groupBy(addedStatuses, statusType) const addedFaves = statusesAndFaves['fave'] || [] @@ -150,7 +163,7 @@ export const mutations = { // const statusesByType = groupStatusesByType(statuses) state.timelines[timeline] = addStatusesToTimeline(statuses, showImmediately, state.timelines[timeline]) - state.allStatuses = unionBy(state.timelines[timeline].statuses, state.allStatuses, 'id') + mergeStatuses(state.allStatuses, state.timelines[timeline].statuses) // Set up retweets with most current status const getRetweets = (result, status) => { diff --git a/test/unit/specs/modules/statuses.spec.js b/test/unit/specs/modules/statuses.spec.js index 28ecbdfbf2..0201303991 100644 --- a/test/unit/specs/modules/statuses.spec.js +++ b/test/unit/specs/modules/statuses.spec.js @@ -90,7 +90,7 @@ describe('The Statuses module', () => { // It refers to the modified status. mutations.addNewStatuses(state, { statuses: [modStatus], timeline: 'public' }) - expect(state.allStatuses).to.eql([retweet, modStatus]) + expect(state.allStatuses).to.eql([modStatus, retweet]) expect(retweet.retweeted_status).to.eql(modStatus) }) @@ -108,7 +108,7 @@ describe('The Statuses module', () => { mutations.addNewStatuses(state, { statuses: [modStatus], showImmediately: true, timeline: 'public' }) expect(state.timelines.public.visibleStatuses).to.have.length(1) expect(state.allStatuses).to.have.length(1) - expect(state.allStatuses[0]).to.equal(modStatus) + expect(state.allStatuses[0]).to.eql(modStatus) }) it('replaces existing statuses with the same id, coming from a retweet', () => { From 8723c35d4305a4ff5d01dee8707c1cc25b405cd2 Mon Sep 17 00:00:00 2001 From: Roger Braun Date: Fri, 18 Nov 2016 16:05:04 +0100 Subject: [PATCH 7/9] A bit more advanced status merging. --- src/modules/statuses.js | 10 ++++++++-- test/unit/specs/modules/statuses.spec.js | 6 +++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/modules/statuses.js b/src/modules/statuses.js index 7fa8a7b24f..268e64fb39 100644 --- a/src/modules/statuses.js +++ b/src/modules/statuses.js @@ -153,6 +153,11 @@ export const findMaxId = (...args) => { export const mutations = { addNewStatuses (state, { statuses, showImmediately = false, timeline }) { + + // Merge in the new status into the old ones. + mergeStatuses(state.allStatuses, statuses) + + // Get relevant timeline const timelineObject = state.timelines[timeline] // Set new maxId @@ -163,7 +168,6 @@ export const mutations = { // const statusesByType = groupStatusesByType(statuses) state.timelines[timeline] = addStatusesToTimeline(statuses, showImmediately, state.timelines[timeline]) - mergeStatuses(state.allStatuses, state.timelines[timeline].statuses) // Set up retweets with most current status const getRetweets = (result, status) => { @@ -175,7 +179,9 @@ export const mutations = { const retweets = reduce(statuses, getRetweets, []) - state.allStatuses = unionBy(retweets, state.allStatuses, 'id') + // state.allStatuses = unionBy(retweets, state.allStatuses, 'id') + + mergeStatuses(state.allStatuses, retweets) each(state.allStatuses, (status) => { if (status.retweeted_status) { diff --git a/test/unit/specs/modules/statuses.spec.js b/test/unit/specs/modules/statuses.spec.js index 0201303991..0370d4aecc 100644 --- a/test/unit/specs/modules/statuses.spec.js +++ b/test/unit/specs/modules/statuses.spec.js @@ -86,11 +86,11 @@ describe('The Statuses module', () => { // It adds both statuses, but only the retweet to visible. mutations.addNewStatuses(state, { statuses: [retweet], timeline: 'public', showImmediately: true }) expect(state.timelines.public.visibleStatuses).to.have.length(1) - expect(state.allStatuses).to.eql([status, retweet]) + expect(state.allStatuses).to.eql([retweet, status]) // It refers to the modified status. mutations.addNewStatuses(state, { statuses: [modStatus], timeline: 'public' }) - expect(state.allStatuses).to.eql([modStatus, retweet]) + expect(state.allStatuses).to.eql([retweet, modStatus]) expect(retweet.retweeted_status).to.eql(modStatus) }) @@ -127,7 +127,7 @@ describe('The Statuses module', () => { mutations.addNewStatuses(state, { statuses: [retweet], showImmediately: false, timeline: 'public' }) expect(state.timelines.public.visibleStatuses).to.have.length(1) expect(state.allStatuses).to.have.length(2) - expect(state.allStatuses[0]).to.equal(modStatus) + expect(state.allStatuses[0]).to.eql(modStatus) }) it('handles favorite actions', () => { From 05afbbaf66ee10b751cf5f250835eaa14de6d7ca Mon Sep 17 00:00:00 2001 From: Roger Braun Date: Fri, 18 Nov 2016 19:47:47 +0100 Subject: [PATCH 8/9] Better diffs for specs. --- package.json | 2 ++ test/unit/karma.conf.js | 5 ++++- yarn.lock | 12 +++++++++++- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 66134807ad..2388ade815 100644 --- a/package.json +++ b/package.json @@ -13,6 +13,8 @@ "lint": "eslint --ext .js,.vue src test/unit/specs test/e2e/specs" }, "dependencies": { + "diff": "^3.0.1", + "karma-mocha-reporter": "^2.2.1", "moment": "^2.15.2", "node-sass": "^3.10.1", "sanitize-html": "^1.13.0", diff --git a/test/unit/karma.conf.js b/test/unit/karma.conf.js index ce413a43e2..78910f426f 100644 --- a/test/unit/karma.conf.js +++ b/test/unit/karma.conf.js @@ -55,7 +55,7 @@ module.exports = function (config) { // 2. add it to the `browsers` array below. browsers: ['PhantomJS'], frameworks: ['mocha', 'sinon-chai'], - reporters: ['spec'], + reporters: ['mocha'], files: ['./index.js'], preprocessors: { './index.js': ['webpack', 'sourcemap'] @@ -64,6 +64,9 @@ module.exports = function (config) { webpackMiddleware: { noInfo: true }, + mochaReporter: { + showDiff: true + }, coverageReporter: { dir: './coverage', reporters: [ diff --git a/yarn.lock b/yarn.lock index 189019c667..dad4e2205c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1061,7 +1061,7 @@ chai@^3.5.0: deep-eql "^0.1.3" type-detect "^1.0.0" -chalk@^1.0.0, chalk@^1.1.0, chalk@^1.1.1, chalk@^1.1.3: +chalk@^1.0.0, chalk@^1.1.0, chalk@^1.1.1, chalk@^1.1.3, chalk@1.1.3: version "1.1.3" resolved "https://registry.yarnpkg.com/chalk/-/chalk-1.1.3.tgz#a8115c55e4a702fe4d150abd3872822a7e09fc98" dependencies: @@ -1656,6 +1656,10 @@ di@^0.0.1: version "0.0.1" resolved "https://registry.yarnpkg.com/di/-/di-0.0.1.tgz#806649326ceaa7caa3306d75d985ea2748ba913c" +diff: + version "3.0.1" + resolved "https://registry.yarnpkg.com/diff/-/diff-3.0.1.tgz#a52d90cc08956994be00877bff97110062582c35" + diff@1.4.0: version "1.4.0" resolved "https://registry.yarnpkg.com/diff/-/diff-1.4.0.tgz#7f28d2eb9ee7b15a97efd89ce63dcfdaa3ccbabf" @@ -3141,6 +3145,12 @@ karma-coverage@^1.1.1: minimatch "^3.0.0" source-map "^0.5.1" +karma-mocha-reporter: + version "2.2.1" + resolved "https://registry.yarnpkg.com/karma-mocha-reporter/-/karma-mocha-reporter-2.2.1.tgz#8508b2f0925433a6417f0c528e53fcb411745758" + dependencies: + chalk "1.1.3" + karma-mocha@^1.2.0: version "1.2.0" resolved "https://registry.yarnpkg.com/karma-mocha/-/karma-mocha-1.2.0.tgz#bca6be2a66805b847417e8d2873fd0b5b27ee7cd" From 9171b382fe51778165d32410d1ce28185d01c947 Mon Sep 17 00:00:00 2001 From: Roger Braun Date: Fri, 18 Nov 2016 19:48:02 +0100 Subject: [PATCH 9/9] Complete rewrite of status adding code. This now uses nearly only mutation, to take advantage of vue's mutation tracking. --- src/modules/statuses.js | 196 ++++++++++------------- test/unit/specs/modules/statuses.spec.js | 50 ++++-- 2 files changed, 120 insertions(+), 126 deletions(-) diff --git a/src/modules/statuses.js b/src/modules/statuses.js index 268e64fb39..c088fa1a06 100644 --- a/src/modules/statuses.js +++ b/src/modules/statuses.js @@ -1,4 +1,4 @@ -import { reduce, map, slice, last, intersectionBy, sortBy, unionBy, toInteger, groupBy, differenceBy, each, find, flatten, maxBy, merge } from 'lodash' +import { map, slice, sortBy, toInteger, each, find, flatten, maxBy, last, merge, max } from 'lodash' import moment from 'moment' import apiService from '../services/api/api.service.js' // import parse from '../services/status_parser/status_parser.js' @@ -37,10 +37,6 @@ export const defaultState = { } } -const statusType = (status) => { - return !status.is_post_verb && status.uri.match(/fave/) ? 'fave' : 'status' -} - export const prepareStatus = (status) => { // Parse nsfw tags if (status.nsfw === undefined) { @@ -54,71 +50,6 @@ export const prepareStatus = (status) => { return status } -// Merges old and new status collections. -const mergeStatuses = (oldStatuses, newStatuses) => { - each(newStatuses, (status) => { - let oldStatus = find(oldStatuses, { id: status.id }) - if (oldStatus) { - merge(oldStatus, status) - } else { - oldStatuses.push(status) - } - }) - return oldStatuses -} - -const addStatusesToTimeline = (addedStatuses, showImmediately, { statuses, visibleStatuses, newStatusCount, faves, loading, maxId }) => { - const statusesAndFaves = groupBy(addedStatuses, statusType) - const addedFaves = statusesAndFaves['fave'] || [] - const unseenFaves = differenceBy(addedFaves, faves, 'id') - - // Update fave count - each(unseenFaves, ({in_reply_to_status_id}) => { - const status = find(statuses, { id: toInteger(in_reply_to_status_id) }) - if (status) { - status.fave_num += 1 - } - }) - - addedStatuses = statusesAndFaves['status'] || [] - - // Add some html and nsfw to the statuses. - addedStatuses = map(addedStatuses, (status) => { - const statusoid = status.retweeted_status || status - - prepareStatus(statusoid) - - return status - }) - - const newStatuses = sortBy( - unionBy(addedStatuses, statuses, 'id'), - ({id}) => -id - ) - - let newNewStatusCount = newStatusCount + (newStatuses.length - statuses.length) - - let newVisibleStatuses = visibleStatuses - - if (showImmediately) { - newVisibleStatuses = unionBy(addedStatuses, newVisibleStatuses, 'id') - newVisibleStatuses = sortBy(newVisibleStatuses, ({id}) => -id) - newNewStatusCount = newStatusCount - }; - - newVisibleStatuses = intersectionBy(newStatuses, newVisibleStatuses, 'id') - - return { - statuses: newStatuses, - visibleStatuses: newVisibleStatuses, - newStatusCount: newNewStatusCount, - minVisibleId: (last(newVisibleStatuses) || { id: undefined }).id, - faves: unionBy(faves, addedFaves, 'id'), - maxId, - loading - } -} - const updateTimestampsInStatuses = (statuses) => { return map(statuses, (statusoid) => { const status = statusoid.retweeted_status || statusoid @@ -129,66 +60,109 @@ const updateTimestampsInStatuses = (statuses) => { }) } -// const groupStatusesByType = (statuses) => { -// return groupBy(statuses, (status) => { -// if (status.is_post_verb) { -// return 'status' -// } +const statusType = (status) => { + if (status.is_post_verb) { + return 'status' + } -// if (status.retweeted_status) { -// return 'retweet' -// } + if (status.retweeted_status) { + return 'retweet' + } -// if (typeof status.uri === 'string' && status.uri.match(/fave/)) { -// return 'favorite' -// } + if (typeof status.uri === 'string' && status.uri.match(/fave/)) { + return 'favorite' + } -// return 'unknown' -// }) -// } + return 'unknown' +} export const findMaxId = (...args) => { return (maxBy(flatten(args), 'id') || {}).id } +const mergeOrAdd = (arr, item) => { + const oldItem = find(arr, {id: item.id}) + if (oldItem) { + // We already have this, so only merge the new info. + merge(oldItem, item) + return oldItem + } else { + // This is a new item, prepare it + prepareStatus(item) + arr.push(item) + return item + } +} + export const mutations = { addNewStatuses (state, { statuses, showImmediately = false, timeline }) { - - // Merge in the new status into the old ones. - mergeStatuses(state.allStatuses, statuses) - - // Get relevant timeline + const allStatuses = state.allStatuses const timelineObject = state.timelines[timeline] - // Set new maxId - const maxId = findMaxId(statuses, timelineObject.statuses) - timelineObject.maxId = maxId - - // Split statuses by type - // const statusesByType = groupStatusesByType(statuses) - - state.timelines[timeline] = addStatusesToTimeline(statuses, showImmediately, state.timelines[timeline]) - - // Set up retweets with most current status - const getRetweets = (result, status) => { - if (status.retweeted_status) { - result.push(status.retweeted_status) - } - return result + // Set the maxId to the new id if it's larger. + const updateMaxId = ({id}) => { + timelineObject.maxId = max([id, timelineObject.maxId]) } - const retweets = reduce(statuses, getRetweets, []) + const addStatus = (status, showImmediately, addToTimeline = true) => { + // Remember the current amount of statuses + // We need that to calculate new status count. + const prevLength = timelineObject.statuses.length - // state.allStatuses = unionBy(retweets, state.allStatuses, 'id') + updateMaxId(status) - mergeStatuses(state.allStatuses, retweets) + status = mergeOrAdd(allStatuses, status) - each(state.allStatuses, (status) => { - if (status.retweeted_status) { - const retweetedStatus = find(state.allStatuses, { id: status.retweeted_status.id }) - status.retweeted_status = retweetedStatus + // Some statuses should only be added to the global status repository. + if (addToTimeline) { + mergeOrAdd(timelineObject.statuses, status) } + + if (showImmediately) { + // Add it directly to the visibleStatuses, don't change + // newStatusCount + mergeOrAdd(timelineObject.visibleStatuses, status) + } else { + // Just change newStatuscount + timelineObject.newStatusCount += (timelineObject.statuses.length - prevLength) + } + + return status + } + + const favoriteStatus = (favorite) => { + const status = find(allStatuses, { id: toInteger(favorite.in_reply_to_status_id) }) + if (status) { + status.fave_num += 1 + } + return status + } + + const processors = { + 'status': (status) => { + addStatus(status, showImmediately) + }, + 'retweet': (status) => { + // RetweetedStatuses are never shown immediately + const retweetedStatus = addStatus(status.retweeted_status, false, false) + const retweet = addStatus(status, showImmediately) + retweet.retweeted_status = retweetedStatus + }, + 'favorite': (status) => { + updateMaxId(status) + favoriteStatus(status) + } + } + + each(statuses, (status) => { + const type = statusType(status) + processors[type](status) }) + + // Keep the visible statuses sorted + timelineObject.visibleStatuses = sortBy(timelineObject.visibleStatuses, ({id}) => -id) + timelineObject.statuses = sortBy(timelineObject.statuses, ({id}) => -id) + timelineObject.minVisibleId = (last(timelineObject.statuses) || {}).id }, showNewStatuses (state, { timeline }) { const oldTimeline = (state.timelines[timeline]) diff --git a/test/unit/specs/modules/statuses.spec.js b/test/unit/specs/modules/statuses.spec.js index 0370d4aecc..80bb5fc681 100644 --- a/test/unit/specs/modules/statuses.spec.js +++ b/test/unit/specs/modules/statuses.spec.js @@ -1,13 +1,14 @@ import { cloneDeep } from 'lodash' import { defaultState, mutations, findMaxId, prepareStatus } from '../../../../src/modules/statuses.js' -const makeMockStatus = ({id, text}) => { +const makeMockStatus = ({id, text, is_post_verb = true}) => { return { id, name: 'status', text: text || `Text number ${id}`, fave_num: 0, - uri: '' + uri: '', + is_post_verb } } @@ -62,6 +63,7 @@ describe('The Statuses module', () => { expect(state.allStatuses).to.eql([status]) expect(state.timelines.public.statuses).to.eql([status]) expect(state.timelines.public.visibleStatuses).to.eql([]) + expect(state.timelines.public.newStatusCount).to.equal(1) }) it('adds the status to allStatuses and to the given timeline, directly visible', () => { @@ -73,12 +75,30 @@ describe('The Statuses module', () => { expect(state.allStatuses).to.eql([status]) expect(state.timelines.public.statuses).to.eql([status]) expect(state.timelines.public.visibleStatuses).to.eql([status]) + expect(state.timelines.public.newStatusCount).to.equal(0) + }) + + it('keeps a descending by id order in timeline.visibleStatuses and timeline.statuses', () => { + const state = cloneDeep(defaultState) + const status = makeMockStatus({id: 2}) + const statusTwo = makeMockStatus({id: 1}) + const statusThree = makeMockStatus({id: 3}) + + mutations.addNewStatuses(state, { statuses: [status], showImmediately: true, timeline: 'public' }) + mutations.addNewStatuses(state, { statuses: [statusTwo], showImmediately: true, timeline: 'public' }) + + expect(state.timelines.public.minVisibleId).to.equal(1) + + mutations.addNewStatuses(state, { statuses: [statusThree], showImmediately: true, timeline: 'public' }) + + expect(state.timelines.public.statuses).to.eql([statusThree, status, statusTwo]) + expect(state.timelines.public.visibleStatuses).to.eql([statusThree, status, statusTwo]) }) it('splits retweets from their status and links them', () => { const state = cloneDeep(defaultState) const status = makeMockStatus({id: 1}) - const retweet = makeMockStatus({id: 2}) + const retweet = makeMockStatus({id: 2, is_post_verb: false}) const modStatus = makeMockStatus({id: 1, text: 'something else'}) retweet.retweeted_status = status @@ -86,12 +106,18 @@ describe('The Statuses module', () => { // It adds both statuses, but only the retweet to visible. mutations.addNewStatuses(state, { statuses: [retweet], timeline: 'public', showImmediately: true }) expect(state.timelines.public.visibleStatuses).to.have.length(1) - expect(state.allStatuses).to.eql([retweet, status]) + expect(state.timelines.public.statuses).to.have.length(1) + expect(state.allStatuses).to.have.length(2) + expect(state.allStatuses[0].id).to.equal(1) + expect(state.allStatuses[1].id).to.equal(2) // It refers to the modified status. mutations.addNewStatuses(state, { statuses: [modStatus], timeline: 'public' }) - expect(state.allStatuses).to.eql([retweet, modStatus]) - expect(retweet.retweeted_status).to.eql(modStatus) + expect(state.allStatuses).to.have.length(2) + expect(state.allStatuses[0].id).to.equal(1) + expect(state.allStatuses[0].text).to.equal(modStatus.text) + expect(state.allStatuses[1].id).to.equal(2) + expect(retweet.retweeted_status.text).to.eql(modStatus.text) }) it('replaces existing statuses with the same id', () => { @@ -108,14 +134,14 @@ describe('The Statuses module', () => { mutations.addNewStatuses(state, { statuses: [modStatus], showImmediately: true, timeline: 'public' }) expect(state.timelines.public.visibleStatuses).to.have.length(1) expect(state.allStatuses).to.have.length(1) - expect(state.allStatuses[0]).to.eql(modStatus) + expect(state.allStatuses[0].text).to.eql(modStatus.text) }) it('replaces existing statuses with the same id, coming from a retweet', () => { const state = cloneDeep(defaultState) const status = makeMockStatus({id: 1}) const modStatus = makeMockStatus({id: 1, text: 'something else'}) - const retweet = makeMockStatus({id: 2}) + const retweet = makeMockStatus({id: 2, is_post_verb: false}) retweet.retweeted_status = modStatus // Add original status @@ -127,7 +153,7 @@ describe('The Statuses module', () => { mutations.addNewStatuses(state, { statuses: [retweet], showImmediately: false, timeline: 'public' }) expect(state.timelines.public.visibleStatuses).to.have.length(1) expect(state.allStatuses).to.have.length(2) - expect(state.allStatuses[0]).to.eql(modStatus) + expect(state.allStatuses[0].text).to.eql(modStatus.text) }) it('handles favorite actions', () => { @@ -148,11 +174,5 @@ describe('The Statuses module', () => { expect(state.timelines.public.visibleStatuses.length).to.eql(1) expect(state.timelines.public.visibleStatuses[0].fave_num).to.eql(1) expect(state.timelines.public.maxId).to.eq(favorite.id) - - // Adding again shouldn't change anything - mutations.addNewStatuses(state, { statuses: [favorite], showImmediately: true, timeline: 'public' }) - - expect(state.timelines.public.visibleStatuses.length).to.eql(1) - expect(state.timelines.public.visibleStatuses[0].fave_num).to.eql(1) }) })