-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Logout of Twitter #645
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
Logout of Twitter #645
Conversation
Signed-off-by: Alex Saveau <[email protected]>
@@ -239,6 +247,10 @@ public Void then(@NonNull Task<GoogleApiClient> task) throws Exception { | |||
// Facebook sign out | |||
LoginManager.getInstance().logOut(); | |||
|
|||
// Twitter sign out |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samtstern Signing out of Twitter and Facebook should probably a method (something like signOutProviders
), but that will cause more thrashing in #609. Would you like me to do the refactor anyway and then undo it in #609 or are we not going through with #609?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep sign out all as one method for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM!
@@ -192,6 +196,10 @@ public Void then(@NonNull Task<GoogleApiClient> task) throws Exception { | |||
// Facebook sign out | |||
LoginManager.getInstance().logOut(); | |||
|
|||
// Twitter sign out | |||
if (!Fabric.isInitialized()) TwitterProvider.initialize(activity); | |||
Twitter.logOut(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we also want to call:
Twitter.getSessionManager().clearActiveSession();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samtstern I thought so too, but Twitter.logOut()
already does that for us:
// In Twitter.java
public static void logOut() {
checkInitialized();
getInstance().core.logOut(); // core == TwitterCore
}
// In TwitterCore.java
public void logOut() {
checkInitialized();
final SessionManager<TwitterSession> sessionManager = getSessionManager();
if (sessionManager != null) {
sessionManager.clearActiveSession();
}
}
public static SessionManager<TwitterSession> getSessionManager() {
checkInitialized();
return getInstance().core.getSessionManager(); // Same reference as logOut in TwitterCore.java
}
Actually, now that I'm paying more attention, it looks like that's all Twitter#logOut()
does.
The docs for TwitterCore#logOut()
say it logs the user out, so I think that's all we have to do:
/**
* Logs out the user, clearing user session. This will not make a network request to invalidate
* the session.
*
* @throws java.lang.IllegalStateException if {@link io.fabric.sdk.android.Fabric}
* or {@link TwitterCore} has not been initialized.
*/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for digging, works for me!
The Twitter docs don't have anything at all on signOut ... oversight I guess.
#644