Search code examples
phpsymfonysymfony4symfony-forms

Did we migrate incorrectly? Validation not happening in forms migrated from Symfony 2.7 to 4.0


In code migrated from Symfony 2.7 to 4.0, validation no longer happens on my form, allowing bad data to pass through and cause a Doctrine constraint violation

I'm new to Symfony and was asked to migrate a 2.7 application to 4.0. I did this in steps (2.7->2.8->3.x->4.0) and addressed issues as they came up, but one thing that broke along the way is automatic form validation. In the original version, if I attempted to create a new user and left the fields blank, it would correctly flag those and pop up " must not be empty" messages in the UI. Now, it lets those past until it attempts to write to the database, at which point Doctrine barfs because the database not null constraints are violated.

I've tried to figure out what I'm doing wrong, but I don't have a firm grasp on how the form creation process and syntax has changed. All of the example documentation on validation in forms assumes the createFormBuilder() approach, and all my existing code uses createForm(). What am I missing?

Here's part of the user object associated with the form showing the @Assert statements that I expect to trigger validation warnings:

/**
 * @ORM\Table(name="users")
 * @ORM\Entity(repositoryClass="Domain\CoreBundle\Repository\UserRepository")
 * @ORM\HasLifecycleCallbacks()
 * @UniqueEntity(fields="email", message="This email address is already in usage")
 * @UniqueEntity(fields="username", message="This username is already in usage")
 */
class User extends BaseUser implements JsonSerializable
{
    /**
     * @var integer
     *
     * @ORM\Column(name="id", type="integer")
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="AUTO")
     */
    protected $id;

    /**
     * @Assert\NotBlank(message="Email should not be empty")
     * @Assert\Email(strict=true)
     * @Assert\Length(max=150, maxMessage="Email should be less than {{ limit }} characters")
     */
    protected $email;

    /**
     * @Assert\NotBlank(message="Username should not be empty")
     * @Assert\Regex(
     *     pattern = "/^\d*[a-zA-Z][ a-zA-Z0-9\!\@\#\$\%\^\&\-\_\=\+\~\?\.]*$/i",
     *     message = "Username should include at least one letter"
     * )
     */
    protected $username;

    /**
     * @var string
     *
     * @Assert\NotBlank(message="First name should not be empty")
     * @ORM\Column(name="first_name", type="string", length=255)
     */
    protected $firstName;

    /**
     * @var string
     *
     * @Assert\NotBlank(message="Last name should not be empty")
     * @ORM\Column(name="last_name", type="string", length=255)
     */
    protected $lastName;

    (rest of code omitted for conciseness)

And here's the addNew action from the controller (AdministratorController extends UserController):

