Mailgun api key check backward (no API key should be required if bounces handled by mail receiver)

Bug
1

I’ve noticed a bunch of sites complaining that no Mailgun API key is defined, but (at least most of) those sites are receiving replies via the mail-receiver container and it handles those bounces so the API key (and corresponding webhook on Mailgun) do not need to be required. It looks like the logic in the specs (and hence, the code) is backward:

The missing_mailgun_api_key_spec

Shows this:

    context "when using Mailgun without an API key" do
      let(:replies_enabled) { true }
      let(:mailgun_address) { "smtp.mailgun.org" }
      let(:api_key) { nil }

      it do
        expect(check).to have_a_problem.with_priority("low").with_message(
          "The server is configured to send emails via Mailgun but you haven't provided an API key used to verify the webhook messages.",
        )
      end

complains if we have replies enabled, but in that case, we don’t need the webhook and

    context "when replies are disabled" do
      let(:replies_enabled) { false }
      let(:mailgun_address) { anything }
      let(:api_key) { anything }

      it { expect(check).to be_chill_about_it }
    end

Here we are NOT processing bounces with the mail receiver and we are being chill about it.

Am I confused or is this backward?

Well, maybe I am confused. It sure seems like the mail receiver isn’t telling Discourse about bounces. A busy site has no recent bounced messages.

But:

And:

So this documentation suggests that if you have a working mail receiver it should be handling this. And code that I once wrote in some of my internal tooling that set up these webhooks says “# don’t need webhooks when we have a mail receiver”

3 Likes