Category count cleanup

temba/flows/models.py

@@ -391,47 +391,49 @@ def get_attrs(self):
         return {"icon": self.TYPE_ICONS.get(self.flow_type, "flow"), "type": self.flow_type, "uuid": self.uuid}
 
     def get_category_counts(self):
-        keys = [r["key"] for r in self.metadata["results"]]
+        # get the possible results from the flow metadata
+        results_by_key = {r["key"]: r for r in self.metadata["results"]}
+
         counts = (
-            FlowCategoryCount.objects.filter(flow_id=self.id)
-            .filter(result_key__in=keys)
+            self.category_counts.filter(result_key__in=results_by_key)
             .values("result_key", "category_name")
-            .annotate(count=Sum("count"), result_name=Max("result_name"))
+            .annotate(total=Sum("count"))
         )
 
-        results = {}
+        # organize into dict of results keys to dicts of category names to counts
+        counts_by_key = defaultdict(dict)
         for count in counts:
-            key = count["result_key"]
-            result = results.get(key, {})
-            if "name" not in result:
-                if count["category_name"] == "All Responses":
-                    continue
-                result["key"] = key
-                result["name"] = count["result_name"]
-                result["categories"] = [dict(name=count["category_name"], count=count["count"])]
-                result["total"] = count["count"]
-            else:
-                result["categories"].append(dict(name=count["category_name"], count=count["count"]))
-                result["total"] += count["count"]
-            results[count["result_key"]] = result
-
-        for result_key, result_dict in results.items():
-            for cat in result_dict["categories"]:
-                if result_dict["total"]:
-                    cat["pct"] = float(cat["count"]) / float(result_dict["total"])
-                else:
-                    cat["pct"] = 0
-
-            result_dict["categories"] = sorted(result_dict["categories"], key=lambda d: d["name"])
-
-        # order counts by their place on the flow
-        result_list = []
-        for key in keys:
-            result = results.get(key)
-            if result:
-                result_list.append(result)
-
-        return result_list
+            counts_by_key[count["result_key"]][count["category_name"]] = count["total"]
+
+        results = []
+        for result_key, result in results_by_key.items():
+            category_counts = counts_by_key.get(result_key, {})
+
+            # TODO maybe we shouldn't store All Responses in the first place
+            if not category_counts or (len(category_counts) == 1 and "All Responses" in category_counts):
+                continue
+
+            result_total = sum(category_counts.values())
+            result_categories = []
+            for cat_name, cat_count in category_counts.items():
+                result_categories.append(
+                    {
+                        "name": cat_name,
+                        "count": cat_count,
+                        "pct": (float(cat_count) / float(result_total)) if result_total else 0,
+                    }
+                )
+
+            result_summary = {
+                "key": result_key,
+                "name": result["name"],
+                "categories": sorted(result_categories, key=lambda c: c["name"]),
+                "total": result_total,
+            }
+
+            results.append(result_summary)
+
+        return results
 
     def lock(self):
         """

temba/flows/tests.py

@@ -671,47 +671,6 @@ def test_flow_update_of_inactive_flow(self):
         # can't delete already released flow
         self.assertEqual(response.status_code, 404)
 
-    def test_flow_results_of_inactive_flow(self):
-        flow = self.get_flow("favorites")
-        flow.release(self.admin)
-
-        self.login(self.admin)
-        response = self.client.get(reverse("flows.flow_results", args=[flow.uuid]))
-
-        self.assertEqual(response.status_code, 404)
-
-    def test_flow_results_with_hidden_results(self):
-        flow = self.get_flow("color_v13")
-        flow_nodes = flow.get_definition()["nodes"]
-        color_split = flow_nodes[4]
-
-        # add a spec for a hidden result to this flow.. which should not be included below
-        flow.metadata[Flow.METADATA_RESULTS].append(
-            {
-                "key": "_color_classification",
-                "name": "_Color Classification",
-                "categories": ["Success", "Skipped", "Failure"],
-                "node_uuids": [color_split["uuid"]],
-            }
-        )
-
-        self.login(self.admin)
-        response = self.client.get(reverse("flows.flow_results", args=[flow.uuid]))
-
-        self.assertEqual(response.status_code, 200)
-        self.assertEqual(
-            response.context["result_fields"],
-            [
-                {
-                    "key": "color",
-                    "name": "Color",
-                    "categories": ["Orange", "Blue", "Other", "Nothing"],
-                    "node_uuids": [color_split["uuid"]],
-                    "has_categories": "true",
-                }
-            ],
-        )
-
     def test_importing_dependencies(self):
         # create channel to be matched by name
         channel = self.create_channel("TG", "RapidPro Test", "12345324635")
@@ -1298,7 +1257,7 @@ def test_views(self):
         self.assertRedirect(response, reverse("flows.flow_editor", args=[flow3.uuid]))
 
         # can see results for a flow
-        response = self.client.get(reverse("flows.flow_results", args=[flow.uuid]))
+        response = self.client.get(reverse("flows.flow_results", args=[flow.id]))
         self.assertEqual(200, response.status_code)
 
         # check flow listing
@@ -2354,7 +2313,7 @@ def add_recent_contact(exit_uuid: str, dest_uuid: str, contact, text: str, ts: f
     def test_category_counts(self):
         flow1 = self.create_flow("Test 1")
 
-        counts_url = reverse("flows.flow_category_counts", args=[flow1.uuid])
+        counts_url = reverse("flows.flow_category_counts", args=[flow1.id])
 
         self.assertRequestDisallowed(counts_url, [None, self.agent])
 
@@ -2422,6 +2381,19 @@ def test_category_counts(self):
             response.json(),
         )
 
+    def test_results(self):
+        flow = self.create_flow("Test 1")
+
+        results_url = reverse("flows.flow_results", args=[flow.id])
+
+        self.assertRequestDisallowed(results_url, [None, self.agent])
+        self.assertReadFetch(results_url, [self.user, self.editor, self.admin])
+
+        flow.release(self.admin)
+
+        response = self.requestView(results_url, self.admin)
+        self.assertEqual(404, response.status_code)
+
     @patch("django.utils.timezone.now")
     def test_engagement(self, mock_now):
         # this test runs as if it's 2024-11-25 12:05:00

temba/flows/views.py

@@ -826,7 +826,7 @@ def build_context_menu(self, menu):
                 )
 
             if self.has_org_perm("flows.flow_results"):
-                menu.add_link(_("Results"), reverse("flows.flow_results", args=[obj.uuid]))
+                menu.add_link(_("Results"), reverse("flows.flow_results", args=[obj.id]))
 
             menu.new_group()
 
@@ -1121,6 +1121,10 @@ def create_export(self, org, user, form):
             )
 
     class Engagement(BaseReadView):
+        """
+        Data for charts on engagement tab of results page.
+        """
+
         permission = "flows.flow_results"
 
         def render_to_response(self, context, **response_kwargs):
@@ -1216,46 +1220,31 @@ def render_to_response(self, context, **response_kwargs):
 
     class CategoryCounts(BaseReadView):
         """
