Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Assigned Location Column to Application Status Report #34744

Merged
merged 20 commits into from
Jul 3, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
+++
@@ -1,5 +1,5 @@
'use strict';
-hqDefine("reports/js/bootstrap3/application_status", [
+hqDefine("reports/js/bootstrap5/application_status", [
"jquery",
], function (
$
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
+++
@@ -1,44 +1,44 @@
@@ -1,45 +1,45 @@
-{% extends "hqwebapp/bootstrap3/two_column.html" %}
+{% extends "hqwebapp/bootstrap5/two_column.html" %}
{% load compress %}
Expand Down Expand Up @@ -48,16 +48,18 @@
<script src="{% static 'reports/js/inspect_data.js' %}"></script>
- <script src="{% static 'reports/js/bootstrap3/project_health_dashboard.js' %}"></script>
- <script src="{% static 'reports/js/bootstrap3/aggregate_user_status.js' %}"></script>
- <script src="{% static 'reports/js/bootstrap3/application_status.js' %}"></script>
+ <script src="{% static 'reports/js/bootstrap5/project_health_dashboard.js' %}"></script>
+ <script src="{% static 'reports/js/bootstrap5/aggregate_user_status.js' %}"></script>
+ <script src="{% static 'reports/js/bootstrap5/application_status.js' %}"></script>
<script src="{% static 'reports/js/user_history.js' %}"></script>
<script src="{% static 'reports/js/case_activity.js' %}"></script>
- <script src="{% static 'hqwebapp/js/bootstrap3/widgets.js' %}"></script>
+ <script src="{% static 'hqwebapp/js/bootstrap5/widgets.js' %}"></script>
{% endcompress %}
{% endblock %}

@@ -58,8 +58,8 @@
@@ -59,8 +59,8 @@
{% block title %}{{ report.title|default:"Project Reports" }}{% endblock %}

{% block page_breadcrumbs %}
Expand All @@ -68,7 +70,7 @@
<li>
<a href="{{ report.default_url }}"><strong>{% trans report.section_name|default:"Reports" %}</strong></a>
</li>
@@ -98,17 +98,17 @@
@@ -99,17 +99,17 @@
{% initial_page_data 'slug' report.slug %}

{% block filter_panel %}
Expand All @@ -90,7 +92,7 @@
aria-label="Close"
data-bind="click: resetModal"><span aria-hidden="true">&times;</span></button>
<h4 class="modal-title">
@@ -120,14 +120,14 @@
@@ -121,14 +121,14 @@
{{ datespan.enddate|date:"Y-m-d" }}
</h4>
</div>
Expand All @@ -107,7 +109,7 @@
<h4>{% trans 'Notice' %}</h4>
<p>{{ report.special_notice }}</p>
</div>
@@ -137,7 +137,7 @@
@@ -138,7 +138,7 @@
{% block reportcontent %}
{% endblock %}
{% else %}
Expand Down
69 changes: 66 additions & 3 deletions corehq/apps/reports/standard/deployments.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
ProjectReportParametersMixin,
)
from corehq.apps.reports.util import format_datatables_data
from corehq.apps.users.models import CouchUser
from corehq.apps.users.util import user_display_string
from corehq.const import USER_DATE_FORMAT
from corehq.util.quickcache import quickcache
Expand Down Expand Up @@ -81,6 +82,10 @@ def _columns(self):
DataTablesColumn(_("Username"),
prop_name='username.exact',
sql_col='user_dim__username'),
DataTablesColumn(_("Assigned Location(s)"),
help_text=_('Assigned locations for the user, with the primary '
'location highlighted in bold.'),
sortable=False),
DataTablesColumn(_("Last Submission"),
prop_name='reporting_metadata.last_submissions.submission_date',
alt_prop_name='reporting_metadata.last_submission_for_user.submission_date',
Expand Down Expand Up @@ -119,7 +124,7 @@ def headers(self):
sortable=False)
)
headers = DataTablesHeader(*columns)
headers.custom_sort = [[1, 'desc']]
headers.custom_sort = [[2, 'desc']]
return headers

@cached_property
Expand Down Expand Up @@ -307,6 +312,23 @@ def include_location_data(self):
)
)

def locations_names_dict(self, user_docs):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: just hit me that we have user docs in two places, couch and ES. Would be nice if this was clear which one this is either by the name of the parameter or by the documentation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this could be a private function? and could ideally be after process_rows to keep readable order of functions (reference: Newspaper metaphor from Clean Code)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I can see how this can make reading the code a little more difficult. I've done a tiny refactor in d6a20e8

"""
Returns a dict of all assigned location names for given `user_docs`.
The dict has the following structure:
{
'loc_id': 'loc_name'
}
"""
all_loc_ids = set()
for user_doc in user_docs:
user = CouchUser.wrap_correctly(user_doc)
for loc_id in user.get_location_ids(self.domain):
all_loc_ids.add(loc_id)
return dict(SQLLocation.objects.filter(
location_id__in=all_loc_ids, domain=self.domain
).values_list('location_id', 'name'))

