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:
- Register, to register user, add user to database and send activation code to their email
- 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 App\Controller;
use App\Entity\User;
use App\Form\UserType;
use App\Serializer\FormErrorSerializer;
use App\Utils\ActivationKeyManager;
use Doctrine\ORM\EntityManagerInterface;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\JsonResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Routing\Generator\UrlGenerator;
use Symfony\Component\Security\Core\Encoder\UserPasswordEncoderInterface;
use Symfony\Component\Translation\TranslatorInterface;
/**
* Class AuthController
* @package App\Controller
*/
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);
}
}