Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

notify chat subsystem on identifies #21910

Merged
merged 7 commits into from Jan 9, 2020

Conversation

@heronhaye
Copy link
Contributor

heronhaye commented Jan 6, 2020

No description provided.

@@ -347,6 +347,11 @@ func (e *Identify2WithUID) run(m libkb.MetaContext) {
err := e.runReturnError(m)
e.unblock(m /* isFinal */, true, err)

if !e.arg.IdentifyBehavior.IsChatIdentify() {

This comment has been minimized.

Copy link
@heronhaye

heronhaye Jan 6, 2020

Author Contributor

Need to be careful not to cause an infinite loop here.

Alternate solution is to export the results of this identify to TLFBreaks and call globals.CtxIdentifyNotifier(ctx) directly from a new notification handler, without running an identify. But I thought that might end up out of sync with the chat's regular identifies which runs Identify2 with different parameters than the original call may be using.

This comment has been minimized.

Copy link
@heronhaye

heronhaye Jan 7, 2020

Author Contributor

Maybe safer would be to only do this on the GUI profile behavior.

This comment has been minimized.

Copy link
@mmaxim

mmaxim Jan 7, 2020

Contributor

We can always just not use UserChanged here

@heronhaye heronhaye requested a review from mmaxim Jan 6, 2020
@heronhaye heronhaye force-pushed the surya/Y2K-1153/notify-chatsystem-on-identifies branch from f17ba35 to 9bb490d Jan 7, 2020
}}
}
m.Debug("Identify2WithUID.run: running HandleChatIdentifyUpdate: %#v", update)
m.G().NotifyRouter.HandleChatIdentifyUpdate(m.Ctx(), update)

This comment has been minimized.

Copy link
@heronhaye

heronhaye Jan 7, 2020

Author Contributor

Note this doesn't update the chat's CachingIdentifyNotifier's cache. Is that ok? If not, my thoughts are
to make another one of these listeners like HandleUserChanged, but for HandleIdentityFinished. That can update the chat's cache.

This comment has been minimized.

Copy link
@mmaxim

mmaxim Jan 8, 2020

Contributor

Should be fine

// affects username coloring in the GUI).
func (b TLFIdentifyBehavior) ShouldRefreshChatView() bool {
switch b {
case TLFIdentifyBehavior_GUI_PROFILE, TLFIdentifyBehavior_CLI:

This comment has been minimized.

Copy link
@heronhaye

heronhaye Jan 7, 2020

Author Contributor

update on both going to gui profile and IDing in CLI

@heronhaye

This comment has been minimized.

Copy link
Contributor Author

heronhaye commented Jan 7, 2020

made some changes, ptal @mmaxim

@mmaxim
mmaxim approved these changes Jan 8, 2020
@heronhaye heronhaye force-pushed the surya/Y2K-1153/notify-chatsystem-on-identifies branch from 3686fd5 to 7a76215 Jan 8, 2020
@heronhaye

This comment has been minimized.

Copy link
Contributor Author

heronhaye commented Jan 9, 2020

Merging this over CI flakes.

@heronhaye heronhaye merged commit 8bb96b0 into master Jan 9, 2020
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/jenkins/pr-head This commit cannot be built
Details
ci/circleci Your tests passed on CircleCI!
Details
@heronhaye heronhaye deleted the surya/Y2K-1153/notify-chatsystem-on-identifies branch Jan 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.