Tuleap 40 register

requested review from @csimiand

assigned to @Sebastien

This merge request references: TULEAP-40.

added 15 commits

• 1adf4820...14f74419 - 4 commits from branch main
• 06499ed2 - Create simple user entity
• e6acd9ac - Migration for simple user
• eef8cbcf - set User compatible to the production database
• a30313b4 - Add View and controllers for user registration
• a0d47256 - Add webpack-encore for symfony project
• c9b538e8 - restore lost dependencies webpack and verify-email
• c2e76d83 - Add page mail sent on post registration
• e78925ac - Add image support on assets
• a864d2dd - Reorganise bootstrap SASS configuration
• 9dd5faf4 - On mail verification success redirection on login page + message
• 610d07fb - Add user story application test

Compare with previous version

marked this merge request as ready

IMO, keeping in mind good development practices, we should aim to a straight separation between logic and style.

Following the official documentation, here we could use twig configuration to customize form: https://symfony.com/doc/current/form/form_customization.html

What do you think ?

I agree with you, but if I want to separate logic and style. I would have to create a whole new theme. That's a lot of work.

I'm not 100% sure but I think it can be quick if you can reuse the bootstrap theme or the symfony built-in form theme:

• https://symfony.com/doc/current/reference/configuration/twig.html#config-twig-form-themes
• https://symfony.com/doc/current/form/form_themes.html

See file config/packages/twig.yaml :

twig:
    default_path: '%kernel.project_dir%/templates'
    form_themes: ['bootstrap_5_layout.html.twig']

We use the bootstrap 5 layout.

Ok, I saw it but I still can't get why can't you add class attributes directly in registration.html.twig instead of passing it as a variable in this controller.

Example:

      {{ form_start(registrationForm) }}
      <div class="form-group d-flex flex-column m-auto mb-4 col-6">
          <div>
              {{ form_row(registrationForm.firstName) }}
          </div>
          <div>
              {{ form_row(registrationForm.lastName) }}
          </div>
          <div>
              {{ form_row(registrationForm.email) }}
          </div>
      </div>
      {{ form_end(registrationForm) }}

The solution doesn't work for the captcha. It depends on whether you consider the RegistratinFormType as the form model for the view (It seems to be the case).

I have tried to set class attributes directly in the twig file. It doesn't work.

The following solution theoretically works (https://symfony.com/doc/current/form/form_customization.html#form-variables-reference) but, I've tried it, in practice it doesn't work.

{{ form_start(registrationForm, {'attr': {'class': 'd-flex flex-column justify-content-center'}}) }}
    {{ form_row(registrationForm.firstName, {'attr': {'class' : 'form-group d-flex flex-column m-auto mb-4 col-6'}})  }}
    {{ form_row(registrationForm.lastName, {'attr': {'class' : 'form-group d-flex flex-column m-auto mb-4 col-6'}}) }}
    {{ form_row(registrationForm.email, {'attr': {'class' : 'form-group d-flex flex-column m-auto mb-4 col-6'}}) }}
    {{ form_row(registrationForm.submit, {'attr': {'class' : 'form-group d-flex flex-column m-auto mb-4 col-6'}}) }}
{{ form_end(registrationForm) }}

changed this line in version 8 of the diff

Following the example of the previous MemoRekallMember app, the logo link should target:

1. /login page when the user is not authenticated
2. /my_capsules (or /homepage if we decide to rename it for more consistency) page when the user is authenticated

• For backports we must keep /my_capsules as home page.
• Going back to login for an unauthenticated user we should use an interceptor that redirect automatically the user to the login page (On every page)

Ok, thanks for your answer.

Could you implement the interceptor in this MR please?

Nope : the aim of this US associated to this MR is not login. But only register.

In the current branch, on login you get an exception, it won't be testable.

I put in the tuleap-45-login MR to remember this discussion.

changed this line in version 6 of the diff

I understand what you're saying but I am not completely convinced. IMO, we shouldn't introduce a broken feature: clicking on MemoRekall logo from /register is currently broken.

I suggest to redirect to /home for this MR (it will be updated later with "login" feature MR) where we will see only:

• the logo
• the login button
• the register button
• no forms displayed

--> What do you think about creating this new page inside this MR?

It's done in the tuleap-45-login MR. So, I Keep my veto for this comment

Please move this file in its directory templates/registration/.

I let it here because base twig file should be in this directory. I should rename it because it's used by both the register and login pages.

maybe a consistent name would be layout?

OK for layout.html.twig

changed this line in version 6 of the diff

Shouldn't this line be inherited by a base.html.twig or layout.html.twig template?

I will remove base.html.twig. It doesn't seem relevant to keep it.

changed this line in version 6 of the diff

See previous comment about how to disable captcha for test purpose.

changed this line in version 7 of the diff

Please add and update copyright here. (see previous app repository)

Please add and update copyright here. (see previous app repository)

Please add and update copyright here. (see previous app repository)

Please add and update copyright here. (see previous app repository)

Please add and update copyright here. (see previous app repository)