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.

Sign up for GitHub

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

Jump to bottom

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

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

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

wants to merge 30 commits into 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

@netlify Netlify
Copy link

netlify bot commented Oct 24, 2024 •
edited
Loading

✅ Deploy Preview for plone-components canceled.

Name Link
🔨 Latest commit 24d9f2e
🔍 Latest deploy log https://app.netlify.com/sites/plone-components/deploys/6748165cf6938d000839dd03

stevepiercy
stevepiercy requested changes Oct 24, 2024
View reviewed changes
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
packages/volto/src/components/theme/Register/Register.stories.jsx Outdated Show resolved Hide resolved
packages/volto/src/components/theme/Register/Register.stories.jsx Outdated Show resolved Hide resolved
stevepiercy
stevepiercy requested changes Oct 26, 2024
View reviewed changes
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.

packages/volto/src/components/theme/Register/__snapshots__/Register.test.jsx.snap Outdated Show resolved Hide resolved
sneridagh
sneridagh approved these changes Oct 29, 2024
View reviewed changes
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 •
edited
Loading

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sneridagh sneridagh sneridagh approved these changes

@stevepiercy stevepiercy stevepiercy approved these changes

Assignees
No one assigned
Labels
24 status: ready 32 needs: review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Registration form should be built with the information provided by the backend
4 participants
@erral @sneridagh @stevepiercy @ionlizarazu