diff --git a/src/message_builder.py b/src/message_builder.py index b0ae865..84675af 100644 --- a/src/message_builder.py +++ b/src/message_builder.py @@ -11,6 +11,9 @@ # This is lower than the actual limit to provide headroom MAX_MESSAGE_CHARACTER_LENGTH = 3000 +# Approximate character length needed to accommodate post headers +# ex: HackGreenville Events for the week of September 10 - 10 of 10 +HEADER_BUFFER_LENGTH = 66 async def build_header(week_start: datetime.datetime, index: int, total: int) -> dict: @@ -84,18 +87,33 @@ async def build_event_blocks(resp, week_start, week_end) -> list: ) -async def chunk_messages(event_blocks, week_start) -> list: +async def total_messages_needed(event_blocks: list) -> int: """ - Chunk up events across messages so that no one message is longer than 4k characters + Determines the total number of posts that will be needed to cover a week's events. + + Will always be at least 1. """ - total_messages_needed = math.ceil( + messages_needed = math.ceil( sum(event["text_length"] for event in event_blocks) - / MAX_MESSAGE_CHARACTER_LENGTH + / (MAX_MESSAGE_CHARACTER_LENGTH - HEADER_BUFFER_LENGTH) ) + # Ensure total count is at least 1 if we're going to post anything + if messages_needed == 0: + return 1 + + return messages_needed + + +async def chunk_messages(event_blocks, week_start) -> list: + """ + Chunk up events across messages so that no one message is longer than 4k characters + """ + messages_needed = await total_messages_needed(event_blocks) + messages = [] - initial_header = await build_header(week_start, 1, total_messages_needed) + initial_header = await build_header(week_start, 1, messages_needed) blocks = initial_header["blocks"] text = initial_header["text"] @@ -110,9 +128,7 @@ async def chunk_messages(event_blocks, week_start) -> list: # Save message and then start a new one messages += [{"blocks": blocks, "text": text}] - new_header = await build_header( - week_start, len(messages) + 1, total_messages_needed - ) + new_header = await build_header(week_start, len(messages) + 1, messages_needed) blocks = new_header["blocks"] text = new_header["text"] diff --git a/tests/test_message_builder.py b/tests/test_message_builder.py index 8769fb4..650dca9 100644 --- a/tests/test_message_builder.py +++ b/tests/test_message_builder.py @@ -13,6 +13,7 @@ build_header, build_single_event_block, chunk_messages, + total_messages_needed, ) week_start = datetime.datetime.strptime("10/22/2023", "%m/%d/%Y").replace( @@ -95,3 +96,52 @@ async def test_chunk_messages(event_api_response_data): len(text) < MAX_MESSAGE_CHARACTER_LENGTH for text in [msg["text"] for msg in result] ) + + +@pytest.mark.asyncio +async def test_total_messages_needed_with_borderline_edge_case(): + """ + Ensures total message count is rounded upwards appropriately + whenever the event data character count is just below n * MAX_MESSAGE_CHARACTER_LENGTH + and the addition of post headers causes it message count to then spillover. + + The test data used is that reported in https://github.com/hackgvl/slack-events-bot/issues/52 + """ + # Abbreviated problem case from actual data that originally caused the bug to manifest. + borderline_test_case = [ + { + "text_length": 463, + }, + { + "text_length": 311, + }, + { + "text_length": 397, + }, + { + "text_length": 507, + }, + { + "text_length": 456, + }, + { + "text_length": 463, + }, + { + "text_length": 352, + }, + ] + + assert await total_messages_needed(borderline_test_case) == 2 + + +@pytest.mark.asyncio +async def test_total_messages_needed_always_at_least_1(): + """ + Ensures that the total message needed count is always at least one. + + This is because we create a post for weeks that have zero events in case + new ones are added later, and we don't want the posts to have "- 1 of 0" in their + headers. + """ + assert await total_messages_needed([]) == 1