From 840ce063971d68063d380fc5f3aacc0564363b50 Mon Sep 17 00:00:00 2001 From: Henry Jameson Date: Tue, 16 Aug 2022 19:24:20 +0300 Subject: [PATCH 1/6] proper journal trimming + remove some old workaround to my local bad data --- src/modules/serverSideStorage.js | 46 +++++++++++-------- .../specs/modules/serverSideStorage.spec.js | 15 +++++- 2 files changed, 39 insertions(+), 22 deletions(-) diff --git a/src/modules/serverSideStorage.js b/src/modules/serverSideStorage.js index eb089be67..08b11b0ba 100644 --- a/src/modules/serverSideStorage.js +++ b/src/modules/serverSideStorage.js @@ -1,5 +1,5 @@ import { toRaw } from 'vue' -import { isEqual, uniqWith, cloneDeep, set, get, clamp } from 'lodash' +import { isEqual, cloneDeep, set, get, clamp, flatten, groupBy, findLastIndex, takeRight } from 'lodash' import { CURRENT_UPDATE_COUNTER } from 'src/components/update_notification/update_notification.js' export const VERSION = 1 @@ -131,25 +131,32 @@ export const _mergeFlags = (recent, stale, allFlagKeys) => { })) } -const _mergeJournal = (a, b) => uniqWith( - // TODO use groupBy to group by path, then trim them depending on operations, - // i.e. if field got removed in the end - no need to sort it beforehand, if field - // got re-added no need to remove it and add it etc. - [ - ...(Array.isArray(a) ? a : []), - ...(Array.isArray(b) ? b : []) - ].sort((a, b) => a.timestamp > b.timestamp ? -1 : 1), - (a, b) => { - if (a.operation !== b.operation) return false - switch (a.operation) { - case 'set': - case 'arrangeSet': - return a.path === b.path - default: - return a.path === b.path && a.timestamp === b.timestamp +const _mergeJournal = (...journals) => { + const allJournals = flatten(journals.map(j => Array.isArray(j) ? j : [])) + const grouped = groupBy(allJournals, 'path') + const trimmedGrouped = Object.entries(grouped).map(([path, journal]) => { + // side effect + journal.sort((a, b) => a.timestamp > b.timestamp ? 1 : -1) + + if (path.startsWith('collections')) { + const lastRemoveIndex = findLastIndex(journal, ({ operation }) => operation === 'removeFromCollection') + // everything before last remove is unimportant + if (lastRemoveIndex > 0) { + return journal.slice(lastRemoveIndex) + } else { + // everything else doesn't need trimming + return journal + } + } else if (path.startsWith('simple')) { + // Only the last record is important + return takeRight(journal) + } else { + return journal } - } -).reverse() + }) + return flatten(trimmedGrouped) + .sort((a, b) => a.timestamp > b.timestamp ? 1 : -1) +} export const _mergePrefs = (recent, stale, allFlagKeys) => { if (!stale) return recent @@ -169,7 +176,6 @@ export const _mergePrefs = (recent, stale, allFlagKeys) => { const resultOutput = { ...recentData } const totalJournal = _mergeJournal(staleJournal, recentJournal) totalJournal.forEach(({ path, timestamp, operation, command, args }) => { - operation = operation || command if (path.startsWith('_')) { console.error(`journal contains entry to edit internal (starts with _) field '${path}', something is incorrect here, ignoring.`) return diff --git a/test/unit/specs/modules/serverSideStorage.spec.js b/test/unit/specs/modules/serverSideStorage.spec.js index f10e21e6d..7adce20d4 100644 --- a/test/unit/specs/modules/serverSideStorage.spec.js +++ b/test/unit/specs/modules/serverSideStorage.spec.js @@ -107,7 +107,7 @@ describe('The serverSideStorage module', () => { }) }) describe('setPreference', () => { - const { setPreference, updateCache } = mutations + const { setPreference, updateCache, addToCollection, removeFromCollection } = mutations it('should set preference and update journal log accordingly', () => { const state = cloneDeep(defaultState) @@ -123,12 +123,15 @@ describe('The serverSideStorage module', () => { }) }) - it('should keep journal to a minimum (one entry per path for sets)', () => { + it('should keep journal to a minimum', () => { const state = cloneDeep(defaultState) setPreference(state, { path: 'simple.testing', value: 1 }) setPreference(state, { path: 'simple.testing', value: 2 }) + addToCollection(state, { path: 'collections.testing', value: 2 }) + removeFromCollection(state, { path: 'collections.testing', value: 2 }) updateCache(state, { username: 'test' }) expect(state.prefsStorage.simple.testing).to.eql(2) + expect(state.prefsStorage.collections.testing).to.eql([]) expect(state.prefsStorage._journal.length).to.eql(1) expect(state.prefsStorage._journal[0]).to.eql({ path: 'simple.testing', @@ -137,6 +140,14 @@ describe('The serverSideStorage module', () => { // should have A timestamp, we don't really care what it is timestamp: state.prefsStorage._journal[0].timestamp }) + expect(state.prefsStorage._journal[1]).to.eql({ + path: 'collection.testing', + operation: 'remove', + args: [2], + // should have A timestamp, we don't really care what it is + timestamp: state.prefsStorage._journal[1].timestamp + }) + }) }) }) }) From 04acf069d1600f097805ce52f958263e68e153c1 Mon Sep 17 00:00:00 2001 From: Henry Jameson Date: Tue, 16 Aug 2022 19:33:34 +0300 Subject: [PATCH 2/6] ignore invalid journal entries --- src/modules/serverSideStorage.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/modules/serverSideStorage.js b/src/modules/serverSideStorage.js index 08b11b0ba..56164be7a 100644 --- a/src/modules/serverSideStorage.js +++ b/src/modules/serverSideStorage.js @@ -132,7 +132,15 @@ export const _mergeFlags = (recent, stale, allFlagKeys) => { } const _mergeJournal = (...journals) => { - const allJournals = flatten(journals.map(j => Array.isArray(j) ? j : [])) + // Ignore invalid journal entries + const allJournals = flatten( + journals.map(j => Array.isArray(j) ? j : []) + ).filter(entry => + Object.prototype.hasOwnProperty.call(entry, 'path') && + Object.prototype.hasOwnProperty.call(entry, 'operation') && + Object.prototype.hasOwnProperty.call(entry, 'args') && + Object.prototype.hasOwnProperty.call(entry, 'timestamp') + ) const grouped = groupBy(allJournals, 'path') const trimmedGrouped = Object.entries(grouped).map(([path, journal]) => { // side effect From 821a09109c4747fcec49059f03c8cc908bd07ac5 Mon Sep 17 00:00:00 2001 From: Henry Jameson Date: Tue, 16 Aug 2022 20:00:29 +0300 Subject: [PATCH 3/6] fix list tests --- src/modules/lists.js | 5 +++-- test/unit/specs/modules/lists.spec.js | 16 ++++++++-------- .../unit/specs/modules/serverSideStorage.spec.js | 1 - 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/modules/lists.js b/src/modules/lists.js index d9fab9695..22fed8005 100644 --- a/src/modules/lists.js +++ b/src/modules/lists.js @@ -15,10 +15,11 @@ export const mutations = { } state.allListsObject[listId].title = title - if (!find(state.allLists, { id: listId })) { + const entry = find(state.allLists, { id: listId }) + if (!entry) { state.allLists.push({ id: listId, title }) } else { - find(state.allLists, { id: listId }).title = title + entry.title = title } }, setListAccounts (state, { listId, accountIds }) { diff --git a/test/unit/specs/modules/lists.spec.js b/test/unit/specs/modules/lists.spec.js index ac9af1b6b..e43106eac 100644 --- a/test/unit/specs/modules/lists.spec.js +++ b/test/unit/specs/modules/lists.spec.js @@ -17,13 +17,13 @@ describe('The lists module', () => { const list = { id: '1', title: 'testList' } const modList = { id: '1', title: 'anotherTestTitle' } - mutations.setList(state, list) - expect(state.allListsObject[list.id]).to.eql({ title: list.title }) + mutations.setList(state, { listId: list.id, title: list.title }) + expect(state.allListsObject[list.id]).to.eql({ title: list.title, accountIds: [] }) expect(state.allLists).to.have.length(1) expect(state.allLists[0]).to.eql(list) - mutations.setList(state, modList) - expect(state.allListsObject[modList.id]).to.eql({ title: modList.title }) + mutations.setList(state, { listId: modList.id, title: modList.title }) + expect(state.allListsObject[modList.id]).to.eql({ title: modList.title, accountIds: [] }) expect(state.allLists).to.have.length(1) expect(state.allLists[0]).to.eql(modList) }) @@ -33,10 +33,10 @@ describe('The lists module', () => { const list = { id: '1', accountIds: ['1', '2', '3'] } const modList = { id: '1', accountIds: ['3', '4', '5'] } - mutations.setListAccounts(state, list) + mutations.setListAccounts(state, { listId: list.id, accountIds: list.accountIds }) expect(state.allListsObject[list.id]).to.eql({ accountIds: list.accountIds }) - mutations.setListAccounts(state, modList) + mutations.setListAccounts(state, { listId: modList.id, accountIds: modList.accountIds }) expect(state.allListsObject[modList.id]).to.eql({ accountIds: modList.accountIds }) }) @@ -47,9 +47,9 @@ describe('The lists module', () => { 1: { title: 'testList', accountIds: ['1', '2', '3'] } } } - const id = '1' + const listId = '1' - mutations.deleteList(state, { id }) + mutations.deleteList(state, { listId }) expect(state.allLists).to.have.length(0) expect(state.allListsObject).to.eql({}) }) diff --git a/test/unit/specs/modules/serverSideStorage.spec.js b/test/unit/specs/modules/serverSideStorage.spec.js index 7adce20d4..69cad0a5b 100644 --- a/test/unit/specs/modules/serverSideStorage.spec.js +++ b/test/unit/specs/modules/serverSideStorage.spec.js @@ -148,7 +148,6 @@ describe('The serverSideStorage module', () => { timestamp: state.prefsStorage._journal[1].timestamp }) }) - }) }) }) From 38bd59ceb0182de15e2e97d750df59ad53dfa51a Mon Sep 17 00:00:00 2001 From: Henry Jameson Date: Tue, 16 Aug 2022 20:14:18 +0300 Subject: [PATCH 4/6] fix journal test --- test/unit/specs/modules/serverSideStorage.spec.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/unit/specs/modules/serverSideStorage.spec.js b/test/unit/specs/modules/serverSideStorage.spec.js index 69cad0a5b..be249eede 100644 --- a/test/unit/specs/modules/serverSideStorage.spec.js +++ b/test/unit/specs/modules/serverSideStorage.spec.js @@ -107,7 +107,7 @@ describe('The serverSideStorage module', () => { }) }) describe('setPreference', () => { - const { setPreference, updateCache, addToCollection, removeFromCollection } = mutations + const { setPreference, updateCache, addCollectionPreference, removeCollectionPreference } = mutations it('should set preference and update journal log accordingly', () => { const state = cloneDeep(defaultState) @@ -127,12 +127,12 @@ describe('The serverSideStorage module', () => { const state = cloneDeep(defaultState) setPreference(state, { path: 'simple.testing', value: 1 }) setPreference(state, { path: 'simple.testing', value: 2 }) - addToCollection(state, { path: 'collections.testing', value: 2 }) - removeFromCollection(state, { path: 'collections.testing', value: 2 }) + addCollectionPreference(state, { path: 'collections.testing', value: 2 }) + removeCollectionPreference(state, { path: 'collections.testing', value: 2 }) updateCache(state, { username: 'test' }) expect(state.prefsStorage.simple.testing).to.eql(2) expect(state.prefsStorage.collections.testing).to.eql([]) - expect(state.prefsStorage._journal.length).to.eql(1) + expect(state.prefsStorage._journal.length).to.eql(2) expect(state.prefsStorage._journal[0]).to.eql({ path: 'simple.testing', operation: 'set', @@ -141,8 +141,8 @@ describe('The serverSideStorage module', () => { timestamp: state.prefsStorage._journal[0].timestamp }) expect(state.prefsStorage._journal[1]).to.eql({ - path: 'collection.testing', - operation: 'remove', + path: 'collections.testing', + operation: 'removeFromCollection', args: [2], // should have A timestamp, we don't really care what it is timestamp: state.prefsStorage._journal[1].timestamp From d074aefb4ffe8fc7bdb0e5f0afec46f7042a90fe Mon Sep 17 00:00:00 2001 From: Henry Jameson Date: Wed, 17 Aug 2022 00:48:10 +0300 Subject: [PATCH 5/6] List edit UI overhaul --- src/App.js | 4 + src/App.vue | 2 +- src/components/lists_edit/lists_edit.js | 84 +++++-- src/components/lists_edit/lists_edit.vue | 214 +++++++++++++----- .../lists_user_search/lists_user_search.js | 7 +- .../lists_user_search/lists_user_search.vue | 22 +- .../mobile_post_status_button.js | 3 +- src/components/tab_switcher/tab_switcher.scss | 1 + src/i18n/en.json | 13 +- 9 files changed, 253 insertions(+), 97 deletions(-) diff --git a/src/App.js b/src/App.js index f5bd7e2e4..12a12f71e 100644 --- a/src/App.js +++ b/src/App.js @@ -85,8 +85,12 @@ export default { isChats () { return this.$route.name === 'chat' || this.$route.name === 'chats' }, + isListEdit () { + return this.$route.name === 'lists-edit' + }, newPostButtonShown () { if (this.isChats) return false + if (this.isListEdit) return false return this.$store.getters.mergedConfig.alwaysShowNewPostButton || this.layoutType === 'mobile' }, showFeaturesPanel () { return this.$store.state.instance.showFeaturesPanel }, diff --git a/src/App.vue b/src/App.vue index c741aa700..cb6a0115f 100644 --- a/src/App.vue +++ b/src/App.vue @@ -33,7 +33,7 @@
{ this.title = this.findListTitle(this.id) }) + .then(() => { + this.title = this.findListTitle(this.id) + this.titleDraft = this.title + }) this.$store.dispatch('fetchListAccounts', { listId: this.id }) .then(() => { - this.selectedUserIds = this.findListAccounts(this.id) - this.selectedUserIds.forEach(userId => { + this.membersUserIds = this.findListAccounts(this.id) + this.membersUserIds.forEach(userId => { this.$store.dispatch('fetchUserIfMissing', userId) }) }) @@ -41,11 +53,12 @@ const ListsNew = { id () { return this.$route.params.id }, - users () { - return this.userIds.map(userId => this.findUser(userId)) + membersUsers () { + return [...this.membersUserIds, ...this.addedUserIds] + .map(userId => this.findUser(userId)).filter(user => user) }, - selectedUsers () { - return this.selectedUserIds.map(userId => this.findUser(userId)).filter(user => user) + searchUsers () { + return this.searchUserIds.map(userId => this.findUser(userId)).filter(user => user) }, ...mapState({ currentUser: state => state.users.currentUser @@ -56,30 +69,51 @@ const ListsNew = { onInput () { this.search(this.query) }, - selectUser (user) { - if (this.selectedUserIds.includes(user.id)) { - this.removeUser(user.id) - } else { + toggleRemoveMember (user) { + if (this.removedUserIds.has(user.id)) { this.addUser(user) + this.removedUserIds.delete(user.id) + } else { + this.removeUser(user.id) + this.removedUserIds.add(user.id) } }, - isSelected (user) { - return this.selectedUserIds.includes(user.id) + toggleAddFromSearch (user) { + if (this.addedUserIds.has(user.id)) { + this.removeUser(user.id) + this.addedUserIds.delete(user.id) + } else { + this.addUser(user) + this.addedUserIds.add(user.id) + } + }, + isRemoved (user) { + return this.removedUserIds.has(user.id) + }, + isAdded (user) { + return this.addedUserIds.has(user.id) }, addUser (user) { - this.selectedUserIds.push(user.id) + // this.selectedUserIds.push(user.id) }, removeUser (userId) { - this.selectedUserIds = this.selectedUserIds.filter(id => id !== userId) + // this.selectedUserIds = this.selectedUserIds.filter(id => id !== userId) }, - onResults (results) { - this.userIds = results + onSearchLoading (results) { + this.searchLoading = true }, - updateList () { - this.$store.dispatch('setList', { listId: this.id, title: this.title }) - this.$store.dispatch('setListAccounts', { listId: this.id, accountIds: this.selectedUserIds }) - - this.$router.push({ name: 'lists-timeline', params: { id: this.id } }) + onSearchLoadingDone (results) { + this.searchLoading = false + }, + onSearchResults (results) { + this.searchLoading = false + this.searchUserIds = results + }, + updateListTitle () { + this.$store.dispatch('setList', { listId: this.id, title: this.titleDraft }) + .then(() => { + this.title = this.findListTitle(this.id) + }) }, deleteList () { this.$store.dispatch('deleteList', { listId: this.id }) diff --git a/src/components/lists_edit/lists_edit.vue b/src/components/lists_edit/lists_edit.vue index 69007b022..dffbff2f5 100644 --- a/src/components/lists_edit/lists_edit.vue +++ b/src/components/lists_edit/lists_edit.vue @@ -1,8 +1,8 @@ @@ -69,28 +144,43 @@ diff --git a/src/components/lists_user_search/lists_user_search.js b/src/components/lists_user_search/lists_user_search.js index 5798841a9..c92ec0eee 100644 --- a/src/components/lists_user_search/lists_user_search.js +++ b/src/components/lists_user_search/lists_user_search.js @@ -15,6 +15,7 @@ const ListsUserSearch = { components: { Checkbox }, + emits: ['loading', 'loadingDone', 'results'], data () { return { loading: false, @@ -33,12 +34,16 @@ const ListsUserSearch = { } this.loading = true + this.$emit('loading') this.userIds = [] this.$store.dispatch('search', { q: query, resolve: true, type: 'accounts', following: this.followingOnly }) .then(data => { - this.loading = false this.$emit('results', data.accounts.map(a => a.id)) }) + .finally(() => { + this.loading = false + this.$emit('loadingDone') + }) } } } diff --git a/src/components/lists_user_search/lists_user_search.vue b/src/components/lists_user_search/lists_user_search.vue index 03fb3ba65..8633170c6 100644 --- a/src/components/lists_user_search/lists_user_search.vue +++ b/src/components/lists_user_search/lists_user_search.vue @@ -1,5 +1,5 @@
diff --git a/src/components/user_list_menu/user_list_menu.vue b/src/components/user_list_menu/user_list_menu.vue index f90c5f87f..d8da16ea5 100644 --- a/src/components/user_list_menu/user_list_menu.vue +++ b/src/components/user_list_menu/user_list_menu.vue @@ -7,18 +7,18 @@ >