Skip to content

Commit 12f1119

Browse files
lekoalalekoala
committed
Fix reversed logic + add test
1 parent 168254d commit 12f1119

File tree

3 files changed

+66
-33
lines changed

3 files changed

+66
-33
lines changed

‎src/ExcelBulkLoader.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -432,11 +432,11 @@ public function findExistingObject($record, $columnMap = [])
432432
$existingRecord = null;
433433
if (is_string($duplicateCheck)) {
434434
// Skip current duplicate check if field value is empty
435-
if (empty($record[$duplicateCheck])) {
435+
if (empty($record[$fieldName])) {
436436
continue;
437437
}
438438

439-
$dbFieldValue = $record[$duplicateCheck];
439+
$dbFieldValue = $record[$fieldName];
440440

441441
// Even if $record['ClassName'] is a subclass, this will work
442442
$existingRecord = DataObject::get($this->objectClass)

‎tests/ExcelImportExportTest.php‎

Lines changed: 62 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,14 @@ class ExcelImportExportTest extends SapphireTest
2828
public function testGetAllFields(): void
2929
{
3030
$fields = ExcelImportExport::allFieldsForClass(Member::class);
31-
$this->assertNotEmpty($fields);
31+
self::assertNotEmpty($fields);
3232
}
3333

3434
public function testExportedFields(): void
3535
{
3636
$fields = ExcelImportExport::exportFieldsForClass(Member::class);
37-
$this->assertNotEmpty($fields);
38-
$this->assertNotContains('Password', $fields);
37+
self::assertNotEmpty($fields);
38+
self::assertNotContains('Password', $fields);
3939
}
4040

4141
public function testCanImportMembers(): void
@@ -48,41 +48,41 @@ public function testCanImportMembers(): void
4848
$loader = new ExcelMemberBulkLoader();
4949
$result = $loader->load(__DIR__ . '/data/members.csv');
5050

51-
$this->assertEquals(1, $result->CreatedCount());
51+
self::assertEquals(1, $result->CreatedCount());
5252

5353
$newCount = Member::get()->count();
5454
$firstGroup = Group::get()->filter('Code', 'Administrators')->first();
5555
$newMembersCount = $firstGroup->Members()->count();
5656

57-
$this->assertEquals($count + 1, $newCount);
58-
$this->assertEquals($membersCount + 1, $newMembersCount, "Groups are not updated");
57+
self::assertEquals($count + 1, $newCount);
58+
self::assertEquals($membersCount + 1, $newMembersCount, "Groups are not updated");
5959

6060
// format is handled according to file extension
6161
$loader = new ExcelMemberBulkLoader();
6262
$result = $loader->load(__DIR__ . '/data/members.xlsx');
6363

64-
$this->assertEquals(1, $result->CreatedCount());
65-
$this->assertEquals(0, $result->UpdatedCount());
64+
self::assertEquals(1, $result->CreatedCount());
65+
self::assertEquals(0, $result->UpdatedCount());
6666

6767
$newCount = Member::get()->count();
6868
$firstGroup = Group::get()->filter('Code', 'Administrators')->first();
6969
$newMembersCount = $firstGroup->Members()->count();
7070

71-
$this->assertEquals($count + 2, $newCount);
72-
$this->assertEquals($membersCount + 2, $newMembersCount, "Groups are not updated");
71+
self::assertEquals($count + 2, $newCount);
72+
self::assertEquals($membersCount + 2, $newMembersCount, "Groups are not updated");
7373

7474
// Loading again does nothing new
7575
$result = $loader->load(__DIR__ . '/data/members.xlsx');
7676

77-
$this->assertEquals(0, $result->CreatedCount());
78-
$this->assertEquals(1, $result->UpdatedCount());
77+
self::assertEquals(0, $result->CreatedCount());
78+
self::assertEquals(1, $result->UpdatedCount());
7979

8080
$newCount = Member::get()->count();
8181
$firstGroup = Group::get()->filter('Code', 'Administrators')->first();
8282
$newMembersCount = $firstGroup->Members()->count();
8383

84-
$this->assertEquals($count + 2, $newCount);
85-
$this->assertEquals($membersCount + 2, $newMembersCount, "Groups are not updated");
84+
self::assertEquals($count + 2, $newCount);
85+
self::assertEquals($membersCount + 2, $newMembersCount, "Groups are not updated");
8686
}
8787

8888
public function testSanitize(): void
@@ -91,7 +91,7 @@ public function testSanitize(): void
9191

9292
$actual = ExcelGridFieldExportButton::sanitizeValue($dangerousInput);
9393
$expected = "\t" . $dangerousInput;
94-
$this->assertEquals($expected, $actual);
94+
self::assertEquals($expected, $actual);
9595
}
9696

