fix(spp_api_v2): skip records without identifiers instead of crashing

spp_api_v2/routers/bulk.py

@@ -119,6 +119,8 @@ async def bulk_export(
 
         # Convert to API schema
         data = service.to_api_schema(record, extensions=extension_list)
+        if data is None:  # pragma: no cover — record has no valid identifiers
+            continue
 
         # Apply consent filtering
         filtered_data = consent_service.filter_response(

spp_api_v2/routers/filter.py

@@ -195,6 +195,8 @@ async def search(
             for record in records:
                 last_record_id = record.id
                 data = service.to_api_schema(record, extensions=extension_list)
+                if data is None:  # pragma: no cover — record has no valid identifiers
+                    continue
 
                 if consent_type:
                     partner_id = record.id if resource_config["model"] == "res.partner" else None

spp_api_v2/routers/group.py

@@ -102,6 +102,11 @@ async def read_group(
     # Convert to API schema
     extension_list = extensions.split(",") if extensions else None
     data = service.to_api_schema(group, extensions=extension_list)
+    if data is None:  # pragma: no cover — safety net; identifier lookup above would 404 first
+        raise HTTPException(
+            status_code=404,
+            detail="Group not found",
+        )
 
     # Apply consent filtering
     consent_service = ConsentService(env)
@@ -203,6 +208,8 @@ def search_function(offset, limit):
 
     def consent_filter_function(group):
         group_data = group_service.to_api_schema(group, extensions=extension_list)
+        if group_data is None:
+            return None
         filtered_data = consent_service.filter_response(group.id, api_client, "group", group_data)
         consent_info = filtered_data.pop("_consent", None)
         if consent_info and consent_info.get("status") in (

spp_api_v2/routers/individual.py

@@ -98,6 +98,11 @@ async def read_individual(
     # Convert to API schema
     extension_list = extensions.split(",") if extensions else None
     data = service.to_api_schema(partner, extensions=extension_list)
+    if data is None:  # pragma: no cover — safety net; identifier lookup above would 404 first
+        raise HTTPException(
+            status_code=404,
+            detail="Individual not found",
+        )
 
     # Apply consent filtering
     consent_service = ConsentService(env)
@@ -218,6 +223,8 @@ def search_function(offset, limit):
 
     def consent_filter_function(partner):
         data = individual_service.to_api_schema(partner, extensions=extension_list)
+        if data is None:
+            return None
         filtered_data = consent_service.filter_response(partner.id, api_client, "individual", data)
         consent_info = filtered_data.pop("_consent", None)
         if consent_info and consent_info.get("status") in (

spp_api_v2/routers/program_membership.py

@@ -72,6 +72,11 @@ async def read_program_membership(
 
     # Convert to API schema
     data = service.to_api_schema(membership)
+    if data is None:  # pragma: no cover — safety net; identifier lookup above would 404 first
+        raise HTTPException(
+            status_code=404,
+            detail="ProgramMembership not found",
+        )
 
     # Apply consent filtering for the beneficiary
     consent_service = ConsentService(env)
@@ -164,6 +169,8 @@ def search_function(offset, limit):
     def consent_filter_function(membership):
         try:
             data = service.to_api_schema(membership)
+            if data is None:
+                return None
             filtered_data = consent_service.filter_response(
                 membership.partner_id.id, api_client, "program_membership", data
             )

spp_api_v2/services/group_service.py

@@ -124,7 +124,13 @@ def to_api_schema(self, group, extensions=None) -> dict[str, Any]:
                 identifiers.append(ident_dict)
 
         if not identifiers:
-            raise ValidationError(f"Group {group.name} has no valid external identifiers")
+            _logger.warning(
+                "Skipping group (id=%s): no valid external identifiers. Created by uid=%s on %s.",
+                group.id,
+                group.create_uid.id if group.create_uid else "unknown",
+                group.create_date,
+            )
+            return None
 
         # Build Group resource
         group_data = {

spp_api_v2/services/individual_service.py

@@ -127,8 +127,13 @@ def to_api_schema(self, partner, extensions=None) -> dict[str, Any]:
                 identifiers.append(identifier)
 
         if not identifiers:
-            # Must have at least one identifier per spec
-            raise ValidationError(f"Partner {partner.name} has no valid external identifiers")
+            _logger.warning(
+                "Skipping individual (id=%s): no valid external identifiers. Created by uid=%s on %s.",
+                partner.id,
+                partner.create_uid.id if partner.create_uid else "unknown",
+                partner.create_date,
+            )
+            return None
 
         # Build name (REQUIRED)
         name = {
@@ -749,7 +754,8 @@ def get_groups(self, individual, status: str | None = None, limit: int = 100) ->
         for membership in memberships:
             try:
                 response = membership_to_response(membership)
-                results.append(response)
+                if response is not None:
+                    results.append(response)
             except Exception as e:
                 # Log error but continue processing other memberships
                 # Use group/individual names instead of database ID

spp_api_v2/services/membership_utils.py

@@ -1,12 +1,13 @@
 # Part of OpenSPP. See LICENSE file for full copyright and licensing details.
 """Shared membership utilities for API V2 services."""
 
+import logging
 from typing import Any
 
-from odoo.exceptions import ValidationError
+_logger = logging.getLogger(__name__)
 
 
-def membership_to_response(membership) -> dict[str, Any]:
+def membership_to_response(membership) -> dict[str, Any] | None:
     """
     Convert spp.group.membership record to MembershipResponse schema.
 
@@ -17,16 +18,19 @@ def membership_to_response(membership) -> dict[str, Any]:
         membership: spp.group.membership record
 
     Returns:
-        Dictionary matching MembershipResponse schema
-
-    Raises:
-        ValidationError: If group or individual lacks external identifiers
+        Dictionary matching MembershipResponse schema, or None if
+        group or individual lacks valid external identifiers.
     """
     # Build group reference
     group = membership.group
-    group_id = group.reg_ids[0] if group.reg_ids else None
+    group_id = next((r for r in group.reg_ids if r.namespace_uri and r.value), None)
     if not group_id:
-        raise ValidationError(f"Group {group.name} has no valid external identifiers")
+        _logger.warning(
+            "Skipping membership (id=%s): group (id=%s) has no valid external identifiers.",
+            membership.id,
+            group.id,
+        )
+        return None
 
     group_ref = {
         "reference": f"Group/{group_id.namespace_uri}|{group_id.value}",
@@ -35,9 +39,14 @@ def membership_to_response(membership) -> dict[str, Any]:
 
     # Build individual reference
     individual = membership.individual
-    individual_id = individual.reg_ids[0] if individual.reg_ids else None
+    individual_id = next((r for r in individual.reg_ids if r.namespace_uri and r.value), None)
     if not individual_id:
-        raise ValidationError(f"Individual {individual.name} has no valid external identifiers")
+        _logger.warning(
+            "Skipping membership (id=%s): individual (id=%s) has no valid external identifiers.",
+            membership.id,
+            individual.id,
+        )
+        return None
 
     individual_ref = {
         "reference": f"Individual/{individual_id.namespace_uri}|{individual_id.value}",

spp_api_v2/tests/test_group_api.py

@@ -134,6 +134,30 @@ def test_read_group_not_found(self):
 
         self.assertEqual(response.status_code, 404)
 
+    def test_read_group_no_identifiers_returns_404(self):
+        """GET group without valid identifiers returns 404"""
+        no_id_group = self.create_test_group(
+            name="Will Lose IDs Group",
+            identifier_value="WILL-LOSE-GRP-001",
+        )
+
+        no_consent_client = self.create_api_client(
+            name="No Consent Client For Group 404",
+            scopes=[{"resource": "group", "action": "read"}],
+            require_consent=False,
+            legal_basis="public_interest",
+        )
+        token = self.generate_jwt_token(no_consent_client)
+
+        # Delete all registry IDs to simulate missing identifiers
+        no_id_group.reg_ids.unlink()
+
+        url = f"{self.api_base_url}/urn:openspp:vocab:id-type%23test_household_id|WILL-LOSE-GRP-001"
+        response = self.url_open(url, headers=self._get_headers(token=token))
+
+        # Partner's identifier was deleted, so lookup by identifier returns 404
+        self.assertEqual(response.status_code, 404)
+
     def test_read_group_invalid_format(self):
         """GET with invalid identifier format returns 400"""
         url = f"{self.api_base_url}/INVALID-FORMAT"
@@ -177,6 +201,28 @@ def test_search_groups_success(self):
         self.assertIn("total", data["meta"])
         self.assertIn("data", data)
 
+    def test_search_skips_groups_without_identifiers(self):
+        """Search skips groups without valid identifiers instead of crashing"""
+        no_id = self.env["res.partner"].create(
+            {
+                "name": "No Identifiers Group Search",
+                "is_registrant": True,
+                "is_group": True,
+            }
+        )
+        no_id.reg_ids.unlink()
+
+        response = self.url_open(self.api_base_url, headers=self._get_headers())
+
+        self.assertEqual(response.status_code, 200)
+        data = json.loads(response.content)
+        self.assertIn("data", data)
+        for resource in data.get("data", []):
+            self.assertNotEqual(
+                resource.get("name", ""),
+                "No Identifiers Group Search",
+            )
+
     def test_search_by_name(self):
         """Search with name parameter filters results"""
         url = f"{self.api_base_url}?name=Smith"

spp_api_v2/tests/test_group_service.py

@@ -179,8 +179,8 @@ def test_to_api_schema_missing_identifiers_raises(self):
             }
         )
 
-        with self.assertRaises(ValidationError):
-            self.service.to_api_schema(group)
+        result = self.service.to_api_schema(group)
+        self.assertIsNone(result)
 
     def test_from_api_schema_creates_vals(self):
         """from_api_schema converts API schema to Odoo vals"""
@@ -322,6 +322,67 @@ def test_to_api_schema_no_members(self):
         self.assertNotIn("member", data)
         self.assertNotIn("quantity", data)
 
+    def test_to_api_schema_member_without_identifiers_skipped(self):
+        """Members without valid identifiers are skipped from member list"""
+        # Create an individual without registry IDs
+        no_id_individual = self.env["res.partner"].create(
+            {
+                "name": "No ID Member",
+                "is_registrant": True,
+                "is_group": False,
+            }
+        )
+        group = self.create_test_group(identifier_value="HH-MEMBER-SKIP")
+
+        # Create membership directly (bypassing the API which would validate)
+        self.env["spp.group.membership"].create(
+            {
+                "group": group.id,
+                "individual": no_id_individual.id,
+            }
+        )
+
+        data = self.service.to_api_schema(group)
+
+        # Member without identifiers should be skipped
+        if "member" in data:
+            for member in data["member"]:
+                self.assertNotIn(
+                    "No ID Member",
+                    member.get("entity", {}).get("display", ""),
+                )
+
+    def test_to_api_schema_member_with_empty_value_identifier_skipped(self):
+        """Members with reg_ids lacking value are skipped from member list"""
+        # Create individual with an identifier that has no value
+        ind_with_bad_id = self.create_test_individual(
+            name="Bad Value Member",
+            given_name="Bad",
+            family_name="Value",
+            identifier_value="TEMP-BAD-VAL",
+        )
+        # Clear the value on the registry ID to simulate invalid identifier
+        for reg_id in ind_with_bad_id.reg_ids:
+            reg_id.value = False
+
+        group = self.create_test_group(identifier_value="HH-MEMBER-BAD-VAL")
+        self.env["spp.group.membership"].create(
+            {
+                "group": group.id,
+                "individual": ind_with_bad_id.id,
+            }
+        )
+
+        data = self.service.to_api_schema(group)
+
+        # Member with empty-value identifier should be skipped
+        if "member" in data:
+            for member in data["member"]:
+                self.assertNotIn(
+                    "Bad Value Member",
+                    member.get("entity", {}).get("display", ""),
+                )
+
     def test_create_member_invalid_reference_ignored(self):
         """Invalid member references are logged and ignored"""
         schema = Group(

spp_api_v2/tests/test_individual_api.py

@@ -108,6 +108,34 @@ def test_read_individual_not_found(self):
 
         self.assertEqual(response.status_code, 404)
 
+    def test_read_individual_no_identifiers_returns_404(self):
+        """GET individual without valid identifiers returns 404"""
+        # Create individual with a known identifier, then remove all reg_ids
+        # so to_api_schema returns None and the router returns 404.
+        no_id_partner = self.create_test_individual(
+            name="Will Lose IDs",
+            given_name="Will",
+            family_name="LoseIDs",
+            identifier_value="WILL-LOSE-001",
+        )
+
+        no_consent_client = self.create_api_client(
+            name="No Consent Client For 404",
+            scopes=[{"resource": "individual", "action": "read"}],
+            require_consent=False,
+            legal_basis="public_interest",
+        )
+        token = self.generate_jwt_token(no_consent_client)
+
+        # Delete all registry IDs to simulate missing identifiers
+        no_id_partner.reg_ids.unlink()
+
+        url = f"{self.api_base_url}/urn:openspp:vocab:id-type%23test_national_id|WILL-LOSE-001"
+        response = self.url_open(url, headers=self._get_headers(token=token))
+
+        # Partner's identifier was deleted, so lookup by identifier returns 404
+        self.assertEqual(response.status_code, 404)
+
     def test_read_individual_invalid_format(self):
         """GET with invalid identifier format returns 400"""
         url = f"{self.api_base_url}/INVALID-FORMAT"
@@ -154,6 +182,32 @@ def test_search_individuals_success(self):
         self.assertIn("links", data)
         self.assertIn("total", data["meta"])
 
+    def test_search_skips_individuals_without_identifiers(self):
+        """Search skips individuals without valid identifiers instead of crashing"""
+        # Create individual then remove all registry IDs
+        no_id = self.env["res.partner"].create(
+            {
+                "name": "No Identifiers Search",
+                "is_registrant": True,
+                "is_group": False,
+            }
+        )
+        # Ensure no reg_ids exist
+        no_id.reg_ids.unlink()
+
+        response = self.url_open(self.api_base_url, headers=self._get_headers())
+
+        self.assertEqual(response.status_code, 200)
+        data = json.loads(response.content)
+        # Should succeed — no-identifier record silently skipped
+        self.assertIn("data", data)
+        # Verify the no-identifier individual is not in results
+        for resource in data.get("data", []):
+            self.assertNotEqual(
+                resource.get("name", {}).get("given", ""),
+                "No Identifiers Search",
+            )
+
     def test_search_by_name(self):
         """Search with name parameter filters results"""
         url = f"{self.api_base_url}?name=Jane"