-
Notifications
You must be signed in to change notification settings - Fork 7
ActiveSync: Custom sent folder #431 #450
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
1578ddc
to
2f21a49
Compare
c4a3273
to
53d8d41
Compare
871e5e5
to
0f52199
Compare
if (email.folder.specialFolder != SpecialFolder.Sent) { | ||
await defaultSentFolder.listMessages(); | ||
} | ||
defaultSentFolder.newMessageCallbacks.add(message => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we do that only when the sent folder is not the default ActiveSync sent folder? I.e. within the if
block above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is that we track all sent messages so that we can explicitly ignore the ones that shouldn't be moved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the advantage? Why not avoid adding the listener at all in this case? We already know at the time before we add the listener.
I want to avoid running this code altogether in the normal case, and do this special stuff only when the user explicitly sets the sent folder to a non-default folder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not the time we need to know, we need to know at the time we sync up the sent items folder which messages need to be moved and which don't, and throwing away some of that information is not going to help.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I don't understand. If we do not need to move a message, i.e. there's nothing to do, why would we add a callback?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are some sent messages. Some of them need to be moved. I'm not going to tell you which ones; you seem to know what you're doing.
@@ -117,6 +117,20 @@ export class ActiveSyncAccount extends MailAccount { | |||
if (email.bcc.hasItems) { | |||
throw new NotSupported("ActiveSync does not support BCC"); | |||
} | |||
assert(email.folder?.id, "Need folder to save the sent email in"); | |||
let defaultSentFolder = this.getSpecialFolder(SpecialFolder.Sent) as ActiveSyncFolder; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that the user can change the Sent folder in settings. In this case, the Sent folder flag does not match the server's idea of what the Sent folder is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing special folders is disabled for Exchange accounts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, this very PR would allow us to remove that limitation.
No assumptions, remember?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: When listing folders, remember (in a variable) the sent folder that the server told us, and use that sent folder here. That allows the user to set a custom sent folder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'll need to be more than a variable, because we don't get reminded each restart.
@@ -117,6 +117,20 @@ export class ActiveSyncAccount extends MailAccount { | |||
if (email.bcc.hasItems) { | |||
throw new NotSupported("ActiveSync does not support BCC"); | |||
} | |||
assert(email.folder?.id, "Need folder to save the sent email in"); | |||
let defaultSentFolder = this.getSpecialFolder(SpecialFolder.Sent) as ActiveSyncFolder; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, this very PR would allow us to remove that limitation.
No assumptions, remember?
@@ -117,6 +117,20 @@ export class ActiveSyncAccount extends MailAccount { | |||
if (email.bcc.hasItems) { | |||
throw new NotSupported("ActiveSync does not support BCC"); | |||
} | |||
assert(email.folder?.id, "Need folder to save the sent email in"); | |||
let defaultSentFolder = this.getSpecialFolder(SpecialFolder.Sent) as ActiveSyncFolder; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: When listing folders, remember (in a variable) the sent folder that the server told us, and use that sent folder here. That allows the user to set a custom sent folder.
if (email.folder.specialFolder != SpecialFolder.Sent) { | ||
await defaultSentFolder.listMessages(); | ||
} | ||
defaultSentFolder.newMessageCallbacks.add(message => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I don't understand. If we do not need to move a message, i.e. there's nothing to do, why would we add a callback?
e97466f
to
cff88be
Compare
Note that the
SendMail
API doesn't actually send the mail, it only queues it to the Outbox, so we have to rely on a subsequentPing
to tell us that the mail got sent.