From 233e8965a7f55ab0bef7dc851f4410ff45aa43e3 Mon Sep 17 00:00:00 2001 From: hemant10yadav Date: Fri, 13 Sep 2024 16:57:52 +0530 Subject: [PATCH 1/9] added helper method --- .../0055_opportunityaccess_invited_date.py | 17 ++++ commcare_connect/opportunity/models.py | 1 + commcare_connect/program/helpers.py | 71 ++++++++++++++++ commcare_connect/program/tables.py | 85 ++++++++++++++++++- .../program/tests/test_helpers.py | 37 ++++++++ commcare_connect/program/urls.py | 6 ++ commcare_connect/program/views.py | 33 ++++++- .../templates/program/dashboard.html | 63 ++++++++++++++ 8 files changed, 310 insertions(+), 3 deletions(-) create mode 100644 commcare_connect/opportunity/migrations/0055_opportunityaccess_invited_date.py create mode 100644 commcare_connect/program/helpers.py create mode 100644 commcare_connect/program/tests/test_helpers.py create mode 100644 commcare_connect/templates/program/dashboard.html diff --git a/commcare_connect/opportunity/migrations/0055_opportunityaccess_invited_date.py b/commcare_connect/opportunity/migrations/0055_opportunityaccess_invited_date.py new file mode 100644 index 00000000..b55f94d5 --- /dev/null +++ b/commcare_connect/opportunity/migrations/0055_opportunityaccess_invited_date.py @@ -0,0 +1,17 @@ +# Generated by Django 4.2.5 on 2024-09-12 18:13 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("opportunity", "0054_opportunity_managed_alter_opportunity_organization"), + ] + + operations = [ + migrations.AddField( + model_name="opportunityaccess", + name="invited_date", + field=models.DateTimeField(auto_now_add=True, null=True), + ), + ] diff --git a/commcare_connect/opportunity/models.py b/commcare_connect/opportunity/models.py index f02264f0..d2f7a266 100644 --- a/commcare_connect/opportunity/models.py +++ b/commcare_connect/opportunity/models.py @@ -236,6 +236,7 @@ class OpportunityAccess(models.Model): suspended = models.BooleanField(default=False) suspension_date = models.DateTimeField(null=True, blank=True) suspension_reason = models.CharField(max_length=300, null=True, blank=True) + invited_date = models.DateTimeField(auto_now_add=True, editable=False, null=True) class Meta: indexes = [models.Index(fields=["invite_id"])] diff --git a/commcare_connect/program/helpers.py b/commcare_connect/program/helpers.py new file mode 100644 index 00000000..7f961b74 --- /dev/null +++ b/commcare_connect/program/helpers.py @@ -0,0 +1,71 @@ +from django.db.models import Avg, Count, DurationField, ExpressionWrapper, F, OuterRef, Q, Subquery + +from commcare_connect.opportunity.models import UserVisit, VisitValidationStatus +from commcare_connect.program.models import ManagedOpportunity, Program + + +def get_annotated_managed_opportunity(program: Program): + filter_for_valid__visit_date = ~Q( + opportunityaccess__uservisit__status__in=[ + VisitValidationStatus.over_limit, + VisitValidationStatus.trial, + ] + ) + + earliest_visits = ( + UserVisit.objects.filter( + opportunity_access=OuterRef("opportunityaccess"), + ) + .exclude(status__in=[VisitValidationStatus.over_limit, VisitValidationStatus.trial]) + .order_by("visit_date") + .values("visit_date")[:1] + ) + + managed_opportunities = ( + ManagedOpportunity.objects.filter(program=program) + .order_by("start_date") + .annotate( + workers_invited=Count("opportunityaccess"), + workers_passing_assessment=Count( + "opportunityaccess__assessment", + filter=Q( + opportunityaccess__assessment__passed=True, + opportunityaccess__assessment__opportunity=F("opportunityaccess__opportunity"), + ), + ), + workers_starting_delivery=Count( + "opportunityaccess__uservisit__user", + filter=filter_for_valid__visit_date, + distinct=True, + ), + percentage_conversion=F("workers_starting_delivery") / F("workers_invited") * 100, + average_time_to_convert=Avg( + ExpressionWrapper( + Subquery(earliest_visits) - F("opportunityaccess__invited_date"), output_field=DurationField() + ), + filter=filter_for_valid__visit_date, + ), + ) + .prefetch_related( + "opportunityaccess_set", + "opportunityaccess_set__uservisit_set", + "opportunityaccess_set__assessment_set", + ) + ) + + return managed_opportunities + + +def get_annotated_managed_opportunity_nm(program: Program, start_date=None, end_date=None): + managed_opportunities = ( + ManagedOpportunity.objects.filter(program=program, start_date__gte=start_date) + .order_by("start_date") + .annotate() + .prefetch_related( + "opportunityaccess_set", + "opportunityaccess_set__uservisit_set", + "opportunityaccess_set__assessment_set", + ) + ) + + return managed_opportunities diff --git a/commcare_connect/program/tables.py b/commcare_connect/program/tables.py index 2a360d3c..b3214703 100644 --- a/commcare_connect/program/tables.py +++ b/commcare_connect/program/tables.py @@ -4,7 +4,7 @@ from django.utils.safestring import mark_safe from django.utils.translation import gettext_lazy as _ -from .models import Program, ProgramApplication, ProgramApplicationStatus +from .models import ManagedOpportunity, Program, ProgramApplication, ProgramApplicationStatus TABLE_TEMPLATE = "django_tables2/bootstrap5.html" RESPONSIVE_TABLE_AND_LIGHT_HEADER = { @@ -160,6 +160,14 @@ def render_manage(self, record): "pk": record.id, }, ) + + dashboard_url = reverse( + "program:dashboard", + kwargs={ + "org_slug": self.context["request"].org.slug, + "pk": record.id, + }, + ) application_url = reverse( "program:applications", kwargs={ @@ -192,6 +200,7 @@ def render_manage(self, record): "color": "success", "icon": "bi bi-people-fill", }, + {"post": False, "url": dashboard_url, "text": "Dashboard", "color": "info", "icon": "bi bi-graph-up"}, ] return get_manage_buttons_html(buttons, self.context["request"]) @@ -221,3 +230,77 @@ def get_manage_buttons_html(buttons, request): request=request, ) return mark_safe(html) + + +class FunnelPerformanceTable(tables.Table): + organization = tables.Column() + start_date = tables.DateColumn() + workers_invited = tables.Column(verbose_name=_("Workers Invited")) + workers_passing_assessment = tables.Column(verbose_name=_("Workers Passing Assessment")) + workers_starting_delivery = tables.Column(verbose_name=_("Workers Starting Delivery")) + percentage_conversion = tables.Column(verbose_name=_("% Conversion")) + average_time_to_convert = tables.Column(verbose_name=_("Average Time To convert")) + + class Meta: + model = ManagedOpportunity + empty_text = "No data available yet." + fields = ( + "organization", + "start_date", + "workers_invited", + "workers_passing_assessment", + "workers_starting_delivery", + "percentage_conversion", + "average_time_to_convert", + ) + orderable = False + + def render_average_time_to_convert(self, record): + total_seconds = record.average_time_to_convert.total_seconds() + hours = total_seconds / 3600 + return f"{round(hours, 2)}hr" + + +class DeliveryPerformanceTable(tables.Table): + organization = tables.Column() + start_date = tables.DateColumn() + workers_invited = tables.Column( + empty_values=(), + verbose_name=_("Workers Invited"), + ) + workers_starting_delivery = tables.Column(empty_values=(), verbose_name=_("Total Workers Starting Delivery")) + active_workers = tables.Column(empty_values=(), verbose_name=_("Active Workers")) + deliveries_per_day_per_worker = tables.Column(empty_values=(), verbose_name=_("Deliveries Per Day Per Worker")) + percentage_records_flagged = tables.Column(empty_values=(), verbose_name="% Records Flagged") + + class Meta: + model = ManagedOpportunity + empty_text = "No data available yet." + fields = ( + "organization", + "start_date", + "Total Workers Starting Delivery", + "Active Workers", + "Deliveries Per Day Per Worker", + "% Records Flagged", + ) + orderable = False + + @staticmethod + def get_queryset(): + queryset = ManagedOpportunity.objects.prefetch_related("opportunityaccess_set") + return queryset + + def precompute_data(self, record): + """Precompute the values for the record and store them in the record.""" + if not hasattr(record, "_precomputed_data"): + data = { + "workers_starting_delivery": 0, + } + for access in record.opportunityaccess_set.all(): + if access.last_visit_date: + data["workers_starting_delivery"] += 1 + + record._precomputed_data = data + + return record._precomputed_data diff --git a/commcare_connect/program/tests/test_helpers.py b/commcare_connect/program/tests/test_helpers.py new file mode 100644 index 00000000..04d4a168 --- /dev/null +++ b/commcare_connect/program/tests/test_helpers.py @@ -0,0 +1,37 @@ +from datetime import timedelta + +import pytest +from django.urls import reverse +from django_celery_beat.utils import now + +from commcare_connect.opportunity.models import VisitValidationStatus +from commcare_connect.opportunity.tests.factories import AssessmentFactory, OpportunityAccessFactory, UserVisitFactory +from commcare_connect.program.tests.factories import ManagedOpportunityFactory, ProgramFactory +from commcare_connect.program.tests.test_views import BaseProgramTest +from commcare_connect.users.tests.factories import OrganizationFactory, UserFactory + + +class TestFunnelPerformanceTable(BaseProgramTest): + @pytest.mark.django_db + class TestProgramListView(BaseProgramTest): + @pytest.fixture(autouse=True) + def test_setup(self): + self.program = ProgramFactory.create(organization=self.organization) + self.list_url = reverse( + "program:funnel_performance_table", kwargs={"org_slug": self.organization.slug, "pk": self.program.id} + ) + + nm_org = OrganizationFactory.create() + opp = ManagedOpportunityFactory.create(program=self.program, organization=nm_org) + users = UserFactory.create_batch(5) + for index, user in enumerate(users): + access = OpportunityAccessFactory.create(opportunity=opp, user=user, invited_date=now()) + AssessmentFactory.create(opportunity=opp, user=user, opportunity_access=access) + visit_status = VisitValidationStatus.pending if index < 9 else VisitValidationStatus.trial + UserVisitFactory.create( + user=user, + opportunity=opp, + status=visit_status, + opportunity_access=access, + visit_date=now() + timedelta(3), + ) diff --git a/commcare_connect/program/urls.py b/commcare_connect/program/urls.py index 53209232..4e93c995 100644 --- a/commcare_connect/program/urls.py +++ b/commcare_connect/program/urls.py @@ -1,12 +1,15 @@ from django.urls import path from commcare_connect.program.views import ( + FunnelPerformanceTableView, ManagedOpportunityInit, ManagedOpportunityList, ProgramApplicationList, ProgramCreateOrUpdate, ProgramList, apply_or_decline_application, + dashboard, + delivery_table, invite_organization, manage_application, ) @@ -26,4 +29,7 @@ view=apply_or_decline_application, name="apply_or_decline_application", ), + path("/dashboard", dashboard, name="dashboard"), + path("/funnel-performance-table", FunnelPerformanceTableView.as_view(), name="funnel_performance_table"), + path("/delivery-performance-table", delivery_table, name="delivery_performance_table"), ] diff --git a/commcare_connect/program/views.py b/commcare_connect/program/views.py index 4c23d224..70850d9b 100644 --- a/commcare_connect/program/views.py +++ b/commcare_connect/program/views.py @@ -1,6 +1,6 @@ from django.contrib import messages from django.contrib.auth.mixins import LoginRequiredMixin, UserPassesTestMixin -from django.shortcuts import get_object_or_404, redirect +from django.shortcuts import get_object_or_404, redirect, render from django.urls import reverse from django.views.decorators.http import require_POST from django.views.generic import ListView, UpdateView @@ -10,8 +10,9 @@ from commcare_connect.organization.decorators import org_admin_required, org_program_manager_required from commcare_connect.organization.models import Organization from commcare_connect.program.forms import ManagedOpportunityInitForm, ProgramForm +from commcare_connect.program.helpers import get_annotated_managed_opportunity from commcare_connect.program.models import ManagedOpportunity, Program, ProgramApplication, ProgramApplicationStatus -from commcare_connect.program.tables import ProgramApplicationTable, ProgramTable +from commcare_connect.program.tables import FunnelPerformanceTable, ProgramApplicationTable, ProgramTable class ProgramManagerMixin(LoginRequiredMixin, UserPassesTestMixin): @@ -232,3 +233,31 @@ def apply_or_decline_application(request, application_id, action, org_slug=None, messages.success(request, action_map[action]["message"]) return redirect(redirect_url) + + +@org_program_manager_required +def dashboard(request, **kwargs): + program = get_object_or_404(Program, id=kwargs.get("pk"), organization=request.org) + context = { + "program": program, + } + return render(request, "program/dashboard.html", context) + + +class FunnelPerformanceTableView(ProgramManagerMixin, SingleTableView): + model = ManagedOpportunity + paginate_by = 10 + table_class = FunnelPerformanceTable + template_name = "tables/single_table.html" + + def get_queryset(self): + program_id = self.kwargs["pk"] + program = get_object_or_404(Program, id=program_id) + return get_annotated_managed_opportunity(program) + + +@org_program_manager_required +def delivery_table(request, **kwargs): + manage_opps = ManagedOpportunity.objects.filter(program__id=kwargs.get("pk")) + delivery_performance_table = FunnelPerformanceTable(manage_opps) + return render(request, "tables/single_table.html", {"table": delivery_performance_table}) diff --git a/commcare_connect/templates/program/dashboard.html b/commcare_connect/templates/program/dashboard.html new file mode 100644 index 00000000..7234964d --- /dev/null +++ b/commcare_connect/templates/program/dashboard.html @@ -0,0 +1,63 @@ +{% extends "program/base.html" %} +{% load static %} +{% load i18n %} +{% load django_tables2 %} +{% block title %}{{ request.org }} - Programs{% endblock %} + +{% block breadcrumbs_inner %} +{{ block.super }} +
  • {{ program.name }}
    • +
  • {% translate "Dashboard" %}
    • +{% endblock %} +{% block content %} +
    +
    +

    Dashboard

    +
    +
    +
      + + + +
    +
    +
    +
    + {% include "tables/table_placeholder.html" with num_cols=6 %} +
    +
    +
    +
    + {% include "tables/table_placeholder.html" with num_cols=7 %} +
    +
    +
    +
    +
    +{% endblock content %} From b73e31fbfc8a577c4ec9af500b886d484f46da33 Mon Sep 17 00:00:00 2001 From: hemant10yadav Date: Mon, 16 Sep 2024 16:05:28 +0530 Subject: [PATCH 2/9] refactor code --- commcare_connect/program/tables.py | 47 +----------------- .../program/tests/test_helpers.py | 49 +++++++++---------- commcare_connect/program/urls.py | 4 +- .../templates/program/dashboard.html | 26 +--------- 4 files changed, 27 insertions(+), 99 deletions(-) diff --git a/commcare_connect/program/tables.py b/commcare_connect/program/tables.py index b3214703..437d4aa2 100644 --- a/commcare_connect/program/tables.py +++ b/commcare_connect/program/tables.py @@ -238,7 +238,7 @@ class FunnelPerformanceTable(tables.Table): workers_invited = tables.Column(verbose_name=_("Workers Invited")) workers_passing_assessment = tables.Column(verbose_name=_("Workers Passing Assessment")) workers_starting_delivery = tables.Column(verbose_name=_("Workers Starting Delivery")) - percentage_conversion = tables.Column(verbose_name=_("% Conversion")) + percentage_conversion = tables.Column(verbose_name=_("Percentage Conversion")) average_time_to_convert = tables.Column(verbose_name=_("Average Time To convert")) class Meta: @@ -259,48 +259,3 @@ def render_average_time_to_convert(self, record): total_seconds = record.average_time_to_convert.total_seconds() hours = total_seconds / 3600 return f"{round(hours, 2)}hr" - - -class DeliveryPerformanceTable(tables.Table): - organization = tables.Column() - start_date = tables.DateColumn() - workers_invited = tables.Column( - empty_values=(), - verbose_name=_("Workers Invited"), - ) - workers_starting_delivery = tables.Column(empty_values=(), verbose_name=_("Total Workers Starting Delivery")) - active_workers = tables.Column(empty_values=(), verbose_name=_("Active Workers")) - deliveries_per_day_per_worker = tables.Column(empty_values=(), verbose_name=_("Deliveries Per Day Per Worker")) - percentage_records_flagged = tables.Column(empty_values=(), verbose_name="% Records Flagged") - - class Meta: - model = ManagedOpportunity - empty_text = "No data available yet." - fields = ( - "organization", - "start_date", - "Total Workers Starting Delivery", - "Active Workers", - "Deliveries Per Day Per Worker", - "% Records Flagged", - ) - orderable = False - - @staticmethod - def get_queryset(): - queryset = ManagedOpportunity.objects.prefetch_related("opportunityaccess_set") - return queryset - - def precompute_data(self, record): - """Precompute the values for the record and store them in the record.""" - if not hasattr(record, "_precomputed_data"): - data = { - "workers_starting_delivery": 0, - } - for access in record.opportunityaccess_set.all(): - if access.last_visit_date: - data["workers_starting_delivery"] += 1 - - record._precomputed_data = data - - return record._precomputed_data diff --git a/commcare_connect/program/tests/test_helpers.py b/commcare_connect/program/tests/test_helpers.py index 04d4a168..8339976f 100644 --- a/commcare_connect/program/tests/test_helpers.py +++ b/commcare_connect/program/tests/test_helpers.py @@ -1,37 +1,34 @@ from datetime import timedelta -import pytest -from django.urls import reverse from django_celery_beat.utils import now from commcare_connect.opportunity.models import VisitValidationStatus from commcare_connect.opportunity.tests.factories import AssessmentFactory, OpportunityAccessFactory, UserVisitFactory +from commcare_connect.organization.models import Organization +from commcare_connect.program.helpers import get_annotated_managed_opportunity from commcare_connect.program.tests.factories import ManagedOpportunityFactory, ProgramFactory -from commcare_connect.program.tests.test_views import BaseProgramTest from commcare_connect.users.tests.factories import OrganizationFactory, UserFactory -class TestFunnelPerformanceTable(BaseProgramTest): - @pytest.mark.django_db - class TestProgramListView(BaseProgramTest): - @pytest.fixture(autouse=True) - def test_setup(self): - self.program = ProgramFactory.create(organization=self.organization) - self.list_url = reverse( - "program:funnel_performance_table", kwargs={"org_slug": self.organization.slug, "pk": self.program.id} - ) +def test_get_annotated_managed_opportunity(program_manager_org: Organization): + program = ProgramFactory.create(organization=program_manager_org) + nm_org = OrganizationFactory.create() + opp = ManagedOpportunityFactory.create(program=program, organization=nm_org) + users = UserFactory.create_batch(5) + for index, user in enumerate(users): + access = OpportunityAccessFactory.create(opportunity=opp, user=user, invited_date=now()) + AssessmentFactory.create(opportunity=opp, user=user, opportunity_access=access) + visit_status = VisitValidationStatus.pending if index < 3 else VisitValidationStatus.trial + UserVisitFactory.create( + user=user, + opportunity=opp, + status=visit_status, + opportunity_access=access, + visit_date=now() + timedelta(1), + ) - nm_org = OrganizationFactory.create() - opp = ManagedOpportunityFactory.create(program=self.program, organization=nm_org) - users = UserFactory.create_batch(5) - for index, user in enumerate(users): - access = OpportunityAccessFactory.create(opportunity=opp, user=user, invited_date=now()) - AssessmentFactory.create(opportunity=opp, user=user, opportunity_access=access) - visit_status = VisitValidationStatus.pending if index < 9 else VisitValidationStatus.trial - UserVisitFactory.create( - user=user, - opportunity=opp, - status=visit_status, - opportunity_access=access, - visit_date=now() + timedelta(3), - ) + opps = get_annotated_managed_opportunity(program) + for opp in opps: + assert nm_org.slug == opp.organization.slug + assert opp.workers_passing_assessment == 5 + assert opp.workers_starting_delivery == 3 diff --git a/commcare_connect/program/urls.py b/commcare_connect/program/urls.py index 4e93c995..339771bb 100644 --- a/commcare_connect/program/urls.py +++ b/commcare_connect/program/urls.py @@ -9,7 +9,6 @@ ProgramList, apply_or_decline_application, dashboard, - delivery_table, invite_organization, manage_application, ) @@ -30,6 +29,5 @@ name="apply_or_decline_application", ), path("/dashboard", dashboard, name="dashboard"), - path("/funnel-performance-table", FunnelPerformanceTableView.as_view(), name="funnel_performance_table"), - path("/delivery-performance-table", delivery_table, name="delivery_performance_table"), + path("/funnel_performance_table", FunnelPerformanceTableView.as_view(), name="funnel_performance_table"), ] diff --git a/commcare_connect/templates/program/dashboard.html b/commcare_connect/templates/program/dashboard.html index 7234964d..c5c7d895 100644 --- a/commcare_connect/templates/program/dashboard.html +++ b/commcare_connect/templates/program/dashboard.html @@ -12,7 +12,7 @@ {% block content %}
    -

    Dashboard

    +

    {% trans "Dashboard" %}

      @@ -23,20 +23,7 @@

      Dashboard

      {% trans "Funnel Performance" %} - - +
    Dashboard {% include "tables/table_placeholder.html" with num_cols=6 %}
    -
    -
    - {% include "tables/table_placeholder.html" with num_cols=7 %} -
    -
    From 0c02dc370db9617d5a6177af62d351a33947f9cf Mon Sep 17 00:00:00 2001 From: hemant10yadav Date: Mon, 16 Sep 2024 16:18:14 +0530 Subject: [PATCH 3/9] Removed unused code --- commcare_connect/program/helpers.py | 15 --------------- commcare_connect/program/views.py | 7 ------- 2 files changed, 22 deletions(-) diff --git a/commcare_connect/program/helpers.py b/commcare_connect/program/helpers.py index 7f961b74..a9bcd9f7 100644 --- a/commcare_connect/program/helpers.py +++ b/commcare_connect/program/helpers.py @@ -54,18 +54,3 @@ def get_annotated_managed_opportunity(program: Program): ) return managed_opportunities - - -def get_annotated_managed_opportunity_nm(program: Program, start_date=None, end_date=None): - managed_opportunities = ( - ManagedOpportunity.objects.filter(program=program, start_date__gte=start_date) - .order_by("start_date") - .annotate() - .prefetch_related( - "opportunityaccess_set", - "opportunityaccess_set__uservisit_set", - "opportunityaccess_set__assessment_set", - ) - ) - - return managed_opportunities diff --git a/commcare_connect/program/views.py b/commcare_connect/program/views.py index 70850d9b..15db42a5 100644 --- a/commcare_connect/program/views.py +++ b/commcare_connect/program/views.py @@ -254,10 +254,3 @@ def get_queryset(self): program_id = self.kwargs["pk"] program = get_object_or_404(Program, id=program_id) return get_annotated_managed_opportunity(program) - - -@org_program_manager_required -def delivery_table(request, **kwargs): - manage_opps = ManagedOpportunity.objects.filter(program__id=kwargs.get("pk")) - delivery_performance_table = FunnelPerformanceTable(manage_opps) - return render(request, "tables/single_table.html", {"table": delivery_performance_table}) From be2ba7d2245a151456c255362355f9e273ca670a Mon Sep 17 00:00:00 2001 From: hemant10yadav Date: Wed, 18 Sep 2024 15:53:44 +0530 Subject: [PATCH 4/9] fixed migration sequence --- ...invited_date.py => 0059_opportunityaccess_invited_date.py} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename commcare_connect/opportunity/migrations/{0055_opportunityaccess_invited_date.py => 0059_opportunityaccess_invited_date.py} (70%) diff --git a/commcare_connect/opportunity/migrations/0055_opportunityaccess_invited_date.py b/commcare_connect/opportunity/migrations/0059_opportunityaccess_invited_date.py similarity index 70% rename from commcare_connect/opportunity/migrations/0055_opportunityaccess_invited_date.py rename to commcare_connect/opportunity/migrations/0059_opportunityaccess_invited_date.py index b55f94d5..1382f847 100644 --- a/commcare_connect/opportunity/migrations/0055_opportunityaccess_invited_date.py +++ b/commcare_connect/opportunity/migrations/0059_opportunityaccess_invited_date.py @@ -1,11 +1,11 @@ -# Generated by Django 4.2.5 on 2024-09-12 18:13 +# Generated by Django 4.2.5 on 2024-09-18 10:16 from django.db import migrations, models class Migration(migrations.Migration): dependencies = [ - ("opportunity", "0054_opportunity_managed_alter_opportunity_organization"), + ("opportunity", "0058_paymentinvoice_payment_invoice"), ] operations = [ From 4f88e2aa8a84e76736c6954c1e540b7857143581 Mon Sep 17 00:00:00 2001 From: hemant10yadav Date: Mon, 7 Oct 2024 10:27:00 +0530 Subject: [PATCH 5/9] fixed code review issues and added more tests --- commcare_connect/program/helpers.py | 44 ++++-- .../program/tests/test_helpers.py | 139 +++++++++++++++--- 2 files changed, 155 insertions(+), 28 deletions(-) diff --git a/commcare_connect/program/helpers.py b/commcare_connect/program/helpers.py index a9bcd9f7..086b7e10 100644 --- a/commcare_connect/program/helpers.py +++ b/commcare_connect/program/helpers.py @@ -1,22 +1,44 @@ -from django.db.models import Avg, Count, DurationField, ExpressionWrapper, F, OuterRef, Q, Subquery +from django.db.models import ( + Avg, + Case, + Count, + DurationField, + ExpressionWrapper, + F, + FloatField, + OuterRef, + Q, + Subquery, + Value, + When, +) +from django.db.models.functions import Cast, Round from commcare_connect.opportunity.models import UserVisit, VisitValidationStatus from commcare_connect.program.models import ManagedOpportunity, Program -def get_annotated_managed_opportunity(program: Program): - filter_for_valid__visit_date = ~Q( - opportunityaccess__uservisit__status__in=[ - VisitValidationStatus.over_limit, - VisitValidationStatus.trial, - ] +def calculate_safe_percentage(numerator, denominator): + return Case( + When(**{denominator: 0}, then=Value(0)), # Handle division by zero + default=Round(Cast(F(numerator), FloatField()) / Cast(F(denominator), FloatField()) * 100, 2), + output_field=FloatField(), ) + +def get_annotated_managed_opportunity(program: Program): + excluded_status = [ + VisitValidationStatus.over_limit, + VisitValidationStatus.trial, + ] + + filter_for_valid__visit_date = ~Q(opportunityaccess__uservisit__status__in=excluded_status) + earliest_visits = ( UserVisit.objects.filter( opportunity_access=OuterRef("opportunityaccess"), ) - .exclude(status__in=[VisitValidationStatus.over_limit, VisitValidationStatus.trial]) + .exclude(status__in=excluded_status) .order_by("visit_date") .values("visit_date")[:1] ) @@ -25,20 +47,20 @@ def get_annotated_managed_opportunity(program: Program): ManagedOpportunity.objects.filter(program=program) .order_by("start_date") .annotate( - workers_invited=Count("opportunityaccess"), + workers_invited=Count("opportunityaccess", distinct=True), workers_passing_assessment=Count( "opportunityaccess__assessment", filter=Q( opportunityaccess__assessment__passed=True, - opportunityaccess__assessment__opportunity=F("opportunityaccess__opportunity"), ), + distinct=True, ), workers_starting_delivery=Count( "opportunityaccess__uservisit__user", filter=filter_for_valid__visit_date, distinct=True, ), - percentage_conversion=F("workers_starting_delivery") / F("workers_invited") * 100, + percentage_conversion=calculate_safe_percentage("workers_starting_delivery", "workers_invited"), average_time_to_convert=Avg( ExpressionWrapper( Subquery(earliest_visits) - F("opportunityaccess__invited_date"), output_field=DurationField() diff --git a/commcare_connect/program/tests/test_helpers.py b/commcare_connect/program/tests/test_helpers.py index 8339976f..150aa305 100644 --- a/commcare_connect/program/tests/test_helpers.py +++ b/commcare_connect/program/tests/test_helpers.py @@ -1,34 +1,139 @@ from datetime import timedelta +import pytest from django_celery_beat.utils import now from commcare_connect.opportunity.models import VisitValidationStatus from commcare_connect.opportunity.tests.factories import AssessmentFactory, OpportunityAccessFactory, UserVisitFactory -from commcare_connect.organization.models import Organization from commcare_connect.program.helpers import get_annotated_managed_opportunity from commcare_connect.program.tests.factories import ManagedOpportunityFactory, ProgramFactory from commcare_connect.users.tests.factories import OrganizationFactory, UserFactory -def test_get_annotated_managed_opportunity(program_manager_org: Organization): - program = ProgramFactory.create(organization=program_manager_org) - nm_org = OrganizationFactory.create() - opp = ManagedOpportunityFactory.create(program=program, organization=nm_org) - users = UserFactory.create_batch(5) - for index, user in enumerate(users): - access = OpportunityAccessFactory.create(opportunity=opp, user=user, invited_date=now()) - AssessmentFactory.create(opportunity=opp, user=user, opportunity_access=access) - visit_status = VisitValidationStatus.pending if index < 3 else VisitValidationStatus.trial +class TestGetAnnotatedManagedOpportunity: + @pytest.fixture(autouse=True) + def setup(self, db): + self.program = ProgramFactory.create() + self.nm_org = OrganizationFactory.create() + self.opp = ManagedOpportunityFactory.create(program=self.program, organization=self.nm_org) + + def create_user_with_access(self, visit_status=VisitValidationStatus.pending, passed_assessment=True): + user = UserFactory.create() + access = OpportunityAccessFactory.create(opportunity=self.opp, user=user, invited_date=now()) + AssessmentFactory.create(opportunity=self.opp, user=user, opportunity_access=access, passed=passed_assessment) UserVisitFactory.create( user=user, - opportunity=opp, + opportunity=self.opp, status=visit_status, opportunity_access=access, - visit_date=now() + timedelta(1), + visit_date=now() + timedelta(days=1), + ) + return user + + def test_basic_scenario(self): + for i in range(5): + self.create_user_with_access( + visit_status=VisitValidationStatus.pending if i < 3 else VisitValidationStatus.trial + ) + + opps = get_annotated_managed_opportunity(self.program) + assert len(opps) == 1 + annotated_opp = opps[0] + assert annotated_opp.organization.slug == self.nm_org.slug + assert annotated_opp.workers_invited == 5 + assert annotated_opp.workers_passing_assessment == 5 + assert annotated_opp.workers_starting_delivery == 3 + assert annotated_opp.percentage_conversion == 60.0 + + def test_empty_scenario(self): + opps = get_annotated_managed_opportunity(self.program) + assert len(opps) == 1 + annotated_opp = opps[0] + assert annotated_opp.workers_invited == 0 + assert annotated_opp.workers_passing_assessment == 0 + assert annotated_opp.workers_starting_delivery == 0 + assert annotated_opp.percentage_conversion == 0.0 + assert annotated_opp.average_time_to_convert is None + + def test_multiple_visits(self): + user = self.create_user_with_access() + UserVisitFactory.create_batch( + 2, + user=user, + opportunity=self.opp, + status=VisitValidationStatus.pending, + opportunity_access=user.opportunityaccess_set.first(), + visit_date=now() + timedelta(days=2), ) - opps = get_annotated_managed_opportunity(program) - for opp in opps: - assert nm_org.slug == opp.organization.slug - assert opp.workers_passing_assessment == 5 - assert opp.workers_starting_delivery == 3 + opps = get_annotated_managed_opportunity(self.program) + assert len(opps) == 1 + annotated_opp = opps[0] + assert annotated_opp.workers_invited == 1 + assert annotated_opp.workers_passing_assessment == 1 + assert annotated_opp.workers_starting_delivery == 1 + assert annotated_opp.percentage_conversion == 100.0 + + def test_excluded_statuses(self): + self.create_user_with_access(visit_status=VisitValidationStatus.over_limit) + self.create_user_with_access(visit_status=VisitValidationStatus.trial) + + opps = get_annotated_managed_opportunity(self.program) + assert len(opps) == 1 + annotated_opp = opps[0] + assert annotated_opp.workers_invited == 2 + assert annotated_opp.workers_passing_assessment == 2 + assert annotated_opp.workers_starting_delivery == 0 + assert annotated_opp.percentage_conversion == 0.0 + + def test_average_time_to_convert(self): + for i in range(3): + user = self.create_user_with_access() + user.opportunityaccess_set.update(invited_date=now() - timedelta(days=i)) + + opps = get_annotated_managed_opportunity(self.program) + assert len(opps) == 1 + annotated_opp = opps[0] + expected_time = timedelta(days=2) + actual_time = annotated_opp.average_time_to_convert + assert abs(actual_time - expected_time) < timedelta(seconds=5) + + def test_multiple_opportunities(self): + nm_org2 = OrganizationFactory.create() + opp2 = ManagedOpportunityFactory.create( + program=self.program, organization=nm_org2, start_date=now() + timedelta(days=1) + ) + + self.create_user_with_access() + user2 = UserFactory.create() + access2 = OpportunityAccessFactory.create(opportunity=opp2, user=user2, invited_date=now()) + AssessmentFactory.create(opportunity=opp2, user=user2, opportunity_access=access2, passed=True) + UserVisitFactory.create( + user=user2, + opportunity=opp2, + status=VisitValidationStatus.pending, + opportunity_access=access2, + visit_date=now() + timedelta(days=1), + ) + + opps = get_annotated_managed_opportunity(self.program) + assert len(opps) == 2 + assert opps[0].organization.slug == self.nm_org.slug + assert opps[1].organization.slug == nm_org2.slug + for annotated_opp in opps: + assert annotated_opp.workers_invited == 1 + assert annotated_opp.workers_passing_assessment == 1 + assert annotated_opp.workers_starting_delivery == 1 + assert annotated_opp.percentage_conversion == 100.0 + + def test_failed_assessments(self): + self.create_user_with_access(passed_assessment=False) + self.create_user_with_access(passed_assessment=True) + + opps = get_annotated_managed_opportunity(self.program) + assert len(opps) == 1 + annotated_opp = opps[0] + assert annotated_opp.workers_invited == 2 + assert annotated_opp.workers_passing_assessment == 1 + assert annotated_opp.workers_starting_delivery == 2 + assert annotated_opp.percentage_conversion == 100.0 From 7075ad4a8b23c1b10cc76d6151660659393d1283 Mon Sep 17 00:00:00 2001 From: hemant10yadav Date: Mon, 7 Oct 2024 10:29:38 +0530 Subject: [PATCH 6/9] added check for none average time --- commcare_connect/program/tables.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/commcare_connect/program/tables.py b/commcare_connect/program/tables.py index 4e964d4c..e9cf0296 100644 --- a/commcare_connect/program/tables.py +++ b/commcare_connect/program/tables.py @@ -259,6 +259,8 @@ class Meta: orderable = False def render_average_time_to_convert(self, record): + if not record.average_time_to_convert: + return "---" total_seconds = record.average_time_to_convert.total_seconds() hours = total_seconds / 3600 return f"{round(hours, 2)}hr" From 56651e5afcf82d984a4807833be35d6f7e9a2a91 Mon Sep 17 00:00:00 2001 From: hemant10yadav Date: Mon, 7 Oct 2024 10:35:12 +0530 Subject: [PATCH 7/9] fixed migration sequence --- ...invited_date.py => 0060_opportunityaccess_invited_date.py} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename commcare_connect/opportunity/migrations/{0059_opportunityaccess_invited_date.py => 0060_opportunityaccess_invited_date.py} (74%) diff --git a/commcare_connect/opportunity/migrations/0059_opportunityaccess_invited_date.py b/commcare_connect/opportunity/migrations/0060_opportunityaccess_invited_date.py similarity index 74% rename from commcare_connect/opportunity/migrations/0059_opportunityaccess_invited_date.py rename to commcare_connect/opportunity/migrations/0060_opportunityaccess_invited_date.py index 1382f847..d96766df 100644 --- a/commcare_connect/opportunity/migrations/0059_opportunityaccess_invited_date.py +++ b/commcare_connect/opportunity/migrations/0060_opportunityaccess_invited_date.py @@ -1,11 +1,11 @@ -# Generated by Django 4.2.5 on 2024-09-18 10:16 +# Generated by Django 4.2.5 on 2024-10-07 05:04 from django.db import migrations, models class Migration(migrations.Migration): dependencies = [ - ("opportunity", "0058_paymentinvoice_payment_invoice"), + ("opportunity", "0059_payment_amount_usd"), ] operations = [ From f87aa0223bd7fcd6b7a3857c1051e023e876068e Mon Sep 17 00:00:00 2001 From: hemant10yadav Date: Tue, 15 Oct 2024 15:51:35 +0530 Subject: [PATCH 8/9] refactor tests --- commcare_connect/program/helpers.py | 5 - .../program/tests/test_helpers.py | 166 +++++++----------- 2 files changed, 63 insertions(+), 108 deletions(-) diff --git a/commcare_connect/program/helpers.py b/commcare_connect/program/helpers.py index 086b7e10..78802a34 100644 --- a/commcare_connect/program/helpers.py +++ b/commcare_connect/program/helpers.py @@ -68,11 +68,6 @@ def get_annotated_managed_opportunity(program: Program): filter=filter_for_valid__visit_date, ), ) - .prefetch_related( - "opportunityaccess_set", - "opportunityaccess_set__uservisit_set", - "opportunityaccess_set__assessment_set", - ) ) return managed_opportunities diff --git a/commcare_connect/program/tests/test_helpers.py b/commcare_connect/program/tests/test_helpers.py index 150aa305..de67f02c 100644 --- a/commcare_connect/program/tests/test_helpers.py +++ b/commcare_connect/program/tests/test_helpers.py @@ -30,110 +30,70 @@ def create_user_with_access(self, visit_status=VisitValidationStatus.pending, pa ) return user - def test_basic_scenario(self): - for i in range(5): - self.create_user_with_access( - visit_status=VisitValidationStatus.pending if i < 3 else VisitValidationStatus.trial - ) + @pytest.mark.parametrize( + "scenario, visit_statuses, passing_assessments, expected_invited," + " expected_passing, expected_delivery, expected_conversion", + [ + ( + "basic_scenario", + [VisitValidationStatus.pending, VisitValidationStatus.pending, VisitValidationStatus.trial], + [True, True, True], + 3, + 3, + 2, + 66.67, + ), + ("empty_scenario", [], [], 0, 0, 0, 0.0), + ("multiple_visits_scenario", [VisitValidationStatus.pending], [True], 1, 1, 1, 100.0), + ( + "excluded_statuses", + [VisitValidationStatus.over_limit, VisitValidationStatus.trial], + [True, True], + 2, + 2, + 0, + 0.0, + ), + ( + "failed_assessments", + [VisitValidationStatus.pending, VisitValidationStatus.pending], + [False, True], + 2, + 1, + 2, + 100.0, + ), + ], + ) + def test_scenarios( + self, + scenario, + visit_statuses, + passing_assessments, + expected_invited, + expected_passing, + expected_delivery, + expected_conversion, + ): + for i, visit_status in enumerate(visit_statuses): + user = self.create_user_with_access(visit_status=visit_status, passed_assessment=passing_assessments[i]) + + # For the "multiple_visits_scenario", create additional visits for the same user + if scenario == "multiple_visits_scenario": + access = user.opportunityaccess_set.first() + UserVisitFactory.create_batch( + 2, + user=user, + opportunity=self.opp, + status=VisitValidationStatus.pending, + opportunity_access=access, + visit_date=now() + timedelta(days=2), + ) opps = get_annotated_managed_opportunity(self.program) assert len(opps) == 1 annotated_opp = opps[0] - assert annotated_opp.organization.slug == self.nm_org.slug - assert annotated_opp.workers_invited == 5 - assert annotated_opp.workers_passing_assessment == 5 - assert annotated_opp.workers_starting_delivery == 3 - assert annotated_opp.percentage_conversion == 60.0 - - def test_empty_scenario(self): - opps = get_annotated_managed_opportunity(self.program) - assert len(opps) == 1 - annotated_opp = opps[0] - assert annotated_opp.workers_invited == 0 - assert annotated_opp.workers_passing_assessment == 0 - assert annotated_opp.workers_starting_delivery == 0 - assert annotated_opp.percentage_conversion == 0.0 - assert annotated_opp.average_time_to_convert is None - - def test_multiple_visits(self): - user = self.create_user_with_access() - UserVisitFactory.create_batch( - 2, - user=user, - opportunity=self.opp, - status=VisitValidationStatus.pending, - opportunity_access=user.opportunityaccess_set.first(), - visit_date=now() + timedelta(days=2), - ) - - opps = get_annotated_managed_opportunity(self.program) - assert len(opps) == 1 - annotated_opp = opps[0] - assert annotated_opp.workers_invited == 1 - assert annotated_opp.workers_passing_assessment == 1 - assert annotated_opp.workers_starting_delivery == 1 - assert annotated_opp.percentage_conversion == 100.0 - - def test_excluded_statuses(self): - self.create_user_with_access(visit_status=VisitValidationStatus.over_limit) - self.create_user_with_access(visit_status=VisitValidationStatus.trial) - - opps = get_annotated_managed_opportunity(self.program) - assert len(opps) == 1 - annotated_opp = opps[0] - assert annotated_opp.workers_invited == 2 - assert annotated_opp.workers_passing_assessment == 2 - assert annotated_opp.workers_starting_delivery == 0 - assert annotated_opp.percentage_conversion == 0.0 - - def test_average_time_to_convert(self): - for i in range(3): - user = self.create_user_with_access() - user.opportunityaccess_set.update(invited_date=now() - timedelta(days=i)) - - opps = get_annotated_managed_opportunity(self.program) - assert len(opps) == 1 - annotated_opp = opps[0] - expected_time = timedelta(days=2) - actual_time = annotated_opp.average_time_to_convert - assert abs(actual_time - expected_time) < timedelta(seconds=5) - - def test_multiple_opportunities(self): - nm_org2 = OrganizationFactory.create() - opp2 = ManagedOpportunityFactory.create( - program=self.program, organization=nm_org2, start_date=now() + timedelta(days=1) - ) - - self.create_user_with_access() - user2 = UserFactory.create() - access2 = OpportunityAccessFactory.create(opportunity=opp2, user=user2, invited_date=now()) - AssessmentFactory.create(opportunity=opp2, user=user2, opportunity_access=access2, passed=True) - UserVisitFactory.create( - user=user2, - opportunity=opp2, - status=VisitValidationStatus.pending, - opportunity_access=access2, - visit_date=now() + timedelta(days=1), - ) - - opps = get_annotated_managed_opportunity(self.program) - assert len(opps) == 2 - assert opps[0].organization.slug == self.nm_org.slug - assert opps[1].organization.slug == nm_org2.slug - for annotated_opp in opps: - assert annotated_opp.workers_invited == 1 - assert annotated_opp.workers_passing_assessment == 1 - assert annotated_opp.workers_starting_delivery == 1 - assert annotated_opp.percentage_conversion == 100.0 - - def test_failed_assessments(self): - self.create_user_with_access(passed_assessment=False) - self.create_user_with_access(passed_assessment=True) - - opps = get_annotated_managed_opportunity(self.program) - assert len(opps) == 1 - annotated_opp = opps[0] - assert annotated_opp.workers_invited == 2 - assert annotated_opp.workers_passing_assessment == 1 - assert annotated_opp.workers_starting_delivery == 2 - assert annotated_opp.percentage_conversion == 100.0 + assert annotated_opp.workers_invited == expected_invited, f"Failed in {scenario}" + assert annotated_opp.workers_passing_assessment == expected_passing, f"Failed in {scenario}" + assert annotated_opp.workers_starting_delivery == expected_delivery, f"Failed in {scenario}" + assert pytest.approx(annotated_opp.percentage_conversion, 0.01) == expected_conversion, f"Failed in {scenario}" From 98f0a2ef5919eda10ed4f02ca7ee5abbbd4175f4 Mon Sep 17 00:00:00 2001 From: hemant10yadav Date: Tue, 15 Oct 2024 15:56:19 +0530 Subject: [PATCH 9/9] Resolved migration conflict --- ...invited_date.py => 0061_opportunityaccess_invited_date.py} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename commcare_connect/opportunity/migrations/{0060_opportunityaccess_invited_date.py => 0061_opportunityaccess_invited_date.py} (74%) diff --git a/commcare_connect/opportunity/migrations/0060_opportunityaccess_invited_date.py b/commcare_connect/opportunity/migrations/0061_opportunityaccess_invited_date.py similarity index 74% rename from commcare_connect/opportunity/migrations/0060_opportunityaccess_invited_date.py rename to commcare_connect/opportunity/migrations/0061_opportunityaccess_invited_date.py index d96766df..b5cebef6 100644 --- a/commcare_connect/opportunity/migrations/0060_opportunityaccess_invited_date.py +++ b/commcare_connect/opportunity/migrations/0061_opportunityaccess_invited_date.py @@ -1,11 +1,11 @@ -# Generated by Django 4.2.5 on 2024-10-07 05:04 +# Generated by Django 4.2.5 on 2024-10-15 10:25 from django.db import migrations, models class Migration(migrations.Migration): dependencies = [ - ("opportunity", "0059_payment_amount_usd"), + ("opportunity", "0060_completedwork_payment_date"), ] operations = [