Skip to content

acdbot: fix resource key management #1665

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

Merged
merged 2 commits into from
Aug 5, 2025
Merged
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
2 changes: 1 addition & 1 deletion .github/ACDbot/meeting_topic_mapping.json
Original file line number Diff line number Diff line change
Expand Up @@ -761,7 +761,7 @@
}
],
"uuid": "VSEv1YKSToeM9+peGbhxDQ==",
"calendar_event_id": "k79qrflnm890eht7miguuitp8s"
"calendar_event_id": "lmssl6qcn2l5f8dj16f6cb1sbk"
},
"eof": {
"call_series": "eof",
Expand Down
16 changes: 8 additions & 8 deletions .github/ACDbot/scripts/handle_protocol_call.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def _handle_discourse_resource(self, call_data: Dict, existing_resources: Dict)
"discourse_created": True,
"discourse_topic_id": existing_occurrence["discourse_topic_id"],
"discourse_url": f"https://ethereum-magicians.org/t/{existing_occurrence['discourse_topic_id']}",
"action": "existing"
"discourse_action": "existing"
}
else:
print(f"[DEBUG] Creating new discourse topic")
Expand Down Expand Up @@ -145,7 +145,7 @@ def _handle_youtube_resource(self, call_data: Dict, existing_resources: Dict) ->
"youtube_streams_created": True,
"youtube_streams": youtube_streams,
"stream_links": stream_links,
"action": "existing"
"youtube_action": "existing"
}
else:
print(f"[DEBUG] Creating new YouTube streams")
Expand Down Expand Up @@ -895,15 +895,15 @@ def _create_discourse_topic(self, call_data: Dict) -> Dict:
"discourse_created": True,
"discourse_topic_id": existing_topic_id,
"discourse_url": discourse_url,
"action": "found_duplicate_series"
"discourse_action": "found_duplicate_series"
}

# If no existing topic found, return error
return {
"discourse_created": False,
"discourse_topic_id": f"placeholder-duplicate-{call_data['issue_number']}",
"discourse_url": "https://ethereum-magicians.org (Duplicate title, ID not found)",
"action": "failed_duplicate_title"
"discourse_action": "failed_duplicate_title"
}

except Exception as e:
Expand All @@ -912,7 +912,7 @@ def _create_discourse_topic(self, call_data: Dict) -> Dict:
"discourse_created": False,
"discourse_topic_id": f"placeholder-error-{call_data['issue_number']}",
"discourse_url": "https://ethereum-magicians.org (API error occurred)",
"action": "failed"
"discourse_action": "failed"
}

def _get_call_series_display_name(self, call_series_key: str) -> str:
Expand Down Expand Up @@ -1052,7 +1052,7 @@ def _create_youtube_streams(self, call_data: Dict) -> Dict:
"youtube_streams_created": True,
"youtube_streams": youtube_streams,
"stream_links": stream_links,
"action": "created"
"youtube_action": "created"
}
else:
print(f"[DEBUG] No YouTube streams created")
Expand Down Expand Up @@ -1220,7 +1220,7 @@ def _resources_changed(self, resource_results: Dict) -> bool:

# Check if YouTube streams were actually created (not just found existing)
youtube_actually_created = (resource_results.get("youtube_streams_created") and
resource_results.get("action") != "existing")
resource_results.get("youtube_action") != "existing")

return any([
zoom_actually_created,
Expand Down Expand Up @@ -1287,7 +1287,7 @@ def _post_results(self, call_data: Dict, issue, resource_results: Dict, is_updat
comment_lines.append("❌ **Discourse**: Failed to create")

if resource_results["youtube_streams_created"]:
youtube_action = resource_results.get("action", "created")
youtube_action = resource_results.get("youtube_action", "created")
if youtube_action == "existing":
# Don't include existing resources in the comment
pass
Expand Down
79 changes: 77 additions & 2 deletions .github/ACDbot/tests/unit/test_protocol_call_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,87 @@ def test_handle_discourse_resource_existing(self):

self.assertTrue(result["discourse_created"])
self.assertEqual(result["discourse_topic_id"], "test-discourse-id")
self.assertEqual(result["action"], "existing")
self.assertEqual(result["discourse_action"], "existing")

def test_handle_youtube_resource_existing(self):
"""Test that youtube resource uses existing data when available."""
result = self.handler._handle_youtube_resource(self.sample_call_data, self.sample_existing_resources)

self.assertTrue(result["youtube_streams_created"])
self.assertEqual(len(result["youtube_streams"]), 1)
self.assertEqual(len(result["stream_links"]), 1)
self.assertEqual(len(result["stream_links"]), 1)
self.assertEqual(result["youtube_action"], "existing")

def test_resources_changed_with_action_fields(self):
"""Test that resources_changed correctly handles action fields."""
# Test with existing resources
resource_results = {
"zoom_created": True,
"zoom_action": "updated",
"calendar_created": True,
"calendar_action": "existing",
"discourse_created": True,
"discourse_action": "existing",
"youtube_streams_created": True,
"youtube_action": "existing"
}
result = self.handler._resources_changed(resource_results)
self.assertFalse(result)

# Test with newly created resources
resource_results = {
"zoom_created": True,
"zoom_action": "created",
"calendar_created": True,
"calendar_action": "created",
"discourse_created": True,
"discourse_action": "created",
"youtube_streams_created": True,
"youtube_action": "created"
}
result = self.handler._resources_changed(resource_results)
self.assertTrue(result)

# Test with mixed existing and new resources
resource_results = {
"zoom_created": True,
"zoom_action": "updated",
"calendar_created": True,
"calendar_action": "created",
"discourse_created": True,
"discourse_action": "existing",
"youtube_streams_created": True,
"youtube_action": "existing"
}
result = self.handler._resources_changed(resource_results)
self.assertTrue(result)

def test_find_existing_discourse_topic(self):
"""Test that _find_existing_discourse_topic correctly finds existing topic IDs."""
# Mock the mapping manager to return test data
with unittest.mock.patch.object(self.handler.mapping_manager, 'load_mapping') as mock_load:
mock_load.return_value = {
"acdt": {
"call_series": "acdt",
"occurrences": [
{
"issue_number": 1648,
"discourse_topic_id": 24956,
"issue_title": "All Core Devs - Testing (ACDT) #47 | August 4 2025"
},
{
"issue_number": 1640,
"discourse_topic_id": 24800,
"issue_title": "All Core Devs - Testing (ACDT) #46 | July 28 2025"
}
]
}
}

# Test finding existing topic for acdt series
result = self.handler._find_existing_discourse_topic("acdt")
self.assertEqual(result, 24956) # Should find the most recent one

# Test finding existing topic for non-existent series
result = self.handler._find_existing_discourse_topic("nonexistent")
self.assertIsNone(result)