Skip to content

Commit af289d8

Browse files
Copilotdelicatacurtis
Copilot
and
delicatacurtis
committed
Fix GEDCOM storage path handling, add file existence check, job failed() methods, and update tests
Co-authored-by: delicatacurtis <247246500+delicatacurtis@users.noreply.github.com>
1 parent f4d1ad9 commit af289d8

File tree

4 files changed

+139
-6
lines changed

4 files changed

+139
-6
lines changed

‎app/Filament/App/Resources/GedcomResource/Pages/CreateGedcom.php‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,42 @@ protected function afterCreate(): void
4646
return;
4747
}
4848

49-
$fullPath = Storage::disk('private')->path($path);
49+
$disk = Storage::disk('private');
50+
51+
$path = (string) $path;
52+
53+
// If the file landed in livewire-tmp (Livewire's temporary upload directory),
54+
// move it to the permanent gedcom-form-imports directory so storage is organised
55+
// correctly and the file survives queue processing.
56+
if (str_starts_with($path, 'livewire-tmp/') && $disk->exists($path)) {
57+
$newPath = 'gedcom-form-imports/' . basename($path);
58+
$disk->move($path, $newPath);
59+
$record->update(['filename' => $newPath]);
60+
$path = $newPath;
61+
62+
Log::info('CreateGedcom: moved upload from livewire-tmp to gedcom-form-imports', [
63+
'gedcom_id' => $record->getKey(),
64+
'new_path' => $newPath,
65+
]);
66+
}
67+
68+
// Verify the file actually exists before dispatching the job
69+
if (! $disk->exists($path)) {
70+
Log::error('CreateGedcom::afterCreate: file does not exist on private disk, aborting dispatch', [
71+
'gedcom_id' => $record->getKey(),
72+
'path' => $path,
73+
]);
74+
75+
Notification::make()
76+
->title('Import failed')
77+
->body('The uploaded file could not be found. Please try uploading again.')
78+
->danger()
79+
->send();
80+
81+
return;
82+
}
83+
84+
$fullPath = $disk->path($path);
5085
$extension = strtolower(pathinfo($fullPath, PATHINFO_EXTENSION));
5186

5287
// Pre-create the ImportJob so the user can track it immediately

‎app/Jobs/ImportGedcom.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,4 +97,26 @@ public function handle(): int
9797

9898
return 0;
9999
}
100+
101+
/**
102+
* Handle a job failure that occurs outside the try/catch in handle()
103+
* (e.g. serialisation errors, queue-worker crashes, max-attempts exceeded).
104+
* Ensures the ImportJob record is always set to 'failed' with an error message.
105+
*/
106+
public function failed(?Throwable $exception): void
107+
{
108+
if ($this->slug) {
109+
ImportJob::where('slug', $this->slug)->update([
110+
'status' => 'failed',
111+
'error_message' => $exception?->getMessage() ?? 'Job failed unexpectedly.',
112+
]);
113+
}
114+
115+
Log::error('ImportGedcom job failed', [
116+
'file_path' => $this->filePath,
117+
'user_id' => $this->user->getKey(),
118+
'slug' => $this->slug,
119+
'error' => $exception?->getMessage(),
120+
]);
121+
}
100122
}

‎app/Jobs/ImportGrampsXml.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,28 @@ public function handle(): int
109109
return 0;
110110
}
111111

