From b9a3f8ddba871b0a412d4dde2c32a50e00961348 Mon Sep 17 00:00:00 2001 From: Pawan Verma Date: Tue, 8 Oct 2024 16:48:09 +0530 Subject: [PATCH 01/13] Fix load internationalization tags on invoice list page --- commcare_connect/templates/opportunity/invoice_list.html | 1 + 1 file changed, 1 insertion(+) diff --git a/commcare_connect/templates/opportunity/invoice_list.html b/commcare_connect/templates/opportunity/invoice_list.html index f3294e0e..aed1abfd 100644 --- a/commcare_connect/templates/opportunity/invoice_list.html +++ b/commcare_connect/templates/opportunity/invoice_list.html @@ -2,6 +2,7 @@ {% load django_tables2 %} {% load static %} {% load crispy_forms_tags %} +{% load i18n %} {% block title %}{{ request.org }} - Invoices{% endblock title %} From 4d48572d260785fbc517e51eebc1260f024016e5 Mon Sep 17 00:00:00 2001 From: Pawan Verma Date: Tue, 8 Oct 2024 16:55:07 +0530 Subject: [PATCH 02/13] Fix format as string --- commcare_connect/templates/opportunity/invoice_list.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commcare_connect/templates/opportunity/invoice_list.html b/commcare_connect/templates/opportunity/invoice_list.html index aed1abfd..c9612f62 100644 --- a/commcare_connect/templates/opportunity/invoice_list.html +++ b/commcare_connect/templates/opportunity/invoice_list.html @@ -21,7 +21,7 @@

Invoices

