fix(bulkdata): #3854 soften polymorphic-lookup validation from ConfigError to warning

cumulusci/tasks/bulkdata/extract.py

@@ -7,7 +7,6 @@
 
 from cumulusci.core.exceptions import (
     BulkDataException,
-    ConfigError,
     CumulusCIException,
     TaskOptionsError,
 )
@@ -368,8 +367,24 @@ def throw(string):  # pragma: no cover
             )
 
             if total_mapping_operations != total_rows:
-                raise ConfigError(
-                    f"Total mapping operations ({total_mapping_operations}) do not match total non-empty rows ({total_rows}) for lookup_key: {lookup_key}. Mention all related tables for lookup: {lookup_key}"
+                # Soft-warn rather than raise: a mismatch here means some
+                # source rows reference records that are not present in the
+                # related table. This is legitimate in many configurations:
+                # polymorphic lookups whose source rows reference records
+                # filtered out of the extract, references to standard org
+                # records (e.g. Standard Pricebook2, PricebookEntry,
+                # FirstPublishLocationId) that are intentionally not
+                # extracted, etc. The strict ``ConfigError`` introduced in
+                # PR #3741 broke ``capture_sample_data`` for these cases; see
+                # issue #3854.
+                self.logger.warning(
+                    f"Total mapping operations ({total_mapping_operations}) "
+                    f"do not match total non-empty rows ({total_rows}) for "
+                    f"lookup_key: {lookup_key}. {total_rows - total_mapping_operations} "
+                    f"row(s) reference records that were not extracted into the "
+                    f"related table(s). The unresolved lookup value(s) will be left "
+                    f"unchanged. If this is unexpected, ensure every related table "
+                    f"is included in the mapping for lookup_key: {lookup_key}."
                 )
         self.session.commit()
 

cumulusci/tasks/bulkdata/tests/test_extract.py

@@ -10,7 +10,6 @@
 
 from cumulusci.core.exceptions import (
     BulkDataException,
-    ConfigError,
     CumulusCIException,
     TaskOptionsError,
 )
@@ -481,8 +480,15 @@ def test_run__poly__wrong_mapping(self, query_op_mock):
 
     @responses.activate
     @mock.patch("cumulusci.tasks.bulkdata.extract.get_query_operation")
-    def test_run__poly__incomplete_mapping(self, query_op_mock):
-        """Test for polymorphic lookups with incomplete mapping file"""
+    def test_run__poly__incomplete_mapping(self, query_op_mock, caplog):
+        """Lookup whose source rows reference records not in the related table.
+
+        Prior to the fix for #3854 this raised ``ConfigError``. That broke
+        legitimate configurations (e.g. ``capture_sample_data`` against orgs
+        where rows reference standard records like the Standard Pricebook
+        which are intentionally not extracted). The current contract: log a
+        warning and continue, leaving unresolved lookup values untouched.
+        """
         base_path = os.path.dirname(__file__)
         mapping_path = os.path.join(base_path, self.mapping_file_poly_incomplete)
         mock_describe_calls()
@@ -520,11 +526,20 @@ def test_run__poly__incomplete_mapping(self, query_op_mock):
             ]
 
             query_op_mock.side_effect = [mock_query_households, mock_query_events]
-            with pytest.raises(ConfigError) as e:
-                task()
+            task()
 
-            assert "Total mapping operations" in str(e.value)
-            assert "do not match total non-empty rows" in str(e.value)
+            warning_messages = [
+                record.message
+                for record in caplog.records
+                if record.levelname == "WARNING"
+                and "Total mapping operations" in record.message
+            ]
+            assert warning_messages, (
+                "Expected a warning about unresolvable lookup rows, got: "
+                f"{[(r.levelname, r.message) for r in caplog.records]}"
+            )
+            assert "do not match total non-empty rows" in warning_messages[0]
+            assert "WhoId" in warning_messages[0]
 
     @mock.patch("cumulusci.tasks.bulkdata.extract.log_progress")
     def test_import_results__oid_as_pk(self, log_mock):
@@ -811,6 +826,58 @@ def test_convert_lookups_to_id__sqlite(self):
         )
         task.session.commit.assert_called_once_with()
 
+    def test_convert_lookups_to_id__unresolvable_lookups_warns_not_raises(self, caplog):
+        """Regression test for #3854.
+
+        When a lookup column has values that do not resolve to any record in the
+        related table (e.g. polymorphic lookups whose source rows reference
+        records filtered out of the extract, or references to standard org
+        records like the Standard Pricebook that are intentionally not
+        extracted), the validation introduced in PR #3741 / commit 2c5d0056e
+        used to raise ``ConfigError``. That broke ``capture_sample_data`` for
+        common real-world configurations (see issue #3854). The expectation
+        post-fix: no exception is raised; a warning is logged instead.
+        """
+        task = _make_task(
+            ExtractData, {"options": {"database_url": "sqlite:///", "mapping": ""}}
+        )
+
+        task.session = mock.Mock()
+        task.models = {
+            "Account": mock.Mock(),
+            "Account_sf_ids": mock.Mock(),
+            "Opportunity": mock.Mock(),
+            "Opportunity_sf_ids": mock.Mock(),
+        }
+        task.mapping = {
+            "Account": MappingStep(sf_object="Account"),
+            "Opportunity": MappingStep(sf_object="Opportunity"),
+        }
+
+        # 0 rows updated (no source values matched any sf_id in related table)
+        task.session.query.return_value.filter.return_value.update.return_value.rowcount = 0
+        # But 3 source rows have non-empty values in the lookup column.
+        task.session.query.return_value.filter.return_value.count.return_value = 3
+
+        # Should NOT raise; should warn and continue.
+        task._convert_lookups_to_id(
+            MappingStep(
+                sf_object="Opportunity",
+                lookups={"AccountId": MappingLookup(table="Account", name="AccountId")},
+            ),
+            ["AccountId"],
+        )
+
+        task.session.commit.assert_called_once_with()
+        # Diagnostic info preserved as a warning.
+        assert any(
+            "AccountId" in record.message and record.levelname == "WARNING"
+            for record in caplog.records
+        ), (
+            "Expected a warning about the unresolvable AccountId lookup, "
+            f"got: {[(r.levelname, r.message) for r in caplog.records]}"
+        )
+
     @mock.patch("cumulusci.tasks.bulkdata.extract.create_table")
     @mock.patch("cumulusci.tasks.bulkdata.extract.mapper")
     def test_create_table(self, mapper_mock, create_mock):