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

Feature/ogc 2070 mark invalid email addresses #1736

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/onegov/directory/collections/directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,16 @@
return self.query().filter(EntryRecipient.id == id).first()
return None

def by_address(
self,
address: str,
) -> EntryRecipient | None:

query = self.query()
query = query.filter(EntryRecipient.address == address)

Check warning on line 98 in src/onegov/directory/collections/directory.py

View check run for this annotation

Codecov / codecov/patch

src/onegov/directory/collections/directory.py#L97-L98

Added lines #L97 - L98 were not covered by tests

return query.first()

Check warning on line 100 in src/onegov/directory/collections/directory.py

View check run for this annotation

Codecov / codecov/patch

src/onegov/directory/collections/directory.py#L100

Added line #L100 was not covered by tests

def add(
self,
address: str,
Expand Down
24 changes: 23 additions & 1 deletion src/onegov/directory/models/directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@
return DirectoryEntryForm


class EntryRecipient(Base, TimestampMixin):
class EntryRecipient(Base, TimestampMixin, ContentMixin):
""" Represents a single recipient.
"""

Expand Down Expand Up @@ -628,6 +628,28 @@
nullable=False
)

@property
def is_inactive(self) -> bool:
"""
Checks if the directory entry recipient's email address is marked as
inactive.

Returns:
bool: True if the email address is marked as inactive, False
otherwise.
"""
return self.meta.get('inactive', False)

Check warning on line 641 in src/onegov/directory/models/directory.py

View check run for this annotation

Codecov / codecov/patch

src/onegov/directory/models/directory.py#L641

Added line #L641 was not covered by tests

def mark_inactive(self) -> None:
"""
Marks the recipient's email address as inactive.

This method sets the 'inactive' flag in the recipient's metadata to
True. It is typically used when a bounce event causes the email
address to be deactivated by Postmark.
"""
self.meta['inactive'] = True

Check warning on line 651 in src/onegov/directory/models/directory.py

View check run for this annotation

Codecov / codecov/patch

src/onegov/directory/models/directory.py#L651

Added line #L651 was not covered by tests


class EntrySubscription:
""" Adds subscription management to a recipient. """
Expand Down
16 changes: 15 additions & 1 deletion src/onegov/directory/upgrade.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"""
from __future__ import annotations

from onegov.core.orm.types import UTCDateTime
from onegov.core.orm.types import JSON, UTCDateTime
from onegov.core.upgrade import upgrade_task, UpgradeContext
from onegov.directory import Directory
from sqlalchemy import Column, Integer
Expand Down Expand Up @@ -69,3 +69,17 @@ def make_directory_models_polymorphic_type_non_nullable(
""")

context.operations.alter_column(table, 'type', nullable=False)


@upgrade_task('Add meta and content columns to entry recipients')
def add_meta_data_and_content_columns_to_entry_recipients(
context:
UpgradeContext
) -> None:
if not context.has_column('entry_recipients', 'meta'):
context.operations.add_column('entry_recipients',
Column('meta', JSON()))

if not context.has_column('entry_recipients', 'content'):
context.operations.add_column('entry_recipients',
Column('content', JSON))
75 changes: 42 additions & 33 deletions src/onegov/org/cronjobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from onegov.core.orm import find_models, Base
from onegov.core.orm.mixins.publication import UTCPublicationMixin
from onegov.core.templates import render_template
from onegov.directory.collections.directory import EntryRecipientCollection
from onegov.event import Occurrence, Event
from onegov.file import FileCollection
from onegov.form import FormSubmission, parse_form, Form
Expand Down Expand Up @@ -772,39 +773,47 @@

return ''

token = get_postmark_token()

recipients = RecipientCollection(request.session)
yesterday = utcnow() - timedelta(days=1)

try:
r = requests.get(
'https://api.postmarkapp.com/bounces?count=500&offset=0',
f'fromDate={yesterday.date()}&toDate='
f'{yesterday.date()}&inactive=true',
headers={
'Accept': 'application/json',
'X-Postmark-Server-Token': token
},
timeout=30
)
r.raise_for_status()
bounces = r.json().get('Bounces', [])
except requests.exceptions.HTTPError as http_err:
if r.status_code == 401:
raise RuntimeWarning(
f'Postmark API token is not set or invalid: {http_err}'
) from None
else:
raise

for bounce in bounces:
email = bounce.get('Email', '')
inactive = bounce.get('Inactive', False)
recipient = recipients.by_address(email)

if recipient and inactive:
recipient.mark_inactive()
def get_bounces() -> list[dict[str, Any]]:
token = get_postmark_token()
yesterday = utcnow() - timedelta(days=1)
r = None

Check warning on line 779 in src/onegov/org/cronjobs.py

View check run for this annotation

Codecov / codecov/patch

src/onegov/org/cronjobs.py#L776-L779

