Single endpoint for ingoing mail with multiple purposes

Question

The following code is not one of my proud moments. In its initial state, I had only a single purpose, but know I'm up to three purposes and I don't know if it would stop there.

I'm using mailin.io to forward mails to my endpoint as JSON data.

There are three main purposes, I've tried to describe it as best as I could as comments in the code. The code is messy and unclean and I would appreciate some pointers on how I can structure it better.

<?php

namespace App\Http\Controllers;

use App\Conversation;
use App\Jobs\SendSurveyEmail;
use App\Message;
use App\Score;
use Illuminate\Http\Request;

use App\Http\Requests;
use Illuminate\Support\Facades\Log;

class Mailcatcher extends Controller
{
    public function handle(Request $request)
    {
        if ($request->has('mailinMsg')) {
            $msg = $request->get('mailinMsg');

            $msg = json_decode($msg);

            $address = $msg->to[0]->address;
            $name = $msg->to[0]->name;

            $parts = explode("@", $address);

            if ($parts[1] != "example.com") {
                // If the mail is not meant for my app, but I'm receiving because my app is a BCC recipient
                $this->_sendSurvey($msg);
            } else {
                // If the mail is meant for my app

                // Check if I'm using an alias (for unsubscrubing)
                $key = $parts[0];
                $key_parts = explode("+", $key);

                if (count($key_parts) > 1) {

                    // Unsubscring the sender (just logging currently)
                    if ($key_parts[0] == "unsub") {
                        Log::info('Survey Unsubscribe: ' . $key);
                    }

                } else {


                    // If the mail is meant to be a part of conversation and if so, store a new
                    if ($conversation = Conversation::where('key', $key)->first()) {
                        $content = $msg->text;

                        if (empty($content)) {
                            if (!empty($msg->html)) {
                                $content =  $msg->html;
                            } else {
                                // Don't do anything since the mail content is empty
                                return;
                            }
                        }

                        $message = new Message();
                        $message->conversation_id = $conversation->id;
                        $message->user_id = 0;
                        $message->name = $msg->from[0]->name;
                        $message->email = $msg->from[0]->address;
                        $message->content = $content;
                        $message->save();
                    } else {
                        dd("I have no idea why I'm receiving this mail");
                    }
                }


            }

        }

        return response(null, 200);

    }

    private function _sendSurvey($msg)
    {
        // Sending a survey to a user

        $units = [
            'm' => 60,
            'h' => 60 * 60,
            'd' => 60 * 60 * 24,
            'w' => 60 * 60 * 24 * 7,
        ];

        $address = $msg->to[0]->address;
        $name = $msg->to[0]->name;

        foreach ($msg->envelopeTo as $envelope) {
            list($key, $domain) = explode("@", $envelope->address);

            if ($domain == "example.com") {
                if ($mailcatcher = \App\Mailcatcher::where('key', $key)->first()) {

                    // Make sure that the application is still active
                    if ($application = $mailcatcher->application) {
                        $score = new Score([
                            'name' => $name,
                            'email' => $address,
                        ]);

                        $application->scores()->save($score);

                        $delay = $mailcatcher->delay * $units[$mailcatcher->units];

                        $job = (new SendSurveyEmail($application, $score))->delay($delay);
                        $this->dispatch($job);

                        $mailcatcher->hits++;
                        $mailcatcher->save();
                        break;
                    }


                }
            }


        }
    }

    public function checkin(Request $request)
    {
        Log::info($request);

        return response(null, 200);
    }
}

Answer 1

1. You probably shouldn't be doing the work in this script. I'd have a MailParser or MailTriage class that does nothing except receive the request, figure out what it's about and forward it to the appropriate service.

2. I'd have one service for each task: add to conversation, send survey, unsubscribe...

3. Because email addresses are case insensitive, I would make sure that all checks are case insensitive.

I would replace...

if ($parts[1] != "example.com")...
if ($key_parts[0] == "unsub")...
if ($conversation = Conversation::where('key', $key)->first()...
...

...with equivalents that would match whether the email is lower, upper or mixed case. The simplest way is to lowercase the address before you do any processing.

$address = strtolower($msg->to[0]->address)

This way you only have to worry about matching against a lowercase string