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

Improved handling of users without cookies #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gert314
Copy link

@gert314 gert314 commented Dec 10, 2011

Hi Dan

Firstly excellent project, a simple non invasive add-on to any site!

Your USER_AGENT_BLACKLIST setting works for the well known search engines (that we know about and can add to the list) but I found a lot of "unknown" search engines and user agents with cookies disabled can really fill up your user table.

All we need to do is detect if the user agent accepts cookies and only then create the lazy user. Detection is normally a two step process but the session middleware already does the first step for us. We just need to check for the presence of the sessionid cookie. If its not there don't create the user.

The add-on is two lines in the decorator but is a huge plus.

Regards
Gert Steyn

@danfairs
Copy link
Owner

Hi Gert,

Looks good to me! Any chance you can add a test an a note in the docs, and I'll be happy to merge.

Incidentally, which version are you running? I'm looking for 'works for me's from folks using th latest and greatest before cutting a non-beta release.

Thanks for your contribution! :)

Cheers
Dan

@gert314
Copy link
Author

gert314 commented Apr 18, 2012

Hi Dan

Sorry, not finding time to do a test... jip, using the latest version.

Regards
Gert

@cridenour
Copy link

So I've been toying around with this off and on for a while now. Finally got a chance to really work on it.

The problem comes up that once you add this line of code, almost half of the tests break. I thought it was because the test suite needed to be tweaked - but the only way you could possibly have the tests (and in consequence, real life) work is if you never wanted the LazySignup to work on the first page users arrived at. If they came to a lazy signup enabled page first, they would not have a cookie from our site and would not be able to leverage the uses.

AFAIK, the session middleware does not in fact do anything to test for cookie support more than anything else. They do provide the calls set_test_cookie and test_cookie_worked, but those would only help us for subsequent calls. However, while it may be helpful to use these calls to provide an error message to the user - their usefulness in this is none.

So while checking for cookie support could absolutely minimize robot usage, it could negatively impact use for end-users as well.

@danfairs
Copy link
Owner

Thanks for taking the time to check this out, @cridenour. I see what you mean. I wonder if there's another way around this. I'll try to find some time to have a play myself.

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

Successfully merging this pull request may close these issues.

None yet

3 participants