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

Add updateuser callback #152 #34

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

stijn-ringeling
Copy link
Contributor

I did change the full name to email. The updateUser does not require a full name.

@arandr
Copy link
Contributor

arandr commented May 13, 2015

Thank you for the update. I have a few comments, but they're mostly merge errors I believe.
I'm not sure why you would want to store the sessionId in shared preferences. The sessionId expires after a (ideally short) time, so you don't need persistent storage for it. The password makes more sense but it is somewhat sensitive information (but I guess storing it that way is necessary if we don't want the user to have to login manually every time they restart the app).

Also, changing the way you stored the sessionId means you need to update it in the saved chars activity (you may have missed it while merging).

In addition, I just tested and at the moment signup doesn't return the sessionId, so we do need to make a login call after the signup one.

@stijn-ringeling
Copy link
Contributor Author

@arandr I pushed a new commit containing updates concerning your coments. The sessionId gets saved in a sharedPreference so other activities like the settingsActivity can use it. I fixed the crash on login by adding the sessionId to the user object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants