Merge pull request #195 from open-apparel-registry/jcw/processing-res…

…ults-are-a-list

Processing results are a list

src/django/api/management/commands/makefixtures.py

@@ -1,7 +1,7 @@
 from django.core.management.base import (BaseCommand,
                                          CommandError)
 
-from api.models import Organization, FacilityMatch
+from api.models import Organization, FacilityMatch, FacilityListItem
 
 import csv
 import json
@@ -190,7 +190,7 @@ def make_facility_list_item(list_pk, item_pk, row_index, raw_data):
             'row_index': row_index,
             'raw_data': raw_data,
             'status': 'UPLOADED',
-            'processing_results': {},
+            'processing_results': [],
             'created_at': created_at,
             'updated_at': updated_at,
         }
@@ -239,6 +239,7 @@ def make_facility(facility_item):
 
 def make_match(item, facility):
     (created_at, updated_at) = make_created_updated()
+    item['fields']['status'] = FacilityListItem.MATCHED
     return {
         'model': 'api.facilitymatch',
         'pk': item['pk'],
@@ -261,11 +262,10 @@ def make_facilities_and_matches(list_items):
     facilities = []
     matches = []
     for item in list_items:
-        if random.randint(1, 10) < 3 and len(facilities) > 0:
-            matches.append(make_match(
-                item, facilities[random.randint(0, len(facilities)-1)]))
-        else:
-            facilities.append(make_facility(item))
+        if random.randint(1, 10) < 6:
+            facility = make_facility(item)
+            facilities.append(facility)
+            matches.append(make_match(item, facility))
     return facilities, matches
 
 

src/django/api/migrations/0001_initial.py

@@ -1,4 +1,4 @@
-# Generated by Django 2.0.10 on 2019-02-05 21:07
+# Generated by Django 2.0.10 on 2019-02-08 18:38
 
 from django.conf import settings
 import django.contrib.gis.db.models.fields
@@ -76,7 +76,7 @@ class Migration(migrations.Migration):
                 ('status', models.CharField(choices=[('UPLOADED', 'UPLOADED'), ('PARSED', 'PARSED'), ('GEOCODED', 'GEOCODED'), ('MATCHED', 'MATCHED'), ('POTENTIAL_MATCH', 'POTENTIAL_MATCH'), ('CONFIRMED_MATCH', 'CONFIRMED_MATCH'), ('ERROR', 'ERROR')], default='UPLOADED', help_text='The current workflow progress of the line item.', max_length=200)),
                 ('processing_started_at', models.DateTimeField(help_text='When background processing of this item started. Items awaiting processing will have a null value.', null=True)),
                 ('processing_completed_at', models.DateTimeField(help_text='When background processing of this item finished. Items awaiting or in process will have a null value.', null=True)),
-                ('processing_results', django.contrib.postgres.fields.jsonb.JSONField(default=dict, help_text='Diagnostic details logged by background processing including details returned from the geocoder.')),
+                ('processing_results', django.contrib.postgres.fields.jsonb.JSONField(default=list, help_text='Diagnostic details logged by background processing including details returned from the geocoder.')),
                 ('name', models.CharField(help_text='The name of the facility taken from the raw data.', max_length=200)),
                 ('address', models.CharField(help_text='The address of the facility taken from the raw data.', max_length=200)),
                 ('country_code', models.CharField(choices=[('AF', 'Afghanistan'), ('AX', 'Åland Islands'), ('AL', 'Albania'), ('DZ', 'Algeria'), ('AS', 'American Samoa'), ('AD', 'Andorra'), ('AO', 'Angola'), ('AI', 'Anguilla'), ('AQ', 'Antarctica'), ('AG', 'Antigua and Barbuda'), ('AR', 'Argentina'), ('AM', 'Armenia'), ('AW', 'Aruba'), ('AU', 'Australia'), ('AT', 'Austria'), ('AZ', 'Azerbaijan'), ('BS', 'Bahamas'), ('BH', 'Bahrain'), ('BD', 'Bangladesh'), ('BB', 'Barbados'), ('BY', 'Belarus'), ('BE', 'Belgium'), ('BZ', 'Belize'), ('BJ', 'Benin'), ('BM', 'Bermuda'), ('BT', 'Bhutan'), ('BO', 'Bolivia, Plurinational State of'), ('BQ', 'Bonaire, Sint Eustatius and Saba'), ('BA', 'Bosnia and Herzegovina'), ('BW', 'Botswana'), ('BV', 'Bouvet Island'), ('BR', 'Brazil'), ('IO', 'British Indian Ocean Territory'), ('BN', 'Brunei Darussalam'), ('BG', 'Bulgaria'), ('BF', 'Burkina Faso'), ('BI', 'Burundi'), ('KH', 'Cambodia'), ('CM', 'Cameroon'), ('CA', 'Canada'), ('CV', 'Cape Verde'), ('KY', 'Cayman Islands'), ('CF', 'Central African Republic'), ('TD', 'Chad'), ('CL', 'Chile'), ('CN', 'China'), ('CX', 'Christmas Island'), ('CC', 'Cocos (Keeling) Islands'), ('CO', 'Colombia'), ('KM', 'Comoros'), ('CG', 'Congo'), ('CD', 'Congo, the Democratic Republic of the'), ('CK', 'Cook Islands'), ('CR', 'Costa Rica'), ('CI', "Côte d'Ivoire"), ('HR', 'Croatia'), ('CU', 'Cuba'), ('CW', 'Curacao'), ('CY', 'Cyprus'), ('CZ', 'Czech Republic'), ('DK', 'Denmark'), ('DJ', 'Djibouti'), ('DM', 'Dominica'), ('DO', 'Dominican Republic'), ('EC', 'Ecuador'), ('EG', 'Egypt'), ('SV', 'El Salvador'), ('GQ', 'Equatorial Guinea'), ('ER', 'Eritrea'), ('EE', 'Estonia'), ('ET', 'Ethiopia'), ('FK', 'Falkland Islands (Malvinas)'), ('FO', 'Faroe Islands'), ('FJ', 'Fiji'), ('FI', 'Finland'), ('FR', 'France'), ('GF', 'French Guiana'), ('PF', 'French Polynesia'), ('TF', 'French Southern Territories'), ('GA', 'Gabon'), ('GM', 'Gambia'), ('GE', 'Georgia'), ('DE', 'Germany'), ('GH', 'Ghana'), ('GI', 'Gibraltar'), ('GR', 'Greece'), ('GL', 'Greenland'), ('GD', 'Grenada'), ('GP', 'Guadeloupe'), ('GU', 'Guam'), ('GT', 'Guatemala'), ('GG', 'Guernsey'), ('GN', 'Guinea'), ('GW', 'Guinea-Bissau'), ('GY', 'Guyana'), ('HT', 'Haiti'), ('HM', 'Heard Island and McDonald Islands'), ('VA', 'Holy See (Vatican City State)'), ('HN', 'Honduras'), ('HK', 'Hong Kong'), ('HU', 'Hungary'), ('IS', 'Iceland'), ('IN', 'India'), ('ID', 'Indonesia'), ('IR', 'Iran, Islamic Republic of'), ('IQ', 'Iraq'), ('IE', 'Ireland'), ('IM', 'Isle of Man'), ('IL', 'Israel'), ('IT', 'Italy'), ('JM', 'Jamaica'), ('JP', 'Japan'), ('JE', 'Jersey'), ('JO', 'Jordan'), ('KZ', 'Kazakhstan'), ('KE', 'Kenya'), ('KI', 'Kiribati'), ('KP', "Korea, Democratic People's Republic of"), ('KR', 'Korea, Republic of'), ('XK', 'Kosovo'), ('KW', 'Kuwait'), ('KG', 'Kyrgyzstan'), ('LA', "Lao People's Democratic Republic"), ('LV', 'Latvia'), ('LB', 'Lebanon'), ('LS', 'Lesotho'), ('LR', 'Liberia'), ('LY', 'Libya'), ('LI', 'Liechtenstein'), ('LT', 'Lithuania'), ('LU', 'Luxembourg'), ('MO', 'Macao'), ('MK', 'Macedonia, the Former Yugoslav Republic of'), ('MG', 'Madagascar'), ('MW', 'Malawi'), ('MY', 'Malaysia'), ('MV', 'Maldives'), ('ML', 'Mali'), ('MT', 'Malta'), ('MH', 'Marshall Islands'), ('MQ', 'Martinique'), ('MR', 'Mauritania'), ('MU', 'Mauritius'), ('YT', 'Mayotte'), ('MX', 'Mexico'), ('FM', 'Micronesia, Federated States of'), ('MD', 'Moldova, Republic of'), ('MC', 'Monaco'), ('MN', 'Mongolia'), ('ME', 'Montenegro'), ('MS', 'Montserrat'), ('MA', 'Morocco'), ('MZ', 'Mozambique'), ('MM', 'Myanmar'), ('NA', 'Namibia'), ('NR', 'Nauru'), ('NP', 'Nepal'), ('NL', 'Netherlands'), ('NC', 'New Caledonia'), ('NZ', 'New Zealand'), ('NI', 'Nicaragua'), ('NE', 'Niger'), ('NG', 'Nigeria'), ('NU', 'Niue'), ('NF', 'Norfolk Island'), ('MP', 'Northern Mariana Islands'), ('NO', 'Norway'), ('OM', 'Oman'), ('PK', 'Pakistan'), ('PW', 'Palau'), ('PS', 'Palestine, State of'), ('PA', 'Panama'), ('PG', 'Papua New Guinea'), ('PY', 'Paraguay'), ('PE', 'Peru'), ('PH', 'Philippines'), ('PN', 'Pitcairn'), ('PL', 'Poland'), ('PT', 'Portugal'), ('PR', 'Puerto Rico'), ('QA', 'Qatar'), ('RE', 'Reunion'), ('RO', 'Romania'), ('RU', 'Russian Federation'), ('RW', 'Rwanda'), ('BL', 'Saint Barthelemy'), ('SH', 'Saint Helena, Ascension and Tristan da Cunha'), ('KN', 'Saint Kitts and Nevis'), ('LC', 'Saint Lucia'), ('MF', 'Saint Martin (French part)'), ('PM', 'Saint Pierre and Miquelon'), ('VC', 'Saint Vincent and the Grenadines'), ('WS', 'Samoa'), ('SM', 'San Marino'), ('ST', 'Sao Tome and Principe,Sao Tome And Principe'), ('SA', 'Saudi Arabia'), ('SN', 'Senegal'), ('RS', 'Serbia'), ('SC', 'Seychelles'), ('SL', 'Sierra Leone'), ('SG', 'Singapore'), ('SX', 'Sint Maarten (Dutch part)'), ('SK', 'Slovakia'), ('SI', 'Slovenia'), ('SB', 'Solomon Islands'), ('SO', 'Somalia'), ('ZA', 'South Africa'), ('GS', 'South Georgia and the South Sandwich Islands'), ('SS', 'South Sudan'), ('ES', 'Spain'), ('LK', 'Sri Lanka'), ('SD', 'Sudan'), ('SR', 'Suriname'), ('SJ', 'Svalbard and Jan Mayen'), ('SZ', 'Swaziland'), ('SE', 'Sweden'), ('CH', 'Switzerland'), ('SY', 'Syrian Arab Republic'), ('TW', 'Taiwan, Province of China'), ('TJ', 'Tajikistan'), ('TZ', 'Tanzania, United Republic of'), ('TH', 'Thailand'), ('TL', 'Timor-Leste'), ('TG', 'Togo'), ('TK', 'Tokelau'), ('TO', 'Tonga'), ('TT', 'Trinidad and Tobago'), ('TN', 'Tunisia'), ('TR', 'Turkey'), ('TM', 'Turkmenistan'), ('TC', 'Turks and Caicos Islands'), ('TV', 'Tuvalu'), ('UG', 'Uganda'), ('UA', 'Ukraine'), ('AE', 'United Arab Emirates'), ('GB', 'United Kingdom'), ('US', 'United States'), ('UM', 'United States Minor Outlying Islands'), ('UY', 'Uruguay'), ('UZ', 'Uzbekistan'), ('VU', 'Vanuatu'), ('VE', 'Venezuela, Bolivarian Republic of'), ('VN', 'Viet Nam'), ('VG', 'Virgin Islands, British'), ('VI', 'Virgin Islands, U.S.'), ('WF', 'Wallis and Futuna'), ('EH', 'Western Sahara'), ('YE', 'Yemen'), ('ZM', 'Zambia'), ('ZW', 'Zimbabwe')], help_text='The ISO 3166-1 alpha-2 country code of the facility taken directly from the raw data or looked up based on a full country name provided in the raw data.', max_length=2)),

src/django/api/models.py

@@ -265,7 +265,7 @@ class Meta:
         help_text=('When background processing of this item finished. '
                    'Items awaiting or in process will have a null value.'))
     processing_results = postgres.JSONField(
-        default=dict,
+        default=list,
         help_text=('Diagnostic details logged by background processing '
                    'including details returned from the geocoder.'))
     name = models.CharField(

src/django/api/processing.py

@@ -44,20 +44,22 @@ def parse_facility_list_item(item):
         if CsvHeaderField.ADDRESS in fields:
             item.address = values[fields.index(CsvHeaderField.ADDRESS)]
         item.status = FacilityListItem.PARSED
-        item.processing_results[ProcessingAction.PARSE] = {
+        item.processing_results.append({
+            'action': ProcessingAction.PARSE,
             'started_at': started,
             'error': False,
             'finished_at': str(datetime.utcnow()),
-        }
+        })
     except Exception as e:
         item.status = FacilityListItem.ERROR
-        item.processing_results[ProcessingAction.PARSE] = {
+        item.processing_results.append({
+            'action': ProcessingAction.PARSE,
             'started_at': started,
             'error': True,
             'message': str(e),
             'trace': traceback.format_exc(),
             'finished_at': str(datetime.utcnow()),
-        }
+        })
 
 
 def geocode_facility_list_item(item):
@@ -74,22 +76,23 @@ def geocode_facility_list_item(item):
             data["geocoded_point"]["lat"]
         )
         item.geocoded_address = data["geocoded_address"]