9797
public function testImportMultipleClasses(): void
@@ -102,28 +102,28 @@ public function testImportMultipleClasses(): void
102102
$result = $loader->load(__DIR__ . '/data/members-class.csv');
103103

104104
$totalCount = Member::get()->count();
105-
$this->assertEquals(2, $result->CreatedCount());
106-
$this->assertEquals($beforeCount, $totalCount - 2);
105+
self::assertEquals(2, $result->CreatedCount());
106+
self::assertEquals($beforeCount, $totalCount - 2);
107107

108108
// Is a subclass AND a base class
109109
$member = TestExcelMember::get()->filter('Email', '[email protected]')->first();
110-
$this->assertNotEmpty($member);
110+
self::assertNotEmpty($member);
111111
$member = Member::get()->filter('Email', '[email protected]')->first();
112-
$this->assertNotEmpty($member);
112+
self::assertNotEmpty($member);
113113

114114
// Not a subclass
115115
$member = TestExcelMember::get()->filter('Email', '[email protected]')->first();
116-
$this->assertEmpty($member);
116+
self::assertEmpty($member);
117117

118118
// Checking duplicate still works even with custom class
119119
$loader = new ExcelMemberBulkLoader();
120120
$result = $loader->load(__DIR__ . '/data/members-class.csv');
121121

122-
$this->assertEquals(0, $result->CreatedCount());
123-
$this->assertEquals(2, $result->UpdatedCount());
122+
self::assertEquals(0, $result->CreatedCount());
123+
self::assertEquals(2, $result->UpdatedCount());
124124

125125
$newTotalCount = Member::get()->count();
126-
$this->assertEquals($newTotalCount, $totalCount);
126+
self::assertEquals($newTotalCount, $totalCount);
127127
}
128128

129129
public function testImportMigrateClasses(): void
@@ -144,13 +144,13 @@ public function testImportMigrateClasses(): void
144144
]
145145
]);
146146

147-
$this->assertEquals(0, $result->CreatedCount());
148-
$this->assertEquals(1, $result->UpdatedCount());
147+
self::assertEquals(0, $result->CreatedCount());
148+
self::assertEquals(1, $result->UpdatedCount());
149149

150150
// Check that our existing member has the new class
151151
$member = TestExcelMember::get()->filter('Email', $duplicateEmail)->first();
152-
$this->assertNotEmpty($member);
153-
$this->assertEquals($baseMember->ID, $member->ID);
152+
self::assertNotEmpty($member);
153+
self::assertEquals($baseMember->ID, $member->ID);
154154
}
155155

156156
public function testTransaction()
@@ -168,10 +168,10 @@ public function testTransaction()
168168

169169
// First row was not created because it was rolled back
170170
$member = Member::get()->filter('Email', '[email protected]')->first();
171-
$this->assertEmpty($member);
171+
self::assertEmpty($member);
172172

173173
$newTotalCount = Member::get()->count();
174-
$this->assertEquals($newTotalCount, $beforeCount);
174+
self::assertEquals($newTotalCount, $beforeCount);
175175

176176

177177
$loader = new ExcelMemberBulkLoader();
@@ -185,9 +185,40 @@ public function testTransaction()
185185

186186
// First row was created because it was not rolled back
187187
$member = Member::get()->filter('Email', '[email protected]')->first();
188-
$this->assertNotEmpty($member);
188+
self::assertNotEmpty($member);
189189

190190
$newTotalCount = Member::get()->count();
191-
$this->assertEquals($newTotalCount - 1, $beforeCount);
191+
self::assertEquals($newTotalCount - 1, $beforeCount);
192+
}
193+
194+
public function testDuplicateChecks()
195+
{
196+
$loader = new ExcelMemberBulkLoader();
197+
$loader->columnMap = [
198+
'e_mail' => 'Email',
199+
'fn' => 'FirstName',
200+
'sn' => 'Surname',
201+
];
202+
$loader->duplicateChecks = [
203+
'e_mail' => 'Email'
204+
];
205+
$result = $loader->load(__DIR__ . '/data/members-mapped.csv');
206+
207+
self::assertCount(1, $result->Created());
208+
209+
// Load again, should not create any new record
210+
$loader = new ExcelMemberBulkLoader();
211+
$loader->columnMap = [
212+
'e_mail' => 'Email',
213+
'fn' => 'FirstName',
214+
'sn' => 'Surname',
215+
];
216+
// Make sure duplicate checks match the mapping
217+
$loader->duplicateChecks = [
218+
'e_mail' => 'Email'
219+
];
220+
$result = $loader->load(__DIR__ . '/data/members-mapped.csv');
221+
222+
self::assertCount(0, $result->Created());
192223
}
193224
}

‎tests/data/members-mapped.csv‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
e_mail;fn;sn
2+
[email protected];John;Mapped

0 commit comments

Comments
 (0)