mirror of
https://github.com/Foundry376/Mailspring.git
synced 2024-09-20 23:36:21 +08:00
fix(mail-rules): Catch and disable mail rules building bad actions
This commit is contained in:
parent
28c5c621a1
commit
a573b70895
|
@ -47,8 +47,9 @@ NotificationStore = Reflux.createStore
|
|||
@_removeNotification(notification)() if action.dismisses
|
||||
@listenTo Actions.postNotification, (data) =>
|
||||
@_postNotification(new Notification(data))
|
||||
@listenTo Actions.multiWindowNotification, (data={}, context={}) =>
|
||||
@_postNotification(new Notification(data)) if @_inWindowContext(context)
|
||||
@listenTo Actions.dismissNotificationsMatching, (criteria) =>
|
||||
@_notifications = _.reject @_notifications, (n) -> _.isMatch(n, criteria)
|
||||
@trigger()
|
||||
|
||||
######### PUBLIC #######################################################
|
||||
|
||||
|
|
|
@ -84,7 +84,12 @@ class PreferencesMailRules extends React.Component
|
|||
className="rule-list"
|
||||
showEditIcon={true}
|
||||
items={@state.rules}
|
||||
itemContent={ (rule) -> rule.name }
|
||||
itemContent={ (rule) ->
|
||||
if rule.disabled
|
||||
return <div className="item-rule-disabled">{rule.name}</div>
|
||||
else
|
||||
return rule.name
|
||||
}
|
||||
onCreateItem={@_onAddRule}
|
||||
onDeleteItem={@_onDeleteRule}
|
||||
onItemEdited={@_onRuleNameEdited}
|
||||
|
@ -96,6 +101,7 @@ class PreferencesMailRules extends React.Component
|
|||
|
||||
if rule
|
||||
<ScrollRegion className="rule-detail">
|
||||
{@_renderDetailDisabledNotice()}
|
||||
<div className="inner">
|
||||
<span>If </span>
|
||||
<select value={rule.conditionMode} onChange={@_onRuleConditionModeEdited}>
|
||||
|
@ -122,6 +128,15 @@ class PreferencesMailRules extends React.Component
|
|||
<div className="no-selection">Create a rule or select one to get started</div>
|
||||
</div>
|
||||
|
||||
_renderDetailDisabledNotice: =>
|
||||
return false unless @state.selectedRule.disabled
|
||||
<div className="disabled-reason">
|
||||
<button className="btn" onClick={@_onRuleEnabled}>Enable</button>
|
||||
This rule has been disabled. Make sure the actions below are valid
|
||||
and re-enable the rule.
|
||||
<div>({@state.selectedRule.disabledReason})</div>
|
||||
</div>
|
||||
|
||||
_renderTasks: =>
|
||||
return false if @state.tasks.length is 0
|
||||
|
||||
|
@ -170,6 +185,9 @@ class PreferencesMailRules extends React.Component
|
|||
_onRuleConditionModeEdited: (event) =>
|
||||
Actions.updateMailRule(@state.selectedRule.id, {conditionMode: event.target.value})
|
||||
|
||||
_onRuleEnabled: =>
|
||||
Actions.updateMailRule(@state.selectedRule.id, {disabled: false, disabledReason: null})
|
||||
|
||||
_onRulesChanged: =>
|
||||
next = @stateForAccount(@props.accountId)
|
||||
nextRules = next.rules
|
||||
|
|
|
@ -10,6 +10,14 @@
|
|||
min-width:200px;
|
||||
height: 350px;
|
||||
}
|
||||
.item-rule-disabled {
|
||||
color: @error-color;
|
||||
padding: 4px 10px;
|
||||
border-bottom: 1px solid @border-color-divider;
|
||||
}
|
||||
.selected .item-rule-disabled {
|
||||
color: @component-active-bg;
|
||||
}
|
||||
}
|
||||
.rule-detail {
|
||||
flex: 1;
|
||||
|
@ -18,6 +26,16 @@
|
|||
border: 1px solid @border-color-divider;
|
||||
border-left: 0;
|
||||
|
||||
.disabled-reason {
|
||||
padding: @padding-base-vertical * 2 @padding-base-vertical * 2;
|
||||
background-color: fade(@background-color-error, 30%);
|
||||
border-bottom: 1px solid @background-color-error;
|
||||
margin-bottom: @padding-base-vertical;
|
||||
.btn {
|
||||
margin-left:@padding-base-horizontal * 2;
|
||||
float:right;
|
||||
}
|
||||
}
|
||||
.inner {
|
||||
padding: @padding-base-vertical @padding-base-horizontal;
|
||||
}
|
||||
|
|
|
@ -26,9 +26,9 @@ class DeveloperBarLongPollItem extends React.Component
|
|||
classname = "item"
|
||||
right = @props.item.cursor
|
||||
|
||||
if @props.item.ignoredBecause
|
||||
if @props.ignoredBecause
|
||||
classname += " ignored"
|
||||
right = @props.item.ignoredBecause + " - " + right
|
||||
right = @props.ignoredBecause + " - " + right
|
||||
|
||||
<div className={classname} onClick={ => @setState expanded: not @state?.expanded}>
|
||||
<div className="cursor">{right}</div>
|
||||
|
|
|
@ -40,15 +40,12 @@ class DeveloperBarStore extends NylasStore
|
|||
|
||||
longPollState: -> @_longPollState
|
||||
|
||||
longPollHistory: ->
|
||||
# We can't use Utils.deepClone because the deltas contain circular references
|
||||
# See delta.attributes._delta = delta
|
||||
JSON.parse(JSON.stringify(@_longPollHistory))
|
||||
longPollHistory: -> @_longPollHistory
|
||||
|
||||
########### PRIVATE ####################################################
|
||||
|
||||
triggerThrottled: ->
|
||||
@_triggerThrottled ?= _.throttle(@trigger, 100)
|
||||
@_triggerThrottled ?= _.throttle(@trigger, 150)
|
||||
@_triggerThrottled()
|
||||
|
||||
_setStoreDefaults: ->
|
||||
|
|
|
@ -77,7 +77,7 @@ class DeveloperBar extends React.Component
|
|||
|
||||
else if @state.section == 'long-polling'
|
||||
itemDivs = @state.longPollHistory.filter(matchingFilter).map (item) ->
|
||||
<DeveloperBarLongPollItem item={item} key={"#{item.cursor}-#{item.timestamp}"}/>
|
||||
<DeveloperBarLongPollItem item={item} ignoredBecause={item.ignoredBecause} key={"#{item.cursor}-#{item.timestamp}"}/>
|
||||
expandedDiv = <div className="expanded-section long-polling">{itemDivs}</div>
|
||||
|
||||
else if @state.section == 'queue'
|
||||
|
|
|
@ -1,7 +1,9 @@
|
|||
_ = require 'underscore'
|
||||
{Message,
|
||||
Contact,
|
||||
Thread,
|
||||
File,
|
||||
DatabaseStore,
|
||||
TaskQueueStatusStore,
|
||||
Actions} = require 'nylas-exports'
|
||||
|
||||
|
@ -142,14 +144,14 @@ describe "MailRulesProcessor", ->
|
|||
it "should queue tasks for messages", ->
|
||||
spyOn(TaskQueueStatusStore, 'waitForPerformLocal')
|
||||
spyOn(Actions, 'queueTask')
|
||||
|
||||
spyOn(DatabaseStore, 'findBy').andReturn(Promise.resolve({}))
|
||||
Tests.forEach ({rule}) =>
|
||||
TaskQueueStatusStore.waitForPerformLocal.reset()
|
||||
Actions.queueTask.reset()
|
||||
|
||||
messageSpy = jasmine.createSpy('message')
|
||||
threadSpy = jasmine.createSpy('thread')
|
||||
response = MailRulesProcessor._applyRuleToMessage(rule, messageSpy, threadSpy)
|
||||
message = new Message({accountId: rule.accountId})
|
||||
thread = new Thread({accountId: rule.accountId})
|
||||
response = MailRulesProcessor._applyRuleToMessage(rule, message, thread)
|
||||
expect(response instanceof Promise).toBe(true)
|
||||
|
||||
waitsForPromise =>
|
||||
|
|
|
@ -6,6 +6,7 @@ Rx = require 'rx-lite'
|
|||
|
||||
{Comparator, Template} = require './scenario-editor-models'
|
||||
|
||||
SOURCE_SELECT_NULL = 'NULL'
|
||||
|
||||
class SourceSelect extends React.Component
|
||||
@displayName: 'SourceSelect'
|
||||
|
@ -40,12 +41,22 @@ class SourceSelect extends React.Component
|
|||
render: =>
|
||||
options = @state.options
|
||||
|
||||
<select value={@props.value} onChange={@props.onChange}>
|
||||
# The React <select> component won't select the correct option if the value
|
||||
# is null or undefined - it just leaves the selection whatever it was in the
|
||||
# previous render. To work around this, we coerce null/undefined to SOURCE_SELECT_NULL.
|
||||
|
||||
<select value={@props.value || SOURCE_SELECT_NULL} onChange={@_onChange}>
|
||||
<option key={SOURCE_SELECT_NULL} value={SOURCE_SELECT_NULL}></option>
|
||||
{ @state.options.map ({value, name}) =>
|
||||
<option key={value} value={value}>{name}</option>
|
||||
}
|
||||
</select>
|
||||
|
||||
_onChange: (event) =>
|
||||
value = event.target.value
|
||||
value = null if value is SOURCE_SELECT_NULL
|
||||
@props.onChange(target: {value})
|
||||
|
||||
class ScenarioEditorRow extends React.Component
|
||||
@displayName: 'ScenarioEditorRow'
|
||||
@propTypes:
|
||||
|
|
|
@ -79,7 +79,6 @@ class Actions
|
|||
@downloadStateChanged: ActionScopeGlobal
|
||||
@linkFileToUpload: ActionScopeGlobal
|
||||
@fileUploaded: ActionScopeGlobal
|
||||
@multiWindowNotification: ActionScopeGlobal
|
||||
@sendDraftSuccess: ActionScopeGlobal
|
||||
@sendToAllWindows: ActionScopeGlobal
|
||||
@draftSendingFailed: ActionScopeGlobal
|
||||
|
@ -424,6 +423,8 @@ class Actions
|
|||
###
|
||||
@postNotification: ActionScopeGlobal
|
||||
|
||||
@dismissNotificationsMatching: ActionScopeGlobal
|
||||
|
||||
###
|
||||
Public: Listen to this action to handle user interaction with notifications you
|
||||
published via `postNotification`.
|
||||
|
@ -511,6 +512,7 @@ class Actions
|
|||
@addMailRule: ActionScopeWindow
|
||||
@updateMailRule: ActionScopeWindow
|
||||
@deleteMailRule: ActionScopeWindow
|
||||
@disableMailRule: ActionScopeWindow
|
||||
|
||||
# Read the actions we declared on the dummy Actions object above
|
||||
# and translate them into Reflux Actions
|
||||
|
|
|
@ -1,7 +1,9 @@
|
|||
NylasStore = require 'nylas-store'
|
||||
_ = require 'underscore'
|
||||
Rx = require 'rx-lite'
|
||||
AccountStore = require './account-store'
|
||||
DatabaseStore = require './database-store'
|
||||
TaskQueueStatusStore = require './task-queue-status-store'
|
||||
Utils = require '../models/utils'
|
||||
Actions = require '../actions'
|
||||
|
||||
|
@ -19,6 +21,8 @@ class MailRulesStore extends NylasStore
|
|||
@listenTo Actions.addMailRule, @_onAddMailRule
|
||||
@listenTo Actions.deleteMailRule, @_onDeleteMailRule
|
||||
@listenTo Actions.updateMailRule, @_onUpdateMailRule
|
||||
@listenTo Actions.disableMailRule, @_onDisableMailRule
|
||||
@listenTo Actions.notificationActionTaken, @_onNotificationActionTaken
|
||||
|
||||
rules: =>
|
||||
@_rules
|
||||
|
@ -38,6 +42,7 @@ class MailRulesStore extends NylasStore
|
|||
conditionMode: ConditionMode.All
|
||||
conditions: [ConditionTemplates[0].createDefaultInstance()]
|
||||
actions: [ActionTemplates[0].createDefaultInstance()]
|
||||
disabled: false
|
||||
|
||||
unless properties.accountId
|
||||
throw new Error("AddMailRule: you must provide an account id.")
|
||||
|
@ -52,10 +57,50 @@ class MailRulesStore extends NylasStore
|
|||
@_saveMailRules()
|
||||
@trigger()
|
||||
|
||||
_onDisableMailRule: (id, reason) =>
|
||||
existing = _.find @_rules, (f) -> id is f.id
|
||||
return if not existing or existing.disabled is true
|
||||
|
||||
Actions.postNotification
|
||||
message: "We were unable to run your mail rules - one or more rules have been disabled."
|
||||
type: "error"
|
||||
tag: 'mail-rule-failure'
|
||||
sticky: true
|
||||
actions: [{
|
||||
label: 'Hide'
|
||||
dismisses: true
|
||||
id: 'hide'
|
||||
},{
|
||||
label: 'View Rules'
|
||||
dismisses: true
|
||||
default: true
|
||||
id: 'mail-rule-failure:view-rules'
|
||||
}]
|
||||
|
||||
# Disable the task
|
||||
existing.disabled = true
|
||||
existing.disabledReason = reason
|
||||
@_saveMailRules()
|
||||
|
||||
# Cancel all bulk processing jobs
|
||||
for task in TaskQueueStatusStore.tasksMatching(ReprocessMailRulesTask, {})
|
||||
Actions.dequeueTask(task.id)
|
||||
|
||||
@trigger()
|
||||
|
||||
_onNotificationActionTaken: ({notification, action}) =>
|
||||
return unless NylasEnv.isMainWindow()
|
||||
if action.id is 'mail-rule-failure:view-rules'
|
||||
Actions.switchPreferencesTab('Mail Rules', {accountId: AccountStore.current().id})
|
||||
Actions.openPreferences()
|
||||
|
||||
_saveMailRules: =>
|
||||
@_saveMailRulesDebounced ?= _.debounce =>
|
||||
DatabaseStore.inTransaction (t) =>
|
||||
t.persistJSONBlob(RulesJSONBlobKey, @_rules)
|
||||
|
||||
if not _.findWhere(@_rules, {disabled: true})
|
||||
Actions.dismissNotificationsMatching({tag: 'mail-rule-failure'})
|
||||
,1000
|
||||
@_saveMailRulesDebounced()
|
||||
|
||||
|
|
|
@ -69,6 +69,9 @@ class ChangeFolderTask extends ChangeMailTask
|
|||
@threads = _.compact(threads)
|
||||
@messages = _.compact(messages)
|
||||
|
||||
if not @folder
|
||||
return Promise.reject(new Error("The specified folder could not be found."))
|
||||
|
||||
# The base class does the heavy lifting and calls changesToModel
|
||||
return super
|
||||
|
||||
|
|
|
@ -56,10 +56,13 @@ class ChangeLabelsTask extends ChangeMailTask
|
|||
messages: DatabaseStore.modelify(Message, @messages)
|
||||
|
||||
).then ({labelsToAdd, labelsToRemove, threads, messages}) =>
|
||||
if _.any([].concat(labelsToAdd, labelsToRemove), _.isUndefined)
|
||||
return Promise.reject(new Error("One or more of the specified labels could not be found."))
|
||||
|
||||
# Remove any objects we weren't able to find. This can happen pretty easily
|
||||
# if you undo an action and other things have happened.
|
||||
@labelsToAdd = _.compact(labelsToAdd)
|
||||
@labelsToRemove = _.compact(labelsToRemove)
|
||||
@labelsToAdd = labelsToAdd
|
||||
@labelsToRemove = labelsToRemove
|
||||
@threads = _.compact(threads)
|
||||
@messages = _.compact(messages)
|
||||
|
||||
|
|
|
@ -29,44 +29,51 @@ MailRulesActions =
|
|||
name: 'important',
|
||||
accountId: thread.accountId
|
||||
}).then (important) ->
|
||||
new ChangeLabelsTask(labelsToAdd: [important], threads: [thread])
|
||||
return Promise.reject(new Error("Could not find `important` label")) unless important
|
||||
return new ChangeLabelsTask(labelsToAdd: [important], threads: [thread])
|
||||
|
||||
moveToTrash: (message, thread) ->
|
||||
account = AccountStore.itemWithId(thread.accountId)
|
||||
CategoryClass = account.categoryClass()
|
||||
TaskClass = if CategoryClass is Label then ChangeLabelsTask else ChangeFolderTask
|
||||
if AccountStore.itemWithId(thread.accountId).categoryClass() is Label
|
||||
return MailRulesActions._applyStandardLabelRemovingInbox(message, thread, 'trash')
|
||||
else
|
||||
DatabaseStore.findBy(Folder, { name: 'trash', accountId: thread.accountId }).then (folder) ->
|
||||
return Promise.reject(new Error("The folder could not be found.")) unless folder
|
||||
return new ChangeFolderTask(folder: folder, threads: [thread])
|
||||
|
||||
Promise.props(
|
||||
inbox: DatabaseStore.findBy(CategoryClass, { name: 'inbox', accountId: thread.accountId })
|
||||
trash: DatabaseStore.findBy(CategoryClass, { name: 'trash', accountId: thread.accountId })
|
||||
).then ({inbox, trash}) ->
|
||||
new TaskClass
|
||||
labelsToRemove: [inbox]
|
||||
labelsToAdd: [trash]
|
||||
threads: [thread]
|
||||
|
||||
markAsRead: (message, thread, value) ->
|
||||
markAsRead: (message, thread) ->
|
||||
new ChangeUnreadTask(unread: false, threads: [thread])
|
||||
|
||||
star: (message, thread, value) ->
|
||||
star: (message, thread) ->
|
||||
new ChangeStarredTask(starred: true, threads: [thread])
|
||||
|
||||
applyLabel: (message, thread, value) ->
|
||||
new ChangeLabelsTask(labelsToAdd: [value], threads: [thread])
|
||||
changeFolder: (message, thread, value) ->
|
||||
return Promise.reject(new Error("A folder is required.")) unless value
|
||||
DatabaseStore.findBy(Folder, { id: value, accountId: thread.accountId }).then (folder) ->
|
||||
return Promise.reject(new Error("The folder could not be found.")) unless folder
|
||||
return new ChangeFolderTask(folder: folder, threads: [thread])
|
||||
|
||||
applyLabelArchive: (message, thread, value) ->
|
||||
applyLabel: (message, thread, value) ->
|
||||
return Promise.reject(new Error("A label is required.")) unless value
|
||||
DatabaseStore.findBy(Label, { id: value, accountId: thread.accountId }).then (label) ->
|
||||
return Promise.reject(new Error("The label could not be found.")) unless label
|
||||
return new ChangeLabelsTask(labelsToAdd: [label], threads: [thread])
|
||||
|
||||
applyLabelArchive: (message, thread) ->
|
||||
return MailRulesActions._applyStandardLabelRemovingInbox(message, thread, 'all')
|
||||
|
||||
# Helpers for other actions
|
||||
|
||||
_applyStandardLabelRemovingInbox: (message, thread, value) ->
|
||||
Promise.props(
|
||||
inbox: DatabaseStore.findBy(Label, { name: 'inbox', accountId: thread.accountId })
|
||||
all: DatabaseStore.findBy(Label, { name: 'all', accountId: thread.accountId })
|
||||
).then ({inbox, all}) ->
|
||||
new ChangeLabelsTask
|
||||
newLabel: DatabaseStore.findBy(Label, { name: value, accountId: thread.accountId })
|
||||
).then ({inbox, newLabel}) ->
|
||||
return Promise.reject(new Error("Could not find `inbox` or `#{value}` label")) unless inbox and newLabel
|
||||
return new ChangeLabelsTask
|
||||
labelsToRemove: [inbox]
|
||||
labelsToAdd: [all]
|
||||
labelsToAdd: [newLabel]
|
||||
threads: [thread]
|
||||
|
||||
changeFolder: (message, thread, value) ->
|
||||
new ChangeFolderTask(folder: value, threads: [thread])
|
||||
|
||||
|
||||
class MailRulesProcessor
|
||||
constructor: ->
|
||||
|
@ -74,10 +81,12 @@ class MailRulesProcessor
|
|||
processMessages: (messages) =>
|
||||
return Promise.resolve() unless messages.length > 0
|
||||
|
||||
enabledRules = MailRulesStore.rules().filter (r) -> not r.disabled
|
||||
|
||||
# When messages arrive, we process all the messages in parallel, but one
|
||||
# rule at a time. This is important, because users can order rules which
|
||||
# may do and undo a change. Ie: "Star if from Ben, Unstar if subject is "Bla"
|
||||
Promise.each MailRulesStore.rules(), (rule) =>
|
||||
Promise.each enabledRules, (rule) =>
|
||||
matching = messages.filter (message) =>
|
||||
@_checkRuleForMessage(rule, message)
|
||||
|
||||
|
@ -107,17 +116,27 @@ class MailRulesProcessor
|
|||
template.evaluate(condition, value)
|
||||
|
||||
_applyRuleToMessage: (rule, message, thread) =>
|
||||
results = rule.actions.map (action) =>
|
||||
MailRulesActions[action.templateKey](message, thread, action.value)
|
||||
actionPromises = rule.actions.map (action) =>
|
||||
actionFunction = MailRulesActions[action.templateKey]
|
||||
if not actionFunction
|
||||
return Promise.reject(new Error("#{action.templateKey} is not a supported action."))
|
||||
return actionFunction(message, thread, action.value)
|
||||
|
||||
Promise.all(results).then (results) ->
|
||||
Promise.all(actionPromises).then (actionResults) ->
|
||||
performLocalPromises = []
|
||||
|
||||
tasks = results.filter (r) -> r instanceof Task
|
||||
tasks.forEach (task) ->
|
||||
actionTasks = actionResults.filter (r) -> r instanceof Task
|
||||
actionTasks.forEach (task) ->
|
||||
performLocalPromises.push TaskQueueStatusStore.waitForPerformLocal(task)
|
||||
Actions.queueTask(task)
|
||||
|
||||
Promise.all(performLocalPromises)
|
||||
|
||||
.catch (err) ->
|
||||
# Errors can occur if a mail rule specifies an invalid label or folder, etc.
|
||||
# Disable the rule. Disable the mail rule so the failure is reflected in the
|
||||
# interface.
|
||||
Actions.disableMailRule(rule.id, err.toString())
|
||||
return Promise.resolve()
|
||||
|
||||
module.exports = new MailRulesProcessor
|
||||
|
|
Loading…
Reference in a new issue