Fix Total Message Count Calculation to Account for Headers

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 = 61
 
 
 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"]

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