micro / services

Real World Micro Services
Apache License 2.0
1.25k stars 137 forks source link

fix validatePostUserData #365

Closed lambdaR closed 2 years ago

lambdaR commented 2 years ago

what is the intended behavior?

do you want username, email and id to be unique? or just username and email. why do check for account.Id != "" is it related to the migration from db to store

this PR corrects just username and email checks.

lambdaR commented 2 years ago

current behavior: 1- users/Create {joe@example.com, joe, 123456} => OK 2- users/Create {debo@example.com, joe, 123456} => OK, previous record get updated (email and created part) 3- users/Create {debo@example.com, joe, 123456} => OK, previous record get updated (created part) 4- users/Create {debo@example.com, debo, 123456} => OK, previous record get updated (username and created part) 5- users/Create {joe@example.com, debo, 123456} => OK, previous record get updated (email and created part) 6- users/Create {joe@example.com, debo, 1234} => ERROR, username already exist

the preferred way i think is to check for uniqueness and existence of username, email and Id all together.

asim commented 2 years ago

I don't really remember the logic around this, so my best guess, related to migration

lambdaR commented 2 years ago

let me handle Id too and test it locally before review

lambdaR commented 2 years ago

Summary:

validatePostUserData:

users/Create:

users/Update now handles three cases:

lambdaR commented 2 years ago

One final note regarding users/Update, the docs says Update updates username or email and didn't mention profile yet the profile attribute is included.

With this PR you can update profile too in the three cases mentioned above but not in the case where only Id and profile were provided.

We can handle that case here too or we can remove the profile attribute and create a new endpoint for that.

What do you think?