    /**
     * Add new administrator
     *
     * @param Request $request
     *
     * @return Response
     */
    public function addNewAction(Request $request)
    {
        $company = $this->getCurrentCompany();
        $form = $this->createForm(AddAdministratorType::class, null,
          array('current_user'=> $this->user, 'restricted_admin'=>$this->getRestrictedAdmin(), 'company'=>$company));

        if ($request->getMethod() == Request::METHOD_POST) {
            $form->handleRequest($request);

            // check if the user already exists
            $userManager = $this->get('fos_user.user_manager');
            $user = $form->getData();

            $oldUser = $userManager->findUserByUsername($user['email']);
            if ($oldUser)
            {
                $alreadyExists = false;
                if ($user["isSuperAdmin"] &&$oldUser->isGrantedSuperAdmin())
                    $alreadyExists = true;
                if ($user["isCompanyAdmin"] && $oldUser->isGranted(UserRepository::ROLE_COMPANY_ADMIN, $company))
                    $alreadyExists = true;
                if (!$user["isCompanyAdmin"] && !$user["isSuperAdmin"] && $oldUser->isGranted(UserRepository::ROLE_ADMIN,$company))
                    $alreadyExists = true;

                if ($alreadyExists)
                    $form->get('email')->addError(new FormError('This email address is already in use'));
            }


            if ($form->isValid()) {
                $user = $form->getData();


                if ($oldUser) // if the user already exists, we just need to add the role
                {
                    if (!$this->getUser()->isGrantedSuperAdmin() && 
                                !in_array($company->getId(), array_map(function($x){return $x->getId();}, $oldUser->getCompaniesWithRole())))
                    {
                        // the user isn't currently in this company and the user adding the role
                        // isn't a super admin, so we have to create a shadow user entry to hide 
                        // the real user info from other in the company until the user logs into
                        // the company
                        $oldShadow=$this->em->getRepository(ShadowUser::class)->findOneBy(array("user" => $oldUser, "company"=>$company));
                        if (!$oldShadow)
                        {
                            $shadow = new ShadowUser();
                            $shadow->setUser($oldUser);
                            $shadow->setFirstName($user["firstName"]);
                            $shadow->setLastName($user["lastName"]);
                            $shadow->setCompany($company);
                            $shadow->setIsVydioUsed($user["isVydioUsed"]);
                            $shadow->setVydioRoomLink($user["vydioRoomLink"]);
                            $shadow->setCreatedDate(new \DateTime());
                            $this->em->persist($shadow);
                        }
                    }

                    if ($user["isSuperAdmin"])
                    {
                        $oldUser->addMyRole(UserRepository::ROLE_SUPER_ADMIN, $company);
                        $this->get('pp_mailer')->onAddNewRole($oldUser,UserRepository::ROLE_SUPER_ADMIN, $company );
                    }
                    if ($user["isCompanyAdmin"])
                    {
                        $oldUser->addMyRole(UserRepository::ROLE_COMPANY_ADMIN, $company);
                        $this->get('pp_mailer')->onAddNewRole($oldUser,UserRepository::ROLE_COMPANY_ADMIN, $company );
                    }
                    if (!$user["isSuperAdmin"] && !$user["isCompanyAdmin"])
                    {
                        $oldUser->addMyRole(UserRepository::ROLE_ADMIN, $company);
                        $this->get('pp_mailer')->onAddNewRole($oldUser,UserRepository::ROLE_ADMIN, $company );
                    }

                    $programRepo = $this->em->getRepository(ProgramUser::class);
                    foreach($user["programs"] as $program)
                    {
                        $oldRelation = $programRepo->findOneBy(array("user"=> $oldUser, "program"=>$program));
                        if (!$oldRelation)
                        {
                            $relation = new ProgramUser();
                            $relation->setUser($oldUser);
                            $relation->setProgram($program);
                            $relation->setCompany($company);
                            $this->em->merge($relation);
                        }
                    }
                    $this->em->persist($oldUser);
                    $this->em->flush();

                }
                else
                {
                    $newUser = new User();
                    $newUser->setPassword($this->get('domain_core_service')->generatePassword());
                    $newUser->setDefaultCompany($company);
                    $newUser->setFirstName($user["firstName"]);
                    $newUser->setLastName($user["lastName"]);
                    $newUser->setEmail($user["email"]);
                    $newUser->setUsername($user["email"]);
                    $newUser->setEnabled($user["enabled"]);
                    $newUser = $this->em->getRepository('DomainCoreBundle:User')->addUserInSystem($userManager, $newUser);

                    $token = $this->get('domain_core_service')->generateToken();
                    $newUser->setConfirmationToken($token);
                    if ($user["isSuperAdmin"])
                    {
                        $newUser->addMyRole(UserRepository::ROLE_SUPER_ADMIN, $company);
                        $this->get('pp_mailer')->onAddNewUser($newUser,UserRepository::ROLE_SUPER_ADMIN, $company );
                    }
                    if ($user["isCompanyAdmin"])
                    {
                        $newUser->addMyRole(UserRepository::ROLE_COMPANY_ADMIN, $company);
                        $this->get('pp_mailer')->onAddNewUser($newUser,UserRepository::ROLE_COMPANY_ADMIN, $company );
                    }
                    if (!$user["isSuperAdmin"] && !$user["isCompanyAdmin"])
                    {
                        $newUser->addMyRole(UserRepository::ROLE_ADMIN, $company);
                        $this->get('pp_mailer')->onAddNewUser($newUser,UserRepository::ROLE_ADMIN, $company );
                    }

                    foreach($user["programs"] as $program)
                    {
                        $relation = new ProgramUser();
                        $relation->setUser($newUser);
                        $relation->setProgram($program);
                        $relation->setCompany($company);
                        $this->em->merge($relation);
                    }

                    $this->em->persist($newUser);
                    $this->em->flush();

                }
                return $this->redirect($this->generateUrl('domain_admin_show_all_administrators_page'));
            }
        }

        return $this->render(
            'DomainAdminBundle:Administrators:add-new.html.twig',
            array(
                 'form'                => $form->createView(),
                 'page_title'          => 'Add New Administrator',
                 'currentSidebar'      => $this->currentSideBar,
                 'currentSidebarItem'  => $this->currentSidebarItem,
            )
        );
    }

And the twig file for the form:

{% extends 'DomainAdminBundle::base-admin-layout.html.twig' %}
{% import '::widgets/form_errors.html.twig' as form_custom_errors %}
{% import '::widgets/label.html.twig' as form_custom_labels %}
{% block title %} My Application| {{ page_title }} {% endblock %}

{% block javascripts %}
    {{ parent() }}

