FEATURE: Implement support for IMAP and SMTP email protocols. (PR #8301)

Co-authored-by: Joffrey JAFFEUX j.jaffeux@gmail.com

GitHub

You’ve signed the CLA, udan11. Thank you! This pull request is ready for review.

Can you import @ember/component please? It’s our new standard way of doing this.

Also super minor but you can Component.extend() without the empty object.

This is a huge one, but overall is looking great. I would like some changes and for @SamSaffron to review it too.

This can also be imported from discourse/routes/discourse

Did we mean to leave this in? Usually debugging information like this behind some kind of flag or setting.

obj here is fairly generic. I would prefer group: or target: to be more consistent with the rest of our codebase.

I dislike boolean parameters like this, because when you read the code at the calling point it looks like this:

open_mailbox('my_mailbox', true) and it’s totally unclear what true means.

Instead we should use a keyword argument like open_mailbox('my_mailbox', write: true) which is self documenting.

Personally I am not a fan of using threads, as historically they have been a source of hard to track down bugs and are very hard to get right. Having said that, if this was discussed outside of this PR and this was the conclusion that is OK, but I would like someone else to review it who has more experience such as @SamSaffron.

Might be able to simplify this: @imap ||= super.tap { |imap| apply_gmail_patch(imap) }

The logic is a little hard to follow so can you clarify something for me here? Does this patch the Imap class, so that every instance of the imap will now be patched for gmail? Or does it only patch the specific instance?

I believe that production instances usually have logging level set to >= warn so it is not an issue and is super handy in development.

Yes, it opens the metaclass (/ eigenclass / singleton class) of the parser instance to add support for Gmail’s IMAP extensions (X-GM-* attributes).

I agree that obj is generic, but I believe that is how it is supposed to be, because type specifies what obj is.

I think I can just pass @group and perform a type check with is_a?, but will have to do a quick benchmark first because I have a feeling it will be a lot slower.

Sure, that’s fine with me.