From d0ab1764a7a7789bfa87d17959b65e32f9051626 Mon Sep 17 00:00:00 2001 From: Sravan Reddy Date: Sun, 15 Dec 2024 19:49:55 +0530 Subject: [PATCH] refactor tests --- .../opportunity/payment_number_report.py | 21 +- .../opportunity/tests/test_views.py | 227 +++++++++++++----- 2 files changed, 186 insertions(+), 62 deletions(-) diff --git a/commcare_connect/opportunity/payment_number_report.py b/commcare_connect/opportunity/payment_number_report.py index 25b80b14..80d80766 100644 --- a/commcare_connect/opportunity/payment_number_report.py +++ b/commcare_connect/opportunity/payment_number_report.py @@ -97,7 +97,7 @@ def object_list(self): connecteid_statuses = fetch_payment_phone_numbers(usernames, status) # display local status when its overridden local_statuses = OrgUserPaymentNumberStatus.objects.filter( - user__username__in=usernames, organization__name=self.request.org + user__username__in=usernames, organization=self.request.org ) local_statuses_by_username = {status.username: status for status in local_statuses} for status in connecteid_statuses: @@ -167,7 +167,7 @@ def update_payment_number_statuses(update_data, opportunity): for u in update_data } - existing_statuses = OrgUserPaymentNumberStatus.objects.filter(user__username=update_by_username.keys()).all() + existing_statuses = OrgUserPaymentNumberStatus.objects.filter(user__username__in=update_by_username.keys()).all() # remove unchanged updates and gather current-status # and status set by other orgs @@ -190,7 +190,7 @@ def update_payment_number_statuses(update_data, opportunity): with transaction.atomic(): objs = [ OrgUserPaymentNumberStatus( - organization=opportunity.org, + organization=opportunity.organization, user=u.user, phone_number=u.phone_number, status=u.new_status, @@ -198,13 +198,24 @@ def update_payment_number_statuses(update_data, opportunity): for u in update_by_username.values() ] # Bulk update/create - objs.bulk_create(objs, update_conflicts=True) + OrgUserPaymentNumberStatus.objects.bulk_create( + objs, + update_conflicts=True, + unique_fields=["user", "organization"], + update_fields=["phone_number", "status"], + ) # Process connect-id updates and push-notifications connectid_updates = [] rejected_usernames = [] approved_usernames = [] + result = { + OrgUserPaymentNumberStatus.APPROVED: 0, + OrgUserPaymentNumberStatus.REJECTED: 0, + OrgUserPaymentNumberStatus.PENDING: 0, + } for update in update_by_username.values(): + result[update.new_status] += result[update.new_status] + 1 if (not update.other_org_statuses) or {update.new_status} == update.other_org_statuses: # only send update on connectid if there is no disaggrement bw orgs # connectid stores status and triggers relevant notifications @@ -228,4 +239,4 @@ def update_payment_number_statuses(update_data, opportunity): approved_msg = f"{opportunity.name} is now able to send payments to you" send_message_bulk(Message(usernames=approved_usernames, body=approved_msg)) - return {"approved": len(approved_usernames), "rejected": len(rejected_usernames)} + return result diff --git a/commcare_connect/opportunity/tests/test_views.py b/commcare_connect/opportunity/tests/test_views.py index f39c365b..1c121fbf 100644 --- a/commcare_connect/opportunity/tests/test_views.py +++ b/commcare_connect/opportunity/tests/test_views.py @@ -13,6 +13,7 @@ ) from commcare_connect.organization.models import Organization, OrgUserPaymentNumberStatus from commcare_connect.users.models import User +from commcare_connect.users.tests.factories import MobileUserFactory, OrganizationFactory @pytest.mark.django_db @@ -45,68 +46,180 @@ def test_add_budget_existing_users( @pytest.mark.django_db @pytest.mark.parametrize( - "update_data, existing_statuses, expected_updates", + "scenario", [ - # Case 1: Agreement exists, an org disagrees - ( - [{"username": "user1", "phone_number": "123", "status": "REJECTED"}], - [OrgUserPaymentNumberStatus(user=User(username="user1"), phone_number="123", status="APPROVED")], - [{"username": "user1", "status": "REJECTED"}], - ), - # Case 2: Disagreement turns into agreement - ( - [{"username": "user2", "phone_number": "456", "status": "APPROVED"}], - [OrgUserPaymentNumberStatus(user=User(username="user2"), phone_number="456", status="REJECTED")], - [{"username": "user2", "status": "APPROVED"}], - ), - # Case 3: No other org has any data - ( - [{"username": "user3", "phone_number": "789", "status": "APPROVED"}], - [], - [{"username": "user3", "status": "APPROVED"}], - ), - # Case 4: No changes - ( - [{"username": "user4", "phone_number": "101", "status": "APPROVED"}], - [OrgUserPaymentNumberStatus(user=User(username="user4"), phone_number="101", status="APPROVED")], - [], - ), - # Case 5: Pending status update - ( - [{"username": "user5", "phone_number": "112", "status": "PENDING"}], - [OrgUserPaymentNumberStatus(user=User(username="user5"), phone_number="112", status="REJECTED")], - [{"username": "user5", "status": "PENDING"}], - ), + { + "name": "new_entries", + "existing_statuses": [], + "update_data": [ + { + "username": "test_user1", + "phone_number": "+1234567890", + "status": OrgUserPaymentNumberStatus.APPROVED, + }, + { + "username": "test_user2", + "phone_number": "+9876543210", + "status": OrgUserPaymentNumberStatus.REJECTED, + }, + ], + "expected_result": {"approved": 1, "rejected": 1, "pending": 0}, + "expect_connectid_update": True, + "expect_message": False, + }, + { + "name": "new_entries_partial", + "existing_statuses": [ + { + "username": "test_user1", + "phone_number": "+1234567890", + "status": OrgUserPaymentNumberStatus.REJECTED, + } + ], + "update_data": [ + { + "username": "test_user1", + "phone_number": "+1234567890", + "status": OrgUserPaymentNumberStatus.APPROVED, + }, + { + "username": "test_user2", + "phone_number": "+9876543210", + "status": OrgUserPaymentNumberStatus.REJECTED, + }, + ], + "expected_result": {"approved": 1, "rejected": 1, "pending": 0}, + "expect_connectid_update": True, + "expect_message": False, + }, + { + "name": "unchanged_status", + "existing_statuses": [ + { + "username": "test_user1", + "phone_number": "+1234567890", + "status": OrgUserPaymentNumberStatus.APPROVED, + } + ], + "update_data": [ + { + "username": "test_user1", + "phone_number": "+1234567890", + "status": OrgUserPaymentNumberStatus.APPROVED, + } + ], + "expected_result": {"approved": 0, "rejected": 0, "pending": 0}, + "expect_connectid_update": False, + "expect_message": False, + }, + { + "name": "changed_status", + "existing_statuses": [ + { + "username": "test_user1", + "phone_number": "+1234567890", + "status": OrgUserPaymentNumberStatus.APPROVED, + } + ], + "update_data": [ + {"username": "test_user1", "phone_number": "+1234567890", "status": OrgUserPaymentNumberStatus.PENDING} + ], + "expected_result": {"approved": 0, "rejected": 0, "pending": 1}, + "expect_connectid_update": True, + "expect_message": False, + }, + { + "name": "phone_changed", + "existing_statuses": [ + { + "username": "test_user1", + "phone_number": "+1234567890", + "status": OrgUserPaymentNumberStatus.APPROVED, + } + ], + "update_data": [ + { + "username": "test_user1", + "phone_number": "+2344567890", + "status": OrgUserPaymentNumberStatus.APPROVED, + } + ], + "expected_result": {"approved": 1, "rejected": 0, "pending": 0}, + "expect_connectid_update": True, + "expect_message": False, + }, + { + "name": "inter_org_conflict", + "existing_statuses": [ + { + "username": "test_user1", + "phone_number": "+1234567890", + "status": OrgUserPaymentNumberStatus.APPROVED, + "organization": "another_org", + } + ], + "update_data": [ + { + "username": "test_user1", + "phone_number": "+1234567890", + "status": OrgUserPaymentNumberStatus.REJECTED, + } + ], + "expected_result": {"approved": 0, "rejected": 1, "pending": 0}, + "expect_connectid_update": False, + "expect_message": True, + }, ], ) -@patch("commcare_connect.opportunity.payment_number_report.send_message_bulk") -@patch("commcare_connect.opportunity.payment_number_report.update_payment_statuses") -@patch("commcare_connect.organization.models.OrgUserPaymentNumberStatus.objects.filter") -@patch("commcare_connect.users.models.User.objects.filter") -def test_validate_payment_profiles( - mock_user_filter, - mock_status_filter, - mock_update_payment_statuses, - mock_send_message_bulk, - update_data, - existing_statuses, - expected_updates, -): - # Setup mocks - mock_users = [User(username=u["username"]) for u in update_data] - mock_user_filter.return_value = MagicMock(all=MagicMock(return_value=mock_users)) +def test_update_payment_number_statuses(scenario, opportunity): + # Prepare existing statuses + usernames = {s["username"] for s in scenario["existing_statuses"] + scenario["update_data"]} + by_username = {} + for username in usernames: + by_username[username] = MobileUserFactory(username=username) + + for status_data in scenario.get("existing_statuses", []): + org = ( + opportunity.organization + if status_data.get("organization") != "another_org" + else OrganizationFactory(name=status_data["organization"]) + ) + user = by_username[status_data["username"]] + + OrgUserPaymentNumberStatus.objects.create( + organization=org, user=user, phone_number=status_data["phone_number"], status=status_data["status"] + ) - mock_update_payment_statuses.return_value = MagicMock(status=200, json=lambda: {"result": "success"}) + # Mock external service calls + with patch( + "commcare_connect.opportunity.payment_number_report.update_payment_statuses" + ) as mock_update_connectid, patch( + "commcare_connect.opportunity.payment_number_report.send_message_bulk" + ) as mock_send_message: + mock_update_connectid.return_value = MagicMock(status=200) + result = update_payment_number_statuses(scenario["update_data"], opportunity) - organization = Organization(name="Test Organization") + if scenario["expect_connectid_update"]: + mock_update_connectid.assert_called_once() + else: + mock_update_connectid.assert_not_called() - opportunity = MagicMock(name="Test Opportunity", organization=organization) + if scenario["expect_message"]: + mock_send_message.assert_called_once() + else: + mock_send_message.assert_not_called() - # Call the function - update_payment_number_statuses(update_data, opportunity) + assert result == scenario["expected_result"] - # Assertions - if expected_updates: - mock_update_payment_statuses.assert_called_once_with(expected_updates) - else: - mock_update_payment_statuses.assert_not_called() + # Verify database entries + for entry in scenario["update_data"]: + try: + status_obj = OrgUserPaymentNumberStatus.objects.get( + user__username=entry["username"], + organization=opportunity.organization, + ) + assert status_obj.phone_number == entry["phone_number"] + assert status_obj.status == entry["status"] + except OrgUserPaymentNumberStatus.DoesNotExist: + if scenario["expected_result"]["approved"] + scenario["expected_result"]["rejected"] > 0: + pytest.fail(f"Expected status entry not found for {entry}")