-        item.processing_results[ProcessingAction.GEOCODE] = {
+        item.processing_results.append({
+            'action': ProcessingAction.GEOCODE,
             'started_at': started,
             'error': False,
             'data': data["full_response"],
-            'trace': traceback.format_exc(),
             'finished_at': str(datetime.utcnow()),
-        }
+        })
     except Exception as e:
         item.status = FacilityListItem.ERROR
-        item.processing_results[ProcessingAction.GEOCODE] = {
+        item.processing_results.append({
+            'action': ProcessingAction.GEOCODE,
             'started_at': started,
             'error': True,
             'message': str(e),
             'trace': traceback.format_exc(),
             'finished_at': str(datetime.utcnow()),
-        }
+        })
 
 
 def match_facility_list_item(item):
@@ -118,19 +121,21 @@ def match_facility_list_item(item):
                               status=FacilityMatch.AUTOMATIC)
 
         item.status = FacilityListItem.MATCHED
-        item.processing_results[ProcessingAction.MATCH] = {
+        item.processing_results.append({
+            'action': ProcessingAction.MATCH,
             'started_at': started,
             'error': False,
             'finished_at': str(datetime.utcnow()),
-        }
+        })
         return facility, match
     except Exception as e:
         item.status = FacilityListItem.ERROR
