Skip to content
This repository has been archived by the owner on Aug 26, 2022. It is now read-only.

Newsletter signup 936123 #1736

Merged
merged 6 commits into from
Dec 9, 2013
Merged

Conversation

groovecoder
Copy link
Contributor

Superseding #1723 with the next step - backend for subscribing and un-subscribing from the newsletter during profile edit.

To spot-check:

  1. Change vendor/ to groovecoder/kuma-lib@d58975d and update its submodules to get the updated basket-client library
  2. Go to https://developer-local.allizom.org/admin/constance/config/ and add the BASKET_API_KEY from the bottom of https://mana.mozilla.org/wiki/display/websites/developer.mozilla.org+Cluster?focusedCommentId=38540359#comment-38540359
  3. Go to https://developer-local.allizom.org/profile/edit
  4. Scroll down to the bottom and check the newsletter form works for checking and un-checking
  5. (Optional) You may get subscription notifications to your profile's email address, but I haven't fully debugged that

groovecoder and others added 4 commits December 3, 2013 10:33
move newsletter subscription form into an include template

show newsletter subscription form on profile edit screen

put profile newsletter subscription behind waffle flag
@openjck
Copy link
Contributor

openjck commented Dec 7, 2013

I was able to check the box and save my profile, but did not see a confirmation email. When I went to edit my profile a second time, the box was unchecked again.

Aside from the check being remembered, how can I check that I was successfully subscribed?

@groovecoder
Copy link
Contributor Author

Hmm ... You have to either sign into exact target (big PITA), or you can use the python basket client directly:

vagrant@developer-local:~/src$ ./manage.py shell
Python 2.7.3 (default, Aug  1 2012, 05:14:39)                                                 [17/91]
Type "copyright", "credits" or "license" for more information.

IPython 0.10 -- An enhanced Interactive Python.
?         -> Introduction and overview of IPython's features.
%quickref -> Quick reference.
help      -> Python's own help system.
object?   -> Details about 'object'. ?object also works, ?? prints more.

In [1]: import basket

In [2]: import constance.config

In [3]: basket.lookup_user(email='luke.crouch@gmail.com', api_key=constance.config.BASKET_API_KEY)
2013-12-06 18:38:34,377 DEBUG (0.000) SELECT `constance_config`.`id`, `constance_config`.`key`, `cons
tance_config`.`value` FROM `constance_config` WHERE `constance_config`.`key` = 'BASKET_API_KEY' ; arg
s=('BASKET_API_KEY',)
2013-12-06 18:38:34,391 INFO Starting new HTTPS connection (1): basket.mozilla.com             [0/91]
2013-12-06 18:38:35,141 DEBUG "GET /news/lookup-user/?email=luke.crouch%40gmail.com HTTP/1.1" 200 Non
e
Out[3]:
{u'confirmed': True,
 u'country': u'us',
 u'created-date': u'11/7/2010 9:13:00 AM',
 u'email': u'luke.crouch@gmail.com',
 u'format': u'H',
 u'lang': u'en-US',
 u'master': True,
 u'newsletters': [u'mozilla-and-you', u'mozilla-phone'],
 u'pending': False,
 u'status': u'ok',
 u'token': u'8a218f17-60e9-4fa6-a992-19d2bcd95493'}

After signing up, it should show:

Out[4]:
{u'confirmed': True,
 u'country': u'us',
 u'created-date': u'11/7/2010 9:13:00 AM',
 u'email': u'luke.crouch@gmail.com',
 u'format': u'H',
 u'lang': u'en-US',
 u'master': True,
 u'newsletters': [u'mozilla-and-you', u'app-dev', u'mozilla-phone'],
 u'pending': False,
 u'status': u'ok',
 u'token': u'8a218f17-60e9-4fa6-a992-19d2bcd95493'}

i.e., 'app-dev' is in the newsletters list.

@openjck
Copy link
Contributor

openjck commented Dec 7, 2013

Your instructions always seem to work exactly as described. Thanks!

I do not see app-dev in the newsletter list.

Out[3]: 
{u'confirmed': True,                                                                                                                                    [0/84]
 u'country': u'',
 u'created-date': u'11/11/2011 10:47:00 AM',
 u'email': u'myrealaddress@example.com,
 u'format': u'H',
 u'lang': u'',
 u'master': True,
 u'newsletters': [u'mozilla-phone'],
 u'pending': False,
 u'status': u'ok',
 u'token': u'e1b1eeaa-99db-41f6-aecd-49dc01bdd4ed'}

I have a sneaking suspicion I have missed something. As I mention earlier, my choice to be on the mailing list does not persist either. But my state does seem normal enough...

$ git log --pretty=oneline | head -n 4
99775caf3dd45327b2bca68c8f8cb296715463cd bug 936123 - put Subscription into UserProfileEdit
f96e91fe033049598e4ed06dc4a87a549b66b3b8 update vendor to include basket-client update
42b1f8ea169229f8d433298f2500e9ebe51a250a Improve newsletter signup front-end
bf87fafba1446cb6abdbbb60da8abc2a56a477e0 bug 936123 - add newsletter signup form to profile

$ git branch | grep \*
* pr/1736

