Skip to content

Commit

Permalink
Use service start date to filter iterated posts
Browse files Browse the repository at this point in the history
  • Loading branch information
rossjrw committed Sep 29, 2023
1 parent cf8af1e commit 2b02579
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 10 deletions.
4 changes: 3 additions & 1 deletion notifier/database/drivers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ def count_user_configs(self) -> int:
"""Count the number of subscribed users."""

@abstractmethod
def get_notifiable_users(self, frequency: str) -> List[str]:
def get_notifiable_users(
self, frequency: str, post_lower_timestamp_limit: int
) -> List[str]:
"""Get the list of IDs for users subscribed to the given channel
frequency who have at least one notification waiting for them.
"""
Expand Down
9 changes: 7 additions & 2 deletions notifier/database/drivers/mysql.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,9 +332,14 @@ def count_user_configs(self) -> int:
)["count"],
)

def get_notifiable_users(self, frequency: str) -> List[str]:
def get_notifiable_users(
self, frequency: str, post_lower_timestamp_limit: int
) -> List[str]:
logger.debug("Caching post context...")
self.execute_named("cache_post_context")
self.execute_named(
"cache_post_context",
{"post_lower_timestamp_limit": post_lower_timestamp_limit},
)
logger.debug("Retrieving notifiable users users...")
user_ids = [
cast(str, row["user_id"])
Expand Down
3 changes: 3 additions & 0 deletions notifier/database/queries/cache_post_context.sql
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,8 @@ WITH cte AS (
WHERE
-- Remove deleted threads/posts
thread.is_deleted = 0 AND post.is_deleted = 0

-- Remove posts from before the notification service started
AND post.posted_timestamp > %(post_lower_timestamp_limit)s
)
SELECT * FROM cte
2 changes: 1 addition & 1 deletion notifier/notify.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ def notify_channel(
# Filter the users only to those with notifications waiting
logger.debug("Filtering users without notifications waiting...")
user_count_pre_filter = len(user_configs)
notifiable_user_ids = database.get_notifiable_users(channel)
notifiable_user_ids = database.get_notifiable_users(channel, config["service_start_timestamp"])
user_configs = [
user for user in user_configs if user["user_id"] in notifiable_user_ids
]
Expand Down
15 changes: 9 additions & 6 deletions tests/test_database.py
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ def check_timestamp(n: int) -> None:


@pytest.mark.needs_database
def test_get_notifiable_users(sample_database: BaseDatabaseDriver) -> None:
def test_get_notifiable_users(sample_database: MySqlDriver) -> None:
"""Test that the notifiable users list returns the correct set of users.
The notifiable users utility lists directly from the database the set of
Expand Down Expand Up @@ -627,15 +627,18 @@ def test_get_notifiable_users(sample_database: BaseDatabaseDriver) -> None:
for post in sample_posts:
sample_database.store_post(post)

users_expected_to_have_notifications = {
assert set(sample_database.get_notifiable_users("hourly", 0)) == {
"1", # UserR1 from base sample DB
"51", # T5U-!P-Sub
"53", # T5U-Starter
"55", # T5U-Poster
}

users_with_notifications = sample_database.get_notifiable_users("hourly")
with sample_database.transaction() as cursor:
cursor.execute("DROP TABLE post_with_context")

assert (
set(users_with_notifications) == users_expected_to_have_notifications
)
assert set(sample_database.get_notifiable_users("hourly", 99)) == {
"51", # T5U-!P-Sub
"53", # T5U-Starter
"55", # T5U-Poster
}

0 comments on commit 2b02579

Please sign in to comment.