Fixed bug that prevented patching when multiple files provided in sheet

4dn-dcic · Jul 26, 2018 · bdc1368 · bdc1368
1 parent 5474b47
commit bdc1368
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 24 deletions.
diff --git a/tests/test_import_data.py b/tests/test_import_data.py
@@ -1150,7 +1150,7 @@ def test_populate_post_json_extrafile_w_existing(
             for ef in pjson['extra_files']:
                 if ef['file_format'] == 'pairs_px2':
                     assert not ef['submitted_filename'].startswith('test2')
-                    assert 'another_field' in ef
+                    assert 'another_field' not in ef
                 assert 'filename' not in ef
 
 
@@ -1199,6 +1199,30 @@ def test_populate_post_json_extrafile_2_files_1_filename(
                 assert 'filename' not in ef
 
 
+def test_populate_post_json_extrafile_2_files_same_format(
+        mocker, connection_mock, post_json_w_extf):
+    post_json_w_extf['extra_files'][1] = {'file_format': 'bai', 'filename': '/test_bai.bam.bai'}
+    with mocker.patch('wranglertools.import_data.get_existing', return_value={}):
+        with mocker.patch('wranglertools.import_data.check_extra_file_meta',
+                          side_effect=[
+                              ({'file_format': 'bai', 'filename': '/test_bai.bam.bai',
+                                'submitted_filename': 'test_bai.bam.bai', 'filesize': 10,
+                                'md5sum': 'test_baimd5'}, ['bai']),
+                              ({'file_format': 'bai', 'filename': '/test2_bai.bam.bai',
+                                'submitted_filename': 'test2_bai.bam.bai', 'filesize': 10,
+                                'md5sum': 'test2_baimd5'}, ['bai', 'bai'])
+                          ]):
+            pjson, _, _, efiles = imp.populate_post_json(
+                post_json_w_extf, connection_mock, 'FileProcessed')
+            assert len(pjson['extra_files']) == 2
+            assert len(efiles) == 1
+            assert efiles['bai'] == '/test2_bai.bam.bai'
+            for ef in pjson['extra_files']:
+                assert 'file_format' in ef
+                assert ef['file_format'] == 'bai'
+                assert 'filename' not in ef
+
+
 class MockedException(Exception):
     def __init__(self, text):
         self.args = [text]

diff --git a/wranglertools/import_data.py b/wranglertools/import_data.py
@@ -596,36 +596,29 @@ def populate_post_json(post_json, connection, sheet):  # , existing_data):
         # in post or patch as well as upload the file if not already there
         existing_formats = []
         existing_extrafiles = []
-        extrafile_meta = []
+        extrafile_metadata = []
         if existing_data:
             if existing_data.get('extra_files'):
                 existing_extrafiles = existing_data.get('extra_files')  # to include existing
                 existing_formats = [ef.get('file_format') for ef in existing_data.get('extra_files')
                                     if ef.get('file_format') is not None]
         seen_formats = []
-        for i, extfile in enumerate(extrafiles):
-            ext_meta, seen_formats = check_extra_file_meta(extfile, seen_formats, existing_formats)
-            if ext_meta is not None:
-                extrafile_meta.append(ext_meta)
-
-        updated_existing = False
-        if extrafile_meta:
-            for i, efm in enumerate(extrafile_meta):
-                if efm.get('filename'):
-                    fp = efm['filename']
-                    extrafiles2upload[efm['file_format']] = fp
-                    del efm['filename']
-
-                for ef in existing_extrafiles:
-                    if ef['file_format'] == efm['file_format']:
-                        if not updated_existing:
-                            ef.update(efm)
-                            del extrafile_meta[i]
-                            updated_existing = True
-
-        if updated_existing or extrafile_meta:
+        for extrafile in extrafiles:
+            extrafile_meta, seen_formats = check_extra_file_meta(extrafile, seen_formats, existing_formats)
+            if extrafile_meta is not None:
+                if extrafile_meta.get('filename'):
+                    extrafiles2upload[extrafile_meta['file_format']] = extrafile_meta['filename']
+                    del extrafile_meta['filename']
+                for ix, eef in enumerate(existing_extrafiles):
+                    if eef['file_format'] == extrafile_meta['file_format']:
+                        # we are patching so want to remove existing entry from existing_extrafiles
+                        del existing_extrafiles[ix]
+                        break
+                extrafile_metadata.append(extrafile_meta)
+
+        if extrafile_metadata:
             # we have data to update
-            post_json['extra_files'] = extrafile_meta + existing_extrafiles
+            post_json['extra_files'] = extrafile_metadata + existing_extrafiles
         else:
             del post_json['extra_files']