Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show registration errors #3588

Closed
wants to merge 4 commits into from
Closed

Conversation

AwesomeKuro
Copy link
Contributor

see in docker desktop). Register in web and app
can be the same?

see in docker desktop). Register in web and app
can be the same?
@AwesomeKuro
Copy link
Contributor Author

image

@r0xsh
Copy link
Member

r0xsh commented Mar 27, 2023

I notice that the proposed changes affect many parts of the code that do not seem to be related to each other. Could you further explain why these modifications are necessary and how they are related to each other? It would be helpful to have a detailed description of your changes and their purpose, so that we can assess their impact on the overall project. Thank you (:

@AwesomeKuro
Copy link
Contributor Author

It will only show the first error for each field. Seems like i would be not able to match fields with errors, because the registration class or user interface is injected directly from the package.

Comment on lines +18 to +30
{% for key, error in form.vars.errors if not break %}
{% if error.cause.propertyPath == 'username' %}
<div class="alert alert-danger">
<ul class="list-unstyled">
<li>
<span class="glyphicon glyphicon-exclamation-sign"></span>
{{ error.message }}
</li>
</ul>
</div>
{% set break = true %}
{% endif %}
{% endfor %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be needed, the form itself should render the error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't, There are 2 problems

  • Error bubbling cannot be activated as the form doesn't inherit, it is injected but also directly referenced from the vendor code. The configuration for validations seems to be not present and without inheriting, i don't know how can i configure error bubbling.
  • Validation also is not set up in a way that we can validate the password's security and choose which validations are active for the email and username (we want to prevent the "already used" message from showing)

@alexsegura alexsegura changed the title Breaking for app. In web, logs to stdout (easy to Show registration errors Jul 4, 2023
@alexsegura
Copy link
Member

Registration errors are not mapped to individual fields in NucleosProfileBundle < 2 (nucleos/NucleosProfileBundle#154)

@alexsegura
Copy link
Member

Superseded by #3719

@alexsegura alexsegura closed this Jul 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants