Skip to content

Removing specific filters for heartbeat notifications #2159

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.

Sign up for GitHub

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

Merged
ben851 merged 7 commits into from
Aug 31, 2026
Merged

Removing specific filters for heartbeat notifications #2159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
186cdab
Removing specific filters for heartbeat notifications.
jimleroyer Apr 28, 2025
13bcbe5
Fixing test where heartbeat filter was still defined
jimleroyer Apr 28, 2025
6618fe0
Merge remote-tracking branch 'origin/main' into refactor/rm-heartbeat…
jimleroyer Apr 28, 2025
8e766fb
Merge remote-tracking branch 'origin/main' into refactor/rm-heartbeat…
jimleroyer Apr 29, 2025
6d0b46b
merge
ben851 Aug 27, 2026
b106a66
Merge branch 'main' into refactor/rm-heartbeat-filters
jimleroyer Aug 31, 2026
979bfbd
formatting
ben851 Aug 31, 2026
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
4 changes: 2 additions & 2 deletions app/main/views/index.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ def sitemap():

@main.route("/activity", endpoint="activity")
def activity():
return render_template("views/activity.html", **get_latest_stats(get_current_locale(current_app), filter_heartbeats=True))
return render_template("views/activity.html", **get_latest_stats(get_current_locale(current_app)))


@cache.memoize(timeout=12 * 60 * 60)
Expand Down Expand Up @@ -457,7 +457,7 @@ def _render_articles_page(response, newsletter_form=None):
nav_items=nav_items,
slug=slug_en,
lang_url=get_lang_url(response, bool(page_id)),
stats=get_latest_stats(get_current_locale(current_app), filter_heartbeats=True) if slug_en == "home" else None,
stats=get_latest_stats(get_current_locale(current_app)) if slug_en == "home" else None,
isHome=True if slug_en == "home" else None,
newsletter_form=newsletter_form,
newsletter_subscribed=request.args.get("subscribed") == "1",
Expand Down
4 changes: 2 additions & 2 deletions app/notify_client/service_api_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,11 @@ def get_services(self, params_dict=None):
"""
return self.get("/service", params=params_dict)

def get_stats_by_month(self, filter_heartbeats=False):
def get_stats_by_month(self):
"""
Retrieve notifications stats by month.
"""
return self.get("/service/delivered-notifications-stats-by-month-data", params={"filter_heartbeats": filter_heartbeats})
return self.get("/service/delivered-notifications-stats-by-month-data")

def find_services_by_name(self, service_name):
return self.get("/service/find-services-by-name", params={"service_name": service_name})
Expand Down
6 changes: 3 additions & 3 deletions app/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ def from_lambda_api(line):


@cache.memoize(timeout=12 * 60 * 60)
def get_latest_stats(lang, filter_heartbeats=None):
results = service_api_client.get_stats_by_month(filter_heartbeats=filter_heartbeats)["data"]
def get_latest_stats(lang):
results = service_api_client.get_stats_by_month()["data"]

monthly_stats = {}
emails_total = 0
Expand Down Expand Up @@ -135,7 +135,7 @@ def get_latest_stats(lang, filter_heartbeats=None):

@cache.memoize(timeout=24 * 60 * 60)
def get_live_services_count():
return len(service_api_client.get_live_services_data({"filter_heartbeats": True})["data"])

@jimleroyer jimleroyer Aug 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hmm I guess we could keep that one, especially for memoization, and let it be called in the other call site, but we need to remove filter_heartbeats. @andrewleith what do you prefer?

@andrewleith andrewleith Aug 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah I think we should keep the caching here - no need to hit the db often for these generalized statistics!

return len(service_api_client.get_live_services_data()["data"])


def user_has_permissions(*permissions, **permission_kwargs):
Expand Down
6 changes: 2 additions & 4 deletions tests/app/notify_client/test_service_api_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,17 +59,15 @@ def test_client_gets_service_statistics(mocker, today_only, limit_days):
)


@pytest.mark.parametrize("filter_heartbeat", [True, False])
def test_client_gets_stats_by_month(mocker, filter_heartbeat):
def test_client_gets_stats_by_month(mocker):
client = ServiceAPIClient()
mock_get = mocker.patch.object(client, "get", return_value={"data": {"a": "b"}})

ret = client.get_stats_by_month(filter_heartbeat)
ret = client.get_stats_by_month()

assert ret["data"] == {"a": "b"}
mock_get.assert_called_once_with(
"/service/delivered-notifications-stats-by-month-data",
params={"filter_heartbeats": filter_heartbeat},
)


Expand Down
Loading