RCORE-2200: Fix switch_user and get_profile during log_in_with_credentials #7894
Conversation
gagik
changed the title
Pull Request Test Coverage Report for Build github_pull_request_299776Details
💛 - Coveralls |
| switch_user(user); | ||
| get_profile(user, std::move(completion)); | ||
|
|
||
| get_profile(user, [this, completion = std::move(completion)](const std::shared_ptr<User>& user, |
There was a problem hiding this comment.
I haven't worked with C++ in a little while, tried to get it to be equivalent but let me know if this move, etc. doesn't make sense
src/realm/object-store/sync/app.cpp
Outdated
|
|
||
| get_profile(user, [this, completion = std::move(completion)](const std::shared_ptr<User>& user, | ||
| Optional<AppError> error) { | ||
| switch_user(user); |
There was a problem hiding this comment.
We probably only want to switch users if the profile fetch succeeded?
There was a problem hiding this comment.
Would that be a better experience? We'd potentially be breaking more functionality which isn't necessarily dependent on the user's profile itself (i.e. giving the client the new user on completition callback but it having no profile and an error to handle seems nicer than not giving the client the new user at all or giving it on the completition callback with the error but not actually switching the user)
There was a problem hiding this comment.
If the initial profile fetch fails then the user effectively isn't logged in yet and isn't really in a usable state.
There was a problem hiding this comment.
@tgoyne made the change, let me know if more is needed
…rofile-notification-order
What, How & Why?
Fixes #7889
☑️ ToDos
bindgen/spec.yml, if public C++ API changed