112+
/**
113+
* Handle a job failure that occurs outside the try/catch in handle()
114+
* (e.g. serialisation errors, queue-worker crashes, max-attempts exceeded).
115+
* Ensures the ImportJob record is always set to 'failed' with an error message.
116+
*/
117+
public function failed(?Throwable $exception): void
118+
{
119+
if ($this->slug) {
120+
ImportJob::where('slug', $this->slug)->update([
121+
'status' => 'failed',
122+
'error_message' => $exception?->getMessage() ?? 'Job failed unexpectedly.',
123+
]);
124+
}
125+
126+
Log::error('ImportGrampsXml job failed', [
127+
'file_path' => $this->filePath,
128+
'user_id' => $this->user->getKey(),
129+
'slug' => $this->slug,
130+
'error' => $exception?->getMessage(),
131+
]);
132+
}
133+
112134
/**
113135
* Convert GrampsXML data to GEDCOM format
114136
* This is a basic conversion that maps GrampsXML structure to GEDCOM

‎tests/Feature/Filament/Resources/GedcomResourceTest.php‎

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,8 @@ public function test_export_gedcom_does_not_dispatch_without_authenticated_user(
8484
public function test_after_create_dispatches_import_gedcom_for_ged_file(): void
8585
{
8686
Auth::login($this->user);
87-
Storage::fake('private');
87+
$disk = Storage::fake('private');
88+
$disk->put('gedcom-form-imports/test.ged', '0 HEAD');
8889

8990
$gedcom = Gedcom::create(['filename' => 'gedcom-form-imports/test.ged']);
9091

@@ -101,7 +102,8 @@ public function test_after_create_dispatches_import_gedcom_for_ged_file(): void
101102
public function test_after_create_dispatches_gedcom_job_when_filename_is_array(): void
102103
{
103104
Auth::login($this->user);
104-
Storage::fake('private');
105+
$disk = Storage::fake('private');
106+
$disk->put('gedcom-form-imports/test.ged', '0 HEAD');
105107

106108
// Filament FileUpload may persist as an array even for single-file uploads;
107109
// CreateGedcom::afterCreate must extract the first element safely.
@@ -122,7 +124,8 @@ public function test_after_create_dispatches_gedcom_job_when_filename_is_array()
122124
public function test_after_create_dispatches_import_gramps_xml_for_gramps_file(): void
123125
{
124126
Auth::login($this->user);
125-
Storage::fake('private');
127+
$disk = Storage::fake('private');
128+
$disk->put('gedcom-form-imports/test.gramps', '<database/>');
126129

127130
$gedcom = Gedcom::create(['filename' => 'gedcom-form-imports/test.gramps']);
128131

@@ -153,10 +156,59 @@ public function test_after_create_does_not_dispatch_when_filename_is_empty(): vo
153156
Queue::assertNotPushed(ImportGrampsXml::class);
154157
}
155158

156-
public function test_after_create_pre_creates_import_job_before_dispatch(): void
159+
public function test_after_create_does_not_dispatch_when_file_not_found_on_disk(): void
157160
{
158161
Auth::login($this->user);
159162
Storage::fake('private');
163+
// File is NOT placed in the fake disk; afterCreate should abort gracefully.
164+
165+
$gedcom = Gedcom::create(['filename' => 'gedcom-form-imports/missing.ged']);
166+
167+
$page = new CreateGedcom();
168+
$page->record = $gedcom;
169+
170+
$method = new \ReflectionMethod($page, 'afterCreate');
171+
$method->invoke($page);
172+
173+
Queue::assertNotPushed(ImportGedcom::class);
174+
Queue::assertNotPushed(ImportGrampsXml::class);
175+
}
176+
177+
public function test_after_create_moves_file_from_livewire_tmp_and_dispatches_job(): void
178+
{
179+
Auth::login($this->user);
180+
$disk = Storage::fake('private');
181+
182+
// Simulate a file that Livewire stored in its temporary directory instead of
183+
// the final gedcom-form-imports directory (e.g. when Filament's file-move step
184+
// did not run before afterCreate was called).
185+
$tmpPath = 'livewire-tmp/abcdef-test.ged';
186+
$disk->put($tmpPath, '0 HEAD');
187+
188+
$gedcom = Gedcom::create(['filename' => $tmpPath]);
189+
190+
$page = new CreateGedcom();
191+
$page->record = $gedcom;
192+
193+
$method = new \ReflectionMethod($page, 'afterCreate');
194+
$method->invoke($page);
195+
196+
// The file should have been moved out of livewire-tmp
197+
$disk->assertMissing($tmpPath);
198+
$disk->assertExists('gedcom-form-imports/abcdef-test.ged');
199+
200+
// The Gedcom record should point to the new location
201+
$this->assertEquals('gedcom-form-imports/abcdef-test.ged', $gedcom->fresh()->filename);
202+
203+
// The import job should still be dispatched
204+
Queue::assertPushed(ImportGedcom::class);
205+
}
206+
207+
public function test_after_create_pre_creates_import_job_before_dispatch(): void
208+
{
209+
Auth::login($this->user);
210+
$disk = Storage::fake('private');
211+
$disk->put('gedcom-form-imports/test.ged', '0 HEAD');
160212

161213
$gedcom = Gedcom::create(['filename' => 'gedcom-form-imports/test.ged']);
162214

@@ -177,7 +229,8 @@ public function test_after_create_pre_creates_import_job_before_dispatch(): void
177229
public function test_after_create_dispatches_gedcom_job_with_slug(): void
178230
{
179231
Auth::login($this->user);
180-
Storage::fake('private');
232+
$disk = Storage::fake('private');
233+
$disk->put('gedcom-form-imports/test.ged', '0 HEAD');
181234

182235
$gedcom = Gedcom::create(['filename' => 'gedcom-form-imports/test.ged']);
183236

@@ -222,3 +275,4 @@ public function test_file_upload_component_accepted_file_types_includes_ged_exte
222275
$this->assertContains('text/plain', $acceptedTypes, 'FileUpload should accept text/plain MIME type');
223276
}
224277
}
278+

0 commit comments

Comments
 (0)