-        Used by the editor for the counts on split exits
+        Data for charts on analytics tab of results page.
         """
 
-        permission = "flows.flow_editor"
-        slug_url_kwarg = "uuid"
+        permission = "flows.flow_results"
 
         def render_to_response(self, context, **response_kwargs):
             return JsonResponse({"counts": self.object.get_category_counts()})
 
     class Results(SpaMixin, ContextMenuMixin, BaseReadView):
-        slug_url_kwarg = "uuid"
-
         def build_context_menu(self, menu):
             obj = self.get_object()
 
             if self.has_org_perm("flows.flow_editor"):
                 menu.add_link(_("Editor"), reverse("flows.flow_editor", args=[obj.uuid]), as_button=True)
 
-            if self.has_org_perm("flows.flow_results"):
-                menu.add_modax(
-                    _("Export"),
-                    "export-results",
-                    f"{reverse('flows.flow_export_results')}?ids={obj.id}",
-                    title=_("Export Results"),
-                )
+            menu.add_modax(
+                _("Export"),
+                "export-results",
+                f"{reverse('flows.flow_export_results')}?ids={obj.id}",
+                title=_("Export Results"),
+            )
 
         def get_context_data(self, *args, **kwargs):
             context = super().get_context_data(*args, **kwargs)
-            flow = self.object
-
-            result_fields = []
-            for result_field in flow.metadata[Flow.METADATA_RESULTS]:
-                if not result_field["name"].startswith("_"):
-                    result_field = result_field.copy()
-                    result_field["has_categories"] = "true" if len(result_field["categories"]) > 1 else "false"
-                    result_fields.append(result_field)
-            context["result_fields"] = result_fields
-
-            context["categories"] = flow.get_category_counts()
-            context["utcoffset"] = int(datetime.now(flow.org.timezone).utcoffset().total_seconds() // 60)
+            context["utcoffset"] = int(datetime.now(self.request.org.timezone).utcoffset().total_seconds() // 60)
             return context
 
     class Activity(BaseReadView):

templates/flows/flow_list.html

@@ -78,11 +78,11 @@
                 {% endif %}
                 {% if object.run_stats.total %}
                   <div onclick="goto(event)"
-                       href="{% url "flows.flow_results" object.uuid %}"
+                       href="{% url "flows.flow_results" object.id %}"
                        class="linked mr-2 whitespace-nowrap">{{ object.run_stats.total|intcomma }}</div>
                   /
                   <div onclick="goto(event)"
-                       href="{% url "flows.flow_results" object.uuid %}"
+                       href="{% url "flows.flow_results" object.id %}"
                        class="text-center linked mx-2 whitespace-nowrap">{{ object.run_stats.completion }}%</div>
                 {% endif %}
               </div>

templates/flows/flow_results.html

@@ -304,7 +304,7 @@
         return;
       }
 
-      store.getUrl("/flow/category_counts/{{object.uuid}}/", {
+      store.getUrl("/flow/category_counts/{{object.id}}/", {
         force: true
       }).then(function(response) {
         var data = response.json;