From 845ed9af3a36700502124b4f986b4391df563d6f Mon Sep 17 00:00:00 2001 From: AChaourova Date: Mon, 7 Sep 2026 10:12:06 +1000 Subject: [PATCH 1/5] Add repeat-count row expansion for report runner multilabel printing --- specifyweb/backend/report_runner/tests.py | 76 ++++++++++++++++++++++- specifyweb/backend/report_runner/views.py | 51 +++++++++++++++ 2 files changed, 126 insertions(+), 1 deletion(-) diff --git a/specifyweb/backend/report_runner/tests.py b/specifyweb/backend/report_runner/tests.py index 60fcd8a1f22..274cf37a034 100644 --- a/specifyweb/backend/report_runner/tests.py +++ b/specifyweb/backend/report_runner/tests.py @@ -3,6 +3,7 @@ from django.test import Client, override_settings +from specifyweb.backend.report_runner.views import _expand_rows_for_repeat_count from specifyweb.backend.stored_queries.tests.test_views.raw_query import ( get_simple_query, ) @@ -177,4 +178,77 @@ def test_create_label_from_new_query(self, context: Mock): self.assertIn( '1.collectionobject.catalogNumber', label_data.get_decoded_data(), - ) \ No newline at end of file + ) + + +_JASPER_NS = 'http://jasperreports.sourceforge.net/jasperreports' + +def _make_jrxml(repeat_field=None): + """Build a minimal jrxml string, optionally with a repeat-count property.""" + prop = ( + f'' + if repeat_field else '' + ) + return ( + f'' + f'{prop}' + f'' + ) + + +class TestExpandRowsForRepeatCount(ApiTests): + + def _data(self, rows): + return { + 'fields': ['id', '1,63-preparations.preparation.countAmt'], + 'rows': rows, + } + + def test_no_property_returns_data_unchanged(self): + data = self._data([[1, 3], [2, 2]]) + result = _expand_rows_for_repeat_count(_make_jrxml(), data) + self.assertEqual(result, data) + + def test_rows_expanded_by_count_field(self): + data = self._data([[1, 3], [2, 2]]) + result = _expand_rows_for_repeat_count( + _make_jrxml('1,63-preparations.preparation.countAmt'), data + ) + self.assertEqual(result['fields'], data['fields']) + self.assertEqual(len(result['rows']), 5) # 3 + 2 + self.assertEqual(result['rows'][0], [1, 3]) + self.assertEqual(result['rows'][3], [2, 2]) + + def test_count_of_one_does_not_duplicate(self): + data = self._data([[1, 1]]) + result = _expand_rows_for_repeat_count( + _make_jrxml('1,63-preparations.preparation.countAmt'), data + ) + self.assertEqual(len(result['rows']), 1) + + def test_null_count_defaults_to_one(self): + data = self._data([[1, None]]) + result = _expand_rows_for_repeat_count( + _make_jrxml('1,63-preparations.preparation.countAmt'), data + ) + self.assertEqual(len(result['rows']), 1) + + def test_zero_count_defaults_to_one(self): + data = self._data([[1, 0]]) + result = _expand_rows_for_repeat_count( + _make_jrxml('1,63-preparations.preparation.countAmt'), data + ) + self.assertEqual(len(result['rows']), 1) + + def test_field_not_in_result_set_returns_data_unchanged(self): + data = self._data([[1, 3]]) + result = _expand_rows_for_repeat_count( + _make_jrxml('1,63-preparations.preparation.nonexistent'), data + ) + self.assertEqual(result, data) + + def test_invalid_jrxml_returns_data_unchanged(self): + data = self._data([[1, 3]]) + result = _expand_rows_for_repeat_count('not valid xml', data) + self.assertEqual(result, data) \ No newline at end of file diff --git a/specifyweb/backend/report_runner/views.py b/specifyweb/backend/report_runner/views.py index 6f394bec3a2..16f2778064d 100644 --- a/specifyweb/backend/report_runner/views.py +++ b/specifyweb/backend/report_runner/views.py @@ -1,4 +1,5 @@ import json +import xml.etree.ElementTree as ET import requests from django.conf import settings @@ -25,6 +26,54 @@ class ReportException(Exception): pass + +_JASPER_NS = 'http://jasperreports.sourceforge.net/jasperreports' +# jrxml property that opts a report into per-row label repetition. +# Set its value to the field name whose integer value controls how many +# times each row should be repeated, e.g.: +# +_REPEAT_COUNT_PROPERTY = 'specify.repeat.count.field' + + +def _expand_rows_for_repeat_count(report_jrxml, report_data): + """Duplicate rows according to a repeat-count field declared in the jrxml. + + If the jrxml does not declare the ``specify.repeat.count.field`` property, + or the named field is not present in the result set, the data is returned + unchanged. This restores the Specify 6 behaviour where a label could be + printed N times based on an integer preparation field (e.g. countAmt). + """ + try: + root = ET.fromstring(report_jrxml) + except ET.ParseError: + return report_data + + repeat_field = next( + (p.get('value') for p in root.findall('{%s}property' % _JASPER_NS) + if p.get('name') == _REPEAT_COUNT_PROPERTY), + None, + ) + + if not repeat_field: + return report_data + + fields = report_data['fields'] + if repeat_field not in fields: + return report_data + + idx = fields.index(repeat_field) + expanded = [] + for row in report_data['rows']: + try: + count = max(1, int(row[idx])) + except (TypeError, ValueError): + count = 1 + expanded.extend([row] * count) + + return {'fields': fields, 'rows': expanded} + + class ReportsPT(PermissionTarget): resource = "/report" execute = PermissionTargetAction() @@ -54,6 +103,8 @@ def run(request): if len(report_data['rows']) < 1: return HttpResponse(_("The report query returned no results."), content_type="text/plain") + report_data = _expand_rows_for_repeat_count(request.POST['report'], report_data) + r = requests.post("http://%s:%s/report" % (settings.REPORT_RUNNER_HOST, port), data={'report': request.POST['report'], From d29bc52ec5f9a3a9c477e7f1e467e88329f3f280 Mon Sep 17 00:00:00 2001 From: AChaourova Date: Tue, 15 Sep 2026 12:11:04 +1000 Subject: [PATCH 2/5] Make label repeat honor spreport RepeatField/RepeatCount for Sp6 compatibility --- specifyweb/backend/report_runner/tests.py | 121 +++++++++++++++++- specifyweb/backend/report_runner/views.py | 101 +++++++++++---- .../lib/components/Reports/ForRecord.tsx | 4 + .../lib/components/Reports/Parameters.tsx | 3 + .../lib/components/Reports/RecordSets.tsx | 3 + .../js_src/lib/components/Reports/Report.tsx | 5 + .../js_src/lib/components/Reports/Run.tsx | 5 + 7 files changed, 218 insertions(+), 24 deletions(-) diff --git a/specifyweb/backend/report_runner/tests.py b/specifyweb/backend/report_runner/tests.py index 274cf37a034..49354959ea9 100644 --- a/specifyweb/backend/report_runner/tests.py +++ b/specifyweb/backend/report_runner/tests.py @@ -3,7 +3,12 @@ from django.test import Client, override_settings -from specifyweb.backend.report_runner.views import _expand_rows_for_repeat_count +from specifyweb.backend.report_runner.views import ( + _expand_rows_by_count, + _expand_rows_by_field, + _expand_rows_for_repeat, + _expand_rows_for_repeat_count, +) from specifyweb.backend.stored_queries.tests.test_views.raw_query import ( get_simple_query, ) @@ -251,4 +256,116 @@ def test_field_not_in_result_set_returns_data_unchanged(self): def test_invalid_jrxml_returns_data_unchanged(self): data = self._data([[1, 3]]) result = _expand_rows_for_repeat_count('not valid xml', data) - self.assertEqual(result, data) \ No newline at end of file + self.assertEqual(result, data) + + +class TestExpandRowsByField(ApiTests): + + def _data(self, rows): + return { + 'fields': ['id', '1,63-preparations.preparation.countAmt'], + 'rows': rows, + } + + def test_rows_expanded_by_field_value(self): + data = self._data([[1, 3], [2, 2]]) + result = _expand_rows_by_field( + data, '1,63-preparations.preparation.countAmt' + ) + self.assertEqual(len(result['rows']), 5) + self.assertEqual(result['rows'][0], [1, 3]) + self.assertEqual(result['rows'][3], [2, 2]) + + def test_null_zero_and_invalid_default_to_one(self): + data = self._data([[1, None], [2, 0], [3, 'x']]) + result = _expand_rows_by_field( + data, '1,63-preparations.preparation.countAmt' + ) + self.assertEqual(len(result['rows']), 3) + + def test_missing_field_returns_data_unchanged(self): + data = self._data([[1, 3]]) + result = _expand_rows_by_field(data, 'not.a.field') + self.assertEqual(result, data) + + def test_empty_field_returns_data_unchanged(self): + data = self._data([[1, 3]]) + result = _expand_rows_by_field(data, None) + self.assertEqual(result, data) + + +class TestExpandRowsByCount(ApiTests): + + def _data(self, rows): + return {'fields': ['id'], 'rows': rows} + + def test_every_row_repeated_fixed_count(self): + data = self._data([[1], [2]]) + result = _expand_rows_by_count(data, 3) + self.assertEqual(len(result['rows']), 6) + self.assertEqual(result['rows'][:3], [[1], [1], [1]]) + + def test_count_of_one_returns_data_unchanged(self): + data = self._data([[1], [2]]) + result = _expand_rows_by_count(data, 1) + self.assertEqual(result, data) + + def test_invalid_count_returns_data_unchanged(self): + data = self._data([[1]]) + self.assertEqual(_expand_rows_by_count(data, None), data) + self.assertEqual(_expand_rows_by_count(data, 'x'), data) + + +class TestExpandRowsForRepeat(ApiTests): + + def _data(self, rows): + return { + 'fields': ['id', '1,63-preparations.preparation.countAmt'], + 'rows': rows, + } + + @patch('specifyweb.backend.report_runner.views.Spreport.objects.get') + def test_repeat_field_takes_precedence(self, get: Mock): + get.return_value = Mock( + repeatfield='1,63-preparations.preparation.countAmt', + repeatcount=99, + ) + data = self._data([[1, 3], [2, 2]]) + result = _expand_rows_for_repeat(_make_jrxml(), data, report_id=1) + self.assertEqual(len(result['rows']), 5) + + @patch('specifyweb.backend.report_runner.views.Spreport.objects.get') + def test_repeat_count_used_when_no_field(self, get: Mock): + get.return_value = Mock(repeatfield=None, repeatcount=3) + data = self._data([[1, 9], [2, 9]]) + result = _expand_rows_for_repeat(_make_jrxml(), data, report_id=1) + self.assertEqual(len(result['rows']), 6) + + @patch('specifyweb.backend.report_runner.views.Spreport.objects.get') + def test_falls_back_to_jrxml_when_report_has_no_config(self, get: Mock): + get.return_value = Mock(repeatfield=None, repeatcount=None) + data = self._data([[1, 3]]) + result = _expand_rows_for_repeat( + _make_jrxml('1,63-preparations.preparation.countAmt'), + data, + report_id=1, + ) + self.assertEqual(len(result['rows']), 3) + + def test_no_report_id_uses_jrxml(self): + data = self._data([[1, 3]]) + result = _expand_rows_for_repeat( + _make_jrxml('1,63-preparations.preparation.countAmt'), data + ) + self.assertEqual(len(result['rows']), 3) + + @patch('specifyweb.backend.report_runner.views.Spreport.objects.get') + def test_missing_report_falls_back_to_jrxml(self, get: Mock): + get.side_effect = Spreport.DoesNotExist + data = self._data([[1, 3]]) + result = _expand_rows_for_repeat( + _make_jrxml('1,63-preparations.preparation.countAmt'), + data, + report_id=999, + ) + self.assertEqual(len(result['rows']), 3) \ No newline at end of file diff --git a/specifyweb/backend/report_runner/views.py b/specifyweb/backend/report_runner/views.py index 16f2778064d..d97e27e9ed8 100644 --- a/specifyweb/backend/report_runner/views.py +++ b/specifyweb/backend/report_runner/views.py @@ -36,30 +36,14 @@ class ReportException(Exception): _REPEAT_COUNT_PROPERTY = 'specify.repeat.count.field' -def _expand_rows_for_repeat_count(report_jrxml, report_data): - """Duplicate rows according to a repeat-count field declared in the jrxml. +def _expand_rows_by_field(report_data, repeat_field): + """Duplicate each row by the integer value of the named result field. - If the jrxml does not declare the ``specify.repeat.count.field`` property, - or the named field is not present in the result set, the data is returned - unchanged. This restores the Specify 6 behaviour where a label could be - printed N times based on an integer preparation field (e.g. countAmt). + Rows whose value is null, zero, or non-numeric default to a single copy. + If the field is not present in the result set, the data is unchanged. """ - try: - root = ET.fromstring(report_jrxml) - except ET.ParseError: - return report_data - - repeat_field = next( - (p.get('value') for p in root.findall('{%s}property' % _JASPER_NS) - if p.get('name') == _REPEAT_COUNT_PROPERTY), - None, - ) - - if not repeat_field: - return report_data - fields = report_data['fields'] - if repeat_field not in fields: + if not repeat_field or repeat_field not in fields: return report_data idx = fields.index(repeat_field) @@ -74,6 +58,75 @@ def _expand_rows_for_repeat_count(report_jrxml, report_data): return {'fields': fields, 'rows': expanded} +def _expand_rows_by_count(report_data, repeat_count): + """Duplicate every row a fixed number of times (Specify 6 RepeatCount).""" + try: + count = max(1, int(repeat_count)) + except (TypeError, ValueError): + return report_data + + if count == 1: + return report_data + + expanded = [row for row in report_data['rows'] for _ in range(count)] + return {'fields': report_data['fields'], 'rows': expanded} + + +def _repeat_field_from_jrxml(report_jrxml): + """Return the field named by the jrxml repeat-count property, or None.""" + try: + root = ET.fromstring(report_jrxml) + except ET.ParseError: + return None + + return next( + (p.get('value') for p in root.findall('{%s}property' % _JASPER_NS) + if p.get('name') == _REPEAT_COUNT_PROPERTY), + None, + ) + + +def _expand_rows_for_repeat_count(report_jrxml, report_data): + """Duplicate rows according to a repeat-count field declared in the jrxml. + + If the jrxml does not declare the ``specify.repeat.count.field`` property, + or the named field is not present in the result set, the data is returned + unchanged. This restores the Specify 6 behaviour where a label could be + printed N times based on an integer preparation field (e.g. countAmt). + """ + return _expand_rows_by_field( + report_data, _repeat_field_from_jrxml(report_jrxml) + ) + + +def _expand_rows_for_repeat(report_jrxml, report_data, report_id=None): + """Expand rows for label repetition, mirroring Specify 6. + + Precedence: + 1. ``spreport.RepeatField`` — repeat each row by the integer value of the + named result field (e.g. preparation.countAmt). + 2. ``spreport.RepeatCount`` — repeat every row a fixed number of times. + 3. ``specify.repeat.count.field`` jrxml property — for self-contained + labels that carry the setting in their own XML. + + The first two mirror how Specify 6 stores this configuration on the report + record, so labels migrated from Specify 6 repeat without editing the jrxml. + """ + if report_id not in (None, ''): + try: + report = Spreport.objects.get(id=report_id) + except (Spreport.DoesNotExist, ValueError, TypeError): + report = None + + if report is not None: + if report.repeatfield: + return _expand_rows_by_field(report_data, report.repeatfield) + if report.repeatcount: + return _expand_rows_by_count(report_data, report.repeatcount) + + return _expand_rows_for_repeat_count(report_jrxml, report_data) + + class ReportsPT(PermissionTarget): resource = "/report" execute = PermissionTargetAction() @@ -103,7 +156,11 @@ def run(request): if len(report_data['rows']) < 1: return HttpResponse(_("The report query returned no results."), content_type="text/plain") - report_data = _expand_rows_for_repeat_count(request.POST['report'], report_data) + report_data = _expand_rows_for_repeat( + request.POST['report'], + report_data, + request.POST.get('reportId'), + ) r = requests.post("http://%s:%s/report" % (settings.REPORT_RUNNER_HOST, port), diff --git a/specifyweb/frontend/js_src/lib/components/Reports/ForRecord.tsx b/specifyweb/frontend/js_src/lib/components/Reports/ForRecord.tsx index 1bf932b9328..162b05877a0 100644 --- a/specifyweb/frontend/js_src/lib/components/Reports/ForRecord.tsx +++ b/specifyweb/frontend/js_src/lib/components/Reports/ForRecord.tsx @@ -18,6 +18,7 @@ export function ReportForRecord({ definition, table, resourceId, + reportId, onClose: handleClose, }: { readonly query: SerializedResource; @@ -25,6 +26,7 @@ export function ReportForRecord({ readonly parameters: IR; readonly table: SpecifyTable; readonly resourceId: number; + readonly reportId: number | undefined; readonly onClose: () => void; }): JSX.Element { const queryFieldFilterSpecs = useQueryFieldFilterSpecs(); @@ -79,6 +81,7 @@ export function ReportForRecord({ parameters={parameters} query={query} recordSetId={undefined} + reportId={reportId} onClose={handleClose} /> ) : ( @@ -87,6 +90,7 @@ export function ReportForRecord({ parameters={parameters} query={query} recordSetId={undefined} + reportId={reportId} onClose={handleClose} /> ); diff --git a/specifyweb/frontend/js_src/lib/components/Reports/Parameters.tsx b/specifyweb/frontend/js_src/lib/components/Reports/Parameters.tsx index 5b792028f9b..22fea92d4dd 100644 --- a/specifyweb/frontend/js_src/lib/components/Reports/Parameters.tsx +++ b/specifyweb/frontend/js_src/lib/components/Reports/Parameters.tsx @@ -28,6 +28,7 @@ export function QueryParametersDialog({ recordSetId, definition, parameters, + reportId, autoRun, onClose: handleClose, }: { @@ -35,6 +36,7 @@ export function QueryParametersDialog({ readonly recordSetId: number | undefined; readonly definition: Element; readonly parameters: IR; + readonly reportId: number | undefined; readonly autoRun: boolean; readonly onClose: () => void; }): JSX.Element { @@ -77,6 +79,7 @@ export function QueryParametersDialog({ parameters={parameters} query={state.query} recordSetId={recordSetId} + reportId={reportId} onClose={(): void => setState({ type: 'Main' })} /> ) : ( diff --git a/specifyweb/frontend/js_src/lib/components/Reports/RecordSets.tsx b/specifyweb/frontend/js_src/lib/components/Reports/RecordSets.tsx index aea626d06c2..6e0c56be44a 100644 --- a/specifyweb/frontend/js_src/lib/components/Reports/RecordSets.tsx +++ b/specifyweb/frontend/js_src/lib/components/Reports/RecordSets.tsx @@ -21,12 +21,14 @@ export function ReportRecordSets({ appResource, definition, parameters, + reportId, onClose: handleClose, }: { readonly query: SerializedResource; readonly appResource: SerializedResource; readonly definition: Element; readonly parameters: IR; + readonly reportId: number | undefined; readonly onClose: () => void; }): JSX.Element { const table = React.useMemo( @@ -105,6 +107,7 @@ export function ReportRecordSets({ parameters={parameters} query={query} recordSetId={state.type === 'RecordSet' ? state.recordSet.id : undefined} + reportId={reportId} onClose={handleClose} /> ); diff --git a/specifyweb/frontend/js_src/lib/components/Reports/Report.tsx b/specifyweb/frontend/js_src/lib/components/Reports/Report.tsx index 9cb770e52ee..a9683398238 100644 --- a/specifyweb/frontend/js_src/lib/components/Reports/Report.tsx +++ b/specifyweb/frontend/js_src/lib/components/Reports/Report.tsx @@ -121,6 +121,7 @@ function ReportDialog({ appResource={appResource} definition={definition} query={query} + reportId={report?.id} resourceId={resourceId} table={table} onClose={handleClose} @@ -247,6 +248,7 @@ function ParametersDialog({ definition, query, appResource, + reportId, resourceId, table, onClose: handleClose, @@ -254,6 +256,7 @@ function ParametersDialog({ readonly definition: Element; readonly query: SerializedResource | false | undefined; readonly appResource: SerializedResource; + readonly reportId: number | undefined; readonly resourceId: number | undefined; readonly table: SpecifyTable | undefined; readonly onClose: () => void; @@ -289,6 +292,7 @@ function ParametersDialog({ definition={definition} parameters={parameters} query={query} + reportId={reportId} resourceId={resourceId} table={table} onClose={handleClose} @@ -299,6 +303,7 @@ function ParametersDialog({ definition={definition} parameters={parameters} query={query} + reportId={reportId} onClose={handleClose} /> ) diff --git a/specifyweb/frontend/js_src/lib/components/Reports/Run.tsx b/specifyweb/frontend/js_src/lib/components/Reports/Run.tsx index 98dbbb05928..10e0437e474 100644 --- a/specifyweb/frontend/js_src/lib/components/Reports/Run.tsx +++ b/specifyweb/frontend/js_src/lib/components/Reports/Run.tsx @@ -13,12 +13,14 @@ export function RunReport({ recordSetId, definition, parameters, + reportId, onClose: handleClose, }: { readonly query: SerializedResource; readonly recordSetId: number | undefined; readonly definition: Element; readonly parameters: IR; + readonly reportId: number | undefined; readonly onClose: () => void; }): JSX.Element { const reportWindowContext = useId('report-window')(''); @@ -76,6 +78,9 @@ export function RunReport({ readOnly type="hidden" /> + {typeof reportId === 'number' ? ( + + ) : undefined} ); From 6ea82cae3a60bc7178ebfd76900b14b9a451df7a Mon Sep 17 00:00:00 2001 From: AChaourova Date: Mon, 21 Sep 2026 07:50:16 +1000 Subject: [PATCH 3/5] tests --- specifyweb/backend/report_runner/tests.py | 115 +++++++++++++++++++++- 1 file changed, 114 insertions(+), 1 deletion(-) diff --git a/specifyweb/backend/report_runner/tests.py b/specifyweb/backend/report_runner/tests.py index 49354959ea9..03465dc0e2e 100644 --- a/specifyweb/backend/report_runner/tests.py +++ b/specifyweb/backend/report_runner/tests.py @@ -14,7 +14,7 @@ ) from specifyweb.backend.stored_queries.tests.tests import SQLAlchemySetup from specifyweb.specify.api.crud import post_resource -from specifyweb.specify.models import Spreport +from specifyweb.specify.models import Spappresourcedir, Spreport from specifyweb.specify.tests.test_api import ApiTests @@ -95,6 +95,119 @@ def test_run_returns_pdf( report_data, ) + def _make_report(self, **report_kwargs): + """Create a real SpReport (with app resource) for run-view tests.""" + appdir = Spappresourcedir.objects.create(discipline=self.discipline) + appresource = appdir.sppersistedappresources.create( + version=0, + mimetype='jrxml/label', + level=0, + name='Repeat Label', + description='Repeat Label', + specifyuser=self.specifyuser, + metadata='tableid=-1;reporttype=Report;', + ) + return Spreport.objects.create( + version=0, + name='Repeat Label', + appresource=appresource, + specifyuser=self.specifyuser, + **report_kwargs, + ) + + @override_settings( + REPORT_RUNNER_HOST='report-runner', + REPORT_RUNNER_PORT='8080', + ) + @patch('specifyweb.backend.report_runner.views.requests.post') + @patch('specifyweb.backend.report_runner.views.run_query') + def test_run_expands_rows_from_repeat_field( + self, + run_query: Mock, + requests_post: Mock, + ): + """End-to-end: the run view reads RepeatField from a real SpReport row + and expands each result row by that field's value before sending the + data to the report renderer.""" + report = self._make_report( + repeatfield='1,63-preparations.preparation.countAmt', + ) + + report_data = { + 'fields': ['id', '1,63-preparations.preparation.countAmt'], + 'rows': [ + [self.collectionobjects[0].id, 3], + [self.collectionobjects[1].id, 2], + ], + } + run_query.return_value = report_data + requests_post.return_value.status_code = 200 + requests_post.return_value.content = b'%PDF-1.4 test' + + client = Client() + client.force_login(self.specifyuser) + + response = client.post( + '/report_runner/run/', + { + 'query': json.dumps({'name': 'New Query'}), + 'parameters': json.dumps({}), + 'report': '', + 'reportId': report.id, + }, + ) + + self._assertStatusCodeEqual(response, 200) + + sent = json.loads(requests_post.call_args.kwargs['data']['data']) + self.assertEqual(len(sent['rows']), 5) # 3 + 2 + self.assertEqual(sent['rows'][0], [self.collectionobjects[0].id, 3]) + self.assertEqual(sent['rows'][3], [self.collectionobjects[1].id, 2]) + + @override_settings( + REPORT_RUNNER_HOST='report-runner', + REPORT_RUNNER_PORT='8080', + ) + @patch('specifyweb.backend.report_runner.views.requests.post') + @patch('specifyweb.backend.report_runner.views.run_query') + def test_run_expands_rows_from_repeat_count( + self, + run_query: Mock, + requests_post: Mock, + ): + """End-to-end: the run view repeats every row a fixed number of times + when the SpReport row has RepeatCount but no RepeatField.""" + report = self._make_report(repeatcount=3) + + report_data = { + 'fields': ['id', '1,63-preparations.preparation.countAmt'], + 'rows': [ + [self.collectionobjects[0].id, 9], + [self.collectionobjects[1].id, 9], + ], + } + run_query.return_value = report_data + requests_post.return_value.status_code = 200 + requests_post.return_value.content = b'%PDF-1.4 test' + + client = Client() + client.force_login(self.specifyuser) + + response = client.post( + '/report_runner/run/', + { + 'query': json.dumps({'name': 'New Query'}), + 'parameters': json.dumps({}), + 'report': '', + 'reportId': report.id, + }, + ) + + self._assertStatusCodeEqual(response, 200) + + sent = json.loads(requests_post.call_args.kwargs['data']['data']) + self.assertEqual(len(sent['rows']), 6) # 2 rows * 3 + class TestCreateReport(SQLAlchemySetup): From 5b90a4c582ee3d8ea71e17948a6030fbd1dac2f7 Mon Sep 17 00:00:00 2001 From: AChaourova <149641773+AChaourova@users.noreply.github.com> Date: Mon, 21 Sep 2026 09:09:50 +1000 Subject: [PATCH 4/5] Update specifyweb/backend/report_runner/views.py Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- specifyweb/backend/report_runner/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specifyweb/backend/report_runner/views.py b/specifyweb/backend/report_runner/views.py index d97e27e9ed8..225fc507f75 100644 --- a/specifyweb/backend/report_runner/views.py +++ b/specifyweb/backend/report_runner/views.py @@ -121,7 +121,7 @@ def _expand_rows_for_repeat(report_jrxml, report_data, report_id=None): if report is not None: if report.repeatfield: return _expand_rows_by_field(report_data, report.repeatfield) - if report.repeatcount: + if report.repeatcount is not None: return _expand_rows_by_count(report_data, report.repeatcount) return _expand_rows_for_repeat_count(report_jrxml, report_data) From 094f51cb2a92358764c327e91447bc9d0c40563c Mon Sep 17 00:00:00 2001 From: AChaourova Date: Mon, 21 Sep 2026 11:15:33 +1000 Subject: [PATCH 5/5] Validate reportId against submitted query/collection before applying repeat settings --- specifyweb/backend/report_runner/tests.py | 89 ++++++++++++++++++----- specifyweb/backend/report_runner/views.py | 51 +++++++++---- 2 files changed, 108 insertions(+), 32 deletions(-) diff --git a/specifyweb/backend/report_runner/tests.py b/specifyweb/backend/report_runner/tests.py index 03465dc0e2e..89129fb674a 100644 --- a/specifyweb/backend/report_runner/tests.py +++ b/specifyweb/backend/report_runner/tests.py @@ -97,6 +97,12 @@ def test_run_returns_pdf( def _make_report(self, **report_kwargs): """Create a real SpReport (with app resource) for run-view tests.""" + query = report_kwargs.pop('query', None) or post_resource( + self.collection, + self.agent, + 'spquery', + get_simple_query(self.specifyuser), + ) appdir = Spappresourcedir.objects.create(discipline=self.discipline) appresource = appdir.sppersistedappresources.create( version=0, @@ -111,6 +117,7 @@ def _make_report(self, **report_kwargs): version=0, name='Repeat Label', appresource=appresource, + query=query, specifyuser=self.specifyuser, **report_kwargs, ) @@ -150,7 +157,10 @@ def test_run_expands_rows_from_repeat_field( response = client.post( '/report_runner/run/', { - 'query': json.dumps({'name': 'New Query'}), + 'query': json.dumps({ + 'id': report.query_id, + 'name': 'New Query', + }), 'parameters': json.dumps({}), 'report': '', 'reportId': report.id, @@ -196,7 +206,10 @@ def test_run_expands_rows_from_repeat_count( response = client.post( '/report_runner/run/', { - 'query': json.dumps({'name': 'New Query'}), + 'query': json.dumps({ + 'id': report.query_id, + 'name': 'New Query', + }), 'parameters': json.dumps({}), 'report': '', 'reportId': report.id, @@ -208,6 +221,51 @@ def test_run_expands_rows_from_repeat_count( sent = json.loads(requests_post.call_args.kwargs['data']['data']) self.assertEqual(len(sent['rows']), 6) # 2 rows * 3 + @override_settings( + REPORT_RUNNER_HOST='report-runner', + REPORT_RUNNER_PORT='8080', + ) + @patch('specifyweb.backend.report_runner.views.requests.post') + @patch('specifyweb.backend.report_runner.views.run_query') + def test_run_ignores_repeat_settings_from_mismatched_report( + self, + run_query: Mock, + requests_post: Mock, + ): + report = self._make_report(repeatcount=3) + other_query = post_resource( + self.collection, + self.agent, + 'spquery', + get_simple_query(self.specifyuser), + ) + report_data = { + 'fields': ['id'], + 'rows': [[self.collectionobjects[0].id]], + } + run_query.return_value = report_data + requests_post.return_value.status_code = 200 + requests_post.return_value.content = b'%PDF-1.4 test' + + client = Client() + client.force_login(self.specifyuser) + response = client.post( + '/report_runner/run/', + { + 'query': json.dumps({ + 'id': other_query.id, + 'name': 'Other Query', + }), + 'parameters': json.dumps({}), + 'report': '', + 'reportId': report.id, + }, + ) + + self._assertStatusCodeEqual(response, 200) + sent = json.loads(requests_post.call_args.kwargs['data']['data']) + self.assertEqual(sent, report_data) + class TestCreateReport(SQLAlchemySetup): @@ -437,31 +495,28 @@ def _data(self, rows): 'rows': rows, } - @patch('specifyweb.backend.report_runner.views.Spreport.objects.get') - def test_repeat_field_takes_precedence(self, get: Mock): - get.return_value = Mock( + def test_repeat_field_takes_precedence(self): + report = Mock( repeatfield='1,63-preparations.preparation.countAmt', repeatcount=99, ) data = self._data([[1, 3], [2, 2]]) - result = _expand_rows_for_repeat(_make_jrxml(), data, report_id=1) + result = _expand_rows_for_repeat(_make_jrxml(), data, report) self.assertEqual(len(result['rows']), 5) - @patch('specifyweb.backend.report_runner.views.Spreport.objects.get') - def test_repeat_count_used_when_no_field(self, get: Mock): - get.return_value = Mock(repeatfield=None, repeatcount=3) + def test_repeat_count_used_when_no_field(self): + report = Mock(repeatfield=None, repeatcount=3) data = self._data([[1, 9], [2, 9]]) - result = _expand_rows_for_repeat(_make_jrxml(), data, report_id=1) + result = _expand_rows_for_repeat(_make_jrxml(), data, report) self.assertEqual(len(result['rows']), 6) - @patch('specifyweb.backend.report_runner.views.Spreport.objects.get') - def test_falls_back_to_jrxml_when_report_has_no_config(self, get: Mock): - get.return_value = Mock(repeatfield=None, repeatcount=None) + def test_falls_back_to_jrxml_when_report_has_no_config(self): + report = Mock(repeatfield=None, repeatcount=None) data = self._data([[1, 3]]) result = _expand_rows_for_repeat( _make_jrxml('1,63-preparations.preparation.countAmt'), data, - report_id=1, + report, ) self.assertEqual(len(result['rows']), 3) @@ -472,13 +527,11 @@ def test_no_report_id_uses_jrxml(self): ) self.assertEqual(len(result['rows']), 3) - @patch('specifyweb.backend.report_runner.views.Spreport.objects.get') - def test_missing_report_falls_back_to_jrxml(self, get: Mock): - get.side_effect = Spreport.DoesNotExist + def test_missing_report_falls_back_to_jrxml(self): data = self._data([[1, 3]]) result = _expand_rows_for_repeat( _make_jrxml('1,63-preparations.preparation.countAmt'), data, - report_id=999, + None, ) self.assertEqual(len(result['rows']), 3) \ No newline at end of file diff --git a/specifyweb/backend/report_runner/views.py b/specifyweb/backend/report_runner/views.py index 225fc507f75..ba1e3da164e 100644 --- a/specifyweb/backend/report_runner/views.py +++ b/specifyweb/backend/report_runner/views.py @@ -99,7 +99,7 @@ def _expand_rows_for_repeat_count(report_jrxml, report_data): ) -def _expand_rows_for_repeat(report_jrxml, report_data, report_id=None): +def _expand_rows_for_repeat(report_jrxml, report_data, report=None): """Expand rows for label repetition, mirroring Specify 6. Precedence: @@ -112,21 +112,38 @@ def _expand_rows_for_repeat(report_jrxml, report_data, report_id=None): The first two mirror how Specify 6 stores this configuration on the report record, so labels migrated from Specify 6 repeat without editing the jrxml. """ - if report_id not in (None, ''): - try: - report = Spreport.objects.get(id=report_id) - except (Spreport.DoesNotExist, ValueError, TypeError): - report = None - - if report is not None: - if report.repeatfield: - return _expand_rows_by_field(report_data, report.repeatfield) - if report.repeatcount is not None: - return _expand_rows_by_count(report_data, report.repeatcount) + if report is not None: + if report.repeatfield: + return _expand_rows_by_field(report_data, report.repeatfield) + if report.repeatcount is not None: + return _expand_rows_by_count(report_data, report.repeatcount) return _expand_rows_for_repeat_count(report_jrxml, report_data) +def _get_report_for_repeat(report_id, query_json, collection, user): + if report_id in (None, ''): + return None + + try: + query_id = int(json.loads(query_json).get('id')) + report_id = int(report_id) + except (AttributeError, TypeError, ValueError): + return None + + return Spreport.objects.filter( + id=report_id, + query_id=query_id, + appresource__spappresourcedir__discipline=collection.discipline, + ).filter( + Q(appresource__spappresourcedir__collection=None) | + Q(appresource__spappresourcedir__collection=collection) + ).filter( + Q(appresource__spappresourcedir__specifyuser=user) | + Q(appresource__spappresourcedir__ispersonal=False) + ).first() + + class ReportsPT(PermissionTarget): resource = "/report" execute = PermissionTargetAction() @@ -152,14 +169,20 @@ def run(request): port = settings.REPORT_RUNNER_PORT if port == '': port = 80 - report_data = run_query(request.specify_collection, request.specify_user, request.POST['query']) + query_json = request.POST['query'] + report_data = run_query(request.specify_collection, request.specify_user, query_json) if len(report_data['rows']) < 1: return HttpResponse(_("The report query returned no results."), content_type="text/plain") report_data = _expand_rows_for_repeat( request.POST['report'], report_data, - request.POST.get('reportId'), + _get_report_for_repeat( + request.POST.get('reportId'), + query_json, + request.specify_collection, + request.specify_user, + ), ) r = requests.post("http://%s:%s/report" %