Cfrtjryk

I've been in an interview some time ago. They objected my Symfony API code with following line:

Too much code in controller actions. Can be moved to services to keep
the business logic apart from the representation. Also violating
single responsibility.

I have been trying to improve my code since then. Trying to bring SOLID in my code design and writing services for different tasks.

Now I am working on an app where I have written a controller. It still looks bulky even though it is just using services.

There are two methods:

1. Register, to register user, add user to database and send activation code to their email


2. Activate, get activation code from URL and activate the user or show error.

I want to know if there is any room for improvement here.

namespace AppController;



use AppEntityUser;

use AppFormUserType;

use AppSerializerFormErrorSerializer;

use AppUtilsActivationKeyManager;

use DoctrineORMEntityManagerInterface;

use SymfonyBundleFrameworkBundleControllerAbstractController;

use SymfonyComponentHttpFoundationJsonResponse;

use SymfonyComponentHttpFoundationRequest;

use SymfonyComponentHttpFoundationResponse;

use SymfonyComponentRoutingGeneratorUrlGenerator;

use SymfonyComponentSecurityCoreEncoderUserPasswordEncoderInterface;

use SymfonyComponentTranslationTranslatorInterface;



/**

 * Class AuthController

 * @package AppController

 */

class AuthController extends AbstractController

{

    /**

     * @param Request $request

     * @param TranslatorInterface $translator

     * @param EntityManagerInterface $em

     * @param UserPasswordEncoderInterface $encoder

     * @param FormErrorSerializer $formErrorSerializer

     * @param ActivationKeyManager $activationKeyManager

     * @param Swift_Mailer $mailer

     * @return JsonResponse

     */

    public function register(

        Request $request,

        TranslatorInterface $translator,

        EntityManagerInterface $em,

        UserPasswordEncoderInterface $encoder,

        FormErrorSerializer $formErrorSerializer,

        ActivationKeyManager $activationKeyManager,

        Swift_Mailer $mailer

    ) : JsonResponse {

        $form = $this->createForm(UserType::class);

        $form->handleRequest($request);



        if ($form->isSubmitted() && $form->isValid()) {

            /**

             * @var User $user

             */

            $user = $form->getData();

            $encoded = $encoder->encodePassword($user, $user->getPlainPassword());

            $user->setPassword($encoded);

            $em->persist( $user );

            $em->flush();



            $activationLink = $this->generateUrl(

                'activate',

                [

                    'code' => $activationKeyManager->getKey($user),

                    'user' => $user->getId()

                ],

                UrlGenerator::ABSOLUTE_URL

            );

            $message = (new Swift_Message($translator->trans('Activate your account')))

                ->setFrom($this->getParameter('admin_email'))

                ->setTo($user->getEmail())

                ->setBody(

                    $this->renderView(

                        'emails/registration.txt.twig',

                        ['activationLink' => $activationLink]

                    ),

                    'text/plain'

                );



            $mailer->send($message);



            return new JsonResponse(['status' => 'success' , 'details' => $translator->trans('User has been created successfully.')]);

        }



        return new JsonResponse(['status' => 'error' , 'details' => $formErrorSerializer->convertFormToArray($form)], Response::HTTP_BAD_REQUEST);

    }



    /**

     * @param Request $request

     * @param TranslatorInterface $translator

     * @param EntityManagerInterface $em

     * @param ActivationKeyManager $activationKeyManager

     * @return JsonResponse

     */

    public function activate(

        Request $request,

        TranslatorInterface $translator,

        EntityManagerInterface $em,

        ActivationKeyManager $activationKeyManager

    ) {

        $user_id = $request->get('user');

        $activationCode = $request->get('code');



        if (!$user_id || !$activationCode) {

            return $this->sendInvalidActionCodeResponse($translator);

        }

        /**

         * @var User $user

         */

        $user = $em->getRepository(User::class)->find($user_id);

        if (!$user) {

            return $this->sendInvalidActionCodeResponse($translator);

        }



        if ($activationKeyManager->keyIsValid($user, $activationCode)) {

            $user->setActive(true);

            $em->flush($user);

            return new JsonResponse(['status' => 'success' , 'details' => $translator->trans('Your account has been activated.')]);

        } else {

            return $this->sendInvalidActionCodeResponse($translator);

        }

    }



    /**

     * @param TranslatorInterface $translator

     * @return JsonResponse

     */

    private function sendInvalidActionCodeResponse(TranslatorInterface $translator)

    {

        return new JsonResponse(['status' => 'error' , 'details' => $translator->trans('The provided activation code is not valid.')], Response::HTTP_BAD_REQUEST);

    }

}