Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions spp_api_v2/tests/test_search_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ def test_search_by_name(self):
self.assertEqual(total, 2)
self.assertEqual(len(records), 2)
names = [r.name for r in records]
self.assertIn("Alice Johnson", names)
self.assertIn("Alice Brown", names)
self.assertIn("JOHNSON, ALICE", names)
self.assertIn("BROWN, ALICE", names)

def test_parse_identifier_param(self):
"""identifier=system|value creates proper domain"""
Expand Down Expand Up @@ -224,7 +224,7 @@ def test_search_combined_params(self):
# Should find Alice Johnson and Alice Brown (both female)
self.assertEqual(total, 2)
for record in records:
self.assertIn("Alice", record.name)
self.assertIn("ALICE", record.name)
self.assertEqual(record.gender_id, self.gender_female)


Expand Down
58 changes: 43 additions & 15 deletions spp_registry/models/individual.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,23 +47,28 @@ class SPPIndividual(models.Model):
# Membership fields
individual_membership_ids = fields.One2many("spp.group.membership", "individual", "Membership to Groups")

# Fields that trigger name recomputation
_name_fields = {"family_name", "given_name", "addl_name"}

@api.model
def _format_individual_name(self, family_name, given_name, addl_name):
"""Compute display name from individual name parts.

Format: "FAMILY_NAME, GIVEN_NAME ADDL_NAME" (uppercase).
Used by onchange (UI), create, and write to ensure consistent naming.
"""
name_vals = [
f"{family_name}," if family_name and (given_name or addl_name) else f"{family_name}" if family_name else "",
given_name,
addl_name,
]
return " ".join(filter(None, name_vals)).upper()

@api.onchange("is_group", "family_name", "given_name", "addl_name")
def name_change(self):
vals = {}
if not self.is_group:
name_vals = [
f"{self.family_name},"
if self.family_name and (self.given_name or self.addl_name)
else f"{self.family_name}"
if self.family_name
else "",
self.given_name,
self.addl_name,
]

name = " ".join(filter(None, name_vals))
vals.update({"name": name.upper()})
self.update(vals)
name = self._format_individual_name(self.family_name, self.given_name, self.addl_name)
self.update({"name": name})

@api.depends("birthdate")
def _compute_calc_age(self):
Expand Down Expand Up @@ -129,8 +134,23 @@ def _recompute_parent_groups(self, records):
self.env.add_to_compute(field, groups)

def write(self, vals):
"""Override to invalidate group metrics when demographics change."""
"""Override to recompute name and invalidate group metrics."""
# Recompute name if any name field changed (only for non-group individuals)
name_updates = {}
if self._name_fields & vals.keys() and not self.env.context.get("skip_name_format"):
for rec in self.filtered(lambda r: not r.is_group):
name_updates[rec.id] = self._format_individual_name(
vals.get("family_name", rec.family_name),
vals.get("given_name", rec.given_name),
vals.get("addl_name", rec.addl_name),
)

res = super().write(vals)

# Apply computed names after super to avoid interfering with the main write
if name_updates:
for rec_id, name in name_updates.items():
super(SPPIndividual, self.browse(rec_id).with_context(skip_name_format=True)).write({"name": name})
Comment on lines +139 to +153
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The write override can be improved for both performance and correctness:

  1. Performance: For single-record updates (the most common case, such as UI edits), you can inject the formatted name directly into vals before calling super().write(). This avoids an extra database write and re-triggering the full write method logic for each record.
  2. Correctness: The filter not r.is_group uses the current database value. If is_group is being changed in the same write call (e.g., converting a group to an individual or vice versa), this logic might incorrectly format or skip the name. It is safer to check vals.get('is_group', r.is_group).
  3. Efficiency: You can store the record object itself in the name_updates dictionary to avoid redundant browse() calls in the second loop.
        name_updates = {}
        if self._name_fields & vals.keys() and not self.env.context.get("skip_name_format"):
            # Optimization: for single record, inject name into vals to avoid extra write
            if len(self) == 1 and not vals.get("is_group", self.is_group):
                vals["name"] = self._format_individual_name(
                    vals.get("family_name", self.family_name),
                    vals.get("given_name", self.given_name),
                    vals.get("addl_name", self.addl_name),
                )
            else:
                for rec in self.filtered(lambda r: not vals.get("is_group", r.is_group)):
                    name_updates[rec] = self._format_individual_name(
                        vals.get("family_name", rec.family_name),
                        vals.get("given_name", rec.given_name),
                        vals.get("addl_name", rec.addl_name),
                    )

        res = super().write(vals)

        # Apply computed names after super to avoid interfering with the main write
        if name_updates:
            for rec, name in name_updates.items():
                super(SPPIndividual, rec.with_context(skip_name_format=True)).write({"name": name})

self._recompute_parent_groups(self)

# Fields that affect group-level metrics
Expand Down Expand Up @@ -177,6 +197,14 @@ def _invalidate_parent_group_metrics(self, individuals):

@api.model_create_multi
def create(self, vals_list):
# Compute name from parts before create (injects into vals, no extra write)
for vals in vals_list:
if not vals.get("is_group") and self._name_fields & vals.keys():
vals["name"] = self._format_individual_name(
vals.get("family_name", ""),
vals.get("given_name", ""),
vals.get("addl_name", ""),
)
res = super().create(vals_list)
self._recompute_parent_groups(res)
return res
Loading