[Fix bug 921107] Update email in Exact Target #833

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
@dpoirier
Contributor

dpoirier commented Feb 27, 2014

When someone changes their email in Mozillians.org, move their exact
target subscriptions to the new email address.

Also take advantage of new basket subscribe(sync=Y) to simplify
the update tasks.

[Fix bug 921107] Update email in exact target
When someone changes their email in Mozillians.org, move their exact
target subscriptions to the new email address.

Also take advantage of new basket subscribe(sync=Y) to simplify
the update tasks.
@dpoirier

This comment has been minimized.

Show comment Hide comment
@dpoirier

dpoirier Mar 17, 2014

Contributor

This is a large-ish change. The rationale is that now basket has a way we can do subscribes synchronously again, so we can rely on getting back the new token immediately, and can remove the code that we needed in order to look up the token later.

Contributor

dpoirier commented Mar 17, 2014

This is a large-ish change. The rationale is that now basket has a way we can do subscribes synchronously again, so we can rely on getting back the new token immediately, and can remove the code that we needed in order to look up the token later.

+ instance.basket_token = token = retval['token']
+ # Don't call .save() on a userprofile from here, it would invoke us again
+ # via the post-save signal, which would be pointless.
+ UserProfile.objects.filter(pk=instance.pk).update(basket_token=token)

This comment has been minimized.

Show comment Hide comment
@glogiotatidis

glogiotatidis Mar 24, 2014

Member

This block, starting from line 102, is both in if and else (line 148). Let's move it after else ends.

@glogiotatidis

glogiotatidis Mar 24, 2014

Member

This block, starting from line 102, is both in if and else (line 148). Let's move it after else ends.

- update_basket_phonebook_task.apply_async(args=[instance.user.pk], countdown=10)
-
+ # Remember the token
+ instance.basket_token = token = retval['token']

This comment has been minimized.

Show comment Hide comment
@glogiotatidis

glogiotatidis Mar 24, 2014

Member

Do you mind changing this into two lines for readability?

@glogiotatidis

glogiotatidis Mar 24, 2014

Member

Do you mind changing this into two lines for readability?

@glogiotatidis

This comment has been minimized.

Show comment Hide comment
@glogiotatidis

glogiotatidis Mar 24, 2014

Member

https://github.com/mozilla/mozillians/pull/833/files#diff-266059f660d7f465446b47aa47888d1dR160

Can you change this line to include functional areas and not just curated groups? Before the "curated groups" changes, functional areas were the only groups with curators therefore we used to filters them this way.

Member

glogiotatidis commented Mar 24, 2014

https://github.com/mozilla/mozillians/pull/833/files#diff-266059f660d7f465446b47aa47888d1dR160

Can you change this line to include functional areas and not just curated groups? Before the "curated groups" changes, functional areas were the only groups with curators therefore we used to filters them this way.

@glogiotatidis

This comment has been minimized.

Show comment Hide comment
@glogiotatidis

glogiotatidis Mar 24, 2014

Member

To make sure I understand it correctly:

  • I 'm subscribed to 4 mozilla newsletters including mozillians.org newsletter with address foo@example.com
  • I change my address on mozillians.org to bar@example.com
  • All my 4 subscriptions now go to bar@example.com?
Member

glogiotatidis commented Mar 24, 2014

To make sure I understand it correctly:

  • I 'm subscribed to 4 mozilla newsletters including mozillians.org newsletter with address foo@example.com
  • I change my address on mozillians.org to bar@example.com
  • All my 4 subscriptions now go to bar@example.com?
@glogiotatidis

This comment has been minimized.

Show comment Hide comment
@glogiotatidis

glogiotatidis Mar 26, 2014

Member

I understand from the bug that we should only change "phonebook" newsletter subscription? @dpoirier can you please verify? If you don't have the time to work on this I can take over.

Member

glogiotatidis commented Mar 26, 2014

I understand from the bug that we should only change "phonebook" newsletter subscription? @dpoirier can you please verify? If you don't have the time to work on this I can take over.

@dpoirier

This comment has been minimized.

Show comment Hide comment
@dpoirier

dpoirier Mar 26, 2014

Contributor

@glogiotatidis I don't really know. I was assuming that someone changing their email address on mozillians would want all their subscriptions moved, but that might not be what was intended by this feature.

Contributor

dpoirier commented Mar 26, 2014

@glogiotatidis I don't really know. I was assuming that someone changing their email address on mozillians would want all their subscriptions moved, but that might not be what was intended by this feature.

@dpoirier

This comment has been minimized.

Show comment Hide comment
@dpoirier

dpoirier Mar 27, 2014

Contributor

It sounds like the equivalent of the old exclude(curator=None) would be filter(functional_area=True)?

Unfortunately I won't have time to continue working on this, but all these comments seem reasonable.

Contributor

dpoirier commented Mar 27, 2014

It sounds like the equivalent of the old exclude(curator=None) would be filter(functional_area=True)?

Unfortunately I won't have time to continue working on this, but all these comments seem reasonable.

@glogiotatidis

This comment has been minimized.

Show comment Hide comment
@glogiotatidis

glogiotatidis Mar 27, 2014

Member

OK i'll take over from now on. Thanks for working on this @dpoirier

Member

glogiotatidis commented Mar 27, 2014

OK i'll take over from now on. Thanks for working on this @dpoirier

@glogiotatidis

This comment has been minimized.

Show comment Hide comment
@glogiotatidis

glogiotatidis Mar 27, 2014

Member

Moved here #872

Member

glogiotatidis commented Mar 27, 2014

Moved here #872

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