clearbold / artx

1 stars 0 forks source link

Describe Remember Me goals? #52

Open heymarkreeves opened 9 years ago

heymarkreeves commented 9 years ago

Hi, @desigonz & @mailbackwards!

Can you describe your goals for Remember Me functionality at a high level?

Some questions that occur to us:

Thanks!

Mark

heymarkreeves commented 9 years ago

Noting that this is under the following bullet:

Caching username and password (on both add-to-homescreen and through Safari)

desigonz commented 9 years ago

Hi Mark,

Here's what we're thinking. We'd love your thoughts on these points, as this is new territory for us!

Thanks! Let us know if you have any questions.

heymarkreeves commented 9 years ago

Can you clarify what you mean by "persist all values until a Log Out"?

Based on what you've described, I think we'll just assume this. It was a catch-all statement.

But is there a way to include a visual cue on other pages? Possibly in the menu or on the Discovery page?

I was thinking in the menu. It doesn't need to compete with content on any of the screens, just be very accessible.

Thanks!

SherriAlexander commented 9 years ago

Hey there, @mailbackwards!

It sounds like the Remember Me checkbox is something that we'd have to add into the Settings API and add to the Settings screen. Is that something that you could put in place on the back end for the API, and then let me know what values it expects so I can implement the front-end piece? Thanks! :)

mailbackwards commented 9 years ago

Sure, I'll get working on this and let you know when it's pushed through. Thanks.

mailbackwards commented 9 years ago

I've just pushed up some code that should allow for a Remember Me checkbox in settings. Any PATCH or PUT to /preferences with a remember_me boolean argument will update the user's remember_created_at field, which is serialized in the user object. If remember_me is true, remember_created_at will be set to now (or will persist). If remember_me is false, remember_created_at will be set to nil. So to check whether a user wants to be remembered, you should be able to just check whether or not remember_created_at is nil.

"Remember Me" defaults to true, so if no remember_me is passed into registration or preferences, remember_created_at will be auto-set if it is not already.

This should be documented in API_ENDPOINTS.md. Let me know if you have any questions about it. Thanks!

SherriAlexander commented 9 years ago

Hey there, Liam!

I've been making progress on the Remember Me functionality today as well, working on the localStorage aspect. I had assumed that most of the Remember Me functionality would be done via a localStorage variable that was keyed to the user's email (so for example, remember_me = {"sma@clearbold.com" : true, "testa@example.com": true, "testb@example.com": false}), so that we didn't have to keep pinging the Preferences API and nesting AJAX calls.

LocalStorage variables are assumed to be permanent unless the user deletes them manually -- we don't have any mechanism in place to calculate expiration of the remember_me localStorage variable, and hopefully that's okay.

To document, my in-progress logic looks like this:

On page load, the site checks for an existing localStorage variable called remember_me:

This remember_me localStorage variable is used mainly to determine whether the auth token and current username are stored in localStorage (persisting) or sessionStorage (session) variables.

Logging in, the system will check remember_me to know which to use, and create the appropriate variables for auth token and username.

Logging out, the system will remove both localStorage and sessionStorage versions of the auth token and username variables (which should provide a clean slate for the next login).

Most of the functions that need an auth token and username will check sessionStorage variables first and then localStorage if there aren't sessionStorage variables defined.

The only time we'd need to check the API, I believe, would be if they went into the Settings screen to change it. Then I'd ping the Settings API to make sure the checkbox is reflecting the correct value (just in case somehow the localStorage got deleted or something, and I'd have the checkbox send any changes to the Settings API and change the remember_me localStorage variable upon success (and switch the existing auth token and username variables from one type to the other).

Let me know if this all sounds good to the team -- thanks! :)

mailbackwards commented 9 years ago

Hi Sherri-- Yes, I think this works for us! As long as you send a remember_me boolean to the /preferences API whenever the user updates their settings, it should update accordingly on our end. If the user wants to be remembered, the user's remember_created_at field will have a date value. If they don't, their remember_created_at will be nil. You can ping the API to get this information via GET /preferences.

It's no problem on our end that there's no way to calculate expiration. Let me know if that all works, or if there's something I'm missing. Thanks!

SherriAlexander commented 9 years ago

Sounds good -- I'll start on the Settings page functionality today then. Thanks!

SherriAlexander commented 9 years ago

Hey there, Liam!

Hmm...you had mentioned that "Remember Me" defaults to true, so if no remember_me is passed into registration or preferences, remember_created_at should be auto-set if it is not already.

But when I ping my sma@clearbold.com user data to find out what it is, it's returning null for the remember_created_at value, so it's actually defaulting to false. Possibly because I'm an already existing user?

Is there something we can do to assign it to pre-existing users? Thoughts? Thanks!

mailbackwards commented 9 years ago

Oh yes, I hadn't thought of that. I just went in and set a remember_created_at for pre-existing users, so you should be set now. Let me know if it doesn't work. Thanks!

SherriAlexander commented 9 years ago

Perfect! I was actually testing at the time, so I saw it come through. :) Thanks!

heymarkreeves commented 9 years ago

This one's ready for the team to test on http://staging.artx.clearbold.com/

You'll want to clear out your cache first.

Mark

desigonz commented 9 years ago

Hi Mark,

Thanks for working on this! I tested the Remember Me functionality on an Android (4.2.2) and it worked great. Liam also reports it working well on iOS 8, both Chrome and Safari.

A few things were wonky on my iOS7, which may or may not be related to the Remember Me changes:

heymarkreeves commented 9 years ago

Hi, @desigonz!

Did you clear all private data/cache/history in both Safari and Chrome on iOS 7 before testing?

I had some issues in Safari when I first tested, and when I switched to Chrome and reset private data, things were working correctly. There's updated JS in this release, so that may be a factor.

Mark

desigonz commented 9 years ago

Hi Mark—yes, I did all of the cache and history in both Safari and Chrome before testing, but I can try it again.

heymarkreeves commented 9 years ago

@desigonz Can you confirm the general difficulties in Safari? I'm not able to reproduce that in iOS 7.

We have reproduced the Chrome items and are investigating.

Thanks!

Mark

desigonz commented 9 years ago

Hi Mark—yes, the Safari difficulties are still happening for me after deleting history/cache/cookies several times. I'll see if I can get my hands on another iOS7 and reproduce it.

SherriAlexander commented 9 years ago

Hey there! A quick update on progress:

I should also note that none of these issues were caused by the recent Remember Me and Forgot Password coding, as far as I can tell. Thanks as always for your keen eyes!

SherriAlexander commented 9 years ago

Hey there, all! Another quick update -- I believe that Liam and I have fixed the 406 errors that were happening in Chrome for iOS now, so hopefully that will take care of the Favorite star issues (and the Interests list not loading, which we noticed while testing) in Chrome iOS! :)