From 592d7556d05e70a8d026f6c4915efac6041a96cf Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Thu, 28 Nov 2024 21:28:42 +0000 Subject: [PATCH 1/2] Cleanup fetching of category counts for results page analytics tab --- temba/flows/models.py | 74 ++++++++++++++++++++++--------------------- temba/flows/tests.py | 54 ++++++++----------------------- temba/flows/views.py | 21 ++++-------- 3 files changed, 58 insertions(+), 91 deletions(-) diff --git a/temba/flows/models.py b/temba/flows/models.py index 16f8c85dea..61a65a948c 100644 --- a/temba/flows/models.py +++ b/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): """ diff --git a/temba/flows/tests.py b/temba/flows/tests.py index 08d2a750c1..238d31eac9 100644 --- a/temba/flows/tests.py +++ b/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") @@ -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.uuid]) + + 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 diff --git a/temba/flows/views.py b/temba/flows/views.py index a8fc70a3c9..d7760ccb88 100644 --- a/temba/flows/views.py +++ b/temba/flows/views.py @@ -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,10 +1220,10 @@ 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" + permission = "flows.flow_results" slug_url_kwarg = "uuid" def render_to_response(self, context, **response_kwargs): @@ -1244,18 +1248,7 @@ def build_context_menu(self, menu): 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): From 901b7b550b650841b9946dc879690f93911dc916 Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Thu, 28 Nov 2024 21:32:03 +0000 Subject: [PATCH 2/2] Use flow id for category and result views --- temba/flows/tests.py | 6 +++--- temba/flows/views.py | 18 +++++++----------- templates/flows/flow_list.html | 4 ++-- templates/flows/flow_results.html | 2 +- 4 files changed, 13 insertions(+), 17 deletions(-) diff --git a/temba/flows/tests.py b/temba/flows/tests.py index 238d31eac9..287e399806 100644 --- a/temba/flows/tests.py +++ b/temba/flows/tests.py @@ -1257,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 @@ -2313,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]) @@ -2384,7 +2384,7 @@ def test_category_counts(self): def test_results(self): flow = self.create_flow("Test 1") - results_url = reverse("flows.flow_results", args=[flow.uuid]) + 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]) diff --git a/temba/flows/views.py b/temba/flows/views.py index d7760ccb88..284853705b 100644 --- a/temba/flows/views.py +++ b/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() @@ -1224,27 +1224,23 @@ class CategoryCounts(BaseReadView): """ permission = "flows.flow_results" - slug_url_kwarg = "uuid" 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) diff --git a/templates/flows/flow_list.html b/templates/flows/flow_list.html index 31dd1dd696..7552aacd94 100644 --- a/templates/flows/flow_list.html +++ b/templates/flows/flow_list.html @@ -78,11 +78,11 @@ {% endif %} {% if object.run_stats.total %}
{{ object.run_stats.total|intcomma }}
/
{{ object.run_stats.completion }}%
{% endif %} diff --git a/templates/flows/flow_results.html b/templates/flows/flow_results.html index 527f8dd356..af6a4c3236 100644 --- a/templates/flows/flow_results.html +++ b/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;