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
This commit is contained in:
Ben Gotow 2016-04-05 19:16:58 -07:00
parent a84b95359c
commit 1476764d00
4 changed files with 37 additions and 4 deletions

View file

@ -260,6 +260,10 @@ class PreferencesMailRules extends React.Component {
} }
render() { render() {
const processDisabled = _.any(this.state.tasks, (task) => {
return (task.accountId === this.state.currentAccount.id);
});
return ( return (
<div className="container-mail-rules"> <div className="container-mail-rules">
<section> <section>
@ -273,8 +277,8 @@ class PreferencesMailRules extends React.Component {
<Flexbox style={{marginTop: 40, maxWidth: 600}}> <Flexbox style={{marginTop: 40, maxWidth: 600}}>
<div> <div>
<button className="btn" style={{float: 'right'}} onClick={this._onReprocessRules}> <button disabled={processDisabled} className="btn" style={{float: 'right'}} onClick={this._onReprocessRules}>
Process all mail Process entire inbox
</button> </button>
</div> </div>
{this._renderTasks()} {this._renderTasks()}
@ -282,7 +286,7 @@ class PreferencesMailRules extends React.Component {
<p style={{marginTop: 10}}> <p style={{marginTop: 10}}>
By default, mail rules are only applied to new mail as it arrives. 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. degrade performance.
</p> </p>
</section> </section>

View file

@ -32,5 +32,17 @@ class AttributeDateTime extends Attribute
throw (new Error "AttributeDateTime::lessThan (#{@modelKey}) - this field cannot be queried against") unless @queryable throw (new Error "AttributeDateTime::lessThan (#{@modelKey}) - this field cannot be queried against") unless @queryable
new Matcher(@, '<', val) 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 module.exports = AttributeDateTime

View file

@ -103,6 +103,8 @@ class Matcher
escaped = 1 escaped = 1
else if val is false else if val is false
escaped = 0 escaped = 0
else if val instanceof Date
escaped = val.getTime() / 1000
else if val instanceof Array else if val instanceof Array
escapedVals = [] escapedVals = []
for v in val for v in val

View file

@ -3,6 +3,7 @@ import Task from './task';
import Thread from '../models/thread'; import Thread from '../models/thread';
import Message from '../models/message'; import Message from '../models/message';
import DatabaseStore from '../stores/database-store'; import DatabaseStore from '../stores/database-store';
import CategoryStore from '../stores/category-store';
import MailRulesProcessor from '../../mail-rules-processor'; import MailRulesProcessor from '../../mail-rules-processor';
import async from 'async'; import async from 'async';
@ -12,6 +13,7 @@ export default class ReprocessMailRulesTask extends Task {
this.accountId = accountId; this.accountId = accountId;
this._processed = this._processed || 0; this._processed = this._processed || 0;
this._offset = this._offset || 0; this._offset = this._offset || 0;
this._lastTimestamp = this._lastTimestamp || null;
this._finished = false; this._finished = false;
} }
@ -36,14 +38,26 @@ export default class ReprocessMailRulesTask extends Task {
} }
_processSomeMessages = (callback) => { _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 // Fetching threads first, and then getting their messages allows us to use
// The same indexes as the thread list / message list in the app // 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 const query = DatabaseStore
.findAll(Thread, {accountId: this.accountId}) .findAll(Thread, {accountId: this.accountId})
.where(Thread.attributes.categories.contains(inboxCategory.id))
.order(Thread.attributes.lastMessageReceivedTimestamp.descending()) .order(Thread.attributes.lastMessageReceivedTimestamp.descending())
.offset(this._offset)
.limit(50) .limit(50)
if (this._lastTimestamp !== null) {
query.where(Thread.attributes.lastMessageReceivedTimestamp.lessThan(this._lastTimestamp))
}
return query.then((threads) => { return query.then((threads) => {
if (threads.length === 0) { if (threads.length === 0) {
this._finished = true; this._finished = true;
@ -61,6 +75,7 @@ export default class ReprocessMailRulesTask extends Task {
return MailRulesProcessor.processMessages(messages).finally(() => { return MailRulesProcessor.processMessages(messages).finally(() => {
this._processed += messages.length; this._processed += messages.length;
this._offset += threads.length; this._offset += threads.length;
this._lastTimestamp = threads.pop().lastMessageReceivedTimestamp;
}); });
}); });
}) })