diff --git a/lib/Contracts/IMailSearch.php b/lib/Contracts/IMailSearch.php index 161aadaa40..1fd82feb65 100644 --- a/lib/Contracts/IMailSearch.php +++ b/lib/Contracts/IMailSearch.php @@ -34,6 +34,8 @@ use OCP\IUser; interface IMailSearch { + public const ORDER_NEWEST_FIRST = 'DESC'; + public const ORDER_OLDEST_FIRST = 'ASC'; /** * @throws DoesNotExistException * @throws ClientException @@ -46,6 +48,7 @@ public function findMessage(Account $account, /** * @param Account $account * @param Mailbox $mailbox + * @param string $sortOrder * @param string|null $filter * @param int|null $cursor * @param int|null $limit @@ -57,6 +60,7 @@ public function findMessage(Account $account, */ public function findMessages(Account $account, Mailbox $mailbox, + string $sortOrder, ?string $filter, ?int $cursor, ?int $limit): array; diff --git a/lib/Controller/AccountsController.php b/lib/Controller/AccountsController.php index 0ddcefc168..5053758d72 100644 --- a/lib/Controller/AccountsController.php +++ b/lib/Controller/AccountsController.php @@ -461,6 +461,7 @@ public function draft(int $id, $draftsMailbox, Horde_Imap_Client::SYNC_NEWMSGSUIDS, [], + null, false ); return new JSONResponse([ diff --git a/lib/Controller/MailboxesController.php b/lib/Controller/MailboxesController.php index ede4181c63..205a5145ae 100644 --- a/lib/Controller/MailboxesController.php +++ b/lib/Controller/MailboxesController.php @@ -27,6 +27,7 @@ use Horde_Imap_Client; use OCA\Mail\Contracts\IMailManager; +use OCA\Mail\Contracts\IMailSearch; use OCA\Mail\Exception\ClientException; use OCA\Mail\Exception\IncompleteSyncException; use OCA\Mail\Exception\MailboxNotCachedException; @@ -145,9 +146,10 @@ public function patch(int $id, * @throws ServiceException */ #[TrapError] - public function sync(int $id, array $ids = [], bool $init = false, string $query = null): JSONResponse { + public function sync(int $id, array $ids = [], ?int $lastMessageTimestamp, bool $init = false, string $sortOrder = 'newest', string $query = null): JSONResponse { $mailbox = $this->mailManager->getMailbox($this->currentUserId, $id); $account = $this->accountService->find($this->currentUserId, $mailbox->getAccountId()); + $order = $sortOrder === 'newest' ? IMailSearch::ORDER_NEWEST_FIRST: IMailSearch::ORDER_OLDEST_FIRST; try { $syncResponse = $this->syncService->syncMailbox( @@ -157,7 +159,9 @@ public function sync(int $id, array $ids = [], bool $init = false, string $query array_map(static function ($id) { return (int)$id; }, $ids), + $lastMessageTimestamp, !$init, + $order, $query ); } catch (MailboxNotCachedException $e) { diff --git a/lib/Controller/MessagesController.php b/lib/Controller/MessagesController.php index 66ff383057..ef82a9db62 100755 --- a/lib/Controller/MessagesController.php +++ b/lib/Controller/MessagesController.php @@ -39,6 +39,7 @@ use OCA\Mail\Contracts\IMailSearch; use OCA\Mail\Contracts\IMailTransmission; use OCA\Mail\Contracts\ITrustedSenderService; +use OCA\Mail\Contracts\IUserPreferences; use OCA\Mail\Db\Message; use OCA\Mail\Exception\ClientException; use OCA\Mail\Exception\ServiceException; @@ -87,6 +88,7 @@ class MessagesController extends Controller { private SmimeService $smimeService; private IMAPClientFactory $clientFactory; private IDkimService $dkimService; + private IUserPreferences $preferences; private SnoozeService $snoozeService; public function __construct(string $appName, @@ -107,6 +109,7 @@ public function __construct(string $appName, SmimeService $smimeService, IMAPClientFactory $clientFactory, IDkimService $dkimService, + IUserPreferences $preferences, SnoozeService $snoozeService) { parent::__construct($appName, $request); $this->accountService = $accountService; @@ -125,6 +128,7 @@ public function __construct(string $appName, $this->smimeService = $smimeService; $this->clientFactory = $clientFactory; $this->dkimService = $dkimService; + $this->preferences = $preferences; $this->snoozeService = $snoozeService; } @@ -153,12 +157,14 @@ public function index(int $mailboxId, return new JSONResponse([], Http::STATUS_FORBIDDEN); } - $this->logger->debug("loading messages of folder <$mailboxId>"); + $this->logger->debug("loading messages of mailbox <$mailboxId>"); + $order = $this->preferences->getPreference($this->currentUserId, 'sort-order', 'newest') === 'newest' ? 'DESC': 'ASC'; return new JSONResponse( $this->mailSearch->findMessages( $account, $mailbox, + $order, $filter === '' ? null : $filter, $cursor, $limit diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index 7c470ae868..8eab8b5480 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -170,6 +170,11 @@ public function index(): TemplateResponse { $this->tagMapper->getAllTagsForUser($this->currentUserId) ); + $this->initialStateService->provideInitialState( + 'sort-order', + $this->preferences->getPreference($this->currentUserId, 'sort-order', 'newest') + ); + try { $password = $this->credentialStore->getLoginCredentials()->getPassword(); $passwordIsUnavailable = $password === null || $password === ''; diff --git a/lib/Db/MessageMapper.php b/lib/Db/MessageMapper.php index 4336bf6484..25ee162132 100644 --- a/lib/Db/MessageMapper.php +++ b/lib/Db/MessageMapper.php @@ -29,6 +29,7 @@ use OCA\Mail\Account; use OCA\Mail\Address; use OCA\Mail\AddressList; +use OCA\Mail\Contracts\IMailSearch; use OCA\Mail\IMAP\Threading\DatabaseMessage; use OCA\Mail\Service\Search\Flag; use OCA\Mail\Service\Search\FlagExpression; @@ -700,7 +701,7 @@ public function findByMessageId(Account $account, string $messageId): array { * * @return int[] */ - public function findIdsByQuery(Mailbox $mailbox, SearchQuery $query, ?int $limit, array $uids = null): array { + public function findIdsByQuery(Mailbox $mailbox, SearchQuery $query, string $sortOrder, ?int $limit, array $uids = null): array { $qb = $this->db->getQueryBuilder(); if ($this->needDistinct($query)) { @@ -828,10 +829,14 @@ public function findIdsByQuery(Mailbox $mailbox, SearchQuery $query, ?int $limit ); } - if ($query->getCursor() !== null) { + if ($query->getCursor() !== null && $sortOrder === IMailSearch::ORDER_NEWEST_FIRST) { $select->andWhere( $qb->expr()->lt('m.sent_at', $qb->createNamedParameter($query->getCursor(), IQueryBuilder::PARAM_INT)) ); + } elseif ($query->getCursor() !== null && $sortOrder === IMailSearch::ORDER_OLDEST_FIRST) { + $select->andWhere( + $qb->expr()->gt('m.sent_at', $qb->createNamedParameter($query->getCursor(), IQueryBuilder::PARAM_INT)) + ); } // createParameter @@ -861,7 +866,11 @@ public function findIdsByQuery(Mailbox $mailbox, SearchQuery $query, ?int $limit $select->andWhere($qb->expr()->isNull('m2.id')); - $select->orderBy('m.sent_at', 'desc'); + if ($sortOrder === 'ASC') { + $select->orderBy('sent_at', $sortOrder); + } else { + $select->orderBy('sent_at', 'DESC'); + } if ($limit !== null) { $select->setMaxResults($limit); @@ -876,9 +885,10 @@ public function findIdsByQuery(Mailbox $mailbox, SearchQuery $query, ?int $limit }, array_chunk($uids, 1000)); } - return array_map(static function (Message $message) { + $result = array_map(static function (Message $message) { return $message->getId(); }, $this->findEntities($select)); + return $result; } public function findIdsGloballyByQuery(IUser $user, SearchQuery $query, ?int $limit, array $uids = null): array { @@ -1115,21 +1125,21 @@ public function findByMailboxAndIds(Mailbox $mailbox, string $userId, array $ids /** * @param string $userId * @param int[] $ids + * @param string $sortOrder * * @return Message[] */ - public function findByIds(string $userId, array $ids): array { + public function findByIds(string $userId, array $ids, string $sortOrder): array { if ($ids === []) { return []; } - $qb = $this->db->getQueryBuilder(); $qb->select('*') ->from($this->getTableName()) ->where( $qb->expr()->in('id', $qb->createParameter('ids')) ) - ->orderBy('sent_at', 'desc'); + ->orderBy('sent_at', $sortOrder); $results = []; foreach (array_chunk($ids, 1000) as $chunk) { @@ -1215,14 +1225,19 @@ public function findRelatedData(array $messages, string $userId): array { /** * @param Mailbox $mailbox * @param array $ids + * @param int|null $lastMessageTimestamp + * @param IMailSearch::ORDER_* $sortOrder + * * @return int[] */ - public function findNewIds(Mailbox $mailbox, array $ids): array { + public function findNewIds(Mailbox $mailbox, array $ids, ?int $lastMessageTimestamp, string $sortOrder): array { $select = $this->db->getQueryBuilder(); $subSelect = $this->db->getQueryBuilder(); $subSelect - ->select($subSelect->func()->min('sent_at')) + ->select($sortOrder === IMailSearch::ORDER_NEWEST_FIRST ? + $subSelect->func()->min('sent_at') : + $subSelect->func()->max('sent_at')) ->from($this->getTableName()) ->where( $subSelect->expr()->eq('mailbox_id', $select->createNamedParameter($mailbox->getId(), IQueryBuilder::PARAM_INT)), @@ -1234,20 +1249,31 @@ public function findNewIds(Mailbox $mailbox, array $ids): array { $selfJoin = $select->expr()->andX( $select->expr()->eq('m.mailbox_id', 'm2.mailbox_id', IQueryBuilder::PARAM_INT), $select->expr()->eq('m.thread_root_id', 'm2.thread_root_id', IQueryBuilder::PARAM_INT), - $select->expr()->lt('m.sent_at', 'm2.sent_at', IQueryBuilder::PARAM_INT) + $sortOrder === IMailSearch::ORDER_NEWEST_FIRST ? + $select->expr()->lt('m.sent_at', 'm2.sent_at', IQueryBuilder::PARAM_INT) : + $select->expr()->gt('m.sent_at', 'm2.sent_at', IQueryBuilder::PARAM_INT) ); + $wheres = [$select->expr()->eq('m.mailbox_id', $select->createNamedParameter($mailbox->getId(), IQueryBuilder::PARAM_INT)), + $select->expr()->andX($subSelect->expr()->notIn('m.id', $select->createParameter('ids'), IQueryBuilder::PARAM_INT_ARRAY)), + $select->expr()->isNull('m2.id'), + ]; + if ($sortOrder === IMailSearch::ORDER_NEWEST_FIRST) { + $wheres[] = $select->expr()->gt('m.sent_at', $select->createFunction('(' . $subSelect->getSQL() . ')'), IQueryBuilder::PARAM_INT); + } else { + $wheres[] = $select->expr()->lt('m.sent_at', $select->createFunction('(' . $subSelect->getSQL() . ')'), IQueryBuilder::PARAM_INT); + } + + if ($lastMessageTimestamp !== null && $sortOrder === IMailSearch::ORDER_OLDEST_FIRST) { + // Don't consider old "new messages" as new when their UID has already been seen before + $wheres[] = $select->expr()->lt('m.sent_at', $select->createNamedParameter($lastMessageTimestamp, IQueryBuilder::PARAM_INT)); + } $select - ->select('m.id') + ->select(['m.id', 'm.sent_at']) ->from($this->getTableName(), 'm') ->leftJoin('m', $this->getTableName(), 'm2', $selfJoin) - ->where( - $select->expr()->eq('m.mailbox_id', $select->createNamedParameter($mailbox->getId(), IQueryBuilder::PARAM_INT)), - $select->expr()->andX($subSelect->expr()->notIn('m.id', $select->createParameter('ids'), IQueryBuilder::PARAM_INT_ARRAY)), - $select->expr()->isNull('m2.id'), - $select->expr()->gt('m.sent_at', $select->createFunction('(' . $subSelect->getSQL() . ')'), IQueryBuilder::PARAM_INT) - ) - ->orderBy('m.sent_at', 'desc'); + ->where(...$wheres) + ->orderBy('m.sent_at', $sortOrder === IMailSearch::ORDER_NEWEST_FIRST ? 'desc' : 'asc'); $results = []; foreach (array_chunk($ids, 1000) as $chunk) { diff --git a/lib/IMAP/Sync/Synchronizer.php b/lib/IMAP/Sync/Synchronizer.php index d179722fbb..b9011a2101 100644 --- a/lib/IMAP/Sync/Synchronizer.php +++ b/lib/IMAP/Sync/Synchronizer.php @@ -101,7 +101,7 @@ public function sync(Horde_Imap_Client_Base $imapClient, $changedMessages = $this->messageMapper->findByIds($imapClient, $request->getMailbox(), $changedUids, $userId); $vanishedMessageUids = $vanishedUids; - return new Response($newMessages, $changedMessages, $vanishedMessageUids); + return new Response($newMessages, $changedMessages, $vanishedMessageUids, null); } /** diff --git a/lib/Service/Search/MailSearch.php b/lib/Service/Search/MailSearch.php index c4068efaee..5be595b6e3 100644 --- a/lib/Service/Search/MailSearch.php +++ b/lib/Service/Search/MailSearch.php @@ -92,6 +92,7 @@ public function findMessage(Account $account, /** * @param Account $account * @param Mailbox $mailbox + * @param string $sortOrder * @param string|null $filter * @param int|null $cursor * @param int|null $limit @@ -103,6 +104,7 @@ public function findMessage(Account $account, */ public function findMessages(Account $account, Mailbox $mailbox, + string $sortOrder, ?string $filter, ?int $cursor, ?int $limit): array { @@ -130,7 +132,8 @@ public function findMessages(Account $account, $account, $mailbox, $this->messageMapper->findByIds($account->getUserId(), - $this->getIdsLocally($account, $mailbox, $query, $limit) + $this->getIdsLocally($account, $mailbox, $query, $sortOrder, $limit), + $sortOrder, ) ); } @@ -155,7 +158,8 @@ public function findMessagesGlobally(IUser $user, } return $this->messageMapper->findByIds($user->getUID(), - $this->getIdsGlobally($user, $query, $limit) + $this->getIdsGlobally($user, $query, $limit), + 'DESC' ); } @@ -164,9 +168,9 @@ public function findMessagesGlobally(IUser $user, * * @throws ServiceException */ - private function getIdsLocally(Account $account, Mailbox $mailbox, SearchQuery $query, ?int $limit): array { + private function getIdsLocally(Account $account, Mailbox $mailbox, SearchQuery $query, string $sortOrder, ?int $limit): array { if (empty($query->getBodies())) { - return $this->messageMapper->findIdsByQuery($mailbox, $query, $limit); + return $this->messageMapper->findIdsByQuery($mailbox, $query, $sortOrder, $limit); } $fromImap = $this->imapSearchProvider->findMatches( @@ -174,7 +178,7 @@ private function getIdsLocally(Account $account, Mailbox $mailbox, SearchQuery $ $mailbox, $query ); - return $this->messageMapper->findIdsByQuery($mailbox, $query, $limit, $fromImap); + return $this->messageMapper->findIdsByQuery($mailbox, $query, $sortOrder, $limit, $fromImap); } /** diff --git a/lib/Service/Sync/SyncService.php b/lib/Service/Sync/SyncService.php index 01b465abb4..dd18b2dc27 100644 --- a/lib/Service/Sync/SyncService.php +++ b/lib/Service/Sync/SyncService.php @@ -26,6 +26,7 @@ namespace OCA\Mail\Service\Sync; use OCA\Mail\Account; +use OCA\Mail\Contracts\IMailSearch; use OCA\Mail\Db\Mailbox; use OCA\Mail\Db\MailboxMapper; use OCA\Mail\Db\Message; @@ -111,7 +112,9 @@ public function syncMailbox(Account $account, Mailbox $mailbox, int $criteria, array $knownIds = null, + ?int $lastMessageTimestamp, bool $partialOnly, + string $sortOrder = IMailSearch::ORDER_NEWEST_FIRST, string $filter = null): Response { if ($partialOnly && !$mailbox->isCached()) { throw MailboxNotCachedException::from($mailbox); @@ -133,6 +136,8 @@ public function syncMailbox(Account $account, $account, $mailbox, $knownIds ?? [], + $lastMessageTimestamp, + $sortOrder, $query ); } @@ -150,24 +155,26 @@ public function syncMailbox(Account $account, private function getDatabaseSyncChanges(Account $account, Mailbox $mailbox, array $knownIds, + ?int $lastMessageTimestamp, + string $sortOrder, ?SearchQuery $query): Response { if ($knownIds === []) { $newIds = $this->messageMapper->findAllIds($mailbox); } else { - $newIds = $this->messageMapper->findNewIds($mailbox, $knownIds); + $newIds = $this->messageMapper->findNewIds($mailbox, $knownIds, $lastMessageTimestamp, $sortOrder); } - + $order = $sortOrder === 'oldest' ? IMailSearch::ORDER_OLDEST_FIRST : IMailSearch::ORDER_NEWEST_FIRST; if ($query !== null) { // Filter new messages to those that also match the current filter $newUids = $this->messageMapper->findUidsForIds($mailbox, $newIds); - $newIds = $this->messageMapper->findIdsByQuery($mailbox, $query, null, $newUids); + $newIds = $this->messageMapper->findIdsByQuery($mailbox, $query, $order, null, $newUids); } $new = $this->messageMapper->findByMailboxAndIds($mailbox, $account->getUserId(), $newIds); // TODO: $changed = $this->messageMapper->findChanged($account, $mailbox, $uids); if ($query !== null) { $changedUids = $this->messageMapper->findUidsForIds($mailbox, $knownIds); - $changedIds = $this->messageMapper->findIdsByQuery($mailbox, $query, null, $changedUids); + $changedIds = $this->messageMapper->findIdsByQuery($mailbox, $query, $order, null, $changedUids); } else { $changedIds = $knownIds; } diff --git a/src/components/AppSettingsMenu.vue b/src/components/AppSettingsMenu.vue index 4d28f7a0ff..274c3c1f6e 100755 --- a/src/components/AppSettingsMenu.vue +++ b/src/components/AppSettingsMenu.vue @@ -99,6 +99,29 @@ +

+ Sorting +

+
+ + {{ t('mail', 'Newest') }} + + + {{ t('mail', 'Oldest') }} + +

{{ t('mail', 'Looking for a way to encrypt your emails?') }} @@ -116,7 +139,7 @@ import { generateUrl } from '@nextcloud/router' import { showError } from '@nextcloud/dialogs' -import { NcButton as ButtonVue, NcLoadingIcon as IconLoading } from '@nextcloud/vue' +import { NcButton as ButtonVue, NcLoadingIcon as IconLoading, NcCheckboxRadioSwitch as CheckboxRadioSwitch } from '@nextcloud/vue' import IconInfo from 'vue-material-design-icons/Information' import IconAdd from 'vue-material-design-icons/Plus' @@ -137,6 +160,7 @@ export default { IconLoading, IconLock, SmimeCertificateModal, + CheckboxRadioSwitch, }, data() { return { @@ -152,6 +176,7 @@ export default { autoTaggingText: t('mail', 'Automatically classify importance of new email'), toggleAutoTagging: false, displaySmimeCertificateModal: false, + sortOrder: 'newest', } }, computed: { @@ -171,6 +196,9 @@ export default { return this.$store.getters.getPreference('allow-new-accounts', true) }, }, + mounted() { + this.sortOrder = this.$store.getters.getPreference('sort-order', 'newest') + }, methods: { onToggleButtonReplies(e) { this.loadingReplySettings = true @@ -211,6 +239,23 @@ export default { this.loadingOptOutSettings = false }) }, + async onSortByDate(e) { + const previousValue = this.sortOrder + try { + this.sortOrder = e + await this.$store + .dispatch('savePreference', { + key: 'sort-order', + value: e, + }) + this.$store.commit('removeAllEnvelopes') + + } catch (error) { + Logger.error('could not save preferences', { error }) + this.sortOrder = previousValue + showError(t('mail', 'Could not update preference')) + } + }, async onToggleAutoTagging(e) { this.toggleAutoTagging = true @@ -310,4 +355,21 @@ p.app-settings { } } } +.material-design-icon { + &.lock-icon { + margin-right: 10px; + } + +} +.section-title { + margin-top: 20px; + margin-bottom: 10px; +} +.sorting { + display: flex; + width: 100%; + &__switch{ + width: 50%; + } +} diff --git a/src/components/EnvelopeList.vue b/src/components/EnvelopeList.vue index feebe04c29..2185249cbb 100644 --- a/src/components/EnvelopeList.vue +++ b/src/components/EnvelopeList.vue @@ -236,7 +236,7 @@ { + return a.dateInt < b.dateInt ? -1 : 1 + }) + } + return [...this.envelopes] + }, selectMode() { // returns true when in selection mode (where the user selects several emails at once) return this.selection.length > 0 @@ -388,15 +400,15 @@ export default { return this.selectedEnvelopes.every((env) => env.flags.flagged === true) }, selectedEnvelopes() { - return this.envelopes.filter((env) => this.selection.includes(env.databaseId)) + return this.sortedEnvelops.filter((env) => this.selection.includes(env.databaseId)) }, hasMultipleAccounts() { - const mailboxIds = this.envelopes.map(envelope => envelope.mailboxId) + const mailboxIds = this.sortedEnvelops.map(envelope => envelope.mailboxId) return Array.from(new Set(mailboxIds)).length > 1 }, }, watch: { - envelopes(newVal, oldVal) { + sortedEnvelops(newVal, oldVal) { // Unselect vanished envelopes const newIds = newVal.map((env) => env.databaseId) this.selection = this.selection.filter((id) => newIds.includes(id)) @@ -484,7 +496,7 @@ export default { let nextEnvelopeToNavigate let isAllSelected - if (this.selectedEnvelopes.length === this.envelopes.length) { + if (this.selectedEnvelopes.length === this.sortedEnvelops.length) { isAllSelected = true } else { const indexSelectedEnvelope = this.selectedEnvelopes.findIndex((selectedEnvelope) => @@ -493,7 +505,7 @@ export default { // one of threads is selected if (indexSelectedEnvelope !== -1) { const lastSelectedEnvelope = this.selectedEnvelopes[this.selectedEnvelopes.length - 1] - const diff = this.envelopes.filter(envelope => envelope === lastSelectedEnvelope || !this.selectedEnvelopes.includes(envelope)) + const diff = this.sortedEnvelops.filter(envelope => envelope === lastSelectedEnvelope || !this.selectedEnvelopes.includes(envelope)) const lastIndex = diff.indexOf(lastSelectedEnvelope) nextEnvelopeToNavigate = diff[lastIndex === 0 ? 1 : lastIndex - 1] } @@ -563,12 +575,12 @@ export default { const end = Math.max(this.lastToggledIndex, index) const selected = this.selection.includes(envelope.databaseId) for (let i = start; i <= end; i++) { - this.setEnvelopeSelected(this.envelopes[i], !selected) + this.setEnvelopeSelected(this.sortedEnvelops[i], !selected) } this.lastToggledIndex = index }, unselectAll() { - this.envelopes.forEach((env) => { + this.sortedEnvelops.forEach((env) => { env.flags.selected = false }) this.selection = [] diff --git a/src/components/Mailbox.vue b/src/components/Mailbox.vue index c94253a732..3206bd14f7 100644 --- a/src/components/Mailbox.vue +++ b/src/components/Mailbox.vue @@ -121,6 +121,9 @@ export default { } }, computed: { + sortOrder() { + return this.$store.getters.getPreference('sort-order', 'DESC') + }, envelopes() { return this.$store.getters.getEnvelopes(this.mailbox.databaseId, this.searchQuery) }, @@ -148,6 +151,9 @@ export default { searchQuery() { this.loadEnvelopes() }, + sortOrder() { + this.loadEnvelopes() + }, }, created() { this.bus.$on('load-more', this.onScroll) diff --git a/src/main.js b/src/main.js index 7ba685ea3e..8bb6fe6e5a 100644 --- a/src/main.js +++ b/src/main.js @@ -66,6 +66,12 @@ store.commit('savePreference', { key: 'ncVersion', value: loadState('mail', 'ncVersion'), }) + +store.commit('savePreference', { + key: 'sort-order', + value: loadState('mail', 'sort-order', 'newest'), +}) + store.commit('savePreference', { key: 'attachment-size-limit', value: Number.parseInt(getPreferenceFromPage('attachment-size-limit'), 10), diff --git a/src/service/MessageService.js b/src/service/MessageService.js index 0faf1e0cf8..1358b02877 100644 --- a/src/service/MessageService.js +++ b/src/service/MessageService.js @@ -62,7 +62,7 @@ export const fetchThread = async (id) => { return resp.data } -export async function syncEnvelopes(accountId, id, ids, query, init = false) { +export async function syncEnvelopes(accountId, id, ids, lastMessageTimestamp, query, init = false, sortOrder) { const url = generateUrl('/apps/mail/api/mailboxes/{id}/sync', { id, }) @@ -70,8 +70,10 @@ export async function syncEnvelopes(accountId, id, ids, query, init = false) { try { const response = await axios.post(url, { ids, - query, + lastMessageTimestamp, init, + sortOrder, + query, }) if (response.status === 202) { diff --git a/src/store/actions.js b/src/store/actions.js index 076541b95d..01b8204c88 100644 --- a/src/store/actions.js +++ b/src/store/actions.js @@ -31,7 +31,7 @@ import { filter, flatten, gt, - head, + lt, identity, last, map, @@ -133,7 +133,13 @@ const findIndividualMailboxes = curry((getMailboxes, specialRole) => ) ) -const combineEnvelopeLists = pipe(flatten, orderBy(prop('dateInt'), 'desc')) +const combineEnvelopeLists = (sortOrder) => { + if (sortOrder === 'oldest') { + return pipe(flatten, orderBy(prop('dateInt'), 'asc')) + } + + return pipe(flatten, orderBy(prop('dateInt'), 'desc')) +} export default { savePreference({ commit, getters }, { key, value }) { @@ -533,8 +539,8 @@ export default { const envelope = await fetchEnvelope(accountId, id) // Only commit if not undefined (not found) if (envelope) { - commit('addEnvelope', { - envelope, + commit('addEnvelopes', { + envelopes: [envelope], }) } @@ -561,16 +567,14 @@ export default { const fetchUnifiedEnvelopes = pipe( findIndividualMailboxes(getters.getMailboxes, mailbox.specialRole), fetchIndividualLists, - andThen(combineEnvelopeLists), + andThen(combineEnvelopeLists(getters.getPreference('sort-order'))), andThen(sliceToPage), andThen( - tap( - map((envelope) => - commit('addEnvelope', { - envelope, - query, - }) - ) + tap((envelopes) => + commit('addEnvelopes', { + envelopes, + query, + }) ) ) ) @@ -581,17 +585,15 @@ export default { return pipe( fetchEnvelopes, andThen( - tap( - map((envelope) => - commit('addEnvelope', { - query, - envelope, - addToUnifiedMailboxes, - }) - ) + tap((envelopes) => + commit('addEnvelopes', { + query, + envelopes, + addToUnifiedMailboxes, + }) ) ) - )(mailbox.accountId, mailboxId, query, undefined, PAGE_SIZE) + )(mailbox.accountId, mailboxId, query, undefined, PAGE_SIZE, getters.getPreference('sort-order')) }) }, async fetchNextEnvelopePage({ commit, getters, dispatch }, { mailboxId, query }) { @@ -618,29 +620,38 @@ export default { if (cursor === undefined) { throw new Error('Unified list has no tail') } + const newestFirst = getters.getPreference('sort-order') === 'newest' const nextLocalUnifiedEnvelopes = pipe( findIndividualMailboxes(getters.getMailboxes, mailbox.specialRole), map(getIndivisualLists(query)), - combineEnvelopeLists, + combineEnvelopeLists(getters.getPreference('sort-order')), filter( where({ - dateInt: gt(cursor), + dateInt: newestFirst ? gt(cursor) : lt(cursor), }) ), slice(0, quantity) ) // We know the next envelopes based on local data // We have to fetch individual envelopes only if it ends in the known - // next fetch. If it ended before, there is no data to fetch anyway. If - // it ends after, we have all the relevant data already + // next fetch. If it ends after, we have all the relevant data already. const needsFetch = curry((query, nextEnvelopes, mb) => { const c = individualCursor(query, mb) - return nextEnvelopes.length < quantity || c >= head(nextEnvelopes).dateInt || c <= last(nextEnvelopes).dateInt + if (nextEnvelopes.length < quantity) { + return true + } + + if (getters.getPreference('sort-order') === 'newest') { + return c >= last(nextEnvelopes).dateInt + } else { + return c <= last(nextEnvelopes).dateInt + } }) const mailboxesToFetch = (accounts) => pipe( findIndividualMailboxes(getters.getMailboxes, mailbox.specialRole), + tap(mbs => console.info('individual mailboxes', mbs)), filter(needsFetch(query, nextLocalUnifiedEnvelopes(accounts))) )(accounts) const mbs = mailboxesToFetch(getters.accounts) @@ -665,7 +676,7 @@ export default { query, quantity, rec: false, - addToUnifiedMailboxes: false, + addToUnifiedMailboxes: true, }) ) )(mbs) @@ -673,12 +684,11 @@ export default { const envelopes = nextLocalUnifiedEnvelopes(getters.accounts) logger.debug('next unified page can be built locally and consists of ' + envelopes.length + ' envelopes', { addToUnifiedMailboxes }) - envelopes.map((envelope) => - commit('addEnvelope', { - query, - envelope, - }) - ) + commit('addEnvelopes', { + query, + envelopes, + addToUnifiedMailboxes, + }) return envelopes } @@ -697,18 +707,16 @@ export default { return Promise.reject(new Error('Cannot find last envelope. Required for the mailbox cursor')) } - return fetchEnvelopes(mailbox.accountId, mailboxId, query, lastEnvelope.dateInt, quantity).then((envelopes) => { + return fetchEnvelopes(mailbox.accountId, mailboxId, query, lastEnvelope.dateInt, quantity, getters.getPreference('sort-order')).then((envelopes) => { logger.debug(`fetched ${envelopes.length} messages for mailbox ${mailboxId}`, { envelopes, addToUnifiedMailboxes, }) - envelopes.forEach((envelope) => - commit('addEnvelope', { - query, - envelope, - addToUnifiedMailboxes, - }) - ) + commit('addEnvelopes', { + query, + envelopes, + addToUnifiedMailboxes, + }) return envelopes }) }) @@ -768,18 +776,20 @@ export default { } const ids = getters.getEnvelopes(mailboxId, query).map((env) => env.databaseId) - logger.debug(`mailbox sync of ${mailboxId} (${query}) has ${ids.length} known IDs`) - return syncEnvelopes(mailbox.accountId, mailboxId, ids, query, init) + const lastTimestamp = getters.getPreference('sort-order') === 'newest' ? null : getters.getEnvelopes(mailboxId, query)[0]?.dateInt + logger.debug(`mailbox sync of ${mailboxId} (${query}) has ${ids.length} known IDs. ${lastTimestamp} is the last known message timestamp`, { mailbox }) + return syncEnvelopes(mailbox.accountId, mailboxId, ids, lastTimestamp, query, init, getters.getPreference('sort-order')) .then((syncData) => { logger.debug(`mailbox ${mailboxId} (${query}) synchronized, ${syncData.newMessages.length} new, ${syncData.changedMessages.length} changed and ${syncData.vanishedMessages.length} vanished messages`) const unifiedMailbox = getters.getUnifiedMailbox(mailbox.specialRole) + commit('addEnvelopes', { + envelopes: syncData.newMessages, + query, + }) + syncData.newMessages.forEach((envelope) => { - commit('addEnvelope', { - envelope, - query, - }) if (unifiedMailbox) { commit('updateEnvelope', { envelope, @@ -999,7 +1009,7 @@ export default { console.error('could not toggle message junk state', error) if (removeEnvelope) { - commit('addEnvelope', envelope) + commit('addEnvelopes', [envelope]) } // Revert change @@ -1119,7 +1129,7 @@ export default { console.error('could not delete message', err) const envelope = getters.getEnvelope(id) if (envelope) { - commit('addEnvelope', { envelope }) + commit('addEnvelopes', { envelopes: [envelope] }) } else { logger.error('could not find envelope', { id }) } @@ -1285,7 +1295,7 @@ export default { await ThreadService.deleteThread(envelope.databaseId) console.debug('thread removed') } catch (e) { - commit('addEnvelope', envelope) + commit('addEnvelopes', { envelopes: [envelope] }) console.error('could not delete thread', e) throw e } @@ -1299,7 +1309,7 @@ export default { await ThreadService.moveThread(envelope.databaseId, destMailboxId) console.debug('thread removed') } catch (e) { - commit('addEnvelope', envelope) + commit('addEnvelopes', { envelopes: [envelope] }) console.error('could not move thread', e) throw e } @@ -1311,6 +1321,7 @@ export default { await ThreadService.snoozeThread(envelope.databaseId, unixTimestamp, destMailboxId) console.debug('thread snoozed') } catch (e) { + commit('addEnvelopes', { envelopes: [envelope] }) console.error('could not snooze thread', e) throw e } diff --git a/src/store/mutations.js b/src/store/mutations.js index 20e3376120..d22c55cf36 100644 --- a/src/store/mutations.js +++ b/src/store/mutations.js @@ -106,27 +106,6 @@ const normalizeTags = (state, envelope) => { Vue.set(envelope, 'tags', tags) } -/** - * Append or replace an envelope id for an existing message list - * - * If the given thread root id exist the message is replaced - * otherwise appended - * - * @param {object} state vuex state - * @param {Array} existing list of envelope ids for a message list - * @param {object} envelope envelope with tag objects - * @return {Array} list of envelope ids - */ -const appendOrReplaceEnvelopeId = (state, existing, envelope) => { - const index = existing.findIndex((id) => state.envelopes[id].threadRootId === envelope.threadRootId) - if (index === -1) { - existing.push(envelope.databaseId) - } else { - existing[index] = envelope.databaseId - } - return existing -} - export default { savePreference(state, { key, value }) { Vue.set(state.preferences, key, value) @@ -279,34 +258,39 @@ export default { Vue.set(state.newMessage, 'type', 'outbox') Vue.set(state.newMessage.data, 'id', message.id) }, - addEnvelope(state, { query, envelope, addToUnifiedMailboxes = true }) { - normalizeTags(state, envelope) - const mailbox = state.mailboxes[envelope.mailboxId] - Vue.set(state.envelopes, envelope.databaseId, Object.assign({}, state.envelopes[envelope.databaseId] || {}, envelope)) - Vue.set(envelope, 'accountId', mailbox.accountId) + addEnvelopes(state, { query, envelopes, addToUnifiedMailboxes = true }) { + if (envelopes.length === 0) { + return + } + const mailbox = state.mailboxes[envelopes[0].mailboxId] const idToDateInt = (id) => state.envelopes[id].dateInt - const orderByDateInt = orderBy(idToDateInt, 'desc') const listId = normalizedEnvelopeListId(query) - const existing = mailbox.envelopeLists[listId] || [] - Vue.set(mailbox.envelopeLists, listId, uniq(orderByDateInt(appendOrReplaceEnvelopeId(state, existing, envelope)))) + const orderByDateInt = orderBy(idToDateInt, state.preferences['sort-order'] === 'newest' ? 'desc' : 'asc') - if (!addToUnifiedMailboxes) { - return - } - const unifiedAccount = state.accounts[UNIFIED_ACCOUNT_ID] - unifiedAccount.mailboxes - .map((mbId) => state.mailboxes[mbId]) - .filter((mb) => mb.specialRole && mb.specialRole === mailbox.specialRole) - .forEach((mailbox) => { - const existing = mailbox.envelopeLists[listId] || [] - Vue.set( - mailbox.envelopeLists, - listId, - uniq(orderByDateInt(appendOrReplaceEnvelopeId(state, existing, envelope))) - ) - }) + envelopes.forEach((envelope) => { + const existing = mailbox.envelopeLists[listId] || [] + normalizeTags(state, envelope) + Vue.set(state.envelopes, envelope.databaseId, Object.assign({}, state.envelopes[envelope.databaseId] || {}, envelope)) + Vue.set(envelope, 'accountId', mailbox.accountId) + Vue.set(mailbox.envelopeLists, listId, uniq(orderByDateInt(existing.concat([envelope.databaseId])))) + if (!addToUnifiedMailboxes) { + return + } + const unifiedAccount = state.accounts[UNIFIED_ACCOUNT_ID] + unifiedAccount.mailboxes + .map((mbId) => state.mailboxes[mbId]) + .filter((mb) => mb.specialRole && mb.specialRole === mailbox.specialRole) + .forEach((mailbox) => { + const existing = mailbox.envelopeLists[listId] || [] + Vue.set( + mailbox.envelopeLists, + listId, + uniq(orderByDateInt(existing.concat([envelope.databaseId]))) + ) + }) + }) }, updateEnvelope(state, { envelope }) { const existing = state.envelopes[envelope.databaseId] @@ -411,6 +395,11 @@ export default { removeEnvelopes(state, { id }) { Vue.set(state.mailboxes[id], 'envelopeLists', []) }, + removeAllEnvelopes(state) { + Object.keys(state.mailboxes).forEach(id => { + Vue.set(state.mailboxes[id], 'envelopeLists', []) + }) + }, addMessage(state, { message }) { Vue.set(state.messages, message.databaseId, message) }, diff --git a/tests/Unit/Controller/MessagesControllerTest.php b/tests/Unit/Controller/MessagesControllerTest.php index 3c05ea938d..b915a74322 100644 --- a/tests/Unit/Controller/MessagesControllerTest.php +++ b/tests/Unit/Controller/MessagesControllerTest.php @@ -36,6 +36,7 @@ use OCA\Mail\Contracts\IMailSearch; use OCA\Mail\Contracts\IMailTransmission; use OCA\Mail\Contracts\ITrustedSenderService; +use OCA\Mail\Contracts\IUserPreferences; use OCA\Mail\Controller\MessagesController; use OCA\Mail\Db\MailAccount; use OCA\Mail\Db\Tag; @@ -134,6 +135,8 @@ class MessagesControllerTest extends TestCase { private $clientFactory; private IDkimService $dkimService; + /** @var MockObject|IUserPreferences */ + private $userPreferences; private SnoozeService $snoozeService; protected function setUp(): void { @@ -158,6 +161,7 @@ protected function setUp(): void { $this->smimeService = $this->createMock(SmimeService::class); $this->clientFactory = $this->createMock(IMAPClientFactory::class); $this->dkimService = $this->createMock(IDkimService::class); + $this->userPreferences = $this->createMock(IUserPreferences::class); $this->snoozeService = $this->createMock(SnoozeService::class); $timeFactory = $this->createMocK(ITimeFactory::class); @@ -188,6 +192,7 @@ protected function setUp(): void { $this->smimeService, $this->clientFactory, $this->dkimService, + $this->userPreferences, $this->snoozeService, ); diff --git a/tests/Unit/Controller/PageControllerTest.php b/tests/Unit/Controller/PageControllerTest.php index 0ead3d56f7..e23c3fe038 100644 --- a/tests/Unit/Controller/PageControllerTest.php +++ b/tests/Unit/Controller/PageControllerTest.php @@ -162,10 +162,11 @@ public function testIndex(): void { $account1 = $this->createMock(Account::class); $account2 = $this->createMock(Account::class); $mailbox = $this->createMock(Mailbox::class); - $this->preferences->expects($this->exactly(6)) + $this->preferences->expects($this->exactly(7)) ->method('getPreference') ->willReturnMap([ [$this->userId, 'account-settings', '[]', json_encode([])], + [$this->userId, 'sort-order', 'newest', 'newest'], [$this->userId, 'external-avatars', 'true', 'true'], [$this->userId, 'reply-mode', 'top', 'bottom'], [$this->userId, 'collect-data', 'true', 'true'], @@ -284,7 +285,7 @@ public function testIndex(): void { ->method('getLoginCredentials') ->willReturn($loginCredentials); - $this->initialState->expects($this->exactly(14)) + $this->initialState->expects($this->exactly(15)) ->method('provideInitialState') ->withConsecutive( ['debug', true], @@ -292,6 +293,7 @@ public function testIndex(): void { ['accounts', $accountsJson], ['account-settings', []], ['tags', []], + ['sort-order', 'newest'], ['password-is-unavailable', true], ['prefill_displayName', 'Jane Doe'], ['prefill_email', 'jane@doe.cz'], diff --git a/tests/Unit/Service/Search/MailSearchTest.php b/tests/Unit/Service/Search/MailSearchTest.php index b4cb29a938..192f29aa17 100644 --- a/tests/Unit/Service/Search/MailSearchTest.php +++ b/tests/Unit/Service/Search/MailSearchTest.php @@ -94,6 +94,7 @@ public function testFindMessagesNotCached() { $this->search->findMessages( $account, $mailbox, + 'DESC', null, null, null @@ -109,6 +110,7 @@ public function testFindMessagesLocked() { $this->search->findMessages( $account, $mailbox, + 'DESC', null, null, null @@ -128,6 +130,7 @@ public function testNoFindMessages() { $messages = $this->search->findMessages( $account, $mailbox, + 'DESC', null, null, null @@ -166,6 +169,7 @@ public function testFindFlagsLocally() { $messages = $this->search->findMessages( $account, $mailbox, + 'DESC', 'my search', null, null @@ -207,6 +211,7 @@ public function testFindText() { $messages = $this->search->findMessages( $account, $mailbox, + 'DESC', 'my search', null, null diff --git a/tests/Unit/Service/Sync/SyncServiceTest.php b/tests/Unit/Service/Sync/SyncServiceTest.php index 5b8de2ee26..34bf9a39e5 100644 --- a/tests/Unit/Service/Sync/SyncServiceTest.php +++ b/tests/Unit/Service/Sync/SyncServiceTest.php @@ -100,7 +100,9 @@ public function testPartialSyncOnUncachedMailbox() { $mailbox, 42, [], - true + null, + true, + 'DESC' ); } @@ -140,6 +142,7 @@ public function testSyncMailboxReturnsFolderStats() { $mailbox, 0, [], + null, false );