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

build the registration from using the userschema provided by backend #6435

Open
wants to merge 32 commits into
base: main
Choose a base branch
from

Conversation

erral
Copy link
Member

@erral erral commented Oct 24, 2024


If your pull request closes an open issue, include the exact text below, immediately followed by the issue number. When your pull request gets merged, then that issue will close automatically.

Closes #6434

Copy link

netlify bot commented Oct 24, 2024

Deploy Preview for plone-components canceled.

Name Link
🔨 Latest commit c76bf75
🔍 Latest deploy log https://app.netlify.com/sites/plone-components/deploys/6756a2be2c3bd8000854cb9e

Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

Minor grammar fixes.

This PR still requires technical review.

packages/volto/news/6434.feature Outdated Show resolved Hide resolved
Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

One more instance, and I think it's GTG.

Copy link
Member

@sneridagh sneridagh left a comment

Choose a reason for hiding this comment

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

LGTM. I am willing to make it enter in 18, but I need someone to test it and give feedback.
Some Cypress tests would be great too.
Any takers?

/cc @plone/volto-team

@sneridagh
Copy link
Member

On the other hand, we can make it for 18.1 since it's a feature.

@erral
Copy link
Member Author

erral commented Oct 29, 2024

LGTM. I am willing to make it enter in 18, but I need someone to test it and give feedback. Some Cypress tests would be great too. Any takers?

/cc @plone/volto-team

We can check this indeed.

@erral
Copy link
Member Author

erral commented Oct 30, 2024

I have added a cypress test that shows that the fields in the /@userschema request are rendered in the form.

I have been checking also the implementation of the registration, and I see that we are using 2 settings userEmailAsLogin and showSelfRegistration which should be coming from backend (there are indeed 2 settings in backend for them).

But using them would mean to change the REST API to expose those settings (right now the /@registry endpoint is reserved for Managers), and then change the implementation not only of the registration form but also of other links and usages of those 2 settings.

Which means it is a larger project.

@sneridagh
Copy link
Member

@erral not sure why the test are breaking now, although it seems ok and legit.

@erral
Copy link
Member Author

erral commented Dec 9, 2024

Mmmm, strange, it looks like the snapshot structure changed. If I update the snapshot I get a completely different structure.

I will look at it more in depth.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Registration form should be built with the information provided by the backend
4 participants