    <script type="text/javascript" src="{{ asset('assets/scripts/admin-add-new.js') }}"></script>
{% endblock %}

{% block stylesheets %}
    {{ parent() }}

    <link rel="stylesheet" type="text/css" href="{{ asset('assets/styles/admin-add-new.css') }}">
{% endblock %}

{% block admin_main_content %}
    <div class="content-block administrator-controller" ng-controller="AdministratorController">
        <div class="content-title-bar">
            <div class="pull-left">
                <h2>{{ page_title }}</h2>
            </div>
        </div>

        <div class="content-block" ng-controller="AdminController">
            {{ form_start(form, {"attr": { "action":"{{ path('domain_admin_add_new_administrator_page') }}", 'enctype': 'multipart/form-data', "method":"POST", "novalidate":"novalidate", "autocomplete":"off", "class":"form-horizontal add-user", "ng-submit":"disableAddButton()" }}) }}
                <div class="base-box info-block">
                    <div class="control-group">
                        <div class="controls">
                            {{ form_widget(form.enabled) }}
                            {{ form_label(form.enabled, 'Active') }}
                        </div>
                    </div>
                    {% if app.user.isGrantedSuperAdmin() %}
                        <div class="control-group">
                            <div class="controls">
                                {% set companyAdminValue = form.isCompanyAdmin.vars.checked ? 'true' : 'false' %}
                                {{ form_widget(form.isCompanyAdmin, { 'attr':{ 'ng-model':'adminForm.isCompanyAdmin', 'ng-init': 'adminForm.isCompanyAdmin=' ~ companyAdminValue } }) }}
                                {{ form_label(form.isCompanyAdmin, 'Company Admin') }}
                                {% set superAdminValue = form.isSuperAdmin.vars.checked ? 'true' : 'false' %}
                                {{ form_widget(form.isSuperAdmin, { 'attr':{ 'ng-model':'adminForm.isSuperAdmin', 'ng-init': 'adminForm.isSuperAdmin=' ~ superAdminValue } }) }}
                                {{ form_label(form.isSuperAdmin, 'Super Admin') }}
                            </div>
                        </div>
                    {% endif %}
                    <div class="control-group" ng-init="initMultiSelect(true)">
                        {{ form_custom_labels.widget(form.programs) }}
                        <div class="controls">
                            {{ form_widget(form.programs) }}
                            {{ form_custom_errors.widget(form.programs) }}
                        </div>
                    </div>
                    <div class="control-group">
                        {{ form_custom_labels.widget(form.firstName) }}
                        <div class="controls">
                            {{ form_widget(form.firstName) }}
                            {{ form_custom_errors.widget(form.firstName) }}
                        </div>
                    </div>
                    <div class="control-group">
                        {{ form_custom_labels.widget(form.lastName) }}
                        <div class="controls">
                            {{ form_widget(form.lastName) }}
                            {{ form_custom_errors.widget(form.lastName) }}
                        </div>
                    </div>
                    <div class="control-group">
                        {{ form_custom_labels.widget(form.email) }}
                        <div class="controls">
                            {{ form_widget(form.email) }}
                            {{ form_custom_errors.widget(form.email) }}
                        </div>
                    </div>
                    <div class="control-group">
                        {{ form_custom_labels.widget(form.timezone) }}
                        <div class="controls">
                            {{ form_widget(form.timezone) }}
                            {{ form_custom_errors.widget(form.timezone) }}
                        </div>
                    </div>
                </div>
                <div class="text-right">
                    <button id="add-admin-submit" type="submit" class="btn btn-do" ng-disabled="isDisabled">Add new administrator</button>
                    <a href="{{ path('domain_admin_show_all_administrators_page') }}" class="btn btn-redo" ng-disabled="isDisabled">Cancel</a>
                </div>
                {{ form_rest(form) }}
            {{ form_end(form) }}
        </div>
    </div>
{% endblock %}

If I leave all fields blank and click "Add New Administrator" it doesn't flag them as blank, instead passing them onto Doctrine. The expected behavior is that it flags them at the UI and doesn't attempt to write them to the database.

I'm sure I've created multiple crimes against Symfony as I've ramped up the learning curve, so go easy. Right now I'm just trying to address this narrow issue; refactoring to more elegantly fit Symfony 4 will have to wait for another day.

Thanks!


Solution

  • Looks like you want to validate the User class against the data from your request.

    Have you set data_class option in your form type class?

    It's required if you want to use validation rules from another class (as you marked your properties with some @Assert* annotations).

    https://symfony.com/doc/current/forms.html#creating-form-classes

    Another way to do validation is to choose validation rules right in your FormType.