diff --git a/config.py b/config.py index c3c6fc78..f3786292 100644 --- a/config.py +++ b/config.py @@ -40,7 +40,7 @@ def get_env_var(name: str) -> str: ] GITHUB_ISSUE_DIR = ".github/ISSUE_TEMPLATE" GITHUB_ISSUE_TEMPLATES: list[str] = ["bug_report.yml", "feature_request.yml"] -GITHUB_NOREPLY_EMAIL_DOMAIN = "users.noreply.github.com" +GITHUB_NOREPLY_EMAIL_DOMAIN = "users.noreply.github.com" # https://docs.github.com/en/account-and-profile/setting-up-and-managing-your-personal-account-on-github/managing-email-preferences/setting-your-commit-email-address GITHUB_PRIVATE_KEY_ENCODED: str = get_env_var(name="GH_PRIVATE_KEY") GITHUB_PRIVATE_KEY: bytes = base64.b64decode(s=GITHUB_PRIVATE_KEY_ENCODED) GITHUB_WEBHOOK_SECRET: str = get_env_var(name="GH_WEBHOOK_SECRET") @@ -103,7 +103,7 @@ def get_env_var(name: str) -> str: OWNER_NAME = "installation-test" EXCEPTION_OWNERS = ["gitautoai", "hiroshinishio"] OWNER_TYPE = "Organization" +TEST_EMAIL = "test@gitauto.ai" UNIQUE_ISSUE_ID = "O/gitautoai/test#1" USER_ID = -1 USER_NAME = "username-test" -EMAIL = "test@gitauto.com" diff --git a/services/gitauto_handler.py b/services/gitauto_handler.py index 68bac59b..91aade12 100644 --- a/services/gitauto_handler.py +++ b/services/gitauto_handler.py @@ -93,8 +93,8 @@ async def handle_gitauto(payload: GitHubLabeledPayload, trigger_type: str) -> No 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) - + sender_email = get_user_public_email(username=sender_name, token=token) + base_args: BaseArgs = { "owner": owner_name, "repo": repo_name, @@ -135,9 +135,10 @@ async def handle_gitauto(payload: GitHubLabeledPayload, trigger_type: str) -> No unique_issue_id = f"{owner_type}/{owner_name}/{repo_name}#{issue_number}" usage_record_id = supabase_manager.create_user_request( user_id=sender_id, + user_name=sender_name, installation_id=installation_id, unique_issue_id=unique_issue_id, - email=email, + email=sender_email, ) add_reaction_to_issue( issue_number=issue_number, content="eyes", base_args=base_args diff --git a/services/github/github_manager.py b/services/github/github_manager.py index 0d1989cd..4a12148b 100644 --- a/services/github/github_manager.py +++ b/services/github/github_manager.py @@ -240,21 +240,17 @@ 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 | None = get_user_public_email(username=user_name, token=token) + user_email: str | None = get_user_public_email(username=user_name, token=token) supabase_manager = SupabaseManager(url=SUPABASE_URL, key=SUPABASE_SERVICE_ROLE_KEY) # Proper issue generation comment, create user if not exist (first issue in an orgnanization) first_issue = False - if not supabase_manager.user_exists(user_id=user_id): - supabase_manager.create_user( - user_id=user_id, - user_name=user_name, - installation_id=installation_id, - email=email, - ) - first_issue = True - elif supabase_manager.is_users_first_issue( + supabase_manager.upsert_user(user_id=user_id, user_name=user_name, email=user_email) + supabase_manager.upsert_user_installation( + user_id=user_id, installation_id=installation_id + ) + if supabase_manager.is_users_first_issue( user_id=user_id, installation_id=installation_id ): first_issue = True @@ -771,8 +767,15 @@ def update_comment( response.raise_for_status() return response.json() + @handle_exceptions(default_return_value=None, raise_on_error=False) def get_user_public_email(username: str, token: str) -> str | None: + """https://docs.github.com/en/rest/users/users?apiVersion=2022-11-28#get-a-user""" + # If the user is a bot, the email is not available. + if "[bot]" in username: + return None + + # If the user is not a bot, get the user's email response: requests.Response = requests.get( url=f"{GITHUB_API_URL}/users/{username}", headers=create_headers(token=token), @@ -780,5 +783,5 @@ def get_user_public_email(username: str, token: str) -> str | None: ) response.raise_for_status() user_data: dict = response.json() - email: str | None = 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 5ce59cee..66b6ec6f 100644 --- a/services/supabase/gitauto_manager.py +++ b/services/supabase/gitauto_manager.py @@ -41,7 +41,7 @@ def create_installation( owner_id: int, user_id: int, user_name: str, - email: str, + email: str | None, ) -> None: """Create owners record with stripe customerId, subscribe to free plan, create installation record, create users record on Installation Webhook event""" # If owner doesn't exist in owners table, insert owner and stripe customer @@ -71,20 +71,6 @@ def create_installation( json={"owner_id": owner_id, "stripe_customer_id": customer_id} ).execute() - users_manager = UsersManager(client=self.client) - if users_manager.user_exists(user_id=user_id): - users_manager.handle_user_email_update( - user_id=user_id, - email=email - ) - else: - # Create the user if it doesn't exist - users_manager.create_user( - user_id=user_id, - user_name=user_name, - installation_id=installation_id, - email=email, - ) # Insert installation record self.client.table(table_name="installations").insert( json={ @@ -94,29 +80,24 @@ def create_installation( "owner_id": owner_id, } ).execute() - # Create User, and set is_selected to True if user has no selected account for this installation - is_selected = True - data, _ = ( - self.client.table(table_name="user_installations") - .select("user_id") - .eq(column="user_id", value=user_id) - .eq(column="is_selected", value=True) - .execute() - ) - if len(data[1]) > 0: - is_selected = False - self.client.table(table_name="user_installations").insert( - json={ - "user_id": user_id, - "installation_id": installation_id, - "is_selected": is_selected, - } - ).execute() + # Upsert user + users_manager = UsersManager(client=self.client) + users_manager.upsert_user(user_id=user_id, user_name=user_name, email=email) + + # Upsert user installation record + users_manager.upsert_user_installation( + user_id=user_id, installation_id=installation_id + ) @handle_exceptions(default_return_value=None, raise_on_error=True) def create_user_request( - self, user_id: int, installation_id: int, unique_issue_id: str, email: str + self, + user_id: int, + user_name: str, + installation_id: int, + unique_issue_id: str, + email: str | None, ) -> int: """Creates record in usage table for this user and issue.""" # If issue doesn't exist, create one @@ -126,14 +107,13 @@ 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 not data[1]: self.client.table(table_name="issues").insert( - json={ - "unique_id": unique_issue_id, - "installation_id": installation_id, - } + json={"unique_id": unique_issue_id, "installation_id": installation_id} ).execute() + # Add user request to usage table data, _ = ( self.client.table(table_name="usage") @@ -146,9 +126,13 @@ def create_user_request( ) .execute() ) + + # Upsert user users_manager = UsersManager(client=self.client) - if users_manager.user_exists(user_id=user_id): - users_manager.handle_user_email_update(user_id=user_id, email=email) + users_manager.upsert_user(user_id=user_id, user_name=user_name, email=email) + users_manager.upsert_user_installation( + user_id=user_id, installation_id=installation_id + ) return data[1][0]["id"] @handle_exceptions(default_return_value=None, raise_on_error=False) diff --git a/services/supabase/users_manager.py b/services/supabase/users_manager.py index d043c7b6..aac5e6fd 100644 --- a/services/supabase/users_manager.py +++ b/services/supabase/users_manager.py @@ -1,13 +1,19 @@ # Standard imports import logging from datetime import datetime +from typing import Any # Third Party imports import stripe from supabase import Client # Local imports -from config import DEFAULT_TIME, GITHUB_NOREPLY_EMAIL_DOMAIN, STRIPE_FREE_TIER_PRICE_ID, TZ +from config import ( + DEFAULT_TIME, + GITHUB_NOREPLY_EMAIL_DOMAIN, + STRIPE_FREE_TIER_PRICE_ID, + TZ, +) from services.stripe.customer import ( get_subscription, get_request_count_from_product_id_metadata, @@ -21,39 +27,16 @@ class UsersManager: def __init__(self, client: Client) -> None: self.client: Client = client - - def check_email_is_valid(self, email: str) -> bool: - """Check if email is valid""" + + def check_email_is_valid(self, email: str | None) -> bool: if email is None: return False - if not isinstance(email, str): - return False if "@" not in email or "." not in email: return False if str(email).lower().endswith(GITHUB_NOREPLY_EMAIL_DOMAIN): return False return True - @handle_exceptions(default_return_value=None, raise_on_error=False) - def create_user(self, user_id: int, user_name: str, installation_id: int, email: str) -> None: - """Creates an account for the user in the users table""" - email = email if self.check_email_is_valid(email=email) else None - self.client.table(table_name="users").upsert( - json={ - "user_id": user_id, - "user_name": user_name, - "email": email, - }, - on_conflict="user_id" - ).execute() - - self.client.table(table_name="user_installations").insert( - json={ - "user_id": user_id, - "installation_id": installation_id, - } - ).execute() - # Check if user has a seat in an org or can be given a seat @handle_exceptions(default_return_value=True, raise_on_error=False) def is_user_eligible_for_seat_handler( @@ -231,22 +214,8 @@ def get_how_many_requests_left_and_cycle( end_date, ) - @handle_exceptions(default_return_value=False, raise_on_error=False) - def user_exists(self, user_id: int) -> bool: - """Check if user exists in users table""" - data, _ = ( - self.client.table(table_name="users") - .select("*") - .eq(column="user_id", value=user_id) - .execute() - ) - if len(data[1]) > 0: - return True - return False - - - @handle_exceptions(default_return_value=False, raise_on_error=False) - def get_user_info(self, user_id: int) -> dict: + @handle_exceptions(default_return_value=None, raise_on_error=False) + def get_user(self, user_id: int): """Get user info from the users table""" data, _ = ( self.client.table(table_name="users") @@ -255,16 +224,34 @@ def get_user_info(self, user_id: int) -> dict: .execute() ) if len(data[1]) > 0: - return data[1][0] - return {} - - - @handle_exceptions(default_return_value=None, raise_on_error=True) - def handle_user_email_update(self, user_id: int, email: str) -> None: - """Update user email in the users table if email is valid and not None and different from the current email""" - if self.check_email_is_valid(email=email): - user_info = self.get_user_info(user_id=user_id) - if user_info.get("email") != email: - self.client.table("users").update( - json={"email": email} - ).eq("user_id", user_id).execute() + user: dict[str, Any] = data[1][0] + return user + return None + + @handle_exceptions(default_return_value=None, raise_on_error=False) + def upsert_user(self, user_id: int, user_name: str, email: str | None) -> None: + # Check if email is valid + email = email if self.check_email_is_valid(email=email) else None + + # Upsert user + self.client.table(table_name="users").upsert( + json={ + "user_id": user_id, + "user_name": user_name, + **({"email": email} if email else {}), + "created_by": str(user_id), # Because created_by is text + }, + on_conflict="user_id", + ).execute() + + @handle_exceptions(default_return_value=None, raise_on_error=False) + def upsert_user_installation(self, user_id: int, installation_id: int) -> None: + # Insert user installation record + self.client.table(table_name="user_installations").upsert( + json={ + "user_id": user_id, + "installation_id": installation_id, + "is_selected": True, + }, + on_conflict="user_id,installation_id", + ).execute() diff --git a/services/webhook_handler.py b/services/webhook_handler.py index 9900671b..4b002af0 100644 --- a/services/webhook_handler.py +++ b/services/webhook_handler.py @@ -17,7 +17,7 @@ create_comment_on_issue_with_gitauto_button, get_installation_access_token, # turn_on_issue, - get_user_public_email + get_user_public_email, ) from services.github.github_types import GitHubInstallationPayload from services.supabase import SupabaseManager @@ -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 | None = get_user_public_email(username=user_name, token=token) + user_email: str | None = get_user_public_email(username=user_name, token=token) # Create installation record in Supabase supabase_manager.create_installation( @@ -49,7 +49,7 @@ async def handle_installation_created(payload: GitHubInstallationPayload) -> Non owner_id=owner_id, user_id=user_id, user_name=user_name, - email=email, + email=user_email, ) # Add issue templates to the repositories diff --git a/tests/services/supabase/test_gitauto_manager.py b/tests/services/supabase/test_gitauto_manager.py index fe38b8ff..c1c9dc6a 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, USER_NAME +from config import OWNER_TYPE, TEST_EMAIL, USER_NAME from services.supabase import SupabaseManager from tests.services.supabase.wipe_data import ( wipe_installation_owner_user_data, @@ -17,7 +17,6 @@ def test_create_update_user_request_works() -> None: # using -1 to not conflict with real data user_id = -1 installation_id = -1 - user_name = USER_NAME # Clean up at the beginning just in case a prior test failed to clean wipe_installation_owner_user_data() @@ -29,16 +28,17 @@ def test_create_update_user_request_works() -> None: owner_name="gitautoai", owner_id=-1, user_id=user_id, - user_name=user_name, - email=EMAIL, + user_name=USER_NAME, + email=TEST_EMAIL, ) usage_record_id = supabase_manager.create_user_request( user_id=user_id, + user_name=USER_NAME, installation_id=installation_id, # fake issue creation unique_issue_id="U/gitautoai/test#01", - email=EMAIL, + email=TEST_EMAIL, ) assert isinstance( usage_record_id, @@ -68,7 +68,6 @@ def test_complete_and_update_usage_record_only_updates_one_record() -> None: # using -1 to not conflict with real data user_id = -1 installation_id = -1 - user_name = "test" unique_issue_id = "U/gitautoai/test#01" # Clean up at the beginning just in case a prior test failed to clean @@ -81,26 +80,28 @@ def test_complete_and_update_usage_record_only_updates_one_record() -> None: owner_name="gitautoai", owner_id=-1, user_id=user_id, - user_name=user_name, - email=EMAIL, + user_name=USER_NAME, + email=TEST_EMAIL, ) # Creating multiple usage records where is_completed = false. for _ in range(0, 5): supabase_manager.create_user_request( user_id=user_id, + user_name=USER_NAME, installation_id=installation_id, # fake issue creation unique_issue_id=unique_issue_id, - email=EMAIL, + email=TEST_EMAIL, ) usage_record_id = supabase_manager.create_user_request( user_id=user_id, + user_name=USER_NAME, installation_id=installation_id, # fake issue creation unique_issue_id=unique_issue_id, - email=EMAIL, + email=TEST_EMAIL, ) assert isinstance( usage_record_id, diff --git a/tests/services/supabase/test_users_manager.py b/tests/services/supabase/test_users_manager.py index 1a7f8c42..9e150eca 100644 --- a/tests/services/supabase/test_users_manager.py +++ b/tests/services/supabase/test_users_manager.py @@ -15,7 +15,7 @@ INSTALLATION_ID, UNIQUE_ISSUE_ID, NEW_INSTALLATION_ID, - EMAIL, + TEST_EMAIL, ) from services.stripe.customer import get_subscription from services.supabase import SupabaseManager @@ -49,14 +49,15 @@ def test_create_and_update_user_request_works() -> None: owner_id=OWNER_ID, user_id=USER_ID, user_name=USER_NAME, - email=EMAIL, + email=TEST_EMAIL, ) usage_record_id = supabase_manager.create_user_request( user_id=USER_ID, + user_name=USER_NAME, installation_id=INSTALLATION_ID, unique_issue_id="U/gitautoai/nextjs-website#52", - email=EMAIL, + email=TEST_EMAIL, ) assert isinstance(usage_record_id, int) assert ( @@ -69,8 +70,8 @@ def test_create_and_update_user_request_works() -> None: is None ) -# test_create_and_update_user_request_works() +# test_create_and_update_user_request_works() def test_how_many_requests_left() -> None: @@ -87,7 +88,7 @@ def test_how_many_requests_left() -> None: owner_id=OWNER_ID, user_id=USER_ID, user_name=USER_NAME, - email=EMAIL, + email=TEST_EMAIL, ) # Testing 0 requests have been made on free tier requests_left, request_count, end_date = ( @@ -159,7 +160,7 @@ def test_is_users_first_issue() -> None: owner_id=OWNER_ID, user_id=USER_ID, user_name=USER_NAME, - email=EMAIL, + email=TEST_EMAIL, ) assert supabase_manager.is_users_first_issue( user_id=USER_ID, installation_id=INSTALLATION_ID @@ -195,7 +196,7 @@ def test_parse_subscription_object() -> None: owner_id=OWNER_ID, user_id=USER_ID, user_name=USER_NAME, - email=EMAIL, + email=TEST_EMAIL, ) def assertion_test(customer_id: str, product_id: str): @@ -425,27 +426,30 @@ def test_handle_user_email_update() -> None: owner_id=OWNER_ID, user_id=USER_ID, user_name=USER_NAME, - email=EMAIL, + email=TEST_EMAIL, ) # Verify github no-reply email is not updated - no_reply_email = f"no_reply_email@{GITHUB_NOREPLY_EMAIL_DOMAIN}" - supabase_manager.handle_user_email_update(user_id=USER_ID, email=no_reply_email) - user_data = supabase_manager.get_user_info(user_id=USER_ID) - assert user_data["email"] == EMAIL - + json_data = {"user_id": USER_ID, "user_name": USER_NAME} + json_data["email"] = f"no_reply_email@{GITHUB_NOREPLY_EMAIL_DOMAIN}" + supabase_manager.upsert_user(**json_data) + user_data = supabase_manager.get_user(user_id=USER_ID) + assert user_data["email"] == TEST_EMAIL + # Verify None email is not updated - none_email = None - supabase_manager.handle_user_email_update(user_id=USER_ID, email=none_email) - user_data = supabase_manager.get_user_info(user_id=USER_ID) - assert user_data["email"] == EMAIL - + json_data["email"] = None + supabase_manager.upsert_user(**json_data) + user_data = supabase_manager.get_user(user_id=USER_ID) + assert user_data["email"] == TEST_EMAIL + # Verify valid email is updated new_email = "new_email@example.com" - supabase_manager.handle_user_email_update(user_id=USER_ID, email=new_email) - user_data = supabase_manager.get_user_info(user_id=USER_ID) + json_data["email"] = "new_email@example.com" + supabase_manager.upsert_user(**json_data) + user_data = supabase_manager.get_user(user_id=USER_ID) assert user_data["email"] == new_email # Clean Up wipe_installation_owner_user_data() + # test_handle_user_email_update()