From f93073401c722a4cb5c658dc7cdb8249f25e1ffd Mon Sep 17 00:00:00 2001 From: Sarina Canelake Date: Mon, 13 Apr 2015 16:05:30 -0400 Subject: [PATCH] Quality fixups for Mailchimp scripts --- .../management/commands/mailchimp_id.py | 11 +++++ .../commands/mailchimp_sync_announcements.py | 36 +++++++++----- .../commands/mailchimp_sync_course.py | 47 +++++++++++++------ 3 files changed, 66 insertions(+), 28 deletions(-) diff --git a/lms/djangoapps/mailing/management/commands/mailchimp_id.py b/lms/djangoapps/mailing/management/commands/mailchimp_id.py index 4d440be3ac..7159b9478f 100644 --- a/lms/djangoapps/mailing/management/commands/mailchimp_id.py +++ b/lms/djangoapps/mailing/management/commands/mailchimp_id.py @@ -1,3 +1,7 @@ +""" +mailchimp_id: Returns whether or not a given mailchimp key represents +a valid list. +""" import sys from optparse import make_option @@ -7,6 +11,10 @@ from mailsnake import MailSnake class Command(BaseCommand): + """ + Given a mailchimp key, validates that a list with that key + exists in mailchimp. + """ args = '' help = 'Get the list id from a web_id' @@ -26,6 +34,9 @@ class Command(BaseCommand): return options['key'], options['web_id'] def handle(self, *args, **options): + """ + Validates that the id passed in exists in mailchimp. + """ key, web_id = self.parse_options(options) mailchimp = MailSnake(key) diff --git a/lms/djangoapps/mailing/management/commands/mailchimp_sync_announcements.py b/lms/djangoapps/mailing/management/commands/mailchimp_sync_announcements.py index bc6c86323b..e160abfce9 100644 --- a/lms/djangoapps/mailing/management/commands/mailchimp_sync_announcements.py +++ b/lms/djangoapps/mailing/management/commands/mailchimp_sync_announcements.py @@ -1,28 +1,28 @@ +""" +Synchronizes the announcement list with all active students. +""" import logging -import math -import random -import itertools -from itertools import chain from optparse import make_option -from collections import namedtuple - from django.core.management.base import BaseCommand, CommandError -from mailsnake import MailSnake - from django.contrib.auth.models import User -from .mailchimp_sync_course import (connect_mailchimp, get_subscribed, - get_unsubscribed, get_cleaned, - subscribe_with_data) +from .mailchimp_sync_course import ( + connect_mailchimp, get_cleaned, + get_subscribed, get_unsubscribed, + subscribe_with_data +) log = logging.getLogger('edx.mailchimp') class Command(BaseCommand): + """ + Synchronizes the announcement list with all active students. + """ args = '' - help = 'Synchronizes a mailchimp list with the students of a course.' + help = 'Synchronizes the announcement list with all active students.' option_list = BaseCommand.option_list + ( make_option('--key', action='store', help='mailchimp api key'), @@ -59,11 +59,21 @@ class Command(BaseCommand): def get_enrolled(): - # cdodge: filter out all users who signed up via a Microsite, which UserSignupSource tracks + """ + Filter out all users who signed up via a Microsite, which UserSignupSource tracks + """ + ## TODO (Feanil) This grabs all INactive students and MUST be changed + ## TODO (Feanil) blame cdodge for your troubles return User.objects.raw('SELECT * FROM auth_user where id not in (SELECT user_id from student_usersignupsource)') def get_data(users, exclude=None): + """ + users: set of Django users + exclude [optional]: set of Django users to exclude + + returns: {'EMAIL': u.email} for all users in users less those in `exclude` + """ exclude = exclude if exclude else set() emails = (u.email for u in users) return ({'EMAIL': e} for e in emails if e not in exclude) diff --git a/lms/djangoapps/mailing/management/commands/mailchimp_sync_course.py b/lms/djangoapps/mailing/management/commands/mailchimp_sync_course.py index 49b0e07e1c..973f21fb7d 100644 --- a/lms/djangoapps/mailing/management/commands/mailchimp_sync_course.py +++ b/lms/djangoapps/mailing/management/commands/mailchimp_sync_course.py @@ -1,7 +1,9 @@ +""" +Synchronizes a mailchimp list with the students of a course. +""" import logging import math import random -import time import itertools from itertools import chain from optparse import make_option @@ -27,6 +29,9 @@ FIELD_TYPES = {'EDX_ID': 'text'} class Command(BaseCommand): + """ + Synchronizes a mailchimp list with the students of a course. + """ args = '' help = 'Synchronizes a mailchimp list with the students of a course.' @@ -56,6 +61,7 @@ class Command(BaseCommand): options['course_id'], options['segments']) def handle(self, *args, **options): + """Synchronizes a mailchimp list with the students of a course.""" key, list_id, course_id, nsegments = self.parse_options(options) log.info('Syncronizing email list for %s', course_id) @@ -86,8 +92,11 @@ class Command(BaseCommand): make_segments(mailchimp, list_id, nsegments, subscribed) -def connect_mailchimp(key): - mailchimp = MailSnake(key) +def connect_mailchimp(api_key): + """ + Initializes connection to the mailchimp api + """ + mailchimp = MailSnake(api_key) result = mailchimp.ping() log.debug(result) @@ -95,6 +104,10 @@ def connect_mailchimp(key): def verify_list(mailchimp, list_id, course_id): + """ + Verifies that the given list_id corresponds to the course_id + Returns boolean: whether or not course_id matches list_id + """ lists = mailchimp.lists(filters={'list_id': list_id})['data'] if len(lists) != 1: @@ -103,7 +116,7 @@ def verify_list(mailchimp, list_id, course_id): list_name = lists[0]['name'] - log.debug('list name: %s' % list_name) + log.debug('list name: %s', list_name) # check that we are connecting to the correct list parts = course_id.replace('_', ' ').replace('/', ' ').split() @@ -128,7 +141,7 @@ def get_student_data(students, exclude=None): e = {'EMAIL': v['user__email'], 'FULLNAME': v['name'].title()} - fake_user = FakeUser(v['user_id'], v['user__username'], lambda:True) + fake_user = FakeUser(v['user_id'], v['user__username'], lambda: True) e['EDX_ID'] = unique_id_for_user(fake_user) return e @@ -231,8 +244,10 @@ def subscribe_with_data(mailchimp, list_id, user_data): batch=batch, double_optin=False, update_existing=True) - log.debug("Added: {} Error on: {}".format( - result['add_count'], result['error_count'])) + + log.debug( + "Added: %s Error on: %s", result['add_count'], result['error_count'] + ) def make_segments(mailchimp, list_id, count, emails): @@ -247,17 +262,19 @@ def make_segments(mailchimp, list_id, count, emails): emails = list(emails) random.shuffle(emails) - chunk_size = int(math.ceil(float(len(emails))/count)) + chunk_size = int(math.ceil(float(len(emails)) / count)) chunks = list(chunk(emails, chunk_size)) # create segments and add emails - for n in xrange(count): - name = 'random_{0:002}'.format(n) + for seg in xrange(count): + name = 'random_{0:002}'.format(seg) seg_id = mailchimp.listStaticSegmentAdd(id=list_id, name=name) - for batch in batches(chunks[n], BATCH_SIZE): - mailchimp.listStaticSegmentMembersAdd(id=list_id, - seg_id=seg_id, - batch=batch) + for batch in batches(chunks[seg], BATCH_SIZE): + mailchimp.listStaticSegmentMembersAdd( + id=list_id, + seg_id=seg_id, + batch=batch + ) def name_to_tag(name): @@ -271,4 +288,4 @@ def batches(iterable, size): def chunk(l, n): for i in xrange(0, len(l), n): - yield l[i:i+n] + yield l[i:i + n]