Skip to content

Commit 4ef7467

Browse files
stevebaumanstevebauman
authored
Support SkipsOnError with OnEachRow (#4303)
* Add support for SkipsOnError for onEachRow * Add tests * CS fix * Fix PHP 7 errors * Introduce same error logic in else branch * Add more skip on error tests * Spacing * Spacing
1 parent c8a1955 commit 4ef7467

File tree

2 files changed

+254
-1
lines changed

2 files changed

+254
-1
lines changed

‎src/Sheet.php‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Maatwebsite\Excel\Concerns\OnEachRow;
1616
use Maatwebsite\Excel\Concerns\ShouldAutoSize;
1717
use Maatwebsite\Excel\Concerns\SkipsEmptyRows;
18+
use Maatwebsite\Excel\Concerns\SkipsOnError;
1819
use Maatwebsite\Excel\Concerns\ToArray;
1920
use Maatwebsite\Excel\Concerns\ToCollection;
2021
use Maatwebsite\Excel\Concerns\ToModel;
@@ -58,6 +59,7 @@
5859
use PhpOffice\PhpSpreadsheet\Spreadsheet;
5960
use PhpOffice\PhpSpreadsheet\Worksheet\BaseDrawing;
6061
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
62+
use Throwable;
6163

6264
/** @mixin Worksheet */
6365
class Sheet
@@ -298,9 +300,23 @@ public function import($import, int $startRow = 1)
298300
app(RowValidator::class)->validate($toValidate, $import);
299301
$import->onRow($sheetRow);
300302
} catch (RowSkippedException $e) {
303+
} catch (Throwable $e) {
304+
if ($import instanceof SkipsOnError) {
305+
$import->onError($e);
306+
} else {
307+
throw $e;
308+
}
301309
}
302310
} else {
303-
$import->onRow($sheetRow);
311+
try {
312+
$import->onRow($sheetRow);
313+
} catch (Throwable $e) {
314+
if ($import instanceof SkipsOnError) {
315+
$import->onError($e);
316+
} else {
317+
throw $e;
318+
}
319+
}
304320
}
305321
}
306322

‎tests/Concerns/SkipsOnErrorTest.php‎

Lines changed: 237 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,17 @@
44

55
use Illuminate\Database\Eloquent\Model;
66
use Illuminate\Database\QueryException;
7+
use Illuminate\Validation\Rule;
78
use Maatwebsite\Excel\Concerns\Importable;
9+
use Maatwebsite\Excel\Concerns\OnEachRow;
810
use Maatwebsite\Excel\Concerns\SkipsErrors;
911
use Maatwebsite\Excel\Concerns\SkipsOnError;
1012
use Maatwebsite\Excel\Concerns\ToModel;
13+
use Maatwebsite\Excel\Concerns\WithValidation;
14+
use Maatwebsite\Excel\Row;
1115
use Maatwebsite\Excel\Tests\Data\Stubs\Database\User;
1216
use Maatwebsite\Excel\Tests\TestCase;
17+
use Maatwebsite\Excel\Validators\ValidationException;
1318
use PHPUnit\Framework\Assert;
1419
use Throwable;
1520

