diff --git a/README.md b/README.md index a2450d14..6e32ed21 100644 --- a/README.md +++ b/README.md @@ -146,6 +146,19 @@ For details on how this actions is configured see: - https://aws.amazon.com/blogs/security/use-iam-roles-to-connect-github-actions-to-actions-in-aws/ - https://docs.github.com/en/actions/deployment/security-hardening-your-deployments/configuring-openid-connect-in-amazon-web-services +### Deploying to the staging environment + +The project has a staging environment at [https://connect-staging.dimagi.com/](https://connect-staging.dimagi.com/), +which is connected to the staging environment of CommCare HQ at +[https://staging.commcarehq.org/](https://staging.commcarehq.org/). + +By convention, the `pkv/staging` branch is used for changes that are on the staging environment. +To put your own changes on the staging environment, you can create merge your own branch into +`pkv/staging` and then push it to GitHub. + +After that, you can deploy to the staging environment by manually running the `deploy` +[workflow from here](https://github.com/dimagi/commcare-connect/actions/workflows/deploy.yml). + ### Custom Bootstrap Compilation The generated CSS is set up with automatic Bootstrap recompilation with variables of your choice. diff --git a/commcare_connect/conftest.py b/commcare_connect/conftest.py index 1cbb6ff4..c984262b 100644 --- a/commcare_connect/conftest.py +++ b/commcare_connect/conftest.py @@ -10,6 +10,7 @@ PaymentUnitFactory, ) from commcare_connect.organization.models import Organization +from commcare_connect.program.tests.factories import ManagedOpportunityFactory from commcare_connect.users.models import User from commcare_connect.users.tests.factories import ( ConnectIdUserLinkFactory, @@ -50,7 +51,12 @@ def user(db) -> User: @pytest.fixture() def opportunity(request): verification_flags = getattr(request, "param", {}).get("verification_flags", {}) - factory = OpportunityFactory(is_test=False) + opp_options = {"is_test": False} + opp_options.update(getattr(request, "param", {}).get("opp_options", {})) + if opp_options.get("managed", False): + factory = ManagedOpportunityFactory(**opp_options) + else: + factory = OpportunityFactory(**opp_options) OpportunityVerificationFlagsFactory(opportunity=factory, **verification_flags) return factory diff --git a/commcare_connect/connect_id_client/main.py b/commcare_connect/connect_id_client/main.py index 1f5307a1..7f117d9f 100644 --- a/commcare_connect/connect_id_client/main.py +++ b/commcare_connect/connect_id_client/main.py @@ -54,8 +54,11 @@ def add_credential(organization: Organization, credential: str, users: list[str] return -def fetch_credentials(): - response = _make_request(GET, "/users/fetch_credentials") +def fetch_credentials(org_slug=None): + params = {} + if org_slug: + params["org_slug"] = org_slug + response = _make_request(GET, "/users/fetch_credentials", params=params) data = response.json() return [Credential(**c) for c in data["credentials"]] diff --git a/commcare_connect/form_receiver/tests/test_receiver_integration.py b/commcare_connect/form_receiver/tests/test_receiver_integration.py index 0be668c8..a831237d 100644 --- a/commcare_connect/form_receiver/tests/test_receiver_integration.py +++ b/commcare_connect/form_receiver/tests/test_receiver_integration.py @@ -23,6 +23,7 @@ OpportunityClaimLimit, OpportunityVerificationFlags, UserVisit, + VisitReviewStatus, VisitValidationStatus, ) from commcare_connect.opportunity.tasks import bulk_approve_completed_work @@ -172,24 +173,14 @@ def test_receiver_deliver_form_daily_visits_reached( def test_receiver_deliver_form_max_visits_reached( mobile_user_with_connect_link: User, api_client: APIClient, opportunity: Opportunity ): - def form_json(payment_unit): - deliver_unit = DeliverUnitFactory(app=opportunity.deliver_app, payment_unit=payment_unit) - stub = DeliverUnitStubFactory(id=deliver_unit.slug) - form_json = get_form_json( - form_block=stub.json, - domain=deliver_unit.app.cc_domain, - app_id=deliver_unit.app.cc_app_id, - ) - return form_json - def submit_form_for_random_entity(form_json): duplicate_json = deepcopy(form_json) duplicate_json["form"]["deliver"]["entity_id"] = str(uuid4()) make_request(api_client, duplicate_json, mobile_user_with_connect_link) payment_units = opportunity.paymentunit_set.all() - form_json1 = form_json(payment_units[0]) - form_json2 = form_json(payment_units[1]) + form_json1 = get_form_json_for_payment_unit(payment_units[0]) + form_json2 = get_form_json_for_payment_unit(payment_units[1]) for _ in range(2): submit_form_for_random_entity(form_json1) submit_form_for_random_entity(form_json2) @@ -465,7 +456,7 @@ def test_reciever_verification_flags_form_submission( assert ["form_submission_period", expected_message] in visit.flag_reason.get("flags", []) -def test_reciever_verification_flags_duration( +def test_receiver_verification_flags_duration( user_with_connectid_link: User, api_client: APIClient, opportunity: Opportunity ): form_json = _create_opp_and_form_json(opportunity, user=user_with_connectid_link) @@ -478,7 +469,7 @@ def test_reciever_verification_flags_duration( assert ["duration", "The form was completed too quickly."] in visit.flag_reason.get("flags", []) -def test_reciever_verification_flags_check_attachments( +def test_receiver_verification_flags_check_attachments( user_with_connectid_link: User, api_client: APIClient, opportunity: Opportunity ): form_json = _create_opp_and_form_json(opportunity, user=user_with_connectid_link) @@ -491,7 +482,7 @@ def test_reciever_verification_flags_check_attachments( assert ["attachment_missing", "Form was submitted without attachements."] in visit.flag_reason.get("flags", []) -def test_reciever_verification_flags_form_json_rule( +def test_receiver_verification_flags_form_json_rule( user_with_connectid_link: User, api_client: APIClient, opportunity: Opportunity ): form_json = _create_opp_and_form_json(opportunity, user=user_with_connectid_link) @@ -509,7 +500,7 @@ def test_reciever_verification_flags_form_json_rule( assert not visit.flagged -def test_reciever_verification_flags_form_json_rule_flagged( +def test_receiver_verification_flags_form_json_rule_flagged( user_with_connectid_link: User, api_client: APIClient, opportunity: Opportunity ): form_json = _create_opp_and_form_json(opportunity, user=user_with_connectid_link) @@ -531,7 +522,7 @@ def test_reciever_verification_flags_form_json_rule_flagged( ] in visit.flag_reason.get("flags", []) -def test_reciever_verification_flags_catchment_areas( +def test_receiver_verification_flags_catchment_areas( user_with_connectid_link: User, api_client: APIClient, opportunity: Opportunity ): verification_flags = OpportunityVerificationFlags.objects.get(opportunity=opportunity) @@ -550,6 +541,40 @@ def test_reciever_verification_flags_catchment_areas( assert ["catchment", "Visit outside worker catchment areas"] in visit.flag_reason.get("flags", []) +@pytest.mark.parametrize("opportunity", [{"opp_options": {"managed": True, "org_pay_per_visit": 2}}], indirect=True) +@pytest.mark.parametrize( + "visit_status, review_status", + [ + (VisitValidationStatus.approved, VisitReviewStatus.agree), + (VisitValidationStatus.pending, VisitReviewStatus.pending), + ], +) +def test_receiver_visit_review_status( + mobile_user_with_connect_link: User, api_client: APIClient, opportunity: Opportunity, visit_status, review_status +): + assert opportunity.managed + form_json = get_form_json_for_payment_unit(opportunity.paymentunit_set.first()) + if visit_status != VisitValidationStatus.approved: + form_json["metadata"]["location"] = None + make_request(api_client, form_json, mobile_user_with_connect_link) + visit = UserVisit.objects.get(user=mobile_user_with_connect_link) + if visit_status != VisitValidationStatus.approved: + assert visit.flagged + assert visit.status == visit_status + assert visit.review_status == review_status + + +def get_form_json_for_payment_unit(payment_unit): + deliver_unit = DeliverUnitFactory(app=payment_unit.opportunity.deliver_app, payment_unit=payment_unit) + stub = DeliverUnitStubFactory(id=deliver_unit.slug) + form_json = get_form_json( + form_block=stub.json, + domain=deliver_unit.app.cc_domain, + app_id=deliver_unit.app.cc_app_id, + ) + return form_json + + def _get_form_json(learn_app, module_id, form_block=None): form_json = get_form_json( form_block=form_block or LearnModuleJsonFactory(id=module_id).json, diff --git a/commcare_connect/opportunity/admin.py b/commcare_connect/opportunity/admin.py index 6bb084a5..e833dd19 100644 --- a/commcare_connect/opportunity/admin.py +++ b/commcare_connect/opportunity/admin.py @@ -26,7 +26,6 @@ admin.site.register(CommCareApp) -admin.site.register(PaymentUnit) admin.site.register(UserInvite) admin.site.register(DeliveryType) admin.site.register(DeliverUnitFlagRules) @@ -48,6 +47,7 @@ class OpportunityAccessAdmin(admin.ModelAdmin): form = OpportunityAccessCreationForm list_display = ["get_opp_name", "get_username"] actions = ["clear_user_progress"] + search_fields = ["user__username"] @admin.display(description="Opportunity Name") def get_opp_name(self, obj): @@ -102,6 +102,7 @@ class CompletedModuleAdmin(admin.ModelAdmin): @admin.register(UserVisit) class UserVisitAdmin(admin.ModelAdmin): list_display = ["deliver_unit", "user", "opportunity", "status"] + search_fields = ["opportunity_access__user__username", "opportunity_access__opportunity__name"] @admin.register(Assessment) @@ -112,6 +113,7 @@ class AssessmentAdmin(admin.ModelAdmin): @admin.register(CompletedWork) class CompletedWorkAdmin(admin.ModelAdmin): list_display = ["get_username", "get_opp_name", "opportunity_access", "payment_unit", "status"] + search_fields = ["opportunity_access__user__username", "opportunity_access__opportunity__name"] @admin.display(description="Opportunity Name") def get_opp_name(self, obj): @@ -120,3 +122,13 @@ def get_opp_name(self, obj): @admin.display(description="Username") def get_username(self, obj): return obj.opportunity_access.user.username + + +@admin.register(PaymentUnit) +class PaymentUnitAdmin(admin.ModelAdmin): + list_display = ["name", "get_opp_name"] + search_fields = ["name"] + + @admin.display(description="Opportunity Name") + def get_opp_name(self, obj): + return obj.opportunity_access.opportunity.name diff --git a/commcare_connect/opportunity/forms.py b/commcare_connect/opportunity/forms.py index 3e472450..7c567d89 100644 --- a/commcare_connect/opportunity/forms.py +++ b/commcare_connect/opportunity/forms.py @@ -33,7 +33,8 @@ class OpportunityUserInviteForm(forms.Form): def __init__(self, *args, **kwargs): - credentials = connect_id_client.fetch_credentials() + org_slug = kwargs.pop("org_slug", None) + credentials = connect_id_client.fetch_credentials(org_slug) super().__init__(*args, **kwargs) self.helper = FormHelper(self) @@ -73,7 +74,10 @@ def clean_users(self): return split_users -class OpportunityChangeForm(forms.ModelForm, OpportunityUserInviteForm): +class OpportunityChangeForm( + OpportunityUserInviteForm, + forms.ModelForm, +): class Meta: model = Opportunity fields = [ diff --git a/commcare_connect/opportunity/management/commands/update_completed_work_paid_date.py b/commcare_connect/opportunity/management/commands/update_completed_work_paid_date.py new file mode 100644 index 00000000..000d1e4f --- /dev/null +++ b/commcare_connect/opportunity/management/commands/update_completed_work_paid_date.py @@ -0,0 +1,23 @@ +from django.core.management import BaseCommand +from django.db import transaction + +from commcare_connect.opportunity.models import OpportunityAccess +from commcare_connect.opportunity.utils.completed_work import update_work_payment_date + + +class Command(BaseCommand): + help = "Updates paid dates from payments for all opportunity accesses" + + def handle(self, *args, **kwargs): + try: + with transaction.atomic(): + accesses = OpportunityAccess.objects.all() + self.stdout.write("Starting to process to update the paid date...") + + for access in accesses: + update_work_payment_date(access) + + self.stdout.write("Process completed") + + except Exception as e: + self.stdout.write(self.style.ERROR(f"An error occurred: {str(e)}")) diff --git a/commcare_connect/opportunity/migrations/0060_completedwork_payment_date.py b/commcare_connect/opportunity/migrations/0060_completedwork_payment_date.py index c8481f41..d1478347 100644 --- a/commcare_connect/opportunity/migrations/0060_completedwork_payment_date.py +++ b/commcare_connect/opportunity/migrations/0060_completedwork_payment_date.py @@ -1,18 +1,6 @@ # Generated by Django 4.2.5 on 2024-10-07 08:54 -from django.db import migrations, models, transaction - -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, Payment, CompletedWork) +from django.db import migrations, models class Migration(migrations.Migration): @@ -26,5 +14,4 @@ class Migration(migrations.Migration): name="payment_date", field=models.DateTimeField(null=True), ), - migrations.RunPython(update_paid_date_from_payments, migrations.RunPython.noop), ] diff --git a/commcare_connect/opportunity/models.py b/commcare_connect/opportunity/models.py index 17f68978..a6351e64 100644 --- a/commcare_connect/opportunity/models.py +++ b/commcare_connect/opportunity/models.py @@ -159,7 +159,16 @@ def approved_visits(self): @property def number_of_users(self): - return self.total_budget / self.budget_per_user + if not self.managed: + return self.total_budget / self.budget_per_user + + budget_per_user = 0 + payment_units = self.paymentunit_set.all() + org_pay = self.managedopportunity.org_pay_per_visit + for pu in payment_units: + budget_per_user += pu.max_total * (pu.amount + org_pay) + + return self.total_budget / budget_per_user @property def allotted_visits(self): @@ -355,6 +364,9 @@ class PaymentUnit(models.Model): null=True, ) + def __str__(self): + return self.name + class DeliverUnit(models.Model): app = models.ForeignKey( diff --git a/commcare_connect/opportunity/tests/factories.py b/commcare_connect/opportunity/tests/factories.py index f65bb809..1a7cb7df 100644 --- a/commcare_connect/opportunity/tests/factories.py +++ b/commcare_connect/opportunity/tests/factories.py @@ -133,6 +133,7 @@ class UserVisitFactory(DjangoModelFactory): visit_date = Faker("date_time", tzinfo=timezone.utc) form_json = Faker("pydict", value_types=[str, int, float, bool]) xform_id = Faker("uuid4") + completed_work = SubFactory(CompletedWorkFactory) class Meta: model = "opportunity.UserVisit" @@ -233,7 +234,7 @@ class Meta: class PaymentFactory(DjangoModelFactory): opportunity_access = SubFactory(OpportunityAccessFactory) amount = Faker("pyint", min_value=1, max_value=10000) - date_paid = Faker("past_date") + date_paid = Faker("date_time", tzinfo=timezone.utc) class Meta: model = "opportunity.Payment" diff --git a/commcare_connect/opportunity/tests/test_forms.py b/commcare_connect/opportunity/tests/test_forms.py index a8f422be..58f6db95 100644 --- a/commcare_connect/opportunity/tests/test_forms.py +++ b/commcare_connect/opportunity/tests/test_forms.py @@ -5,8 +5,8 @@ import pytest from factory.fuzzy import FuzzyDate, FuzzyText -from commcare_connect.opportunity.forms import OpportunityCreationForm -from commcare_connect.opportunity.tests.factories import ApplicationFactory +from commcare_connect.opportunity.forms import OpportunityChangeForm, OpportunityCreationForm +from commcare_connect.opportunity.tests.factories import ApplicationFactory, CommCareAppFactory, OpportunityFactory class TestOpportunityCreationForm: @@ -111,3 +111,197 @@ def test_save(self, user, organization): ) form.is_valid() form.save() + + +@pytest.mark.django_db +class TestOpportunityChangeForm: + @pytest.fixture(autouse=True) + def setup_credentials_mock(self, monkeypatch): + self.mock_credentials = [ + type("Credential", (), {"slug": "cert1", "name": "Work for test"}), + type("Credential", (), {"slug": "cert2", "name": "Work for test"}), + ] + monkeypatch.setattr( + "commcare_connect.connect_id_client.fetch_credentials", lambda org_slug: self.mock_credentials + ) + + @pytest.fixture + def valid_opportunity(self, organization): + return OpportunityFactory( + organization=organization, + active=True, + learn_app=CommCareAppFactory(cc_app_id="test_learn_app"), + deliver_app=CommCareAppFactory(cc_app_id="test_deliver_app"), + name="Test Opportunity", + description="Test Description", + short_description="Short Description", + currency="USD", + is_test=False, + end_date=datetime.date.today() + datetime.timedelta(days=30), + ) + + @pytest.fixture + def base_form_data(self, valid_opportunity): + return { + "name": "Updated Opportunity", + "description": "Updated Description", + "short_description": "Updated Short Description", + "active": True, + "currency": "EUR", + "is_test": False, + "delivery_type": valid_opportunity.delivery_type.id, + "additional_users": 5, + "end_date": (datetime.date.today() + datetime.timedelta(days=60)).isoformat(), + "users": "+1234567890\n+9876543210", + "filter_country": "US", + "filter_credential": "cert1", + } + + def test_form_initialization(self, valid_opportunity, organization): + form = OpportunityChangeForm(instance=valid_opportunity, org_slug=organization.slug) + expected_fields = { + "name", + "description", + "short_description", + "active", + "currency", + "is_test", + "delivery_type", + "additional_users", + "end_date", + "users", + "filter_country", + "filter_credential", + } + assert all(field in form.fields for field in expected_fields) + + expected_initial = { + "name": valid_opportunity.name, + "description": valid_opportunity.description, + "short_description": valid_opportunity.short_description, + "active": valid_opportunity.active, + "currency": valid_opportunity.currency, + "is_test": valid_opportunity.is_test, + "delivery_type": valid_opportunity.delivery_type.id, + "end_date": valid_opportunity.end_date.isoformat(), + "filter_country": [""], + "filter_credential": [""], + } + assert all(form.initial.get(key) == value for key, value in expected_initial.items()) + + @pytest.mark.parametrize( + "field", + [ + "name", + "description", + "short_description", + "currency", + ], + ) + def test_required_fields(self, valid_opportunity, organization, field, base_form_data): + data = base_form_data.copy() + data[field] = "" + form = OpportunityChangeForm(data=data, instance=valid_opportunity, org_slug=organization.slug) + assert not form.is_valid() + assert field in form.errors + + @pytest.mark.parametrize( + "test_data", + [ + pytest.param( + { + "field": "additional_users", + "value": "invalid", + "error_expected": True, + "error_message": "Enter a whole number.", + }, + id="invalid_additional_users", + ), + pytest.param( + { + "field": "end_date", + "value": "invalid-date", + "error_expected": True, + "error_message": "Enter a valid date.", + }, + id="invalid_end_date", + ), + pytest.param( + { + "field": "users", + "value": " +1234567890 \n +9876543210 ", + "error_expected": False, + "expected_clean": ["+1234567890", "+9876543210"], + }, + id="valid_users_with_whitespace", + ), + ], + ) + def test_field_validation(self, valid_opportunity, organization, base_form_data, test_data): + data = base_form_data.copy() + data[test_data["field"]] = test_data["value"] + form = OpportunityChangeForm(data=data, instance=valid_opportunity, org_slug=organization.slug) + if test_data["error_expected"]: + assert not form.is_valid() + assert test_data["error_message"] in str(form.errors[test_data["field"]]) + else: + assert form.is_valid() + if "expected_clean" in test_data: + assert form.cleaned_data[test_data["field"]] == test_data["expected_clean"] + + @pytest.mark.parametrize( + "app_scenario", + [ + pytest.param( + { + "active_app_ids": ("unique_app1", "unique_app2"), + "new_app_ids": ("different_app1", "different_app2"), + "expected_valid": True, + }, + id="unique_apps", + ), + pytest.param( + { + "active_app_ids": ("shared_app1", "shared_app2"), + "new_app_ids": ("shared_app1", "shared_app2"), + "expected_valid": False, + }, + id="reused_apps", + ), + ], + ) + def test_app_reuse_validation(self, organization, base_form_data, app_scenario): + OpportunityFactory( + organization=organization, + active=True, + learn_app=CommCareAppFactory(cc_app_id=app_scenario["active_app_ids"][0]), + deliver_app=CommCareAppFactory(cc_app_id=app_scenario["active_app_ids"][1]), + ) + + inactive_opp = OpportunityFactory( + organization=organization, + active=False, + learn_app=CommCareAppFactory(cc_app_id=app_scenario["new_app_ids"][0]), + deliver_app=CommCareAppFactory(cc_app_id=app_scenario["new_app_ids"][1]), + ) + + form = OpportunityChangeForm(data=base_form_data, instance=inactive_opp, org_slug=organization.slug) + + assert form.is_valid() == app_scenario["expected_valid"] + if not app_scenario["expected_valid"]: + assert "Cannot reactivate opportunity with reused applications" in str(form.errors["active"]) + + @pytest.mark.parametrize( + "data_updates,expected_valid", + [ + ({"currency": "USD", "additional_users": 5}, True), + ({"currency": "EUR", "additional_users": 10}, True), + ({"currency": "INVALID", "additional_users": 5}, False), + ({"currency": "USD", "additional_users": -5}, True), + ], + ) + def test_valid_combinations(self, valid_opportunity, organization, base_form_data, data_updates, expected_valid): + data = base_form_data.copy() + data.update(data_updates) + form = OpportunityChangeForm(data=data, instance=valid_opportunity, org_slug=organization.slug) + assert form.is_valid() == expected_valid diff --git a/commcare_connect/opportunity/tests/test_views.py b/commcare_connect/opportunity/tests/test_views.py index da418aae..064ee424 100644 --- a/commcare_connect/opportunity/tests/test_views.py +++ b/commcare_connect/opportunity/tests/test_views.py @@ -1,14 +1,25 @@ import pytest from django.test import Client from django.urls import reverse +from django.utils.timezone import now -from commcare_connect.opportunity.models import Opportunity, OpportunityAccess, OpportunityClaimLimit +from commcare_connect.opportunity.models import ( + Opportunity, + OpportunityAccess, + OpportunityClaimLimit, + UserVisit, + VisitReviewStatus, + VisitValidationStatus, +) from commcare_connect.opportunity.tests.factories import ( + OpportunityAccessFactory, OpportunityClaimFactory, OpportunityClaimLimitFactory, PaymentUnitFactory, + UserVisitFactory, ) from commcare_connect.organization.models import Organization +from commcare_connect.program.tests.factories import ManagedOpportunityFactory, ProgramFactory from commcare_connect.users.models import User @@ -38,3 +49,65 @@ 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 + + +class TestUserVisitReviewView: + @pytest.fixture(autouse=True) + def setup( + self, + client: Client, + program_manager_org: Organization, + program_manager_org_user_admin: User, + organization: Organization, + org_user_admin: User, + ): + self.client = client + self.pm_org = program_manager_org + self.pm_user = program_manager_org_user_admin + self.nm_org = organization + self.nm_user = org_user_admin + self.program = ProgramFactory(organization=self.pm_org) + self.opportunity = ManagedOpportunityFactory(program=self.program, organization=self.nm_org) + access = OpportunityAccessFactory(opportunity=self.opportunity, accepted=True) + self.visits = UserVisitFactory.create_batch( + 10, + opportunity=self.opportunity, + status=VisitValidationStatus.approved, + review_created_on=now(), + review_status=VisitReviewStatus.pending, + opportunity_access=access, + ) + + def test_user_visit_review_program_manager_table(self): + self.url = reverse("opportunity:user_visit_review", args=(self.pm_org.slug, self.opportunity.id)) + self.client.force_login(self.pm_user) + response = self.client.get(self.url) + assert response.status_code == 200 + table = response.context["table"] + assert len(table.rows) == 10 + assert "pk" in table.columns.names() + + @pytest.mark.parametrize("review_status", [(VisitReviewStatus.agree), (VisitReviewStatus.disagree)]) + def test_user_visit_review_program_manager_approval(self, review_status): + self.url = reverse("opportunity:user_visit_review", args=(self.pm_org.slug, self.opportunity.id)) + self.client.force_login(self.pm_user) + response = self.client.post(self.url, {"pk": [], "review_status": review_status.value}) + assert response.status_code == 200 + visits = UserVisit.objects.filter(id__in=[visit.id for visit in self.visits]) + for visit in visits: + assert visit.review_status == VisitReviewStatus.pending + + visit_ids = [visit.id for visit in self.visits][:5] + response = self.client.post(self.url, {"pk": visit_ids, "review_status": review_status.value}) + assert response.status_code == 200 + visits = UserVisit.objects.filter(id__in=visit_ids) + for visit in visits: + assert visit.review_status == review_status + + def test_user_visit_review_network_manager_table(self): + self.url = reverse("opportunity:user_visit_review", args=(self.nm_org.slug, self.opportunity.id)) + self.client.force_login(self.nm_user) + response = self.client.get(self.url) + table = response.context["table"] + assert len(table.rows) == 10 + assert "pk" not in table.columns.names() diff --git a/commcare_connect/opportunity/tests/test_visit_import.py b/commcare_connect/opportunity/tests/test_visit_import.py index 4c5f10ee..c8aa16e5 100644 --- a/commcare_connect/opportunity/tests/test_visit_import.py +++ b/commcare_connect/opportunity/tests/test_visit_import.py @@ -19,6 +19,7 @@ Payment, PaymentUnit, UserVisit, + VisitReviewStatus, VisitValidationStatus, ) from commcare_connect.opportunity.tests.factories import ( @@ -539,3 +540,65 @@ def get_assignable_completed_work_count(access: OpportunityAccess) -> int: total_assigned_count += 1 return total_assigned_count + + +@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 + access = OpportunityAccess.objects.get(user=mobile_user, opportunity=opportunity) + visits = UserVisitFactory.create_batch( + 5, opportunity=opportunity, status=VisitValidationStatus.pending, user=mobile_user, opportunity_access=access + ) + dataset = Dataset(headers=["visit id", "status", "rejected reason", "justification"]) + dataset.extend([[visit.xform_id, visit_status.value, "", "justification"] for visit in visits]) + before_update = now() + import_status = _bulk_update_visit_status(opportunity, dataset) + after_update = now() + assert not import_status.missing_visits + updated_visits = UserVisit.objects.filter(opportunity=opportunity) + for visit in updated_visits: + assert visit.status == visit_status + assert visit.status_modified_date is not None + assert before_update <= visit.status_modified_date <= after_update + if visit.status == VisitValidationStatus.approved: + assert before_update <= visit.review_created_on <= after_update + assert visit.review_status == VisitReviewStatus.pending + assert visit.justification == "justification" + + +@pytest.mark.parametrize("opportunity", [{"opp_options": {"managed": True}}], indirect=True) +@pytest.mark.parametrize( + "review_status, cw_status", + [ + (VisitReviewStatus.pending, CompletedWorkStatus.pending), + (VisitReviewStatus.agree, CompletedWorkStatus.approved), + (VisitReviewStatus.disagree, CompletedWorkStatus.pending), + ], +) +def test_review_completed_work_status( + mobile_user_with_connect_link: User, opportunity: Opportunity, review_status, cw_status +): + deliver_unit = DeliverUnitFactory(app=opportunity.deliver_app, payment_unit=opportunity.paymentunit_set.first()) + access = OpportunityAccess.objects.get(user=mobile_user_with_connect_link, opportunity=opportunity) + UserVisitFactory.create_batch( + 2, + opportunity_access=access, + status=VisitValidationStatus.approved, + review_status=review_status, + review_created_on=now(), + completed_work__status=CompletedWorkStatus.pending, + completed_work__opportunity_access=access, + completed_work__payment_unit=opportunity.paymentunit_set.first(), + deliver_unit=deliver_unit, + ) + assert access.payment_accrued == 0 + update_payment_accrued(opportunity, {mobile_user_with_connect_link.id}) + completed_works = CompletedWork.objects.filter(opportunity_access=access) + payment_accrued = 0 + for cw in completed_works: + assert cw.status == cw_status + if cw.status == CompletedWorkStatus.approved: + payment_accrued += cw.payment_accrued + access.refresh_from_db() + assert access.payment_accrued == payment_accrued diff --git a/commcare_connect/opportunity/utils/completed_work.py b/commcare_connect/opportunity/utils/completed_work.py index 2050e30b..df2d9906 100644 --- a/commcare_connect/opportunity/utils/completed_work.py +++ b/commcare_connect/opportunity/utils/completed_work.py @@ -44,19 +44,9 @@ def update_status(completed_works, opportunity_access, compute_payment=True): opportunity_access.save() -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" - ) +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") if not payments or not completed_works: return @@ -86,4 +76,4 @@ def update_work_payment_date(access: OpportunityAccess, payment_model=None, comp break if works_to_update: - completed_work_model_ref.objects.bulk_update(works_to_update, ["payment_date"]) + CompletedWork.objects.bulk_update(works_to_update, ["payment_date"]) diff --git a/commcare_connect/opportunity/views.py b/commcare_connect/opportunity/views.py index 9b6c00aa..a53c0050 100644 --- a/commcare_connect/opportunity/views.py +++ b/commcare_connect/opportunity/views.py @@ -65,6 +65,7 @@ PaymentInvoice, PaymentUnit, UserVisit, + VisitReviewStatus, VisitValidationStatus, ) from commcare_connect.opportunity.tables import ( @@ -211,6 +212,11 @@ class OpportunityEdit(OrganizationUserMemberRoleMixin, UpdateView): def get_success_url(self): return reverse("opportunity:detail", args=(self.request.org.slug, self.object.id)) + def get_form_kwargs(self): + kwargs = super().get_form_kwargs() + kwargs["org_slug"] = self.request.org.slug + return kwargs + def form_valid(self, form): opportunity = form.instance opportunity.modified_by = self.request.user.email @@ -357,21 +363,20 @@ def get_queryset(self): @org_member_required -def export_user_visits(request, **kwargs): - opportunity_id = kwargs["pk"] - get_opportunity_or_404(org_slug=request.org.slug, pk=opportunity_id) +def export_user_visits(request, org_slug, pk): + get_opportunity_or_404(org_slug=request.org.slug, pk=pk) form = VisitExportForm(data=request.POST) if not form.is_valid(): messages.error(request, form.errors) - return redirect("opportunity:detail", request.org.slug, opportunity_id) + return redirect("opportunity:detail", request.org.slug, pk) export_format = form.cleaned_data["format"] date_range = DateRanges(form.cleaned_data["date_range"]) status = form.cleaned_data["status"] flatten = form.cleaned_data["flatten_form_data"] - result = generate_visit_export.delay(opportunity_id, date_range, status, export_format, flatten) - redirect_url = reverse("opportunity:detail", args=(request.org.slug, opportunity_id)) + result = generate_visit_export.delay(pk, date_range, status, export_format, flatten) + redirect_url = reverse("opportunity:detail", args=(request.org.slug, pk)) return redirect(f"{redirect_url}?export_task_id={result.id}") @@ -483,17 +488,16 @@ def get_queryset(self): @org_member_required -def export_users_for_payment(request, **kwargs): - opportunity_id = kwargs["pk"] - get_opportunity_or_404(org_slug=request.org.slug, pk=opportunity_id) +def export_users_for_payment(request, org_slug, pk): + get_opportunity_or_404(org_slug=request.org.slug, pk=pk) form = PaymentExportForm(data=request.POST) if not form.is_valid(): messages.error(request, form.errors) - return redirect("opportunity:detail", request.org.slug, opportunity_id) + return redirect("opportunity:detail", request.org.slug, pk) export_format = form.cleaned_data["format"] - result = generate_payment_export.delay(opportunity_id, export_format) - redirect_url = reverse("opportunity:detail", args=(request.org.slug, opportunity_id)) + result = generate_payment_export.delay(pk, export_format) + redirect_url = reverse("opportunity:detail", args=(request.org.slug, pk)) return redirect(f"{redirect_url}?export_task_id={result.id}") @@ -629,17 +633,16 @@ def get_queryset(self): @org_member_required -def export_user_status(request, **kwargs): - opportunity_id = kwargs["pk"] - get_opportunity_or_404(org_slug=request.org.slug, pk=opportunity_id) +def export_user_status(request, org_slug, pk): + get_opportunity_or_404(org_slug=request.org.slug, pk=pk) form = PaymentExportForm(data=request.POST) if not form.is_valid(): messages.error(request, form.errors) - return redirect("opportunity:detail", request.org.slug, opportunity_id) + return redirect("opportunity:detail", request.org.slug, pk) export_format = form.cleaned_data["format"] - result = generate_user_status_export.delay(opportunity_id, export_format) - redirect_url = reverse("opportunity:detail", args=(request.org.slug, opportunity_id)) + result = generate_user_status_export.delay(pk, export_format) + redirect_url = reverse("opportunity:detail", args=(request.org.slug, pk)) return redirect(f"{redirect_url}?export_task_id={result.id}") @@ -658,17 +661,16 @@ def get_queryset(self): @org_member_required -def export_deliver_status(request, **kwargs): - opportunity_id = kwargs["pk"] - get_opportunity_or_404(pk=opportunity_id, org_slug=request.org.slug) +def export_deliver_status(request, org_slug, pk): + get_opportunity_or_404(pk=pk, org_slug=request.org.slug) form = PaymentExportForm(data=request.POST) if not form.is_valid(): messages.error(request, form.errors) - return redirect("opportunity:detail", request.org.slug, opportunity_id) + return redirect("opportunity:detail", request.org.slug, pk) export_format = form.cleaned_data["format"] - result = generate_deliver_status_export.delay(opportunity_id, export_format) - redirect_url = reverse("opportunity:detail", args=(request.org.slug, opportunity_id)) + result = generate_deliver_status_export.delay(pk, export_format) + redirect_url = reverse("opportunity:detail", args=(request.org.slug, pk)) return redirect(f"{redirect_url}?export_task_id={result.id}") @@ -987,17 +989,16 @@ def get_queryset(self): @org_member_required -def export_completed_work(request, **kwargs): - opportunity_id = kwargs["pk"] - get_opportunity_or_404(org_slug=request.org.slug, pk=opportunity_id) +def export_completed_work(request, org_slug, pk): + get_opportunity_or_404(org_slug=request.org.slug, pk=pk) form = PaymentExportForm(data=request.POST) if not form.is_valid(): messages.error(request, form.errors) - return redirect("opportunity:detail", request.org.slug, opportunity_id) + return redirect("opportunity:detail", request.org.slug, pk) export_format = form.cleaned_data["format"] - result = generate_work_status_export.delay(opportunity_id, export_format) - redirect_url = reverse("opportunity:detail", args=(request.org.slug, opportunity_id)) + result = generate_work_status_export.delay(pk, export_format) + redirect_url = reverse("opportunity:detail", args=(request.org.slug, pk)) return redirect(f"{redirect_url}?export_task_id={result.id}") @@ -1052,17 +1053,16 @@ def suspended_users_list(request, org_slug=None, pk=None): @org_member_required -def export_catchment_area(request, **kwargs): - opportunity_id = kwargs["pk"] - get_opportunity_or_404(org_slug=request.org.slug, pk=opportunity_id) +def export_catchment_area(request, org_slug, pk): + get_opportunity_or_404(org_slug=request.org.slug, pk=pk) form = PaymentExportForm(data=request.POST) if not form.is_valid(): messages.error(request, form.errors) - return redirect("opportunity:detail", request.org.slug, opportunity_id) + return redirect("opportunity:detail", request.org.slug, pk) export_format = form.cleaned_data["format"] - result = generate_catchment_area_export.delay(opportunity_id, export_format) - redirect_url = reverse("opportunity:detail", args=(request.org.slug, opportunity_id)) + result = generate_catchment_area_export.delay(pk, export_format) + redirect_url = reverse("opportunity:detail", args=(request.org.slug, pk)) return redirect(f"{redirect_url}?export_task_id={result.id}") @@ -1084,7 +1084,7 @@ def import_catchment_area(request, org_slug=None, pk=None): @org_member_required def opportunity_user_invite(request, org_slug=None, pk=None): opportunity = get_object_or_404(Opportunity, organization=request.org, id=pk) - form = OpportunityUserInviteForm(data=request.POST or None) + form = OpportunityUserInviteForm(data=request.POST or None, org_slug=request.org.slug) if form.is_valid(): users = form.cleaned_data["users"] filter_country = form.cleaned_data["filter_country"] @@ -1119,7 +1119,7 @@ def user_visit_review(request, org_slug, opp_id): review_status = request.POST.get("review_status").lower() updated_reviews = request.POST.getlist("pk") user_visits = UserVisit.objects.filter(pk__in=updated_reviews) - if review_status in ["agree", "disagree"]: + if review_status in [VisitReviewStatus.agree.value, VisitReviewStatus.disagree.value]: user_visits.update(review_status=review_status) update_payment_accrued(opportunity=opportunity, users=[visit.user for visit in user_visits]) diff --git a/commcare_connect/organization/forms.py b/commcare_connect/organization/forms.py index f22a4b7b..4bbde435 100644 --- a/commcare_connect/organization/forms.py +++ b/commcare_connect/organization/forms.py @@ -1,5 +1,6 @@ from crispy_forms import helper, layout from django import forms +from django.core.exceptions import ValidationError from django.utils.translation import gettext from commcare_connect.organization.models import Organization, UserOrganizationMembership @@ -25,29 +26,42 @@ def __init__(self, *args, **kwargs): class MembershipForm(forms.ModelForm): + email = forms.CharField( + max_length=254, + required=True, + label="", + widget=forms.TextInput(attrs={"placeholder": "Enter email address"}), + ) + class Meta: model = UserOrganizationMembership - fields = ("user", "role") - labels = {"user": "", "role": ""} + fields = ("role",) + labels = {"role": ""} def __init__(self, *args, **kwargs): self.organization = kwargs.pop("organization") super().__init__(*args, **kwargs) - self.fields["user"].queryset = User.objects.filter(email__isnull=False).exclude( - memberships__organization=self.organization - ) - self.helper = helper.FormHelper(self) self.helper.layout = layout.Layout( layout.Row( layout.HTML("