Added lines #L776 - L779 were not covered by tests

try:
r = requests.get(

Check warning on line 782 in src/onegov/org/cronjobs.py

View check run for this annotation

Codecov / codecov/patch

src/onegov/org/cronjobs.py#L781-L782

Added lines #L781 - L782 were not covered by tests
'https://api.postmarkapp.com/bounces?count=500&offset=0',
f'fromDate={yesterday.date()}&toDate='
f'{yesterday.date()}&inactive=true',
headers={
'Accept': 'application/json',
'X-Postmark-Server-Token': token,
},
timeout=30,
)
r.raise_for_status()
bounces = r.json().get('Bounces', [])
except requests.exceptions.HTTPError as http_err:
if r and r.status_code == 401:
raise RuntimeWarning(

Check warning on line 796 in src/onegov/org/cronjobs.py

View check run for this annotation

Codecov / codecov/patch

src/onegov/org/cronjobs.py#L792-L796

Added lines #L792 - L796 were not covered by tests
f'Postmark API token is not set or invalid: {http_err}'
) from None
else:
raise

Check warning on line 800 in src/onegov/org/cronjobs.py

View check run for this annotation

Codecov / codecov/patch

src/onegov/org/cronjobs.py#L800

Added line #L800 was not covered by tests

return bounces

Check warning on line 802 in src/onegov/org/cronjobs.py

View check run for this annotation

Codecov / codecov/patch

src/onegov/org/cronjobs.py#L802

Added line #L802 was not covered by tests

postmark_bounces = get_bounces()
collections = (RecipientCollection, EntryRecipientCollection)
for collection in collections:
recipients = collection(request.session)

Check warning on line 807 in src/onegov/org/cronjobs.py

View check run for this annotation

Codecov / codecov/patch

src/onegov/org/cronjobs.py#L804-L807

Added lines #L804 - L807 were not covered by tests

for bounce in postmark_bounces:
email = bounce.get('Email', '')
inactive = bounce.get('Inactive', False)
recipient = recipients.by_address(email)

Check warning on line 812 in src/onegov/org/cronjobs.py

View check run for this annotation

Codecov / codecov/patch

src/onegov/org/cronjobs.py#L809-L812

Added lines #L809 - L812 were not covered by tests

if recipient and inactive:
print(f'Mark recipient {recipient.address} as inactive')
recipient.mark_inactive()

Check warning on line 816 in src/onegov/org/cronjobs.py

View check run for this annotation

Codecov / codecov/patch

src/onegov/org/cronjobs.py#L814-L816

Added lines #L814 - L816 were not covered by tests


@OrgApp.cronjob(hour=4, minute=30, timezone='Europe/Zurich')
Expand Down
11 changes: 11 additions & 0 deletions src/onegov/org/templates/directory_entry_recipients.pt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@
${title}
</tal:b>
<tal:b metal:fill-slot="content">
<tal:b condition="not:count">
<p i18n:translate="">No subscribers yet</p>
</tal:b>

<tal:b condition="count">
<p i18n:translate>There are currently <tal:b i18n:name="count">${count}</tal:b> recipients registered.</p>
</tal:b>

<tal:b tal:repeat="letter by_letter">
<h2>${letter}</h2>

Expand All @@ -26,6 +34,9 @@
i18n:attributes="data-confirm-extra;data-confirm-yes;data-confirm-no">
unsubscribe
</a>
<span tal:condition="recipient.is_inactive|nothing" class="small-text info-text" title="This recipient has delivery failures, including hard bounces, invalid email addresses, spam complaints, manual deactivations, or being blocked. We recommend unsubscribing it from the list." i18n:attributes="title">
<i class="fa fa-exclamation-triangle"></i>
</span>
</li>
</ul>
</tal:b>
Expand Down
1 change: 1 addition & 0 deletions src/onegov/org/views/directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -1190,6 +1190,7 @@ def view_directory_entry_update_recipients(
'recipients': recipients,
'warning': warning,
'by_letter': by_letter,
'count': recipients.count()
}


Expand Down
11 changes: 11 additions & 0 deletions src/onegov/town6/templates/directory_entry_recipients.pt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@
${title}
</tal:b>
<tal:b metal:fill-slot="content">
<tal:b condition="not:count">
<p i18n:translate="">No subscribers yet</p>
</tal:b>

<tal:b condition="count">
<p i18n:translate>There are currently <tal:b i18n:name="count">${count}</tal:b> recipients registered.</p>
</tal:b>

<tal:b tal:repeat="letter by_letter">
<h2>${letter}</h2>