-        item.processing_results[ProcessingAction.MATCH] = {
+        item.processing_results.append({
+            'action': ProcessingAction.MATCH,
             'started_at': started,
             'error': True,
             'message': str(e),
             'trace': traceback.format_exc(),
             'finished_at': str(datetime.utcnow()),
-        }
+        })
         return None, None

src/django/api/tests.py

@@ -252,12 +252,20 @@ def test_list_request_by_user_with_no_organization_returns_400(self):
         self.assertEqual(response.status_code, 400)
 
 
-class FacilityListItemParseTest(TestCase):
+class ProcessingTestCase(TestCase):
+    def get_first_status(self, item, action):
+        return next(
+            r for r in item.processing_results if r['action'] == action)
+
+    def assert_status(self, item, action):
+        self.assertIsNotNone(self.get_first_status(item, action))
+
+
+class FacilityListItemParseTest(ProcessingTestCase):
     def assert_successful_parse_results(self, item):
         self.assertEqual(FacilityListItem.PARSED, item.status)
-        self.assertTrue(ProcessingAction.PARSE
-                        in item.processing_results)
-        results = item.processing_results[ProcessingAction.PARSE]
+        self.assert_status(item, ProcessingAction.PARSE)
+        results = self.get_first_status(item, ProcessingAction.PARSE)
         self.assertTrue('error' in results)
         self.assertFalse(results['error'])
         self.assertTrue('started_at' in results)
