From 9b3628bd6e7806ce7edddb0f5b45a1535594ba67 Mon Sep 17 00:00:00 2001 From: Sravan Reddy Date: Sun, 10 Nov 2024 16:55:31 +0530 Subject: [PATCH 01/12] UI to validate payment phone numbers --- commcare_connect/connect_id_client/main.py | 12 ++ .../opportunity/payment_number_report.py | 106 ++++++++++++++++++ commcare_connect/opportunity/urls.py | 2 + commcare_connect/reports/views.py | 58 ++++++---- .../opportunity/payment_numbers_table.html | 16 +++ .../templates/reports/report_table.html | 2 +- 6 files changed, 174 insertions(+), 22 deletions(-) create mode 100644 commcare_connect/opportunity/payment_number_report.py create mode 100644 commcare_connect/templates/opportunity/payment_numbers_table.html diff --git a/commcare_connect/connect_id_client/main.py b/commcare_connect/connect_id_client/main.py index 1f5307a1..c85b091d 100644 --- a/commcare_connect/connect_id_client/main.py +++ b/commcare_connect/connect_id_client/main.py @@ -67,6 +67,18 @@ def filter_users(country_code: str, credential: list[str]): return [ConnectIdUser(**user_dict) for user_dict in data["found_users"]] +def fetch_payment_phone_numbers(usernames, status): + response = _make_request( + GET, "/users/fetch_payment_phone_numbers", params={"usernames": usernames, "status": status} + ) + return response.json()["found_payment_numbers"] + + +def validate_phone_numbers(update_data): + response = _make_request(POST, "/users/validate_payment_phone_numbers", json={"updates": update_data}) + return response.json()["result"] + + def _make_request(method, path, params=None, json=None, timeout=5) -> Response: if json and not method == "POST": raise ValueError("json can only be used with POST requests") diff --git a/commcare_connect/opportunity/payment_number_report.py b/commcare_connect/opportunity/payment_number_report.py new file mode 100644 index 00000000..bfe31702 --- /dev/null +++ b/commcare_connect/opportunity/payment_number_report.py @@ -0,0 +1,106 @@ +from collections import defaultdict + +import django_filters +import django_tables2 as tables +from django.http import HttpResponse +from django.urls import reverse +from django.utils.html import format_html + +from commcare_connect.connect_id_client.main import fetch_payment_phone_numbers, validate_phone_numbers +from commcare_connect.opportunity.models import Opportunity, OpportunityAccess +from commcare_connect.reports.views import NonModelTableBaseView, SuperUserRequiredMixin + + +class PaymentNumberReportTable(tables.Table): + username = tables.Column(verbose_name="Username") + phone_number = tables.Column(verbose_name="Payment Phone Number") + status = tables.Column(verbose_name="Status") + + class Meta: + template_name = "opportunity/payment_numbers_table.html" + attrs = {"class": "table table-striped table-hover"} + empty_text = "No data available." + orderable = False + + def render_status(self, value, record): + options = ["pending", "approved", "rejected"] + username = record["username"] + phone_number = record["phone_number"] + + radio_buttons = [ + f' {option.capitalize()}' + for option in options + ] + radio_buttons.append(f'') + return format_html("
".join(radio_buttons)) + + +class PaymentFilters(django_filters.FilterSet): + STATUS_CHOICES = [ + ("pending", "Pending"), + ("approved", "Approved"), + ("rejected", "Rejected"), + ] + + opportunity = django_filters.ChoiceFilter(method="filter_by_ignore") + status = django_filters.ChoiceFilter(choices=STATUS_CHOICES, label="Payment Status") + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + opportunities = Opportunity.objects.values_list("id", "name") + self.filters["opportunity"] = django_filters.ChoiceFilter( + choices=opportunities, label="Opportunity", empty_label=None + ) + + class Meta: + model = None + fields = ["status"] + + def filter_by_ignore(self, queryset, name, value): + return queryset + + +class PaymentNumberReport(tables.SingleTableMixin, SuperUserRequiredMixin, NonModelTableBaseView): + table_class = PaymentNumberReportTable + filterset_class = PaymentFilters + htmx_template = "opportunity/payment_numbers_table.html" + report_title = "Verify Payment Phone Numbers" + + @property + def report_url(self): + return reverse("opportunity:payment_number_report", args=(self.request.org.slug,)) + + @property + def object_list(self): + if not self.filter_values: + return [] + + status = self.filter_values["status"] + opportunity = self.filter_values["opportunity"] + if not opportunity: + return [] + usernames = OpportunityAccess.objects.filter(opportunity_id=opportunity).values_list( + "user__username", flat=True + ) + return fetch_payment_phone_numbers(usernames, status) + + def post(self, request, *args, **kwargs): + user_statuses = defaultdict(dict) + + for key, value in request.POST.items(): + if key.startswith("status_"): + username = key.split("status_")[1] + user_statuses[username].update({"status": value}) + if key.startswith("phone_"): + username = key.split("phone_")[1] + user_statuses[username].update({"phone_number": value}) + + updates = [{"username": username, **values} for username, values in user_statuses.items()] + result = validate_phone_numbers(updates) + return HttpResponse( + format_html( + """{}""".format( + f"Approved: {result['approved']}, Rejected: {result['rejected']}, Pending: {result['pending']}" + ) + ) + ) diff --git a/commcare_connect/opportunity/urls.py b/commcare_connect/opportunity/urls.py index 66b7a957..8eec654c 100644 --- a/commcare_connect/opportunity/urls.py +++ b/commcare_connect/opportunity/urls.py @@ -1,6 +1,7 @@ from django.urls import path from commcare_connect.opportunity import views +from commcare_connect.opportunity.payment_number_report import PaymentNumberReport from commcare_connect.opportunity.views import ( OpportunityCompletedWorkTable, OpportunityCreate, @@ -112,4 +113,5 @@ path("/invoice_table/", views.PaymentInvoiceTableView.as_view(), name="invoice_table"), path("/invoice/create/", views.invoice_create, name="invoice_create"), path("/invoice/approve/", views.invoice_approve, name="invoice_approve"), + path("payment_numbers", view=PaymentNumberReport.as_view(), name="payment_number_report"), ] diff --git a/commcare_connect/reports/views.py b/commcare_connect/reports/views.py index 9cea2c50..5fdf219f 100644 --- a/commcare_connect/reports/views.py +++ b/commcare_connect/reports/views.py @@ -253,16 +253,47 @@ class Meta: unknown_field_behavior = django_filters.UnknownFieldBehavior.IGNORE -class NonModelFilterView(FilterView): +class NonModelTableBaseView(FilterView): + # Inherit this for a tabular report + # e.g. DeliveryStatsReportView + page_template = "reports/report_table.html" + htmx_table_template = "reports/htmx_table.html" + report_title = "Override this" + def get_queryset(self): # Doesn't matter which model it is here return CompletedWork.objects.none() + @cached_property + def filter_values(self): + if not self.filterset.form.is_valid(): + return None + else: + return self.filterset.form.cleaned_data + + def get_template_names(self): + if self.request.htmx: + return self.htmx_table_template + else: + return self.page_template + @property def object_list(self): # Override this return [] + @property + def report_url(self): + # Override this + # This report's url + return "" + + def get_context_data(self, *args, **kwargs): + context = super().get_context_data(**kwargs) + context["report_url"] = self.report_url + context["report_title"] = self.report_title + return context + def get(self, request, *args, **kwargs): filterset_class = self.get_filterset_class() self.filterset = self.get_filterset(filterset_class) @@ -270,29 +301,14 @@ def get(self, request, *args, **kwargs): return self.render_to_response(context) -class DeliveryStatsReportView(tables.SingleTableMixin, SuperUserRequiredMixin, NonModelFilterView): +class DeliveryStatsReportView(tables.SingleTableMixin, SuperUserRequiredMixin, NonModelTableBaseView): table_class = AdminReportTable filterset_class = DeliveryReportFilters + report_title = "Delivery Stats Report" - def get_template_names(self): - if self.request.htmx: - template_name = "reports/htmx_table.html" - else: - template_name = "reports/report_table.html" - - return template_name - - def get_context_data(self, *args, **kwargs): - context = super().get_context_data(**kwargs) - context["report_url"] = reverse("reports:delivery_stats_report") - return context - - @cached_property - def filter_values(self): - if not self.filterset.form.is_valid(): - return None - else: - return self.filterset.form.cleaned_data + @property + def report_url(self): + return reverse("reports:delivery_stats_report") @property def object_list(self): diff --git a/commcare_connect/templates/opportunity/payment_numbers_table.html b/commcare_connect/templates/opportunity/payment_numbers_table.html new file mode 100644 index 00000000..aed8fb0d --- /dev/null +++ b/commcare_connect/templates/opportunity/payment_numbers_table.html @@ -0,0 +1,16 @@ +{% extends "reports/htmx_table.html" %} + +{% load django_tables2 %} +{% load i18n %} + + +{% block table%} +
+ {% csrf_token %} + {{ block.super }} + + +
+{% endblock table %} diff --git a/commcare_connect/templates/reports/report_table.html b/commcare_connect/templates/reports/report_table.html index bb6fc331..2310e4ce 100644 --- a/commcare_connect/templates/reports/report_table.html +++ b/commcare_connect/templates/reports/report_table.html @@ -5,7 +5,7 @@ {% load crispy_forms_tags %} {% block content %} -

Delivery Stats Report

+

{{ report_title }}

Date: Sun, 10 Nov 2024 17:32:01 +0530 Subject: [PATCH 02/12] add payment-numbers flag to opp --- commcare_connect/opportunity/api/serializers.py | 1 + commcare_connect/opportunity/forms.py | 17 +++++++++++------ .../0061_opportunity_payment_info_required.py | 17 +++++++++++++++++ commcare_connect/opportunity/models.py | 2 ++ .../opportunity/payment_number_report.py | 6 ++++-- commcare_connect/reports/views.py | 1 - .../templates/reports/report_table.html | 1 - 7 files changed, 35 insertions(+), 10 deletions(-) create mode 100644 commcare_connect/opportunity/migrations/0061_opportunity_payment_info_required.py diff --git a/commcare_connect/opportunity/api/serializers.py b/commcare_connect/opportunity/api/serializers.py index bf558935..d7cc92dc 100644 --- a/commcare_connect/opportunity/api/serializers.py +++ b/commcare_connect/opportunity/api/serializers.py @@ -118,6 +118,7 @@ class Meta: "organization", "learn_app", "deliver_app", + "payment_info_required", "start_date", "end_date", "max_visits_per_user", diff --git a/commcare_connect/opportunity/forms.py b/commcare_connect/opportunity/forms.py index 3e472450..fa3cf8be 100644 --- a/commcare_connect/opportunity/forms.py +++ b/commcare_connect/opportunity/forms.py @@ -84,6 +84,7 @@ class Meta: "short_description", "is_test", "delivery_type", + "payment_info_required", ] def __init__(self, *args, **kwargs): @@ -98,6 +99,7 @@ def __init__(self, *args, **kwargs): Row(Field("description")), Row(Field("short_description")), Row(Field("currency")), + Row(Field("payment_info_required")), Row( Field("additional_users", wrapper_class="form-group col-md-6 mb-0"), Field("end_date", wrapper_class="form-group col-md-6 mb-0"), @@ -114,6 +116,9 @@ def __init__(self, *args, **kwargs): Submit("submit", "Submit"), ) + self.fields["payment_info_required"] = forms.BooleanField( + label="Require Phone Numbers from users for payments", required=False + ) self.fields["additional_users"] = forms.IntegerField( required=False, help_text="Adds budget for additional users." ) @@ -143,12 +148,7 @@ class OpportunityInitForm(forms.ModelForm): class Meta: model = Opportunity - fields = [ - "name", - "description", - "short_description", - "currency", - ] + fields = ["name", "description", "short_description", "currency", "payment_info_required"] def __init__(self, *args, **kwargs): self.domains = kwargs.pop("domains", []) @@ -176,6 +176,7 @@ def __init__(self, *args, **kwargs): data_loading_states=True, ), Row(Field("currency")), + Row(Field("payment_info_required")), Row(Field("api_key")), Submit("submit", "Submit"), ) @@ -214,6 +215,10 @@ def __init__(self, *args, **kwargs): self.fields["deliver_app"] = forms.Field( widget=forms.Select(choices=[(None, "Loading...")], attrs={"data-loading-disable": True}) ) + self.fields["payment_info_required"] = forms.BooleanField( + label="Require Phone Numbers from users for payments", required=False + ) + self.fields["api_key"] = forms.CharField(max_length=50) def clean(self): diff --git a/commcare_connect/opportunity/migrations/0061_opportunity_payment_info_required.py b/commcare_connect/opportunity/migrations/0061_opportunity_payment_info_required.py new file mode 100644 index 00000000..84f5255b --- /dev/null +++ b/commcare_connect/opportunity/migrations/0061_opportunity_payment_info_required.py @@ -0,0 +1,17 @@ +# Generated by Django 4.2.5 on 2024-11-10 11:28 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("opportunity", "0060_completedwork_payment_date"), + ] + + operations = [ + migrations.AddField( + model_name="opportunity", + name="payment_info_required", + field=models.BooleanField(default=False), + ), + ] diff --git a/commcare_connect/opportunity/models.py b/commcare_connect/opportunity/models.py index 17f68978..db2b2462 100644 --- a/commcare_connect/opportunity/models.py +++ b/commcare_connect/opportunity/models.py @@ -82,6 +82,8 @@ class Opportunity(BaseModel): # to be removed budget_per_visit = models.IntegerField(null=True) total_budget = models.IntegerField(null=True) + # Whether users payment phone numbers are required or not + payment_info_required = models.BooleanField(default=False) api_key = models.ForeignKey(HQApiKey, on_delete=models.DO_NOTHING, null=True) currency = models.CharField(max_length=3, null=True) auto_approve_visits = models.BooleanField(default=True) diff --git a/commcare_connect/opportunity/payment_number_report.py b/commcare_connect/opportunity/payment_number_report.py index bfe31702..b9d1ab04 100644 --- a/commcare_connect/opportunity/payment_number_report.py +++ b/commcare_connect/opportunity/payment_number_report.py @@ -47,7 +47,9 @@ class PaymentFilters(django_filters.FilterSet): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - opportunities = Opportunity.objects.values_list("id", "name") + opportunities = Opportunity.objects.filter( + organization=self.request.org, payment_info_required=True + ).values_list("id", "name") self.filters["opportunity"] = django_filters.ChoiceFilter( choices=opportunities, label="Opportunity", empty_label=None ) @@ -63,7 +65,7 @@ def filter_by_ignore(self, queryset, name, value): class PaymentNumberReport(tables.SingleTableMixin, SuperUserRequiredMixin, NonModelTableBaseView): table_class = PaymentNumberReportTable filterset_class = PaymentFilters - htmx_template = "opportunity/payment_numbers_table.html" + htmx_table_template = "opportunity/payment_numbers_table.html" report_title = "Verify Payment Phone Numbers" @property diff --git a/commcare_connect/reports/views.py b/commcare_connect/reports/views.py index 5fdf219f..9e1a3991 100644 --- a/commcare_connect/reports/views.py +++ b/commcare_connect/reports/views.py @@ -255,7 +255,6 @@ class Meta: class NonModelTableBaseView(FilterView): # Inherit this for a tabular report - # e.g. DeliveryStatsReportView page_template = "reports/report_table.html" htmx_table_template = "reports/htmx_table.html" report_title = "Override this" diff --git a/commcare_connect/templates/reports/report_table.html b/commcare_connect/templates/reports/report_table.html index 2310e4ce..42a9b483 100644 --- a/commcare_connect/templates/reports/report_table.html +++ b/commcare_connect/templates/reports/report_table.html @@ -16,7 +16,6 @@

{{ report_title }}

class="form-inline"> {% crispy filter.form %}
- {% render_table table %} {% endblock %} From 6290c5720a02b03ef72841f3251c159d3db23af4 Mon Sep 17 00:00:00 2001 From: Sravan Reddy Date: Thu, 14 Nov 2024 18:45:03 +0530 Subject: [PATCH 03/12] Let any org user review payment numbers --- commcare_connect/opportunity/payment_number_report.py | 5 +++-- commcare_connect/reports/views.py | 7 ++----- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/commcare_connect/opportunity/payment_number_report.py b/commcare_connect/opportunity/payment_number_report.py index b9d1ab04..bb1c870e 100644 --- a/commcare_connect/opportunity/payment_number_report.py +++ b/commcare_connect/opportunity/payment_number_report.py @@ -8,7 +8,8 @@ from commcare_connect.connect_id_client.main import fetch_payment_phone_numbers, validate_phone_numbers from commcare_connect.opportunity.models import Opportunity, OpportunityAccess -from commcare_connect.reports.views import NonModelTableBaseView, SuperUserRequiredMixin +from commcare_connect.opportunity.views import OrganizationUserMixin +from commcare_connect.reports.views import NonModelTableBaseView class PaymentNumberReportTable(tables.Table): @@ -62,7 +63,7 @@ def filter_by_ignore(self, queryset, name, value): return queryset -class PaymentNumberReport(tables.SingleTableMixin, SuperUserRequiredMixin, NonModelTableBaseView): +class PaymentNumberReport(tables.SingleTableMixin, OrganizationUserMixin, NonModelTableBaseView): table_class = PaymentNumberReportTable filterset_class = PaymentFilters htmx_table_template = "opportunity/payment_numbers_table.html" diff --git a/commcare_connect/reports/views.py b/commcare_connect/reports/views.py index 9e1a3991..017b45f4 100644 --- a/commcare_connect/reports/views.py +++ b/commcare_connect/reports/views.py @@ -278,14 +278,11 @@ def get_template_names(self): @property def object_list(self): - # Override this - return [] + raise NotImplementedError() @property def report_url(self): - # Override this - # This report's url - return "" + raise NotImplementedError() def get_context_data(self, *args, **kwargs): context = super().get_context_data(**kwargs) From 7117a754c6446414b2f4bb12e65e7e7d98160747 Mon Sep 17 00:00:00 2001 From: Sravan Reddy Date: Wed, 20 Nov 2024 15:15:30 +0530 Subject: [PATCH 04/12] use correct perm --- commcare_connect/opportunity/payment_number_report.py | 4 ++-- commcare_connect/reports/views.py | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/commcare_connect/opportunity/payment_number_report.py b/commcare_connect/opportunity/payment_number_report.py index bb1c870e..f23dc546 100644 --- a/commcare_connect/opportunity/payment_number_report.py +++ b/commcare_connect/opportunity/payment_number_report.py @@ -8,7 +8,7 @@ from commcare_connect.connect_id_client.main import fetch_payment_phone_numbers, validate_phone_numbers from commcare_connect.opportunity.models import Opportunity, OpportunityAccess -from commcare_connect.opportunity.views import OrganizationUserMixin +from commcare_connect.opportunity.views import OrganizationUserMemberRoleMixin from commcare_connect.reports.views import NonModelTableBaseView @@ -63,7 +63,7 @@ def filter_by_ignore(self, queryset, name, value): return queryset -class PaymentNumberReport(tables.SingleTableMixin, OrganizationUserMixin, NonModelTableBaseView): +class PaymentNumberReport(tables.SingleTableMixin, OrganizationUserMemberRoleMixin, NonModelTableBaseView): table_class = PaymentNumberReportTable filterset_class = PaymentFilters htmx_table_template = "opportunity/payment_numbers_table.html" diff --git a/commcare_connect/reports/views.py b/commcare_connect/reports/views.py index 017b45f4..ed60a988 100644 --- a/commcare_connect/reports/views.py +++ b/commcare_connect/reports/views.py @@ -257,7 +257,8 @@ class NonModelTableBaseView(FilterView): # Inherit this for a tabular report page_template = "reports/report_table.html" htmx_table_template = "reports/htmx_table.html" - report_title = "Override this" + # Override this + report_title = None def get_queryset(self): # Doesn't matter which model it is here From 56a253f99aecc939247c23e9720dff75eb4b5833 Mon Sep 17 00:00:00 2001 From: Sravan Reddy Date: Thu, 21 Nov 2024 13:44:59 +0530 Subject: [PATCH 05/12] validate post params --- .../opportunity/payment_number_report.py | 13 +++++++++++++ .../opportunity/payment_numbers_table.html | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/commcare_connect/opportunity/payment_number_report.py b/commcare_connect/opportunity/payment_number_report.py index f23dc546..bbe7e149 100644 --- a/commcare_connect/opportunity/payment_number_report.py +++ b/commcare_connect/opportunity/payment_number_report.py @@ -98,6 +98,19 @@ def post(self, request, *args, **kwargs): username = key.split("phone_")[1] user_statuses[username].update({"phone_number": value}) + # validate that usernames do belong to this opportunity + opportunity_id = request.GET.get("opportunity") + if not opportunity_id: + return HttpResponse("Opportunity must be specified", status=400) + opportunity = Opportunity.objects.get(pk=opportunity_id) + if opportunity.organization != request.org: + return HttpResponse("You can't specify this opportunity", status=400) + is_valid = OpportunityAccess.objects.filter( + opportunity_id=opportunity_id, user__username__in=user_statuses.keys() + ).count() == len(user_statuses) + if not is_valid: + return HttpResponse("Unknown usernames", status=400) + updates = [{"username": username, **values} for username, values in user_statuses.items()] result = validate_phone_numbers(updates) return HttpResponse( diff --git a/commcare_connect/templates/opportunity/payment_numbers_table.html b/commcare_connect/templates/opportunity/payment_numbers_table.html index aed8fb0d..a1b4a17c 100644 --- a/commcare_connect/templates/opportunity/payment_numbers_table.html +++ b/commcare_connect/templates/opportunity/payment_numbers_table.html @@ -5,7 +5,7 @@ {% block table%} -
{% csrf_token %} From 07b9c7003a4e57a5f6cf0a845eb1ae052fc9bf32 Mon Sep 17 00:00:00 2001 From: Sravan Reddy Date: Fri, 22 Nov 2024 15:34:02 +0530 Subject: [PATCH 06/12] skip passing phone-numbers usernames are enough --- commcare_connect/connect_id_client/main.py | 2 +- commcare_connect/opportunity/payment_number_report.py | 10 ++-------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/commcare_connect/connect_id_client/main.py b/commcare_connect/connect_id_client/main.py index c85b091d..ac773679 100644 --- a/commcare_connect/connect_id_client/main.py +++ b/commcare_connect/connect_id_client/main.py @@ -74,7 +74,7 @@ def fetch_payment_phone_numbers(usernames, status): return response.json()["found_payment_numbers"] -def validate_phone_numbers(update_data): +def validate_payment_profiles(update_data): response = _make_request(POST, "/users/validate_payment_phone_numbers", json={"updates": update_data}) return response.json()["result"] diff --git a/commcare_connect/opportunity/payment_number_report.py b/commcare_connect/opportunity/payment_number_report.py index bbe7e149..b55a96b9 100644 --- a/commcare_connect/opportunity/payment_number_report.py +++ b/commcare_connect/opportunity/payment_number_report.py @@ -6,7 +6,7 @@ from django.urls import reverse from django.utils.html import format_html -from commcare_connect.connect_id_client.main import fetch_payment_phone_numbers, validate_phone_numbers +from commcare_connect.connect_id_client.main import fetch_payment_phone_numbers, validate_payment_profiles from commcare_connect.opportunity.models import Opportunity, OpportunityAccess from commcare_connect.opportunity.views import OrganizationUserMemberRoleMixin from commcare_connect.reports.views import NonModelTableBaseView @@ -26,13 +26,11 @@ class Meta: def render_status(self, value, record): options = ["pending", "approved", "rejected"] username = record["username"] - phone_number = record["phone_number"] radio_buttons = [ f' {option.capitalize()}' for option in options ] - radio_buttons.append(f'') return format_html("
".join(radio_buttons)) @@ -89,14 +87,10 @@ def object_list(self): def post(self, request, *args, **kwargs): user_statuses = defaultdict(dict) - for key, value in request.POST.items(): if key.startswith("status_"): username = key.split("status_")[1] user_statuses[username].update({"status": value}) - if key.startswith("phone_"): - username = key.split("phone_")[1] - user_statuses[username].update({"phone_number": value}) # validate that usernames do belong to this opportunity opportunity_id = request.GET.get("opportunity") @@ -112,7 +106,7 @@ def post(self, request, *args, **kwargs): return HttpResponse("Unknown usernames", status=400) updates = [{"username": username, **values} for username, values in user_statuses.items()] - result = validate_phone_numbers(updates) + result = validate_payment_profiles(updates) return HttpResponse( format_html( """{}""".format( From 845d1a72de5d12fb666e12a90355268abde4edc0 Mon Sep 17 00:00:00 2001 From: Sravan Reddy Date: Wed, 11 Dec 2024 19:08:41 +0530 Subject: [PATCH 07/12] store org specific payment statuses --- commcare_connect/connect_id_client/main.py | 2 +- .../opportunity/payment_number_report.py | 110 +++++++++++++++++- .../0007_orguserpaymentnumberstatus.py | 41 +++++++ commcare_connect/organization/models.py | 37 ++++++ 4 files changed, 186 insertions(+), 4 deletions(-) create mode 100644 commcare_connect/organization/migrations/0007_orguserpaymentnumberstatus.py diff --git a/commcare_connect/connect_id_client/main.py b/commcare_connect/connect_id_client/main.py index ac773679..f9a058da 100644 --- a/commcare_connect/connect_id_client/main.py +++ b/commcare_connect/connect_id_client/main.py @@ -76,7 +76,7 @@ def fetch_payment_phone_numbers(usernames, status): def validate_payment_profiles(update_data): response = _make_request(POST, "/users/validate_payment_phone_numbers", json={"updates": update_data}) - return response.json()["result"] + return response def _make_request(method, path, params=None, json=None, timeout=5) -> Response: diff --git a/commcare_connect/opportunity/payment_number_report.py b/commcare_connect/opportunity/payment_number_report.py index b55a96b9..0b0437d6 100644 --- a/commcare_connect/opportunity/payment_number_report.py +++ b/commcare_connect/opportunity/payment_number_report.py @@ -1,15 +1,24 @@ +import dataclasses from collections import defaultdict import django_filters import django_tables2 as tables +from django.db import transaction from django.http import HttpResponse from django.urls import reverse from django.utils.html import format_html -from commcare_connect.connect_id_client.main import fetch_payment_phone_numbers, validate_payment_profiles +from commcare_connect.connect_id_client.main import ( + fetch_payment_phone_numbers, + send_message_bulk, + validate_payment_profiles, +) +from commcare_connect.connect_id_client.models import Message from commcare_connect.opportunity.models import Opportunity, OpportunityAccess from commcare_connect.opportunity.views import OrganizationUserMemberRoleMixin +from commcare_connect.organization.models import OrgUserPaymentNumberStatus from commcare_connect.reports.views import NonModelTableBaseView +from commcare_connect.users.models import User class PaymentNumberReportTable(tables.Table): @@ -83,7 +92,17 @@ def object_list(self): usernames = OpportunityAccess.objects.filter(opportunity_id=opportunity).values_list( "user__username", flat=True ) - return fetch_payment_phone_numbers(usernames, status) + 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 + ) + local_statuses_by_username = {status.username: status for status in local_statuses} + for status in connecteid_statuses: + local_status = local_statuses_by_username.get(status["username"]) + if local_status and local_status.phone_number == status["phone_number"]: + status["status"] = local_status.status + return connecteid_statuses def post(self, request, *args, **kwargs): user_statuses = defaultdict(dict) @@ -106,7 +125,7 @@ def post(self, request, *args, **kwargs): return HttpResponse("Unknown usernames", status=400) updates = [{"username": username, **values} for username, values in user_statuses.items()] - result = validate_payment_profiles(updates) + result = update_payment_number_statuses(updates, opportunity) return HttpResponse( format_html( """{}""".format( @@ -114,3 +133,88 @@ def post(self, request, *args, **kwargs): ) ) ) + + +def update_payment_number_statuses(update_data, opportunity): + """ + Updates payment number status in bulk + """ + + @dataclasses.dataclass + class PaymentStatus: + user: str + phone_number: str + current_status: str + new_status: str + other_org_statuses: set() + + user_obj_by_username = User.objects.filter(username__in=[u["username"] for u in update_data]) + update_by_username = { + u["username"]: PaymentStatus( + user=user_obj_by_username["username"], + phone_number=u["phone_number"], + new_status=u["status"], + ) + for u in update_data + } + + existing_statuses = OrgUserPaymentNumberStatus.objects.filter(user__username=update_by_username.keys()).all() + + # remove unchanged updates and gather current-status + # and status set by other orgs + for status in existing_statuses: + update = update_by_username[status.user.username] + if status.organization == opportunity.organization: + if update.phone_number == status.phone_number: + if update.new_status == status.status: + # No change in status, so remove it + update_by_username.pop(status.user.username) + else: + update.current_status = status.status + else: + # the status is for an updated number, so default to PENDING + update.current_status = OrgUserPaymentNumberStatus.PENDING + else: + if update.phone_number == status.phone_number: + update.other_org_statuses.add(status.status) + + with transaction.atomic(): + objs = [ + OrgUserPaymentNumberStatus( + organization=opportunity.org, + user=update_by_username[u["username"]].user, + phone_number=u.phone_number, + status=u.new_status, + ) + for u in update_by_username.values() + ] + # Bulk update/create + objs.bulk_create(objs, update_conflicts=True) + + # Process connect-id updates and push-notifications + connectid_updates = [] + rejected_usernames = [] + approved_usernames = [] + for update in update_by_username.values(): + 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 + connectid_updates.append(update) + else: + if update.new_status == OrgUserPaymentNumberStatus.REJECTED: + rejected_usernames.append("username") + else: + approved_usernames.append("username") + + if connectid_updates: + response = validate_payment_profiles(connectid_updates) + if response.status not in [200, 201]: + raise Exception("Error sending payment number status updates to ConnectID") + + rejected_msg = f"{opportunity.name} is unable to send payments to you" + send_message_bulk(Message(usernames=rejected_usernames, body=rejected_msg)) + + 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)} diff --git a/commcare_connect/organization/migrations/0007_orguserpaymentnumberstatus.py b/commcare_connect/organization/migrations/0007_orguserpaymentnumberstatus.py new file mode 100644 index 00000000..2c95f072 --- /dev/null +++ b/commcare_connect/organization/migrations/0007_orguserpaymentnumberstatus.py @@ -0,0 +1,41 @@ +# Generated by Django 4.2.5 on 2024-12-11 13:22 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ("organization", "0006_organization_program_manager"), + ] + + operations = [ + migrations.CreateModel( + name="OrgUserPaymentNumberStatus", + fields=[ + ("id", models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID")), + ("phone_number", models.CharField(max_length=15)), + ( + "status", + models.CharField( + choices=[("pending", "Pending"), ("approved", "Approved"), ("rejected", "Rejected")], + default="pending", + max_length=10, + ), + ), + ( + "organization", + models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to="organization.organization"), + ), + ( + "user", + models.ForeignKey(on_delete=django.db.models.deletion.DO_NOTHING, to=settings.AUTH_USER_MODEL), + ), + ], + options={ + "unique_together": {("user", "organization")}, + }, + ), + ] diff --git a/commcare_connect/organization/models.py b/commcare_connect/organization/models.py index bc013e03..64c629a0 100644 --- a/commcare_connect/organization/models.py +++ b/commcare_connect/organization/models.py @@ -61,3 +61,40 @@ class Meta: db_table = "organization_membership" unique_together = ("user", "organization") indexes = [models.Index(fields=["invite_id"])] + + +class OrgUserPaymentNumberStatus(models.Model): + """ + This model stores whether a payment phone number of a + Connect mobile user is working or not. + The same is stored on ConnectID side per user level + This model stores any overrides at org level. + """ + + PENDING = "pending" + APPROVED = "approved" + REJECTED = "rejected" + + STATUS_CHOICES = [ + (PENDING, "Pending"), + (APPROVED, "Approved"), + (REJECTED, "Rejected"), + ] + + organization = models.ForeignKey( + Organization, + on_delete=models.CASCADE, + ) + user = models.ForeignKey( + User, + on_delete=models.DO_NOTHING, + ) + phone_number = models.CharField(max_length=15) + status = models.CharField( + max_length=10, + choices=STATUS_CHOICES, + default=PENDING, + ) + + class Meta: + unique_together = ("user", "organization") From 1b1ef447e1f3ebcd93e1ee98d97b9de8ff0d35fc Mon Sep 17 00:00:00 2001 From: Sravan Reddy Date: Wed, 11 Dec 2024 19:10:56 +0530 Subject: [PATCH 08/12] revert 07b9c7003 --- commcare_connect/connect_id_client/main.py | 2 +- commcare_connect/opportunity/payment_number_report.py | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/commcare_connect/connect_id_client/main.py b/commcare_connect/connect_id_client/main.py index f9a058da..82b2d401 100644 --- a/commcare_connect/connect_id_client/main.py +++ b/commcare_connect/connect_id_client/main.py @@ -74,7 +74,7 @@ def fetch_payment_phone_numbers(usernames, status): return response.json()["found_payment_numbers"] -def validate_payment_profiles(update_data): +def update_payment_statuses(update_data): response = _make_request(POST, "/users/validate_payment_phone_numbers", json={"updates": update_data}) return response diff --git a/commcare_connect/opportunity/payment_number_report.py b/commcare_connect/opportunity/payment_number_report.py index 0b0437d6..44cb1c50 100644 --- a/commcare_connect/opportunity/payment_number_report.py +++ b/commcare_connect/opportunity/payment_number_report.py @@ -11,7 +11,7 @@ from commcare_connect.connect_id_client.main import ( fetch_payment_phone_numbers, send_message_bulk, - validate_payment_profiles, + update_payment_statuses, ) from commcare_connect.connect_id_client.models import Message from commcare_connect.opportunity.models import Opportunity, OpportunityAccess @@ -35,11 +35,13 @@ class Meta: def render_status(self, value, record): options = ["pending", "approved", "rejected"] username = record["username"] + phone_number = record["phone_number"] radio_buttons = [ f' {option.capitalize()}' for option in options ] + radio_buttons.append(f'') return format_html("
".join(radio_buttons)) @@ -106,10 +108,14 @@ def object_list(self): def post(self, request, *args, **kwargs): user_statuses = defaultdict(dict) + for key, value in request.POST.items(): if key.startswith("status_"): username = key.split("status_")[1] user_statuses[username].update({"status": value}) + if key.startswith("phone_"): + username = key.split("phone_")[1] + user_statuses[username].update({"phone_number": value}) # validate that usernames do belong to this opportunity opportunity_id = request.GET.get("opportunity") @@ -207,7 +213,7 @@ class PaymentStatus: approved_usernames.append("username") if connectid_updates: - response = validate_payment_profiles(connectid_updates) + response = update_payment_statuses(connectid_updates) if response.status not in [200, 201]: raise Exception("Error sending payment number status updates to ConnectID") From f00079187edc6605404e9c31fb592a1ef53ab30f Mon Sep 17 00:00:00 2001 From: Sravan Reddy Date: Wed, 11 Dec 2024 23:42:23 +0530 Subject: [PATCH 09/12] add test --- .../opportunity/payment_number_report.py | 37 ++++++---- .../opportunity/tests/test_views.py | 74 ++++++++++++++++++- 2 files changed, 94 insertions(+), 17 deletions(-) diff --git a/commcare_connect/opportunity/payment_number_report.py b/commcare_connect/opportunity/payment_number_report.py index 44cb1c50..25b80b14 100644 --- a/commcare_connect/opportunity/payment_number_report.py +++ b/commcare_connect/opportunity/payment_number_report.py @@ -141,23 +141,26 @@ def post(self, request, *args, **kwargs): ) +@dataclasses.dataclass +class PaymentStatus: + user: str + phone_number: str + new_status: str + current_status: str = None + other_org_statuses: set = dataclasses.field(default_factory=set) + + def update_payment_number_statuses(update_data, opportunity): """ Updates payment number status in bulk """ - @dataclasses.dataclass - class PaymentStatus: - user: str - phone_number: str - current_status: str - new_status: str - other_org_statuses: set() - - user_obj_by_username = User.objects.filter(username__in=[u["username"] for u in update_data]) + user_obj_by_username = { + user.username: user for user in User.objects.filter(username__in=[u["username"] for u in update_data]).all() + } update_by_username = { u["username"]: PaymentStatus( - user=user_obj_by_username["username"], + user=user_obj_by_username[u["username"]], phone_number=u["phone_number"], new_status=u["status"], ) @@ -188,7 +191,7 @@ class PaymentStatus: objs = [ OrgUserPaymentNumberStatus( organization=opportunity.org, - user=update_by_username[u["username"]].user, + user=u.user, phone_number=u.phone_number, status=u.new_status, ) @@ -205,7 +208,7 @@ class PaymentStatus: 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 - connectid_updates.append(update) + connectid_updates.append({"username": update.user.username, "status": update.new_status}) else: if update.new_status == OrgUserPaymentNumberStatus.REJECTED: rejected_usernames.append("username") @@ -217,10 +220,12 @@ class PaymentStatus: if response.status not in [200, 201]: raise Exception("Error sending payment number status updates to ConnectID") - rejected_msg = f"{opportunity.name} is unable to send payments to you" - send_message_bulk(Message(usernames=rejected_usernames, body=rejected_msg)) + if rejected_usernames: + rejected_msg = f"{opportunity.name} is unable to send payments to you" + send_message_bulk(Message(usernames=rejected_usernames, body=rejected_msg)) - approved_msg = f"{opportunity.name} is now able to send payments to you" - send_message_bulk(Message(usernames=approved_usernames, body=approved_msg)) + if approved_usernames: + 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)} diff --git a/commcare_connect/opportunity/tests/test_views.py b/commcare_connect/opportunity/tests/test_views.py index da418aae..f39c365b 100644 --- a/commcare_connect/opportunity/tests/test_views.py +++ b/commcare_connect/opportunity/tests/test_views.py @@ -1,14 +1,17 @@ +from unittest.mock import MagicMock, patch + import pytest from django.test import Client from django.urls import reverse from commcare_connect.opportunity.models import Opportunity, OpportunityAccess, OpportunityClaimLimit +from commcare_connect.opportunity.payment_number_report import update_payment_number_statuses from commcare_connect.opportunity.tests.factories import ( OpportunityClaimFactory, OpportunityClaimLimitFactory, PaymentUnitFactory, ) -from commcare_connect.organization.models import Organization +from commcare_connect.organization.models import Organization, OrgUserPaymentNumberStatus from commcare_connect.users.models import User @@ -38,3 +41,72 @@ def test_add_budget_existing_users( assert opportunity.total_budget == 205 assert opportunity.claimed_budget == 15 assert OpportunityClaimLimit.objects.get(pk=ocl.pk).max_visits == 15 + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "update_data, existing_statuses, expected_updates", + [ + # 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"}], + ), + ], +) +@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)) + + mock_update_payment_statuses.return_value = MagicMock(status=200, json=lambda: {"result": "success"}) + + organization = Organization(name="Test Organization") + + opportunity = MagicMock(name="Test Opportunity", organization=organization) + + # Call the function + update_payment_number_statuses(update_data, opportunity) + + # Assertions + if expected_updates: + mock_update_payment_statuses.assert_called_once_with(expected_updates) + else: + mock_update_payment_statuses.assert_not_called() From d0ab1764a7a7789bfa87d17959b65e32f9051626 Mon Sep 17 00:00:00 2001 From: Sravan Reddy Date: Sun, 15 Dec 2024 19:49:55 +0530 Subject: [PATCH 10/12] 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}") From 60e9fcb3255715c32352f60dfe112bd82e8be9fb Mon Sep 17 00:00:00 2001 From: Sravan Reddy Date: Sun, 15 Dec 2024 20:27:02 +0530 Subject: [PATCH 11/12] fix migration path --- ...uired.py => 0063_opportunity_payment_info_required.py} | 2 +- commcare_connect/opportunity/payment_number_report.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) rename commcare_connect/opportunity/migrations/{0061_opportunity_payment_info_required.py => 0063_opportunity_payment_info_required.py} (84%) diff --git a/commcare_connect/opportunity/migrations/0061_opportunity_payment_info_required.py b/commcare_connect/opportunity/migrations/0063_opportunity_payment_info_required.py similarity index 84% rename from commcare_connect/opportunity/migrations/0061_opportunity_payment_info_required.py rename to commcare_connect/opportunity/migrations/0063_opportunity_payment_info_required.py index 84f5255b..5cd2ab93 100644 --- a/commcare_connect/opportunity/migrations/0061_opportunity_payment_info_required.py +++ b/commcare_connect/opportunity/migrations/0063_opportunity_payment_info_required.py @@ -5,7 +5,7 @@ class Migration(migrations.Migration): dependencies = [ - ("opportunity", "0060_completedwork_payment_date"), + ("opportunity", "0062_opportunityaccess_invited_date"), ] operations = [ diff --git a/commcare_connect/opportunity/payment_number_report.py b/commcare_connect/opportunity/payment_number_report.py index 80d80766..6caf2c1f 100644 --- a/commcare_connect/opportunity/payment_number_report.py +++ b/commcare_connect/opportunity/payment_number_report.py @@ -94,17 +94,17 @@ def object_list(self): usernames = OpportunityAccess.objects.filter(opportunity_id=opportunity).values_list( "user__username", flat=True ) - connecteid_statuses = fetch_payment_phone_numbers(usernames, status) + connectid_statuses = fetch_payment_phone_numbers(usernames, status) # display local status when its overridden local_statuses = OrgUserPaymentNumberStatus.objects.filter( 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: + for status in connectid_statuses: local_status = local_statuses_by_username.get(status["username"]) if local_status and local_status.phone_number == status["phone_number"]: status["status"] = local_status.status - return connecteid_statuses + return connectid_statuses def post(self, request, *args, **kwargs): user_statuses = defaultdict(dict) @@ -228,7 +228,7 @@ def update_payment_number_statuses(update_data, opportunity): if connectid_updates: response = update_payment_statuses(connectid_updates) - if response.status not in [200, 201]: + if response.status_code not in [200, 201]: raise Exception("Error sending payment number status updates to ConnectID") if rejected_usernames: From 363b3e27ff4875792a4237dc6785c27cb7086db6 Mon Sep 17 00:00:00 2001 From: Sravan Reddy Date: Sun, 15 Dec 2024 21:28:49 +0530 Subject: [PATCH 12/12] change user language --- .../opportunity/payment_number_report.py | 15 +++++++-------- commcare_connect/opportunity/tests/test_views.py | 2 +- .../templates/opportunity/opportunity_detail.html | 9 +++++++++ 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/commcare_connect/opportunity/payment_number_report.py b/commcare_connect/opportunity/payment_number_report.py index 6caf2c1f..4b977134 100644 --- a/commcare_connect/opportunity/payment_number_report.py +++ b/commcare_connect/opportunity/payment_number_report.py @@ -33,13 +33,12 @@ class Meta: orderable = False def render_status(self, value, record): - options = ["pending", "approved", "rejected"] username = record["username"] phone_number = record["phone_number"] radio_buttons = [ - f' {option.capitalize()}' - for option in options + f' {label}' + for (filter_value, label) in PaymentFilters.STATUS_CHOICES ] radio_buttons.append(f'') return format_html("
".join(radio_buttons)) @@ -47,9 +46,9 @@ def render_status(self, value, record): class PaymentFilters(django_filters.FilterSet): STATUS_CHOICES = [ - ("pending", "Pending"), - ("approved", "Approved"), - ("rejected", "Rejected"), + ("pending", "Pending Review"), + ("approved", "Working"), + ("rejected", "Not working"), ] opportunity = django_filters.ChoiceFilter(method="filter_by_ignore") @@ -76,7 +75,7 @@ class PaymentNumberReport(tables.SingleTableMixin, OrganizationUserMemberRoleMix table_class = PaymentNumberReportTable filterset_class = PaymentFilters htmx_table_template = "opportunity/payment_numbers_table.html" - report_title = "Verify Payment Phone Numbers" + report_title = "Review Payment Phone Numbers" @property def report_url(self): @@ -99,7 +98,7 @@ def object_list(self): local_statuses = OrgUserPaymentNumberStatus.objects.filter( user__username__in=usernames, organization=self.request.org ) - local_statuses_by_username = {status.username: status for status in local_statuses} + local_statuses_by_username = {status.user.username: status for status in local_statuses} for status in connectid_statuses: local_status = local_statuses_by_username.get(status["username"]) if local_status and local_status.phone_number == status["phone_number"]: diff --git a/commcare_connect/opportunity/tests/test_views.py b/commcare_connect/opportunity/tests/test_views.py index c1010b7b..2b65e228 100644 --- a/commcare_connect/opportunity/tests/test_views.py +++ b/commcare_connect/opportunity/tests/test_views.py @@ -207,7 +207,7 @@ def test_update_payment_number_statuses(scenario, opportunity): ) 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) + mock_update_connectid.return_value = MagicMock(status_code=200) result = update_payment_number_statuses(scenario["update_data"], opportunity) if scenario["expect_connectid_update"]: diff --git a/commcare_connect/templates/opportunity/opportunity_detail.html b/commcare_connect/templates/opportunity/opportunity_detail.html index b1aef96d..4813c420 100644 --- a/commcare_connect/templates/opportunity/opportunity_detail.html +++ b/commcare_connect/templates/opportunity/opportunity_detail.html @@ -114,6 +114,15 @@

{{ object.name }}

+ {% if opportunity.payment_info_required %} +
  • + + + {% translate "Review Payment Phone Numbers" %} + +
  • + {% endif %}