|
| 1 | +""" |
| 2 | +Merge duplicate ``Person`` records, driven by a reviewed decisions file. |
| 3 | +
|
| 4 | +Part of the issue #1275 dedup work. Over ~15 years the database accumulated |
| 5 | +duplicate ``Person`` rows (one human, several rows from un-deduped imports). |
| 6 | +This command consolidates each cluster into one canonical record, relocating |
| 7 | +**every** related object (publications, talks, posters, grants, awards, |
| 8 | +positions, project roles, news, and the advisor/co-advisor/grad-mentor |
| 9 | +self-references) before deleting the now-empty duplicate. |
| 10 | +
|
| 11 | +Why a management command (not a data migration): migrations are gitignored and |
| 12 | +regenerated per-environment here, so the established pattern is a one-shot |
| 13 | +command verified via the logs (see ``generate_slugs_for_old_news_items`` etc.). |
| 14 | +
|
| 15 | +Safety model: |
| 16 | + * **Dry-run by default.** Without ``--apply`` it prints the plan and changes |
| 17 | + nothing. Pass ``--apply`` to actually mutate. |
| 18 | + * **Idempotent.** A ``merge`` whose source row is already gone is a no-op, so |
| 19 | + re-running an applied decisions file is safe. |
| 20 | + * **Atomic per row.** Each merge runs in ``transaction.atomic()``. |
| 21 | + * **Generic relation walk.** Relations are discovered via |
| 22 | + ``Person._meta.get_fields()`` rather than hardcoded, so a new FK/M2M to |
| 23 | + Person can't silently orphan data. |
| 24 | +
|
| 25 | +Decisions file: CSV with columns ``source_id, action, target_id, note``. |
| 26 | + * ``merge`` — relocate everything from ``source_id`` onto ``target_id``, then |
| 27 | + delete ``source_id``. |
| 28 | + * ``delete`` — delete ``source_id`` (refused unless it has zero references). |
| 29 | + * ``keep`` — no-op; documents a reviewed namesake to leave alone. |
| 30 | +Ids only (no emails) so the reviewed file is safe to commit to the public repo; |
| 31 | +member names are already public. |
| 32 | +
|
| 33 | +Usage: |
| 34 | + python manage.py merge_duplicate_people --decisions dedup_decisions.csv |
| 35 | + python manage.py merge_duplicate_people --decisions dedup_decisions.csv --apply |
| 36 | +""" |
| 37 | + |
| 38 | +import csv |
| 39 | +import logging |
| 40 | + |
| 41 | +from django.core.management.base import BaseCommand, CommandError |
| 42 | +from django.db import transaction |
| 43 | + |
| 44 | +from website.models import Person |
| 45 | +from website.utils.name_utils import normalize_person_name |
| 46 | + |
| 47 | +_logger = logging.getLogger(__name__) |
| 48 | + |
| 49 | +# Scalar (non-relational) fields whose *blank* value on the target is backfilled |
| 50 | +# from the source during a merge. We never overwrite a populated target field, |
| 51 | +# and we deliberately leave the image/cropping fields and the derived url_name / |
| 52 | +# bio_datetime_modified alone (the target is the chosen canonical record; its |
| 53 | +# headshot stays put — see module docstring / issue #1275). |
| 54 | +SCALAR_BACKFILL_FIELDS = [ |
| 55 | + 'email', 'personal_website', 'github', 'twitter', 'linkedin', |
| 56 | + 'mastodon', 'threads', 'bluesky', 'bio', 'next_position', |
| 57 | + 'next_position_url', |
| 58 | +] |
| 59 | + |
| 60 | + |
| 61 | +def _person_reverse_relations(): |
| 62 | + """ |
| 63 | + Return ``(fk_rels, m2m_rels)`` — the auto-created reverse relations that |
| 64 | + point at ``Person``, discovered generically from the model meta. |
| 65 | +
|
| 66 | + ``fk_rels`` are reverse foreign keys / one-to-ones (``ManyToOneRel`` / |
| 67 | + ``OneToOneRel``), including the three Position advisor self-references. |
| 68 | + ``m2m_rels`` are reverse many-to-manys (``ManyToManyRel``), e.g. the sorted |
| 69 | + ``authors`` / ``recipients`` sets and the plain ``News.people`` set. |
| 70 | + """ |
| 71 | + fk_rels, m2m_rels = [], [] |
| 72 | + for field in Person._meta.get_fields(): |
| 73 | + if not (field.is_relation and field.auto_created and not field.concrete): |
| 74 | + continue |
| 75 | + if field.many_to_many: |
| 76 | + m2m_rels.append(field) |
| 77 | + elif field.one_to_many or field.one_to_one: |
| 78 | + fk_rels.append(field) |
| 79 | + return fk_rels, m2m_rels |
| 80 | + |
| 81 | + |
| 82 | +def count_references(person): |
| 83 | + """Total number of objects across all relations pointing at ``person``.""" |
| 84 | + fk_rels, m2m_rels = _person_reverse_relations() |
| 85 | + total = 0 |
| 86 | + for field in fk_rels: |
| 87 | + total += field.related_model.objects.filter(**{field.field.name: person}).count() |
| 88 | + for field in m2m_rels: |
| 89 | + total += getattr(person, field.get_accessor_name()).count() |
| 90 | + return total |
| 91 | + |
| 92 | + |
| 93 | +class Command(BaseCommand): |
| 94 | + help = 'Merge duplicate Person records from a reviewed decisions CSV (dry-run by default).' |
| 95 | + |
| 96 | + def add_arguments(self, parser): |
| 97 | + parser.add_argument( |
| 98 | + '--decisions', required=True, |
| 99 | + help='Path to the decisions CSV (columns: source_id, action, target_id, note).', |
| 100 | + ) |
| 101 | + parser.add_argument( |
| 102 | + '--apply', action='store_true', |
| 103 | + help='Actually perform the merges/deletes. Without this flag the command is a dry-run.', |
| 104 | + ) |
| 105 | + parser.add_argument( |
| 106 | + '--allow-name-mismatch', action='store_true', |
| 107 | + help=('Permit merging two rows whose normalized names differ (e.g. a ' |
| 108 | + 'documented cross-name same-person case). By default such a row ' |
| 109 | + 'is refused — this guards against running a prod-id decisions ' |
| 110 | + 'file against the wrong database (e.g. the test server).'), |
| 111 | + ) |
| 112 | + |
| 113 | + def handle(self, *args, **options): |
| 114 | + decisions = self._read_decisions(options['decisions']) |
| 115 | + apply = options['apply'] |
| 116 | + self.allow_name_mismatch = options['allow_name_mismatch'] |
| 117 | + |
| 118 | + mode = 'APPLY' if apply else 'DRY-RUN' |
| 119 | + self.stdout.write(f"=== merge_duplicate_people [{mode}] — {len(decisions)} decision(s) ===") |
| 120 | + |
| 121 | + for row in decisions: |
| 122 | + action = row['action'] |
| 123 | + if action == 'merge': |
| 124 | + self._handle_merge(row, apply) |
| 125 | + elif action == 'delete': |
| 126 | + self._handle_delete(row, apply) |
| 127 | + elif action == 'keep': |
| 128 | + self.stdout.write(f" keep source={row['source_id']} (namesake / leave alone){self._note(row)}") |
| 129 | + else: |
| 130 | + self.stdout.write(self.style.WARNING( |
| 131 | + f" SKIP unknown action {action!r} for source={row['source_id']}")) |
| 132 | + |
| 133 | + if not apply: |
| 134 | + self.stdout.write(self.style.WARNING( |
| 135 | + "\nDry-run only — no changes made. Re-run with --apply to perform these actions.")) |
| 136 | + |
| 137 | + # ----- decisions parsing ------------------------------------------------- |
| 138 | + |
| 139 | + def _read_decisions(self, path): |
| 140 | + """Parse and validate the decisions CSV into a list of normalized rows.""" |
| 141 | + try: |
| 142 | + with open(path, newline='', encoding='utf-8') as fh: |
| 143 | + rows = list(csv.DictReader(fh)) |
| 144 | + except OSError as exc: |
| 145 | + raise CommandError(f"Could not read decisions file {path!r}: {exc}") |
| 146 | + |
| 147 | + required = {'source_id', 'action'} |
| 148 | + if not rows or not required.issubset({k.strip() for k in rows[0].keys()}): |
| 149 | + raise CommandError( |
| 150 | + "Decisions CSV must have a header row with at least " |
| 151 | + "'source_id' and 'action' columns (plus 'target_id', 'note').") |
| 152 | + |
| 153 | + cleaned = [] |
| 154 | + for i, raw in enumerate(rows, start=2): # row 1 is the header |
| 155 | + row = {(k or '').strip(): (v or '').strip() for k, v in raw.items()} |
| 156 | + if not row.get('source_id'): |
| 157 | + continue # allow blank spacer rows |
| 158 | + row['action'] = row.get('action', '').lower() |
| 159 | + if row['action'] == 'merge' and not row.get('target_id'): |
| 160 | + raise CommandError(f"Row {i}: action 'merge' requires a target_id.") |
| 161 | + cleaned.append(row) |
| 162 | + return cleaned |
| 163 | + |
| 164 | + @staticmethod |
| 165 | + def _note(row): |
| 166 | + return f" # {row['note']}" if row.get('note') else '' |
| 167 | + |
| 168 | + # ----- actions ----------------------------------------------------------- |
| 169 | + |
| 170 | + def _handle_merge(self, row, apply): |
| 171 | + source = self._get_person(row['source_id']) |
| 172 | + if source is None: |
| 173 | + self.stdout.write(f" no-op source={row['source_id']} already gone (merged?){self._note(row)}") |
| 174 | + return |
| 175 | + target = self._get_person(row['target_id']) |
| 176 | + if target is None: |
| 177 | + self.stdout.write(self.style.ERROR( |
| 178 | + f" ERROR target={row['target_id']} not found for source={row['source_id']} — skipping")) |
| 179 | + return |
| 180 | + if source.pk == target.pk: |
| 181 | + self.stdout.write(self.style.ERROR( |
| 182 | + f" ERROR source and target are the same person ({source.pk}) — skipping")) |
| 183 | + return |
| 184 | + |
| 185 | + # Safety guard: refuse to merge two rows whose normalized names differ. |
| 186 | + # The decisions file is keyed by prod ids; this stops a prod-id file from |
| 187 | + # silently merging unrelated people if run against the wrong database |
| 188 | + # (e.g. the test server, where id 328 is someone else entirely). |
| 189 | + if not self.allow_name_mismatch: |
| 190 | + src_key = normalize_person_name(source.first_name, source.last_name) |
| 191 | + tgt_key = normalize_person_name(target.first_name, target.last_name) |
| 192 | + if src_key != tgt_key: |
| 193 | + self.stdout.write(self.style.ERROR( |
| 194 | + f" ERROR name mismatch: {source.pk} ({source.get_full_name()!r}) vs " |
| 195 | + f"{target.pk} ({target.get_full_name()!r}) — skipping. Pass " |
| 196 | + f"--allow-name-mismatch for a deliberate cross-name merge.")) |
| 197 | + _logger.error( |
| 198 | + "merge_duplicate_people: refused name-mismatch merge %s -> %s", |
| 199 | + source.pk, target.pk) |
| 200 | + return |
| 201 | + |
| 202 | + if not apply: |
| 203 | + self.stdout.write( |
| 204 | + f" merge {source.pk} ({source.get_full_name()}, refs={count_references(source)}) " |
| 205 | + f"-> {target.pk} ({target.get_full_name()}, refs={count_references(target)})" |
| 206 | + f"{self._note(row)}") |
| 207 | + return |
| 208 | + |
| 209 | + with transaction.atomic(): |
| 210 | + summary, backfilled = self._merge(source, target) |
| 211 | + moved = ', '.join(f"{k}={v}" for k, v in summary.items()) or 'no related objects' |
| 212 | + backfill = f"; backfilled {', '.join(backfilled)}" if backfilled else '' |
| 213 | + msg = f"merged {row['source_id']} into {target.pk}: {moved}{backfill}" |
| 214 | + self.stdout.write(self.style.SUCCESS(f" {msg}")) |
| 215 | + _logger.info("merge_duplicate_people: %s", msg) |
| 216 | + |
| 217 | + def _handle_delete(self, row, apply): |
| 218 | + source = self._get_person(row['source_id']) |
| 219 | + if source is None: |
| 220 | + self.stdout.write(f" no-op source={row['source_id']} already gone{self._note(row)}") |
| 221 | + return |
| 222 | + refs = count_references(source) |
| 223 | + if refs != 0: |
| 224 | + self.stdout.write(self.style.ERROR( |
| 225 | + f" ERROR refusing to delete {source.pk} ({source.get_full_name()}) — " |
| 226 | + f"has {refs} reference(s); merge it instead")) |
| 227 | + return |
| 228 | + if not apply: |
| 229 | + self.stdout.write(f" delete {source.pk} ({source.get_full_name()}, refs=0){self._note(row)}") |
| 230 | + return |
| 231 | + source.delete() |
| 232 | + self.stdout.write(self.style.SUCCESS(f" deleted {row['source_id']} (was a 0-ref shell)")) |
| 233 | + _logger.info("merge_duplicate_people: deleted shell %s", row['source_id']) |
| 234 | + |
| 235 | + # ----- core merge -------------------------------------------------------- |
| 236 | + |
| 237 | + def _merge(self, source, target): |
| 238 | + """ |
| 239 | + Relocate every relation from ``source`` onto ``target``, backfill blank |
| 240 | + scalar fields, then delete ``source``. Returns ``(summary, backfilled)``. |
| 241 | + Must run inside an atomic block. |
| 242 | + """ |
| 243 | + fk_rels, m2m_rels = _person_reverse_relations() |
| 244 | + summary = {} |
| 245 | + |
| 246 | + # Reverse FKs (incl. advisor / co_advisor / grad_mentor self-refs): just |
| 247 | + # repoint the foreign key. SET_NULL/CASCADE on_delete is irrelevant once |
| 248 | + # nothing points at source. |
| 249 | + for field in fk_rels: |
| 250 | + model, fk = field.related_model, field.field.name |
| 251 | + n = model.objects.filter(**{fk: source}).update(**{fk: target}) |
| 252 | + if n: |
| 253 | + summary[f"{model.__name__}.{fk}"] = n |
| 254 | + |
| 255 | + # Reverse M2Ms: rebuild each related object's ordered author/recipient/ |
| 256 | + # people list with source swapped for target (dropping source if target |
| 257 | + # is already present — dedup). For SortedManyToManyField, .set() assigns |
| 258 | + # sort_value by list position, so author order is preserved. |
| 259 | + for field in m2m_rels: |
| 260 | + forward = field.field.name # 'authors' / 'recipients' / 'people' |
| 261 | + moved = 0 |
| 262 | + for obj in list(getattr(source, field.get_accessor_name()).all()): |
| 263 | + manager = getattr(obj, forward) |
| 264 | + new_members, seen = [], set() |
| 265 | + for member in manager.all(): # ordered for sorted M2M |
| 266 | + replacement = target if member.pk == source.pk else member |
| 267 | + if replacement.pk not in seen: |
| 268 | + seen.add(replacement.pk) |
| 269 | + new_members.append(replacement) |
| 270 | + manager.set(new_members) |
| 271 | + moved += 1 |
| 272 | + if moved: |
| 273 | + summary[f"{field.related_model.__name__}.{forward}"] = moved |
| 274 | + |
| 275 | + # Scalar backfill: fill target's blanks from source; never overwrite. |
| 276 | + backfilled = [] |
| 277 | + for fld in SCALAR_BACKFILL_FIELDS: |
| 278 | + if not getattr(target, fld) and getattr(source, fld): |
| 279 | + setattr(target, fld, getattr(source, fld)) |
| 280 | + backfilled.append(fld) |
| 281 | + if backfilled: |
| 282 | + # Bypass Person.save() (avoids url_name recompute + Star Wars image |
| 283 | + # side effects); we only want the scalar columns written. |
| 284 | + Person.objects.filter(pk=target.pk).update( |
| 285 | + **{fld: getattr(target, fld) for fld in backfilled}) |
| 286 | + |
| 287 | + # Delete the now-orphaned source. Its pre_delete signal removes source's |
| 288 | + # own image file (target's image is untouched, so nothing shared breaks). |
| 289 | + source.delete() |
| 290 | + return summary, backfilled |
| 291 | + |
| 292 | + @staticmethod |
| 293 | + def _get_person(pk): |
| 294 | + if not pk: |
| 295 | + return None |
| 296 | + try: |
| 297 | + return Person.objects.get(pk=int(pk)) |
| 298 | + except (Person.DoesNotExist, ValueError): |
| 299 | + return None |
0 commit comments