@@ -266,9 +274,8 @@ def assert_successful_parse_results(self, item):
 
     def assert_failed_parse_results(self, item, message=None):
         self.assertEqual(FacilityListItem.ERROR, item.status)
-        self.assertTrue(ProcessingAction.PARSE
-                        in item.processing_results)
-        results = item.processing_results[ProcessingAction.PARSE]
+        self.assert_status(item, ProcessingAction.PARSE)
+        results = self.get_first_status(item, ProcessingAction.PARSE)
         self.assertTrue('error' in results)
         self.assertTrue(results['error'])
         self.assertTrue('message' in results)
@@ -375,7 +382,7 @@ def test_ungeocodable_address_returns_value_error(self):
         self.assertEqual(cm.exception.args, ('No results were found',))
 
 
-class FacilityListItemGeocodingTest(TestCase):
+class FacilityListItemGeocodingTest(ProcessingTestCase):
     def test_invalid_argument_raises_error(self):
         with self.assertRaises(ValueError) as cm:
             geocode_facility_list_item("hello")
@@ -417,7 +424,7 @@ def test_successfully_geocoded_item_has_correct_results(self):
         self.assertEqual(item.status, FacilityListItem.GEOCODED)
         self.assertIn(
             'results',
-            item.processing_results[ProcessingAction.GEOCODE]['data'],
+            self.get_first_status(item, ProcessingAction.GEOCODE)['data']
         )
 
     def test_failed_geocoded_item_has_error_results(self):
@@ -435,7 +442,7 @@ def test_failed_geocoded_item_has_error_results(self):
         self.assertIsNone(item.geocoded_point)
         self.assertIn(
             'error',
-            item.processing_results[ProcessingAction.GEOCODE],
+            self.get_first_status(item, ProcessingAction.GEOCODE)
         )