$ git log --pretty=oneline vendor | head -n 1
f96e91fe033049598e4ed06dc4a87a549b66b3b8 update vendor to include basket-client update

The profile_subscription flag is also enabled and BASKET_API_KEY is set. Are you sure the key is correct?

@groovecoder
Copy link
Contributor Author

Yeah, that's it. Hmm ... did you git submodule update --init inside vendor/? Is your basket-client submodule on 1ca127a17e17f5e67c2d9f9a500d85e97460f13d?

(kuma)lcrouch-14912:vendor lcrouch$ git submodule
...
 1ca127a17e17f5e67c2d9f9a500d85e97460f13d src/basket-client (v1.0-51-g1ca127a)
...

If you're feeling adventurous for python, you can put import pdb; pdb.set_trace() in devmo.views.profile_edit on line 132. i.e., where the HTTP request is sent to the backend. Then you can step thru the code line-by-line and inspect values with pdb debugger commands. Or you could put some logging.debug() lines in.

Or I'll be around today so you can ping me.

@groovecoder
Copy link
Contributor Author

I went ahead and refactored the browser_register view to match since the basket subscribe WFM.

So, this is done as far as the backend goes, except for any changes we may need to make the UX better via the front-end, or until we discover why the basket subscribe doesn't work on other machines. :/

@groovecoder
Copy link
Contributor Author

@openjck - email me the email address you're using to spot-check and I'll check Exact Target manually for it.

@openjck
Copy link
Contributor

openjck commented Dec 8, 2013

Yeah, that's it. Hmm ... did you git submodule update --init inside vendor/?

I get the following, but I had previously run git submodule update --init --recursive from the Kuma root.

$ git submodule update --init
fatal: reference is not a tree: 2906726af82ed0063a559124cee610bcd2e5859c
Unable to checkout '2906726af82ed0063a559124cee610bcd2e5859c' in submodule path 'src/bleach'

Is your basket-client submodule on 1ca127a17e17f5e67c2d9f9a500d85e97460f13d?

Yeah, seems to be.

$ git submodule | grep basket-client
 1ca127a17e17f5e67c2d9f9a500d85e97460f13d src/basket-client (v1.0-51-g1ca127a)

@openjck
Copy link
Contributor

openjck commented Dec 8, 2013

Sure enough, I was missing something. My local pr/1736 branch was out of sync with mozilla. All of the commits were there in my history, but somehow apps/devmo/views.py was out of date.

(Usually I git branch -D pr/#### && git fetch mozilla && git co pr/#### when re-testing a pull request where history has been rewritten, but I must have forgotten to do it this time.)

Anyway, I was able to sign up from both the profile edit form and the new account sign-up form. app-dev showed up in the newsletters lists and I received confirmation emails.

The interface is good enough to launch with, in my opinion. Might try moving the checkbox to the left of the label later tonight, as well as moving the form higher up in the page.

@groovecoder
Copy link
Contributor Author

w00t! Hopefully @darkwing will have some spare cycles before the push to maybe give this a once-over? :)

@groovecoder
Copy link
Contributor Author

Updated with an update to mock basket out of test_valid_assertion_with_new_account_creation, but I'd like to refactor that mock to something like this setUp method code to fix the rest.

@lmorchard or @ubernostrum - have a good idea how to do that? That current code gives me _patch object has no method start :(

@lmorchard
Copy link
Contributor

That looks a bit like what I tried doing here in the wiki view test, but can't do it without upgrading the Mock package in vendor.

@darkwing
Copy link
Contributor

darkwing commented Dec 9, 2013

My concern is that we don't appear to be storing if the user has signed up or not, so they can't go to the "edit profile" page to unsubscribe. The other issue is that form submission sends the user to their profile page without a "thank you for subscribing" message. Would either of these be possible?

@darkwing darkwing closed this Dec 9, 2013
@darkwing darkwing reopened this Dec 9, 2013
@darkwing
Copy link
Contributor

darkwing commented Dec 9, 2013

Sorry, didn't mean to close this.

@openjck
Copy link
Contributor

openjck commented Dec 9, 2013

My concern is that we don't appear to be storing if the user has signed up or not, so they can't go to the "edit profile" page to unsubscribe. The other issue is that form submission sends the user to their profile page without a "thank you for subscribing" message. Would either of these be possible?

We are storing the setting. I had trouble seeing this on my box, too, but saw it once I corrected my setup. Good point about a welcome message. That should be easy enough with Django, right?

@groovecoder
Copy link
Contributor Author

We aren't storing the subscription, we're asking basket if they're subscribed or not. We shouldn't store it because users can unsubscribe via the email links that go straight to Exact Target and not to MDN. Users are able to unsubscribe via the Edit Profile page - they can un-check the newsletter box. But that's the part that probably isn't obvious.

update tests with newsletter fields
@groovecoder
Copy link
Contributor Author

Updated tests pass jenkins so this is good to merge & iterate.

openjck added a commit that referenced this pull request Dec 9, 2013
@openjck openjck merged commit 3df9251 into mdn:master Dec 9, 2013
@groovecoder groovecoder deleted the newsletter-signup-936123 branch September 1, 2015 13:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants