From d6c526318696f7fba72c7f6544f36ef59f74b0a0 Mon Sep 17 00:00:00 2001 From: Hiroshi Nishio Date: Sat, 21 Feb 2026 17:42:26 -0800 Subject: [PATCH] Remove Supabase issues table usage and drop table in dev The issues table was write-only - data was inserted but never read for any business logic. The usage table already tracks everything. Dropped the issues table and usage_with_issues view in dev. --- schemas/supabase/types.py | 66 --- services/supabase/create_user_request.py | 20 - services/supabase/issues/get_issue.py | 30 -- services/supabase/issues/insert_issue.py | 25 - services/supabase/issues/test_get_issue.py | 253 ----------- services/supabase/issues/test_insert_issue.py | 202 --------- .../issues/test_update_issue_merged.py | 426 ------------------ .../supabase/issues/update_issue_merged.py | 21 - services/supabase/test_create_user_request.py | 257 +---------- 9 files changed, 23 insertions(+), 1277 deletions(-) delete mode 100644 services/supabase/issues/get_issue.py delete mode 100644 services/supabase/issues/insert_issue.py delete mode 100644 services/supabase/issues/test_get_issue.py delete mode 100644 services/supabase/issues/test_insert_issue.py delete mode 100644 services/supabase/issues/test_update_issue_merged.py delete mode 100644 services/supabase/issues/update_issue_merged.py diff --git a/schemas/supabase/types.py b/schemas/supabase/types.py index 46b20b072..07bc5b8d6 100644 --- a/schemas/supabase/types.py +++ b/schemas/supabase/types.py @@ -181,34 +181,6 @@ class InstallationsInsert(TypedDict): uninstalled_by: NotRequired[str | None] -class Issues(TypedDict): - id: int - created_at: datetime.datetime - run_id: int | None - installation_id: int - merged: bool - created_by: str | None - owner_id: int - owner_type: str - owner_name: str - repo_id: int - repo_name: str - issue_number: int - - -class IssuesInsert(TypedDict): - run_id: NotRequired[int | None] - installation_id: int - merged: bool - created_by: NotRequired[str | None] - owner_id: int - owner_type: str - owner_name: str - repo_id: int - repo_name: str - issue_number: int - - class JiraGithubLinks(TypedDict): id: int jira_site_id: str @@ -606,44 +578,6 @@ class UsageInsert(TypedDict): lambda_request_id: NotRequired[str | None] -class UsageWithIssues(TypedDict): - id: int | None - created_at: datetime.datetime | None - is_completed: bool | None - token_input: int | None - token_output: int | None - user_id: int | None - installation_id: int | None - created_by: str | None - total_seconds: int | None - owner_id: int | None - owner_type: str | None - owner_name: str | None - repo_id: int | None - repo_name: str | None - issue_number: int | None - source: str | None - merged: bool | None - - -class UsageWithIssuesInsert(TypedDict): - is_completed: NotRequired[bool | None] - token_input: NotRequired[int | None] - token_output: NotRequired[int | None] - user_id: NotRequired[int | None] - installation_id: NotRequired[int | None] - created_by: NotRequired[str | None] - total_seconds: NotRequired[int | None] - owner_id: NotRequired[int | None] - owner_type: NotRequired[str | None] - owner_name: NotRequired[str | None] - repo_id: NotRequired[int | None] - repo_name: NotRequired[str | None] - issue_number: NotRequired[int | None] - source: NotRequired[str | None] - merged: NotRequired[bool | None] - - class Users(TypedDict): id: int user_name: str diff --git a/services/supabase/create_user_request.py b/services/supabase/create_user_request.py index 5828b9344..d661bb683 100644 --- a/services/supabase/create_user_request.py +++ b/services/supabase/create_user_request.py @@ -1,6 +1,4 @@ from constants.triggers import Trigger -from services.supabase.issues.get_issue import get_issue -from services.supabase.issues.insert_issue import insert_issue from services.supabase.usage.insert_usage import insert_usage from services.supabase.users.upsert_user import upsert_user from utils.error.handle_exceptions import handle_exceptions @@ -22,24 +20,6 @@ def create_user_request( email: str | None, lambda_info: dict[str, str | None] | None = None, ): - existing_issue = get_issue( - owner_type=owner_type, - owner_name=owner_name, - repo_name=repo_name, - pr_number=pr_number, - ) - - if not existing_issue: - insert_issue( - owner_id=owner_id, - owner_type=owner_type, - owner_name=owner_name, - repo_id=repo_id, - repo_name=repo_name, - pr_number=pr_number, - installation_id=installation_id, - ) - # Extract Lambda context info if provided lambda_log_group = lambda_info.get("log_group") if lambda_info else None lambda_log_stream = lambda_info.get("log_stream") if lambda_info else None diff --git a/services/supabase/issues/get_issue.py b/services/supabase/issues/get_issue.py deleted file mode 100644 index df55f4138..000000000 --- a/services/supabase/issues/get_issue.py +++ /dev/null @@ -1,30 +0,0 @@ -# Standard imports -from typing import cast - -# Third-party imports -from schemas.supabase.types import Issues - -# Local imports -from services.supabase.client import supabase -from utils.error.handle_exceptions import handle_exceptions - - -@handle_exceptions(default_return_value=None, raise_on_error=False) -def get_issue( - owner_type: str, - owner_name: str, - repo_name: str, - pr_number: int, -): - result = ( - supabase.table(table_name="issues") - .select("*") - .eq(column="owner_type", value=owner_type) - .eq(column="owner_name", value=owner_name) - .eq(column="repo_name", value=repo_name) - .eq(column="issue_number", value=pr_number) - .execute() - ) - if not result.data: - return None - return cast(Issues, result.data[0]) diff --git a/services/supabase/issues/insert_issue.py b/services/supabase/issues/insert_issue.py deleted file mode 100644 index 2da6762c7..000000000 --- a/services/supabase/issues/insert_issue.py +++ /dev/null @@ -1,25 +0,0 @@ -from services.supabase.client import supabase -from utils.error.handle_exceptions import handle_exceptions - - -@handle_exceptions(default_return_value=None, raise_on_error=True) -def insert_issue( - owner_id: int, - owner_type: str, - owner_name: str, - repo_id: int, - repo_name: str, - pr_number: int, - installation_id: int, -): - supabase.table(table_name="issues").insert( - json={ - "owner_id": owner_id, - "owner_type": owner_type, - "owner_name": owner_name, - "repo_id": repo_id, - "repo_name": repo_name, - "issue_number": pr_number, - "installation_id": installation_id, - } - ).execute() diff --git a/services/supabase/issues/test_get_issue.py b/services/supabase/issues/test_get_issue.py deleted file mode 100644 index cb743e571..000000000 --- a/services/supabase/issues/test_get_issue.py +++ /dev/null @@ -1,253 +0,0 @@ -import os -import unittest -from unittest.mock import patch, MagicMock - -import pytest - -from services.supabase.issues.get_issue import get_issue - - -def _mock_response(data): - mock = MagicMock() - mock.data = data - return mock - - -class TestGetIssue(unittest.TestCase): - def setUp(self): - self.sample_issue_data = { - "id": 1, - "owner_type": "Organization", - "owner_name": "test-owner", - "repo_name": "test-repo", - "issue_number": 123, - "installation_id": 456, - "merged": False, - "created_at": "2024-01-01T00:00:00Z", - "owner_id": 789, - "repo_id": 101112, - "created_by": "test-user", - "run_id": None, - } - - self.test_params = { - "owner_type": "Organization", - "owner_name": "test-owner", - "repo_name": "test-repo", - "pr_number": 123, - } - - def _setup_supabase_mock(self, mock_supabase, return_data): - mock_chain = ( - mock_supabase.table.return_value.select.return_value.eq.return_value.eq.return_value.eq.return_value.eq.return_value - ) - mock_chain.execute.return_value = _mock_response(return_data) - - @patch("services.supabase.issues.get_issue.supabase") - def test_get_issue_success_with_data(self, mock_supabase): - mock_issue_data = { - "id": 1, - "owner_type": "Organization", - "owner_name": "test-owner", - "repo_name": "test-repo", - "issue_number": 123, - "installation_id": 456, - "merged": False, - "created_at": "2024-01-01T00:00:00Z", - "owner_id": 789, - "repo_id": 101112, - "created_by": "test-user", - "run_id": None, - } - - self._setup_supabase_mock(mock_supabase, [mock_issue_data]) - - result = get_issue( - owner_type="Organization", - owner_name="test-owner", - repo_name="test-repo", - pr_number=123, - ) - - assert result is not None - self.assertIsInstance(result, dict) - self.assertEqual(result["id"], 1) - self.assertEqual(result["owner_type"], "Organization") - mock_supabase.table.assert_called_once_with(table_name="issues") - - @patch("services.supabase.issues.get_issue.supabase") - def test_get_issue_no_data_found(self, mock_supabase): - self._setup_supabase_mock(mock_supabase, []) - - result = get_issue( - owner_type="Organization", - owner_name="nonexistent-owner", - repo_name="nonexistent-repo", - pr_number=999, - ) - - self.assertIsNone(result) - - @patch("services.supabase.issues.get_issue.supabase") - def test_get_issue_with_different_owner_types(self, mock_supabase): - mock_issue_data = { - "id": 2, - "owner_type": "User", - "owner_name": "individual-user", - "repo_name": "personal-repo", - "issue_number": 789, - "installation_id": 123, - "merged": True, - "created_at": "2024-02-01T00:00:00Z", - "owner_id": 456, - "repo_id": 789012, - "created_by": "individual-user", - "run_id": 999, - } - - self._setup_supabase_mock(mock_supabase, [mock_issue_data]) - - result = get_issue( - owner_type="User", - owner_name="individual-user", - repo_name="personal-repo", - pr_number=789, - ) - - assert result is not None - self.assertEqual(result["owner_type"], "User") - self.assertEqual(result["merged"], True) - - @patch("services.supabase.issues.get_issue.supabase") - def test_get_issue_database_exception(self, mock_supabase): - mock_supabase.table.side_effect = Exception("Database connection error") - - result = get_issue( - owner_type="Organization", - owner_name="test-owner", - repo_name="test-repo", - pr_number=123, - ) - - self.assertIsNone(result) - - @patch("services.supabase.issues.get_issue.supabase") - def test_get_issue_with_zero_pr_number(self, mock_supabase): - issue_data = self.sample_issue_data.copy() - issue_data["issue_number"] = 0 - self._setup_supabase_mock(mock_supabase, [issue_data]) - - result = get_issue( - owner_type="Organization", - owner_name="test-owner", - repo_name="test-repo", - pr_number=0, - ) - - assert result is not None - self.assertEqual(result["issue_number"], 0) - - @patch("services.supabase.issues.get_issue.supabase") - def test_get_issue_with_large_pr_number(self, mock_supabase): - large_pr_number = 999999999 - issue_data = self.sample_issue_data.copy() - issue_data["issue_number"] = large_pr_number - self._setup_supabase_mock(mock_supabase, [issue_data]) - - result = get_issue( - owner_type="User", - owner_name="test-user", - repo_name="test-repo", - pr_number=large_pr_number, - ) - - assert result is not None - self.assertEqual(result["issue_number"], large_pr_number) - - @patch("services.supabase.issues.get_issue.supabase") - def test_get_issue_returns_first_result_only(self, mock_supabase): - first_issue = self.sample_issue_data.copy() - second_issue = self.sample_issue_data.copy() - second_issue["id"] = 2 - - self._setup_supabase_mock(mock_supabase, [first_issue, second_issue]) - - result = get_issue(**self.test_params) - - assert result is not None - self.assertEqual(result["id"], 1) - - @patch("services.supabase.issues.get_issue.supabase") - def test_get_issue_returns_correct_data_structure(self, mock_supabase): - self._setup_supabase_mock(mock_supabase, [self.sample_issue_data]) - - result = get_issue(**self.test_params) - - self.assertEqual(result, self.sample_issue_data) - - @patch("services.supabase.issues.get_issue.supabase") - def test_get_issue_with_none_values(self, mock_supabase): - issue_data_with_nones = self.sample_issue_data.copy() - issue_data_with_nones["run_id"] = None - issue_data_with_nones["created_by"] = None - - self._setup_supabase_mock(mock_supabase, [issue_data_with_nones]) - - result = get_issue(**self.test_params) - - assert result is not None - self.assertIsNone(result["run_id"]) - self.assertIsNone(result["created_by"]) - - @patch("services.supabase.issues.get_issue.supabase") - def test_get_issue_supabase_execute_exception(self, mock_supabase): - mock_chain = ( - mock_supabase.table.return_value.select.return_value.eq.return_value.eq.return_value.eq.return_value.eq.return_value - ) - mock_chain.execute.side_effect = Exception("Supabase execute error") - - result = get_issue(**self.test_params) - - self.assertIsNone(result) - - @patch("services.supabase.issues.get_issue.supabase") - def test_get_issue_with_minimal_required_fields(self, mock_supabase): - minimal_issue_data = { - "id": 999, - "owner_type": "Organization", - "owner_name": "minimal-owner", - "repo_name": "minimal-repo", - "issue_number": 1, - "installation_id": 123, - "merged": False, - "created_at": "2024-01-01T00:00:00Z", - "owner_id": 456, - "repo_id": 789, - } - - self._setup_supabase_mock(mock_supabase, [minimal_issue_data]) - - result = get_issue( - owner_type="Organization", - owner_name="minimal-owner", - repo_name="minimal-repo", - pr_number=1, - ) - - assert result is not None - self.assertEqual(result["id"], 999) - - -@pytest.mark.skipif(bool(os.environ.get("CI")), reason="Integration test") -def test_get_issue_integration(): - result = get_issue("Organization", "gitautoai", "website", 323) - assert result is not None - assert result["owner_name"] == "gitautoai" - assert result["issue_number"] == 323 - - result = get_issue("Organization", "nonexistent", "nonexistent", 999999) - assert result is None - - -if __name__ == "__main__": - unittest.main() diff --git a/services/supabase/issues/test_insert_issue.py b/services/supabase/issues/test_insert_issue.py deleted file mode 100644 index bff78d804..000000000 --- a/services/supabase/issues/test_insert_issue.py +++ /dev/null @@ -1,202 +0,0 @@ -from unittest.mock import patch, MagicMock - -import pytest - -from config import ( - TEST_OWNER_ID, - TEST_OWNER_TYPE, - TEST_OWNER_NAME, - TEST_REPO_ID, - TEST_REPO_NAME, - TEST_ISSUE_NUMBER, - TEST_INSTALLATION_ID, -) -from services.supabase.issues.insert_issue import insert_issue - - -@pytest.fixture -def mock_supabase(): - """Fixture to provide a mocked Supabase client.""" - with patch("services.supabase.issues.insert_issue.supabase") as mock: - mock_table = MagicMock() - mock_insert = MagicMock() - MagicMock() - - mock.table.return_value = mock_table - mock_table.insert.return_value = mock_insert - mock_insert.execute.return_value = None - - yield mock - - -def test_insert_issue_success(mock_supabase): - """Test successful issue insertion with all required parameters.""" - # Act - result = insert_issue( - owner_id=TEST_OWNER_ID, - owner_type=TEST_OWNER_TYPE, - owner_name=TEST_OWNER_NAME, - repo_id=TEST_REPO_ID, - repo_name=TEST_REPO_NAME, - pr_number=TEST_ISSUE_NUMBER, - installation_id=TEST_INSTALLATION_ID, - ) - - # Assert - assert result is None # Function returns None when successful - - # Verify supabase operations were called correctly - mock_supabase.table.assert_called_once_with(table_name="issues") - mock_supabase.table.return_value.insert.assert_called_once_with( - json={ - "owner_id": TEST_OWNER_ID, - "owner_type": TEST_OWNER_TYPE, - "owner_name": TEST_OWNER_NAME, - "repo_id": TEST_REPO_ID, - "repo_name": TEST_REPO_NAME, - "issue_number": TEST_ISSUE_NUMBER, - "installation_id": TEST_INSTALLATION_ID, - } - ) - mock_supabase.table.return_value.insert.return_value.execute.assert_called_once() - - -def test_insert_issue_with_zero_values(mock_supabase): - """Test issue insertion with zero values for numeric fields.""" - # Arrange - owner_id = 0 - repo_id = 0 - pr_number = 0 - installation_id = 0 - - # Act - result = insert_issue( - owner_id=owner_id, - owner_type=TEST_OWNER_TYPE, - owner_name=TEST_OWNER_NAME, - repo_id=repo_id, - repo_name=TEST_REPO_NAME, - pr_number=pr_number, - installation_id=installation_id, - ) - - # Assert - assert result is None - mock_supabase.table.assert_called_once_with(table_name="issues") - mock_supabase.table.return_value.insert.assert_called_once_with( - json={ - "owner_id": owner_id, - "owner_type": TEST_OWNER_TYPE, - "owner_name": TEST_OWNER_NAME, - "repo_id": repo_id, - "repo_name": TEST_REPO_NAME, - "issue_number": pr_number, - "installation_id": installation_id, - } - ) - - -def test_insert_issue_with_empty_strings(mock_supabase): - """Test issue insertion with empty string values.""" - # Arrange - owner_type = "" - owner_name = "" - repo_name = "" - - # Act - result = insert_issue( - owner_id=TEST_OWNER_ID, - owner_type=owner_type, - owner_name=owner_name, - repo_id=TEST_REPO_ID, - repo_name=repo_name, - pr_number=TEST_ISSUE_NUMBER, - installation_id=TEST_INSTALLATION_ID, - ) - - # Assert - assert result is None - mock_supabase.table.assert_called_once_with(table_name="issues") - mock_supabase.table.return_value.insert.assert_called_once_with( - json={ - "owner_id": TEST_OWNER_ID, - "owner_type": owner_type, - "owner_name": owner_name, - "repo_id": TEST_REPO_ID, - "repo_name": repo_name, - "issue_number": TEST_ISSUE_NUMBER, - "installation_id": TEST_INSTALLATION_ID, - } - ) - - -def test_insert_issue_with_large_values(mock_supabase): - """Test issue insertion with large numeric values.""" - # Arrange - large_owner_id = 9999999999 - large_repo_id = 9999999999 - large_pr_number = 9999999999 - large_installation_id = 9999999999 - - # Act - result = insert_issue( - owner_id=large_owner_id, - owner_type=TEST_OWNER_TYPE, - owner_name=TEST_OWNER_NAME, - repo_id=large_repo_id, - repo_name=TEST_REPO_NAME, - pr_number=large_pr_number, - installation_id=large_installation_id, - ) - - # Assert - assert result is None - mock_supabase.table.assert_called_once_with(table_name="issues") - mock_supabase.table.return_value.insert.assert_called_once_with( - json={ - "owner_id": large_owner_id, - "owner_type": TEST_OWNER_TYPE, - "owner_name": TEST_OWNER_NAME, - "repo_id": large_repo_id, - "repo_name": TEST_REPO_NAME, - "issue_number": large_pr_number, - "installation_id": large_installation_id, - } - ) - - -def test_insert_issue_supabase_exception_raises(mock_supabase): - """Test that Supabase exceptions are raised due to @handle_exceptions(raise_on_error=True).""" - # Arrange - mock_supabase.table.return_value.insert.return_value.execute.side_effect = ( - Exception("Database error") - ) - - # Act & Assert - with pytest.raises(Exception, match="Database error"): - insert_issue( - owner_id=TEST_OWNER_ID, - owner_type=TEST_OWNER_TYPE, - owner_name=TEST_OWNER_NAME, - repo_id=TEST_REPO_ID, - repo_name=TEST_REPO_NAME, - pr_number=TEST_ISSUE_NUMBER, - installation_id=TEST_INSTALLATION_ID, - ) - - -def test_insert_issue_table_method_called_correctly(mock_supabase): - """Test that the supabase table method is called with the correct table name.""" - # Act - insert_issue( - owner_id=TEST_OWNER_ID, - owner_type=TEST_OWNER_TYPE, - owner_name=TEST_OWNER_NAME, - repo_id=TEST_REPO_ID, - repo_name=TEST_REPO_NAME, - pr_number=TEST_ISSUE_NUMBER, - installation_id=TEST_INSTALLATION_ID, - ) - - # Assert - Verify table method is called with correct table name - mock_supabase.table.assert_called_once_with(table_name="issues") diff --git a/services/supabase/issues/test_update_issue_merged.py b/services/supabase/issues/test_update_issue_merged.py deleted file mode 100644 index 1a4b34360..000000000 --- a/services/supabase/issues/test_update_issue_merged.py +++ /dev/null @@ -1,426 +0,0 @@ -import json -from unittest.mock import patch, MagicMock - -import pytest -import requests - -from config import ( - TEST_OWNER_TYPE, - TEST_OWNER_NAME, - TEST_REPO_NAME, - TEST_ISSUE_NUMBER, -) -from services.supabase.issues.update_issue_merged import update_issue_merged - - -@pytest.fixture -def mock_supabase(): - """Fixture to provide a mocked supabase client.""" - with patch("services.supabase.issues.update_issue_merged.supabase") as mock: - yield mock - - -@pytest.fixture -def sample_parameters(): - """Fixture providing sample parameters for the function.""" - return { - "owner_type": TEST_OWNER_TYPE, - "owner_name": TEST_OWNER_NAME, - "repo_name": TEST_REPO_NAME, - "pr_number": TEST_ISSUE_NUMBER, - "merged": True, - } - - -class TestUpdateIssueMerged: - """Test cases for update_issue_merged function.""" - - def test_update_issue_merged_success_with_default_merged_true( - self, mock_supabase, sample_parameters - ): - """Test successful update with default merged=True.""" - # Arrange - mock_table = MagicMock() - mock_update = MagicMock() - mock_eq_chain = MagicMock() - - mock_supabase.table.return_value = mock_table - mock_table.update.return_value = mock_update - mock_update.eq.return_value = mock_eq_chain - mock_eq_chain.eq.return_value = mock_eq_chain - mock_eq_chain.execute.return_value = MagicMock() - - # Act - call without merged parameter to test default - update_issue_merged( - owner_type=sample_parameters["owner_type"], - owner_name=sample_parameters["owner_name"], - repo_name=sample_parameters["repo_name"], - pr_number=sample_parameters["pr_number"], - ) - - # Assert - mock_supabase.table.assert_called_once_with(table_name="issues") - mock_table.update.assert_called_once_with(json={"merged": True}) - - # Verify the chain of eq calls - eq_calls = mock_update.eq.call_args_list + mock_eq_chain.eq.call_args_list - assert len(eq_calls) == 4 - - # Check that all expected eq calls were made with keyword arguments - expected_calls = [ - {"column": "owner_type", "value": TEST_OWNER_TYPE}, - {"column": "owner_name", "value": TEST_OWNER_NAME}, - {"column": "repo_name", "value": TEST_REPO_NAME}, - {"column": "issue_number", "value": TEST_ISSUE_NUMBER}, - ] - - for expected_call in expected_calls: - assert expected_call in [call[1] for call in eq_calls] - - mock_eq_chain.execute.assert_called_once() - - def test_update_issue_merged_success_with_merged_true( - self, mock_supabase, sample_parameters - ): - """Test successful update with merged=True explicitly.""" - # Arrange - mock_table = MagicMock() - mock_update = MagicMock() - mock_eq_chain = MagicMock() - - mock_supabase.table.return_value = mock_table - mock_table.update.return_value = mock_update - mock_update.eq.return_value = mock_eq_chain - mock_eq_chain.eq.return_value = mock_eq_chain - mock_eq_chain.execute.return_value = MagicMock() - - # Act - update_issue_merged(**sample_parameters) - - # Assert - mock_supabase.table.assert_called_once_with(table_name="issues") - mock_table.update.assert_called_once_with(json={"merged": True}) - mock_eq_chain.execute.assert_called_once() - - def test_update_issue_merged_success_with_merged_false( - self, mock_supabase, sample_parameters - ): - """Test successful update with merged=False.""" - # Arrange - mock_table = MagicMock() - mock_update = MagicMock() - mock_eq_chain = MagicMock() - - mock_supabase.table.return_value = mock_table - mock_table.update.return_value = mock_update - mock_update.eq.return_value = mock_eq_chain - mock_eq_chain.eq.return_value = mock_eq_chain - mock_eq_chain.execute.return_value = MagicMock() - - # Modify parameters to set merged=False - params = sample_parameters.copy() - params["merged"] = False - - # Act - update_issue_merged(**params) - - # Assert - mock_supabase.table.assert_called_once_with(table_name="issues") - mock_table.update.assert_called_once_with(json={"merged": False}) - mock_eq_chain.execute.assert_called_once() - - def test_update_issue_merged_with_different_owner_types(self, mock_supabase): - """Test function with different owner types.""" - # Arrange - mock_table = MagicMock() - mock_update = MagicMock() - mock_eq_chain = MagicMock() - - mock_supabase.table.return_value = mock_table - mock_table.update.return_value = mock_update - mock_update.eq.return_value = mock_eq_chain - mock_eq_chain.eq.return_value = mock_eq_chain - mock_eq_chain.execute.return_value = MagicMock() - - owner_types = ["User", "Organization", "Bot"] - - for owner_type in owner_types: - # Reset mocks for each iteration - mock_update.reset_mock() - mock_eq_chain.reset_mock() - # Act - update_issue_merged( - owner_type=owner_type, - owner_name="test_owner", - repo_name="test_repo", - pr_number=123, - ) - - # Assert that the correct owner_type was used - eq_calls = mock_update.eq.call_args_list + mock_eq_chain.eq.call_args_list - owner_type_call = next( - call for call in eq_calls if call[1].get("column") == "owner_type" - ) - assert owner_type_call[1]["value"] == owner_type - - def test_update_issue_merged_with_various_pr_numbers(self, mock_supabase): - """Test function with various PR number values.""" - # Arrange - mock_table = MagicMock() - mock_update = MagicMock() - mock_eq_chain = MagicMock() - - mock_supabase.table.return_value = mock_table - mock_table.update.return_value = mock_update - mock_update.eq.return_value = mock_eq_chain - mock_eq_chain.eq.return_value = mock_eq_chain - mock_eq_chain.execute.return_value = MagicMock() - - pr_numbers = [1, 999, 12345, 999999] - - for pr_num in pr_numbers: - # Reset mocks for each iteration - mock_update.reset_mock() - mock_eq_chain.reset_mock() - # Act - update_issue_merged( - owner_type="Organization", - owner_name="test_owner", - repo_name="test_repo", - pr_number=pr_num, - ) - - # Assert that the correct issue_number DB column was used - eq_calls = mock_update.eq.call_args_list + mock_eq_chain.eq.call_args_list - issue_number_call = next( - call for call in eq_calls if call[1].get("column") == "issue_number" - ) - assert issue_number_call[1]["value"] == pr_num - - def test_update_issue_merged_with_special_characters_in_names(self, mock_supabase): - """Test function with special characters in owner and repo names.""" - # Arrange - mock_table = MagicMock() - mock_update = MagicMock() - mock_eq_chain = MagicMock() - - mock_supabase.table.return_value = mock_table - mock_table.update.return_value = mock_update - mock_update.eq.return_value = mock_eq_chain - mock_eq_chain.eq.return_value = mock_eq_chain - mock_eq_chain.execute.return_value = MagicMock() - - test_cases = [ - ("owner-with-dashes", "repo-with-dashes"), - ("owner_with_underscores", "repo_with_underscores"), - ("owner.with.dots", "repo.with.dots"), - ("owner123", "repo456"), - ("UPPERCASE", "REPO"), - ] - - for owner_name, repo_name in test_cases: - # Reset mocks for each iteration - mock_update.reset_mock() - mock_eq_chain.reset_mock() - # Act - update_issue_merged( - owner_type="Organization", - owner_name=owner_name, - repo_name=repo_name, - pr_number=1, - ) - - # Assert that the correct names were used - eq_calls = mock_update.eq.call_args_list + mock_eq_chain.eq.call_args_list - - owner_name_call = next( - call for call in eq_calls if call[1].get("column") == "owner_name" - ) - assert owner_name_call[1]["value"] == owner_name - - repo_name_call = next( - call for call in eq_calls if call[1].get("column") == "repo_name" - ) - assert repo_name_call[1]["value"] == repo_name - - def test_update_issue_merged_exception_handling_returns_none(self, mock_supabase): - """Test that function returns None when an exception occurs.""" - # Arrange - mock_supabase.table.side_effect = Exception("Database connection error") - - # Act - result = update_issue_merged( - owner_type="Organization", - owner_name="test_owner", - repo_name="test_repo", - pr_number=123, - ) - - # Assert - assert result is None - - def test_update_issue_merged_http_error_500_returns_none(self, mock_supabase): - """Test that function returns None when HTTP 500 error occurs.""" - # Arrange - mock_response = MagicMock() - mock_response.status_code = 500 - mock_response.reason = "Internal Server Error" - mock_response.text = "Server error" - - http_error = requests.exceptions.HTTPError("500 Server Error") - http_error.response = mock_response - mock_supabase.table.side_effect = http_error - - # Act - result = update_issue_merged( - owner_type="Organization", - owner_name="test_owner", - repo_name="test_repo", - pr_number=123, - ) - - # Assert - assert result is None - - def test_update_issue_merged_http_error_other_returns_none(self, mock_supabase): - """Test that function returns None when other HTTP errors occur.""" - # Arrange - mock_response = MagicMock() - mock_response.status_code = 404 - mock_response.reason = "Not Found" - mock_response.text = "Resource not found" - - http_error = requests.exceptions.HTTPError("404 Not Found") - http_error.response = mock_response - mock_supabase.table.side_effect = http_error - - # Act - result = update_issue_merged( - owner_type="Organization", - owner_name="test_owner", - repo_name="test_repo", - pr_number=123, - ) - - # Assert - assert result is None - - def test_update_issue_merged_json_decode_error_returns_none(self, mock_supabase): - """Test that function returns None when JSON decode error occurs.""" - # Arrange - json_error = json.JSONDecodeError("Invalid JSON", "", 0) - mock_supabase.table.side_effect = json_error - - # Act - result = update_issue_merged( - owner_type="Organization", - owner_name="test_owner", - repo_name="test_repo", - pr_number=123, - ) - - # Assert - assert result is None - - def test_update_issue_merged_attribute_error_returns_none(self, mock_supabase): - """Test that function returns None when AttributeError occurs.""" - # Arrange - mock_supabase.table.side_effect = AttributeError("Attribute not found") - - # Act - result = update_issue_merged( - owner_type="Organization", - owner_name="test_owner", - repo_name="test_repo", - pr_number=123, - ) - - # Assert - assert result is None - - def test_update_issue_merged_key_error_returns_none(self, mock_supabase): - """Test that function returns None when KeyError occurs.""" - # Arrange - mock_supabase.table.side_effect = KeyError("Key not found") - - # Act - result = update_issue_merged( - owner_type="Organization", - owner_name="test_owner", - repo_name="test_repo", - pr_number=123, - ) - - # Assert - assert result is None - - def test_update_issue_merged_type_error_returns_none(self, mock_supabase): - """Test that function returns None when TypeError occurs.""" - # Arrange - mock_supabase.table.side_effect = TypeError("Type error") - - # Act - result = update_issue_merged( - owner_type="Organization", - owner_name="test_owner", - repo_name="test_repo", - pr_number=123, - ) - - # Assert - assert result is None - - def test_update_issue_merged_supabase_query_structure(self, mock_supabase): - """Test that the correct Supabase query structure is constructed.""" - # Arrange - mock_table = MagicMock() - mock_update = MagicMock() - mock_eq_chain = MagicMock() - - mock_supabase.table.return_value = mock_table - mock_table.update.return_value = mock_update - mock_update.eq.return_value = mock_eq_chain - mock_eq_chain.eq.return_value = mock_eq_chain - mock_eq_chain.execute.return_value = MagicMock() - - # Act - update_issue_merged( - owner_type="Organization", - owner_name="test_owner", - repo_name="test_repo", - pr_number=123, - merged=True, - ) - - # Assert the complete query chain - mock_supabase.table.assert_called_once_with(table_name="issues") - mock_table.update.assert_called_once_with(json={"merged": True}) - mock_eq_chain.execute.assert_called_once() - - @pytest.mark.parametrize("merged_value", [True, False]) - def test_update_issue_merged_with_parametrized_merged_values( - self, mock_supabase, merged_value - ): - """Test function with various merged values using parametrize.""" - # Arrange - mock_table = MagicMock() - mock_update = MagicMock() - mock_eq_chain = MagicMock() - - mock_supabase.table.return_value = mock_table - mock_table.update.return_value = mock_update - mock_update.eq.return_value = mock_eq_chain - mock_eq_chain.eq.return_value = mock_eq_chain - mock_eq_chain.execute.return_value = MagicMock() - - # Act - update_issue_merged( - owner_type="Organization", - owner_name="test_owner", - repo_name="test_repo", - pr_number=123, - merged=merged_value, - ) - - # Assert - mock_table.update.assert_called_once_with(json={"merged": merged_value}) diff --git a/services/supabase/issues/update_issue_merged.py b/services/supabase/issues/update_issue_merged.py deleted file mode 100644 index cfda67446..000000000 --- a/services/supabase/issues/update_issue_merged.py +++ /dev/null @@ -1,21 +0,0 @@ -from services.supabase.client import supabase -from utils.error.handle_exceptions import handle_exceptions - - -@handle_exceptions(default_return_value=None, raise_on_error=False) -def update_issue_merged( - owner_type: str, - owner_name: str, - repo_name: str, - pr_number: int, - merged: bool = True, -): - ( - supabase.table(table_name="issues") - .update(json={"merged": merged}) - .eq(column="owner_type", value=owner_type) - .eq(column="owner_name", value=owner_name) - .eq(column="repo_name", value=repo_name) - .eq(column="issue_number", value=pr_number) - .execute() - ) diff --git a/services/supabase/test_create_user_request.py b/services/supabase/test_create_user_request.py index d895d47ab..e637d1bd1 100644 --- a/services/supabase/test_create_user_request.py +++ b/services/supabase/test_create_user_request.py @@ -32,47 +32,26 @@ def sample_params(self): def mock_dependencies(self): """Mock all dependencies.""" with patch( - "services.supabase.create_user_request.get_issue" - ) as mock_get_issue, patch( - "services.supabase.create_user_request.insert_issue" - ) as mock_insert_issue, patch( "services.supabase.create_user_request.insert_usage" ) as mock_insert_usage, patch( "services.supabase.create_user_request.upsert_user" ) as mock_upsert_user: yield { - "get_issue": mock_get_issue, - "insert_issue": mock_insert_issue, "insert_usage": mock_insert_usage, "upsert_user": mock_upsert_user, } - def test_create_user_request_existing_issue(self, sample_params, mock_dependencies): - """Test create_user_request when issue already exists.""" - # Setup - existing_issue = {"id": 1, "pr_number": 123} - mock_dependencies["get_issue"].return_value = existing_issue + def test_create_user_request_returns_usage_id( + self, sample_params, mock_dependencies + ): + """Test create_user_request returns usage_id from insert_usage.""" mock_dependencies["insert_usage"].return_value = 999 - # Execute result = create_user_request(**sample_params) - # Assert assert result == 999 - # Verify get_issue was called with correct parameters - mock_dependencies["get_issue"].assert_called_once_with( - owner_type="Organization", - owner_name="test_org", - repo_name="test_repo", - pr_number=123, - ) - - # Verify insert_issue was NOT called since issue exists - mock_dependencies["insert_issue"].assert_not_called() - - # Verify insert_usage was called with correct parameters mock_dependencies["insert_usage"].assert_called_once_with( owner_id=11111, owner_type="Organization", @@ -89,78 +68,33 @@ def test_create_user_request_existing_issue(self, sample_params, mock_dependenci lambda_request_id=None, ) - # Verify upsert_user was called with correct parameters mock_dependencies["upsert_user"].assert_called_once_with( user_id=12345, user_name="test_user", email="test@example.com", ) - def test_create_user_request_new_issue(self, sample_params, mock_dependencies): - """Test create_user_request when issue doesn't exist.""" - # Setup - mock_dependencies["get_issue"].return_value = None - mock_dependencies["insert_usage"].return_value = 888 - - # Execute - result = create_user_request(**sample_params) - - # Assert - assert result == 888 - - # Verify get_issue was called - mock_dependencies["get_issue"].assert_called_once_with( - owner_type="Organization", - owner_name="test_org", - repo_name="test_repo", - pr_number=123, - ) - - # Verify insert_issue WAS called since issue doesn't exist - mock_dependencies["insert_issue"].assert_called_once_with( - owner_id=11111, - owner_type="Organization", - owner_name="test_org", - repo_id=22222, - repo_name="test_repo", - pr_number=123, - installation_id=67890, - ) - - # Verify insert_usage was called - mock_dependencies["insert_usage"].assert_called_once() - - # Verify upsert_user was called - mock_dependencies["upsert_user"].assert_called_once() - def test_create_user_request_without_pr_number( self, sample_params, mock_dependencies ): """Test create_user_request without pr_number raises TypeError.""" - # Setup - pr_number is now required, so removing it should cause an error params_without_pr = sample_params.copy() del params_without_pr["pr_number"] - # Execute - should raise TypeError for missing required argument with pytest.raises(TypeError): create_user_request(**params_without_pr) def test_create_user_request_without_email(self, sample_params, mock_dependencies): """Test create_user_request without email.""" - # Setup params_without_email = sample_params.copy() params_without_email["email"] = None - mock_dependencies["get_issue"].return_value = {"id": 1} mock_dependencies["insert_usage"].return_value = 666 - # Execute result = create_user_request(**params_without_email) - # Assert assert result == 666 - # Verify upsert_user was called with email=None mock_dependencies["upsert_user"].assert_called_once_with( user_id=12345, user_name="test_user", @@ -179,189 +113,68 @@ def test_create_user_request_different_trigger_types( ] for trigger in triggers: - # Setup params = sample_params.copy() params["trigger"] = trigger - mock_dependencies["get_issue"].return_value = {"id": 1} mock_dependencies["insert_usage"].return_value = 555 - # Reset mocks for mock in mock_dependencies.values(): mock.reset_mock() - # Execute result = create_user_request(**params) - # Assert assert result == 555 - # Verify insert_usage was called with correct trigger mock_dependencies["insert_usage"].assert_called_once() call_args = mock_dependencies["insert_usage"].call_args[1] assert call_args["trigger"] == trigger - def test_create_user_request_user_owner_type( - self, sample_params, mock_dependencies - ): - """Test create_user_request with User owner type.""" - # Setup - params = sample_params.copy() - params["owner_type"] = "User" - params["owner_name"] = "individual_user" - - mock_dependencies["get_issue"].return_value = None - mock_dependencies["insert_usage"].return_value = 444 - - # Execute - result = create_user_request(**params) - - # Assert - assert result == 444 - - # Verify get_issue was called with User owner_type - mock_dependencies["get_issue"].assert_called_once_with( - owner_type="User", - owner_name="individual_user", - repo_name="test_repo", - pr_number=123, - ) - - # Verify insert_issue was called with User owner_type - mock_dependencies["insert_issue"].assert_called_once_with( - owner_id=11111, - owner_type="User", - owner_name="individual_user", - repo_id=22222, - repo_name="test_repo", - pr_number=123, - installation_id=67890, - ) - - def test_create_user_request_with_special_characters( - self, sample_params, mock_dependencies - ): - """Test create_user_request with special characters in names.""" - # Setup - params = sample_params.copy() - params["owner_name"] = "org-with-dashes" - params["repo_name"] = "repo_with_underscores" - params["user_name"] = "user.with.dots" - - mock_dependencies["get_issue"].return_value = {"id": 1} - mock_dependencies["insert_usage"].return_value = 333 - - # Execute - result = create_user_request(**params) - - # Assert - assert result == 333 - - # Verify all functions were called with special character names - mock_dependencies["get_issue"].assert_called_once_with( - owner_type="Organization", - owner_name="org-with-dashes", - repo_name="repo_with_underscores", - pr_number=123, - ) - - mock_dependencies["upsert_user"].assert_called_once_with( - user_id=12345, - user_name="user.with.dots", - email="test@example.com", - ) - - def test_create_user_request_with_zero_pr_number( - self, sample_params, mock_dependencies - ): - """Test create_user_request with pr number 0.""" - # Setup - params = sample_params.copy() - params["pr_number"] = 0 - - mock_dependencies["get_issue"].return_value = None - mock_dependencies["insert_usage"].return_value = 222 - - # Execute - result = create_user_request(**params) - - # Assert - assert result == 222 - - # Verify functions were called with pr_number=0 - mock_dependencies["get_issue"].assert_called_once_with( - owner_type="Organization", - owner_name="test_org", - repo_name="test_repo", - pr_number=0, - ) - def test_create_user_request_exception_handling(self, sample_params): """Test that exceptions are raised by the decorator.""" - with patch("services.supabase.create_user_request.get_issue") as mock_get_issue: - # Setup mock to raise exception - mock_get_issue.side_effect = Exception("Database error") + with patch( + "services.supabase.create_user_request.insert_usage" + ) as mock_insert_usage: + mock_insert_usage.side_effect = Exception("Database error") - # Execute - should raise exception due to @handle_exceptions(raise_on_error=True) with pytest.raises(Exception) as exc_info: create_user_request(**sample_params) - # Assert - should raise the original exception assert str(exc_info.value) == "Database error" def test_create_user_request_insert_usage_returns_none( self, sample_params, mock_dependencies ): """Test when insert_usage returns None.""" - # Setup - mock_dependencies["get_issue"].return_value = {"id": 1} mock_dependencies["insert_usage"].return_value = None - # Execute result = create_user_request(**sample_params) - # Assert assert result is None - # Verify all functions were still called - mock_dependencies["get_issue"].assert_called_once() mock_dependencies["insert_usage"].assert_called_once() mock_dependencies["upsert_user"].assert_called_once() - def test_create_user_request_large_ids(self, sample_params, mock_dependencies): - """Test create_user_request with large ID values.""" - # Setup + def test_create_user_request_with_lambda_info( + self, sample_params, mock_dependencies + ): + """Test create_user_request with lambda_info provided.""" params = sample_params.copy() - params["user_id"] = 999999999 - params["owner_id"] = 888888888 - params["repo_id"] = 777777777 - params["installation_id"] = 666666666 + params["lambda_info"] = { + "log_group": "/aws/lambda/test", + "log_stream": "2026/01/01/[$LATEST]abc123", + "request_id": "req-123", + } - mock_dependencies["get_issue"].return_value = None - mock_dependencies["insert_usage"].return_value = 111 + mock_dependencies["insert_usage"].return_value = 777 - # Execute result = create_user_request(**params) - # Assert - assert result == 111 - - # Verify functions were called with large IDs - mock_dependencies["insert_issue"].assert_called_once_with( - owner_id=888888888, - owner_type="Organization", - owner_name="test_org", - repo_id=777777777, - repo_name="test_repo", - pr_number=123, - installation_id=666666666, - ) + assert result == 777 - mock_dependencies["upsert_user"].assert_called_once_with( - user_id=999999999, - user_name="test_user", - email="test@example.com", - ) + call_args = mock_dependencies["insert_usage"].call_args[1] + assert call_args["lambda_log_group"] == "/aws/lambda/test" + assert call_args["lambda_log_stream"] == "2026/01/01/[$LATEST]abc123" + assert call_args["lambda_request_id"] == "req-123" def test_create_user_request_different_sources( self, sample_params, mock_dependencies @@ -370,42 +183,18 @@ def test_create_user_request_different_sources( sources = ["github", "webhook", "api", "manual"] for source in sources: - # Setup params = sample_params.copy() params["source"] = source - mock_dependencies["get_issue"].return_value = {"id": 1} mock_dependencies["insert_usage"].return_value = 100 - # Reset mocks for mock in mock_dependencies.values(): mock.reset_mock() - # Execute result = create_user_request(**params) - # Assert assert result == 100 - # Verify insert_usage was called with correct source mock_dependencies["insert_usage"].assert_called_once() call_args = mock_dependencies["insert_usage"].call_args[1] assert call_args["source"] == source - - def test_create_user_request_function_call_order( - self, sample_params, mock_dependencies - ): - """Test that functions are called in the correct order.""" - # Setup - mock_dependencies["get_issue"].return_value = None - mock_dependencies["insert_usage"].return_value = 123 - - # Execute - create_user_request(**sample_params) - - # Verify call order by checking that get_issue is called before insert_issue - # and insert_usage is called after both - assert mock_dependencies["get_issue"].call_count == 1 - assert mock_dependencies["insert_issue"].call_count == 1 - assert mock_dependencies["insert_usage"].call_count == 1 - assert mock_dependencies["upsert_user"].call_count == 1