def process_rows(self, users, fmt_for_export=False):
rows = []
users = list(users)
Expand All @@ -316,6 +338,7 @@ def process_rows(self, users, fmt_for_export=False):
grouped_ancestor_locs = self.get_bulk_ancestors(location_ids)
self.required_loc_columns = self.get_location_columns(grouped_ancestor_locs)

loc_names_dict = self.locations_names_dict(users)
for user in users:
last_build = last_seen = last_sub = last_sync = last_sync_date = app_name = commcare_version = None
last_build_profile_name = device = device_app_meta = num_unsent_forms = None
Expand Down Expand Up @@ -373,6 +396,7 @@ def process_rows(self, users, fmt_for_export=False):
user_display_string(user.get('username', ''),
user.get('first_name', ''),
user.get('last_name', '')),
self.get_location_column(user, loc_names_dict),
_fmt_date(last_seen, fmt_for_export), _fmt_date(last_sync_date, fmt_for_export),
app_name or "---", build_version, commcare_version or '---',
num_unsent_forms if num_unsent_forms is not None else "---",
Expand Down Expand Up @@ -477,12 +501,51 @@ def _fmt_timestamp(timestamp):

for row in table[1:]:
# Last submission
row[len(location_colums) + 1] = _fmt_timestamp(row[len(location_colums) + 1])
# Last sync
row[len(location_colums) + 2] = _fmt_timestamp(row[len(location_colums) + 2])
# Last sync
row[len(location_colums) + 3] = _fmt_timestamp(row[len(location_colums) + 3])
result[0][1] = table
return result

def get_location_column(self, user_doc, user_loc_dict):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this could be a private function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in d6a20e8

user = CouchUser.wrap_correctly(user_doc)
if not user.get_location_ids(self.domain):
return '---'
return self._get_formatted_assigned_location_names(user, user_loc_dict)

def _get_formatted_assigned_location_names(self, user, user_loc_dict):
"""
Create an HTML formatted string of the given assigned location names.
The primary location will be highlighted in bold.
"""
assigned_location_ids = user.get_location_ids(self.domain)
primary_location_id = user.get_location_id(self.domain)
formatted_loc_names = []
for loc_id in assigned_location_ids:
loc_name = user_loc_dict.get(loc_id)
if loc_id == primary_location_id:
formatted_loc_names.insert(
0, f'<strong>{loc_name}</strong>'
)
else:
formatted_loc_names.append(loc_name)

formatted_str = ', '.join(formatted_loc_names[:4])
html_nodes = [
f'<span class="locations-list">{formatted_str}</span>',
]
if len(formatted_loc_names) > 4:
all_str = ', '.join(formatted_loc_names)
view_controls_html_nodes = [
f'<span class="loc-view-control">{_("...See more")}</span>',
f'<span class="loc-view-control" style="display:none">{_("...Collapse")}</span>',
]
html_nodes += [
f'<span class="all-locations-list" style="display:none">{all_str}</span>',
f'<a href="#" class="toggle-all-locations">{"".join(view_controls_html_nodes)}</a>',
]
return format_html(f'<div>{"".join(html_nodes)}</div>')


def _get_commcare_version(app_version_info):
commcare_version = (
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
'use strict';
hqDefine("reports/js/bootstrap3/application_status", [
"jquery",
], function (
$
) {
$(function () {
$('#report-content').on('click', '.toggle-all-locations', function (e) {
$(this).prevAll('.locations-list').toggle();
$(this).children('.loc-view-control').toggle();
$(this).prevAll('.all-locations-list').toggle();
e.preventDefault();
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
'use strict';
hqDefine("reports/js/bootstrap5/application_status", [
"jquery",
], function (
$
) {
$(function () {
$('#report-content').on('click', '.toggle-all-locations', function (e) {
$(this).prevAll('.locations-list').toggle();
$(this).children('.loc-view-control').toggle();
$(this).prevAll('.all-locations-list').toggle();
e.preventDefault();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
<script src="{% static 'reports/js/inspect_data.js' %}"></script>
<script src="{% static 'reports/js/bootstrap3/project_health_dashboard.js' %}"></script>
<script src="{% static 'reports/js/bootstrap3/aggregate_user_status.js' %}"></script>
<script src="{% static 'reports/js/bootstrap3/application_status.js' %}"></script>
<script src="{% static 'reports/js/user_history.js' %}"></script>
<script src="{% static 'reports/js/case_activity.js' %}"></script>
<script src="{% static 'hqwebapp/js/bootstrap3/widgets.js' %}"></script>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
<script src="{% static 'reports/js/inspect_data.js' %}"></script>
<script src="{% static 'reports/js/bootstrap5/project_health_dashboard.js' %}"></script>
<script src="{% static 'reports/js/bootstrap5/aggregate_user_status.js' %}"></script>
<script src="{% static 'reports/js/bootstrap5/application_status.js' %}"></script>
<script src="{% static 'reports/js/user_history.js' %}"></script>
<script src="{% static 'reports/js/case_activity.js' %}"></script>
<script src="{% static 'hqwebapp/js/bootstrap5/widgets.js' %}"></script>
Expand Down
Loading