diff --git a/app/Actions/Teams/DownloadTeamDataAction.php b/app/Actions/Teams/DownloadTeamDataAction.php index d1ba17164..e07e5056f 100644 --- a/app/Actions/Teams/DownloadTeamDataAction.php +++ b/app/Actions/Teams/DownloadTeamDataAction.php @@ -25,6 +25,7 @@ public function run(User $user, Team $team, array $dateFilter = []) /* Dispatch job to create CSV file for export */ (new CreateCSVExport(null, null, $team->id, null, $dateFilter)) + ->notifyOnFailure($user->email) ->queue($path, 's3', null, ['visibility' => 'public']) ->chain([ // These jobs are executed when above is finished. diff --git a/app/Exports/CreateCSVExport.php b/app/Exports/CreateCSVExport.php index ac85b18d8..8820afac4 100644 --- a/app/Exports/CreateCSVExport.php +++ b/app/Exports/CreateCSVExport.php @@ -14,6 +14,9 @@ use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; +use Illuminate\Support\Facades\Log; +use Illuminate\Support\Facades\Mail; +use App\Mail\ExportFailed; use Maatwebsite\Excel\Concerns\FromQuery; use Maatwebsite\Excel\Concerns\Exportable; @@ -25,6 +28,8 @@ class CreateCSVExport implements FromQuery, WithMapping, WithHeadings use Exportable, Dispatchable, InteractsWithQueue, Queueable, SerializesModels; public $location_type, $location_id, $team_id, $user_id; + /** @var string|null recipient for failure notification */ + public ?string $notifyEmail = null; /** @var array */ private $dateFilter; @@ -180,7 +185,7 @@ public function map($row): array { $result = [ $row->id, - $row->verified->value, + $row->verified?->value ?? 0, $row->model, $row->datetime, $row->created_at, @@ -283,6 +288,43 @@ public function query() ); } + /** + * Fluent setter for the recipient email — used to notify the user if the export fails. + */ + public function notifyOnFailure(?string $email): self + { + $this->notifyEmail = $email; + + return $this; + } + + /** + * Called by Maatwebsite Excel when any queued sheet job fails. + * We email the user so they aren't left waiting forever for a download that will never arrive. + */ + public function failed(\Throwable $e): void + { + Log::error('CreateCSVExport failed', [ + 'user_id' => $this->user_id, + 'team_id' => $this->team_id, + 'location_type' => $this->location_type, + 'location_id' => $this->location_id, + 'notifyEmail' => $this->notifyEmail, + 'error' => $e->getMessage(), + ]); + + if ($this->notifyEmail) { + try { + Mail::to($this->notifyEmail)->send(new ExportFailed()); + } catch (\Throwable $mailError) { + Log::error('Failed to send ExportFailed mail', [ + 'to' => $this->notifyEmail, + 'error' => $mailError->getMessage(), + ]); + } + } + } + /** * Apply the export scope (user/team/location + date filter + verification) to a query. */ diff --git a/app/Http/Controllers/DownloadControllerNew.php b/app/Http/Controllers/DownloadControllerNew.php index 0945cfac9..5bd61df40 100644 --- a/app/Http/Controllers/DownloadControllerNew.php +++ b/app/Http/Controllers/DownloadControllerNew.php @@ -69,6 +69,7 @@ public function index (Request $request) /* Dispatch job to create CSV file for export */ (new CreateCSVExport($request->locationType, $location_id)) + ->notifyOnFailure($email) ->queue($path, 's3', null, ['visibility' => 'public']) ->chain([ // These jobs are executed when above is finished. diff --git a/app/Http/Controllers/User/ProfileController.php b/app/Http/Controllers/User/ProfileController.php index df7b5d3d1..bfb1b89f5 100644 --- a/app/Http/Controllers/User/ProfileController.php +++ b/app/Http/Controllers/User/ProfileController.php @@ -49,6 +49,7 @@ public function download (Request $request) /* Dispatch job to create CSV file for export */ (new CreateCSVExport(null, null, null, $user->id, $dateFilter)) + ->notifyOnFailure($user->email) ->queue($path, 's3', null, ['visibility' => 'public']) ->chain([ // These jobs are executed when above is finished. diff --git a/app/Mail/ExportFailed.php b/app/Mail/ExportFailed.php new file mode 100644 index 000000000..55f3a3366 --- /dev/null +++ b/app/Mail/ExportFailed.php @@ -0,0 +1,19 @@ +from('info@openlittermap.com') + ->subject('OpenLitterMap Data Export — Failed') + ->view('emails.downloads.export_failed'); + } +} diff --git a/database/migrations/2026_04_08_120000_upgrade_subscriptions_to_cashier_v15.php b/database/migrations/2026_04_08_120000_upgrade_subscriptions_to_cashier_v15.php new file mode 100644 index 000000000..8159929cb --- /dev/null +++ b/database/migrations/2026_04_08_120000_upgrade_subscriptions_to_cashier_v15.php @@ -0,0 +1,194 @@ +renameColumn('name', 'type'); + }); + } + + // Rename `stripe_plan` → `stripe_price` (Cashier v13) + if (Schema::hasColumn('subscriptions', 'stripe_plan') && ! Schema::hasColumn('subscriptions', 'stripe_price')) { + Schema::table('subscriptions', function (Blueprint $t) { + $t->renameColumn('stripe_plan', 'stripe_price'); + }); + } + + // Make stripe_price nullable (Cashier v13 — sub may have multiple items, no top-level price) + Schema::table('subscriptions', function (Blueprint $t) { + $t->string('stripe_price')->nullable()->change(); + }); + + // Drop legacy `stripe_active` (Cashier v9 era — replaced by stripe_status) + if (Schema::hasColumn('subscriptions', 'stripe_active')) { + Schema::table('subscriptions', function (Blueprint $t) { + $t->dropColumn('stripe_active'); + }); + } + + // Add unique on stripe_id (Cashier expects this; webhook lookups depend on uniqueness). + // Existence-checked rather than try/catch so genuine errors (e.g. duplicate data) + // surface loudly instead of being swallowed. + $subIndexes = collect(DB::select('SHOW INDEX FROM subscriptions')) + ->pluck('Key_name')->unique()->all(); + + if (! in_array('subscriptions_stripe_id_unique', $subIndexes, true)) { + Schema::table('subscriptions', function (Blueprint $t) { + $t->unique('stripe_id'); + }); + } + + // ─── subscription_items ───────────────────────────────────────────── + // Drop the legacy unique key that referenced `stripe_plan` (Cashier v15's index is non-unique + // on (subscription_id, stripe_price); per-row uniqueness is enforced via stripe_id instead). + $itemsIndexes = collect(DB::select('SHOW INDEX FROM subscription_items')) + ->pluck('Key_name')->unique()->all(); + + if (in_array('subscription_items_subscription_id_stripe_plan_unique', $itemsIndexes, true)) { + Schema::table('subscription_items', function (Blueprint $t) { + $t->dropUnique('subscription_items_subscription_id_stripe_plan_unique'); + }); + } + + if (in_array('subscription_items_stripe_id_index', $itemsIndexes, true)) { + Schema::table('subscription_items', function (Blueprint $t) { + $t->dropIndex('subscription_items_stripe_id_index'); + }); + } + + // Rename stripe_plan → stripe_price + if (Schema::hasColumn('subscription_items', 'stripe_plan') && ! Schema::hasColumn('subscription_items', 'stripe_price')) { + Schema::table('subscription_items', function (Blueprint $t) { + $t->renameColumn('stripe_plan', 'stripe_price'); + }); + } + + // Add stripe_product (Cashier v13 — Stripe Product id, separate from Price id). + // Nullable so backfilled rows from old Cashier installs don't violate NOT NULL; + // Cashier's webhook handler always writes it on the next event, so it self-heals. + if (! Schema::hasColumn('subscription_items', 'stripe_product')) { + Schema::table('subscription_items', function (Blueprint $t) { + $t->string('stripe_product')->nullable()->after('stripe_id'); + }); + } + + // Make quantity nullable (Cashier v13 — metered prices may have null quantity) + Schema::table('subscription_items', function (Blueprint $t) { + $t->integer('quantity')->nullable()->change(); + }); + + // Re-add Cashier-shape indexes — existence-checked, not try/catch. + $itemsIndexes = collect(DB::select('SHOW INDEX FROM subscription_items')) + ->pluck('Key_name')->unique()->all(); + + if (! in_array('subscription_items_stripe_id_unique', $itemsIndexes, true)) { + Schema::table('subscription_items', function (Blueprint $t) { + $t->unique('stripe_id'); + }); + } + + if (! in_array('subscription_items_subscription_id_stripe_price_index', $itemsIndexes, true)) { + Schema::table('subscription_items', function (Blueprint $t) { + $t->index(['subscription_id', 'stripe_price']); + }); + } + + // ─── users (Cashier v13 payment-method column rename) ─────────────── + // Cashier v15's ManagesPaymentMethods writes $user->pm_type and $user->pm_last_four + // (renamed from card_brand / card_last_four in Cashier v13). Without this rename, + // every payment-method update or `customer.updated` webhook fatals the same way the + // subscriptions schema bug did. + if (Schema::hasColumn('users', 'card_brand') && ! Schema::hasColumn('users', 'pm_type')) { + Schema::table('users', function (Blueprint $t) { + $t->renameColumn('card_brand', 'pm_type'); + }); + } + + if (Schema::hasColumn('users', 'card_last_four') && ! Schema::hasColumn('users', 'pm_last_four')) { + Schema::table('users', function (Blueprint $t) { + $t->renameColumn('card_last_four', 'pm_last_four'); + }); + } + } + + public function down(): void + { + // Reverse to legacy shape so a rollback is possible. Data preserved. + if (Schema::hasColumn('users', 'pm_last_four')) { + Schema::table('users', function (Blueprint $t) { + $t->renameColumn('pm_last_four', 'card_last_four'); + }); + } + + if (Schema::hasColumn('users', 'pm_type')) { + Schema::table('users', function (Blueprint $t) { + $t->renameColumn('pm_type', 'card_brand'); + }); + } + + Schema::table('subscription_items', function (Blueprint $t) { + try { $t->dropUnique('subscription_items_stripe_id_unique'); } catch (\Throwable $e) {} + try { $t->dropIndex('subscription_items_subscription_id_stripe_price_index'); } catch (\Throwable $e) {} + }); + + if (Schema::hasColumn('subscription_items', 'stripe_product')) { + Schema::table('subscription_items', function (Blueprint $t) { + $t->dropColumn('stripe_product'); + }); + } + + if (Schema::hasColumn('subscription_items', 'stripe_price')) { + Schema::table('subscription_items', function (Blueprint $t) { + $t->renameColumn('stripe_price', 'stripe_plan'); + }); + } + + Schema::table('subscription_items', function (Blueprint $t) { + $t->integer('quantity')->nullable(false)->change(); + $t->index('stripe_id'); + }); + + Schema::table('subscriptions', function (Blueprint $t) { + try { $t->dropUnique('subscriptions_stripe_id_unique'); } catch (\Throwable $e) {} + }); + + if (! Schema::hasColumn('subscriptions', 'stripe_active')) { + Schema::table('subscriptions', function (Blueprint $t) { + $t->unsignedInteger('stripe_active')->default(0); + }); + } + + if (Schema::hasColumn('subscriptions', 'stripe_price')) { + Schema::table('subscriptions', function (Blueprint $t) { + $t->renameColumn('stripe_price', 'stripe_plan'); + }); + } + + if (Schema::hasColumn('subscriptions', 'type')) { + Schema::table('subscriptions', function (Blueprint $t) { + $t->renameColumn('type', 'name'); + }); + } + } +}; diff --git a/package.json b/package.json index 0f371bed5..ca228f4f7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "openlittermap-web", - "version": "5.8.2", + "version": "5.8.4", "type": "module", "author": "Seán Lynch", "license": "GPL v3", diff --git a/readme/changelog/2026-04-08.md b/readme/changelog/2026-04-08.md new file mode 100644 index 000000000..556a64ae5 --- /dev/null +++ b/readme/changelog/2026-04-08.md @@ -0,0 +1,4 @@ +# 2026-04-08 + +- 5.8.4 — Fix Stripe webhook fatal "Unknown column 'type'" — bring `subscriptions`, `subscription_items` AND `users` up to Cashier v15 shape (rename `subscriptions.name`→`type`, `stripe_plan`→`stripe_price`, drop `stripe_active`, add `stripe_product`, nullable `quantity`, unique `stripe_id`, Cashier-shape indexes; rename `users.card_brand`→`pm_type`, `card_last_four`→`pm_last_four` so payment-method updates don't fatal next). Idempotent migration verified against `olm_postmig_1` (54 prod-like rows preserved, full round-trip). 4 feature tests pin the post-migration write paths used by `Cashier\Http\Controllers\WebhookController::handleCustomerSubscriptionUpdated()`. CSV export tests also gained a `method_exists` guard on `CreateCSVExport::failed()` so the production failure-notification path can't silently break via a rename. +- 5.8.3 — Fix data export fatal (`VerificationStatus` enum stringify in PhpSpreadsheet) — null-safe `$row->verified?->value ?? 0` in `CreateCSVExport::map()`; add `failed()` hook on the export that emails the requester via new `ExportFailed` mail so users aren't left waiting when a job fails; wired `notifyOnFailure()` into user, team and location export dispatch sites; added regression tests covering stringifiable cells for both user and team branches plus the failure-hook email path (10 tests passing). diff --git a/resources/views/emails/downloads/export_failed.blade.php b/resources/views/emails/downloads/export_failed.blade.php new file mode 100644 index 000000000..4aa7c8141 --- /dev/null +++ b/resources/views/emails/downloads/export_failed.blade.php @@ -0,0 +1,9 @@ + + +

Hi,

+

Unfortunately your OpenLitterMap data export could not be generated. Our team has been notified and is looking into it.

+

Please try requesting the export again in a little while. If the problem persists, reply to this email and we'll help you sort it out.

+

Thank you for helping us clean the planet!

+

— OpenLitterMap

+ + diff --git a/tests/Feature/Cashier/SubscriptionsSchemaTest.php b/tests/Feature/Cashier/SubscriptionsSchemaTest.php new file mode 100644 index 000000000..782081735 --- /dev/null +++ b/tests/Feature/Cashier/SubscriptionsSchemaTest.php @@ -0,0 +1,120 @@ +create(); + + $sub = new Subscription(); + $sub->user_id = $user->id; + $sub->type = 'default'; + $sub->stripe_id = 'sub_test_' . uniqid(); + $sub->stripe_status = 'past_due'; + $sub->stripe_price = 'price_startup'; + $sub->quantity = 1; + $sub->save(); + + $this->assertDatabaseHas('subscriptions', [ + 'id' => $sub->id, + 'type' => 'default', + 'stripe_status' => 'past_due', + 'stripe_price' => 'price_startup', + ]); + } + + public function test_can_update_subscription_status_like_webhook_does() + { + // Mirror Cashier WebhookController::handleCustomerSubscriptionUpdated() line 183 + $user = User::factory()->create(); + $sub = Subscription::create([ + 'user_id' => $user->id, + 'type' => 'default', + 'stripe_id' => 'sub_test_' . uniqid(), + 'stripe_status' => 'active', + 'stripe_price' => 'price_pro', + 'quantity' => 1, + ]); + + // The exact assignment that was fatalling in production + $sub->type = $sub->type ?? 'default'; + $sub->stripe_price = 'price_startup'; + $sub->quantity = 1; + $sub->stripe_status = 'past_due'; + $sub->save(); + + $this->assertSame('past_due', $sub->fresh()->stripe_status); + $this->assertSame('price_startup', $sub->fresh()->stripe_price); + } + + public function test_can_create_subscription_item_with_stripe_product() + { + $user = User::factory()->create(); + $sub = Subscription::create([ + 'user_id' => $user->id, + 'type' => 'default', + 'stripe_id' => 'sub_test_' . uniqid(), + 'stripe_status' => 'active', + 'stripe_price' => 'price_pro', + 'quantity' => 1, + ]); + + $item = $sub->items()->updateOrCreate( + ['stripe_id' => 'si_test_' . uniqid()], + [ + 'stripe_product' => 'prod_test', + 'stripe_price' => 'price_pro', + 'quantity' => 2, + ] + ); + + $this->assertInstanceOf(SubscriptionItem::class, $item); + $this->assertDatabaseHas('subscription_items', [ + 'id' => $item->id, + 'subscription_id' => $sub->id, + 'stripe_product' => 'prod_test', + 'stripe_price' => 'price_pro', + 'quantity' => 2, + ]); + } + + public function test_subscription_item_quantity_is_nullable() + { + // Cashier v13+ allows null quantity for metered prices + $user = User::factory()->create(); + $sub = Subscription::create([ + 'user_id' => $user->id, + 'type' => 'default', + 'stripe_id' => 'sub_test_' . uniqid(), + 'stripe_status' => 'active', + 'quantity' => 1, + ]); + + $item = $sub->items()->create([ + 'stripe_id' => 'si_test_' . uniqid(), + 'stripe_product' => 'prod_metered', + 'stripe_price' => 'price_metered', + 'quantity' => null, + ]); + + $this->assertNull($item->fresh()->quantity); + } +} diff --git a/tests/Unit/Exports/CreateCSVExportTest.php b/tests/Unit/Exports/CreateCSVExportTest.php index 64dddd312..f26075871 100644 --- a/tests/Unit/Exports/CreateCSVExportTest.php +++ b/tests/Unit/Exports/CreateCSVExportTest.php @@ -3,6 +3,8 @@ namespace Tests\Unit\Exports; use App\Exports\CreateCSVExport; +use App\Mail\ExportFailed; +use Illuminate\Support\Facades\Mail; use App\Models\Litter\Tags\BrandList; use App\Models\Litter\Tags\Category; use App\Models\Litter\Tags\CategoryObject; @@ -318,6 +320,81 @@ public function test_brands_formatted_as_delimited_string() $this->assertStringContainsString(';', $brandsValue); } + public function test_mapped_cells_are_all_stringifiable_for_phpspreadsheet() + { + // Regression: PhpSpreadsheet's DefaultValueBinder casts every cell to string. + // A raw backed enum (e.g. VerificationStatus) has no __toString() and fatals the export. + // Guard: every cell returned by map() must be scalar, null, or Stringable. + // Covers BOTH user-export and team-export branches (team branch was the one that + // failed in production — Horizon job 5c86f106, team_id 211). + $team = \App\Models\Teams\Team::factory()->create(); + $user = User::factory()->create(); + $photo = Photo::factory()->create([ + 'verified' => 2, + 'user_id' => $user->id, + 'team_id' => $team->id, + 'datetime' => now()->toDateTimeString(), + 'lat' => 42.0, + 'lon' => 42.0, + 'address_array' => ['country' => 'Ireland'], + 'summary' => ['tags' => [], 'totals' => ['litter' => 0, 'materials' => 0, 'brands' => 0, 'custom_tags' => 0]], + ]); + + $userExport = new CreateCSVExport(null, null, null, $user->id); + $teamExport = new CreateCSVExport(null, null, $team->id, null); + + foreach ([$userExport, $teamExport] as $export) { + $mapped = $export->map($photo->fresh()); + + foreach ($mapped as $i => $cell) { + $this->assertTrue( + $cell === null || is_scalar($cell) || $cell instanceof \Stringable, + "Cell {$i} is not stringifiable: " . (is_object($cell) ? get_class($cell) : gettype($cell)) + ); + } + + // verified column must be the int value, not the enum instance + $this->assertSame(2, $mapped[1]); + } + } + + public function test_failed_method_exists_with_throwable_signature() + { + // Maatwebsite's ProxyFailures trait calls $this->sheetExport->failed($e) via direct + // method invocation. If someone renames or removes the method, the regression test + // for the email behaviour would still pass (Mail::fake intercepts before the dispatch + // pipeline) but the production failure-notification path would silently break. + // Pin the method existence and signature. + $this->assertTrue(method_exists(CreateCSVExport::class, 'failed')); + + $reflection = new \ReflectionMethod(CreateCSVExport::class, 'failed'); + $params = $reflection->getParameters(); + $this->assertCount(1, $params); + $this->assertSame('Throwable', (string) $params[0]->getType()); + } + + public function test_failed_hook_emails_the_user() + { + Mail::fake(); + + $export = (new CreateCSVExport(null, null, null, 1)) + ->notifyOnFailure('user@example.com'); + + $export->failed(new \RuntimeException('boom')); + + Mail::assertSent(ExportFailed::class, fn ($mail) => $mail->hasTo('user@example.com')); + } + + public function test_failed_hook_is_noop_without_notify_email() + { + Mail::fake(); + + $export = new CreateCSVExport(null, null, null, 1); + $export->failed(new \RuntimeException('boom')); + + Mail::assertNothingSent(); + } + public function test_types_are_mapped_from_summary() { $category = Category::with(['litterObjects' => fn ($q) => $q->orderBy('litter_objects.id')])