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

feat: make sure emails are lowercase and valid #1451

Merged
merged 1 commit into from
Nov 16, 2023
Merged

feat: make sure emails are lowercase and valid #1451

merged 1 commit into from
Nov 16, 2023

Conversation

Serpentiel
Copy link
Contributor

@Serpentiel Serpentiel commented Nov 16, 2023 •
edited
Loading

About this change—what it does

makes sure emails are lowercase and valid

uppercase emails are not accepted by our backend

resolves #1450

Why this way

it should be this way

@Serpentiel Serpentiel added the bug Something isn't working label Nov 16, 2023
@Serpentiel Serpentiel requested a review from a team November 16, 2023 08:56
@Serpentiel Serpentiel force-pushed the aleks-fix-emails-should-be-lowercase branch from 57e5b14 to 4654ae5 Compare November 16, 2023 08:58
@byashimov
Copy link
Contributor

byashimov commented Nov 16, 2023 •
edited
Loading

  1. The original issue refers to v3, though it probably is related to v4 too
  2. Why this infinite loop even happens? Does it ignore an error in create handler or something?

@ivan-savciuc
Copy link
Contributor

It is also relevant to v4

ivan-savciuc
ivan-savciuc suggested changes Nov 16, 2023
View reviewed changes
Copy link
Contributor

@ivan-savciuc ivan-savciuc left a comment

Choose a reason for hiding this comment

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

Consider adding validation to the field instead of force converting to the lowercase string!

internal/sdkprovider/service/account/account_team_member.go Outdated Show resolved Hide resolved
@Serpentiel
Copy link
Contributor Author

Serpentiel commented Nov 16, 2023 •
edited
Loading

Why this infinite loop even happens?

because the emails are lowercased by our backend, and when an uppercase email is provided, our backend replies with a lowercased version of it, and Read infinitely waits for a different email, e.g:

@Serpentiel Serpentiel force-pushed the aleks-fix-emails-should-be-lowercase branch from 4654ae5 to c207f5a Compare November 16, 2023 09:55
@Serpentiel Serpentiel changed the title fix: emails should be lowercase feat: make sure emails are lowercase and valid Nov 16, 2023
@Serpentiel Serpentiel added enhancement New feature or request and removed bug Something isn't working labels Nov 16, 2023
ivan-savciuc
ivan-savciuc suggested changes Nov 16, 2023
View reviewed changes
Copy link
Contributor

@ivan-savciuc ivan-savciuc left a comment

Choose a reason for hiding this comment

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

Regexp is wrong and please use validation helpers

internal/sdkprovider/service/account/account_team_member.go Show resolved Hide resolved
internal/schemautil/schemautil.go Outdated Show resolved Hide resolved
@Serpentiel Serpentiel force-pushed the aleks-fix-emails-should-be-lowercase branch from c207f5a to 2e2e196 Compare November 16, 2023 10:29
ivan-savciuc
ivan-savciuc approved these changes Nov 16, 2023
View reviewed changes
Copy link
Contributor

@ivan-savciuc ivan-savciuc left a comment

Choose a reason for hiding this comment

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

LGTM

@ivan-savciuc ivan-savciuc merged commit d0ae4f6 into main Nov 16, 2023
10 checks passed
@ivan-savciuc ivan-savciuc deleted the aleks-fix-emails-should-be-lowercase branch November 16, 2023 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ivan-savciuc ivan-savciuc ivan-savciuc approved these changes

Assignees

@ivan-savciuc ivan-savciuc

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

User invite with capitals in email would cause an infinite loop
3 participants
@Serpentiel @byashimov @ivan-savciuc