Merge remote-tracking branch 'origin/main' into pkv/tests-program-man…

…agement

commcare_connect/conftest.py

@@ -47,12 +47,13 @@ def user(db) -> User:
     return UserFactory()
 
 
-@pytest.fixture
+@pytest.fixture()
 def opportunity(request):
+    verification_flags = getattr(request, "param", {}).get("verification_flags", {})
     opp_options = {"is_test": False}
-    opp_options.update(request.param if hasattr(request, "param") else {})
+    opp_options.update(getattr(request, "param", {}).get("opp_options", {}))
     factory = OpportunityFactory(**opp_options)
-    OpportunityVerificationFlagsFactory(opportunity=factory)
+    OpportunityVerificationFlagsFactory(opportunity=factory, **verification_flags)
     return factory
 
 

commcare_connect/form_receiver/tests/test_receiver_integration.py

@@ -451,55 +451,47 @@ def test_auto_approve_visits_and_payments(
     assert access.payment_accrued == completed_work.payment_accrued
 
 
-def test_receiver_verification_flags_form_submission(
-    user_with_connectid_link: User, api_client: APIClient, opportunity: Opportunity
+@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,
+    submission_time_hour,
+    expected_message,
 ):
-    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)
-    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_receiver_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()
+    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, 9, 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_receiver_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)
-    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_receiver_verification_flags_duration(
@@ -587,7 +579,7 @@ def test_receiver_verification_flags_catchment_areas(
     assert ["catchment", "Visit outside worker catchment areas"] in visit.flag_reason.get("flags", [])
 
 
-@pytest.mark.parametrize("opportunity", [{"managed": True}], indirect=True)
+@pytest.mark.parametrize("opportunity", [{"opp_options": {"managed": True}}], indirect=True)
 @pytest.mark.parametrize(
     "visit_status, review_status",
     [

commcare_connect/opportunity/api/serializers.py

@@ -15,6 +15,7 @@
     OpportunityAccess,
     OpportunityClaim,
     OpportunityClaimLimit,
+    OpportunityVerificationFlags,
     Payment,
     PaymentUnit,
     UserVisit,
@@ -91,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()
@@ -105,6 +112,7 @@ class OpportunitySerializer(serializers.ModelSerializer):
     payment_units = serializers.SerializerMethodField()
     is_user_suspended = serializers.SerializerMethodField()
     catchment_areas = serializers.SerializerMethodField()
+    verification_flags = OpportunityVerificationFlagsSerializer(source="opportunityverificationflags", read_only=True)
 
     class Meta:
         model = Opportunity
@@ -133,6 +141,7 @@ class Meta:
             "payment_units",
             "is_user_suspended",
             "catchment_areas",
+            "verification_flags",
         ]
 
     def get_claim(self, obj):

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")

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):

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"
@@ -71,6 +72,8 @@ class Meta:
 
 class OpportunityVerificationFlagsFactory(DjangoModelFactory):
     opportunity = SubFactory(OpportunityFactory)
+    form_submission_start = None  # Default to None
+    form_submission_end = None  # Default to None
 
     class Meta:
         model = "opportunity.OpportunityVerificationFlags"

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/")
@@ -160,6 +174,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 = 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(

commcare_connect/opportunity/tests/test_visit_import.py

@@ -542,7 +542,7 @@ def get_assignable_completed_work_count(access: OpportunityAccess) -> int:
     return total_assigned_count
 
 
-@pytest.mark.parametrize("opportunity", [{"managed": True}], indirect=True)
+@pytest.mark.parametrize("opportunity", [{"opp_options": {"managed": True}}], indirect=True)
 @pytest.mark.parametrize("visit_status", [VisitValidationStatus.approved, VisitValidationStatus.rejected])
 def test_network_manager_flagged_visit_review_status(mobile_user: User, opportunity: Opportunity, visit_status):
     assert opportunity.managed
@@ -567,7 +567,7 @@ def test_network_manager_flagged_visit_review_status(mobile_user: User, opportun
             assert visit.justification == "justification"
 
 
-@pytest.mark.parametrize("opportunity", [{"managed": True}], indirect=True)
+@pytest.mark.parametrize("opportunity", [{"opp_options": {"managed": True}}], indirect=True)
 @pytest.mark.parametrize(
     "review_status, cw_status",
     [

commcare_connect/opportunity/utils/completed_work.py

@@ -44,9 +44,19 @@ 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=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.
+    """
+    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(
+        "status_modified_date"
+    )
 
     if not payments or not completed_works:
         return
@@ -76,4 +86,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"])

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"

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"])

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