From bc052a09d833a12821574ee5f2caa0a99cb06fe2 Mon Sep 17 00:00:00 2001 From: Martin Burchell Date: Tue, 11 Feb 2020 17:44:30 +0000 Subject: [PATCH 1/2] Change API params from comma-separated to arrays https://community.projectredcap.org/questions/77217/export-records-api-silently-ignores-invalid-form-w.html According to Rob Taylor these parameters should not be comma-separated and represented as arrays instead. --- redcap/project.py | 20 +++++++++------- redcap/request.py | 3 +++ test/test_project.py | 56 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 9 deletions(-) diff --git a/redcap/project.py b/redcap/project.py index 3e3e0ad..518c193 100755 --- a/redcap/project.py +++ b/redcap/project.py @@ -179,11 +179,11 @@ def export_fem(self, arms=None, format='json', df_kwargs=None): from pandas import read_csv ret_format = 'csv' pl = self.__basepl('formEventMapping', format=ret_format) - to_add = [arms] - str_add = ['arms'] - for key, data in zip(str_add, to_add): - if data: - pl[key] = ','.join(data) + + if arms: + for i, value in enumerate(arms): + pl["arms[{}]".format(i)] = value + response, _ = self._call_api(pl, 'exp_fem') if format in ('json', 'csv', 'xml'): return response @@ -226,7 +226,9 @@ def export_metadata(self, fields=None, forms=None, format='json', str_add = ['fields', 'forms'] for key, data in zip(str_add, to_add): if data: - pl[key] = ','.join(data) + for i, value in enumerate(data): + pl["{}[{}]".format(key, i)] = value + response, _ = self._call_api(pl, 'metadata') if format in ('json', 'csv', 'xml'): return response @@ -311,9 +313,9 @@ def export_records(self, records=None, fields=None, forms=None, 'exportCheckboxLabel') for key, data in zip(str_keys, keys_to_add): if data: - # Make a url-ok string if key in ('fields', 'records', 'forms', 'events'): - pl[key] = ','.join(data) + for i, value in enumerate(data): + pl["{}[{}]".format(key, i)] = value else: pl[key] = data @@ -665,7 +667,7 @@ def generate_next_record_name(self): pl = self.__basepl(content='generateNextRecordName') return self._call_api(pl, 'exp_next_id')[0] - + def export_project_info(self, format='json'): """ Export Project Information diff --git a/redcap/request.py b/redcap/request.py index 487ea6b..8b99a93 100644 --- a/redcap/request.py +++ b/redcap/request.py @@ -172,3 +172,6 @@ def raise_for_status(self, r): # specifically 10.5 if 500 <= r.status_code < 600: raise RedcapError(r.content) + + if 400 == r.status_code and self.type == 'exp_record': + raise RedcapError(r.content) diff --git a/test/test_project.py b/test/test_project.py index f2661c4..24e14dc 100644 --- a/test/test_project.py +++ b/test/test_project.py @@ -30,6 +30,9 @@ def is_bytestring(s): return isinstance(s, bytes) +from unittest import mock + + class ProjectTests(unittest.TestCase): """docstring for ProjectTests""" @@ -392,6 +395,23 @@ def test_metadata_export(self): csv = self.reg_proj.export_metadata(format='csv') self.assertTrue(self.is_good_csv(csv)) + def test_metadata_export_passes_filters_as_arrays(self): + self.reg_proj._call_api = mock.Mock() + self.reg_proj._call_api.return_value = (None, None) + self.reg_proj.export_metadata( + fields=['field0', 'field1', 'field2'], + forms=['form0', 'form1', 'form2'], + ) + + args, _ = self.reg_proj._call_api.call_args + + payload = args[0] + + self.assertEqual(payload['fields[0]'], 'field0') + self.assertEqual(payload['fields[1]'], 'field1') + self.assertEqual(payload['fields[2]'], 'field2') + self.assertEqual(payload['forms[2]'], 'form2') + def test_bad_creds(self): "Test that exceptions are raised with bad URL or tokens" with self.assertRaises(RedcapError): @@ -408,6 +428,21 @@ def test_fem_export(self): for arm in fem: self.assertIsInstance(arm, dict) + def test_fem_export_passes_filters_as_arrays(self): + self.reg_proj._call_api = mock.Mock() + self.reg_proj._call_api.return_value = (None, None) + self.reg_proj.export_fem( + arms=['arm0', 'arm1', 'arm2'], + ) + + args, _ = self.reg_proj._call_api.call_args + + payload = args[0] + + self.assertEqual(payload['arms[0]'], 'arm0') + self.assertEqual(payload['arms[1]'], 'arm1') + self.assertEqual(payload['arms[2]'], 'arm2') + @responses.activate def test_file_export(self): """Test file export and proper content-type parsing""" @@ -634,6 +669,27 @@ def test_export_always_include_def_field(self): for record in records: self.assertIn(self.reg_proj.def_field, record) + def test_export_passes_filters_as_arrays(self): + self.reg_proj._call_api = mock.Mock() + self.reg_proj._call_api.return_value = (None, None) + self.reg_proj.export_records( + records=['record0', 'record1', 'record2'], + fields=['field0', 'field1', 'field2'], + forms=['form0', 'form1', 'form2'], + events=['event0', 'event1', 'event2'] + ) + + args, _ = self.reg_proj._call_api.call_args + + payload = args[0] + + self.assertEqual(payload['records[0]'], 'record0') + self.assertEqual(payload['records[1]'], 'record1') + self.assertEqual(payload['records[2]'], 'record2') + self.assertEqual(payload['fields[1]'], 'field1') + self.assertEqual(payload['forms[2]'], 'form2') + self.assertEqual(payload['events[0]'], 'event0') + @responses.activate def test_generate_next_record_name(self): "Test exporting the next potential record ID for a project" From 31abef46dbe0dfb4d4cf3e2b5f9fe68f096b2554 Mon Sep 17 00:00:00 2001 From: Martin Burchell Date: Tue, 11 Feb 2020 18:02:58 +0000 Subject: [PATCH 2/2] Fix tests for Python 2.7 --- test/test_project.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/test_project.py b/test/test_project.py index 24e14dc..83f7c2e 100644 --- a/test/test_project.py +++ b/test/test_project.py @@ -29,8 +29,10 @@ def is_str(s): def is_bytestring(s): return isinstance(s, bytes) - -from unittest import mock +try: + from unittest import mock +except ImportError: + import mock class ProjectTests(unittest.TestCase):