@@ -113,4 +118,236 @@ public function model(array $row)
113118
'email' => '[email protected]',
114119
]);
115120
}
121+
122+
public function test_can_skip_on_error_when_using_oneachrow_with_validation()
123+
{
124+
$import = new class implements OnEachRow, WithValidation, SkipsOnError
125+
{
126+
use Importable;
127+
128+
public $errors = 0;
129+
public $processedRows = 0;
130+
131+
/**
132+
* @param Row $row
133+
*/
134+
public function onRow(Row $row)
135+
{
136+
$this->processedRows++;
137+
138+
// This will be called for valid rows
139+
$rowArray = $row->toArray();
140+
141+
User::create([
142+
'name' => $rowArray[0],
143+
'email' => $rowArray[1],
144+
'password' => 'secret',
145+
]);
146+
}
147+
148+
/**
149+
* @return array
150+
*/
151+
public function rules(): array
152+
{
153+
return [
154+
'1' => Rule::in(['[email protected]']),
155+
];
156+
}
157+
158+
/**
159+
* @param Throwable $e
160+
*/
161+
public function onError(Throwable $e)
162+
{
163+
Assert::assertInstanceOf(ValidationException::class, $e);
164+
Assert::stringContains($e->getMessage(), 'The selected 1 is invalid');
165+
166+
$this->errors++;
167+
}
168+
};
169+
170+
$import->import('import-users.xlsx');
171+
172+
$this->assertEquals(1, $import->errors);
173+
$this->assertEquals(1, $import->processedRows); // Only the valid row should be processed
174+
175+
// Should have inserted the valid row
176+
$this->assertDatabaseHas('users', [
177+
'email' => '[email protected]',
178+
]);
179+
180+
// Should have skipped inserting the invalid row
181+
$this->assertDatabaseMissing('users', [
182+
'email' => '[email protected]',
183+
]);
184+
}
185+
186+
public function test_can_skip_errors_and_collect_all_errors_when_using_oneachrow_with_validation()
187+
{
188+
$import = new class implements OnEachRow, WithValidation, SkipsOnError
189+
{
190+
use Importable, SkipsErrors;
191+
192+
public $processedRows = 0;
193+
194+
/**
195+
* @param Row $row
196+
*/
197+
public function onRow(Row $row)
198+
{
199+
$this->processedRows++;
200+
201+
// This will be called for valid rows
202+
$rowArray = $row->toArray();
203+
204+
User::create([
205+
'name' => $rowArray[0],
206+
'email' => $rowArray[1],
207+
'password' => 'secret',
208+
]);
209+
}
210+
211+
/**
212+
* @return array
213+
*/
214+
public function rules(): array
215+
{
216+
return [
217+
'1' => Rule::in(['[email protected]']),
218+
];
219+
}
220+
};
221+
222+
$import->import('import-users.xlsx');
223+
224+
$this->assertCount(1, $import->errors());
225+
$this->assertEquals(1, $import->processedRows); // Only the valid row should be processed
226+
227+
/** @var Throwable $e */
228+
$e = $import->errors()->first();
229+
230+
$this->assertInstanceOf(ValidationException::class, $e);
231+
$this->stringContains($e->getMessage(), 'The selected 1 is invalid');
232+
233+
// Should have inserted the valid row
234+
$this->assertDatabaseHas('users', [
235+
'email' => '[email protected]',
236+
]);
237+
238+
// Should have skipped inserting the invalid row
239+
$this->assertDatabaseMissing('users', [
240+
'email' => '[email protected]',
241+
]);
242+
}
243+
244+
public function test_can_skip_on_error_when_exception_thrown_in_onrow()
245+
{
246+
$import = new class implements OnEachRow, SkipsOnError
247+
{
248+
use Importable;
249+
250+
public $errors = 0;
251+
public $processedRows = 0;
252+
253+
/**
254+
* @param Row $row
255+
*/
256+
public function onRow(Row $row)
257+
{
258+
$this->processedRows++;
259+
260+
$rowArray = $row->toArray();
261+
262+
// Throw an exception for the second row (Taylor Otwell)
263+
if ($rowArray[1] === '[email protected]') {
264+
throw new \Exception('Custom error in onRow for Taylor');
265+
}
266+
267+
User::create([
268+
'name' => $rowArray[0],
269+
'email' => $rowArray[1],
270+
'password' => 'secret',
271+
]);
272+
}
273+
274+
/**
275+
* @param Throwable $e
276+
*/
277+
public function onError(Throwable $e)
278+
{
279+
Assert::assertInstanceOf(\Exception::class, $e);
280+
Assert::assertEquals('Custom error in onRow for Taylor', $e->getMessage());
281+
282+
$this->errors++;
283+
}
284+
};
285+
286+
$import->import('import-users.xlsx');
287+
288+
$this->assertEquals(1, $import->errors);
289+
$this->assertEquals(2, $import->processedRows); // Both rows should be processed, but one throws exception
290+
291+
// Should have inserted the valid row
292+
$this->assertDatabaseHas('users', [
293+
'email' => '[email protected]',
294+
]);
295+
296+
// Should have skipped inserting the row that threw exception
297+
$this->assertDatabaseMissing('users', [
298+
'email' => '[email protected]',
299+
]);
300+
}
301+
302+
public function test_can_skip_errors_and_collect_all_errors_when_exception_thrown_in_onrow()
303+
{
304+
$import = new class implements OnEachRow, SkipsOnError
305+
{
306+
use Importable, SkipsErrors;
307+
308+
public $processedRows = 0;
309+
310+
/**
311+
* @param Row $row
312+
*/
313+
public function onRow(Row $row)
314+
{
315+
$this->processedRows++;
316+
317+
$rowArray = $row->toArray();
318+
319+
// Throw an exception for the second row (Taylor Otwell)
320+
if ($rowArray[1] === '[email protected]') {
321+
throw new \RuntimeException('Runtime error in onRow for Taylor');
322+
}
323+
324+
User::create([
325+
'name' => $rowArray[0],
326+
'email' => $rowArray[1],
327+
'password' => 'secret',
328+
]);
329+
}
330+
};
331+
332+
$import->import('import-users.xlsx');
333+
334+
$this->assertCount(1, $import->errors());
335+
$this->assertEquals(2, $import->processedRows); // Both rows should be processed, but one throws exception
336+
337+
/** @var Throwable $e */
338+
$e = $import->errors()->first();
339+
340+
$this->assertInstanceOf(\RuntimeException::class, $e);
341+
$this->assertEquals('Runtime error in onRow for Taylor', $e->getMessage());
342+
343+
// Should have inserted the valid row
344+
$this->assertDatabaseHas('users', [
345+
'email' => '[email protected]',
346+
]);
347+
348+
// Should have skipped inserting the row that threw exception
349+
$this->assertDatabaseMissing('users', [
350+
'email' => '[email protected]',
351+
]);
352+
}
116353
}

0 commit comments

Comments
 (0)