From 0eb0f57ae5c7049dd6ab7be9a0f55b545d01c783 Mon Sep 17 00:00:00 2001 From: bigolehealz Date: Mon, 28 Oct 2024 11:39:49 -0400 Subject: [PATCH] addressing comments made by Wes on PR --- services/gitauto_handler.py | 6 +++--- services/github/github_manager.py | 20 +++++++++---------- services/supabase/gitauto_manager.py | 2 +- services/webhook_handler.py | 2 +- .../services/supabase/test_gitauto_manager.py | 7 ++----- tests/services/supabase/test_users_manager.py | 3 --- 6 files changed, 16 insertions(+), 24 deletions(-) diff --git a/services/gitauto_handler.py b/services/gitauto_handler.py index 12a711b3..d39c2b45 100644 --- a/services/gitauto_handler.py +++ b/services/gitauto_handler.py @@ -50,6 +50,7 @@ supabase_manager = SupabaseManager(url=SUPABASE_URL, key=SUPABASE_SERVICE_ROLE_KEY) + async def handle_gitauto(payload: GitHubLabeledPayload, trigger_type: str) -> None: """Core functionality to create comments on issue, create PRs, and update progress.""" current_time: float = time.time() @@ -84,12 +85,13 @@ async def handle_gitauto(payload: GitHubLabeledPayload, trigger_type: str) -> No sender_id: int = payload["sender"]["id"] is_automation: bool = sender_id == GITHUB_APP_USER_ID sender_name: str = payload["sender"]["login"] - email: str = get_user_public_email(username=sender_name) # Extract other information github_urls, other_urls = extract_urls(text=issue_body) installation_id: int = payload["installation"]["id"] token: str = get_installation_access_token(installation_id=installation_id) + email: str | None = get_user_public_email(username=sender_name, token=token) + base_args: BaseArgs = { "owner": owner_name, "repo": repo_name, @@ -255,7 +257,5 @@ async def handle_gitauto(payload: GitHubLabeledPayload, trigger_type: str) -> No token_input=token_input, token_output=token_output, total_seconds=int(end_time - current_time), - user_id=sender_id, - email=email, ) return diff --git a/services/github/github_manager.py b/services/github/github_manager.py index 041fb79a..3e8be3ba 100644 --- a/services/github/github_manager.py +++ b/services/github/github_manager.py @@ -237,7 +237,7 @@ def create_comment_on_issue_with_gitauto_button(payload: GitHubLabeledPayload) - issue_number: int = payload["issue"]["number"] user_id: int = payload["sender"]["id"] user_name: str = payload["sender"]["login"] - email: str = get_user_public_email(username=user_name) + email: str | None = get_user_public_email(username=user_name, token=token) supabase_manager = SupabaseManager(url=SUPABASE_URL, key=SUPABASE_SERVICE_ROLE_KEY) @@ -832,17 +832,15 @@ def update_comment_for_raised_errors( raise RuntimeError("Error occurred") -@handle_exceptions(default_return_value=None, raise_on_error=False) -def get_user_public_email(username: str): - url = f"https://api.github.com/users/{username}" - headers = { - "Accept": "application/vnd.github.v3+json" - } - response: requests.Response = requests.get(url, headers=headers) +@handle_exceptions(default_return_value=None, raise_on_error=False) +def get_user_public_email(username: str, token: str) -> str | None: + response: requests.Response = requests.get( + url=f"{GITHUB_API_URL}/users/{username}", + headers=create_headers(token=token), + timeout=TIMEOUT, + ) response.raise_for_status() user_data: dict = response.json() - - email: str = user_data.get('email') - + email: str | None = user_data.get('email') return email diff --git a/services/supabase/gitauto_manager.py b/services/supabase/gitauto_manager.py index dfb65530..602253f2 100644 --- a/services/supabase/gitauto_manager.py +++ b/services/supabase/gitauto_manager.py @@ -134,7 +134,7 @@ def create_user_request( .eq(column="unique_id", value=unique_issue_id) .execute() ) - # If no issue exists with that unique_issue_id, create one + # If no issue exists with that unique_issue_id, create one if not data[1]: self.client.table(table_name="issues").insert( json={ diff --git a/services/webhook_handler.py b/services/webhook_handler.py index 9ec7d9d8..ffd0292d 100644 --- a/services/webhook_handler.py +++ b/services/webhook_handler.py @@ -39,7 +39,7 @@ async def handle_installation_created(payload: GitHubInstallationPayload) -> Non user_id: int = payload["sender"]["id"] user_name: str = payload["sender"]["login"] token: str = get_installation_access_token(installation_id=installation_id) - email: str = get_user_public_email(username=user_name) + email: str | None = get_user_public_email(username=user_name, token=token) # Create installation record in Supabase supabase_manager.create_installation( diff --git a/tests/services/supabase/test_gitauto_manager.py b/tests/services/supabase/test_gitauto_manager.py index ab3f1272..fe278491 100644 --- a/tests/services/supabase/test_gitauto_manager.py +++ b/tests/services/supabase/test_gitauto_manager.py @@ -1,6 +1,6 @@ # run this file locally with: python -m tests.services.supabase.test_gitauto_manager import os -from config import (EMAIL, OWNER_TYPE) +from config import EMAIL, OWNER_TYPE, USER_NAME from services.supabase import SupabaseManager from tests.services.supabase.wipe_data import ( wipe_installation_owner_user_data, @@ -17,7 +17,7 @@ def test_create_update_user_request_works() -> None: # using -1 to not conflict with real data user_id = -1 installation_id = -1 - user_name = "test" + user_name = USER_NAME # Clean up at the beginning just in case a prior test failed to clean wipe_installation_owner_user_data() @@ -58,9 +58,6 @@ def test_create_update_user_request_works() -> None: wipe_installation_owner_user_data() -# test_create_update_user_request_works() - - def test_complete_and_update_usage_record_only_updates_one_record() -> None: """Tests based on creating a record and updating it in usage table""" supabase_manager = SupabaseManager(url=SUPABASE_URL, key=SUPABASE_SERVICE_ROLE_KEY) diff --git a/tests/services/supabase/test_users_manager.py b/tests/services/supabase/test_users_manager.py index 81ac6416..d29291e0 100644 --- a/tests/services/supabase/test_users_manager.py +++ b/tests/services/supabase/test_users_manager.py @@ -69,9 +69,6 @@ def test_create_and_update_user_request_works() -> None: ) -# test_create_and_update_user_request_works() - - def test_how_many_requests_left() -> None: """Test that get_how_many_requests_left_and_cycle returns the correct values""" supabase_manager = SupabaseManager(url=SUPABASE_URL, key=SUPABASE_SERVICE_ROLE_KEY)