fix: validate display name on account creation #4994
Conversation
|
@igor-sirotin you are the owner of an account with |
Jenkins BuildsClick to see older builds (15)
|
| @@ -74,6 +76,10 @@ func ValidateAccountCreationRequest(c CreateAccount) error { | |||
| return ErrCreateAccountInvalidDisplayName | |||
There was a problem hiding this comment.
This can be removed, I suppose?
There was a problem hiding this comment.
I need it as it will point to a specific error in a specific case. In this example it will tell, that create account has an error
There was a problem hiding this comment.
validate tells only why validation was failed
There was a problem hiding this comment.
I meant the if len(c.DisplayName) == 0 but wasn't able to reach this line in code review.
There was a problem hiding this comment.
ValidateDisplayName all us to have an empty name, but for account creation it is restricted
There was a problem hiding this comment.
Ah, I see. Then maybe we could parameterize ValidateDisplayName. Not a big deal though.
There was a problem hiding this comment.
There's parametrisation introduced here: #4980
mprakhov
force-pushed
the
mp/issue-14128
branch
from
49795ea
to
6b05eb8
Compare
mprakhov
force-pushed
the
mp/issue-14128
branch
from
3ad8f76
to
d4a37a7
Compare
There was a problem hiding this comment.
As I mentioned, the fix doesn't seem to apply for status-desktop, because CreateAccountAndLogin endpoint is not yet used by it (there's a PR: status-im/status-desktop#14139).
Currently this code is used in desktop to create account and store a new account:
Could be that we don't need to update the old endpoints, if this PR and status-im/status-desktop#14139 are going to be released in same milestone. There's a keycard case though, which is not yet switch to new endpoints (#4977) 🤔
| @@ -74,6 +76,10 @@ func ValidateAccountCreationRequest(c CreateAccount) error { | |||
| return ErrCreateAccountInvalidDisplayName | |||
There was a problem hiding this comment.
There's parametrisation introduced here: #4980
|
@igor-sirotin From what I see you don't validate This PR will just add this check. I don't think there will be a huge conflicts |
@mprakhov sure, that's cool! Or do we want/need to ensure that old endpoints are fine as well? I wonder what will happen with keycard account creation even with my PR. @jrainville |
Up to you, I can put these changes into your branch in order to test all changes in one PR :) |
mprakhov
force-pushed
the
mp/issue-14128
branch
from
d4a37a7
to
835adce
Compare
|
Taking into account that @igor-sirotin merged his changes already, will rebase on develop branch and merge mine. No sense in postponing now :) |
Check that the display name was set according to the requirements
Closes # status-im/status-desktop#14128