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

[framework] added support for customer user registration to Frontend API #2100

Merged
merged 1 commit into from Nov 10, 2020
Merged

[framework] added support for customer user registration to Frontend API #2100

merged 1 commit into from Nov 10, 2020

Conversation

malyMiso
Copy link
Contributor

@malyMiso malyMiso commented Nov 5, 2020 •
edited by grossmannmartin

Q A
Description, reason for the PR added support for customer user registration to Frontend API
New feature Yes
[BC breaks] No
Fixes issues ...
Have you read and signed our License Agreement for contributions? Yes

@malyMiso malyMiso force-pushed the mt-registration-feapi branch 6 times, most recently from 52b7f07 to da111f6 Compare November 6, 2020 10:56
grossmannmartin
grossmannmartin requested changes Nov 6, 2020
View reviewed changes
Copy link
Member

@grossmannmartin grossmannmartin left a comment

Choose a reason for hiding this comment

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

Thanks! Nice work.
Can you check my comments, please?

packages/frontend-api/apiary.apib Outdated Show resolved Hide resolved
packages/frontend-api/src/Resources/config/graphql-types/RegistrationDecorator.types.yaml Outdated Show resolved Hide resolved
packages/frontend-api/apiary.apib Outdated Show resolved Hide resolved
project-base/tests/FrontendApiBundle/Functional/Customer/User/RegisterTest.php Outdated Show resolved Hide resolved
project-base/tests/FrontendApiBundle/Functional/Customer/User/RegisterTest.php Outdated Show resolved Hide resolved
packages/frontend-api/src/Model/Mutation/Customer/User/CustomerUserMutation.php Show resolved Hide resolved
@malyMiso malyMiso force-pushed the mt-registration-feapi branch 4 times, most recently from b13a6bd to f0904dd Compare November 9, 2020 08:47
grossmannmartin
grossmannmartin previously approved these changes Nov 9, 2020
View reviewed changes
Copy link
Member

@grossmannmartin grossmannmartin left a comment

Choose a reason for hiding this comment

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

Thanks!

@grossmannmartin
Copy link
Member

I tested this PR and found the following

  • fields are not trimmed (I think it's not probably related only with this PR, but it's a broader issue)
    So, trying to register with email " dd@dd.dd" (without quotes) returns "Internal server Error". A new user is created in the database, but I'm not able to login as this user on the frontend (in API the login method works even with spaces).

Similarly, name and surname are trimmed when I perform registration on frontend, but not in FE API (and I don't know if it's a problem or not).

Can you check it out and create an issue for resolution if it's outside the scope of this PR?

Other than that I did not found any problems.

@malyMiso
Copy link
Contributor Author

malyMiso commented Nov 9, 2020

I tested this PR and found the following

  • fields are not trimmed (I think it's not probably related only with this PR, but it's a broader issue)
    So, trying to register with email " dd@dd.dd" (without quotes) returns "Internal server Error". A new user is created in the database, but I'm not able to login as this user on the frontend (in API the login method works even with spaces).

Similarly, name and surname are trimmed when I perform registration on frontend, but not in FE API (and I don't know if it's a problem or not).

Can you check it out and create an issue for resolution if it's outside the scope of this PR?

Other than that I did not found any problems.

I agree, I've implemented trim function on customer user data creation. It solves only this PR, for other implemented features I'll create issue.

@malyMiso malyMiso force-pushed the mt-registration-feapi branch 2 times, most recently from 6fe4cac to 8678505 Compare November 9, 2020 19:16
@malyMiso malyMiso mentioned this pull request Nov 10, 2020
Closed
grossmannmartin
grossmannmartin previously approved these changes Nov 10, 2020
View reviewed changes
Copy link
Member

@grossmannmartin grossmannmartin left a comment

Choose a reason for hiding this comment

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

Nice one! Thanks.

@TomasLudvik TomasLudvik added the Enhancement New feature or request for change from user point of view label Nov 10, 2020
@TomasLudvik TomasLudvik added this to the SSFW 9.1.0 milestone Nov 10, 2020
@TomasLudvik TomasLudvik changed the title added support for customer user registration to Frontend API [framework] added support for customer user registration to Frontend API Nov 10, 2020
sslt
sslt previously approved these changes Nov 10, 2020
View reviewed changes
@malyMiso malyMiso dismissed stale reviews from sslt and grossmannmartin via 5ec8db1 November 10, 2020 10:21
@sonarcloud
Copy link

sonarcloud bot commented Nov 10, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 1 Security Hotspot to review)
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@malyMiso malyMiso merged commit 438ee26 into master Nov 10, 2020
@malyMiso malyMiso deleted the mt-registration-feapi branch November 10, 2020 11:49
malyMiso added a commit that referenced this pull request Nov 11, 2020
@malyMiso
added support for customer user registration to Frontend API (#2100)
f0bb289
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grossmannmartin grossmannmartin grossmannmartin approved these changes

@sslt sslt sslt approved these changes

Assignees
No one assigned
Labels
Enhancement New feature or request for change from user point of view
Projects
None yet
Milestone
SSFW 9.1.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@malyMiso @grossmannmartin @sslt @TomasLudvik