CRNS-317: handling deleted properties of user #727
Conversation
|
Size Change: +1.02 kB (0%) Total Size: 242 kB
|
vishalnarkhede
requested review from
AnatolyRugalev,
ferhatelmas,
mahboubii,
thesyncim and
tsirlucas
src/client.ts
Outdated
|
|
||
| // Remove deleted properties from user objects. | ||
| for (const key in this.user) { | ||
| if (key in event.user || ownUserProperties.includes(key)) { |
There was a problem hiding this comment.
why do we care about the ownUserProperties here?
There was a problem hiding this comment.
We don't want to remove those ownUserProperties ... they are kinda reserved fields!!
There was a problem hiding this comment.
oh i see and they are not in the event object, in that case maybe we should extract these fields and put them somewhere as a source of truth, in future, if we add more reserve fields and forgot to add them here it will cause hard to find issues
vishalnarkhede
force-pushed
the
vishal/user-deleted-property-fix
branch
from
117b0d4
to
62e289a
Compare
src/utils.ts
Outdated
| ChannelType, | ||
| CommandType, | ||
| UserType | ||
| >() as string[]).includes(property); |
There was a problem hiding this comment.
is ts forcing you to cast here?
vishalnarkhede
force-pushed
the
vishal/user-deleted-property-fix
branch
from
f80062c
to
95a5f3b
Compare
src/client.ts
Outdated
| } | ||
|
|
||
| if (this.user?.[key]) { | ||
| delete this.user[key]; |
There was a problem hiding this comment.
hey maybe create a shallow copy of user and delete properties on it instead of deleting this.user[key] directly? this way we can only set this.user in the end of the function so we will only mutate it once instead of manually deleting the keys during the function execution.
this is useful to avoid half-updated states in case this function fails in the middle of its execution.
There was a problem hiding this comment.
makes sense :)
src/client.ts
Outdated
| delete this.user[key]; | ||
| } | ||
|
|
||
| if (this._user?.[key]) { |
There was a problem hiding this comment.
whats the difference between user and _user here? just a question, not really a review feedback
There was a problem hiding this comment.
This comment might answer this one!!
…am/stream-chat-js into vishal/user-deleted-property-fix
CLA
Description of the changes, What, Why and How?
When user gets updated on backend, the event handler for event
user.updateddoesn't take care of properties which were removed. Purpose of this PR is to remove such deleted properties fromclient.userandclient._userobject.