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 more ActionKit methods #870

Merged
merged 6 commits into from
Sep 8, 2023
Merged

Conversation

crayolakat
Copy link
Collaborator

  • get_blackholed_email
  • blackhole_email
  • delete_user_data

Copy link
Contributor

@jafayer jafayer 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 documentation questions but otherwise lgtm! I can't merge but @shaunagm can take over!

@@ -233,6 +233,59 @@ def update_event(self, event_id, **kwargs):
)
logger.info(f"{resp.status_code}: {event_id}")

def get_blackholed_email(self, email):
"""
Get a blackholed email.
Copy link
Contributor

Choose a reason for hiding this comment

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

Because "blackholing" an email is a bit of an ActionKit-specific term, it might be helpful to either have a brief description of what a blackholed email is or a link to the ActionKit docs for blackholed emails.

Definition per ActionKit:

You may want to prevent certain email addresses or domains from receiving bulk and transactional messages from ActionKit.

Link: https://docs.actionkit.com/docs/manual/guide/mailings_tools.html#blackhole


def blackhole_email(self, email):
"""
Blackhole an email.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could also use clarification on the definition of "blackhole".

Copy link
Collaborator

@shaunagm shaunagm left a comment

Choose a reason for hiding this comment

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

Agree with Josh that a little clarification in the docstrings for the "blackholed email" methods would be helpful, but it's not a blocker so I'll approve the PR. @crayolakat, feel free to merge as-is or update the docstrings and then merge.

@shaunagm shaunagm merged commit c291ad3 into move-coop:main Sep 8, 2023
5 checks passed
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.

3 participants