Improved 'foreign-key' error with column-number, if not composite key (…

…#346) * Improved 'foreign-key' error, if not caused by a composite foreign key, adding the cell causing the violation to improve the error details with the column-number * Fixed line too long and removed one-liner version in comments * Fixed foreign-key tests with column-number Co-authored-by: roll <[email protected]>
frictionlessdata · Mar 24, 2020 · 0bc16a3 · 0bc16a3
1 parent e75381b
commit 0bc16a3
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 9 deletions.
diff --git a/goodtables/contrib/checks/foreign_key.py b/goodtables/contrib/checks/foreign_key.py
@@ -68,8 +68,19 @@ def check_row(self, cells):
             if success is None:
                 message = 'Foreign key "{fields}" violation in row {row_number}'
                 message_substitutions = {'fields': foreign_key['fields']}
+
+                # if not a composite foreign-key, add the cell causing the violation to improve the error details
+                # with the column-number
+                error_cell = None
+                if len(foreign_key['fields']) == 1:
+                    for cell in cells:
+                        if cell['header'] == foreign_key['fields'][0]:
+                            error_cell = cell
+                            break
+
                 errors.append(Error(
                     self.__code,
+                    cell=error_cell,
                     row_number=row_number,
                     message=message,
                     message_substitutions=message_substitutions,

diff --git a/tests/contrib/checks/test_foreign_key.py b/tests/contrib/checks/test_foreign_key.py
@@ -75,7 +75,7 @@ def test_foreign_key_self_referenced_resource_violation(log):
     del descriptor['resources'][0]['data'][4]
     report = validate(descriptor, checks=['foreign-key'])
     assert log(report) == [
-        (1, 4, None, 'foreign-key'),
+        (1, 4, 3, 'foreign-key'),
     ]
 
 
@@ -84,7 +84,7 @@ def test_foreign_key_internal_resource_violation(log):
     del descriptor['resources'][1]['data'][4]
     report = validate(descriptor, checks=['foreign-key'])
     assert log(report) == [
-        (1, 5, None, 'foreign-key'),
+        (1, 5, 1, 'foreign-key'),
     ]
 
 
@@ -93,10 +93,10 @@ def test_foreign_key_internal_resource_violation_non_existent(log):
     del descriptor['resources'][1]
     report = validate(descriptor, checks=['foreign-key'])
     assert log(report) == [
-        (1, 2, None, 'foreign-key'),
-        (1, 3, None, 'foreign-key'),
-        (1, 4, None, 'foreign-key'),
-        (1, 5, None, 'foreign-key'),
+        (1, 2, 1, 'foreign-key'),
+        (1, 3, 1, 'foreign-key'),
+        (1, 4, 1, 'foreign-key'),
+        (1, 5, 1, 'foreign-key'),
     ]
 
 
@@ -110,8 +110,8 @@ def test_foreign_key_external_resource_errors(log):
     descriptor = 'data/datapackages_linked_errors/cities/datapackage.json'
     report = validate(descriptor, checks=['structure', 'schema', 'foreign-key'])
     assert log(report) == [
-        (1, 4, None, 'foreign-key'),  # self-referenced
-        (1, 4, None, 'foreign-key'),  # external
+        (1, 4, 1, 'foreign-key'),  # self-referenced
+        (1, 4, 3, 'foreign-key'),  # external
     ]
 
 
@@ -140,5 +140,5 @@ def test_foreign_key_external_resource_remote_datapackage(log):
     }
     report = validate(descriptor, checks=['structure', 'schema', 'foreign-key'])
     assert log(report) == [
-        (1, 3, None, 'foreign-key'),
+        (1, 3, 1, 'foreign-key'),
     ]