From 1476764d0007448d1f8c196d1e97f4fc6a9d3088 Mon Sep 17 00:00:00 2001 From: Ben Gotow Date: Tue, 5 Apr 2016 19:16:58 -0700 Subject: [PATCH] fix(mail-rules): Only process inbox, never skip threads Summary: - Disable processing button while already processing - Only process mail in the inbox in bulk reprocess task - Advance through mail using "after X" rather than "offset X", avoiding the issue where mail can be deleted as you're advancing. Test Plan: Run existing tests Reviewers: evan, juan Reviewed By: juan Differential Revision: https://phab.nylas.com/D2847 --- .../lib/tabs/preferences-mail-rules.jsx | 10 +++++++--- src/flux/attributes/attribute-datetime.coffee | 12 ++++++++++++ src/flux/attributes/matcher.coffee | 2 ++ src/flux/tasks/reprocess-mail-rules-task.es6 | 17 ++++++++++++++++- 4 files changed, 37 insertions(+), 4 deletions(-) diff --git a/internal_packages/preferences/lib/tabs/preferences-mail-rules.jsx b/internal_packages/preferences/lib/tabs/preferences-mail-rules.jsx index 1ad825cd1..1b5afebe4 100644 --- a/internal_packages/preferences/lib/tabs/preferences-mail-rules.jsx +++ b/internal_packages/preferences/lib/tabs/preferences-mail-rules.jsx @@ -260,6 +260,10 @@ class PreferencesMailRules extends React.Component { } render() { + const processDisabled = _.any(this.state.tasks, (task) => { + return (task.accountId === this.state.currentAccount.id); + }); + return (
@@ -273,8 +277,8 @@ class PreferencesMailRules extends React.Component {
-
{this._renderTasks()} @@ -282,7 +286,7 @@ class PreferencesMailRules extends React.Component {

By default, mail rules are only applied to new mail as it arrives. - Applying rules to your entire mailbox may take a long time and + Applying rules to your entire inbox may take a long time and degrade performance.

diff --git a/src/flux/attributes/attribute-datetime.coffee b/src/flux/attributes/attribute-datetime.coffee index 625930aa8..5fec13e9e 100644 --- a/src/flux/attributes/attribute-datetime.coffee +++ b/src/flux/attributes/attribute-datetime.coffee @@ -32,5 +32,17 @@ class AttributeDateTime extends Attribute throw (new Error "AttributeDateTime::lessThan (#{@modelKey}) - this field cannot be queried against") unless @queryable new Matcher(@, '<', val) + # Public: Returns a {Matcher} for objects greater than the provided value. + greaterThanOrEqualTo: (val) -> + throw (new Error "AttributeNumber::greaterThanOrEqualTo (#{@modelKey}) - you must provide a value") unless val? + throw (new Error "AttributeNumber::greaterThanOrEqualTo (#{@modelKey}) - this field cannot be queried against") unless @queryable + new Matcher(@, '>=', val) + + # Public: Returns a {Matcher} for objects less than the provided value. + lessThanOrEqualTo: (val) -> + throw (new Error "AttributeNumber::lessThanOrEqualTo (#{@modelKey}) - you must provide a value") unless val? + throw (new Error "AttributeNumber::lessThanOrEqualTo (#{@modelKey}) - this field cannot be queried against") unless @queryable + new Matcher(@, '<=', val) + module.exports = AttributeDateTime diff --git a/src/flux/attributes/matcher.coffee b/src/flux/attributes/matcher.coffee index 5a48525fc..f896abbcf 100644 --- a/src/flux/attributes/matcher.coffee +++ b/src/flux/attributes/matcher.coffee @@ -103,6 +103,8 @@ class Matcher escaped = 1 else if val is false escaped = 0 + else if val instanceof Date + escaped = val.getTime() / 1000 else if val instanceof Array escapedVals = [] for v in val diff --git a/src/flux/tasks/reprocess-mail-rules-task.es6 b/src/flux/tasks/reprocess-mail-rules-task.es6 index ff04800a4..25b10db62 100644 --- a/src/flux/tasks/reprocess-mail-rules-task.es6 +++ b/src/flux/tasks/reprocess-mail-rules-task.es6 @@ -3,6 +3,7 @@ import Task from './task'; import Thread from '../models/thread'; import Message from '../models/message'; import DatabaseStore from '../stores/database-store'; +import CategoryStore from '../stores/category-store'; import MailRulesProcessor from '../../mail-rules-processor'; import async from 'async'; @@ -12,6 +13,7 @@ export default class ReprocessMailRulesTask extends Task { this.accountId = accountId; this._processed = this._processed || 0; this._offset = this._offset || 0; + this._lastTimestamp = this._lastTimestamp || null; this._finished = false; } @@ -36,14 +38,26 @@ export default class ReprocessMailRulesTask extends Task { } _processSomeMessages = (callback) => { + const inboxCategory = CategoryStore.getStandardCategory(this.accountId, 'inbox'); + if (!inboxCategory) { + return callback(new Error("ReprocessMailRulesTask: No inbox category found.")); + } + // Fetching threads first, and then getting their messages allows us to use // The same indexes as the thread list / message list in the app + + // Note that we look for "50 after X" rather than "offset 150", because + // running mail rules can move things out of the inbox! const query = DatabaseStore .findAll(Thread, {accountId: this.accountId}) + .where(Thread.attributes.categories.contains(inboxCategory.id)) .order(Thread.attributes.lastMessageReceivedTimestamp.descending()) - .offset(this._offset) .limit(50) + if (this._lastTimestamp !== null) { + query.where(Thread.attributes.lastMessageReceivedTimestamp.lessThan(this._lastTimestamp)) + } + return query.then((threads) => { if (threads.length === 0) { this._finished = true; @@ -61,6 +75,7 @@ export default class ReprocessMailRulesTask extends Task { return MailRulesProcessor.processMessages(messages).finally(() => { this._processed += messages.length; this._offset += threads.length; + this._lastTimestamp = threads.pop().lastMessageReceivedTimestamp; }); }); })