CKAN: Ticket #1413: Ask users to 'add email address' when logged in

status, description changed; owner set

dread — Thu, 27 Oct 2011 13:20:46 GMT

owner set to seanh
status changed from new to assigned
description modified (diff)

seanh — Mon, 28 Nov 2011 11:18:33 GMT

I've made a first attempt at this here:

https://github.com/seanh/ckan/compare/feature-1413-ask-users-to-add-email-address

I used flash_notice() to ask the user to add an email whenever they visit their account page, if they don't have one already. This means they'll see the notice when they login, because after login they are redirected to their account page. Issues:

Is it okay to approximate 'once they log in' as 'whenever they visit their account page' like this?

Do I need to do something to support i18n for the string I added? I looked at users of flash_*() elsewhere and they seemed to be hardcoding the strings in English.

dread — Mon, 28 Nov 2011 11:53:55 GMT

approximate 'once they log in' as 'whenever they visit their account page'

I had imagined that this would be done in the user.py:logged_in method, but doing it in your way sounds like an even better plan.

Another thought is: a lot of people might just use the cookie and v. rarely logout/in. Perhaps it could be done when they come back to the site at intervals (e.g. daily)?

i18n

Yes your strings should be internationalised because they are user visible.

You're right that these other strings should be internationalised. I've created a separate ticket for this: #1491

seanh — Tue, 29 Nov 2011 14:05:34 GMT

Another thought is: a lot of people might just use the cookie and v. rarely logout/in. Perhaps it could be done when they come back to the site at intervals (e.g. daily)?

As discussed on IRC I've moved the notice from the user account page to the front page, in the hopes that people will see it often enough even if they are always logged in via a cookie. As discussed on IRC, we could make the notice appear on every page but no more than once per day, but that would be more complicated to implement and more likely to break.

Yes your strings should be internationalised because they are user visible.

Done.

You're right that these other strings should be internationalised. I've created a separate ticket for this: #1491

Also done! :)

seanh — Tue, 29 Nov 2011 19:32:40 GMT

Added a 'please enter your full name' flash notice as well (the two are combined into one if they have neither full name nor email), added unit tests, fixed some issues.

Still need to adjust the strings and change the full name notice so that it applies to people with gmail IDs only.

https://github.com/seanh/ckan/compare/feature-1413-ask-users-to-add-email-address

seanh — Mon, 05 Dec 2011 12:28:11 GMT

Adjusted the 'please add your email' string and changed the 'please add your full name' notice to be used only for Google OpenID users. Finished?

https://github.com/seanh/ckan/commits/feature-1413-ask-users-to-add-email-address

status changed; resolution set

seanh — Wed, 07 Dec 2011 13:24:06 GMT

status changed from assigned to closed
resolution set to fixed

dread — Wed, 07 Dec 2011 14:51:12 GMT

Code reviewed - looks really good. Do merge in to master if it is not already.

A couple of tips:

we all use 'model.User' instead of 'model.user.User' but it's not important for tests...
probably better to use CreateTestData?.create_user() to create user objects in the future, avoiding faffing around with creating revisions etc. that are also nastier to debug when they go wrong.

dread — Thu, 23 Feb 2012 18:08:06 GMT

Went into CKAN 1.6