Expand All @@ -26,6 +34,9 @@
i18n:attributes="data-confirm-extra;data-confirm-yes;data-confirm-no">
unsubscribe
</a>
<span tal:condition="recipient.is_inactive|nothing" class="small-text info-text" title="This recipient has delivery failures, including hard bounces, invalid email addresses, spam complaints, manual deactivations, or being blocked. We recommend unsubscribing it from the list." i18n:attributes="title">
<i class="fa fa-exclamation-triangle"></i>
</span>
</li>
</ul>
</tal:b>
Expand Down
48 changes: 46 additions & 2 deletions tests/onegov/org/test_cronjobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -1495,7 +1495,7 @@ def test_update_newsletter_email_bounce_statistics(org_app, handlers):
job = get_cronjob_by_name(org_app,
'update_newsletter_email_bounce_statistics')
job.app = org_app
# tz = ensure_timezone('Europe/Zurich')
tz = ensure_timezone('Europe/Zurich')

transaction.begin()

Expand All @@ -1505,6 +1505,30 @@ def test_update_newsletter_email_bounce_statistics(org_app, handlers):
recipients.add('[email protected]', confirmed=True)
recipients.add('[email protected]', confirmed=True)

# create directory entry recipients
directories = DirectoryCollection(org_app.session(), type='extended')
directory_entries = directories.add(
title='Baugesuche (Planauflage)',
structure="""
Gesuchsteller/in *= ___
""",
configuration=DirectoryConfiguration(
title="[Gesuchsteller/in]",
)
)
directory_entries.add(values=dict(
gesuchsteller_in='Amon',
publication_start=datetime(2024, 4, 1, tzinfo=tz),
publication_end=datetime(2024, 4, 10, tzinfo=tz),
))
entry_recipients = EntryRecipientCollection(org_app.session())
entry_recipients.add('[email protected]', directory_entries.id,
confirmed=True)
entry_recipients.add('[email protected]', directory_entries.id,
confirmed=True)
entry_recipients.add('[email protected]', directory_entries.id,
confirmed=True)

transaction.commit()
close_all_sessions()

Expand All @@ -1517,7 +1541,9 @@ def test_update_newsletter_email_bounce_statistics(org_app, handlers):
{'RecordType': 'Bounce', 'ID': 3719297970,
'Inactive': False, 'Email': '[email protected]'},
{'RecordType': 'Bounce', 'ID': 4739297971,
'Inactive': True, 'Email': '[email protected]'}
'Inactive': True, 'Email': '[email protected]'},
{'RecordType': 'Bounce', 'ID': 5739297972,
'Inactive': True, 'Email': '[email protected]'}
]
},
raise_for_status=Mock(return_value=None),
Expand All @@ -1534,6 +1560,12 @@ def test_update_newsletter_email_bounce_statistics(org_app, handlers):
'[email protected]').is_inactive is True
assert RecipientCollection(org_app.session()).by_address(
'[email protected]').is_inactive is False
assert EntryRecipientCollection(org_app.session()).by_address(
'[email protected]').is_inactive is False
assert EntryRecipientCollection(org_app.session()).by_address(
'[email protected]').is_inactive is True
assert EntryRecipientCollection(org_app.session()).by_address(
'[email protected]').is_inactive is False

# test raising runtime warning exception for status code 401
with patch('requests.get') as mock_get:
Expand Down Expand Up @@ -1562,6 +1594,18 @@ def test_update_newsletter_email_bounce_statistics(org_app, handlers):
with pytest.raises(requests.exceptions.HTTPError):
client.get(get_cronjob_url(job))

recipients = RecipientCollection(org_app.session())
assert recipients.query().count() == 3
assert recipients.by_address('[email protected]').is_inactive is False
assert recipients.by_address('[email protected]').is_inactive is True
assert recipients.by_address('[email protected]').is_inactive is False

entry_recipients = EntryRecipientCollection(org_app.session())
assert entry_recipients.query().count() == 3
assert entry_recipients.by_address('[email protected]').is_inactive is False
assert entry_recipients.by_address('[email protected]').is_inactive is True
assert entry_recipients.by_address('[email protected]').is_inactive is False


def test_delete_unconfirmed_subscribers(org_app, handlers):
register_echo_handler(handlers)
Expand Down
1 change: 1 addition & 0 deletions tests/onegov/org/test_views_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -986,6 +986,7 @@ def test_directory_entry_subscription(client):
assert "[email protected] wurde erfolgreich" in page

page = client.get('/directories/trainers/+recipients')
assert 'Zur Zeit sind 2 Abonnenten registriert' in page
assert '[email protected]' in page
assert '[email protected]' in page

Expand Down
1 change: 1 addition & 0 deletions tests/onegov/town6/test_views_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ def test_directory_entry_subscription(client):
assert "[email protected] wurde erfolgreich" in page

page = client.get('/directories/trainers/+recipients')
assert 'Zur Zeit sind 3 Abonnenten registriert' in page
assert '[email protected]' in page
assert '[email protected]' in page
assert '[email protected]' in page
Expand Down
Loading