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

Adds logic to manage suppressions #214

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

Conversation

gavin-truework
Copy link

  • Adds calls to list, add, and delete suppression support
  • Adds doc pages

I wanted to open this to add some basic support for managing suppressions in Postmark. This just adds a few methods to manage them based on the provided API.

I am not 100% sure how I should test this. Do you have a testing account setup that the test should attempt to list, add, and remove a suppression to? I also am not super familiar with docs, so please let me know if I need to do something else.

- Adds calls to list, add, and delete suppression support
- Adds doc pages
Copy link
Owner

@Stranger6667 Stranger6667 left a comment

Choose a reason for hiding this comment

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

Hm, I need some time to check the testing side here, I didn't touch the repo in a while, not sure if my test account is still active

To view the current suppression list:
.. code-block:: python

>>> response = postmark.get_suppressions(stream_id="test")
Copy link
Owner

Choose a reason for hiding this comment

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

I think it would be better to unify suppressions with other sub-sections, so it is postmark.suppressions.all(stream_id="test")

Copy link
Owner

Choose a reason for hiding this comment

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

See an example here

Copy link
Author

Choose a reason for hiding this comment

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

Gotcha, I think I switched over to a sub-section.

>>> print(response['Suppressions'])
{
"Suppressions":[
{
Copy link
Owner

Choose a reason for hiding this comment

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

It might be useful to return a list of some light-weight object instead of a dict with Suppressions inside

data = {'Suppressions': []}
for email in emails:
data['Suppressions'].append({'EmailAddress': email})
response = self._call("POST", url, "", data)
Copy link
Owner

Choose a reason for hiding this comment

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

I'd suggest using self.root_api_url instead for root and endpoint separately

- Add a suppression and suppression_response object to handle responses from the API
- Updates _call calls to use endpoints
- Dedupes code
- Update docs to reflect new returns
@Z33DD
Copy link
Contributor

Z33DD commented Feb 29, 2024

@Stranger6667 Check this later

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