Skip to content

Update the right call.id into state#1818

Merged
PorterNan merged 8 commits intomainfrom
jinan/update-call-id-fix
Apr 26, 2022
Merged

Update the right call.id into state#1818
PorterNan merged 8 commits intomainfrom
jinan/update-call-id-fix

Conversation

@PorterNan
Copy link
Copy Markdown
Contributor

What

Update the call.id in state when call id gets changed

Why

How Tested

Process & policy checklist

  • I have updated the project documentation to reflect my changes if necessary.
  • I have read the CONTRIBUTING documentation.

Is this a breaking change?

  • This change causes current functionality to break.

@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "ui change" label to the PR for updating the snapshot.

public setCallId(newCallId: string, oldCallId: string): void {
this.modifyState((draft: CallClientState) => {
const call = draft.calls[oldCallId];
call.id = newCallId;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So this means the Call object that we get may not be the most updated call object and we want to apply on the correct Id before we set the Call object to the correct key?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The id value in state.call.id has never been updated before this fix... Although we only use it in a very extreme case

Since this will be run at for the first callIdChange event, so this code will execute ahead of all the other state change, we should be good to assume customer will always get the right value in state

Comment thread packages/calling-stateful-client/src/CallContext.ts Outdated
@prprabhu-ms prprabhu-ms self-assigned this Apr 26, 2022
@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "ui change" label to the PR for updating the snapshot.

@PorterNan PorterNan enabled auto-merge (squash) April 26, 2022 21:08
@PorterNan PorterNan disabled auto-merge April 26, 2022 21:17
@PorterNan PorterNan enabled auto-merge (squash) April 26, 2022 21:24
@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "ui change" label to the PR for updating the snapshot.

@prprabhu-ms
Copy link
Copy Markdown
Contributor

Unassigning to remove from attention set. Please reassign if you need another review.

@prprabhu-ms prprabhu-ms removed their assignment Apr 26, 2022
@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "ui change" label to the PR for updating the snapshot.

@PorterNan PorterNan merged commit 38ad6c7 into main Apr 26, 2022
@PorterNan PorterNan deleted the jinan/update-call-id-fix branch April 26, 2022 23:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants