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.

Sign up for GitHub

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

Jump to bottom

refresh-user #224

Merged
merged 6 commits into from
Mar 15, 2018
Merged

refresh-user #224

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5a4f144
Refreshing the user on app resumes.
eoji Mar 13, 2018
3d044ee
:sweat_smile:
eoji Mar 14, 2018
1b795e6
Lots of changes!
eoji Mar 15, 2018
f9857d6
cleaning up the lint
eoji Mar 15, 2018
e40626c
remove log
eoji Mar 15, 2018
baa95f1
Adding a comment to help clarify what `userHasChanged` does
eoji Mar 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions app/src/main/java/com/kickstarter/libs/utils/ApplicationLifecycleUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,28 @@
import android.os.Bundle;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.util.Pair;

import com.facebook.appevents.AppEventsLogger;
import com.kickstarter.KSApplication;
import com.kickstarter.libs.CurrentConfigType;
import com.kickstarter.libs.CurrentUserType;
import com.kickstarter.libs.Koala;
import com.kickstarter.libs.Logout;
import com.kickstarter.libs.rx.transformers.Transformers;
import com.kickstarter.models.User;
import com.kickstarter.services.ApiClientType;
import com.kickstarter.services.apiresponses.ErrorEnvelope;

import javax.inject.Inject;

import rx.Observable;

public final class ApplicationLifecycleUtil implements Application.ActivityLifecycleCallbacks, ComponentCallbacks2 {
protected @Inject Koala koala;
protected @Inject ApiClientType client;
protected @Inject CurrentConfigType config;
protected @Inject CurrentUserType currentUser;
protected @Inject Logout logout;

private final KSApplication application;
Expand Down Expand Up @@ -55,6 +61,16 @@ public void onActivityResumed(final @NonNull Activity activity) {
.compose(Transformers.neverError())
.subscribe(c -> this.config.config(c));

// Refresh the user
final Observable<User> user = this.currentUser.observable();

final String accessToken = this.currentUser.getAccessToken();

Observable.just(accessToken)
.filter(ObjectUtils::isNotNull)
.zipWith(user, Pair::create)
.subscribe(tokenUserPair -> this.currentUser.refresh(tokenUserPair.second));
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey as far as I can tell (but I might be missing something in the Java) is that this.currentUser.refresh() will just refresh with whatever is passed to it. So I think this will just take the currentUser and "refresh" it again. At which point do we make a new call to the API to fetch a fresh user?

Also, in this particular scenario it might not even be necessary to use RxJava because we're just imperatively lifting the token into FRP land in order to subscribe to it (with the user) and then to refresh the user with the user 🤔

I think I might need a little more insight into how the lifecycle stuff works in Android but maybe we should pair on this 💪

Copy link
Contributor Author

@eoji eoji Mar 14, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

😅 look for my calendar invite!

I changed this line! Which is where we get a fresh user 🙏 https://github.com/kickstarter/android-oss/pull/224/files#diff-40963cc3fde7a9c5d143820bc2075139R65

Copy link
Contributor

Choose a reason for hiding this comment

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

Aah got it! I missed the fetch part of that 😄

Then actually this is probably fine but couldn't it just be:

this.client.fetchCurrentUser()
  .filter(accessToken != null) //pseudeocode
  .subscribe(freshUser -> this.currentUser.refresh(freshUser))

Think that just removes the need to lift the accessToken into RxJava and to zip it with the new user.


this.isInBackground = false;
}
}
Expand Down