Category email in requires reply_by_email_enabled?

Bug
1

Situation

  • reply_by_email_enabled disabled, we do not want people to reply by email
  • an admin-only category for notifications from an external system
  • the category has an email address set up
  • email_in is enabled
  • emails are sent to the email address of the category
  • emails are bouncing with BadDestinationAddress

The cause

After an hour of debugging, I came across the following FIX: Disallow replies to categories when reply by email disabled (#33… · discourse/discourse@e05ef50 · GitHub

FIX: Disallow replies to categories when reply by email disabled (#33641)

When the reply_by_email_enabled setting is set to false, we no longer include a reply link in e-mail notifications. However, we do not prevent actual e-mails to a configured email_in address associated with a category. This change into consideration the setting in Email::Receiver#check_address.

and the affected code will no longer return a category if reply_by_email_enabled is not enabled.

    def self.check_address(address, include_verp = false)
      # only check for a group/category when 'email_in' is enabled
      if SiteSetting.email_in
        group = Group.find_by_email(address)
        return group if group

        category = Category.find_by_email(address)
        return category if category && SiteSetting.reply_by_email_enabled? <-- added
      end

Why? :scream:

I have so many questions:

  1. why this change? At all? It only makes the system more inflexible. If I don’t want people to email to a category, then I would simply remove the email address from the category?
  2. why is this implemented this way?
  • apparently email in to groups is not a problem?
  • if reply_by_email_enabled is false then there is no need to even look up the category?
  • by not returning the category the error becomes BadDestinationAddress which is VERY wrong and REALLY hard to debug
  1. the setting this now suddenly depends on, is called reply by email. That is not what I am doing.
  2. the same confusion is visible in the title of the PR, which is called “disallow replies to categories”, which is not what this is about (in Discourse a “reply to a category” is not even possible).

Besides the way this was implemented, I honestly fail to see the point.

I cannot think of ANY situation where simply removing the email address from the category would not do the job if one wants to prevent people from emailing to a category. And the implication of this is that it has now become impossible to have a category receiving emails without enabling site-wide reply by email.

If there is a really good reason that I am missing, feel free to recategorize as Feature

4 Likes
2

I actually found out about this myself and am already looking into it.

I agree the fix done in FIX: Disallow replies to categories when reply by email disabled (#33… · discourse/discourse@e05ef50 · GitHub was bad as it relied on and introduced a side effect, but we didn’t notice as the tests are arranged such that the conditions for triggering it weren’t present. I’m working on correcting that now.

2 Likes