{% if not request.org_membership.is_program_manager %} {% endif %} From cd63708c8cdd1df11a182462eda9fb930e5e9ff1 Mon Sep 17 00:00:00 2001 From: Cal Ellowitz Date: Wed, 9 Oct 2024 04:21:20 -0400 Subject: [PATCH 03/13] Handle null currency --- commcare_connect/opportunity/visit_import.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/commcare_connect/opportunity/visit_import.py b/commcare_connect/opportunity/visit_import.py index c3926feb..e57cbd57 100644 --- a/commcare_connect/opportunity/visit_import.py +++ b/commcare_connect/opportunity/visit_import.py @@ -285,7 +285,9 @@ def _cache_key(currency_code, date=None): def get_exchange_rate(currency_code, date=None): # date should be a date object or None for latest rate - if currency_code in ["USD", None]: + if currency_code is None: + raise ImportException("Opportunity must have specified currency to import payments") + if currency_code == "USD": return 1 base_url = "https://openexchangerates.org/api" From dc05b374c25be5e34633ff9f3041f76c5976867c Mon Sep 17 00:00:00 2001 From: Cal Ellowitz Date: Wed, 9 Oct 2024 04:50:40 -0400 Subject: [PATCH 04/13] add default currency for OpportunityFactory --- commcare_connect/opportunity/tests/factories.py | 1 + 1 file changed, 1 insertion(+) diff --git a/commcare_connect/opportunity/tests/factories.py b/commcare_connect/opportunity/tests/factories.py index d69b3ffb..daa64837 100644 --- a/commcare_connect/opportunity/tests/factories.py +++ b/commcare_connect/opportunity/tests/factories.py @@ -56,6 +56,7 @@ class OpportunityFactory(DjangoModelFactory): total_budget = Faker("pyint", min_value=1000, max_value=10000) api_key = SubFactory(HQApiKeyFactory) delivery_type = SubFactory(DeliveryTypeFactory) + currency = "USD" class Meta: model = "opportunity.Opportunity" From e1b9236074ea39da6ef9a642664dc177099819f8 Mon Sep 17 00:00:00 2001 From: Cory Zue Date: Wed, 9 Oct 2024 11:30:20 +0200 Subject: [PATCH 05/13] fix for null/missing/invalid lat/lon --- commcare_connect/reports/views.py | 35 +++++++++++++++++++------------ 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/commcare_connect/reports/views.py b/commcare_connect/reports/views.py index 601f0415..9cea2c50 100644 --- a/commcare_connect/reports/views.py +++ b/commcare_connect/reports/views.py @@ -192,19 +192,28 @@ def _results_to_geojson(results): "rejected": "#FF0000", } for result in results: - feature = { - "type": "Feature", - "geometry": { - "type": "Point", - "coordinates": [float(result["gps_location_long"]), float(result["gps_location_lat"])], - }, - "properties": { - key: value for key, value in result.items() if key not in ["gps_location_lat", "gps_location_long"] - }, - } - color = status_to_color.get(result["status"], "#FFFF00") - feature["properties"]["color"] = color - geojson["features"].append(feature) + # Check if both latitude and longitude are not None and can be converted to float + if result.get("gps_location_long") and result.get("gps_location_lat"): + try: + longitude = float(result["gps_location_long"]) + latitude = float(result["gps_location_lat"]) + except ValueError: + # Skip this result if conversion to float fails + continue + + feature = { + "type": "Feature", + "geometry": { + "type": "Point", + "coordinates": [longitude, latitude], + }, + "properties": { + key: value for key, value in result.items() if key not in ["gps_location_lat", "gps_location_long"] + }, + } + color = status_to_color.get(result.get("status", ""), "#FFFF00") + feature["properties"]["color"] = color + geojson["features"].append(feature) return geojson From 8cbe507d7332c744269b405782f45e53514827bb Mon Sep 17 00:00:00 2001 From: Cory Zue Date: Wed, 9 Oct 2024 11:30:28 +0200 Subject: [PATCH 06/13] add test --- .../reports/tests/test_reports.py | 46 ++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/commcare_connect/reports/tests/test_reports.py b/commcare_connect/reports/tests/test_reports.py index 319d4100..632a43ff 100644 --- a/commcare_connect/reports/tests/test_reports.py +++ b/commcare_connect/reports/tests/test_reports.py @@ -13,7 +13,7 @@ PaymentUnitFactory, UserVisitFactory, ) -from commcare_connect.reports.views import _get_table_data_for_quarter +from commcare_connect.reports.views import _get_table_data_for_quarter, _results_to_geojson @pytest.mark.django_db @@ -62,3 +62,47 @@ def test_delivery_stats(opportunity: Opportunity): assert unknown_delivery_type_data[0]["users"] == 0 assert unknown_delivery_type_data[0]["services"] == 0 assert unknown_delivery_type_data[0]["beneficiaries"] == 0 + + +def test_results_to_geojson(): + # Test input + results = [ + {"gps_location_long": "10.123", "gps_location_lat": "20.456", "status": "approved", "other_field": "value1"}, + {"gps_location_long": "30.789", "gps_location_lat": "40.012", "status": "rejected", "other_field": "value2"}, + {"gps_location_long": "invalid", "gps_location_lat": "50.678", "status": "unknown", "other_field": "value3"}, + {"status": "approved", "other_field": "value4"}, # Case where lat/lon are not present + { # Case where lat/lon are null + "gps_location_long": None, + "gps_location_lat": None, + "status": "rejected", + "other_field": "value5", + }, + ] + + # Call the function + geojson = _results_to_geojson(results) + + # Assertions + assert geojson["type"] == "FeatureCollection" + assert len(geojson["features"]) == 2 # Only the first two results should be included + + # Check the first feature + feature1 = geojson["features"][0] + assert feature1["type"] == "Feature" + assert feature1["geometry"]["type"] == "Point" + assert feature1["geometry"]["coordinates"] == [10.123, 20.456] + assert feature1["properties"]["status"] == "approved" + assert feature1["properties"]["other_field"] == "value1" + assert feature1["properties"]["color"] == "#00FF00" + + # Check the second feature + feature2 = geojson["features"][1] + assert feature2["type"] == "Feature" + assert feature2["geometry"]["type"] == "Point" + assert feature2["geometry"]["coordinates"] == [30.789, 40.012] + assert feature2["properties"]["status"] == "rejected" + assert feature2["properties"]["other_field"] == "value2" + assert feature2["properties"]["color"] == "#FF0000" + + # Check that the other cases are not included + assert all(f["properties"]["other_field"] not in ["value3", "value4", "value5"] for f in geojson["features"]) From 43e9109a715ee4ebaf7187a9e768f39e98e4fc62 Mon Sep 17 00:00:00 2001 From: Cal Ellowitz Date: Wed, 9 Oct 2024 15:18:35 +0200 Subject: [PATCH 07/13] order export rows --- commcare_connect/opportunity/export.py | 1 + 1 file changed, 1 insertion(+) diff --git a/commcare_connect/opportunity/export.py b/commcare_connect/opportunity/export.py index 3e1cece8..953ae53c 100644 --- a/commcare_connect/opportunity/export.py +++ b/commcare_connect/opportunity/export.py @@ -36,6 +36,7 @@ def export_user_visit_data( user_visits = user_visits.filter(visit_date__gte=date_range.get_cutoff_date()) if status and "all" not in status: user_visits = user_visits.filter(status__in=status) + user_visits = user_visits.order_by("visit_date") table = UserVisitTable(user_visits) exclude_columns = ("visit_date", "form_json", "details", "justification") From dca64e5e07695145a66d87ec4d866178f9b157c1 Mon Sep 17 00:00:00 2001 From: Sravan Reddy Date: Wed, 9 Oct 2024 22:17:22 +0530 Subject: [PATCH 08/13] include submission time slot in opportunity API --- .../tests/test_receiver_integration.py | 15 +++------------ .../opportunity/api/serializers.py | 19 +++++++++++++++++++ .../opportunity/tests/factories.py | 4 +++- .../opportunity/tests/test_api_views.py | 4 ++++ 4 files changed, 29 insertions(+), 13 deletions(-) diff --git a/commcare_connect/form_receiver/tests/test_receiver_integration.py b/commcare_connect/form_receiver/tests/test_receiver_integration.py index c52af2a5..908fa0df 100644 --- a/commcare_connect/form_receiver/tests/test_receiver_integration.py +++ b/commcare_connect/form_receiver/tests/test_receiver_integration.py @@ -464,12 +464,9 @@ def test_reciever_verification_flags_form_submission( user_with_connectid_link: User, api_client: APIClient, opportunity: Opportunity ): verification_flags = OpportunityVerificationFlags.objects.get(opportunity=opportunity) - verification_flags.form_submission_start = datetime.time(hour=10, minute=0) - verification_flags.form_submission_end = datetime.time(hour=12, minute=0) - verification_flags.save() form_json = _create_opp_and_form_json(opportunity, user=user_with_connectid_link) - time = datetime.datetime(2024, 4, 17, 10, 0, 0) + time = datetime.datetime(2024, 4, 17, verification_flags.form_submission_start.hour, 0, 0) form_json["metadata"]["timeStart"] = time form_json["metadata"]["timeEnd"] = time + datetime.timedelta(minutes=10) make_request(api_client, form_json, user_with_connectid_link) @@ -481,12 +478,9 @@ def test_reciever_verification_flags_form_submission_start( user_with_connectid_link: User, api_client: APIClient, opportunity: Opportunity ): verification_flags = OpportunityVerificationFlags.objects.get(opportunity=opportunity) - verification_flags.form_submission_start = datetime.time(hour=10, minute=0) - verification_flags.form_submission_end = datetime.time(hour=12, minute=0) - verification_flags.save() form_json = _create_opp_and_form_json(opportunity, user=user_with_connectid_link) - time = datetime.datetime(2024, 4, 17, 9, 0, 0) + time = datetime.datetime(2024, 4, 17, verification_flags.form_submission_start.hour - 1, 0, 0) form_json["metadata"]["timeStart"] = time make_request(api_client, form_json, user_with_connectid_link) visit = UserVisit.objects.get(user=user_with_connectid_link) @@ -498,12 +492,9 @@ def test_reciever_verification_flags_form_submission_end( user_with_connectid_link: User, api_client: APIClient, opportunity: Opportunity ): verification_flags = OpportunityVerificationFlags.objects.get(opportunity=opportunity) - verification_flags.form_submission_start = datetime.time(hour=10, minute=0) - verification_flags.form_submission_end = datetime.time(hour=12, minute=0) - verification_flags.save() form_json = _create_opp_and_form_json(opportunity, user=user_with_connectid_link) - time = datetime.datetime(2024, 4, 17, 13, 0, 0) + time = datetime.datetime(2024, 4, 17, verification_flags.form_submission_end.hour + 1, 0, 0) form_json["metadata"]["timeStart"] = time make_request(api_client, form_json, user_with_connectid_link) visit = UserVisit.objects.get(user=user_with_connectid_link) diff --git a/commcare_connect/opportunity/api/serializers.py b/commcare_connect/opportunity/api/serializers.py index bf558935..cbeafe7d 100644 --- a/commcare_connect/opportunity/api/serializers.py +++ b/commcare_connect/opportunity/api/serializers.py @@ -1,3 +1,5 @@ +from functools import lru_cache + from django.conf import settings from django.db.models import Sum from rest_framework import serializers @@ -15,6 +17,7 @@ OpportunityAccess, OpportunityClaim, OpportunityClaimLimit, + OpportunityVerificationFlags, Payment, PaymentUnit, UserVisit, @@ -105,6 +108,8 @@ class OpportunitySerializer(serializers.ModelSerializer): payment_units = serializers.SerializerMethodField() is_user_suspended = serializers.SerializerMethodField() catchment_areas = serializers.SerializerMethodField() + start_time_threshold = serializers.SerializerMethodField() + end_time_threshold = serializers.SerializerMethodField() class Meta: model = Opportunity @@ -133,6 +138,8 @@ class Meta: "payment_units", "is_user_suspended", "catchment_areas", + "start_time_threshold", + "end_time_threshold", ] def get_claim(self, obj): @@ -178,6 +185,18 @@ def get_catchment_areas(self, obj): catchments = CatchmentArea.objects.filter(opportunity_access=opp_access) return CatchmentAreaSerializer(catchments, many=True).data + @lru_cache + def _get_flags(self, obj): + return OpportunityVerificationFlags.objects.filter(opportunity=obj).first() + + def get_start_time_threshold(self, obj): + flags = self._get_flags(obj) + return flags.form_submission_start + + def get_end_time_threshold(self, obj): + flags = self._get_flags(obj) + return flags.form_submission_end + @quickcache(vary_on=["user.pk", "opportunity.pk"], timeout=60 * 60) def _get_opp_access(user, opportunity): diff --git a/commcare_connect/opportunity/tests/factories.py b/commcare_connect/opportunity/tests/factories.py index daa64837..1d7bf83a 100644 --- a/commcare_connect/opportunity/tests/factories.py +++ b/commcare_connect/opportunity/tests/factories.py @@ -1,4 +1,4 @@ -from datetime import timezone +from datetime import time, timezone from factory import DictFactory, Faker, LazyAttribute, SelfAttribute, SubFactory from factory.django import DjangoModelFactory @@ -72,6 +72,8 @@ class Meta: class OpportunityVerificationFlagsFactory(DjangoModelFactory): opportunity = SubFactory(OpportunityFactory) + form_submission_start = time(10, 0, 0) + form_submission_end = time(12, 0, 0) class Meta: model = "opportunity.OpportunityVerificationFlags" diff --git a/commcare_connect/opportunity/tests/test_api_views.py b/commcare_connect/opportunity/tests/test_api_views.py index e7d26a88..c574c934 100644 --- a/commcare_connect/opportunity/tests/test_api_views.py +++ b/commcare_connect/opportunity/tests/test_api_views.py @@ -15,6 +15,7 @@ OpportunityAccess, OpportunityClaim, OpportunityClaimLimit, + OpportunityVerificationFlags, Payment, VisitValidationStatus, ) @@ -160,6 +161,9 @@ def test_opportunity_list_endpoint( assert response.data[0]["budget_per_visit"] == max([pu.amount for pu in payment_units]) claim_limits = OpportunityClaimLimit.objects.filter(opportunity_claim__opportunity_access__opportunity=opportunity) assert response.data[0]["claim"]["max_payments"] == sum([cl.max_visits for cl in claim_limits]) + verification_flags = OpportunityVerificationFlags.objects.get(opportunity=opportunity) + assert response.data[0]["start_time_threshold"] == verification_flags.form_submission_start + assert response.data[0]["end_time_threshold"] == verification_flags.form_submission_end def test_delivery_progress_endpoint( From 55ab9fc7090b588fcf2c01046c902975df6377c8 Mon Sep 17 00:00:00 2001 From: hemant10yadav Date: Wed, 16 Oct 2024 08:29:58 +0530 Subject: [PATCH 09/13] refactor old migration to load historical model --- .../0060_completedwork_payment_date.py | 6 ++++-- .../opportunity/utils/completed_work.py | 19 +++++++++++++++---- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/commcare_connect/opportunity/migrations/0060_completedwork_payment_date.py b/commcare_connect/opportunity/migrations/0060_completedwork_payment_date.py index d8c8622a..c8481f41 100644 --- a/commcare_connect/opportunity/migrations/0060_completedwork_payment_date.py +++ b/commcare_connect/opportunity/migrations/0060_completedwork_payment_date.py @@ -2,15 +2,17 @@ from django.db import migrations, models, transaction -from commcare_connect.opportunity.models import OpportunityAccess from commcare_connect.opportunity.utils.completed_work import update_work_payment_date @transaction.atomic def update_paid_date_from_payments(apps, schema_editor): + OpportunityAccess = apps.get_model("opportunity.OpportunityAccess") + Payment = apps.get_model("opportunity.Payment") + CompletedWork = apps.get_model("opportunity.CompletedWork") accesses = OpportunityAccess.objects.all() for access in accesses: - update_work_payment_date(access) + update_work_payment_date(access, Payment, CompletedWork) class Migration(migrations.Migration): diff --git a/commcare_connect/opportunity/utils/completed_work.py b/commcare_connect/opportunity/utils/completed_work.py index df2d9906..fa0fb459 100644 --- a/commcare_connect/opportunity/utils/completed_work.py +++ b/commcare_connect/opportunity/utils/completed_work.py @@ -44,9 +44,20 @@ def update_status(completed_works, opportunity_access, compute_payment=True): opportunity_access.save() -def update_work_payment_date(access: OpportunityAccess): - payments = Payment.objects.filter(opportunity_access=access).order_by("date_paid") - completed_works = CompletedWork.objects.filter(opportunity_access=access).order_by("status_modified_date") +def update_work_payment_date(access: OpportunityAccess, payment_model_ref=None, completed_work_model_ref=None): + """ + Import models dynamically within the function helps us avoid issues with historical models during migrations. + """ + if not payment_model_ref: + payment_model_ref = Payment + + if not completed_work_model_ref: + completed_work_model_ref = CompletedWork + + payments = payment_model_ref.objects.filter(opportunity_access=access).order_by("date_paid") + completed_works = completed_work_model_ref.objects.filter(opportunity_access=access).order_by( + "status_modified_date" + ) if not payments or not completed_works: return @@ -76,4 +87,4 @@ def update_work_payment_date(access: OpportunityAccess): break if works_to_update: - CompletedWork.objects.bulk_update(works_to_update, ["payment_date"]) + completed_work_model_ref.objects.bulk_update(works_to_update, ["payment_date"]) From 979298c62f0a02365acb87326ee9c169f4cc7e3c Mon Sep 17 00:00:00 2001 From: hemant10yadav Date: Wed, 16 Oct 2024 08:33:40 +0530 Subject: [PATCH 10/13] updated the comment --- commcare_connect/opportunity/utils/completed_work.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/commcare_connect/opportunity/utils/completed_work.py b/commcare_connect/opportunity/utils/completed_work.py index fa0fb459..e8d8c431 100644 --- a/commcare_connect/opportunity/utils/completed_work.py +++ b/commcare_connect/opportunity/utils/completed_work.py @@ -46,7 +46,9 @@ def update_status(completed_works, opportunity_access, compute_payment=True): def update_work_payment_date(access: OpportunityAccess, payment_model_ref=None, completed_work_model_ref=None): """ - Import models dynamically within the function helps us avoid issues with historical models during migrations. + Dynamically assign models to avoid issues with historical models during migrations. + Top-level imports use the current model, which may not match the schema at migration + time. This ensures we use historical models during migrations and current models in normal execution. """ if not payment_model_ref: payment_model_ref = Payment From 61001288cac96be8c0db1ac22d587ae120bfcab7 Mon Sep 17 00:00:00 2001 From: hemant10yadav Date: Wed, 16 Oct 2024 12:21:47 +0530 Subject: [PATCH 11/13] renamed variables --- commcare_connect/opportunity/utils/completed_work.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/commcare_connect/opportunity/utils/completed_work.py b/commcare_connect/opportunity/utils/completed_work.py index e8d8c431..2050e30b 100644 --- a/commcare_connect/opportunity/utils/completed_work.py +++ b/commcare_connect/opportunity/utils/completed_work.py @@ -44,17 +44,14 @@ def update_status(completed_works, opportunity_access, compute_payment=True): opportunity_access.save() -def update_work_payment_date(access: OpportunityAccess, payment_model_ref=None, completed_work_model_ref=None): +def update_work_payment_date(access: OpportunityAccess, payment_model=None, completed_work_model=None): """ Dynamically assign models to avoid issues with historical models during migrations. Top-level imports use the current model, which may not match the schema at migration time. This ensures we use historical models during migrations and current models in normal execution. """ - if not payment_model_ref: - payment_model_ref = Payment - - if not completed_work_model_ref: - completed_work_model_ref = CompletedWork + payment_model_ref = payment_model or Payment + completed_work_model_ref = completed_work_model or CompletedWork payments = payment_model_ref.objects.filter(opportunity_access=access).order_by("date_paid") completed_works = completed_work_model_ref.objects.filter(opportunity_access=access).order_by( From 97c2f2515a09e5a19c368f6e39baab56fa90d47a Mon Sep 17 00:00:00 2001 From: Sravan Reddy Date: Thu, 17 Oct 2024 22:58:00 +0530 Subject: [PATCH 12/13] move to nested fields --- .../opportunity/api/serializers.py | 26 ++++++------------- .../opportunity/tests/test_api_views.py | 9 ++++--- 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/commcare_connect/opportunity/api/serializers.py b/commcare_connect/opportunity/api/serializers.py index cbeafe7d..d293d5ab 100644 --- a/commcare_connect/opportunity/api/serializers.py +++ b/commcare_connect/opportunity/api/serializers.py @@ -1,5 +1,3 @@ -from functools import lru_cache - from django.conf import settings from django.db.models import Sum from rest_framework import serializers @@ -94,6 +92,12 @@ class Meta: fields = ["id", "name", "latitude", "longitude", "radius", "active"] +class OpportunityVerificationFlagsSerializer(serializers.ModelSerializer): + class Meta: + model = OpportunityVerificationFlags + fields = ["form_submission_start", "form_submission_end"] + + class OpportunitySerializer(serializers.ModelSerializer): organization = serializers.SlugRelatedField(read_only=True, slug_field="slug") learn_app = CommCareAppSerializer() @@ -108,8 +112,7 @@ class OpportunitySerializer(serializers.ModelSerializer): payment_units = serializers.SerializerMethodField() is_user_suspended = serializers.SerializerMethodField() catchment_areas = serializers.SerializerMethodField() - start_time_threshold = serializers.SerializerMethodField() - end_time_threshold = serializers.SerializerMethodField() + verification_flags = OpportunityVerificationFlagsSerializer(source="opportunityverificationflags", read_only=True) class Meta: model = Opportunity @@ -138,8 +141,7 @@ class Meta: "payment_units", "is_user_suspended", "catchment_areas", - "start_time_threshold", - "end_time_threshold", + "verification_flags", ] def get_claim(self, obj): @@ -185,18 +187,6 @@ def get_catchment_areas(self, obj): catchments = CatchmentArea.objects.filter(opportunity_access=opp_access) return CatchmentAreaSerializer(catchments, many=True).data - @lru_cache - def _get_flags(self, obj): - return OpportunityVerificationFlags.objects.filter(opportunity=obj).first() - - def get_start_time_threshold(self, obj): - flags = self._get_flags(obj) - return flags.form_submission_start - - def get_end_time_threshold(self, obj): - flags = self._get_flags(obj) - return flags.form_submission_end - @quickcache(vary_on=["user.pk", "opportunity.pk"], timeout=60 * 60) def _get_opp_access(user, opportunity): diff --git a/commcare_connect/opportunity/tests/test_api_views.py b/commcare_connect/opportunity/tests/test_api_views.py index c574c934..9afb6c37 100644 --- a/commcare_connect/opportunity/tests/test_api_views.py +++ b/commcare_connect/opportunity/tests/test_api_views.py @@ -15,7 +15,6 @@ OpportunityAccess, OpportunityClaim, OpportunityClaimLimit, - OpportunityVerificationFlags, Payment, VisitValidationStatus, ) @@ -161,9 +160,11 @@ def test_opportunity_list_endpoint( assert response.data[0]["budget_per_visit"] == max([pu.amount for pu in payment_units]) claim_limits = OpportunityClaimLimit.objects.filter(opportunity_claim__opportunity_access__opportunity=opportunity) assert response.data[0]["claim"]["max_payments"] == sum([cl.max_visits for cl in claim_limits]) - verification_flags = OpportunityVerificationFlags.objects.get(opportunity=opportunity) - assert response.data[0]["start_time_threshold"] == verification_flags.form_submission_start - assert response.data[0]["end_time_threshold"] == verification_flags.form_submission_end + verification_flags = opportunity.opportunityverificationflags + assert response.data[0]["verification_flags"]["form_submission_start"] == str( + verification_flags.form_submission_start + ) + assert response.data[0]["verification_flags"]["form_submission_end"] == str(verification_flags.form_submission_end) def test_delivery_progress_endpoint( From 05b8f2f58977c4877ce6c755e0ebb922fe9b562e Mon Sep 17 00:00:00 2001 From: Sravan Reddy Date: Mon, 21 Oct 2024 11:38:12 +0530 Subject: [PATCH 13/13] refactor tests parametrize --- commcare_connect/conftest.py | 5 +- .../tests/test_receiver_integration.py | 67 ++++++++++--------- .../opportunity/tests/factories.py | 6 +- .../opportunity/tests/test_api_views.py | 16 ++++- 4 files changed, 55 insertions(+), 39 deletions(-) diff --git a/commcare_connect/conftest.py b/commcare_connect/conftest.py index 8c91a1df..1cbb6ff4 100644 --- a/commcare_connect/conftest.py +++ b/commcare_connect/conftest.py @@ -48,9 +48,10 @@ def user(db) -> User: @pytest.fixture() -def opportunity(): +def opportunity(request): + verification_flags = getattr(request, "param", {}).get("verification_flags", {}) factory = OpportunityFactory(is_test=False) - OpportunityVerificationFlagsFactory(opportunity=factory) + OpportunityVerificationFlagsFactory(opportunity=factory, **verification_flags) return factory diff --git a/commcare_connect/form_receiver/tests/test_receiver_integration.py b/commcare_connect/form_receiver/tests/test_receiver_integration.py index 908fa0df..731ef697 100644 --- a/commcare_connect/form_receiver/tests/test_receiver_integration.py +++ b/commcare_connect/form_receiver/tests/test_receiver_integration.py @@ -460,46 +460,47 @@ def test_auto_approve_visits_and_payments( assert access.payment_accrued == completed_work.payment_accrued +@pytest.mark.parametrize( + "opportunity", + [ + { + "verification_flags": { + "form_submission_start": datetime.time(10, 0), + "form_submission_end": datetime.time(14, 0), + } + } + ], + indirect=True, +) +@pytest.mark.parametrize( + "submission_time_hour, expected_message", + [ + (11, None), + (9, "Form was submitted before the start time"), + (15, "Form was submitted after the end time"), + ], +) def test_reciever_verification_flags_form_submission( - user_with_connectid_link: User, api_client: APIClient, opportunity: Opportunity + user_with_connectid_link: User, + api_client: APIClient, + opportunity: Opportunity, + submission_time_hour, + expected_message, ): - verification_flags = OpportunityVerificationFlags.objects.get(opportunity=opportunity) - form_json = _create_opp_and_form_json(opportunity, user=user_with_connectid_link) - time = datetime.datetime(2024, 4, 17, verification_flags.form_submission_start.hour, 0, 0) - form_json["metadata"]["timeStart"] = time - form_json["metadata"]["timeEnd"] = time + datetime.timedelta(minutes=10) - make_request(api_client, form_json, user_with_connectid_link) - visit = UserVisit.objects.get(user=user_with_connectid_link) - assert not visit.flagged - - -def test_reciever_verification_flags_form_submission_start( - user_with_connectid_link: User, api_client: APIClient, opportunity: Opportunity -): - verification_flags = OpportunityVerificationFlags.objects.get(opportunity=opportunity) + submission_time = datetime.datetime(2024, 5, 17, hour=submission_time_hour, minute=0) + form_json["metadata"]["timeStart"] = submission_time - form_json = _create_opp_and_form_json(opportunity, user=user_with_connectid_link) - time = datetime.datetime(2024, 4, 17, verification_flags.form_submission_start.hour - 1, 0, 0) - form_json["metadata"]["timeStart"] = time make_request(api_client, form_json, user_with_connectid_link) - visit = UserVisit.objects.get(user=user_with_connectid_link) - assert visit.flagged - assert ["form_submission_period", "Form was submitted before the start time"] in visit.flag_reason.get("flags", []) - - -def test_reciever_verification_flags_form_submission_end( - user_with_connectid_link: User, api_client: APIClient, opportunity: Opportunity -): - verification_flags = OpportunityVerificationFlags.objects.get(opportunity=opportunity) - form_json = _create_opp_and_form_json(opportunity, user=user_with_connectid_link) - time = datetime.datetime(2024, 4, 17, verification_flags.form_submission_end.hour + 1, 0, 0) - form_json["metadata"]["timeStart"] = time - make_request(api_client, form_json, user_with_connectid_link) visit = UserVisit.objects.get(user=user_with_connectid_link) - assert visit.flagged - assert ["form_submission_period", "Form was submitted after the end time"] in visit.flag_reason.get("flags", []) + + # Assert based on the expected message + if expected_message is None: + assert not visit.flagged + else: + assert visit.flagged + assert ["form_submission_period", expected_message] in visit.flag_reason.get("flags", []) def test_reciever_verification_flags_duration( diff --git a/commcare_connect/opportunity/tests/factories.py b/commcare_connect/opportunity/tests/factories.py index 1d7bf83a..f65bb809 100644 --- a/commcare_connect/opportunity/tests/factories.py +++ b/commcare_connect/opportunity/tests/factories.py @@ -1,4 +1,4 @@ -from datetime import time, timezone +from datetime import timezone from factory import DictFactory, Faker, LazyAttribute, SelfAttribute, SubFactory from factory.django import DjangoModelFactory @@ -72,8 +72,8 @@ class Meta: class OpportunityVerificationFlagsFactory(DjangoModelFactory): opportunity = SubFactory(OpportunityFactory) - form_submission_start = time(10, 0, 0) - form_submission_end = time(12, 0, 0) + form_submission_start = None # Default to None + form_submission_end = None # Default to None class Meta: model = "opportunity.OpportunityVerificationFlags" diff --git a/commcare_connect/opportunity/tests/test_api_views.py b/commcare_connect/opportunity/tests/test_api_views.py index 9afb6c37..0471ebbd 100644 --- a/commcare_connect/opportunity/tests/test_api_views.py +++ b/commcare_connect/opportunity/tests/test_api_views.py @@ -146,8 +146,22 @@ def test_learn_progress_endpoint(mobile_user: User, api_client: APIClient): assert list(response.data["assessments"][0].keys()) == ["date", "score", "passing_score", "passed"] +@pytest.mark.parametrize( + "opportunity", + [ + { + "verification_flags": { + "form_submission_start": datetime.time(10, 0), + "form_submission_end": datetime.time(14, 0), + } + } + ], + indirect=True, +) def test_opportunity_list_endpoint( - mobile_user_with_connect_link: User, api_client: APIClient, opportunity: Opportunity + mobile_user_with_connect_link: User, + api_client: APIClient, + opportunity: Opportunity, ): api_client.force_authenticate(mobile_user_with_connect_link) response = api_client.get("/api/opportunity/")