From df94a5ee2109c42aa135d3e7326fb081610b6f7f Mon Sep 17 00:00:00 2001 From: Holger Dehnhardt Date: Fri, 22 May 2020 18:02:11 +0200 Subject: [PATCH 1/2] Build proper folder hierarchy Signed-off-by: Holger Dehnhardt --- src/components/NavigationMailbox.vue | 5 +- src/imap/MailboxHierarchy.js | 62 -- src/imap/MailboxPrefix.js | 34 -- src/store/getters.js | 1 - src/store/index.js | 4 + src/store/mutations.js | 72 +-- src/tests/unit/imap/MailboxHierarchy.spec.js | 188 ------ src/tests/unit/imap/MailboxPrefix.spec.js | 58 -- src/tests/unit/store/mutations.spec.js | 590 ++++++++++++++++--- 9 files changed, 540 insertions(+), 474 deletions(-) delete mode 100644 src/imap/MailboxHierarchy.js delete mode 100644 src/imap/MailboxPrefix.js delete mode 100644 src/tests/unit/imap/MailboxHierarchy.spec.js delete mode 100644 src/tests/unit/imap/MailboxPrefix.spec.js diff --git a/src/components/NavigationMailbox.vue b/src/components/NavigationMailbox.vue index 390995543f..c649d49b97 100644 --- a/src/components/NavigationMailbox.vue +++ b/src/components/NavigationMailbox.vue @@ -70,7 +70,7 @@ @click="clearCache"> {{ t('mail', 'Clear locally cached data, in case there are issues with synchronization.') }} - + {{ t('mail', 'Delete folder') }} @@ -176,6 +176,9 @@ export default { }, } }, + hasSubMailboxes() { + return this.subMailboxes.length > 0 + }, subMailboxes() { return this.$store.getters.getSubMailboxes(this.mailbox.databaseId) }, diff --git a/src/imap/MailboxHierarchy.js b/src/imap/MailboxHierarchy.js deleted file mode 100644 index c418c3e0b2..0000000000 --- a/src/imap/MailboxHierarchy.js +++ /dev/null @@ -1,62 +0,0 @@ -/* - * @copyright 2019 Christoph Wurst - * - * @author 2019 Christoph Wurst - * - * @license GNU AGPL version 3 or any later version - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ - -const getParentId = (mailbox, hasPrefix) => { - const top = hasPrefix ? 1 : 0 - const hierarchy = mailbox.name.split(mailbox.delimiter) - if (hierarchy.length <= top + 1 || mailbox.name === 'INBOX/FLAGGED') { - return - } - if (hasPrefix) { - return hierarchy[0] + mailbox.delimiter + hierarchy[1] - } else { - return hierarchy[0] - } -} - -export const buildMailboxHierarchy = (mailboxes, havePrefix) => { - if (!mailboxes.length) { - // Nothing to do - return mailboxes - } - - const cloned = mailboxes.map((mailbox) => { - return { - mailboxes: [], - ...mailbox, - } - }) - const top = cloned.filter((mailbox) => getParentId(mailbox, havePrefix) === undefined) - - cloned.forEach((mailbox) => { - if (top.indexOf(mailbox) !== -1) { - return - } - - const parentId = getParentId(mailbox, havePrefix) - const parent = cloned.filter((mailbox) => mailbox.name === parentId)[0] - if (parent) { - parent.mailboxes.push(mailbox) - } - }) - - return top -} diff --git a/src/imap/MailboxPrefix.js b/src/imap/MailboxPrefix.js deleted file mode 100644 index 68a49b4bfc..0000000000 --- a/src/imap/MailboxPrefix.js +++ /dev/null @@ -1,34 +0,0 @@ -/* - * @copyright 2019 Christoph Wurst - * - * @author 2019 Christoph Wurst - * - * @license GNU AGPL version 3 or any later version - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ - -const PREFIX = 'INBOX' - -export const havePrefix = (mailboxes) => { - return ( - mailboxes.filter((mailbox) => { - const hierarchy = mailbox.name.split(mailbox.delimiter) - if (hierarchy.length < 1 || hierarchy[0] !== PREFIX) { - return false - } - return true - }).length === mailboxes.length - ) -} diff --git a/src/store/getters.js b/src/store/getters.js index 752840857f..6addbff150 100644 --- a/src/store/getters.js +++ b/src/store/getters.js @@ -42,7 +42,6 @@ export const getters = { }, getSubMailboxes: (state, getters) => (id) => { const mailbox = getters.getMailbox(id) - return mailbox.mailboxes.map((id) => state.mailboxes[id]) }, getUnifiedMailbox: (state) => (specialRole) => { diff --git a/src/store/index.js b/src/store/index.js index 35af89ce61..224dcf24b8 100644 --- a/src/store/index.js +++ b/src/store/index.js @@ -57,11 +57,13 @@ export default new Vuex.Store({ accountId: 0, attributes: ['\\subscribed'], isUnified: true, + path: '', specialUse: ['inbox'], specialRole: 'inbox', unread: 0, mailboxes: [], envelopeLists: {}, + name: 'UNIFIED INBOX', }, [PRIORITY_INBOX_ID]: { id: PRIORITY_INBOX_ID, @@ -69,11 +71,13 @@ export default new Vuex.Store({ accountId: 0, attributes: ['\\subscribed'], isPriorityInbox: true, + path: '', specialUse: ['inbox'], specialRole: 'inbox', unread: 0, mailboxes: [], envelopeLists: {}, + name: 'PRIORITY INBOX', }, }, envelopes: {}, diff --git a/src/store/mutations.js b/src/store/mutations.js index 64e57b3031..4d526b40fd 100644 --- a/src/store/mutations.js +++ b/src/store/mutations.js @@ -19,22 +19,37 @@ * along with this program. If not, see . */ +import { curry } from 'ramda' import orderBy from 'lodash/fp/orderBy' import sortedUniqBy from 'lodash/fp/sortedUniqBy' import Vue from 'vue' -import { buildMailboxHierarchy } from '../imap/MailboxHierarchy' -import { havePrefix } from '../imap/MailboxPrefix' import { sortMailboxes } from '../imap/MailboxSorter' import { normalizedEnvelopeListId } from './normalization' import { UNIFIED_ACCOUNT_ID } from './constants' -const addMailboxToState = (state, account) => (mailbox) => { +const addMailboxToState = curry((state, account, mailbox) => { mailbox.accountId = account.id + mailbox.mailboxes = [] mailbox.envelopeLists = {} + + // Add all mailboxes (including submailboxes to state, but only toplevel to account + if (mailbox.name.includes(mailbox.delimiter)) { + mailbox.displayName = mailbox.name.substr(mailbox.name.lastIndexOf(mailbox.delimiter) + 1) + mailbox.path = mailbox.name.substr(0, mailbox.name.lastIndexOf(mailbox.delimiter)) + } else { + mailbox.displayName = mailbox.name + mailbox.path = '' + } + Vue.set(state.mailboxes, mailbox.databaseId, mailbox) - return mailbox.databaseId -} + const parent = Object.values(state.mailboxes).find(mb => mb.name === mailbox.path) + if (mailbox.path === '' || !parent) { + account.mailboxes.push(mailbox.databaseId) + } else { + parent.mailboxes.push(mailbox.databaseId) + } +}) const sortAccounts = (accounts) => { accounts.sort((a1, a2) => a1.order - a2.order) @@ -55,16 +70,9 @@ export default { ) // Save the mailboxes to the store, but only keep IDs in the account's mailboxes list - const mailboxes = buildMailboxHierarchy(sortMailboxes(account.mailboxes), havePrefix(account.mailboxes)) + const mailboxes = sortMailboxes(account.mailboxes || []) Vue.set(account, 'mailboxes', []) - const addToState = addMailboxToState(state, account) - mailboxes.forEach((mailbox) => { - // Add all mailboxes (including submailboxes to state, but only toplevel to account - const id = addToState(mailbox) - Vue.set(mailbox, 'mailboxes', mailbox.mailboxes.map(addToState)) - - account.mailboxes.push(id) - }) + mailboxes.map(addMailboxToState(state, account)) }, editAccount(state, account) { Vue.set(state.accounts, account.id, Object.assign({}, state.accounts[account.id], account)) @@ -87,27 +95,7 @@ export default { state.accounts[accountId].collapsed = false }, addMailbox(state, { account, mailbox }) { - // Flatten the existing ones before updating the hierarchy - const existing = account.mailboxes.map((id) => state.mailboxes[id]) - existing.forEach((mailbox) => { - if (!mailbox.mailboxes) { - return - } - mailbox.mailboxes.map((id) => existing.push(state.mailboxes[id])) - mailbox.mailboxes = [] - }) - // Save the mailboxes to the store, but only keep IDs in the account's mailboxes list - existing.push(mailbox) - const mailboxes = buildMailboxHierarchy(sortMailboxes(existing), havePrefix(existing)) - Vue.set(account, 'mailboxes', []) - const addToState = addMailboxToState(state, account) - mailboxes.forEach((mailbox) => { - // Add all mailboxes (including submailboxes to state, but only toplevel to account - const id = addToState(mailbox) - Vue.set(mailbox, 'mailboxes', mailbox.mailboxes.map(addToState)) - - account.mailboxes.push(id) - }) + addMailboxToState(state, account, mailbox) }, removeMailbox(state, { id }) { const mailbox = state.mailboxes[id] @@ -119,13 +107,13 @@ export default { throw new Error(`Account ${mailbox.accountId} of mailbox ${id} is unknown`) } Vue.delete(state.mailboxes, id) - account.mailboxes = account.mailboxes.filter((mbId) => mbId !== id) - account.mailboxes.forEach((fId) => { - const mailbox = state.mailboxes[fId] - if (mailbox.mailboxes) { - mailbox.mailboxes = mailbox.mailboxes.filter((mbId) => mbId !== id) - } - }) + + // Travers through the account and the full mailbox tree to find any dangling pointers + const removeRec = (parent) => { + parent.mailboxes = parent.mailboxes.filter((mbId) => mbId !== id) + parent.mailboxes.map(mbid => removeRec(state.mailboxes[mbid])) + } + removeRec(account) }, addEnvelope(state, { query, envelope }) { const mailbox = state.mailboxes[envelope.mailboxId] diff --git a/src/tests/unit/imap/MailboxHierarchy.spec.js b/src/tests/unit/imap/MailboxHierarchy.spec.js deleted file mode 100644 index 0b8fb60503..0000000000 --- a/src/tests/unit/imap/MailboxHierarchy.spec.js +++ /dev/null @@ -1,188 +0,0 @@ -/* - * @copyright 2019 Christoph Wurst - * - * @author 2019 Christoph Wurst - * - * @license GNU AGPL version 3 or any later version - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ - -import { buildMailboxHierarchy } from '../../../imap/MailboxHierarchy' - -describe('mailboxHierarchyBuilder', () => { - it('handles empty collections', () => { - const mailboxes = [] - - const hierarchy = buildMailboxHierarchy(mailboxes) - - expect(hierarchy).to.deep.equal(mailboxes) - }) - - it('builds a flat hierarchy', () => { - const mb1 = { - name: 'INBOX', - delimiter: '.', - } - const mb2 = { - name: 'Sent', - delimiter: '.', - } - const mailboxes = [mb1, mb2] - - const hierarchy = buildMailboxHierarchy(mailboxes) - - expect(hierarchy).to.deep.equal([ - { - name: 'INBOX', - delimiter: '.', - mailboxes: [], - }, - { - name: 'Sent', - delimiter: '.', - mailboxes: [], - }, - ]) - }) - - it('builds a nested hierarchy with one level', () => { - const mb1 = { - name: 'Archive', - delimiter: '.', - } - const mb2 = { - name: 'Archive.Sent', - delimiter: '.', - } - const mailboxes = [mb1, mb2] - - const hierarchy = buildMailboxHierarchy(mailboxes) - - expect(hierarchy).to.deep.equal([ - { - name: 'Archive', - delimiter: '.', - mailboxes: [ - { - name: 'Archive.Sent', - delimiter: '.', - mailboxes: [], - }, - ], - }, - ]) - }) - - it('builds a nested hierarchy with two levels', () => { - const mb1 = { - name: 'Archive', - delimiter: '.', - } - const mb2 = { - name: 'Archive.Sent', - delimiter: '.', - } - const mb3 = { - name: 'Archive.Sent.Old', - delimiter: '.', - } - const mailboxes = [mb1, mb2, mb3] - - const hierarchy = buildMailboxHierarchy(mailboxes) - - expect(hierarchy).to.deep.equal([ - { - name: 'Archive', - delimiter: '.', - mailboxes: [ - { - name: 'Archive.Sent', - delimiter: '.', - mailboxes: [], - }, - { - name: 'Archive.Sent.Old', - delimiter: '.', - mailboxes: [], - }, - ], - }, - ]) - }) - - it('does not use the flagged inbox as submailbox of inbox', () => { - const mb1 = { - name: 'INBOX', - delimiter: '/', - } - const mb2 = { - name: 'INBOX/FLAGGED', - delimiter: '/', - } - const mb3 = { - name: 'Archive', - delimiter: '/', - } - const mailboxes = [mb1, mb2, mb3] - - const hierarchy = buildMailboxHierarchy(mailboxes) - - expect(hierarchy).to.deep.equal([ - { - name: 'INBOX', - delimiter: '/', - mailboxes: [], - }, - { - name: 'INBOX/FLAGGED', - delimiter: '/', - mailboxes: [], - }, - { - name: 'Archive', - delimiter: '/', - mailboxes: [], - }, - ]) - }) - - it('builds a nested hierarchy with a prefix', () => { - const mb1 = { - name: 'INBOX.Archive', - delimiter: '.', - } - const mb2 = { - name: 'INBOX.Archive.Sent', - delimiter: '.', - } - const mailboxes = [mb1, mb2] - - const hierarchy = buildMailboxHierarchy(mailboxes, true) - - expect(hierarchy).to.deep.equal([ - { - name: 'INBOX.Archive', - delimiter: '.', - mailboxes: [ - { - name: 'INBOX.Archive.Sent', - delimiter: '.', - mailboxes: [], - }, - ], - }, - ]) - }) -}) diff --git a/src/tests/unit/imap/MailboxPrefix.spec.js b/src/tests/unit/imap/MailboxPrefix.spec.js deleted file mode 100644 index f9816bfc73..0000000000 --- a/src/tests/unit/imap/MailboxPrefix.spec.js +++ /dev/null @@ -1,58 +0,0 @@ -/* - * @copyright 2019 Christoph Wurst - * - * @author 2019 Christoph Wurst - * - * @license GNU AGPL version 3 or any later version - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ - -import { havePrefix } from '../../../imap/MailboxPrefix' - -describe('MailboxPrefix', () => { - it('does not find a prefix if there is none', () => { - const mailboxes = [ - { - name: 'INBOX', - delimiter: '.', - }, - { - name: 'Sent', - delimiter: '.', - }, - ] - - const result = havePrefix(mailboxes) - - expect(result).to.equal(false) - }) - - it('detects a prefix', () => { - const mailboxes = [ - { - name: 'INBOX', - delimiter: '.', - }, - { - name: 'INBOX.Sent', - delimiter: '.', - }, - ] - - const result = havePrefix(mailboxes) - - expect(result).to.equal(true) - }) -}) diff --git a/src/tests/unit/store/mutations.spec.js b/src/tests/unit/store/mutations.spec.js index 6099c9cac3..8616474f3e 100644 --- a/src/tests/unit/store/mutations.spec.js +++ b/src/tests/unit/store/mutations.spec.js @@ -27,6 +27,508 @@ import { } from '../../../store/constants' describe('Vuex store mutations', () => { + it('adds an account with no mailboxes', () => { + const state = { + accountList: [], + accounts: {}, + envelopes: {}, + mailboxes: {}, + } + + mutations.addAccount(state, { + accountId: 13, + id: 13, + mailboxes: [], + }) + + expect(state).to.deep.equal({ + accountList: [13], + accounts: { + 13: { + accountId: 13, + id: 13, + mailboxes: [], + collapsed: true, + }, + }, + envelopes: {}, + mailboxes: {}, + }) + }) + + it('adds an account with one level of mailboxes', () => { + const state = { + accountList: [], + accounts: {}, + envelopes: {}, + mailboxes: {}, + } + + mutations.addAccount(state, { + accountId: 13, + id: 13, + mailboxes: [ + { + databaseId: 345, + name: 'INBOX', + delimiter: '.', + }, + ], + }) + + expect(state).to.deep.equal({ + accountList: [13], + accounts: { + 13: { + accountId: 13, + id: 13, + mailboxes: [ + 345, + ], + collapsed: true, + }, + }, + envelopes: {}, + mailboxes: { + 345: { + accountId: 13, + databaseId: 345, + name: 'INBOX', + displayName: 'INBOX', + delimiter: '.', + envelopeLists: {}, + path: '', + mailboxes: [], + }, + }, + }) + }) + + it('adds an account with two levels of mailboxes', () => { + const state = { + accountList: [], + accounts: {}, + envelopes: {}, + mailboxes: {}, + } + + mutations.addAccount(state, { + accountId: 13, + id: 13, + mailboxes: [ + { + databaseId: 345, + name: 'Archive', + delimiter: '.', + specialUse: ['archive'], + specialRole: 'archive', + }, + { + databaseId: 346, + name: 'Archive.2020', + delimiter: '.', + specialUse: ['archive'], + specialRole: 'archive', + }, + ], + }) + + expect(state).to.deep.equal({ + accountList: [13], + accounts: { + 13: { + accountId: 13, + id: 13, + mailboxes: [ + 345, + ], + collapsed: true, + }, + }, + envelopes: {}, + mailboxes: { + 345: { + accountId: 13, + databaseId: 345, + name: 'Archive', + displayName: 'Archive', + delimiter: '.', + envelopeLists: {}, + path: '', + specialUse: ['archive'], + specialRole: 'archive', + mailboxes: [ + 346, + ], + }, + 346: { + accountId: 13, + databaseId: 346, + name: 'Archive.2020', + displayName: '2020', + delimiter: '.', + envelopeLists: {}, + path: 'Archive', + specialUse: ['archive'], + specialRole: 'archive', + mailboxes: [], + }, + }, + }) + }) + + it('adds an account with three levels of mailboxes', () => { + const state = { + accountList: [], + accounts: {}, + envelopes: {}, + mailboxes: {}, + } + + mutations.addAccount(state, { + accountId: 13, + id: 13, + mailboxes: [ + { + databaseId: 345, + name: 'Archive', + delimiter: '.', + specialUse: ['archive'], + specialRole: 'archive', + }, + { + databaseId: 346, + name: 'Archive.2020', + delimiter: '.', + specialUse: ['archive'], + specialRole: 'archive', + }, + { + databaseId: 347, + name: 'Archive.2020.08', + delimiter: '.', + specialUse: ['archive'], + specialRole: 'archive', + }, + ], + }) + + expect(state).to.deep.equal({ + accountList: [13], + accounts: { + 13: { + accountId: 13, + id: 13, + mailboxes: [ + 345, + ], + collapsed: true, + }, + }, + envelopes: {}, + mailboxes: { + 345: { + accountId: 13, + databaseId: 345, + name: 'Archive', + displayName: 'Archive', + delimiter: '.', + envelopeLists: {}, + path: '', + specialUse: ['archive'], + specialRole: 'archive', + mailboxes: [ + 346, + ], + }, + 346: { + accountId: 13, + databaseId: 346, + name: 'Archive.2020', + displayName: '2020', + delimiter: '.', + envelopeLists: {}, + path: 'Archive', + specialUse: ['archive'], + specialRole: 'archive', + mailboxes: [ + 347, + ], + }, + 347: { + accountId: 13, + databaseId: 347, + name: 'Archive.2020.08', + displayName: '08', + delimiter: '.', + envelopeLists: {}, + path: 'Archive.2020', + specialUse: ['archive'], + specialRole: 'archive', + mailboxes: [], + }, + }, + }) + }) + + it('adds a top level mailbox', () => { + const account = { + accountId: 13, + id: 13, + mailboxes: [ + 345, + ], + collapsed: true, + } + const state = { + accountList: [13], + accounts: { + 13: account, + }, + envelopes: {}, + mailboxes: { + 345: { + accountId: 13, + databaseId: 345, + name: 'Archive', + displayName: 'Archive', + delimiter: '.', + envelopeLists: {}, + path: '', + specialUse: ['archive'], + specialRole: 'archive', + mailboxes: [], + } + }, + } + + mutations.addMailbox( + state, + { + account, + mailbox: { + databaseId: 346, + name: 'Brchive', + delimiter: '.', + specialUse: ['archive'], + specialRole: 'archive', + } + }) + + expect(state).to.deep.equal({ + accountList: [13], + accounts: { + 13: { + accountId: 13, + id: 13, + mailboxes: [ + 345, + 346, + ], + collapsed: true, + }, + }, + envelopes: {}, + mailboxes: { + 345: { + accountId: 13, + databaseId: 345, + name: 'Archive', + displayName: 'Archive', + delimiter: '.', + envelopeLists: {}, + path: '', + specialUse: ['archive'], + specialRole: 'archive', + mailboxes: [], + }, + 346: { + accountId: 13, + databaseId: 346, + name: 'Brchive', + displayName: 'Brchive', + delimiter: '.', + envelopeLists: {}, + path: '', + specialUse: ['archive'], + specialRole: 'archive', + mailboxes: [], + }, + }, + }) + }) + + it('adds a sub-mailbox', () => { + const account = { + accountId: 13, + id: 13, + mailboxes: [ + 345, + ], + collapsed: true, + } + const state = { + accountList: [13], + accounts: { + 13: account, + }, + envelopes: {}, + mailboxes: { + 345: { + accountId: 13, + databaseId: 345, + name: 'Archive', + displayName: 'Archive', + delimiter: '.', + envelopeLists: {}, + path: '', + specialUse: ['archive'], + specialRole: 'archive', + mailboxes: [], + } + }, + } + + mutations.addMailbox( + state, + { + account, + mailbox: { + databaseId: 346, + name: 'Archive.2020', + delimiter: '.', + specialUse: ['archive'], + specialRole: 'archive', + } + }) + + expect(state).to.deep.equal({ + accountList: [13], + accounts: { + 13: { + accountId: 13, + id: 13, + mailboxes: [ + 345, + ], + collapsed: true, + }, + }, + envelopes: {}, + mailboxes: { + 345: { + accountId: 13, + databaseId: 345, + name: 'Archive', + displayName: 'Archive', + delimiter: '.', + envelopeLists: {}, + path: '', + specialUse: ['archive'], + specialRole: 'archive', + mailboxes: [ + 346, + ], + }, + 346: { + accountId: 13, + databaseId: 346, + name: 'Archive.2020', + displayName: '2020', + delimiter: '.', + envelopeLists: {}, + path: 'Archive', + specialUse: ['archive'], + specialRole: 'archive', + mailboxes: [], + }, + }, + }) + }) + + it('removes a mailbox', () => { + const state = { + accounts: { + 13: { + accountId: 13, + id: 13, + mailboxes: [27], + }, + }, + mailboxes: { + 27: { + accountId: 13, + specialUse: ['inbox'], + specialRole: 'inbox', + mailboxes: [], + }, + }, + } + + mutations.removeMailbox(state, { + id: 27, + }) + + expect(state).to.deep.equal({ + accounts: { + 13: { + accountId: 13, + id: 13, + mailboxes: [], + }, + }, + mailboxes: {}, + }) + }) + + it('removes a sub-mailbox', () => { + const state = { + accounts: { + 13: { + accountId: 13, + id: 13, + mailboxes: [27], + }, + }, + mailboxes: { + 27: { + accountId: 13, + specialUse: ['inbox'], + specialRole: 'inbox', + mailboxes: [28], + }, + 28: { + accountId: 13, + specialUse: ['inbox'], + specialRole: 'inbox', + mailboxes: [], + }, + }, + } + + mutations.removeMailbox(state, { + id: 28, + }) + + expect(state).to.deep.equal({ + accounts: { + 13: { + accountId: 13, + id: 13, + mailboxes: [27], + }, + }, + mailboxes: { + 27: { + accountId: 13, + specialUse: ['inbox'], + specialRole: 'inbox', + mailboxes: [], + }, + }, + }) + }) + it('adds envelopes', () => { const state = { accounts: { @@ -177,7 +679,6 @@ describe('Vuex store mutations', () => { }, mailboxes: { 27: { - id: 'INBOX', specialUse: ['inbox'], specialRole: 'inbox', envelopeLists: { @@ -224,7 +725,6 @@ describe('Vuex store mutations', () => { }, mailboxes: { 27: { - id: 'INBOX', specialUse: ['inbox'], specialRole: 'inbox', envelopeLists: { @@ -250,90 +750,4 @@ describe('Vuex store mutations', () => { }, }) }) - - it('removes a mailbox', () => { - const state = { - accounts: { - 13: { - accountId: 13, - id: 13, - mailboxes: [27], - }, - }, - mailboxes: { - 27: { - id: 'INBOX', - accountId: 13, - specialUse: ['inbox'], - specialRole: 'inbox', - }, - }, - } - - mutations.removeMailbox(state, { - id: 27, - }) - - expect(state).to.deep.equal({ - accounts: { - 13: { - accountId: 13, - id: 13, - mailboxes: [], - }, - }, - mailboxes: {}, - }) - }) - - it('removes a sub-mailbox', () => { - const state = { - accounts: { - 13: { - accountId: 13, - id: 13, - mailboxes: [27], - }, - }, - mailboxes: { - 27: { - id: 'INBOX', - accountId: 13, - specialUse: ['inbox'], - specialRole: 'inbox', - mailboxes: [28], - }, - 28: { - id: 'INBOX.sub', - accountId: 13, - specialUse: ['inbox'], - specialRole: 'inbox', - mailboxes: [], - }, - }, - } - - mutations.removeMailbox(state, { - id: 28, - }) - - expect(state).to.deep.equal({ - accounts: { - 13: { - accountId: 13, - id: 13, - mailboxes: [27], - }, - }, - mailboxes: { - 27: { - id: 'INBOX', - accountId: 13, - specialUse: ['inbox'], - specialRole: 'inbox', - mailboxes: [], - }, - }, - }) - }) }) From 9d1365ef776f816eb2da83fd34d2e9dd36d29c19 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Mon, 31 Aug 2020 15:42:59 +0200 Subject: [PATCH 2/2] Detect and persist the personal namespace Signed-off-by: Christoph Wurst --- appinfo/info.xml | 2 +- lib/Db/MailAccount.php | 5 ++ lib/IMAP/MailboxSync.php | 23 ++++++ .../Version1050Date20200831124954.php | 31 ++++++++ src/store/actions.js | 8 +- src/store/mutations.js | 20 ++++- src/tests/unit/store/actions.spec.js | 75 +++++++++++++++++++ src/tests/unit/store/mutations.spec.js | 74 ++++++++++++++++++ tests/Integration/Db/MailAccountTest.php | 2 + tests/Unit/IMAP/MailboxSyncTest.php | 4 + 10 files changed, 239 insertions(+), 5 deletions(-) create mode 100644 lib/Migration/Version1050Date20200831124954.php diff --git a/appinfo/info.xml b/appinfo/info.xml index 665b7cc409..913fd3afab 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -12,7 +12,7 @@ - **🙈 We’re not reinventing the wheel!** Based on the great [Horde](http://horde.org) libraries. - **📬 Want to host your own mail server?** We don’t have to reimplement this as you could set up [Mail-in-a-Box](https://mailinabox.email)! ]]> - 1.5.0-alpha2 + 1.5.0-alpha3 agpl Christoph Wurst Roeland Jago Douma diff --git a/lib/Db/MailAccount.php b/lib/Db/MailAccount.php index c63abe27ca..80b554c46b 100644 --- a/lib/Db/MailAccount.php +++ b/lib/Db/MailAccount.php @@ -69,6 +69,8 @@ * @method void setOrder(int $order) * @method bool getShowSubscribedOnly() * @method void setShowSubscribedOnly(bool $showSubscribedOnly) + * @method string|null getPersonalNamespace() + * @method void setPersonalNamespace(string|null $personalNamespace) */ class MailAccount extends Entity { protected $userId; @@ -90,6 +92,7 @@ class MailAccount extends Entity { protected $provisioned; protected $order; protected $showSubscribedOnly; + protected $personalNamespace; /** * @param array $params @@ -146,6 +149,7 @@ public function __construct(array $params=[]) { $this->addType('provisioned', 'bool'); $this->addType('order', 'integer'); $this->addType('showSubscribedOnly', 'boolean'); + $this->addType('personalNamespace', 'string'); } /** @@ -166,6 +170,7 @@ public function toJson() { 'editorMode' => $this->getEditorMode(), 'provisioned' => $this->getProvisioned(), 'showSubscribedOnly' => $this->getShowSubscribedOnly(), + 'personalNamespace' => $this->getPersonalNamespace(), ]; if (!is_null($this->getOutboundHost())) { diff --git a/lib/IMAP/MailboxSync.php b/lib/IMAP/MailboxSync.php index ad40934730..baeadae882 100644 --- a/lib/IMAP/MailboxSync.php +++ b/lib/IMAP/MailboxSync.php @@ -25,7 +25,10 @@ namespace OCA\Mail\IMAP; +use Horde_Imap_Client; +use Horde_Imap_Client_Data_Namespace; use Horde_Imap_Client_Exception; +use Horde_Imap_Client_Namespace_List; use OCA\Mail\Exception\ServiceException; use function in_array; use function json_encode; @@ -81,6 +84,16 @@ public function sync(Account $account, bool $force = false): void { } $client = $this->imapClientFactory->getClient($account); + try { + $namespaces = $client->getNamespaces([], [ + 'ob_return' => true, + ]); + $account->getMailAccount()->setPersonalNamespace( + $this->getPersonalNamespace($namespaces) + ); + } catch (Horde_Imap_Client_Exception $e) { + $this->logger->debug('Getting namespaces for account ' . $account->getId() . ' failed: ' . $e->getMessage()); + } try { $folders = $this->folderMapper->getFolders($account, $client); @@ -125,6 +138,16 @@ private function persist(Account $account, array $folders, array $existing): voi $this->mailAccountMapper->update($account->getMailAccount()); } + private function getPersonalNamespace(Horde_Imap_Client_Namespace_List $namespaces): ?string { + foreach ($namespaces as $namespace) { + /** @var Horde_Imap_Client_Data_Namespace $namespace */ + if ($namespace->type === Horde_Imap_Client::NS_PERSONAL) { + return $namespace->name; + } + } + return null; + } + private function updateMailboxFromFolder(Folder $folder, Mailbox $mailbox): void { $mailbox->setDelimiter($folder->getDelimiter()); $mailbox->setAttributes(json_encode($folder->getAttributes())); diff --git a/lib/Migration/Version1050Date20200831124954.php b/lib/Migration/Version1050Date20200831124954.php new file mode 100644 index 0000000000..f546005e39 --- /dev/null +++ b/lib/Migration/Version1050Date20200831124954.php @@ -0,0 +1,31 @@ +getTable('mail_accounts'); + $accountsTable->addColumn('personal_namespace', 'string', [ + 'notnull' => false, + ]); + + return $schema; + } +} diff --git a/src/store/actions.js b/src/store/actions.js index d0a62e26eb..a6d29a3b18 100644 --- a/src/store/actions.js +++ b/src/store/actions.js @@ -156,10 +156,14 @@ export default { commit('removeMailbox', { id: mailbox.databaseId }) }, async createMailbox({ commit }, { account, name }) { - const mailbox = await createMailbox(account.id, name) - console.debug(`mailbox ${name} created for account ${account.id}`, { mailbox }) + const prefixed = (account.personalNamespace && !name.startsWith(account.personalNamespace)) + ? account.personalNamespace + name + : name + const mailbox = await createMailbox(account.id, prefixed) + console.debug(`mailbox ${prefixed} created for account ${account.id}`, { mailbox }) commit('addMailbox', { account, mailbox }) commit('expandAccount', account.id) + return mailbox }, moveAccount({ commit, getters }, { account, up }) { const accounts = getters.accounts diff --git a/src/store/mutations.js b/src/store/mutations.js index 4d526b40fd..13a54cadcb 100644 --- a/src/store/mutations.js +++ b/src/store/mutations.js @@ -20,6 +20,7 @@ */ import { curry } from 'ramda' +import escapeRegExp from 'lodash/fp/escapeRegExp' import orderBy from 'lodash/fp/orderBy' import sortedUniqBy from 'lodash/fp/sortedUniqBy' import Vue from 'vue' @@ -34,11 +35,26 @@ const addMailboxToState = curry((state, account, mailbox) => { mailbox.envelopeLists = {} // Add all mailboxes (including submailboxes to state, but only toplevel to account - if (mailbox.name.includes(mailbox.delimiter)) { + const nameWithoutPrefix = account.personalNamespace + ? mailbox.name.replace(new RegExp(escapeRegExp(account.personalNamespace)), '') + : mailbox.name + if (nameWithoutPrefix.includes(mailbox.delimiter)) { + /** + * Sub-mailbox, e.g. 'Archive.2020' or 'INBOX.Archive.2020' + */ mailbox.displayName = mailbox.name.substr(mailbox.name.lastIndexOf(mailbox.delimiter) + 1) mailbox.path = mailbox.name.substr(0, mailbox.name.lastIndexOf(mailbox.delimiter)) + } else if (account.personalNamespace && mailbox.name.startsWith(account.personalNamespace)) { + /** + * Top-level mailbox, but with a personal namespace, e.g. 'INBOX.Sent' + */ + mailbox.displayName = nameWithoutPrefix + mailbox.path = account.personalNamespace } else { - mailbox.displayName = mailbox.name + /** + * Top-level mailbox, e.g. 'INBOX' or 'Draft' + */ + mailbox.displayName = nameWithoutPrefix mailbox.path = '' } diff --git a/src/tests/unit/store/actions.spec.js b/src/tests/unit/store/actions.spec.js index 9764c8c30a..68036661d0 100644 --- a/src/tests/unit/store/actions.spec.js +++ b/src/tests/unit/store/actions.spec.js @@ -24,6 +24,7 @@ import { curry, prop, range, reverse } from 'ramda' import orderBy from 'lodash/fp/orderBy' import actions from '../../../store/actions' +import * as MailboxService from '../../../service/MailboxService' import * as MessageService from '../../../service/MessageService' import * as NotificationService from '../../../service/NotificationService' import { UNIFIED_ACCOUNT_ID, UNIFIED_INBOX_ID } from '../../../store/constants' @@ -38,6 +39,8 @@ describe('Vuex store actions', () => { let context beforeEach(() => { + sinon.stub(MailboxService, 'create') + context = { commit: sinon.stub(), dispatch: sinon.stub(), @@ -55,6 +58,78 @@ describe('Vuex store actions', () => { sinon.restore() }) + it('creates a mailbox', async () => { + const account = { + id: 13, + personalNamespace: '', + } + const name = 'Important' + const mailbox = { + 'name': 'Important', + } + MailboxService.create.withArgs(13, 'Important').returns(mailbox) + + const result = await actions.createMailbox(context, {account, name}) + + expect(result).to.deep.equal(mailbox) + expect(context.commit).to.have.been.calledTwice + expect(context.commit).to.have.been.calledWith('addMailbox', { account, mailbox}) + }) + + it('creates a sub-mailbox', async () => { + const account = { + id: 13, + personalNamespace: '', + } + const name = 'Archive.2020' + const mailbox = { + 'name': 'Archive.2020', + } + MailboxService.create.withArgs(13, 'Archive.2020').returns(mailbox) + + const result = await actions.createMailbox(context, {account, name}) + + expect(result).to.deep.equal(mailbox) + expect(context.commit).to.have.been.calledTwice + expect(context.commit).to.have.been.calledWith('addMailbox', { account, mailbox}) + }) + + it('adds a prefix to new mailboxes if the account has a personal namespace', async () => { + const account = { + id: 13, + personalNamespace: 'INBOX.', + } + const name = 'Important' + const mailbox = { + 'name': 'INBOX.Important', + } + MailboxService.create.withArgs(13, 'INBOX.Important').returns(mailbox) + + const result = await actions.createMailbox(context, {account, name}) + + expect(result).to.deep.equal(mailbox) + expect(context.commit).to.have.been.calledTwice + expect(context.commit).to.have.been.calledWith('addMailbox', { account, mailbox}) + }) + + it('adds no prefix to new sub-mailboxes if the account has a personal namespace', async () => { + const account = { + id: 13, + personalNamespace: 'INBOX.', + } + const name = 'INBOX.Archive.2020' + const mailbox = { + 'name': 'INBOX.Archive.2020', + } + MailboxService.create.withArgs(13, 'INBOX.Archive.2020').returns(mailbox) + + const result = await actions.createMailbox(context, {account, name}) + + expect(result).to.deep.equal(mailbox) + expect(context.commit).to.have.been.calledTwice + expect(context.commit).to.have.been.calledWith('addMailbox', { account, mailbox}) + }) + it('combines unified inbox even if no inboxes are present', () => { context.getters.getMailbox.returns({ isUnified: true, diff --git a/src/tests/unit/store/mutations.spec.js b/src/tests/unit/store/mutations.spec.js index 8616474f3e..7d7a6eb49b 100644 --- a/src/tests/unit/store/mutations.spec.js +++ b/src/tests/unit/store/mutations.spec.js @@ -104,6 +104,80 @@ describe('Vuex store mutations', () => { }) }) + it('adds an account with a personal namespace', () => { + const state = { + accountList: [], + accounts: {}, + envelopes: {}, + mailboxes: {}, + } + + mutations.addAccount(state, { + accountId: 13, + id: 13, + mailboxes: [ + { + databaseId: 345, + name: 'INBOX', + delimiter: '.', + specialUse: ['inbox'], + specialRole: 'inbox', + }, + { + databaseId: 346, + name: 'INBOX.Sent', + delimiter: '.', + specialUse: ['sent'], + specialRole: 'sent', + }, + ], + personalNamespace: 'INBOX.', + }) + + expect(state).to.deep.equal({ + accountList: [13], + accounts: { + 13: { + accountId: 13, + id: 13, + mailboxes: [ + 345, + 346, + ], + collapsed: true, + personalNamespace: 'INBOX.', + }, + }, + envelopes: {}, + mailboxes: { + 345: { + accountId: 13, + databaseId: 345, + name: 'INBOX', + displayName: 'INBOX', + specialUse: ['inbox'], + specialRole: 'inbox', + delimiter: '.', + envelopeLists: {}, + path: '', + mailboxes: [], + }, + 346: { + accountId: 13, + databaseId: 346, + name: 'INBOX.Sent', + displayName: 'Sent', + specialUse: ['sent'], + specialRole: 'sent', + delimiter: '.', + envelopeLists: {}, + path: 'INBOX.', + mailboxes: [], + } + }, + }) + }) + it('adds an account with two levels of mailboxes', () => { const state = { accountList: [], diff --git a/tests/Integration/Db/MailAccountTest.php b/tests/Integration/Db/MailAccountTest.php index 5958687548..c05f4c342f 100644 --- a/tests/Integration/Db/MailAccountTest.php +++ b/tests/Integration/Db/MailAccountTest.php @@ -65,6 +65,7 @@ public function testToAPI() { 'provisioned' => false, 'order' => 13, 'showSubscribedOnly' => null, + 'personalNamespace' => null, ], $a->toJson()); } @@ -87,6 +88,7 @@ public function testMailAccountConstruct() { 'provisioned' => false, 'order' => null, 'showSubscribedOnly' => null, + 'personalNamespace' => null, ]; $a = new MailAccount($expected); // TODO: fix inconsistency diff --git a/tests/Unit/IMAP/MailboxSyncTest.php b/tests/Unit/IMAP/MailboxSyncTest.php index fb8db39abe..c8b26b8592 100644 --- a/tests/Unit/IMAP/MailboxSyncTest.php +++ b/tests/Unit/IMAP/MailboxSyncTest.php @@ -23,6 +23,7 @@ namespace OCA\Mail\Tests\Unit\IMAP; use ChristophWurst\Nextcloud\Testing\TestCase; +use Horde_Imap_Client_Exception; use Horde_Imap_Client_Socket; use OC\AppFramework\Utility\TimeFactory; use OCA\Mail\Account; @@ -103,6 +104,9 @@ public function testSync() { ->method('getClient') ->with($account) ->willReturn($client); + $client->expects($this->once()) + ->method('getNamespaces') + ->willThrowException(new Horde_Imap_Client_Exception()); $folders = [ $this->createMock(Folder::class), $this->createMock(Folder::class),