Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

basket api notifier #818

Merged
merged 1 commit into from Sep 29, 2014
Merged

basket api notifier #818

merged 1 commit into from Sep 29, 2014

Conversation

dannycoates
Copy link
Contributor

Adds a new process for sending verified email addresses to the Basket API.

New Config

basket: {
  region: 'The region where the queues live',
  apiUrl: 'Url for the Basket API server',
  apiKey:  'Basket API key',
  queueUrl: 'The bounce queue URL'
}

New Deployment

This requires a new SQS queue that gets fed by the existing auth-server events SNS topic. The new node process bin/basket.js empties the queue.

@dannycoates
Copy link
Contributor Author

The SQS bits shared by email_bouncer.js and basket.js will be moved out as a module and each process will eventually become a separate repo.

/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

Copy link
Contributor

Choose a reason for hiding this comment

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

Particularly given the opaque name "basket", this file could use a high-level introductory comment to explain WTF it is for. Even just a couple of lines like Send details of account activity to engagment team using their "basket" API and a link to their wiki page or whatever.

@rfk
Copy link
Contributor

rfk commented Sep 28, 2014

LGTM overall

dannycoates added a commit that referenced this pull request Sep 29, 2014
@dannycoates dannycoates merged commit d2e2b1e into mozilla:master Sep 29, 2014
@ckarlof
Copy link
Contributor

ckarlof commented Sep 30, 2014

@dannycoates, given that we need to eventually notify them of legacy accounts created before we deploy this, do we have a way to distinguish between accounts that we've already notified them about vs ones we haven't?

@dannycoates
Copy link
Contributor Author

do we have a way to distinguish between accounts that we've already notified them about vs ones we haven't?

We have the createdAt date of the account. My plan was to backfill verified accounts created before notifier was deployed from most recently created to least. If the launch goes well that should be all we need.

If the launch doesn't go as smoothly as we hope we may need to get more crafty. In the worst case we can parse the notifier log to build a list of emails that succeeded then go through all the accounts again. I don't think we should be storing anything in the auth-server db for this.

Also it seems like the basket api will do an update if the email already exists, so it might be ok if an email gets registered multiple times, but it looks like an email might get sent in that case also https://github.com/mozilla/basket/blob/master/news/tasks.py#L311

@pmclanahan what happens if an email gets /news/fxa-registered twice?

@ckarlof
Copy link
Contributor

ckarlof commented Oct 1, 2014

Thanks Danny. SGTM.

@pmclanahan
Copy link

@pmclanahan what happens if an email gets /news/fxa-registered twice?

Currently they would get the welcome message again. We could easily make it skip sending the message if the user already had an fxa_id in basket.

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

4 participants