Skip to content
This repository was archived by the owner on Jan 5, 2026. It is now read-only.

Revised user id logic.#1590

Merged
tonyanziano merged 1 commit into
masterfrom
toanzian/id-fix
May 22, 2019
Merged

Revised user id logic.#1590
tonyanziano merged 1 commit into
masterfrom
toanzian/id-fix

Conversation

@tonyanziano
Copy link
Copy Markdown
Contributor

This PR implements the following behavior:

  1. Gets rid of default id (unnecessary because the emulator initializes with a GUID on startup anyway)
  2. Starting a conversation will use the custom id defined in settings or it will fallback to a GUID
  3. Restarting with a new id will start with a new GUID no matter what
  4. Restarting with the same id will start with whatever id was being used before (could be previous custom id, or GUID)

NOTE There is one case in which the above logic does not hold. When previously using a custom userId, and then clearing it out in the settings page, and then starting a new conversation, that previous custom user id is still used. This is because it was set as both the previous id and the custom id, so the logic falls back to the previous id.

Due to the fragility and complexity of the start conversation logic, I feel that it's not worth it to try to accommodate this case because it is easily avoided by restarting the conversation again with a new user id. We also have plans in the near future to simplify this logic, so we can implement this behavior at that time.

justinwilaby
justinwilaby previously approved these changes May 22, 2019
Copy link
Copy Markdown
Contributor

@justinwilaby justinwilaby left a comment

Choose a reason for hiding this comment

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

:shipit:

@tonyanziano tonyanziano merged commit a562689 into master May 22, 2019
@tonyanziano tonyanziano deleted the toanzian/id-fix branch May 22, 2019 20:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants