Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add mailchimp opt-in uploader #1850

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Add mailchimp opt-in uploader #1850

wants to merge 7 commits into from

Conversation

ajparsons
Copy link
Contributor

@ajparsons ajparsons commented Dec 16, 2024

This PR is a simplified version of #1813

Currently we have contact optins when people create accounts, but we don't automatically move them to mailchimp.

This adds uploading new registrations from yesterday to the mailchimp account.

This also adds a basic django ORM approach to accessing the TWFY database from python.

It needs a new MAILCHIMP_API_KEY in the config.

- Upload optins to relevant mailchimp lists daily
@ajparsons ajparsons changed the title Contact io Add mailchimp opt-in uploader Dec 17, 2024
@ajparsons ajparsons requested a review from dracos December 17, 2024 09:59
Copy link
Member

@dracos dracos left a comment

Choose a reason for hiding this comment

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

Couple of small things is all

from ..common.enum_backport import StrEnum
from .model_helper import UnmanagedDataclassModel, field

datetime_min = datetime.datetime(1, 1, 1, 0, 0, 0)
Copy link
Member

Choose a reason for hiding this comment

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

datetime.min exists :)

client = MailChimpHandler(config.MAILCHIMP_API_KEY)

# get internal id for the mailing list
mailing_list_id = client.list_name_to_unique_id(mailing_list_name)
Copy link
Member

Choose a reason for hiding this comment

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

I mean, it doesn't matter apart from the fact I have to deal with inefficient bots bringing down our services and so I don't want to ever be thought of as something like that, but can we call this once rather than 4 times (once per option, then again in line 65 before the batch add? :)

@@ -0,0 +1,59 @@
from typing import Any, Callable, TypeVar
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand the typing but I'm sure it's lovely

@@ -0,0 +1,686 @@
import datetime
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, this is quite a big file and lots of it doesn't appear to be used by this PR. Do you want me to review the whole file? Seems to be quite a lot of repetitive boilerplate, e.g. list_name_to_unique_id/ segment_name_to_unique_id/ template_name_to_unique_id all seem to do the same thing, just on a different thing; I don't see why MailchimpHandler has to call top-level functions of exactly the same name with all the arguments having to be repeated, that kind of thing. I'm sure it's all fine 🤷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants