fix(importing): use receiver_names for provisional person display name
resolveReceivers passed the slug as both `sourceRef` AND `lastName`, so an unresolved receiver "smith-john" became a provisional Person with lastName="smith-john" — a regression of the existing senderName→Person contract. Fix: zip the parallel `receiver_person_ids` and `receiver_names` columns by position (the normalizer emits them 1:1 like sender_person_id/sender_name). When the names list is shorter than the slugs list, fall back to slug-as-name for the missing entries. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -211,7 +211,7 @@ public class DocumentImporter {
|
||||
// shrunk set on re-import prunes stale links rather than accumulating them. The
|
||||
// "preserve human edits" rule does NOT extend to these collections.
|
||||
private void applyAuthoritativeAssociations(Document doc, CanonicalSheetReader.Row row) {
|
||||
Set<Person> receivers = resolveReceivers(row.get("receiver_person_ids"));
|
||||
Set<Person> receivers = resolveReceivers(row.get("receiver_person_ids"), row.get("receiver_names"));
|
||||
doc.getReceivers().clear();
|
||||
doc.getReceivers().addAll(receivers);
|
||||
attachTag(doc, row.get("tags"));
|
||||
@@ -256,10 +256,18 @@ public class DocumentImporter {
|
||||
return resolvePerson(slug, rawName);
|
||||
}
|
||||
|
||||
private Set<Person> resolveReceivers(String slugs) {
|
||||
// Zips the parallel `receiver_person_ids` and `receiver_names` columns by position so an
|
||||
// unresolved receiver becomes a provisional Person whose lastName is the human name from
|
||||
// `receiver_names`, not the slug. If the names list is shorter than the slugs list (rare —
|
||||
// canonical data zips them 1:1), missing entries fall back to slug-as-name.
|
||||
private Set<Person> resolveReceivers(String slugs, String names) {
|
||||
List<String> slugList = CanonicalSheetReader.splitList(slugs);
|
||||
List<String> nameList = CanonicalSheetReader.splitList(names);
|
||||
Set<Person> receivers = new LinkedHashSet<>();
|
||||
for (String slug : CanonicalSheetReader.splitList(slugs)) {
|
||||
receivers.add(resolvePerson(slug, slug));
|
||||
for (int i = 0; i < slugList.size(); i++) {
|
||||
String slug = slugList.get(i);
|
||||
String name = i < nameList.size() ? nameList.get(i) : slug;
|
||||
receivers.add(resolvePerson(slug, name));
|
||||
}
|
||||
return receivers;
|
||||
}
|
||||
|
||||
@@ -424,6 +424,60 @@ class DocumentImporterTest {
|
||||
&& "Herbert Cram|Clara".equals(d.getReceiverText())));
|
||||
}
|
||||
|
||||
@Test
|
||||
void load_provisionalReceiverUsesHumanNameFromReceiverNames_notSlug(@TempDir Path tempDir) throws Exception {
|
||||
// Regression: resolveReceivers used to pass the slug as both `sourceRef` AND `lastName`,
|
||||
// so an unresolved receiver "smith-john" became a provisional Person with
|
||||
// lastName="smith-john". The fix consumes the parallel `receiver_names` column.
|
||||
ReflectionTestUtils.setField(importer, "importDir", tempDir.toString());
|
||||
Person provisional = Person.builder().id(UUID.randomUUID()).sourceRef("smith-john")
|
||||
.lastName("John Smith").provisional(true).build();
|
||||
when(documentService.findByOriginalFilename("W-0050")).thenReturn(Optional.empty());
|
||||
when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0));
|
||||
when(personService.findBySourceRef("smith-john")).thenReturn(Optional.empty());
|
||||
when(personService.upsertBySourceRef(any())).thenReturn(provisional);
|
||||
Path xlsx = writeDocs(tempDir, docRow("W-0050", "", "",
|
||||
"smith-john", "John Smith", "", "", "", ""));
|
||||
|
||||
importer.load(xlsx.toFile());
|
||||
|
||||
org.mockito.ArgumentCaptor<PersonUpsertCommand> captor =
|
||||
org.mockito.ArgumentCaptor.forClass(PersonUpsertCommand.class);
|
||||
verify(personService).upsertBySourceRef(captor.capture());
|
||||
assertThat(captor.getValue().sourceRef()).isEqualTo("smith-john");
|
||||
assertThat(captor.getValue().lastName()).isEqualTo("John Smith");
|
||||
assertThat(captor.getValue().provisional()).isTrue();
|
||||
}
|
||||
|
||||
@Test
|
||||
void load_provisionalReceiverFallsBackToSlug_whenNamesListShorterThanSlugs(@TempDir Path tempDir) throws Exception {
|
||||
// Parallel-list zip: if the names list is shorter than the slugs list, slugs without a
|
||||
// matching name fall back to slug as the display name. This is the "missing name" case
|
||||
// (rare in canonical data but the contract must define it).
|
||||
ReflectionTestUtils.setField(importer, "importDir", tempDir.toString());
|
||||
Person alice = Person.builder().id(UUID.randomUUID()).sourceRef("alice-jones")
|
||||
.lastName("Alice Jones").provisional(true).build();
|
||||
Person bob = Person.builder().id(UUID.randomUUID()).sourceRef("bob-roe")
|
||||
.lastName("bob-roe").provisional(true).build();
|
||||
when(documentService.findByOriginalFilename("W-0051")).thenReturn(Optional.empty());
|
||||
when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0));
|
||||
when(personService.findBySourceRef("alice-jones")).thenReturn(Optional.empty());
|
||||
when(personService.findBySourceRef("bob-roe")).thenReturn(Optional.empty());
|
||||
when(personService.upsertBySourceRef(any())).thenReturn(alice).thenReturn(bob);
|
||||
Path xlsx = writeDocs(tempDir, docRow("W-0051", "", "",
|
||||
"alice-jones|bob-roe", "Alice Jones", "", "", "", ""));
|
||||
|
||||
importer.load(xlsx.toFile());
|
||||
|
||||
org.mockito.ArgumentCaptor<PersonUpsertCommand> captor =
|
||||
org.mockito.ArgumentCaptor.forClass(PersonUpsertCommand.class);
|
||||
verify(personService, org.mockito.Mockito.times(2)).upsertBySourceRef(captor.capture());
|
||||
assertThat(captor.getAllValues()).extracting(PersonUpsertCommand::sourceRef)
|
||||
.containsExactly("alice-jones", "bob-roe");
|
||||
assertThat(captor.getAllValues()).extracting(PersonUpsertCommand::lastName)
|
||||
.containsExactly("Alice Jones", "bob-roe");
|
||||
}
|
||||
|
||||
// ─── clean date values parse without semantic logic ──────────────────────────────
|
||||
|
||||
@Test
|
||||
|
||||
Reference in New Issue
Block a user