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

Fix inddex reports #35605

Merged
merged 4 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from all 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
10 changes: 10 additions & 0 deletions corehq/apps/reports/static/reports/js/bootstrap3/tabular.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ hqDefine("reports/js/bootstrap3/tabular", [

// Handle async reports
$(document).on('ajaxSuccess', function (e, xhr, ajaxOptions, data) {
if (!data || !data.slug) {
// This file is imported by inddex/main, which then gets this event handler, which it doesn't need,
// and which errors sometimes (presumably because there are ajax requests happening that aren't the
// same as what this handler expects). Checking for data.slug is pretty innocuous and fixes the issue.
return;
}
var jsOptions = initialPageData.get("js_options");
if (jsOptions && ajaxOptions.url.indexOf(jsOptions.asyncUrl) === -1) {
return;
Expand All @@ -81,4 +87,8 @@ hqDefine("reports/js/bootstrap3/tabular", [
renderPage(initialPageData.get("js_options").slug, initialPageData.get("report_table_js_options"));
}
});

return {
renderPage: renderPage,
};
});
10 changes: 10 additions & 0 deletions corehq/apps/reports/static/reports/js/bootstrap5/tabular.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ hqDefine("reports/js/bootstrap5/tabular", [

// Handle async reports
$(document).on('ajaxSuccess', function (e, xhr, ajaxOptions, data) {
if (!data || !data.slug) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because inddex/main is now importing this file, it gets this event handler, which it doesn't need, and which errors sometimes (presumably because there are ajax requests happening that aren't the same as what this handler expects). But checking for data.slug is pretty innocuous and fixes the issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it would be cleaner to put the event handlers in an init function, and make code that imports this module explicitly call it, but I wanted to limit changes to core reports as much as possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think of putting this comment in the code? This seems far away from inddex/main, and it might be nice to know why this check was added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure - 715c441

// This file is imported by inddex/main, which then gets this event handler, which it doesn't need,
// and which errors sometimes (presumably because there are ajax requests happening that aren't the
// same as what this handler expects). Checking for data.slug is pretty innocuous and fixes the issue.
return;
}
var jsOptions = initialPageData.get("js_options");
if (jsOptions && ajaxOptions.url.indexOf(jsOptions.asyncUrl) === -1) {
return;
Expand All @@ -81,4 +87,8 @@ hqDefine("reports/js/bootstrap5/tabular", [
renderPage(initialPageData.get("js_options").slug, initialPageData.get("report_table_js_options"));
}
});

return {
renderPage: renderPage,
};
});
15 changes: 13 additions & 2 deletions custom/inddex/reports/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,22 @@ class MultiTabularReport(DatespanMixin, CustomProjectReport, GenericTabularRepor
exportable = True
exportable_all = True
export_only = False
fix_left_col = True
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This and fixed_cols_spec below are to support the fixColumns, fixColsNumLeft, and fixColsWidth values that were previously specified directly in the inddex js.


@property
def data_providers(self):
# data providers should supply a title, slug, headers, and rows
return []

@property
def fixed_cols_spec(self):
"""
Override
Returns a dict formatted like:
dict(num=<num_cols_to_fix>, width=<width_of_total_fixed_cols>)
"""
return dict(num=1, width=130)

@property
def report_context(self):

Expand All @@ -34,10 +44,11 @@ def _to_context_dict(data_provider):
'rows': list(data_provider.rows),
}

context = {
context = super().report_context
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This pulls in the report_table_js_options specified here. This is where the assumption is made that all of the tables in a single inddex report will have the same js options - I'm not pulling them separately for each data_provider.

context.update({
'name': self.name,
'export_only': self.export_only
}
})
if not self.export_only and not self.needs_filters:
try:
context['data_providers'] = list(map(_to_context_dict, self.data_providers))
Expand Down
46 changes: 5 additions & 41 deletions custom/inddex/static/inddex/js/main.js
Original file line number Diff line number Diff line change
@@ -1,45 +1,9 @@
hqDefine("inddex/js/main", function () {
$(function () {
$("[data-report-table]").each(function (table) {
const $table = $(table),
reportTable = $table.data("report-table");

if (!reportTable || !reportTable.datatables) {
return;
}

let options = {
dataTableElem: '#report_table_' + reportTable.slug,
defaultRows: reportTable.default_rows || 10,
startAtRowNum: reportTable.default_rows || 10,
showAllRowsOption: reportTable.show_all_rows,
loadingTemplateSelector: '#js-template-loading-report',
defaultSort: true,
autoWidth: reportTable.headers.auto_width,
fixColumns: true,
fixColsNumLeft: 1,
fixColsWidth: 130,
};
if (reportTable.headers.render_aoColumns) {
options.aoColumns = reportTable.headers.render_aoColumns;
}
if (reportTable.headers.custom_sort) {
options.customSort = reportTable.headers.custom_sort;
}
if (reportTable.pagination.is_on) {
options.ajaxSource = reportTable.pagination.source;
options.ajaxParams = reportTable.pagination.params;
}
if (reportTable.bad_request_error_text) {
options.badRequestErrorText = "<span class='label label-danger'>Sorry!</span> " + reportTable.bad_request_error_text;
}

var reportTables = hqImport("reports/js/bootstrap3/datatables_config").HQReportDataTables(options);
var standardHQReport = hqImport("reports/js/bootstrap3/standard_hq_report").getStandardHQReport();
if (typeof standardHQReport !== 'undefined') {
standardHQReport.handleTabularReportCookies(reportTables);
}
reportTables.render();
const initialPageData = hqImport("hqwebapp/js/initial_page_data"),
tabular = hqImport("reports/js/bootstrap3/tabular");
$(document).on('ajaxSuccess', function (e, xhr, ajaxOptions, data) {
$(".datatable[data-slug]").each(function (index, table) {

Check failure on line 5 in custom/inddex/static/inddex/js/main.js

View workflow job for this annotation

GitHub Actions / Lint Javascript

'index' is defined but never used

Check failure on line 5 in custom/inddex/static/inddex/js/main.js

View workflow job for this annotation

GitHub Actions / Lint Javascript

'table' is defined but never used
tabular.renderPage(data.slug, initialPageData.get("report_table_js_options"));
});
});
});
6 changes: 5 additions & 1 deletion custom/inddex/templates/inddex/partials/report_table.html
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
{% load hq_shared_tags %}
{% load i18n %}

<table id="report_table_{{ report_table.slug }}" class="table table-striped datatable" {% if pagination.filter %} data-filter="true"{% endif %} data-report-table="{{ report_table|JSON }}">
<table id="report_table_{{ report_table.slug }}"
class="table table-striped datatable"
{% if pagination.filter %} data-filter="true"{% endif %}
data-slug="{{ report_table.slug }}"
>
<thead>
{% if report_table.headers.complex %}
{{ report_table.headers.render_html }}
Expand Down
Loading