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

Bug 742951 - changing email address #84

Closed
wants to merge 1 commit into from
Closed

Bug 742951 - changing email address #84

wants to merge 1 commit into from

Conversation

dpoirier
Copy link

Ready to review, but not to merge. The triggered email sends don't work for brand new email addresses (that aren't in ET yet), and the confirmation messages in ET are just placeholders, not the final text.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 58cb1a3 on dpoirier:bug-742951-change-email into 01125e4 on mozilla:master.

@dpoirier
Copy link
Author

dpoirier commented Oct 3, 2013

Squashed.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling c568322 on dpoirier:bug-742951-change-email into 01125e4 on mozilla:master.

@pmclanahan
Copy link

I'm wondering if we need the activation thing to be so complex, and whether we need them to hang around. I'm thinking perhaps all we need is for a row to be created when a change is requested, and to be considered active if it's been less than CONFIRMATION_DAYS (or whatever). Then we run a daily cron to delete the expired ones, and they delete themselves when they complete successfully. We can log success and failure activity, but I don't think we need for this table to just grow, and having that status column seems superfluous.

@dpoirier
Copy link
Author

Will simplify the activation thing as suggested.

@dpoirier
Copy link
Author

@pmclanahan Updated the activation logic, but spotted some other problems. Updated the pull request description with the latest status.

@@ -183,7 +184,7 @@ The following URLs are available (assuming "/news" is app url):
Returns information about all of the available newsletters::

method: GET
fiends: *none*

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol

@dpoirier
Copy link
Author

This is currently blocked because ET won't let us send a triggered email to an address that's not already in our data at ET (as best i can tell, see this bug comment). Waiting for jdavis to respond.

@dpoirier
Copy link
Author

Could this pull be closed if it's not going to be needed? I'm trying to neaten my GitHub open issues and pulls.

@pmclanahan
Copy link

Sure. Also, hi Dan 👋

@pmclanahan pmclanahan closed this Sep 15, 2015
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.

None yet

3 participants