From 219697c883a3ff85de364f3d36c76b62dbb6053e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Se=C3=A1n=20L=2E?= Date: Mon, 14 Sep 2026 19:52:53 +0100 Subject: [PATCH 1/5] simplify branch --- .ai/skills/tagging-system/SKILL.md | 7 + .gitignore | 1 + CLAUDE.md | 7 +- app/Actions/QuickTags/SyncQuickTagsAction.php | 10 + app/Actions/Tags/AddTagsToPhotoAction.php | 67 ++- app/Console/Commands/Tags/MigrateTag.php | 288 ++++++++++ .../Brands/AutoCreateBrandRelationships.php | 4 + .../Controllers/API/QuickTagsController.php | 1 + .../API/Tags/GetTagsController.php | 8 +- .../Requests/Api/ReplacePhotoTagsRequest.php | 1 + app/Models/Litter/Tags/LitterObject.php | 2 + app/Services/Metrics/MetricsService.php | 18 +- app/Services/Redis/RedisMetricsCollector.php | 66 ++- app/Services/Tags/ResolveLitterObject.php | 63 +++ ...merged_into_id_to_litter_objects_table.php | 34 ++ ..._merged_into_type_id_to_litter_objects.php | 23 + package-lock.json | 4 +- package.json | 2 +- phpunit.xml | 3 +- readme/API.md | 11 + readme/ArtisanCommands.md | 3 +- readme/PostTagMigrationClean.md | 141 +++++ .../audit/LitterObjectInventory-2026-08.csv | 199 +++++++ readme/changelog/2026-09-14.md | 12 + tests/CreatesApplication.php | 25 + .../Achievements/LongTermAchievementsTest.php | 2 +- tests/Feature/Photos/AddTagsToPhotoTest.php | 3 +- tests/Feature/Photos/PhotoSignedUrlTest.php | 8 + tests/Feature/QuickTags/QuickTagsApiTest.php | 4 + tests/Feature/Tags/AddNewTagsToPhotosTest.php | 34 +- tests/Feature/Tags/MigrateTagResumeTest.php | 86 +++ tests/Feature/Tags/MigrateTagTest.php | 491 ++++++++++++++++++ tests/TestCase.php | 11 +- .../Achievements/TagKeyCacheZeroGuardTest.php | 2 +- .../RedisMetricsCollectorLocationTest.php | 4 +- .../Unit/Redis/RedisMetricsCollectorTest.php | 4 +- .../Unit/Services/LocationServiceTagTest.php | 2 +- tests/Unit/TestDatabaseSafetyTest.php | 67 +++ 38 files changed, 1602 insertions(+), 116 deletions(-) create mode 100644 app/Console/Commands/Tags/MigrateTag.php create mode 100644 app/Services/Tags/ResolveLitterObject.php create mode 100644 database/migrations/2026_08_08_182838_add_retired_at_and_merged_into_id_to_litter_objects_table.php create mode 100644 database/migrations/2026_09_14_000001_add_merged_into_type_id_to_litter_objects.php create mode 100644 readme/PostTagMigrationClean.md create mode 100644 readme/audit/LitterObjectInventory-2026-08.csv create mode 100644 readme/changelog/2026-09-14.md create mode 100644 tests/Feature/Tags/MigrateTagResumeTest.php create mode 100644 tests/Feature/Tags/MigrateTagTest.php create mode 100644 tests/Unit/TestDatabaseSafetyTest.php diff --git a/.ai/skills/tagging-system/SKILL.md b/.ai/skills/tagging-system/SKILL.md index aa18e002a..ede609aab 100644 --- a/.ai/skills/tagging-system/SKILL.md +++ b/.ai/skills/tagging-system/SKILL.md @@ -38,6 +38,13 @@ V5 uses a normalized hierarchy: Photo -> PhotoTag (category + object + quantity) 7. **No unique constraint on `photo_tags` for (CLO, type) pairs.** There is no DB-level unique constraint on `(photo_id, category_litter_object_id, litter_object_type_id)`. Duplicate CLO+type pairs are possible (each is a separate PhotoTag row). Do NOT assume uniqueness. Extra-tag deduplication (materials/brands within a single tag) is handled via `upsert` inside a single PhotoTag's extra tags, not across multiple PhotoTag rows. 8. **`getNewTags()` serializer contract.** `UsersUploadsController::getNewTags()` conditionally includes `category` and `object` only when both `category_id` and `litter_object_id` resolve. For extra-tag-only PhotoTags (brand/material/custom-only), `category` and `object` are returned as `null`. Always includes `litter_object_type_id` (may be null), `quantity`, `picked_up` (cast to bool with photo-level fallback), and `extra_tags` array. +## Object retirement + +- `olm:migrate-tag OLD NEW` previews one category-preserving object migration; `--apply` updates observations and records the replacement on the old object. +- `ResolveLitterObject` follows `retired_at`, `merged_into_id` and `merged_into_type_id` before choosing the destination CLO. No source CLO or global seeder is required. +- The latest recorded replacement type wins. Without a recorded type, invalid client types return 422. Explicit object-format categories are preserved or rejected. +- See `readme/PostTagMigrationClean.md` for the write freeze, rehearsal, Redis recovery and follow-up scope. + ## Patterns ### Creating a tag with extras diff --git a/.gitignore b/.gitignore index 2a62e6442..43cfdd3a3 100644 --- a/.gitignore +++ b/.gitignore @@ -19,3 +19,4 @@ yarn-error.log .eslintignore .DS_Store /storage/seeders/brands.txt +readme/audit/TagCleanupReview-2026-09-12.csv diff --git a/CLAUDE.md b/CLAUDE.md index 44e0b3242..589c24c88 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -109,8 +109,8 @@ tests/ ## CI (GitHub Actions) -Runs on push to `master`, `staging`, `upgrade/tagging-2025` and PRs. -Pipeline: PHP 8.2, Node 18, MySQL 5.7, Redis 7 — composer install, npm build, phpunit. +Runs on pushes and PRs targeting `master` or `upgrade/tagging-2025`. +Pipeline: PHP 8.3, MySQL 8.0, Redis 7 — composer install, migrations and PHPUnit. No Node or frontend build step. ## Current State (v5 — shipped to `master`) @@ -140,6 +140,7 @@ Built by a single developer over 17 years. - `photo_tags` table uses FK columns (`category_id`, `litter_object_id`), NOT string columns. All three (`category_litter_object_id`, `category_id`, `litter_object_id`) are **nullable** — extra-tag-only PhotoTags have null CLO - `AddTagsToPhotoAction` (v5) auto-resolves category from object — frontend need not send category. Brand-only, material-only, and custom-only tags use `createExtraTagOnly()` with null CLO - Replace tags (`PUT /api/v3/tags`) accepts empty `tags: []` to clear all tags from a photo +- Object retirement uses `litter_objects.retired_at`, `merged_into_id` and `merged_into_type_id`; resolve the object replacement before choosing its CLO. `olm:migrate-tag` needs no source CLO or production seeder. See `readme/PostTagMigrationClean.md`. - `TagsConfig` provides helper methods: `buildObjectMap()`, `buildObjectMaps()`, `allMaterialKeys()`, `allTypeKeys()` — use these instead of hardcoding lists - Legacy v1/v2 mobile endpoints removed (2026-03-01) — mobile uses v3 endpoints with CLO format only - `Photo` model has `SoftDeletes` trait but all delete endpoints use `forceDelete()` for hard deletion. Cascading FKs on `photo_tags` (→ `photo_tag_extras`) handle relationship cleanup @@ -153,7 +154,7 @@ Built by a single developer over 17 years. - Points API returns `page` (not `current_page`) at root level — frontend normalizes to `current_page` - Nav.vue `isAdmin` check includes `'superadmin'` role (not just `'admin'` and `'helper'`) - `MetricsService` is the SINGLE WRITER for all metrics (MySQL `metrics` table + Redis). Never use `DB::table('metrics')->increment()` or `Redis::hincrby()` directly -- Redis is a derived cache, rebuildable from `metrics` table. Never treat Redis as source of truth +- Redis is a derived cache; per-object counts rebuild from processed photo snapshots, not the totals-only `metrics` table. Never treat Redis as source of truth - Location tables store identity only (no aggregate counters). Stats live in Redis + `metrics` table - `auth:sanctum` routes in tests: use `actingAs($user)` with NO guard arg, NOT `actingAs($user, 'api')` - `result_string` and `total_litter` columns are write-only — all active endpoints use `summary` and `total_tags` diff --git a/app/Actions/QuickTags/SyncQuickTagsAction.php b/app/Actions/QuickTags/SyncQuickTagsAction.php index 76bbba304..cad5e126c 100644 --- a/app/Actions/QuickTags/SyncQuickTagsAction.php +++ b/app/Actions/QuickTags/SyncQuickTagsAction.php @@ -3,6 +3,7 @@ namespace App\Actions\QuickTags; use App\Models\Users\User; +use App\Services\Tags\ResolveLitterObject; use App\Models\Users\UserQuickTag; use Illuminate\Support\Facades\DB; @@ -19,6 +20,15 @@ class SyncQuickTagsAction public function run(User $user, array $tags) { return DB::transaction(function () use ($user, $tags) { + foreach ($tags as &$tag) { + [$clo, $typeId] = app(ResolveLitterObject::class)->fromClo( + (int) $tag['clo_id'], isset($tag['type_id']) ? (int) $tag['type_id'] : null + ); + $tag['clo_id'] = $clo->id; + $tag['type_id'] = $typeId; + } + unset($tag); + UserQuickTag::where('user_id', $user->id)->delete(); $now = now(); diff --git a/app/Actions/Tags/AddTagsToPhotoAction.php b/app/Actions/Tags/AddTagsToPhotoAction.php index da92efeec..d2593a6a8 100644 --- a/app/Actions/Tags/AddTagsToPhotoAction.php +++ b/app/Actions/Tags/AddTagsToPhotoAction.php @@ -13,6 +13,7 @@ use App\Models\Litter\Tags\Materials; use App\Models\Litter\Tags\PhotoTag; use App\Models\Photo; +use App\Services\Tags\ResolveLitterObject; use App\Models\Teams\Team; use App\Models\Users\User; use Illuminate\Support\Facades\DB; @@ -143,33 +144,14 @@ protected function createExtraTagOnly(int $userId, int $photoId, array $tag): Ph */ protected function createTagFromClo(int $userId, int $photoId, array $tag): PhotoTag { - $cloId = $tag['category_litter_object_id']; - $clo = CategoryObject::find($cloId); - - if (! $clo) { - throw ValidationException::withMessages([ - 'tags' => ["Invalid category_litter_object_id: {$cloId}"], - ]); - } - + [$clo, $typeId] = app(ResolveLitterObject::class)->fromClo( + (int) $tag['category_litter_object_id'], + isset($tag['litter_object_type_id']) ? (int) $tag['litter_object_type_id'] : null + ); + $cloId = $clo->id; $quantity = max(1, (int) ($tag['quantity'] ?? 1)); $pickedUp = $tag['picked_up'] ?? null; - // Validate type if provided - $typeId = $tag['litter_object_type_id'] ?? null; - if ($typeId) { - $validType = DB::table('category_object_types') - ->where('category_litter_object_id', $cloId) - ->where('litter_object_type_id', $typeId) - ->exists(); - - if (! $validType) { - throw ValidationException::withMessages([ - 'tags' => ["Type {$typeId} is not valid for CLO {$cloId}"], - ]); - } - } - // Create the PhotoTag $photoTag = PhotoTag::create([ 'photo_id' => $photoId, @@ -194,7 +176,9 @@ protected function createTagFromClo(int $userId, int $photoId, array $tag): Phot } /** - * Legacy format tag creation (backward compatibility for old frontend/mobile). + * - Web editors can send object and category separately instead of category_litter_object_id. + * - Example: { object: "plasticBags", category_id: 8, quantity: 2 }. + * - Resolve retired objects before creating the observation. * * @throws \Exception */ @@ -202,6 +186,23 @@ protected function createTagLegacy(int $userId, int $photoId, array $tag): Photo { [$category, $object, $quantity, $pickedUp] = $this->resolveTag($tag); + if (! $object) { + throw ValidationException::withMessages(['tags' => 'Unknown litter object.']); + } + if ($object->retired_at !== null) { + [$clo, $typeId] = app(ResolveLitterObject::class)->resolve( + $object, $category?->id, + isset($tag['litter_object_type_id']) ? (int) $tag['litter_object_type_id'] : null + ); + $tag['category_litter_object_id'] = $clo->id; + $tag['litter_object_type_id'] = $typeId; + + return $this->createTagFromClo($userId, $photoId, $tag); + } + if (! $category) { + throw ValidationException::withMessages(['tags' => 'Category does not contain object.']); + } + // Resolve CLO from category + object $clo = null; if ($category && $object) { @@ -383,18 +384,16 @@ protected function resolveTag(array $tag): array ? LitterObject::find($tag['object']['id']) : LitterObject::where('key', $tag['object'])->first(); - if ($object) { - // Validate provided category belongs to this object, fall back otherwise - if ($category && ! $object->categories()->where('categories.id', $category->id)->exists()) { - $category = null; - } - - if (! $category) { - $category = $object->categories()->first(); - } + if ($object && $object->retired_at === null && ! $category + && ! isset($tag['category_id']) && ! isset($tag['category'])) { + $category = $object->categories()->first(); } } + if ((isset($tag['category_id']) || isset($tag['category'])) && ! $category) { + throw ValidationException::withMessages(['tags' => 'Unknown category.']); + } + return [ $category, $object, diff --git a/app/Console/Commands/Tags/MigrateTag.php b/app/Console/Commands/Tags/MigrateTag.php new file mode 100644 index 000000000..2eb9c53e8 --- /dev/null +++ b/app/Console/Commands/Tags/MigrateTag.php @@ -0,0 +1,288 @@ +option('apply')) { + $locked = (int) DB::selectOne('SELECT GET_LOCK(?, 0) AS acquired', [self::LOCK_NAME])->acquired === 1; + if (! $locked) { + throw new RuntimeException('Another tag migration is running.'); + } + } + + $source = LitterObject::where('key', $this->argument('old'))->first(); + $destination = LitterObject::where('key', $this->argument('new'))->first(); + if (! $source || ! $destination || $source->id === $destination->id) { + throw new RuntimeException('Choose two different existing object keys.'); + } + $type = $this->option('type') === null ? null : LitterObjectType::where('key', $this->option('type'))->first(); + if ($this->option('type') !== null && $type === null) { + throw new RuntimeException('Unknown replacement type.'); + } + $typeId = $type?->id; + $plan = $this->plan($source, $destination, $typeId); + $this->report($source, $destination, $type, $plan); + if ($plan === [] && $source->retired_at !== null) { + $this->info('Already applied: no source observations or quick tags remain.'); + return self::SUCCESS; + } + if (! $this->option('apply')) { + return self::SUCCESS; + } + + $this->checkConnections(); + $destinations = DB::transaction(function () use ($source, $destination, $typeId) { + $objects = LitterObject::whereIn('id', [$source->id, $destination->id]) + ->orderBy('id')->lockForUpdate()->get()->keyBy('id'); + $source = $objects[$source->id]; + $destination = $objects[$destination->id]; + $plan = $this->plan($source, $destination, $typeId); + $destinations = []; + foreach ($plan as $categoryId => $entry) { + $cloId = $entry['clo_id']; + if ($cloId === null) { + $cloId = CategoryObject::create([ + 'category_id' => $categoryId, 'litter_object_id' => $destination->id, + ])->id; + foreach ($entry['create_types'] as $id) { + DB::table('category_object_types')->insert([ + 'category_litter_object_id' => $cloId, 'litter_object_type_id' => $id, + ]); + } + } + $destinations[$categoryId] = $cloId; + } + $source->update([ + 'retired_at' => $source->retired_at ?? now(), + 'merged_into_id' => $destination->id, + 'merged_into_type_id' => $typeId, + ]); + foreach ($destinations as $categoryId => $cloId) { + $values = ['clo_id' => $cloId]; + if ($typeId !== null) { + $values['type_id'] = $typeId; + } + DB::table('user_quick_tags')->whereIn('clo_id', + DB::table('category_litter_object')->where('litter_object_id', $source->id) + ->where('category_id', $categoryId)->select('id') + )->update($values); + } + return $destinations; + }); + + $moved = 0; + while (true) { + $this->checkConnections(); + $photoIds = $this->sourceRows($source->id)->select('photo_id')->distinct() + ->orderBy('photo_id')->limit(200)->pluck('photo_id'); + if ($photoIds->isEmpty()) { + break; + } + $started = microtime(true); + $count = DB::transaction(function () use ($photoIds, $source, $destination, $typeId, $destinations, $summaries, $metrics) { + $photos = Photo::withTrashed()->whereIn('id', $photoIds)->orderBy('id')->lockForUpdate()->get(); + $count = 0; + foreach ($photos as $photo) { + foreach ($destinations as $categoryId => $cloId) { + $count += $this->sourceRows($source->id)->where('photo_id', $photo->id) + ->where('category_id', $categoryId)->update([ + 'litter_object_id' => $destination->id, + 'category_litter_object_id' => $cloId, + 'litter_object_type_id' => $typeId, + ]); + } + $summaries->run($photo); + if ($photo->processed_at !== null && ! $photo->trashed()) { + $metrics->processPhoto($photo); + } + } + return $count; + }); + if ($count === 0) { + throw new RuntimeException('Source rows remain but the batch made no progress. Keep writes paused and inspect.'); + } + $moved += $count; + $this->line(sprintf('Committed %d photos, %d rows (%.2fs); %d rows moved.', $photoIds->count(), $count, microtime(true) - $started, $moved)); + } + $this->info('Migration complete. Verify summaries, XP, exports and Redis deltas before reopening writes.'); + return self::SUCCESS; + } catch (Throwable $e) { + $this->error($e->getMessage()); + if ($this->option('apply')) { + $this->warn('Keep writes paused. Inspect the failure and resume with the same arguments; Redis may need rebuilding.'); + } + return self::FAILURE; + } finally { + if ($locked) { + DB::selectOne('SELECT RELEASE_LOCK(?) AS released', [self::LOCK_NAME]); + } + } + } + + /** - Build the same read-only checks for preview and the retirement transaction. */ + private function plan(LitterObject $source, LitterObject $destination, ?int $typeId): array + { + if ($source->retired_at !== null) { + if ($source->merged_into_id !== $destination->id || $source->merged_into_type_id !== $typeId) { + throw new RuntimeException('The recorded replacement or type disagrees with these arguments.'); + } + if (! $this->sourceRows($source->id)->exists() && ! $this->quickTags($source->id)->exists()) { + return []; + } + } + if ($destination->retired_at !== null) { + throw new RuntimeException('The destination object is retired.'); + } + foreach (LitterObject::where('merged_into_id', $source->id)->get() as $previous) { + if ($this->sourceRows($previous->id)->exists() || $this->quickTags($previous->id)->exists()) { + throw new RuntimeException("Finish {$previous->key} → {$source->key} before starting this mapping."); + } + } + if ($this->sourceRows($source->id)->whereNull('category_id')->exists()) { + throw new RuntimeException('Source observations have no category. Resolve those before migrating.'); + } + if ($this->sourceRows($source->id)->whereNotNull('litter_object_type_id')->exists()) { + throw new RuntimeException('Already-typed source observations require a separately reviewed migration.'); + } + if (XpScore::getObjectXp($source->key) !== XpScore::getObjectXp($destination->key) + && ! $this->option('allow-xp-change')) { + throw new RuntimeException('Per-item object XP differs. Review and use --allow-xp-change if approved.'); + } + + $categories = $this->sourceRows($source->id)->distinct()->pluck('category_id') + ->merge($this->quickTags($source->id)->distinct()->pluck('clo.category_id'))->unique(); + $plan = []; + foreach ($categories as $categoryId) { + $category = Category::findOrFail($categoryId); + $clo = CategoryObject::where('category_id', $categoryId)->where('litter_object_id', $destination->id)->first(); + $createTypes = []; + if ($clo === null) { + if (! $this->option('create-destination')) { + throw new RuntimeException("Missing destination CLO: {$category->key}/{$destination->key}. Use --create-destination only after review."); + } + $config = TagsConfig::get()[$category->key] ?? []; + if (! array_key_exists($destination->key, $config)) { + throw new RuntimeException("Destination {$category->key}/{$destination->key} is not declared in TagsConfig."); + } + foreach ($config[$destination->key]['types'] ?? [] as $key) { + $id = LitterObjectType::where('key', $key)->value('id'); + if ($id === null) { + throw new RuntimeException("Declared type {$key} is missing; no types will be created."); + } + $createTypes[] = (int) $id; + } + } + $allowed = $clo ? $clo->types()->pluck('litter_object_types.id')->all() : $createTypes; + $quickTypes = $this->quickTags($source->id)->where('clo.category_id', $categoryId)->whereNotNull('qt.type_id')->distinct()->pluck('qt.type_id')->all(); + $required = $typeId === null ? $quickTypes : [$typeId]; + if (array_diff($required, $allowed)) { + $ids = $this->quickTags($source->id)->where('clo.category_id', $categoryId) + ->whereNotNull('qt.type_id')->whereNotIn('qt.type_id', $allowed)->pluck('qt.id')->implode(', '); + throw new RuntimeException("Incompatible destination or quick-tag type for {$category->key}/{$destination->key}. Quick tag IDs: {$ids}"); + } + $plan[(int) $categoryId] = ['clo_id' => $clo?->id, 'category' => $category->key, 'create_types' => $createTypes]; + } + return $plan; + } + + private function sourceRows(int $objectId): Builder + { + return DB::table('photo_tags')->where('litter_object_id', $objectId); + } + + private function quickTags(int $objectId): Builder + { + return DB::table('user_quick_tags as qt')->join('category_litter_object as clo', 'clo.id', '=', 'qt.clo_id') + ->where('clo.litter_object_id', $objectId); + } + + private function report(LitterObject $source, LitterObject $destination, ?LitterObjectType $type, array $plan): void + { + $mode = $this->option('apply') ? 'APPLY' : 'DRY RUN'; + $this->info("{$mode}: {$source->key} ({$source->id}) → {$destination->key} ({$destination->id})"); + if ($source->retired_at !== null && $plan !== []) { + $this->line('Resume: matching retirement is recorded; processing remaining rows.'); + } + $this->line('Rows: '.$this->sourceRows($source->id)->count()); + $this->line('Photos: '.$this->sourceRows($source->id)->distinct()->count('photo_id')); + $this->line('Quantity: '.$this->sourceRows($source->id)->sum('quantity')); + $this->line('Example photo IDs: '.$this->sourceRows($source->id)->select('photo_id')->distinct()->orderBy('photo_id')->limit(5)->pluck('photo_id')->implode(', ')); + $this->line('Quick tags to repoint: '.$this->quickTags($source->id)->count()); + foreach ($this->quickTags($source->id)->select('qt.id', 'qt.clo_id', 'qt.type_id', 'clo.category_id')->get() as $quick) { + $destinationClo = $plan[$quick->category_id]['clo_id'] ?? 'new destination CLO'; + $destinationType = $type?->id ?? $quick->type_id ?? 'none'; + $this->line("Quick tag {$quick->id}: CLO {$quick->clo_id} → {$destinationClo}, type {$destinationType}."); + } + $this->line('Per-item object XP delta: '.(XpScore::getObjectXp($destination->key) - XpScore::getObjectXp($source->key))); + $this->line('Whole-photo XP is recalculated using current rules; the per-item delta does not predict that total.'); + $this->line('Replacement type: '.($type?->key ?? 'none')); + foreach ($plan as $categoryId => $entry) { + if ($entry['clo_id'] === null) { + $this->line("Would create category_litter_object: category_id={$categoryId}, litter_object_id={$destination->id} ({$entry['category']}/{$destination->key})."); + foreach ($entry['create_types'] as $id) { + $this->line("Would create category_object_types: new destination CLO, litter_object_type_id={$id}."); + } + } else { + $this->line("Destination: {$entry['category']}/{$destination->key}, CLO {$entry['clo_id']}."); + } + } + $empty = CategoryObject::where('litter_object_id', $source->id)->whereNotIn('category_id', array_keys($plan))->count(); + $this->line("Unused source CLOs: {$empty} (informational; no destination required)."); + } + + private function checkConnections(): void + { + $lock = DB::selectOne('SELECT IS_USED_LOCK(?) AS owner, CONNECTION_ID() AS connection_id', [self::LOCK_NAME]); + if ($lock->owner === null || (int) $lock->owner !== (int) $lock->connection_id) { + throw new RuntimeException('The migration database lock was lost.'); + } + if (Redis::connection()->ping() === false) { + throw new RuntimeException('Redis is unreachable.'); + } + } +} diff --git a/app/Console/Commands/tmp/v5/Migration/Brands/AutoCreateBrandRelationships.php b/app/Console/Commands/tmp/v5/Migration/Brands/AutoCreateBrandRelationships.php index 488e34bc3..e70d4b893 100644 --- a/app/Console/Commands/tmp/v5/Migration/Brands/AutoCreateBrandRelationships.php +++ b/app/Console/Commands/tmp/v5/Migration/Brands/AutoCreateBrandRelationships.php @@ -409,6 +409,10 @@ protected function createRelationship( // Get or create object $object = LitterObject::firstOrCreate(['key' => $normalizedObjectKey]); + if ($object->retired_at !== null) { + $this->warn("Skipping retired object: {$normalizedObjectKey}"); + return; + } // Get or create CategoryObject pivot $categoryObject = CategoryObject::firstOrCreate([ diff --git a/app/Http/Controllers/API/QuickTagsController.php b/app/Http/Controllers/API/QuickTagsController.php index a48d2fb4e..a16bc0676 100644 --- a/app/Http/Controllers/API/QuickTagsController.php +++ b/app/Http/Controllers/API/QuickTagsController.php @@ -58,6 +58,7 @@ public function topTags(Request $request): JsonResponse ->join('categories as c', 'c.id', '=', 'clo.category_id') ->join('litter_objects as lo', 'lo.id', '=', 'clo.litter_object_id') ->leftJoin('litter_object_types as lot', 'lot.id', '=', 'pt.litter_object_type_id') + ->whereNull('lo.retired_at') ->where('p.user_id', $userId) ->whereNotNull('pt.category_litter_object_id') ->select( diff --git a/app/Http/Controllers/API/Tags/GetTagsController.php b/app/Http/Controllers/API/Tags/GetTagsController.php index 016084d29..007a6c86d 100644 --- a/app/Http/Controllers/API/Tags/GetTagsController.php +++ b/app/Http/Controllers/API/Tags/GetTagsController.php @@ -53,7 +53,7 @@ public function getAllTags(): JsonResponse $objectTypesMap = $objectMaps['types']; $objectMaterialsMap = $objectMaps['materials']; - $litterObjects = LitterObject::with(['categories:id,key']) + $litterObjects = LitterObject::with(['categories:id,key'])->whereNull('retired_at') ->whereHas('categories') ->select('id', 'key') ->orderBy('key') @@ -72,9 +72,11 @@ public function getAllTags(): JsonResponse $types = LitterObjectType::select('id', 'key', 'name')->orderBy('key')->get(); - $categoryObjects = CategoryObject::select('id', 'category_id', 'litter_object_id')->get(); + $categoryObjects = CategoryObject::select('id', 'category_id', 'litter_object_id') + ->whereHas('litterObject', fn ($q) => $q->whereNull('retired_at'))->get(); $categoryObjectTypes = DB::table('category_object_types') + ->whereIn('category_litter_object_id', $categoryObjects->pluck('id')) ->select('category_litter_object_id', 'litter_object_type_id') ->get(); @@ -99,7 +101,7 @@ protected function generateQueryFromRequest (Request $request): array $materialsKeys = $request['materials'] ? explode(',', $request['materials']) : null; $searchQuery = $request['search'] ?? null; - $query = CategoryObject::query(); + $query = CategoryObject::whereHas('litterObject', fn ($q) => $q->whereNull('retired_at')); if ($categoryKey) { $query->whereHas('category', function($q) use ($categoryKey) { diff --git a/app/Http/Requests/Api/ReplacePhotoTagsRequest.php b/app/Http/Requests/Api/ReplacePhotoTagsRequest.php index 71dba3843..90600ed0b 100644 --- a/app/Http/Requests/Api/ReplacePhotoTagsRequest.php +++ b/app/Http/Requests/Api/ReplacePhotoTagsRequest.php @@ -36,6 +36,7 @@ public function rules(): array 'tags.*.custom_tags' => 'sometimes|array', // Legacy format fields (backward compat) + 'tags.*.category_id' => 'nullable|integer|exists:categories,id', 'tags.*.category' => 'sometimes', 'tags.*.object' => 'sometimes', 'tags.*.brand_only' => 'sometimes', diff --git a/app/Models/Litter/Tags/LitterObject.php b/app/Models/Litter/Tags/LitterObject.php index df661dd3e..6aa46d1e1 100644 --- a/app/Models/Litter/Tags/LitterObject.php +++ b/app/Models/Litter/Tags/LitterObject.php @@ -16,6 +16,8 @@ class LitterObject extends Model protected $hidden = ['pivot']; + protected $casts = ['retired_at' => 'datetime', 'merged_into_id' => 'integer', 'merged_into_type_id' => 'integer']; + public function getRouteKeyName(): string { return 'key'; diff --git a/app/Services/Metrics/MetricsService.php b/app/Services/Metrics/MetricsService.php index e82ef12bb..c67d09f9e 100644 --- a/app/Services/Metrics/MetricsService.php +++ b/app/Services/Metrics/MetricsService.php @@ -99,9 +99,12 @@ public function deletePhoto(Photo $photo): void 'processed_xp' => null, ]); - // Reverse users.xp — processed_xp includes upload base + tag XP - User::where('id', $photo->user_id) - ->update(['xp' => DB::raw("GREATEST(CAST(xp AS SIGNED) - {$oldXp}, 0)")]); + // Reverse users.xp — processed_xp includes upload base + tag XP. + // An ownerless photo never credited a user, so there is nothing to reverse. + if ($photo->user_id !== null) { + User::where('id', $photo->user_id) + ->update(['xp' => DB::raw("GREATEST(CAST(xp AS SIGNED) - {$oldXp}, 0)")]); + } // FIX #3: Use unified Redis update (pass positive values, collector will negate) $this->updateRedis($photo, [ @@ -182,7 +185,7 @@ private function doCreate(Photo $photo, array $metrics, string $fingerprint): vo // Sync users.xp — doCreate runs when no prior metrics exist // (e.g., school photos deferred until teacher approval, or revoke+reapprove). $xp = (int) $metrics['xp']; - if ($xp > 0) { + if ($xp > 0 && $photo->user_id !== null) { User::where('id', $photo->user_id) ->update(['xp' => DB::raw("xp + {$xp}")]); } @@ -239,7 +242,7 @@ private function doUpdate(Photo $photo, array $newMetrics, string $newFingerprin // Sync users.xp with tag XP delta (upload XP cancels out in delta). // Use GREATEST to prevent unsigned underflow on users.xp column. - if ($xpDelta !== 0) { + if ($xpDelta !== 0 && $photo->user_id !== null) { User::where('id', $photo->user_id) ->update(['xp' => DB::raw("GREATEST(CAST(xp AS SIGNED) + {$xpDelta}, 0)")]); } @@ -451,8 +454,9 @@ private function buildTimeSeriesRows(Photo $photo, array $metrics, int $uploadsD // Aggregate row (user_id=0) $rows[] = $this->buildSingleRow($timescale, $locationType, $locationId, $timestamp, $metrics, $uploadsDelta); - // Per-user row (user_id>0) for leaderboard queries - $rows[] = $this->buildSingleRow($timescale, $locationType, $locationId, $timestamp, $metrics, $uploadsDelta, $photo->user_id); + if ($photo->user_id !== null) { + $rows[] = $this->buildSingleRow($timescale, $locationType, $locationId, $timestamp, $metrics, $uploadsDelta, (int) $photo->user_id); + } } } diff --git a/app/Services/Redis/RedisMetricsCollector.php b/app/Services/Redis/RedisMetricsCollector.php index bebe32b0d..4538e3a25 100644 --- a/app/Services/Redis/RedisMetricsCollector.php +++ b/app/Services/Redis/RedisMetricsCollector.php @@ -20,13 +20,15 @@ final class RedisMetricsCollector { /** - * Process a photo into Redis (called by MetricsService after MySQL update) + * - Update Redis after MetricsService commits MySQL changes. + * - Count ownerless photos in global and location totals. + * - Skip user rankings for those photos; a null user must not become an empty member. */ public static function processPhoto(Photo $photo, array $metrics, string $operation): void { try { $scopes = RedisKeys::getPhotoScopes($photo); - $userId = $photo->user_id; + $userId = $photo->user_id === null ? null : (int) $photo->user_id; Redis::pipeline(function($pipe) use ($scopes, $userId, $metrics, $operation, $photo) { foreach ($scopes as $scope) { @@ -35,16 +37,6 @@ public static function processPhoto(Photo $photo, array $metrics, string $operat $pipe->hIncrBy(RedisKeys::stats($scope), 'photos', 1); $pipe->hIncrBy(RedisKeys::stats($scope), 'litter', $metrics['litter']); $pipe->hIncrBy(RedisKeys::stats($scope), 'xp', $metrics['xp']); - - // Track unique contributors (append-only) - $pipe->pfAdd(RedisKeys::hll($scope), [(string)$userId]); - - // Contributor ranking - $pipe->zIncrBy(RedisKeys::contributorRanking($scope), 1, (string)$userId); - - // XP leaderboard ranking - $pipe->zIncrBy(RedisKeys::xpRanking($scope), $metrics['xp'], (string)$userId); - } elseif ($operation === 'update') { // Apply deltas only if (isset($metrics['litter']) && $metrics['litter'] !== 0) { @@ -52,7 +44,6 @@ public static function processPhoto(Photo $photo, array $metrics, string $operat } if (isset($metrics['xp']) && $metrics['xp'] !== 0) { $pipe->hIncrBy(RedisKeys::stats($scope), 'xp', $metrics['xp']); - $pipe->zIncrBy(RedisKeys::xpRanking($scope), $metrics['xp'], (string)$userId); } } elseif ($operation === 'delete') { @@ -60,16 +51,10 @@ public static function processPhoto(Photo $photo, array $metrics, string $operat $pipe->hIncrBy(RedisKeys::stats($scope), 'photos', -1); $pipe->hIncrBy(RedisKeys::stats($scope), 'litter', -abs($metrics['litter'])); $pipe->hIncrBy(RedisKeys::stats($scope), 'xp', -abs($metrics['xp'])); - - // Decrement contributor ranking (HLL cannot be decremented) - $pipe->zIncrBy(RedisKeys::contributorRanking($scope), -1, (string)$userId); - - // XP leaderboard ranking — decrement then prune members at ≤ 0 - // to keep Redis consistent with MySQL (which filters xp > 0) - $pipe->zIncrBy(RedisKeys::xpRanking($scope), -abs($metrics['xp']), (string)$userId); - $pipe->zRemRangeByScore(RedisKeys::xpRanking($scope), '-inf', '0'); } + self::updateUserRankings($pipe, $scope, $userId, $metrics, $operation); + // Update tag counts and rankings self::updateTags($pipe, $scope, $metrics['tags'] ?? [], $operation); } @@ -87,6 +72,39 @@ public static function processPhoto(Photo $photo, array $metrics, string $operat } } + /** + * - Update contributor counts and rankings only when the photo has an owner. + */ + private static function updateUserRankings($pipe, string $scope, ?int $userId, array $metrics, string $operation): void + { + if ($userId === null) { + return; + } + + $member = (string) $userId; + + if ($operation === 'create') { + // Unique contributors (append-only) + contributor and XP rankings + $pipe->pfAdd(RedisKeys::hll($scope), [$member]); + $pipe->zIncrBy(RedisKeys::contributorRanking($scope), 1, $member); + $pipe->zIncrBy(RedisKeys::xpRanking($scope), $metrics['xp'], $member); + + } elseif ($operation === 'update') { + if (isset($metrics['xp']) && $metrics['xp'] !== 0) { + $pipe->zIncrBy(RedisKeys::xpRanking($scope), $metrics['xp'], $member); + } + + } elseif ($operation === 'delete') { + // Decrement contributor ranking (HLL cannot be decremented) + $pipe->zIncrBy(RedisKeys::contributorRanking($scope), -1, $member); + + // XP leaderboard ranking — decrement then prune members at ≤ 0 + // to keep Redis consistent with MySQL (which filters xp > 0) + $pipe->zIncrBy(RedisKeys::xpRanking($scope), -abs($metrics['xp']), $member); + $pipe->zRemRangeByScore(RedisKeys::xpRanking($scope), '-inf', '0'); + } + } + /** * Update tag counts and rankings */ @@ -121,8 +139,12 @@ private static function updateTags($pipe, string $scope, array $tags, string $op /** * Update user-specific metrics */ - private static function updateUserMetrics($pipe, int $userId, array $metrics, string $operation, Photo $photo): void + private static function updateUserMetrics($pipe, ?int $userId, array $metrics, string $operation, Photo $photo): void { + if ($userId === null) { + return; + } + $userScope = RedisKeys::user($userId); if ($operation === 'create') { diff --git a/app/Services/Tags/ResolveLitterObject.php b/app/Services/Tags/ResolveLitterObject.php new file mode 100644 index 000000000..35d6aa1cc --- /dev/null +++ b/app/Services/Tags/ResolveLitterObject.php @@ -0,0 +1,63 @@ +retired_at !== null) { + if (isset($seen[$object->id])) { + throw ValidationException::withMessages(['tags' => 'Invalid object replacement chain.']); + } + $seen[$object->id] = true; + $typeId = $object->merged_into_type_id === null ? $typeId : (int) $object->merged_into_type_id; + $object = LitterObject::find($object->merged_into_id); + if ($object === null) { + throw ValidationException::withMessages(['tags' => 'The replacement object is missing.']); + } + } + + $choices = CategoryObject::where('litter_object_id', $object->id); + if ($categoryId !== null) { + $choices->where('category_id', $categoryId); + } elseif ($typeId !== null) { + $choices->whereHas('types', fn ($query) => $query->where('litter_object_types.id', $typeId)); + } + $clos = $choices->limit(2)->get(); + if ($clos->count() !== 1) { + throw ValidationException::withMessages(['tags' => $clos->isEmpty() + ? 'Category does not contain the replacement object.' : 'Choose a category for this object.']); + } + + $clo = $clos->first(); + if ($typeId !== null && ! $clo->types()->where('litter_object_types.id', $typeId)->exists()) { + throw ValidationException::withMessages(['tags' => 'Type is not allowed for this category and object.']); + } + + return [$clo, $typeId]; + } + + /** - Saved quick tags and mobile requests identify the category and object by CLO ID. */ + public function fromClo(int $cloId, ?int $typeId = null): array + { + $clo = CategoryObject::with('litterObject')->find($cloId); + if ($clo === null || $clo->litterObject === null) { + throw ValidationException::withMessages(['tags' => 'Invalid category_litter_object_id.']); + } + + return $this->resolve($clo->litterObject, (int) $clo->category_id, $typeId); + } +} diff --git a/database/migrations/2026_08_08_182838_add_retired_at_and_merged_into_id_to_litter_objects_table.php b/database/migrations/2026_08_08_182838_add_retired_at_and_merged_into_id_to_litter_objects_table.php new file mode 100644 index 000000000..14f9784d9 --- /dev/null +++ b/database/migrations/2026_08_08_182838_add_retired_at_and_merged_into_id_to_litter_objects_table.php @@ -0,0 +1,34 @@ +timestamp('retired_at')->nullable()->after('crowdsourced'); + $table->unsignedBigInteger('merged_into_id')->nullable()->after('retired_at'); + + $table->foreign('merged_into_id')->references('id')->on('litter_objects')->nullOnDelete(); + }); + } + + public function down(): void + { + Schema::table('litter_objects', function (Blueprint $table) { + $table->dropForeign(['merged_into_id']); + $table->dropColumn(['retired_at', 'merged_into_id']); + }); + } +}; diff --git a/database/migrations/2026_09_14_000001_add_merged_into_type_id_to_litter_objects.php b/database/migrations/2026_09_14_000001_add_merged_into_type_id_to_litter_objects.php new file mode 100644 index 000000000..78f67eea0 --- /dev/null +++ b/database/migrations/2026_09_14_000001_add_merged_into_type_id_to_litter_objects.php @@ -0,0 +1,23 @@ +foreignId('merged_into_type_id')->nullable() + ->constrained('litter_object_types')->restrictOnDelete(); + }); + } + + public function down(): void + { + Schema::table('litter_objects', function (Blueprint $table) { + $table->dropConstrainedForeignId('merged_into_type_id'); + }); + } +}; diff --git a/package-lock.json b/package-lock.json index ee18bcd52..3ea291d19 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "openlittermap-web", - "version": "5.12.6", + "version": "5.13.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "openlittermap-web", - "version": "5.12.6", + "version": "5.13.1", "license": "GPL v3", "dependencies": { "@headlessui/tailwindcss": "^0.2.1", diff --git a/package.json b/package.json index a29647115..d79361cd6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "openlittermap-web", - "version": "5.13.0", + "version": "5.13.1", "type": "module", "author": "Seán Lynch", "license": "GPL v3", diff --git a/phpunit.xml b/phpunit.xml index 289de2ead..cf3ad3436 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -22,7 +22,8 @@ - + + diff --git a/readme/API.md b/readme/API.md index ed06f3ec9..2ac5e582b 100644 --- a/readme/API.md +++ b/readme/API.md @@ -507,6 +507,17 @@ Category is auto-resolved from `category_litter_object_id`. Generates summary, c --- +### Retired object submissions + +- Old object IDs/keys and old `category_litter_object_id` values resolve to the current replacement before saving. A retired object need not have a CLO when submitted in object format. +- The object's recorded replacement type overrides a stale submitted type. Across a chain, the latest non-null recorded type wins. Without a recorded type, the supplied type must be valid at the final destination. +- An explicit object-format `category_id` or `category` is preserved or rejected; the API no longer silently substitutes another category. PUT now validates and retains `category_id`, as POST already does. +- For retired objects without a category, the destination plus effective type must identify a single category. Example: `energy_can → can --type=energy` can identify `softdrinks/can` when only that category permits energy. Ambiguous choices return 422. +- Unknown objects/categories, unresolved combinations, broken replacement chains and incompatible destination types return 422. Rejected PUTs leave the existing photo, observations and extras intact. The usual response is `{ "message": "...", "errors": { "tags": ["..."] } }`; older unresolved-combination errors may contain structured details in `errors.tags`. +- Quick-tag sync resolves old CLOs and validates the final type before replacing presets. Invalid syncs preserve the old presets. GET catalogue/search/top-tags excludes retired objects. + +This release does not change the existing first-category inference for **active** object-only submissions. Send a category or CLO to avoid that ambiguity. It also does not make unrelated unresolved observations editable: a full-photo replacement containing one still fails atomically. + ### PUT /api/v3/tags — Replace All Tags on Photo **Auth:** Required (Sanctum) diff --git a/readme/ArtisanCommands.md b/readme/ArtisanCommands.md index 5bd33bb08..f158555ad 100644 --- a/readme/ArtisanCommands.md +++ b/readme/ArtisanCommands.md @@ -63,7 +63,8 @@ These live in `tmp/` and are intended for the v5 migration period only. | Command | Purpose | |---------|---------| -| `seed:tags` | Run GenerateTagsSeeder (required for test DB setup) | +| `seed:tags` | Catalogue setup for a new local/test database; not a production migration step | +| `olm:migrate-tag OLD NEW` | Preview one object migration; `--apply` updates data. Optional `--type`, `--create-destination`, `--allow-xp-change`. See [deployment guide](PostTagMigrationClean.md). | | `tags:verify-for-user-id {user_id}` | Verify remaining tags for a user | --- diff --git a/readme/PostTagMigrationClean.md b/readme/PostTagMigrationClean.md new file mode 100644 index 000000000..bb7aff77f --- /dev/null +++ b/readme/PostTagMigrationClean.md @@ -0,0 +1,141 @@ +# Migrate one object tag + +`olm:migrate-tag OLD NEW` updates one object at a time. This release keeps each observation's category. It does not require a source CLO or run `GenerateTagsSeeder` in production. + +## Plastic bags first + +```bash +# Preview only — no changes +php artisan olm:migrate-tag plasticBags plastic_bag + +# Apply after the checks below, with writes paused +php artisan olm:migrate-tag plasticBags plastic_bag --apply + +# Same preview after completion reports Already applied +php artisan olm:migrate-tag plasticBags plastic_bag +``` + +The old `plasticBags` object records `plastic_bag` as its replacement. Existing `photo_tags` rows get the new object ID and the destination category's CLO ID. Row IDs, categories, quantities, collection status, extras and both observation timestamps stay unchanged. Existing `plastic_bag` observations stay unchanged too. + +Every affected photo gets a regenerated summary using the existing XP rules; `photos.updated_at` changes. Previously processed, non-deleted photos get metrics deltas, regardless of verification level. Unprocessed school photos wait for approval. Soft-deleted photos get corrected rows and summaries without metrics updates. The command does not approve or publish photos. + +Old object submissions and old CLO submissions resolve through the recorded replacement before saving. Retired objects disappear from the catalogue, search and generated top tags. Saved quick tags move to the destination CLO; their other fields stay intact, except an explicitly recorded replacement type overrides their type. + +## What else changes when a key is renamed? + +The command updates stored data. It does **not** rename references in PHP, JavaScript, translations or the mobile app. Review these references for every approved mapping. + +For `plasticBags → plastic_bag`: + +- **Catalogue:** `TagsConfig` already declares `other/plastic_bag`. The replacement object and its category/object entry (CLO) already exist in the fresh local database. Confirm they exist in the deployment database too; no production seeder is needed for this mapping. +- **Brand configuration — still to update:** replace the 12 `plasticBags` references in `app/Tags/BrandsConfig.php` with `plastic_bag`. For example, Aldi's `other` objects should contain `plastic_bag`. No current runtime reader of this configuration was found; updating the file keeps its associations consistent. It does not update stored brand relationships or require running the old brand tooling. +- **Translations — still to update:** add `"plastic_bag": "Plastic Bags"` under `litter.other` in `resources/js/langs/en.json`, which the current web app loads. Add the same entry under `other` in `resources/js/langs/en/litter.json`. Keep the old entries so historical data can still display a label. Without the new entry, tagging search falls back to “Plastic Bag” from the key. +- **Photo summaries:** the command regenerates affected summaries with the new object ID and key. For example, five `plasticBags` become five `plastic_bag`; the quantity stays five. +- **Downloads:** newly generated CSV exports read the updated catalogue and summaries, so they use `plastic_bag`. The command does not rewrite previously generated or downloaded files; generate a fresh export to see the change. +- **Suggestions and saved choices:** the server hides the retired object from new suggestions and updates saved quick tags that reference its CLO. An already-open app can still hold an old catalogue; supported stale submissions use the recorded replacement when saved. Refresh the catalogue during smoke checks. +- **Mobile app — still to check:** inspect its own key references and translations, then check a supported build with a refreshed catalogue and a stale submission. This web-repository review does not verify the mobile repository. + +Keep `plasticBags` where it describes the old data: + +- Historical conversion definitions in `OldTagsConfig` and `ClassifyTagsService`, and old repair scripts. +- Migration tests, command examples and audit records showing the old-to-new mapping. +- Legacy translations needed to display old data. The older `plastic_bags` database columns are separate historical fields; do not rename them with this object migration. + +Before deployment, finish the pending configuration and translation edits above, build the frontend, and check the displayed label and a fresh export. These edits are separate from `--apply`; running the command alone does not complete them. + +## Supported mappings and limits + +Both objects must already exist. The destination must be active and have a CLO in each source category with observations or saved quick tags. Empty source CLOs do not require destinations. No source CLO is created. + +```bash +# Separate, later mapping — review and rehearse independently +php artisan olm:migrate-tag energy_can can --type=energy +php artisan olm:migrate-tag energy_can can --type=energy --apply +``` + +Optional `--create-destination` creates only missing destination CLOs declared in `TagsConfig`, plus their declared type associations. All those type records must already exist. The preview prints the exact proposed rows. It does not create objects, categories, materials or types, or run seeders. Neither of the first two mappings should need this flag if their approved destination CLOs already exist. + +Source observations with a null category or an existing type are refused. Category moves and migrations of already-typed observations are follow-up work. Incompatible destination types or saved quick-tag types stop the run before retirement. The same checks run in preview and apply. + +`--allow-xp-change` is required when the old and new object keys have different per-item XP under the existing summary calculator. A zero per-item delta is **not** a promise of zero whole-photo XP drift: every affected photo is rescored using current rules, including its other observations. Compare XP during rehearsal before approving a mapping. + +The latest non-null type recorded in an object replacement chain wins. Without a recorded type, a supplied type must be allowed at the final destination or the API returns 422 atomically. For a retired object submitted without a category, the destination and effective type must identify one category; otherwise the API returns 422. Explicit object-format categories are preserved or rejected, never silently substituted. See [API contract](API.md). + +Matching retries resume the remaining rows; conflicting object/type arguments fail. A completed A → B can be replayed after B → C. B → C is refused while an earlier migration into B still has source rows or quick tags. The command uses one MySQL advisory lock for the entire apply, with source and destination row locks in ID order during retirement. It checks lock ownership and Redis connectivity before each batch of 200 photos. This does not replace the write freeze. + +## Rehearsal and production window + +1. Record the candidate commit, MySQL/Redis identities and versions, and reviewed mapping arguments. Rehearse on a **fresh disposable production copy** with production-compatible runtimes. The current local baseline is `olm_postmig_7`; keep it read-only and rehearse on a separate copy. It has 10,051 `plasticBags` observations and no retirement columns as checked on 2026-09-14. Install the retirement schema on the rehearsal copy before previewing the command. Earlier `_6` rehearsal figures are historical evidence, not this baseline. +2. Take verified database backups. Record source rows/quantities by category, destination totals, extras, saved quick tags, per-photo/user XP, MySQL metric values, and Redis object counts in every affected global/location/user scope. Record unresolved combinations separately; this command does not fix unrelated tags. +3. Measure the actual mapping, including batch time, summary regeneration and verification. Earlier estimates were about 51 photo batches for plastic bags and 103 for energy cans; size the window from the fresh rehearsal, not those estimates. +4. Freeze uploads, tag edits, approvals and other writes on **every node**. Drain in-flight requests, pause schedules and workers, and verify they are stopped. `horizon:terminate` alone allows a supervisor to restart workers. +5. Deploy the candidate code and apply its two schema migrations with `php artisan migrate --force`. Confirm no unrelated pending migrations will run. Schema and replacement-aware application code must both be live before writes reopen. Clear/restart long-lived application processes as appropriate for the deployed runtime. +6. Run the dry run and review its used categories, destinations, quantities, quick tags, type and XP warning. Apply **only** `plasticBags → plastic_bag`. Do not run `GenerateTagsSeeder`, `composer seed:tags`, `olm:v5` or the temporary brand relationship tooling in this window. +7. Confirm no source observations or source quick tags remain. Compare observation IDs and fields, extras, summary keys and totals, photo/user XP and MySQL metric values against the baseline. Verify the existing new-tag ×5 observation is unchanged. Execute wide and long CSV exports and check the new keys and quantities. Replay must be a no-op. +8. Verify Redis old-object counts decreased by the moved processed quantities and new-object counts increased by the same amount **in each affected scope**, including user scopes. Exclude unprocessed and deleted photos. Global/location object counts use `RedisKeys::objects($scope)`; user object counts use `{u:ID}:tags` fields `obj:OBJECT_ID`. Other tags, aggregate litter, XP and rankings must reconcile with the rehearsal. Do not rely on the command's success message alone. +9. Smoke-test stale object and CLO submissions, ordinary editing and school approval. A mixed photo with another unresolved category/object combination can still reject an edit; acknowledge this limitation rather than claiming it is fixed. Resume traffic only after checks pass. + +An interrupted run keeps writes paused. Each completed photo batch is committed; an incomplete batch rolls back. Inspect the error and rerun the exact approved arguments. Do not roll back schema/application code after retirement: stale requests still need the resolver. Restoring pre-migration data requires the backup and a coordinated outage. + +## Redis recovery + +MySQL and Redis do not share a transaction. Redis failures can leave SQL committed; the collector logs failures, so even a successful command needs independent delta verification. Re-running `processPhoto` cannot repair counters when the stored fingerprint already matches. + +The existing recovery command is: + +```bash +php artisan olm:redis:rebuild +``` + +It confirms before flushing the configured Redis database, then replays **all processed non-deleted photos**, using `processed_tags` and `processed_xp`. SQL metrics totals alone cannot reconstruct per-object hashes. Check the target Redis database and all of its contents first, and measure this full rebuild on the rehearsal copy. Keep writes paused through recovery and verification. Do not use `--no-flush` against populated counters: replay adds counts again. A shared Redis database needs a separate reviewed recovery procedure. + +## Follow-up PRs + +Use `bugs/2026/08/plastic-bag-count` as a source of reviewed implementations and regression tests. The comparison below was made against its tip `8f4e4087`. Port each useful change onto the merged command branch and test it there; the old branch's CLO redirects and historical catalogue are different from this release's object replacements. + +The current PR already carries the single-object command, replacement-aware submissions, saved quick-tag migration, retired-object filtering, ownerless metrics fixes and test-database protections. Keep those implementations when bringing over follow-up work. + +### Recommended order + +1. **Preserve tags through every editor.** Reuse the original branch's pure `useTagEditorState.js` adapter and endpoint-backed editor tests, adapted to the current API. Repair the web editor, admin queue, facilitator queue and team photo modal together with their server contracts. Preserve explicit categories, types, independent brand quantities, all extras and `true`/`false`/`null` collection status. Take the photo-row lock before replacing tags; failed edits must restore both photo fields and observations. Keep school privacy and approval, including editing already-approved photos and updating their metrics. Retain the agreed custom-tag behaviour: separate new cards remain separate observations, while an existing combined observation stays combined. Test quantities 2 and 5 producing 7 custom-tag XP in both frontend and backend. Bring the Node test bridge, Node 22 setup, dependency installation and frontend build into CI with these tests. These are endpoint-backed tests, not browser tests; perform focused editor smoke checks separately. + +2. **Allow safe edits while other tags await migration.** This is the main gap the original branch solved through global historical-CLO preparation. Do not copy that requirement into this rollout. Proposed behaviour: when a photo contains `plastic_bag ×2` and an unresolved `randomLitter` observation, changing bags to 3 should preserve the existing `randomLitter` observation unchanged. Design an explicit way to identify and preserve an existing observation, scoped to the authorised photo; do not accept arbitrary new category/object combinations. Changes to the unresolved observation itself still need a defined validation rule. Prove this with a mixed-photo round trip on an unprepared catalogue. Earlier evidence found 3,156 mixed plastic-bag photos and 179,467 other unresolved observations after a plastic-bag run; recount on the fresh rehearsal copy. PR 1 alone does not close this gap. + +3. **Make integrity checks useful for staged migrations.** Adapt the original verifier's missing-combination checks, scoped-repair safeguards and recount-after-repair tests. Report remaining observations and quick tags on retired objects, and broken object replacement chains, using this branch's schema. Distinguish missing catalogue entries from stale stored CLO IDs and legitimate extra-only observations. Explain exactly what `--fix` can repair; it must not invent taxonomy. Test that a photo-scoped repair cannot modify another photo. Do not copy checks for historical or redirected CLO columns that this branch does not have. + +4. **Make typed-object XP consistent.** Port the original branch's per-observation type-aware scoring with matching frontend calculations and regression tests. Example: `dumping` with type `small` should use the approved small-dumping score. Update the migration's XP warning in the same PR so it agrees with the actual summary calculator. Measure effects on existing photos and decide any rescore separately; do not silently recalculate all users' XP as part of deployment. + +5. **Make achievement seeding repeatable.** Port the insert-only timestamp behaviour and repeat-execution tests, including definitions with null tag IDs. Preserve existing achievement definitions and `user_achievements`. Decide separately how achievements attached to retired objects should be treated; the timestamp fix does not transfer them. Run any approved seeding outside the tag-migration window, off peak. + +6. **Support category moves or already-typed observations when needed.** Use the old branch's category/type and retry tests as examples, then design the smallest extension compatible with object replacements. Example: moving only `sanitary/gloves` to `medical/gloves` must not retire the shared `gloves` object globally. This needs a category-specific mapping and a rule for stale submissions. Add it when an approved mapping requires it, with interruption and replay coverage. + +### Later migrations do not each need a new PR + +- After plastic bags passes verification, review and rehearse energy cans as a separate command run. Other supported mappings can follow independently. +- A mapping needs code changes only when its destination declarations, translations, brand configuration or required command behaviour are missing. Review those references as described above. +- Confirm meaning before execution: for example, whether a legacy size distinction should be retained. Do not turn an unreviewed inventory into an approved execution list. + +### Work from the original branch that is not required for parity + +- Global historical-CLO preparation, `is_selectable` and production `GenerateTagsSeeder` runs belonged to the earlier rollout plan. They are not prerequisites for individual migrations. +- Do not add the old CLO retirement schema or duplicate command alongside this command. Category-specific redirects require their own later design. +- The approved-plan runner and bulk rehearsal scripts are optional tooling if the number of approved runs warrants them. Keep the first run manual and explicit. +- Deprecated-column removal and broad resolver refactors remain separate architectural work. Neither is needed to rename plastic bags. +- A mounted-browser automation harness was not delivered by the original branch. It remains separate new infrastructure, not missing coverage that can simply be copied. + +Brand configuration and translation updates for plastic bags belong in the current release, as listed above; they are not deferred by this follow-up sequence. + +## Release evidence + +Local test results below do not substitute for the fresh-copy rehearsal or CI on the final commit. CI currently runs PHP 8.3/MySQL 8.0/Redis 7, without Node, for pushes and PRs targeting `master` or `upgrade/tagging-2025`. Open a PR to a configured target so the exact release candidate receives CI. + +Before production, attach the tested commit, approved arguments, fresh-copy timings and reconciliation results, remaining unresolved counts, and manual smoke-check results. Production execution remains a separate action. No browser or production rehearsal is implied by endpoint-backed tests. + +### Local verification — 2026-09-14 + +- Tested the uncommitted working tree on `fix/tags/migrate-tag`, based on `ac688aa6`, using PHP 8.3.16, local MySQL `olm_test` and Redis database 2. +- Full PHPUnit suite: **1,388 tests, 6,997 assertions, zero failures, one skipped**. The existing `TypesCheckerTest::test_it_unlocks_per_type_achievement` skips because its fixture has no types; achievement changes remain follow-up work. +- `npm run build` passes, with the existing large-chunk warning. PHP syntax and `git diff --check` pass. +- Covered missing source CLOs, explicit destination creation, dry runs, stale object/CLO submissions, type rules, quick tags, observation/extras/timestamp preservation, school approval, ownerless/deleted photos, retry agreement, chains and actual wide/long CSV output. +- The interruption test commits 200 photos, fails the next batch, resumes the remaining photo and verifies replay and Redis rebuild without double-counting. A second MySQL connection verifies advisory-lock contention. This is exception-based interruption coverage, not a process-kill rehearsal. +- No mounted browser session, concurrent editor execution, final-commit CI or fresh production-copy rehearsal was run. Production and `olm_postmig_6` were not modified. Those deployment checks remain pending. diff --git a/readme/audit/LitterObjectInventory-2026-08.csv b/readme/audit/LitterObjectInventory-2026-08.csv new file mode 100644 index 000000000..c99a2afe7 --- /dev/null +++ b/readme/audit/LitterObjectInventory-2026-08.csv @@ -0,0 +1,199 @@ +object_id,object_key,crowdsourced,in_tagsconfig,tagsconfig_categories,pivot_categories,tagged_categories,total_tag_rows,total_items,total_photos,pairs_total,pairs_pivotless,pivotless_items,cfg_materials,cfg_types,rows_with_type,ref_tagsconfig,ref_brandsconfig,ref_classifyservice,ref_fixorphaned,ref_translations,ref_other_app_code,duplicate_candidates,duplicate_reason +99,appliance,0,yes,other,other,,0,0,0,0,0,0,,,0,1,0,0,0,1,0,, +117,ashtray,0,yes,smoking,smoking,,0,0,0,0,0,0,glass|ceramic|metal,,0,1,0,0,0,1,0,, +180,automobile,1,no,,,other,794,987,794,1,1,987,,,0,0,0,1,1,3,2,, +36,bag,0,yes,food,food,food|marine,1216,1718,1216,2,1,1662,plastic|paper|cloth|bioplastic,,0,1,0,1,0,1,0,, +15,bags_litter,0,yes,civic|other,civic|other,civic|other,49,161,49,2,0,0,,,0,2,0,1,0,1,3,bagsLitter#176,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +176,bagsLitter,1,no,,,other,1041,2239,1041,1,1,2239,,,0,0,0,1,1,1,0,bags_litter#15,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +96,balloon,0,yes,other,other,other,24,28,24,1,0,0,plastic|latex,,0,1,0,0,0,0,0,balloons#178,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +178,balloons,1,no,,,marine|other,2180,2491,2180,2,2,2491,,,0,0,0,0,2,2,2,balloon#96,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +78,bandage,0,yes,medical,medical,medical,2,2,2,1,0,0,cotton|elastic,,0,1,0,0,0,1,0,, +181,batteries,1,no,,,other,291,363,291,1,1,363,,,0,0,1,0,1,1,1,, +29,battery,0,yes,electronics|vehicles,electronics|vehicles,electronics,3,3,3,1,0,0,metal|plastic,alkaline|lithium|rechargeable,1,2,0,0,0,2,0,, +138,beer_bottle,1,no,,,alcohol,7337,18074,7337,1,1,18074,,,0,0,14,1,1,1,0,, +137,beer_can,1,no,,,alcohol,19275,26488,19275,1,1,26488,,,0,0,13,1,2,1,0,, +127,bicycle,0,yes,vehicles,vehicles,vehicles,1,1,1,1,0,0,,,0,1,0,0,0,0,0,, +16,blocked_drain,0,yes,civic,civic,civic,5,5,5,1,0,0,,,0,1,0,0,0,0,0,, +192,books,1,no,,,other,38,71,38,1,1,71,,,0,0,0,0,1,1,1,, +2,bottle,0,yes,alcohol|softdrinks,alcohol|softdrinks,alcohol|marine|softdrinks,1793,2233,1783,3,1,1070,glass|plastic,beer|wine|spirits|cider|premixed|unknown|water|soda|juice|energy|sports|tea|milk|smoothie,941,2,1,1,0,0,1,, +4,bottle_cap,0,yes,alcohol|softdrinks,alcohol|softdrinks,alcohol|softdrinks,217,225,217,2,0,0,metal|plastic|cork,,0,2,0,0,0,0,0,, +146,bottletops,1,no,,,alcohol,4902,13018,4902,1,1,13018,,,0,0,8,0,1,0,0,, +37,box,0,yes,food|smoking,food|smoking,food|smoking,168,176,168,2,0,0,cardboard|plastic|wood|metal|foil,cigarette|match|unknown,139,2,0,0,0,1,0,, +58,bricks,0,yes,industrial,industrial,industrial,83,242,83,1,0,0,clay|concrete|stone,,0,1,0,0,0,1,1,, +3,broken_glass,0,yes,alcohol|softdrinks,alcohol|softdrinks,alcohol|softdrinks,79,107,79,2,0,0,glass,,0,2,0,1,0,1,1,brokenglass#164,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +164,brokenglass,1,no,,,alcohol|softdrinks,3412,8160,3412,2,2,8160,,,0,0,0,0,2,0,0,broken_glass#3,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +128,bumper,0,yes,vehicles,vehicles,,0,0,0,0,0,0,plastic|metal,,0,1,0,0,0,1,0,, +63,buoy,0,yes,marine,marine,,0,0,0,0,0,0,plastic|foam|metal,,0,1,0,0,0,0,0,buoys#196,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +196,buoys,1,no,,,marine,145,189,145,1,1,189,,,0,0,0,0,1,1,1,buoy#63,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +118,butts,0,yes,smoking,smoking,smoking,82848,145241,82848,1,0,0,plastic|paper,,0,1,3,2,0,1,1,, +30,cable,0,yes,electronics,electronics,electronics,1,1,1,1,0,0,plastic|copper,,0,1,0,0,0,1,0,, +95,cable_tie,0,yes,other,other,other,14,17,14,1,0,0,,,0,1,0,1,0,1,1,cableTie#187,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +187,cableTie,1,no,,,other,1021,1203,1021,1,1,1203,,,0,0,0,1,1,1,0,cable_tie#95,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +5,can,0,yes,alcohol|food|softdrinks,alcohol|food|softdrinks,alcohol|food|softdrinks,511,538,502,3,0,0,aluminium|steel,beer|cider|spirits|premixed|unknown|soda|energy|juice|iced_tea|sparkling_water,334,3,0,0,0,1,1,, +129,car_part,0,yes,vehicles,vehicles,vehicles,11,15,11,1,0,0,metal|plastic|rubber|glass,,0,1,0,0,0,1,0,, +124,carton,0,yes,softdrinks,softdrinks,softdrinks,108,117,108,1,0,0,cardboard|foil|plastic,juice|milk|iced_tea|plant_milk|unknown,66,1,0,0,0,0,0,, +31,charger,0,yes,electronics,electronics,,0,0,0,0,0,0,plastic|metal,,0,1,0,0,0,1,0,, +195,chemical,1,no,,,industrial,27,36,27,1,1,36,,,0,0,0,0,1,1,1,, +56,chemical_container,0,yes,industrial,industrial,industrial,1,1,1,1,0,0,plastic|metal|glass,,0,1,0,0,0,0,0,, +38,chopsticks,0,yes,food,food,,0,0,0,0,0,0,wood|bamboo|plastic,,0,1,0,0,0,0,0,, +141,cigarette_box,1,no,,,smoking,9803,10245,9803,1,1,10245,,,0,0,3,1,1,1,0,, +88,clothing,0,yes,other,other,other,2524,2899,2524,1,0,0,,,0,1,2,0,0,1,1,, +115,condom,0,yes,sanitary,sanitary,sanitary,1,1,1,1,0,0,latex,,0,1,1,0,0,0,0,condoms#158,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +116,condom_wrapper,0,yes,sanitary,sanitary,sanitary,2,2,2,1,0,0,plastic|foil,,0,1,0,0,0,1,0,, +158,condoms,1,no,,,sanitary,190,366,190,1,1,366,,,0,0,0,0,1,1,1,condom#115,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +57,construction,0,yes,industrial,industrial,industrial,15,27,15,1,0,0,clay|concrete|plastic|metal|fiberglass|foam|asphalt|ceramic|stone,,0,1,0,0,0,1,0,, +39,container,0,yes,food|industrial,food|industrial,food,36,38,36,1,0,0,plastic|metal,,0,2,0,0,0,1,0,, +64,cotton_bud_stick,0,yes,marine,marine,marine,1,1,1,1,0,0,plastic,,0,1,0,0,0,0,0,, +65,crate,0,yes,marine,marine,,0,0,0,0,0,0,plastic,,0,1,0,0,0,0,0,, +163,crisp_large,1,no,,,food,431,469,431,1,1,469,,,0,0,3,0,1,1,1,, +40,crisp_packet,0,yes,food,food,food,58,58,58,1,0,0,foil|plastic,,0,1,0,0,0,0,0,, +157,crisp_small,1,no,,,food,4654,5037,4654,1,1,5037,,,0,0,4,0,1,1,1,, +6,cup,0,yes,alcohol|coffee|softdrinks,alcohol|coffee|softdrinks,alcohol|coffee|softdrinks,24613,27900,24519,3,0,0,plastic|paper|foam|ceramic|metal|bioplastic,tea|soda|smoothie|unknown,111,3,33,5,0,3,0,, +22,cup_carrier,0,yes,coffee|softdrinks,coffee|softdrinks,coffee,1,1,1,1,0,0,cardboard|plastic,,0,2,0,0,0,0,0,, +41,cutlery,0,yes,food,food,food,2730,3249,2730,1,0,0,plastic|wood|bioplastic|bamboo|metal,,0,1,1,1,0,1,0,, +110,dental_floss,0,yes,sanitary,sanitary,sanitary,9,10,9,1,0,0,nylon|plastic,,0,1,0,0,0,0,0,, +111,deodorant_can,0,yes,sanitary,sanitary,sanitary,161,172,161,1,0,0,aluminium,,0,1,0,1,0,1,0,, +102,dogshit,0,yes,pets,pets,other|pets,1657,2187,1657,2,1,1383,,,0,1,0,2,1,4,4,, +103,dogshit_in_bag,0,yes,pets,pets,other|pets,1560,2113,1560,2,1,649,plastic,,0,1,0,2,1,1,0,, +165,dump,1,no,,,other,1875,9733,1875,1,1,9733,,,0,0,0,0,1,1,1,, +28,dumping,0,yes,dumping,dumping,dumping,1906,20649,1890,1,0,0,,small|medium|large,35,1,0,3,0,2,6,, +161,ear_plugs,1,no,,,other,26,46,26,1,1,46,,,0,0,0,0,1,1,1,, +106,ear_swabs,0,yes,sanitary,sanitary,sanitary,10,19,10,1,0,0,plastic|cotton,,0,1,0,1,0,1,1,earSwabs#166,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +166,earSwabs,1,no,,,sanitary,172,294,172,1,1,294,,,0,0,0,1,1,1,0,ear_swabs#106,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +169,elec_large,1,no,,,other,60,100,60,1,1,100,,,0,0,0,0,1,1,1,, +174,elec_small,1,no,,,other,291,385,291,1,1,385,,,0,0,0,0,1,1,1,, +156,energy_can,1,no,,,softdrinks,20498,20931,20498,1,1,20931,,,0,0,6,2,2,1,1,, +79,face_mask,0,yes,medical,medical,medical,13,17,13,1,0,0,cotton|polyester|paper,,0,1,0,0,0,0,0,facemask#183,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +183,facemask,1,no,,,sanitary,7742,7910,7742,1,1,7910,,,0,0,0,0,1,1,1,face_mask#79,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +17,fallen_tree,0,yes,civic,civic,,0,0,0,0,0,0,,,0,1,0,0,0,0,0,, +167,filters,1,no,,,smoking,603,852,602,1,1,852,,,0,0,0,1,1,1,1,, +66,fishing_hook,0,yes,marine,marine,,0,0,0,0,0,0,metal,,0,1,0,0,0,0,0,, +67,fishing_line,0,yes,marine,marine,marine,5,5,5,1,0,0,nylon,,0,1,0,0,0,0,0,, +68,fishing_lure,0,yes,marine,marine,,0,0,0,0,0,0,plastic|metal,,0,1,0,0,0,0,0,, +69,fishing_net,0,yes,marine,marine,marine,4,4,4,1,0,0,nylon|plastic,,0,1,0,0,0,0,0,fishing_nets#173,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +173,fishing_nets,1,no,,,marine,894,1154,893,1,1,1154,,,0,0,0,2,1,0,0,fishing_net#69,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +145,fizzy_bottle,1,no,,,softdrinks,6448,7831,6448,1,1,7831,,,0,0,8,1,1,1,0,, +97,furniture,0,yes,other,other,,0,0,0,0,0,0,,,0,1,0,0,0,1,0,, +7,glass,0,yes,alcohol,alcohol,alcohol,106,123,106,1,0,0,glass,beer|wine|spirits|cider|unknown,15,18,0,9,0,2,2,, +159,glass_jar,1,no,,,food,131,188,131,1,1,188,,,0,0,0,0,1,1,1,, +80,gloves,0,yes,medical,medical,medical|sanitary,2444,2586,2444,2,1,2562,latex|rubber|plastic,,0,1,0,0,1,1,1,, +13,graffiti,0,yes,art,art,art,9,11,9,1,0,0,,,0,1,0,0,0,1,0,, +42,gum,0,yes,food,food,food,432,490,432,1,0,0,rubber,,0,1,0,1,0,1,0,, +160,hair_tie,1,no,,,other,515,546,515,1,1,546,,,0,0,0,0,1,1,1,, +32,headphones,0,yes,electronics,electronics,electronics,1,1,1,1,0,0,plastic|metal,,0,1,0,0,0,1,0,, +130,hubcap,0,yes,vehicles,vehicles,,0,0,0,0,0,0,plastic,,0,1,0,0,0,0,0,, +194,icedTea_can,1,no,,,softdrinks,588,607,588,1,1,607,,,0,0,0,1,1,1,0,, +186,iceTea_bottle,1,no,,,softdrinks,829,851,829,1,1,851,,,0,0,0,1,1,1,0,, +81,inhaler,0,yes,medical,medical,,0,0,0,0,0,0,plastic|metal,,0,1,0,0,0,0,0,, +185,item,1,no,,,art,42,43,42,1,1,43,,,0,0,0,0,0,1,1,, +43,jar,0,yes,food,food,food,1,1,1,1,0,0,glass|plastic|metal,,0,1,0,0,0,1,0,, +155,juice_bottle,1,no,,,softdrinks,1862,1940,1862,1,1,1940,,,0,0,1,1,1,1,0,, +154,juice_carton,1,no,,,softdrinks,2243,2327,2243,1,1,2327,,,0,0,2,1,1,1,0,, +125,juice_pouch,0,yes,softdrinks,softdrinks,softdrinks,2172,2263,2172,1,0,0,plastic|foil,,0,1,0,1,0,1,0,, +8,label,0,yes,alcohol|softdrinks,alcohol|softdrinks,softdrinks,2667,3154,2667,1,0,0,paper|plastic,,0,2,13,1,75,1,0,, +197,lego,1,no,,,marine,3,3,3,1,1,3,,,0,0,1,0,1,2,2,, +131,license_plate,0,yes,vehicles,vehicles,,0,0,0,0,0,0,metal|plastic,,0,1,0,0,0,1,0,, +23,lid,0,yes,coffee|food|softdrinks,coffee|food|softdrinks,coffee|food|softdrinks,20038,25001,19888,3,0,0,plastic|paper|bioplastic|ceramic|metal|glass,,0,3,28,4,0,3,0,, +184,life_buoy,1,no,,,other,6,11,6,1,1,11,,,0,0,0,0,1,1,1,, +132,light,0,yes,vehicles,vehicles,,0,0,0,0,0,0,glass|plastic,,0,1,0,0,0,1,0,, +33,light_bulb,0,yes,electronics,electronics,electronics,1,1,1,1,0,0,glass|plastic|metal,,0,1,0,0,0,0,0,, +119,lighters,0,yes,smoking,smoking,marine|smoking,843,886,843,2,1,60,plastic|metal,,0,1,0,1,0,1,1,, +18,loose_cables,0,yes,civic,civic,,0,0,0,0,0,0,,,0,1,0,0,0,0,0,, +70,macroplastics,0,yes,marine,marine,marine,360,790,360,1,0,0,plastic,,0,1,0,0,0,1,1,, +191,magazine,1,no,,,other,93,124,93,1,1,124,,,0,0,0,0,1,2,1,, +98,mattress,0,yes,other,other,other,1,1,1,1,0,0,,,0,1,0,0,0,1,0,, +82,medicine_bottle,0,yes,medical,medical,medical,2,2,2,1,0,0,plastic|glass,,0,1,0,0,0,0,0,, +147,mediumplastics,1,no,,,marine,1564,4004,1564,1,1,4004,,,0,0,0,0,1,1,1,, +182,menstrual,1,no,,,sanitary,179,201,179,1,1,201,,,0,0,0,1,1,0,0,, +114,menstrual_cup,0,yes,sanitary,sanitary,,0,0,0,0,0,0,plastic,,0,1,0,0,0,0,0,, +91,metal,0,yes,other,other,other,2722,4134,2722,1,0,0,,,0,40,0,1,0,2,2,, +71,microplastics,0,yes,marine,marine,marine,6698,9948,6698,1,0,0,plastic,,0,1,0,0,0,1,1,, +153,milk_bottle,1,no,,,softdrinks,901,935,901,1,1,935,,,0,0,0,2,1,1,1,, +151,milk_carton,1,no,,,softdrinks,827,874,827,1,1,874,,,0,0,0,2,1,1,1,, +133,mirror,0,yes,vehicles,vehicles,,0,0,0,0,0,0,glass|plastic,,0,1,0,0,0,1,0,, +14,mural,0,yes,art,art,,0,0,0,0,0,0,,,0,1,0,0,0,0,0,, +47,napkins,0,yes,food,food,food,3772,4397,3772,1,0,0,paper|cloth,,0,1,9,0,0,1,1,, +105,nappies,0,yes,sanitary,sanitary,sanitary,462,861,462,1,0,0,plastic|cloth,,0,1,0,0,0,1,1,, +72,nurdles,0,yes,marine,marine,,0,0,0,0,0,0,plastic,,0,1,0,0,0,0,0,, +198,oil,1,no,,,industrial,57,69,57,1,1,69,,,0,0,0,0,1,1,1,, +54,oil_container,0,yes,industrial,industrial,,0,0,0,0,0,0,plastic|metal,,0,1,0,0,0,0,0,, +55,oil_drum,0,yes,industrial,industrial,,0,0,0,0,0,0,metal|plastic,,0,1,0,0,0,0,0,, +1,other,0,yes,alcohol|art|civic|coffee|dumping|electronics|food|industrial|marine|medical|other|pets|sanitary|smoking|softdrinks|vehicles,alcohol|art|civic|coffee|dumping|electronics|food|industrial|marine|medical|other|pets|sanitary|smoking|softdrinks|unclassified|vehicles,alcohol|art|civic|coffee|dumping|electronics|food|industrial|marine|medical|other|pets|sanitary|smoking|softdrinks|vehicles,38074,46906,38065,16,0,0,,,0,16,21,9,0,13,14,, +19,overflowing_bin,0,yes,civic|other,civic|other,civic,6,6,6,1,0,0,,,0,2,0,0,0,0,0,overflowingBins#188,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +188,overflowingBins,1,no,,,other,517,524,517,1,1,524,,,0,0,0,1,1,1,0,overflowing_bin#19,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +9,packaging,0,yes,alcohol|coffee|food|smoking|softdrinks,alcohol|coffee|food|smoking|softdrinks,alcohol|coffee|food|smoking|softdrinks,50475,56301,49672,5,0,0,cardboard|paper|plastic|foil|foam|bioplastic,,0,5,45,5,0,5,1,, +44,packet,0,yes,food,food,food,2910,3095,2910,1,0,0,plastic|foil|paper,,0,1,6,1,0,1,0,, +100,paint_can,0,yes,other,other,other,3,7,3,1,0,0,metal|plastic,,0,1,0,0,0,0,0,, +60,pallet,0,yes,industrial,industrial,,0,0,0,0,0,0,wood|plastic,,0,1,0,0,0,1,0,, +93,paper,0,yes,other,other,other,28548,31919,28548,1,0,0,paper,,0,27,0,6,0,2,2,papers#120,"NORMALISED MATCH BUT DISJOINT CATEGORIES — likely genuinely distinct (cf. paper/papers)" +120,papers,0,yes,smoking,smoking,smoking,6,6,6,1,0,0,paper,,0,1,0,0,0,0,0,paper#93,"NORMALISED MATCH BUT DISJOINT CATEGORIES — likely genuinely distinct (cf. paper/papers)" +34,phone,0,yes,electronics,electronics,,0,0,0,0,0,0,metal|plastic|glass,,0,1,0,0,0,0,3,, +83,pill_pack,0,yes,medical,medical,medical,2,2,2,1,0,0,plastic|aluminium,,0,1,0,0,0,0,0,, +62,pipe,0,yes,industrial,industrial,,0,0,0,0,0,0,metal|plastic|concrete,,0,1,0,0,0,2,0,, +46,pizza_box,0,yes,food,food,food,185,208,185,1,0,0,cardboard,,0,1,0,0,0,1,1,, +84,plaster,0,yes,medical,medical,,0,0,0,0,0,0,plastic|fabric,,0,1,0,0,0,1,0,, +89,plastic,0,yes,other,other,industrial|other,88999,100142,88999,2,1,2937,,,0,105,2,39,0,3,2,, +92,plastic_bag,0,yes,other,other,other,253,264,253,1,0,0,,,0,1,0,0,0,0,0,plasticBags#149,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +149,plasticBags,1,no,,,other,10051,12946,10051,1,1,12946,,,0,0,12,1,1,1,0,plastic_bag#92,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +45,plate,0,yes,food,food,food,1096,1710,1096,1,0,0,plastic|paper|foam|ceramic|metal|glass|bioplastic,,0,1,4,1,0,1,0,, +26,pod,0,yes,coffee,coffee,coffee,1,1,1,1,0,0,plastic|aluminium,,0,2,0,0,0,1,0,, +73,polystyrene_fragment,0,yes,marine,marine,marine,37,42,37,1,0,0,polystyrene,,0,1,0,0,0,0,0,, +94,poster,0,yes,other,other,other,6,6,6,1,0,0,,,0,1,0,0,0,0,0,posters#177,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +177,posters,1,no,,,other,208,268,206,1,1,268,,,0,0,0,2,1,1,0,poster#94,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +20,pothole,0,yes,civic,civic,,0,0,0,0,0,0,,,0,1,0,0,0,0,0,, +121,pouch,0,yes,smoking,smoking,smoking,8,8,8,1,0,0,plastic|foil,tobacco,8,1,0,0,0,0,0,, +35,printer_cartridge,0,yes,electronics,electronics,,0,0,0,0,0,0,plastic,,0,1,0,0,0,0,0,, +10,pull_ring,0,yes,alcohol|softdrinks,alcohol|softdrinks,softdrinks,5,5,5,1,0,0,aluminium,,0,2,0,0,0,1,0,pullRing#175,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +175,pullRing,1,no,,,softdrinks,758,1168,758,1,1,1168,,,0,0,2,1,1,1,0,pull_ring#10,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +170,randomLitter,1,no,,,other,12821,33801,12821,1,1,33801,,,0,0,0,1,1,1,0,, +122,rolling_filter,0,yes,smoking,smoking,smoking,8,8,8,1,0,0,paper|plastic,,0,1,0,0,0,0,0,, +148,rollingPapers,1,no,,,smoking,525,546,525,1,1,546,,,0,0,0,1,1,1,0,, +74,rope,0,yes,marine,marine,marine,3576,4670,3497,1,0,0,nylon|plastic|polyester,small|medium|large,0,1,0,8,0,0,0,, +48,sachet,0,yes,food,food,food,5,5,5,1,0,0,plastic|foil|paper,,0,1,0,0,0,0,0,, +112,sanitary_pad,0,yes,sanitary,sanitary,sanitary,6,6,6,1,0,0,cotton|plastic,,0,1,0,0,0,0,0,, +85,sanitiser,0,yes,medical,medical,medical|sanitary,43,54,43,2,1,53,plastic,,0,1,0,1,1,1,0,, +75,shellfish_bag,0,yes,marine,marine,,0,0,0,0,0,0,plastic,,0,1,0,0,0,0,0,, +76,shotgun_cartridge,0,yes,marine,marine,marine,79,81,79,1,0,0,metal|plastic,,0,1,0,0,0,0,0,shotgun_cartridges#193,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +193,shotgun_cartridges,1,no,,,marine,493,725,493,1,1,725,,,0,0,0,0,1,1,1,shotgun_cartridge#76,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +11,six_pack_rings,0,yes,alcohol,alcohol,alcohol,236,271,236,1,0,0,plastic|cardboard,,0,1,3,0,0,1,1,, +49,skewer,0,yes,food,food,,0,0,0,0,0,0,wood|bamboo|metal,,0,1,0,0,0,0,0,, +27,sleeve,0,yes,coffee,coffee,coffee,4,4,4,1,0,0,cardboard|silicone,,0,1,9,0,0,0,0,, +142,soda_can,1,no,,,softdrinks,16462,18034,16462,1,1,18034,,,0,0,6,1,2,1,0,, +144,spirits_bottle,1,no,,,alcohol,2661,3457,2661,1,1,3457,,,0,0,1,1,1,1,1,, +143,sports_bottle,1,no,,,softdrinks,2931,3277,2931,1,1,3277,,,0,0,5,1,1,1,0,, +24,stirrer,0,yes,coffee,coffee,coffee,13,13,13,1,0,0,wood|plastic|metal|bamboo,,0,1,4,0,0,1,0,, +25,straw,0,yes,coffee|food|softdrinks,coffee|food|softdrinks,coffee|food|softdrinks,247,261,247,3,0,0,plastic|paper|metal|bamboo,,0,3,0,0,0,2,0,straws#150,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +172,straw_packaging,1,no,,,softdrinks,1039,1111,1039,1,1,1111,,,0,0,0,1,1,1,0,, +126,straw_wrapper,0,yes,softdrinks,softdrinks,softdrinks,59,59,59,1,0,0,paper|plastic,,0,1,0,0,0,0,0,, +150,straws,1,no,,,marine|softdrinks,7543,8664,7543,2,2,8664,,,0,0,9,1,2,1,1,straw#25,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +77,styrofoam,0,yes,marine,marine,marine,3308,4799,3248,1,0,0,polystyrene,,0,1,0,8,0,0,0,, +86,syringe,0,yes,medical,medical,,0,0,0,0,0,0,plastic|metal,,0,1,0,0,0,1,0,, +50,takeaway_container,0,yes,food,food,food,9,9,9,1,0,0,plastic|foam|aluminium|cardboard,,0,1,0,0,0,0,0,, +113,tampon,0,yes,sanitary,sanitary,,0,0,0,0,0,0,plastic,,0,1,0,0,0,1,0,, +59,tape,0,yes,industrial,industrial,industrial,370,391,370,1,0,0,plastic,,0,1,0,0,0,1,1,, +87,test_kit,0,yes,medical,medical,medical,1,1,1,1,0,0,plastic,,0,1,0,0,0,0,0,, +51,tinfoil,0,yes,food,food,food,2450,2600,2450,1,0,0,aluminium,,0,1,0,1,0,1,0,, +107,tissue,0,yes,sanitary,sanitary,sanitary,600,602,600,1,0,0,paper,,0,1,0,0,0,0,0,, +162,tobaccopouch,1,no,,,smoking,408,434,408,1,1,434,,,0,0,0,0,1,0,0,, +108,toothbrush,0,yes,sanitary,sanitary,sanitary,71,89,71,1,0,0,plastic|nylon|bamboo|wood,,0,1,1,1,0,1,0,, +109,toothpaste_tube,0,yes,sanitary,sanitary,,0,0,0,0,0,0,plastic|aluminium,,0,1,0,0,0,0,0,, +168,toothpick,1,no,,,sanitary,284,309,284,1,1,309,,,0,0,0,1,1,0,0,, +90,traffic_cone,0,yes,other,other,other,1,1,1,1,0,0,,,0,1,0,1,0,1,1,trafficCone#171,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +171,trafficCone,1,no,,,other,116,136,116,1,1,136,,,0,0,0,1,1,1,0,traffic_cone#90,"normalised key match (lowercase/underscore/plural) — CANDIDATE ONLY, requires human review" +52,tray,0,yes,food,food,food,2,2,2,1,0,0,plastic|foam|cardboard|aluminium,,0,1,0,0,0,0,0,, +134,trim,0,yes,vehicles,vehicles,,0,0,0,0,0,0,plastic,,0,1,0,1,0,0,0,, +135,tyre,0,yes,vehicles,vehicles,other,202,436,202,1,1,436,rubber,,0,1,0,0,1,2,1,, +101,umbrella,0,yes,other,other,,0,0,0,0,0,0,plastic|metal|cloth,,0,1,0,0,0,1,0,, +123,vape,0,yes,smoking,smoking,smoking,27,27,27,1,0,0,plastic|metal|glass,disposable|pen|device|pod|cartridge|mouthpiece|e_liquid_bottle|unknown,8,1,0,0,0,0,0,, +189,vapeOil,1,no,,,smoking,63,74,63,1,1,74,,,0,0,0,1,1,1,0,, +190,vapePen,1,no,,,smoking,484,582,484,1,1,582,,,0,0,0,1,1,1,0,, +21,walkability,0,yes,civic,civic,,0,0,0,0,0,0,,,0,1,0,0,0,0,0,, +152,washingUp,1,no,,,other,79,102,79,1,1,102,,,0,0,0,1,1,1,0,, +140,water_bottle,1,no,,,softdrinks,17189,21889,17189,1,1,21889,,,0,0,5,1,2,1,0,, +179,wetwipes,1,no,,,sanitary,3326,6052,3326,1,1,6052,,,0,0,0,0,1,1,1,, +136,wheel,0,yes,vehicles,vehicles,,0,0,0,0,0,0,metal|plastic|rubber,,0,1,0,0,0,1,0,, +139,wine_bottle,1,no,,,alcohol,638,863,638,1,1,863,,,0,0,0,1,1,1,0,, +12,wine_cork,0,yes,alcohol,alcohol,,0,0,0,0,0,0,cork|plastic,,0,1,0,0,0,0,0,, +104,wipes,0,yes,sanitary,sanitary,sanitary,36,80,36,1,0,0,polyester|plastic,,0,1,0,0,0,1,1,, +61,wire,0,yes,industrial,industrial,industrial,1,1,1,1,0,0,copper|plastic|steel,,0,1,0,0,0,1,0,, +53,wrapper,0,yes,food,food,food,19672,22565,19672,1,0,0,plastic|paper|foil|bioplastic,,0,1,24,1,0,1,0,, diff --git a/readme/changelog/2026-09-14.md b/readme/changelog/2026-09-14.md new file mode 100644 index 000000000..6707de894 --- /dev/null +++ b/readme/changelog/2026-09-14.md @@ -0,0 +1,12 @@ +# 2026-09-14 + +## Session — Migrate individual object tags (v5.13.1) + +- Add preview and resumable migration of one old object to its replacement. — v5.13.1 +- Redirect retired object submissions and saved quick tags to the replacement. — v5.13.1 +- Hide retired objects from tag suggestions. — v5.13.1 +- Preserve explicit categories or reject the edit without deleting observations. — v5.13.1 +- Preserve global and location metrics for photos without an owner. — v5.13.1 +- Restrict database tests to local test databases and replace server-wide Redis flushing. — v5.13.1 + +Production requires a write freeze and a fresh-copy rehearsal. Run each approved mapping separately; no production catalogue seeder is required. See [deployment steps and follow-up PRs](../PostTagMigrationClean.md). diff --git a/tests/CreatesApplication.php b/tests/CreatesApplication.php index e3a47be44..f35eae50a 100644 --- a/tests/CreatesApplication.php +++ b/tests/CreatesApplication.php @@ -4,6 +4,8 @@ use Illuminate\Contracts\Console\Kernel; use Illuminate\Foundation\Application; +use Illuminate\Support\ConfigurationUrlParser; +use RuntimeException; trait CreatesApplication { @@ -18,6 +20,29 @@ public function createApplication(): Application $app->make(Kernel::class)->bootstrap(); + $this->assertSafeTestConfiguration($app); + return $app; } + + /** - Refuse destructive test setup outside the dedicated local test databases. */ + protected function assertSafeTestConfiguration(Application $app): void + { + $config = $app['config']; + $parser = new ConfigurationUrlParser; + $database = $parser->parseConfiguration($config->get('database.connections.'.$config->get('database.default'), [])); + $redis = $parser->parseConfiguration($config->get('database.redis.default', [])); + $localHosts = ['127.0.0.1', 'localhost', '::1']; + + if (! $app->environment('testing') + || $config->get('cache.default') !== 'array' + || ($database['driver'] ?? null) !== 'mysql' + || ($database['database'] ?? null) !== 'olm_test' + || ! in_array($database['host'] ?? null, $localHosts, true) + || isset($database['read']) || isset($database['write']) + || (string) ($redis['database'] ?? '') !== '2' + || ! in_array($redis['host'] ?? null, $localHosts, true)) { + throw new RuntimeException('Tests require testing, array cache, local MySQL olm_test and local Redis database 2.'); + } + } } diff --git a/tests/Feature/Achievements/LongTermAchievementsTest.php b/tests/Feature/Achievements/LongTermAchievementsTest.php index f495b2a5b..7c955c41e 100644 --- a/tests/Feature/Achievements/LongTermAchievementsTest.php +++ b/tests/Feature/Achievements/LongTermAchievementsTest.php @@ -30,7 +30,7 @@ protected function setUp(): void parent::setUp(); // Complete Redis flush - Redis::flushall(); + Redis::flushdb(); Cache::flush(); TagKeyCache::forgetAll(); diff --git a/tests/Feature/Photos/AddTagsToPhotoTest.php b/tests/Feature/Photos/AddTagsToPhotoTest.php index e154e47a6..2570c617f 100644 --- a/tests/Feature/Photos/AddTagsToPhotoTest.php +++ b/tests/Feature/Photos/AddTagsToPhotoTest.php @@ -119,7 +119,7 @@ public function test_a_user_can_add_tags_with_materials() 'tags' => [ [ 'category' => 'alcohol', - 'object' => 'beer_bottle', + 'object' => 'bottle', 'quantity' => 2, 'picked_up' => false, 'materials' => [ @@ -131,6 +131,7 @@ public function test_a_user_can_add_tags_with_materials() $photoTag = PhotoTag::where('photo_id', $photo->id)->first(); $this->assertNotNull($photoTag); + $this->assertEquals('bottle', $photoTag->object->key); $materialExtras = $photoTag->extraTags()->where('tag_type', 'material')->get(); $this->assertCount(1, $materialExtras); diff --git a/tests/Feature/Photos/PhotoSignedUrlTest.php b/tests/Feature/Photos/PhotoSignedUrlTest.php index 873ab40b1..94361eee9 100644 --- a/tests/Feature/Photos/PhotoSignedUrlTest.php +++ b/tests/Feature/Photos/PhotoSignedUrlTest.php @@ -13,6 +13,14 @@ class PhotoSignedUrlTest extends TestCase private string $endpoint = '/api/photos'; + protected function tearDown(): void + { + if ($this->app) { + $this->app->detectEnvironment(fn () => 'testing'); + } + parent::tearDown(); + } + /** @test */ public function it_returns_403_without_origin_header() { diff --git a/tests/Feature/QuickTags/QuickTagsApiTest.php b/tests/Feature/QuickTags/QuickTagsApiTest.php index 2111fc662..36d6fce5b 100644 --- a/tests/Feature/QuickTags/QuickTagsApiTest.php +++ b/tests/Feature/QuickTags/QuickTagsApiTest.php @@ -382,6 +382,10 @@ public function test_type_id_is_stored_when_provided(): void $user = User::factory()->create(); $clo = $this->createClo(); $typeId = $this->createType(); + DB::table('category_object_types')->insert([ + 'category_litter_object_id' => $clo, + 'litter_object_type_id' => $typeId, + ]); $this->actingAs($user) ->putJson('/api/v3/user/quick-tags', [ diff --git a/tests/Feature/Tags/AddNewTagsToPhotosTest.php b/tests/Feature/Tags/AddNewTagsToPhotosTest.php index bb5aa0149..401ce1330 100644 --- a/tests/Feature/Tags/AddNewTagsToPhotosTest.php +++ b/tests/Feature/Tags/AddNewTagsToPhotosTest.php @@ -142,7 +142,7 @@ public function test_it_adds_tags_to_a_photo (): void ]); } - public function test_it_falls_back_when_object_does_not_match_category(): void + public function test_it_rejects_an_explicit_category_that_does_not_contain_the_object(): void { $this->seed(GenerateTagsSeeder::class); @@ -154,7 +154,6 @@ public function test_it_falls_back_when_object_does_not_match_category(): void // "butts" belongs only to "smoking", not "alcohol" $object = LitterObject::where('key', 'butts')->first(); $wrongCategory = Category::where('key', CategoryKey::Alcohol->value)->first(); - $correctCategory = $object->categories()->first(); $response = $this->postJson('/api/v3/tags', [ 'photo_id' => $photo->id, @@ -166,14 +165,8 @@ public function test_it_falls_back_when_object_does_not_match_category(): void ] ]); - $response->assertOk(); - - // Falls back to the correct category (smoking) instead of erroring - $this->assertDatabaseHas('photo_tags', [ - 'photo_id' => $photo->id, - 'category_id' => $correctCategory->id, - 'litter_object_id' => $object->id, - ]); + $response->assertUnprocessable(); + $this->assertDatabaseMissing('photo_tags', ['photo_id' => $photo->id]); } public function test_it_fails_to_upload_if_the_user_does_not_own_the_photo (): void @@ -385,9 +378,9 @@ public function test_multi_category_object_falls_back_when_no_category_sent(): v } /** - * Multi-category object with WRONG category sent falls back to first(). + * - Reject an explicit category that does not contain the object. */ - public function test_multi_category_object_falls_back_when_wrong_category_sent(): void + public function test_multi_category_object_rejects_wrong_explicit_category(): void { $this->seed(GenerateTagsSeeder::class); @@ -399,7 +392,6 @@ public function test_multi_category_object_falls_back_when_wrong_category_sent() // "bottle" belongs to "alcohol" and "soft_drinks" — NOT "smoking" $object = LitterObject::where('key', 'bottle')->first(); $wrongCategory = Category::where('key', CategoryKey::Smoking->value)->first(); - $firstCategory = $object->categories()->first(); // Verify smoking is not a valid category for bottle $this->assertFalse( @@ -418,20 +410,8 @@ public function test_multi_category_object_falls_back_when_wrong_category_sent() ], ]); - $response->assertOk(); - - // Should fall back to the first valid category, not the wrong one - $this->assertDatabaseHas('photo_tags', [ - 'photo_id' => $photo->id, - 'category_id' => $firstCategory->id, - 'litter_object_id' => $object->id, - ]); - - $this->assertDatabaseMissing('photo_tags', [ - 'photo_id' => $photo->id, - 'category_id' => $wrongCategory->id, - 'litter_object_id' => $object->id, - ]); + $response->assertUnprocessable(); + $this->assertDatabaseMissing('photo_tags', ['photo_id' => $photo->id]); } /** diff --git a/tests/Feature/Tags/MigrateTagResumeTest.php b/tests/Feature/Tags/MigrateTagResumeTest.php new file mode 100644 index 000000000..535b0eaca --- /dev/null +++ b/tests/Feature/Tags/MigrateTagResumeTest.php @@ -0,0 +1,86 @@ +artisan('migrate:fresh', $this->migrateFreshUsing()); + $this->app[Kernel::class]->setArtisan(null); + RefreshDatabaseState::$migrated = false; + } + + public function test_interruption_after_a_committed_batch_resumes_without_double_counting(): void + { + $category = Category::factory()->create(['key' => 'other']); + $old = LitterObject::factory()->create(['key' => 'plasticBags']); + $new = LitterObject::factory()->create(['key' => 'plastic_bag']); + CategoryObject::create(['category_id' => $category->id, 'litter_object_id' => $new->id]); + $first = Photo::factory()->create(); + $photos = collect([$first])->merge(Photo::factory()->count(200)->create([ + 'user_id' => $first->user_id, 'country_id' => $first->country_id, 'state_id' => $first->state_id, + ])); + $summaries = new GeneratePhotoSummaryService; + foreach ($photos as $photo) { + PhotoTag::create(['photo_id' => $photo->id, 'category_id' => $category->id, 'litter_object_id' => $old->id, 'quantity' => 2]); + $summaries->run($photo); + app(MetricsService::class)->processPhoto($photo); + } + $xp = DB::table('users')->where('id', $first->user_id)->value('xp'); + $totals = DB::table('metrics')->get()->map(fn ($r) => [$r->uploads, $r->litter, $r->xp])->all(); + $calls = 0; + $mock = Mockery::mock(GeneratePhotoSummaryService::class); + $mock->shouldReceive('run')->andReturnUsing(function ($photo) use (&$calls, $summaries) { + if (++$calls === 201) { + throw new RuntimeException('Simulated interruption in the second batch.'); + } + return $summaries->run($photo); + }); + $this->app->instance(GeneratePhotoSummaryService::class, $mock); + $options = ['old' => 'plasticBags', 'new' => 'plastic_bag', '--apply' => true]; + $this->artisan('olm:migrate-tag', $options)->assertFailed(); + $this->assertEquals(0, DB::transactionLevel()); + $this->assertEquals(200, PhotoTag::where('litter_object_id', $new->id)->count()); + $this->assertEquals(1, PhotoTag::where('litter_object_id', $old->id)->count()); + $this->assertNotNull($old->fresh()->retired_at); + $this->assertEquals(400, Redis::hget(RedisKeys::objects(RedisKeys::global()), $new->id)); + $this->assertEquals(2, Redis::hget(RedisKeys::objects(RedisKeys::global()), $old->id)); + $this->app->instance(GeneratePhotoSummaryService::class, $summaries); + $this->artisan('olm:migrate-tag', $options)->assertSuccessful(); + $this->artisan('olm:migrate-tag', $options)->assertSuccessful(); + $this->assertEquals(201, PhotoTag::where('litter_object_id', $new->id)->count()); + $this->assertEquals(0, PhotoTag::where('litter_object_id', $old->id)->count()); + foreach (RedisKeys::getPhotoScopes($first) as $scope) { + $this->assertEquals(402, Redis::hget(RedisKeys::objects($scope), $new->id)); + $this->assertEquals(0, Redis::hget(RedisKeys::objects($scope), $old->id)); + } + Redis::connection()->flushdb(); + $this->artisan('olm:redis:rebuild')->expectsConfirmation('This will FLUSHDB Redis. Continue?', 'yes')->assertSuccessful(); + foreach (RedisKeys::getPhotoScopes($first) as $scope) { + $this->assertEquals(402, Redis::hget(RedisKeys::objects($scope), $new->id)); + $this->assertEquals(0, Redis::hget(RedisKeys::objects($scope), $old->id)); + } + $this->assertEquals(402, Redis::hget(RedisKeys::user($first->user_id).':tags', 'obj:'.$new->id)); + $this->assertEquals(0, Redis::hget(RedisKeys::user($first->user_id).':tags', 'obj:'.$old->id)); + $this->assertEquals($xp, DB::table('users')->where('id', $first->user_id)->value('xp')); + $this->assertEquals($totals, DB::table('metrics')->get()->map(fn ($r) => [$r->uploads, $r->litter, $r->xp])->all()); + } +} diff --git a/tests/Feature/Tags/MigrateTagTest.php b/tests/Feature/Tags/MigrateTagTest.php new file mode 100644 index 000000000..9877b529a --- /dev/null +++ b/tests/Feature/Tags/MigrateTagTest.php @@ -0,0 +1,491 @@ +category = Category::factory()->create(['key' => 'other']); + $this->old = LitterObject::factory()->create(['key' => 'plasticBags']); + $this->new = LitterObject::factory()->create(['key' => 'plastic_bag']); + $this->destination = CategoryObject::create(['category_id' => $this->category->id, 'litter_object_id' => $this->new->id]); + } + + private function observation(?Photo $photo = null, array $attributes = []): PhotoTag + { + return PhotoTag::create(array_merge([ + 'photo_id' => ($photo ?? Photo::factory()->create())->id, + 'category_id' => $this->category->id, + 'litter_object_id' => $this->old->id, + 'quantity' => 2, + 'picked_up' => null, + 'created_at' => '2020-01-02 12:00:00', + 'updated_at' => '2020-01-03 12:00:00', + ], $attributes))->fresh(); + } + + private function migrate(array $options = [], int $exit = 0): void + { + $this->artisan('olm:migrate-tag', array_merge(['old' => $this->old->key, 'new' => $this->new->key, '--apply' => true], $options)) + ->assertExitCode($exit); + } + + private function oldClo(): CategoryObject + { + return CategoryObject::create(['category_id' => $this->category->id, 'litter_object_id' => $this->old->id]); + } + + private function allowType(CategoryObject $clo, string $key): LitterObjectType + { + $type = LitterObjectType::factory()->create(['key' => $key]); + $clo->types()->attach($type->id); + return $type; + } + + public function test_missing_retirement_schema_is_refused_even_in_preview(): void + { + $schema = \Mockery::mock(\Illuminate\Support\Facades\Schema::getFacadeRoot()); + \Illuminate\Support\Facades\Schema::swap($schema); + $schema->shouldReceive('hasColumns') + ->with('litter_objects', ['retired_at', 'merged_into_id', 'merged_into_type_id']) + ->once()->andReturn(false); + $this->artisan('olm:migrate-tag', ['old' => 'plasticBags', 'new' => 'plastic_bag']) + ->expectsOutput('Apply the pending object-retirement schema migrations first.')->assertFailed(); + $this->assertNull($this->old->fresh()->retired_at); + } + + public function test_preview_changes_nothing_and_never_requires_a_source_clo(): void + { + $tag = $this->observation(); + $before = $tag->getRawOriginal(); + $this->artisan('olm:migrate-tag', ['old' => 'plasticBags', 'new' => 'plastic_bag']) + ->expectsOutput('Rows: 1')->expectsOutput('Quantity: 2')->assertSuccessful(); + $this->assertSame($before, $tag->fresh()->getRawOriginal()); + $this->assertNull($this->old->fresh()->retired_at); + $this->assertDatabaseCount('category_litter_object', 1); + $this->assertNull($tag->photo->summary); + } + + public function test_apply_preserves_observations_extras_totals_and_existing_new_tags(): void + { + $photo = Photo::factory()->create(['verified' => 0, 'is_public' => false, 'team_approved_at' => '2020-01-01 00:00:00']); + $tags = collect([true, false, null])->map(fn ($status) => $this->observation($photo, ['picked_up' => $status])); + $tag = $tags->first(); + $tag->attachExtraTags([['id' => BrandList::factory()->create()->id, 'quantity' => 3]], 'brand'); + $tag->attachExtraTags([['id' => Materials::factory()->create()->id]], 'material'); + $tag->attachExtraTags([['id' => CustomTagNew::factory()->create()->id]], 'custom_tag'); + $new = $this->observation($photo, ['litter_object_id' => $this->new->id, 'category_litter_object_id' => $this->destination->id, 'quantity' => 5]); + $unresolved = LitterObject::factory()->create(['key' => 'randomLitter']); + $other = $this->observation($photo, ['litter_object_id' => $unresolved->id]); + $before = $tags->map(fn ($tag) => $tag->fresh()->getRawOriginal()); + $newBefore = $new->getRawOriginal(); + $otherBefore = $other->getRawOriginal(); + $extras = DB::table('photo_tag_extra_tags')->get()->toArray(); + app(GeneratePhotoSummaryService::class)->run($photo); + app(MetricsService::class)->processPhoto($photo); + $photo->refresh(); + $totals = $photo->summary['totals']; + $xp = $photo->xp; + $userXp = $photo->user->xp; + $metricTotals = DB::table('metrics')->get()->map(fn ($r) => [$r->tags, $r->litter, $r->xp, $r->uploads])->all(); + $this->migrate(); + foreach ($tags as $index => $row) { + $expected = $before[$index]; + $expected['litter_object_id'] = $this->new->id; + $expected['category_litter_object_id'] = $this->destination->id; + $this->assertSame($expected, $row->fresh()->getRawOriginal()); + } + $this->assertSame($newBefore, $new->fresh()->getRawOriginal()); + $this->assertSame($otherBefore, $other->fresh()->getRawOriginal()); + $this->assertEquals($extras, DB::table('photo_tag_extra_tags')->get()->toArray()); + $photo->refresh(); + $this->assertSame($totals, $photo->summary['totals']); + $this->assertEquals($xp, $photo->xp); + $this->assertEquals($userXp, $photo->user->fresh()->xp); + $this->assertEquals($metricTotals, DB::table('metrics')->get()->map(fn ($r) => [$r->tags, $r->litter, $r->xp, $r->uploads])->all()); + $this->assertEquals(0, $photo->verified->value); + $this->assertFalse($photo->is_public); + $this->assertEquals('2020-01-01 00:00:00', $photo->team_approved_at->format('Y-m-d H:i:s')); + $this->assertArrayNotHasKey($this->old->id, $photo->summary['keys']['objects']); + foreach (RedisKeys::getPhotoScopes($photo) as $scope) { + $this->assertEquals(0, Redis::hget(RedisKeys::objects($scope), $this->old->id)); + $this->assertEquals(11, Redis::hget(RedisKeys::objects($scope), $this->new->id)); + } + $this->assertEquals(11, Redis::hget(RedisKeys::user($photo->user_id).':tags', 'obj:'.$this->new->id)); + $this->assertEquals(0, Redis::hget(RedisKeys::user($photo->user_id).':tags', 'obj:'.$this->old->id)); + $this->assertDatabaseCount('category_litter_object', 1); + $this->assertEquals($this->new->id, $this->old->fresh()->merged_into_id); + $this->migrate(); + $this->assertEquals(11, Redis::hget(RedisKeys::objects(RedisKeys::global()), $this->new->id)); + } + + public function test_missing_destination_requires_explicit_creation_and_declaration(): void + { + $this->destination->delete(); + $this->observation(); + $this->migrate([], 1); + $this->assertNull($this->old->fresh()->retired_at); + $this->migrate(['--create-destination' => true, '--apply' => false]); + $this->assertDatabaseCount('category_litter_object', 0); + $this->migrate(['--create-destination' => true]); + $this->assertDatabaseCount('category_litter_object', 1); + $this->assertDatabaseHas('category_litter_object', ['litter_object_id' => $this->new->id, 'category_id' => $this->category->id]); + } + + public function test_undeclared_destination_is_not_created(): void + { + $this->destination->delete(); + $this->new->update(['key' => 'unapproved_destination']); + $this->observation(); + $this->migrate(['--create-destination' => true], 1); + $this->assertDatabaseCount('category_litter_object', 0); + $this->assertNull($this->old->fresh()->retired_at); + } + + public function test_missing_category_and_already_typed_observations_are_refused(): void + { + $tag = $this->observation(null, ['category_id' => null]); + $this->migrate([], 1); + $type = $this->allowType($this->destination, 'energy'); + $tag->update(['category_id' => $this->category->id, 'litter_object_type_id' => $type->id]); + $this->migrate([], 1); + $this->assertNull($this->old->fresh()->retired_at); + $this->assertEquals($this->old->id, $tag->fresh()->litter_object_id); + } + + public function test_empty_source_clos_do_not_require_a_destination(): void + { + $empty = Category::factory()->create(); + CategoryObject::create(['category_id' => $empty->id, 'litter_object_id' => $this->old->id]); + $tag = $this->observation(); + $this->migrate(); + $this->assertEquals($this->new->id, $tag->fresh()->litter_object_id); + $this->assertDatabaseMissing('category_litter_object', ['category_id' => $empty->id, 'litter_object_id' => $this->new->id]); + } + + public function test_incompatible_quick_tags_refuse_every_write(): void + { + $clo = $this->oldClo(); + $type = $this->allowType($clo, 'energy'); + $quick = UserQuickTag::create(['materials' => [], 'brands' => [], 'sort_order' => 0, 'user_id' => User::factory()->create()->id, 'clo_id' => $clo->id, 'type_id' => $type->id, 'quantity' => 4]); + $tag = $this->observation(); + $this->migrate(['--apply' => false], 1); + $this->migrate([], 1); + $this->assertNull($this->old->fresh()->retired_at); + $this->assertEquals($this->old->id, $tag->fresh()->litter_object_id); + $this->assertEquals($clo->id, $quick->fresh()->clo_id); + } + + public function test_energy_type_quick_tags_stale_requests_and_conflicting_retries(): void + { + $this->category->update(['key' => 'softdrinks']); + $this->old->update(['key' => 'energy_can']); + $this->new->update(['key' => 'can']); + $type = $this->allowType($this->destination, 'energy'); + $wrong = $this->allowType($this->destination, 'water'); + $otherCategory = Category::factory()->create(['key' => 'alcohol']); + CategoryObject::create(['category_id' => $otherCategory->id, 'litter_object_id' => $this->new->id]); + $clo = $this->oldClo(); + $user = User::factory()->create(); + $quick = UserQuickTag::create(['materials' => [], 'brands' => [], 'sort_order' => 0, 'user_id' => $user->id, 'clo_id' => $clo->id, 'type_id' => $wrong->id, 'quantity' => 4, 'picked_up' => false, 'brands' => [['id' => 9, 'quantity' => 3]]]); + $before = $quick->fresh()->getRawOriginal(); + $tag = $this->observation(); + app(GeneratePhotoSummaryService::class)->run($tag->photo); + app(MetricsService::class)->processPhoto($tag->photo); + $this->migrate(['--type' => 'energy']); + $this->assertEquals(2, Redis::hget(RedisKeys::types(RedisKeys::global()), $type->id)); + $before['clo_id'] = $this->destination->id; + $before['type_id'] = $type->id; + $this->assertSame($before, $quick->fresh()->getRawOriginal()); + $this->assertEquals($type->id, $tag->fresh()->litter_object_type_id); + $this->migrate(['--type' => 'water'], 1); + $this->migrate([], 1); + $this->migrate(['--type' => 'energy']); + foreach ([['object' => 'energy_can'], ['category_litter_object_id' => $clo->id, 'litter_object_type_id' => $wrong->id]] as $input) { + $photo = Photo::factory()->create(['user_id' => $user->id]); + $this->actingAs($user)->postJson('/api/v3/tags', ['photo_id' => $photo->id, 'tags' => [array_merge($input, ['quantity' => 3])]])->assertOk(); + $this->assertDatabaseHas('photo_tags', ['photo_id' => $photo->id, 'litter_object_id' => $this->new->id, 'category_id' => $this->category->id, 'litter_object_type_id' => $type->id]); + } + $this->actingAs($user)->putJson('/api/v3/user/quick-tags', ['tags' => [['materials' => [], 'brands' => [], 'clo_id' => $clo->id, 'type_id' => $wrong->id, 'quantity' => 6]]])->assertOk(); + $this->assertEquals($type->id, $user->quickTags()->first()->type_id); + } + + public function test_mixed_unresolved_photo_rejection_is_atomic(): void + { + $user = User::factory()->create(); + $photo = Photo::factory()->create(['user_id' => $user->id]); + $this->observation($photo); + $unresolved = LitterObject::factory()->create(['key' => 'randomLitter']); + $this->observation($photo, ['litter_object_id' => $unresolved->id]); + $this->migrate(); + $before = $photo->fresh()->getRawOriginal(); + $rows = DB::table('photo_tags')->get()->toArray(); + $this->actingAs($user)->putJson('/api/v3/tags', ['photo_id' => $photo->id, 'tags' => [ + ['category_litter_object_id' => $this->destination->id, 'quantity' => 3], + ['object' => 'randomLitter', 'category_id' => $this->category->id, 'quantity' => 2], + ]])->assertUnprocessable(); + $this->assertSame($before, $photo->fresh()->getRawOriginal()); + $this->assertEquals($rows, DB::table('photo_tags')->get()->toArray()); + } + + public function test_explicit_category_is_preserved_or_rejected_without_deletion(): void + { + $user = User::factory()->create(); + $photo = Photo::factory()->create(['user_id' => $user->id]); + $tag = $this->observation($photo); + $this->migrate(); + $wrong = Category::factory()->create(); + foreach ([$this->old->key, $this->new->key, 'unknown_object'] as $key) { + $this->actingAs($user)->putJson('/api/v3/tags', ['photo_id' => $photo->id, 'tags' => [['object' => $key, 'category_id' => $wrong->id, 'quantity' => 3]]])->assertUnprocessable(); + $this->assertDatabaseHas('photo_tags', ['id' => $tag->id, 'quantity' => 2]); + } + $this->actingAs($user)->putJson('/api/v3/tags', ['photo_id' => $photo->id, 'tags' => [['object' => $this->old->key, 'category_id' => $this->category->id, 'quantity' => 3]]])->assertOk(); + $this->assertDatabaseHas('photo_tags', ['photo_id' => $photo->id, 'category_id' => $this->category->id, 'litter_object_id' => $this->new->id, 'quantity' => 3]); + } + + public function test_unapproved_client_type_preserves_saved_tags_and_quick_tags(): void + { + $user = User::factory()->create(); + $photo = Photo::factory()->create(['user_id' => $user->id]); + $tag = $this->observation($photo); + $clo = $this->oldClo(); + $type = $this->allowType($clo, 'wrong_type'); + $this->migrate(); + $quick = UserQuickTag::create(['materials' => [], 'brands' => [], 'sort_order' => 0, 'user_id' => $user->id, 'clo_id' => $this->destination->id, 'quantity' => 4]); + $this->actingAs($user)->putJson('/api/v3/tags', ['photo_id' => $photo->id, 'tags' => [['category_litter_object_id' => $clo->id, 'litter_object_type_id' => $type->id, 'quantity' => 3]]])->assertUnprocessable(); + $this->assertDatabaseHas('photo_tags', ['id' => $tag->id, 'quantity' => 2]); + $this->actingAs($user)->putJson('/api/v3/user/quick-tags', ['tags' => [['materials' => [], 'brands' => [], 'clo_id' => $clo->id, 'type_id' => $type->id, 'quantity' => 3]]])->assertUnprocessable(); + $this->assertDatabaseHas('user_quick_tags', ['id' => $quick->id, 'quantity' => 4]); + } + + public function test_retired_objects_are_absent_from_catalogue_search_and_top_tags(): void + { + $user = User::factory()->create(); + $clo = $this->oldClo(); + $photo = Photo::factory()->create(['user_id' => $user->id]); + $this->observation($photo, ['category_litter_object_id' => $clo->id, 'quantity' => 3]); + $this->old->update(['retired_at' => now(), 'merged_into_id' => $this->new->id]); + $all = $this->getJson('/api/tags/all')->assertOk(); + $this->assertNotContains($this->old->id, array_column($all->json('objects'), 'id')); + $this->assertNotContains($clo->id, array_column($all->json('category_objects'), 'id')); + $this->getJson('/api/tags?search=plasticBags')->assertOk()->assertJsonCount(0, 'tags'); + $this->actingAs($user)->getJson('/api/v3/user/top-tags')->assertOk()->assertJsonCount(0, 'tags'); + } + + public function test_chain_and_matching_replay_after_a_second_migration(): void + { + $tag = $this->observation(); + $this->migrate(); + $third = LitterObject::factory()->create(); + $thirdClo = CategoryObject::create(['category_id' => $this->category->id, 'litter_object_id' => $third->id]); + $this->migrate(['old' => $this->new->key, 'new' => $third->key]); + $this->assertEquals($third->id, $tag->fresh()->litter_object_id); + $this->migrate(); + $user = User::factory()->create(); + $photo = Photo::factory()->create(['user_id' => $user->id]); + $this->actingAs($user)->postJson('/api/v3/tags', ['photo_id' => $photo->id, 'tags' => [['object' => $this->old->key, 'quantity' => 3]]])->assertOk(); + $this->assertDatabaseHas('photo_tags', ['photo_id' => $photo->id, 'category_litter_object_id' => $thirdClo->id]); + } + + public function test_unfinished_predecessor_blocks_next_migration(): void + { + $this->observation(); + $this->old->update(['retired_at' => now(), 'merged_into_id' => $this->new->id]); + $third = LitterObject::factory()->create(); + $this->migrate(['old' => $this->new->key, 'new' => $third->key], 1); + $this->assertNull($this->new->fresh()->retired_at); + $this->artisan('olm:migrate-tag', ['old' => $this->old->key, 'new' => $this->new->key]) + ->expectsOutput('Resume: matching retirement is recorded; processing remaining rows.')->assertSuccessful(); + $this->migrate(); + } + + public function test_ownerless_and_soft_deleted_photos_are_migrated(): void + { + $photo = Photo::factory()->create(['user_id' => null]); + $tag = $this->observation($photo); + app(GeneratePhotoSummaryService::class)->run($photo); + app(MetricsService::class)->processPhoto($photo); + $deleted = Photo::factory()->create(['deleted_at' => now()]); + $deletedTag = $this->observation($deleted); + $this->migrate(); + $this->assertEquals($this->new->id, $tag->fresh()->litter_object_id); + $this->assertEquals($this->new->id, $deletedTag->fresh()->litter_object_id); + $this->assertArrayHasKey($this->new->id, Photo::withTrashed()->find($deleted->id)->summary['keys']['objects']); + $this->assertFalse(DB::table('metrics')->where('user_id', '>', 0)->exists()); + $this->assertEmpty(Redis::zrange(RedisKeys::xpRanking(RedisKeys::global()), 0, -1)); + } + + public function test_school_photo_summary_changes_but_metrics_wait_for_approval(): void + { + $teacher = User::factory()->create(); + $student = User::factory()->create(); + $school = \App\Models\Teams\TeamType::factory()->create(['team' => 'school']); + $team = \App\Models\Teams\Team::factory()->create(['type_id' => $school->id, 'leader' => $teacher->id, 'safeguarding' => true]); + $team->users()->attach([$teacher->id, $student->id]); + $photo = Photo::factory()->create(['user_id' => $student->id, 'team_id' => $team->id, 'verified' => 1, 'is_public' => false]); + $this->observation($photo); + $this->migrate(); + $photo->refresh(); + $this->assertArrayHasKey($this->new->id, $photo->summary['keys']['objects']); + $this->assertNull($photo->processed_at); + $this->assertNull($photo->team_approved_at); + $this->assertEquals(1, $photo->verified->value); + $this->assertFalse($photo->is_public); + $this->assertDatabaseCount('metrics', 0); + \Illuminate\Support\Facades\Event::fake([\App\Events\SchoolDataApproved::class]); + $this->actingAs($teacher)->postJson('/api/teams/photos/approve', ['team_id' => $team->id, 'photo_ids' => [$photo->id]])->assertOk(); + $photo->refresh(); + $this->assertNotNull($photo->processed_at); + $this->assertEquals(2, Redis::hget(RedisKeys::objects(RedisKeys::global()), $this->new->id)); + $this->assertEquals(0, Redis::hget(RedisKeys::objects(RedisKeys::global()), $this->old->id)); + } + + public function test_recorded_chain_type_is_validated_at_the_final_destination(): void + { + $type = $this->allowType($this->destination, 'energy'); + $this->old->update(['retired_at' => now(), 'merged_into_id' => $this->new->id, 'merged_into_type_id' => $type->id]); + $final = LitterObject::factory()->create(); + $finalClo = CategoryObject::create(['category_id' => $this->category->id, 'litter_object_id' => $final->id]); + $this->new->update(['retired_at' => now(), 'merged_into_id' => $final->id]); + $user = User::factory()->create(); + $photo = Photo::factory()->create(['user_id' => $user->id]); + $payload = ['photo_id' => $photo->id, 'tags' => [['object' => $this->old->key, 'category_id' => $this->category->id, 'quantity' => 1]]]; + $this->actingAs($user)->postJson('/api/v3/tags', $payload)->assertUnprocessable(); + $replacementType = $this->allowType($finalClo, 'beer'); + $this->new->update(['merged_into_type_id' => $replacementType->id]); + $this->postJson('/api/v3/tags', $payload)->assertOk(); + $this->assertDatabaseHas('photo_tags', ['photo_id' => $photo->id, 'litter_object_type_id' => $replacementType->id]); + } + + public function test_cycles_missing_targets_and_ambiguous_replacements_are_rejected(): void + { + $user = User::factory()->create(); + $photo = Photo::factory()->create(['user_id' => $user->id]); + $this->old->update(['retired_at' => now(), 'merged_into_id' => $this->new->id]); + $another = Category::factory()->create(); + CategoryObject::create(['category_id' => $another->id, 'litter_object_id' => $this->new->id]); + $payload = ['photo_id' => $photo->id, 'tags' => [['object' => $this->old->key, 'quantity' => 1]]]; + $this->actingAs($user)->postJson('/api/v3/tags', $payload)->assertUnprocessable(); + $this->new->update(['retired_at' => now(), 'merged_into_id' => $this->old->id]); + $this->postJson('/api/v3/tags', $payload)->assertUnprocessable(); + $this->new->update(['merged_into_id' => null]); + $this->postJson('/api/v3/tags', $payload)->assertUnprocessable(); + $this->assertDatabaseCount('photo_tags', 0); + } + + public function test_creating_a_destination_lists_and_creates_only_declared_existing_types(): void + { + $this->destination->delete(); + $this->category->update(['key' => 'dumping']); + $this->old->update(['key' => 'dumping_small']); + $this->new->update(['key' => 'dumping']); + $this->observation(); + $small = LitterObjectType::factory()->create(['key' => 'small']); + $this->migrate(['--create-destination' => true, '--type' => 'small', '--allow-xp-change' => true], 1); + $this->assertNull($this->old->fresh()->retired_at); + LitterObjectType::factory()->create(['key' => 'medium']); + LitterObjectType::factory()->create(['key' => 'large']); + $options = ['old' => $this->old->key, 'new' => $this->new->key, '--create-destination' => true, '--type' => 'small', '--allow-xp-change' => true]; + $this->artisan('olm:migrate-tag', $options) + ->expectsOutput("Would create category_object_types: new destination CLO, litter_object_type_id={$small->id}.")->assertSuccessful(); + $this->assertDatabaseCount('category_object_types', 0); + $this->migrate($options); + $this->assertDatabaseCount('category_object_types', 3); + $this->assertDatabaseCount('taggables', 0); + $this->assertDatabaseCount('litter_object_types', 3); + } + + public function test_catalogue_creation_failure_rolls_back_the_retirement_transaction(): void + { + $this->destination->delete(); + $tag = $this->observation(); + \Illuminate\Support\Facades\Event::listen('eloquent.created: '.CategoryObject::class, function () { + throw new \RuntimeException('Simulated catalogue write failure.'); + }); + $this->migrate(['--create-destination' => true], 1); + $this->assertDatabaseCount('category_litter_object', 0); + $this->assertNull($this->old->fresh()->retired_at); + $this->assertEquals($this->old->id, $tag->fresh()->litter_object_id); + } + + public function test_type_option_does_not_leak_to_the_next_command_in_the_same_process(): void + { + $type = $this->allowType($this->destination, 'energy'); + $this->observation(); + $this->migrate(['--type' => $type->key]); + $old = LitterObject::factory()->create(); + $new = LitterObject::factory()->create(); + CategoryObject::create(['category_id' => $this->category->id, 'litter_object_id' => $new->id]); + $tag = $this->observation(null, ['litter_object_id' => $old->id]); + $this->migrate(['old' => $old->key, 'new' => $new->key]); + $this->assertEquals($new->id, $tag->fresh()->litter_object_id); + $this->assertNull($old->fresh()->merged_into_type_id); + $this->assertNull($tag->fresh()->litter_object_type_id); + } + + public function test_unknown_identical_and_retired_destinations_are_refused(): void + { + $this->observation(); + $this->migrate(['new' => 'unknown_destination'], 1); + $this->migrate(['new' => $this->old->key], 1); + $this->migrate(['--type' => 'unknown_type'], 1); + $this->new->update(['retired_at' => now(), 'merged_into_id' => $this->old->id]); + $this->migrate([], 1); + $this->assertNull($this->old->fresh()->retired_at); + } + + public function test_actual_wide_and_long_csv_use_replacement_keys(): void + { + $user = User::factory()->create(); + $photo = Photo::factory()->create(['user_id' => $user->id, 'verified' => 2, 'is_public' => true]); + $this->observation($photo); + $this->migrate(); + foreach (['wide', 'long'] as $format) { + $csv = (new CreateCSVExport(null, null, null, $user->id, [], [], [], $format))->raw(\Maatwebsite\Excel\Excel::CSV); + $this->assertStringContainsString('plastic_bag', $csv); + $this->assertStringNotContainsString('plasticBags', $csv); + } + } + + public function test_another_connection_holding_the_lock_blocks_apply(): void + { + config(['database.connections.migration_lock_test' => config('database.connections.mysql')]); + $connection = DB::connection('migration_lock_test'); + try { + $this->assertEquals(1, $connection->selectOne('SELECT GET_LOCK(?, 0) AS acquired', [MigrateTag::LOCK_NAME])->acquired); + $this->observation(); + $this->migrate([], 1); + $this->assertNull($this->old->fresh()->retired_at); + } finally { + $connection->selectOne('SELECT RELEASE_LOCK(?)', [MigrateTag::LOCK_NAME]); + DB::purge('migration_lock_test'); + } + } +} diff --git a/tests/TestCase.php b/tests/TestCase.php index 5b401ddf4..255123cef 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -23,11 +23,6 @@ protected function setUp(): void { parent::setUp(); - if (! app()->environment('testing')) { - echo "Warning: Not using testing env. Please run php artisan cache:clear \n"; - return; - } - // Flush Redis before each test Redis::connection()->flushdb(); @@ -110,8 +105,10 @@ protected function getUnclassifiedOtherCloId(): int */ protected function tearDown(): void { - // Flush Redis after each test - Redis::connection()->flushdb(); + if ($this->app) { + $this->assertSafeTestConfiguration($this->app); + Redis::connection()->flushdb(); + } parent::tearDown(); } diff --git a/tests/Unit/Achievements/TagKeyCacheZeroGuardTest.php b/tests/Unit/Achievements/TagKeyCacheZeroGuardTest.php index cf1fa27d5..2efca63cd 100644 --- a/tests/Unit/Achievements/TagKeyCacheZeroGuardTest.php +++ b/tests/Unit/Achievements/TagKeyCacheZeroGuardTest.php @@ -22,7 +22,7 @@ protected function setUp(): void // Start from a completely cold state TagKeyCache::forgetAll(); Cache::flush(); - Redis::flushall(); + Redis::flushdb(); // *** Intentionally DO NOT pre-insert 'alcohol' *** // We want getOrCreateId() to exercise the full upsert path. diff --git a/tests/Unit/Redis/RedisMetricsCollectorLocationTest.php b/tests/Unit/Redis/RedisMetricsCollectorLocationTest.php index 1ba7199c9..92564e744 100644 --- a/tests/Unit/Redis/RedisMetricsCollectorLocationTest.php +++ b/tests/Unit/Redis/RedisMetricsCollectorLocationTest.php @@ -24,7 +24,7 @@ class RedisMetricsCollectorLocationTest extends TestCase protected function setUp(): void { parent::setUp(); - Redis::flushall(); + Redis::flushdb(); TagKeyCache::forgetAll(); // Seed brands for tests that need them @@ -36,7 +36,7 @@ protected function setUp(): void protected function tearDown(): void { - Redis::flushall(); + Redis::flushdb(); TagKeyCache::forgetAll(); parent::tearDown(); } diff --git a/tests/Unit/Redis/RedisMetricsCollectorTest.php b/tests/Unit/Redis/RedisMetricsCollectorTest.php index fe652c452..8da6c949e 100644 --- a/tests/Unit/Redis/RedisMetricsCollectorTest.php +++ b/tests/Unit/Redis/RedisMetricsCollectorTest.php @@ -22,12 +22,12 @@ class RedisMetricsCollectorTest extends TestCase protected function setUp(): void { parent::setUp(); - Redis::flushall(); + Redis::flushdb(); } protected function tearDown(): void { - Redis::flushall(); + Redis::flushdb(); parent::tearDown(); } diff --git a/tests/Unit/Services/LocationServiceTagTest.php b/tests/Unit/Services/LocationServiceTagTest.php index 9d7be6bce..a80849c65 100644 --- a/tests/Unit/Services/LocationServiceTagTest.php +++ b/tests/Unit/Services/LocationServiceTagTest.php @@ -18,7 +18,7 @@ protected function setUp(): void { parent::setUp(); $this->service = new LocationService(); - Redis::flushall(); + Redis::flushdb(); Cache::flush(); // Seed test tags in database diff --git a/tests/Unit/TestDatabaseSafetyTest.php b/tests/Unit/TestDatabaseSafetyTest.php new file mode 100644 index 000000000..5ddd0dde1 --- /dev/null +++ b/tests/Unit/TestDatabaseSafetyTest.php @@ -0,0 +1,67 @@ +safeApplication(); + $app['config']->set($key, $value); + $this->expectException(RuntimeException::class); + $this->assertSafeTestConfiguration($app); + } + + public static function unsafeConfigurations(): array + { + return [ + ['database.connections.mysql.database', 'olm_postmig_6'], + ['database.connections.mysql.host', 'production.example'], + ['database.connections.mysql.url', 'mysql://root:secret@127.0.0.1/olm_postmig_6'], + ['database.connections.mysql.read', ['host' => 'production.example']], + ['database.connections.mysql.write', ['host' => 'production.example']], + ['database.redis.default.database', 0], + ['database.redis.default.host', 'production.example'], + ['database.redis.default.url', 'redis://127.0.0.1:6379/0'], + ['cache.default', 'redis'], + ]; + } + + public function test_ci_configuration_is_accepted(): void + { + $this->assertSafeTestConfiguration($this->safeApplication()); + $this->addToAssertionCount(1); + } + + public function test_non_testing_environment_is_rejected(): void + { + $app = $this->safeApplication(); + $app['env'] = 'local'; + $this->expectException(RuntimeException::class); + $this->assertSafeTestConfiguration($app); + } + + private function safeApplication(): Application + { + $app = new Application; + $app['env'] = 'testing'; + $app['config'] = new Repository([ + 'cache' => ['default' => 'array'], + 'database' => [ + 'default' => 'mysql', + 'connections' => ['mysql' => ['driver' => 'mysql', 'host' => '127.0.0.1', 'database' => 'olm_test']], + 'redis' => ['default' => ['host' => '127.0.0.1', 'database' => 2]], + ], + ]); + return $app; + } +} From a9234c04adf5709ad22b1109c8c2879c85375280 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Se=C3=A1n=20L=2E?= Date: Mon, 14 Sep 2026 20:28:53 +0100 Subject: [PATCH 2/5] updates --- CLAUDE.md | 2 +- app/Actions/Tags/AddTagsToPhotoAction.php | 19 ++++--- app/Console/Commands/Tags/MigrateTag.php | 55 ++++++++++++++++--- app/Tags/BrandsConfig.php | 24 ++++---- readme/PostTagMigrationClean.md | 27 +++++++-- readme/Tags.md | 4 +- readme/changelog/2026-09-14.md | 3 + resources/js/langs/en.json | 1 + resources/js/langs/en/litter.json | 1 + tests/Feature/Tags/AddNewTagsToPhotosTest.php | 2 +- tests/Feature/Tags/MigrateTagResumeTest.php | 22 +++++++- tests/Feature/Tags/MigrateTagTest.php | 14 ++++- 12 files changed, 133 insertions(+), 41 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 589c24c88..0fed81f25 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -109,7 +109,7 @@ tests/ ## CI (GitHub Actions) -Runs on pushes and PRs targeting `master` or `upgrade/tagging-2025`. +Runs on pushes and PRs targeting `master` or another branch. Pipeline: PHP 8.3, MySQL 8.0, Redis 7 — composer install, migrations and PHPUnit. No Node or frontend build step. ## Current State (v5 — shipped to `master`) diff --git a/app/Actions/Tags/AddTagsToPhotoAction.php b/app/Actions/Tags/AddTagsToPhotoAction.php index d2593a6a8..992a1306b 100644 --- a/app/Actions/Tags/AddTagsToPhotoAction.php +++ b/app/Actions/Tags/AddTagsToPhotoAction.php @@ -59,9 +59,8 @@ public function run(int $userId, int $photoId, array $tags, bool $skipVerificati /** * Create PhotoTag records with extra tags (materials, brands, custom tags). * - * Accepts two payload formats: - * - New: { category_litter_object_id, litter_object_type_id?, ... } - * - Legacy: { object: {id, key}, category?, brand_only?, material_only?, ... } + * - Accept a CLO ID, object/category fields, or standalone extras. + * - Example: { category_litter_object_id: 111, quantity: 2 }. * * @throws \Exception */ @@ -72,11 +71,11 @@ protected function addTagsToPhoto(int $userId, int $photoId, array $tags): array foreach ($tags as $tag) { // Detect payload format if (isset($tag['category_litter_object_id'])) { - $photoTags[] = $this->createTagFromClo($userId, $photoId, $tag); + $photoTags[] = $this->createPhotoTagFromClo($userId, $photoId, $tag); } elseif ($this->isExtraTagOnly($tag)) { $photoTags[] = $this->createExtraTagOnly($userId, $photoId, $tag); } else { - $photoTags[] = $this->createTagLegacy($userId, $photoId, $tag); + $photoTags[] = $this->createPhotoTagFromObject($userId, $photoId, $tag); } } @@ -138,11 +137,13 @@ protected function createExtraTagOnly(int $userId, int $photoId, array $tag): Ph } /** - * New CLO-based tag creation. + * - Create a PhotoTag from category_litter_object_id and attach its extras. + * - Follow a retired object's replacement before saving. + * - Example: { category_litter_object_id: 111, quantity: 2 }. * * @throws \Exception */ - protected function createTagFromClo(int $userId, int $photoId, array $tag): PhotoTag + protected function createPhotoTagFromClo(int $userId, int $photoId, array $tag): PhotoTag { [$clo, $typeId] = app(ResolveLitterObject::class)->fromClo( (int) $tag['category_litter_object_id'], @@ -182,7 +183,7 @@ protected function createTagFromClo(int $userId, int $photoId, array $tag): Phot * * @throws \Exception */ - protected function createTagLegacy(int $userId, int $photoId, array $tag): PhotoTag + protected function createPhotoTagFromObject(int $userId, int $photoId, array $tag): PhotoTag { [$category, $object, $quantity, $pickedUp] = $this->resolveTag($tag); @@ -197,7 +198,7 @@ protected function createTagLegacy(int $userId, int $photoId, array $tag): Photo $tag['category_litter_object_id'] = $clo->id; $tag['litter_object_type_id'] = $typeId; - return $this->createTagFromClo($userId, $photoId, $tag); + return $this->createPhotoTagFromClo($userId, $photoId, $tag); } if (! $category) { throw ValidationException::withMessages(['tags' => 'Category does not contain object.']); diff --git a/app/Console/Commands/Tags/MigrateTag.php b/app/Console/Commands/Tags/MigrateTag.php index 2eb9c53e8..5d6ef94e5 100644 --- a/app/Console/Commands/Tags/MigrateTag.php +++ b/app/Console/Commands/Tags/MigrateTag.php @@ -69,6 +69,9 @@ public function handle(GeneratePhotoSummaryService $summaries, MetricsService $m $this->report($source, $destination, $type, $plan); if ($plan === [] && $source->retired_at !== null) { $this->info('Already applied: no source observations or quick tags remain.'); + if ($this->option('apply')) { + $this->reportCompletion($source, $destination, $type, 0, 0, 0, 0); + } return self::SUCCESS; } if (! $this->option('apply')) { @@ -76,7 +79,7 @@ public function handle(GeneratePhotoSummaryService $summaries, MetricsService $m } $this->checkConnections(); - $destinations = DB::transaction(function () use ($source, $destination, $typeId) { + [$destinations, $quickTagsMoved] = DB::transaction(function () use ($source, $destination, $typeId) { $objects = LitterObject::whereIn('id', [$source->id, $destination->id]) ->orderBy('id')->lockForUpdate()->get()->keyBy('id'); $source = $objects[$source->id]; @@ -102,20 +105,27 @@ public function handle(GeneratePhotoSummaryService $summaries, MetricsService $m 'merged_into_id' => $destination->id, 'merged_into_type_id' => $typeId, ]); + $quickTagsMoved = 0; foreach ($destinations as $categoryId => $cloId) { $values = ['clo_id' => $cloId]; if ($typeId !== null) { $values['type_id'] = $typeId; } - DB::table('user_quick_tags')->whereIn('clo_id', + $quickTagsMoved += DB::table('user_quick_tags')->whereIn('clo_id', DB::table('category_litter_object')->where('litter_object_id', $source->id) ->where('category_id', $categoryId)->select('id') )->update($values); } - return $destinations; + return [$destinations, $quickTagsMoved]; }); $moved = 0; + $quantityMoved = 0; + $photosUpdated = 0; + $totalRows = $this->sourceRows($source->id)->count(); + if ($totalRows > 0) { + $this->line('Progress: 0.0% (0/'.number_format($totalRows).' photo-tag records committed this run).'); + } while (true) { $this->checkConnections(); $photoIds = $this->sourceRows($source->id)->select('photo_id')->distinct() @@ -124,8 +134,9 @@ public function handle(GeneratePhotoSummaryService $summaries, MetricsService $m break; } $started = microtime(true); - $count = DB::transaction(function () use ($photoIds, $source, $destination, $typeId, $destinations, $summaries, $metrics) { + [$count, $quantity, $photoCount] = DB::transaction(function () use ($photoIds, $source, $destination, $typeId, $destinations, $summaries, $metrics) { $photos = Photo::withTrashed()->whereIn('id', $photoIds)->orderBy('id')->lockForUpdate()->get(); + $quantity = (int) $this->sourceRows($source->id)->whereIn('photo_id', $photos->modelKeys())->sum('quantity'); $count = 0; foreach ($photos as $photo) { foreach ($destinations as $categoryId => $cloId) { @@ -141,15 +152,24 @@ public function handle(GeneratePhotoSummaryService $summaries, MetricsService $m $metrics->processPhoto($photo); } } - return $count; + return [$count, $quantity, $photos->count()]; }); if ($count === 0) { throw new RuntimeException('Source rows remain but the batch made no progress. Keep writes paused and inspect.'); } $moved += $count; - $this->line(sprintf('Committed %d photos, %d rows (%.2fs); %d rows moved.', $photoIds->count(), $count, microtime(true) - $started, $moved)); + $quantityMoved += $quantity; + $photosUpdated += $photoCount; + // - Advance only after commit; do not round unfinished work up to 100%. + $percent = floor($moved / max(1, $totalRows) * 1000) / 10; + $this->line(sprintf( + 'Progress: %.1f%% (%s/%s photo-tag records committed this run). Batch: %d photos, %.2fs.', + $percent, number_format($moved), number_format($totalRows), $photoCount, microtime(true) - $started + )); } - $this->info('Migration complete. Verify summaries, XP, exports and Redis deltas before reopening writes.'); + $this->info('Migration complete.'); + $this->reportCompletion($source, $destination, $type, $moved, $quantityMoved, $photosUpdated, $quickTagsMoved); + $this->warn('Verify summaries, XP, exports and Redis deltas before reopening writes. These counts do not verify those checks.'); return self::SUCCESS; } catch (Throwable $e) { $this->error($e->getMessage()); @@ -275,6 +295,27 @@ private function report(LitterObject $source, LitterObject $destination, ?Litter $this->line("Unused source CLOs: {$empty} (informational; no destination required)."); } + /** + * - Report committed work from this invocation, including a resumed run. + * - Destination totals include observations that existed before this run. + */ + private function reportCompletion(LitterObject $source, LitterObject $destination, ?LitterObjectType $type, int $rows, int $quantity, int $photos, int $quickTags): void + { + $this->newLine(); + $this->info("Post-migration summary: {$source->key} → {$destination->key}"); + $this->line('Replacement type: '.($type?->key ?? 'none')); + $this->line('Photo-tag records migrated this run: '.number_format($rows)); + $this->line('Total quantity migrated this run: '.number_format($quantity)); + $this->line('Photos with summaries regenerated this run: '.number_format($photos)); + $this->line('Saved quick tags repointed this run: '.number_format($quickTags)); + $this->line('Source photo-tag records remaining: '.number_format($this->sourceRows($source->id)->count())); + $this->line('Source quick tags remaining: '.number_format($this->quickTags($source->id)->count())); + $this->line("Current {$destination->key} totals (all categories/types, including existing records): " + .number_format($this->sourceRows($destination->id)->count()).' photo-tag records, ' + .number_format((int) $this->sourceRows($destination->id)->sum('quantity')).' total quantity.'); + $this->line('Source object is retired; its recorded replacement remains available for old submissions.'); + } + private function checkConnections(): void { $lock = DB::selectOne('SELECT IS_USED_LOCK(?) AS owner, CONNECTION_ID() AS connection_id', [self::LOCK_NAME]); diff --git a/app/Tags/BrandsConfig.php b/app/Tags/BrandsConfig.php index ae7c22a77..3ea496a93 100644 --- a/app/Tags/BrandsConfig.php +++ b/app/Tags/BrandsConfig.php @@ -19,7 +19,7 @@ class BrandsConfig 'aldi' => [ 'food' => ['packaging'], - 'other' => ['plasticBags'], + 'other' => ['plastic_bag'], ], 'amazon' => [ @@ -108,13 +108,13 @@ class BrandsConfig 'centra' => [ 'food' => ['packaging', 'wrapper'], 'coffee' => ['cup'], - 'other' => ['plasticBags'], + 'other' => ['plastic_bag'], ], 'circlek' => [ 'food' => ['packaging'], 'coffee' => ['cup'], - 'other' => ['plasticBags'], + 'other' => ['plastic_bag'], ], 'coke' => [ @@ -145,7 +145,7 @@ class BrandsConfig 'dunnes' => [ 'food' => ['packaging'], - 'other' => ['plasticBags'], + 'other' => ['plastic_bag'], ], 'duracell' => [ @@ -243,7 +243,7 @@ class BrandsConfig 'lidl' => [ 'food' => ['packaging'], - 'other' => ['plasticBags'], + 'other' => ['plastic_bag'], ], 'lindenvillage' => [ @@ -316,7 +316,7 @@ class BrandsConfig 'ok_' => [ // OK convenience store brand 'food' => ['packaging'], - 'other' => ['plasticBags'], + 'other' => ['plastic_bag'], ], // P @@ -341,7 +341,7 @@ class BrandsConfig // S 'sainsburys' => [ 'food' => ['packaging'], - 'other' => ['plasticBags'], + 'other' => ['plastic_bag'], ], 'samsung' => [ @@ -368,7 +368,7 @@ class BrandsConfig 'spar' => [ 'food' => ['packaging', 'wrapper'], - 'other' => ['plasticBags'], + 'other' => ['plastic_bag'], ], 'sprite' => [ @@ -397,7 +397,7 @@ class BrandsConfig 'supervalu' => [ 'food' => ['packaging'], - 'other' => ['plasticBags'], + 'other' => ['plastic_bag'], ], // T @@ -407,7 +407,7 @@ class BrandsConfig 'tesco' => [ 'food' => ['packaging', 'wrapper'], - 'other' => ['plasticBags'], + 'other' => ['plastic_bag'], ], 'thins' => [ @@ -427,7 +427,7 @@ class BrandsConfig // W 'waitrose' => [ 'food' => ['packaging'], - 'other' => ['plasticBags'], + 'other' => ['plastic_bag'], ], 'walkers' => [ @@ -450,7 +450,7 @@ class BrandsConfig 'woolworths' => [ 'food' => ['packaging'], - 'other' => ['plasticBags'], + 'other' => ['plastic_bag'], ], 'wrigleys' => [ diff --git a/readme/PostTagMigrationClean.md b/readme/PostTagMigrationClean.md index bb7aff77f..b4c5a276c 100644 --- a/readme/PostTagMigrationClean.md +++ b/readme/PostTagMigrationClean.md @@ -17,6 +17,10 @@ php artisan olm:migrate-tag plasticBags plastic_bag The old `plasticBags` object records `plastic_bag` as its replacement. Existing `photo_tags` rows get the new object ID and the destination category's CLO ID. Row IDs, categories, quantities, collection status, extras and both observation timestamps stay unchanged. Existing `plastic_bag` observations stay unchanged too. +During migration, each committed batch prints the percentage complete, migrated/total photo-tag records and batch time. Progress covers the records remaining at the start of this invocation; a resumed run starts at 0% of its remaining work. It advances only after a batch commits. + +After applying, the command prints the photo-tag records and total quantity migrated, photos with regenerated summaries, saved quick tags repointed, remaining source records and quick tags, and current destination totals. Migrated counts cover **this invocation only**: a resumed run counts only its remaining work, and an already-completed replay reports zero migrated. Destination totals include existing observations across all categories and types. This summary does not certify XP, exports or Redis reconciliation; those checks remain separate. + Every affected photo gets a regenerated summary using the existing XP rules; `photos.updated_at` changes. Previously processed, non-deleted photos get metrics deltas, regardless of verification level. Unprocessed school photos wait for approval. Soft-deleted photos get corrected rows and summaries without metrics updates. The command does not approve or publish photos. Old object submissions and old CLO submissions resolve through the recorded replacement before saving. Retired objects disappear from the catalogue, search and generated top tags. Saved quick tags move to the destination CLO; their other fields stay intact, except an explicitly recorded replacement type overrides their type. @@ -28,8 +32,8 @@ The command updates stored data. It does **not** rename references in PHP, JavaS For `plasticBags → plastic_bag`: - **Catalogue:** `TagsConfig` already declares `other/plastic_bag`. The replacement object and its category/object entry (CLO) already exist in the fresh local database. Confirm they exist in the deployment database too; no production seeder is needed for this mapping. -- **Brand configuration — still to update:** replace the 12 `plasticBags` references in `app/Tags/BrandsConfig.php` with `plastic_bag`. For example, Aldi's `other` objects should contain `plastic_bag`. No current runtime reader of this configuration was found; updating the file keeps its associations consistent. It does not update stored brand relationships or require running the old brand tooling. -- **Translations — still to update:** add `"plastic_bag": "Plastic Bags"` under `litter.other` in `resources/js/langs/en.json`, which the current web app loads. Add the same entry under `other` in `resources/js/langs/en/litter.json`. Keep the old entries so historical data can still display a label. Without the new entry, tagging search falls back to “Plastic Bag” from the key. +- **Brand configuration — updated:** all 12 associations in `app/Tags/BrandsConfig.php` now use `plastic_bag`. For example, Aldi's `other` objects contain `plastic_bag`. No current runtime reader of this configuration was found; the change keeps its associations consistent. It does not update stored brand relationships or require running the old brand tooling. +- **Translations — updated:** `"plastic_bag": "Plastic Bags"` is present under `litter.other` in `resources/js/langs/en.json`, which the current web app loads, and under `other` in `resources/js/langs/en/litter.json`. The old entries remain so historical data can still display a label. Deploy the rebuilt frontend to deliver the new translation. - **Photo summaries:** the command regenerates affected summaries with the new object ID and key. For example, five `plasticBags` become five `plastic_bag`; the quantity stays five. - **Downloads:** newly generated CSV exports read the updated catalogue and summaries, so they use `plastic_bag`. The command does not rewrite previously generated or downloaded files; generate a fresh export to see the change. - **Suggestions and saved choices:** the server hides the retired object from new suggestions and updates saved quick tags that reference its CLO. An already-open app can still hold an old catalogue; supported stale submissions use the recorded replacement when saved. Refresh the catalogue during smoke checks. @@ -41,7 +45,22 @@ Keep `plasticBags` where it describes the old data: - Migration tests, command examples and audit records showing the old-to-new mapping. - Legacy translations needed to display old data. The older `plastic_bags` database columns are separate historical fields; do not rename them with this object migration. -Before deployment, finish the pending configuration and translation edits above, build the frontend, and check the displayed label and a fresh export. These edits are separate from `--apply`; running the command alone does not complete them. +Before deployment, build the frontend and check the displayed label and a fresh export. The configuration and translation edits above ship with the application; running `--apply` alone does not deploy them. + +## Recorded tag updates and seeding + +Local database check on 2026-09-14: `olm_postmig_7` records exactly one retired object. This is local execution evidence, not a record of production deployment or completed reconciliation. + +| Old key | New key | Replacement type | Local state | +| --- | --- | --- | --- | +| `plasticBags` | `plastic_bag` | None | Retirement recorded at `2026-09-14 19:16:19` (database value); zero old photo-tag records remain. | + +The brand configuration and English translations have also been updated for this mapping. `energy_can → can --type=energy` is a proposed later run, not a completed migration. Add further completed mappings to this list only after checking their recorded replacements and remaining data. + +- **Fresh database:** `GenerateTagsSeeder` reads `TagsConfig`, which already declares `other/plastic_bag`. It creates the current object and CLO without needing to create or migrate `plasticBags`. +- **Existing database:** run the reviewed `olm:migrate-tag OLD NEW --apply` command separately, with the write freeze and verification described here. The command records the old object's replacement and updates its existing observations. +- **Normal seeding:** do not call data-migration commands automatically from `DatabaseSeeder` or `CreateAllTagsSeeder`. A seed would then change observations, summaries, XP and Redis, and a fresh database may not even contain the old object required by the command. +- **Future automation:** if several approved runs need a runner, make it a separate explicit command with structured old/new/type arguments and failure handling. Listing a mapping must not automatically execute it during seeding. No runner is needed for the single completed mapping above. ## Supported mappings and limits @@ -65,7 +84,7 @@ Matching retries resume the remaining rows; conflicting object/type arguments fa ## Rehearsal and production window -1. Record the candidate commit, MySQL/Redis identities and versions, and reviewed mapping arguments. Rehearse on a **fresh disposable production copy** with production-compatible runtimes. The current local baseline is `olm_postmig_7`; keep it read-only and rehearse on a separate copy. It has 10,051 `plasticBags` observations and no retirement columns as checked on 2026-09-14. Install the retirement schema on the rehearsal copy before previewing the command. Earlier `_6` rehearsal figures are historical evidence, not this baseline. +1. Record the candidate commit, MySQL/Redis identities and versions, and reviewed mapping arguments. Rehearse on a **fresh disposable production copy** with production-compatible runtimes. `olm_postmig_7` initially had 10,051 `plasticBags` observations and no retirement columns, but now contains the user's completed plastic-bag run; it is no longer an untouched baseline. Use a fresh import or a verified pre-run backup for a new rehearsal, and install the retirement schema on that disposable copy before previewing the command. Earlier `_6` figures are historical evidence too. 2. Take verified database backups. Record source rows/quantities by category, destination totals, extras, saved quick tags, per-photo/user XP, MySQL metric values, and Redis object counts in every affected global/location/user scope. Record unresolved combinations separately; this command does not fix unrelated tags. 3. Measure the actual mapping, including batch time, summary regeneration and verification. Earlier estimates were about 51 photo batches for plastic bags and 103 for energy cans; size the window from the fresh rehearsal, not those estimates. 4. Freeze uploads, tag edits, approvals and other writes on **every node**. Drain in-flight requests, pause schedules and workers, and verify they are stopped. `horizon:terminate` alone allows a supervisor to restart workers. diff --git a/readme/Tags.md b/readme/Tags.md index f25470f38..b773cb97d 100644 --- a/readme/Tags.md +++ b/readme/Tags.md @@ -243,9 +243,9 @@ XP details and level thresholds: see `readme/XP.md`. `AddTagsToPhotoAction::run($userId, $photoId, $tags, $skipVerification = false)` is the single entry point for adding tags. Everything runs inside a `DB::transaction()`: 1. **Create rows** — `addTagsToPhoto()` iterates the payload, detecting the format per tag: - - `createTagFromClo()` when `category_litter_object_id` is present (resolves denormalised `category_id`/`litter_object_id` from the CLO). + - `createPhotoTagFromClo()` when `category_litter_object_id` is present (follows retired-object replacements, then takes the category and object from the destination CLO). - `createExtraTagOnly()` when the tag is brand-only / material-only / custom-only (null CLO fields). - - `createTagLegacy()` for the legacy `{ object: {id, key}, … }` payload (auto-resolves category from the object). + - `createPhotoTagFromObject()` for an object/category payload, such as `{ object: {id, key}, category_id, quantity }` (preserves an explicit category and resolves retired objects before creating the PhotoTag). 2. **Generate summary + XP** — `$photo->generateSummary()` calls `GeneratePhotoSummaryService`, populating `summary`, `xp`, `total_tags`, `total_brands`. 3. **Verification** — unless `skipVerification` is true, `updateVerification()` runs. For trusted/non-school users it fires `TagsVerifiedByAdmin`, which drives `ProcessPhotoMetrics → MetricsService::processPhoto()`. Admin controllers pass `skipVerification = true` because they handle verification + metrics atomically themselves. diff --git a/readme/changelog/2026-09-14.md b/readme/changelog/2026-09-14.md index 6707de894..2b954e786 100644 --- a/readme/changelog/2026-09-14.md +++ b/readme/changelog/2026-09-14.md @@ -3,6 +3,9 @@ ## Session — Migrate individual object tags (v5.13.1) - Add preview and resumable migration of one old object to its replacement. — v5.13.1 +- Report migrated records, quantities, photos, quick tags and remaining records after each migration. — v5.13.1 +- Show migration percentage after each committed batch. — v5.13.1 +- Use plastic_bag in brand associations and add its English label. — v5.13.1 - Redirect retired object submissions and saved quick tags to the replacement. — v5.13.1 - Hide retired objects from tag suggestions. — v5.13.1 - Preserve explicit categories or reject the edit without deleting observations. — v5.13.1 diff --git a/resources/js/langs/en.json b/resources/js/langs/en.json index d80424d2c..dac2aa651 100644 --- a/resources/js/langs/en.json +++ b/resources/js/langs/en.json @@ -944,6 +944,7 @@ "cableTie": "Cable Tie", "overflowingBins": "Overflowing Bins", "plasticBags": "Plastic Bags", + "plastic_bag": "Plastic Bags", "posters": "Posters", "randomLitter": "Random Litter", "trafficCone": "Traffic Cone", diff --git a/resources/js/langs/en/litter.json b/resources/js/langs/en/litter.json index d80262c46..ad5ca9767 100644 --- a/resources/js/langs/en/litter.json +++ b/resources/js/langs/en/litter.json @@ -394,6 +394,7 @@ "cableTie": "Cable Tie", "overflowingBins": "Overflowing Bins", "plasticBags": "Plastic Bags", + "plastic_bag": "Plastic Bags", "posters": "Posters", "randomLitter": "Random Litter", "trafficCone": "Traffic Cone", diff --git a/tests/Feature/Tags/AddNewTagsToPhotosTest.php b/tests/Feature/Tags/AddNewTagsToPhotosTest.php index 401ce1330..55882a35b 100644 --- a/tests/Feature/Tags/AddNewTagsToPhotosTest.php +++ b/tests/Feature/Tags/AddNewTagsToPhotosTest.php @@ -529,7 +529,7 @@ public function test_standalone_custom_tag_with_ampersand_saves(): void } /** - * Custom tags attached to an object with . ' / save (createTagFromClo path). + * Custom tags attached to an object with . ' / save (createPhotoTagFromClo path). */ public function test_object_custom_tags_with_punctuation_save(): void { diff --git a/tests/Feature/Tags/MigrateTagResumeTest.php b/tests/Feature/Tags/MigrateTagResumeTest.php index 535b0eaca..988e3ab48 100644 --- a/tests/Feature/Tags/MigrateTagResumeTest.php +++ b/tests/Feature/Tags/MigrateTagResumeTest.php @@ -56,7 +56,11 @@ public function test_interruption_after_a_committed_batch_resumes_without_double }); $this->app->instance(GeneratePhotoSummaryService::class, $mock); $options = ['old' => 'plasticBags', 'new' => 'plastic_bag', '--apply' => true]; - $this->artisan('olm:migrate-tag', $options)->assertFailed(); + $this->artisan('olm:migrate-tag', $options) + ->expectsOutput('Progress: 0.0% (0/201 photo-tag records committed this run).') + ->expectsOutputToContain('Progress: 99.5% (200/201 photo-tag records committed this run).') + ->doesntExpectOutputToContain('Progress: 100.0%') + ->assertFailed(); $this->assertEquals(0, DB::transactionLevel()); $this->assertEquals(200, PhotoTag::where('litter_object_id', $new->id)->count()); $this->assertEquals(1, PhotoTag::where('litter_object_id', $old->id)->count()); @@ -64,8 +68,20 @@ public function test_interruption_after_a_committed_batch_resumes_without_double $this->assertEquals(400, Redis::hget(RedisKeys::objects(RedisKeys::global()), $new->id)); $this->assertEquals(2, Redis::hget(RedisKeys::objects(RedisKeys::global()), $old->id)); $this->app->instance(GeneratePhotoSummaryService::class, $summaries); - $this->artisan('olm:migrate-tag', $options)->assertSuccessful(); - $this->artisan('olm:migrate-tag', $options)->assertSuccessful(); + $this->artisan('olm:migrate-tag', $options) + ->expectsOutput('Progress: 0.0% (0/1 photo-tag records committed this run).') + ->expectsOutputToContain('Progress: 100.0% (1/1 photo-tag records committed this run).') + ->expectsOutput('Photo-tag records migrated this run: 1') + ->expectsOutput('Total quantity migrated this run: 2') + ->expectsOutput('Photos with summaries regenerated this run: 1') + ->expectsOutput('Current plastic_bag totals (all categories/types, including existing records): 201 photo-tag records, 402 total quantity.') + ->assertSuccessful(); + $this->artisan('olm:migrate-tag', $options) + ->expectsOutput('Photo-tag records migrated this run: 0') + ->expectsOutput('Total quantity migrated this run: 0') + ->expectsOutput('Photos with summaries regenerated this run: 0') + ->expectsOutput('Current plastic_bag totals (all categories/types, including existing records): 201 photo-tag records, 402 total quantity.') + ->assertSuccessful(); $this->assertEquals(201, PhotoTag::where('litter_object_id', $new->id)->count()); $this->assertEquals(0, PhotoTag::where('litter_object_id', $old->id)->count()); foreach (RedisKeys::getPhotoScopes($first) as $scope) { diff --git a/tests/Feature/Tags/MigrateTagTest.php b/tests/Feature/Tags/MigrateTagTest.php index 9877b529a..95ac987c8 100644 --- a/tests/Feature/Tags/MigrateTagTest.php +++ b/tests/Feature/Tags/MigrateTagTest.php @@ -115,7 +115,15 @@ public function test_apply_preserves_observations_extras_totals_and_existing_new $xp = $photo->xp; $userXp = $photo->user->xp; $metricTotals = DB::table('metrics')->get()->map(fn ($r) => [$r->tags, $r->litter, $r->xp, $r->uploads])->all(); - $this->migrate(); + $this->artisan('olm:migrate-tag', ['old' => 'plasticBags', 'new' => 'plastic_bag', '--apply' => true]) + ->expectsOutput('Photo-tag records migrated this run: 3') + ->expectsOutput('Total quantity migrated this run: 6') + ->expectsOutput('Photos with summaries regenerated this run: 1') + ->expectsOutput('Saved quick tags repointed this run: 0') + ->expectsOutput('Source photo-tag records remaining: 0') + ->expectsOutput('Source quick tags remaining: 0') + ->expectsOutput('Current plastic_bag totals (all categories/types, including existing records): 4 photo-tag records, 11 total quantity.') + ->assertSuccessful(); foreach ($tags as $index => $row) { $expected = $before[$index]; $expected['litter_object_id'] = $this->new->id; @@ -219,7 +227,9 @@ public function test_energy_type_quick_tags_stale_requests_and_conflicting_retri $tag = $this->observation(); app(GeneratePhotoSummaryService::class)->run($tag->photo); app(MetricsService::class)->processPhoto($tag->photo); - $this->migrate(['--type' => 'energy']); + $this->artisan('olm:migrate-tag', ['old' => $this->old->key, 'new' => $this->new->key, '--type' => 'energy', '--apply' => true]) + ->expectsOutput('Saved quick tags repointed this run: 1') + ->assertSuccessful(); $this->assertEquals(2, Redis::hget(RedisKeys::types(RedisKeys::global()), $type->id)); $before['clo_id'] = $this->destination->id; $before['type_id'] = $type->id; From 4bfa71c4a978524b0f22f7ddbed28ec37b7897e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Se=C3=A1n=20L=2E?= Date: Mon, 14 Sep 2026 20:33:43 +0100 Subject: [PATCH 3/5] simplify command --- app/Console/Commands/Tags/MigrateTag.php | 54 +++--------------------- readme/ArtisanCommands.md | 2 +- readme/PostTagMigrationClean.md | 8 ++-- readme/changelog/2026-09-14.md | 1 + tests/Feature/Tags/MigrateTagTest.php | 50 ++++++++++++---------- 5 files changed, 40 insertions(+), 75 deletions(-) diff --git a/app/Console/Commands/Tags/MigrateTag.php b/app/Console/Commands/Tags/MigrateTag.php index 5d6ef94e5..6f840afa1 100644 --- a/app/Console/Commands/Tags/MigrateTag.php +++ b/app/Console/Commands/Tags/MigrateTag.php @@ -12,7 +12,6 @@ use App\Models\Photo; use App\Services\Metrics\MetricsService; use App\Services\Tags\GeneratePhotoSummaryService; -use App\Tags\TagsConfig; use Illuminate\Console\Command; use Illuminate\Database\Query\Builder; use Illuminate\Support\Facades\DB; @@ -35,8 +34,6 @@ class MigrateTag extends Command {old : Object key to retire} {new : Replacement object key} {--type= : Approved replacement type key} - {--create-destination : Create missing destination CLOs declared in TagsConfig} - {--allow-xp-change : Allow objects with different per-item XP} {--apply : Apply the migration; default is a read-only preview}'; protected $description = 'Retire an object and migrate its data to its replacement.'; @@ -87,18 +84,7 @@ public function handle(GeneratePhotoSummaryService $summaries, MetricsService $m $plan = $this->plan($source, $destination, $typeId); $destinations = []; foreach ($plan as $categoryId => $entry) { - $cloId = $entry['clo_id']; - if ($cloId === null) { - $cloId = CategoryObject::create([ - 'category_id' => $categoryId, 'litter_object_id' => $destination->id, - ])->id; - foreach ($entry['create_types'] as $id) { - DB::table('category_object_types')->insert([ - 'category_litter_object_id' => $cloId, 'litter_object_type_id' => $id, - ]); - } - } - $destinations[$categoryId] = $cloId; + $destinations[$categoryId] = $entry['clo_id']; } $source->update([ 'retired_at' => $source->retired_at ?? now(), @@ -209,35 +195,16 @@ private function plan(LitterObject $source, LitterObject $destination, ?int $typ if ($this->sourceRows($source->id)->whereNotNull('litter_object_type_id')->exists()) { throw new RuntimeException('Already-typed source observations require a separately reviewed migration.'); } - if (XpScore::getObjectXp($source->key) !== XpScore::getObjectXp($destination->key) - && ! $this->option('allow-xp-change')) { - throw new RuntimeException('Per-item object XP differs. Review and use --allow-xp-change if approved.'); - } - $categories = $this->sourceRows($source->id)->distinct()->pluck('category_id') ->merge($this->quickTags($source->id)->distinct()->pluck('clo.category_id'))->unique(); $plan = []; foreach ($categories as $categoryId) { $category = Category::findOrFail($categoryId); $clo = CategoryObject::where('category_id', $categoryId)->where('litter_object_id', $destination->id)->first(); - $createTypes = []; if ($clo === null) { - if (! $this->option('create-destination')) { - throw new RuntimeException("Missing destination CLO: {$category->key}/{$destination->key}. Use --create-destination only after review."); - } - $config = TagsConfig::get()[$category->key] ?? []; - if (! array_key_exists($destination->key, $config)) { - throw new RuntimeException("Destination {$category->key}/{$destination->key} is not declared in TagsConfig."); - } - foreach ($config[$destination->key]['types'] ?? [] as $key) { - $id = LitterObjectType::where('key', $key)->value('id'); - if ($id === null) { - throw new RuntimeException("Declared type {$key} is missing; no types will be created."); - } - $createTypes[] = (int) $id; - } + throw new RuntimeException("Missing destination CLO: {$category->key}/{$destination->key}. Prepare it separately before migrating."); } - $allowed = $clo ? $clo->types()->pluck('litter_object_types.id')->all() : $createTypes; + $allowed = $clo->types()->pluck('litter_object_types.id')->all(); $quickTypes = $this->quickTags($source->id)->where('clo.category_id', $categoryId)->whereNotNull('qt.type_id')->distinct()->pluck('qt.type_id')->all(); $required = $typeId === null ? $quickTypes : [$typeId]; if (array_diff($required, $allowed)) { @@ -245,7 +212,7 @@ private function plan(LitterObject $source, LitterObject $destination, ?int $typ ->whereNotNull('qt.type_id')->whereNotIn('qt.type_id', $allowed)->pluck('qt.id')->implode(', '); throw new RuntimeException("Incompatible destination or quick-tag type for {$category->key}/{$destination->key}. Quick tag IDs: {$ids}"); } - $plan[(int) $categoryId] = ['clo_id' => $clo?->id, 'category' => $category->key, 'create_types' => $createTypes]; + $plan[(int) $categoryId] = ['clo_id' => $clo->id, 'category' => $category->key]; } return $plan; } @@ -274,22 +241,15 @@ private function report(LitterObject $source, LitterObject $destination, ?Litter $this->line('Example photo IDs: '.$this->sourceRows($source->id)->select('photo_id')->distinct()->orderBy('photo_id')->limit(5)->pluck('photo_id')->implode(', ')); $this->line('Quick tags to repoint: '.$this->quickTags($source->id)->count()); foreach ($this->quickTags($source->id)->select('qt.id', 'qt.clo_id', 'qt.type_id', 'clo.category_id')->get() as $quick) { - $destinationClo = $plan[$quick->category_id]['clo_id'] ?? 'new destination CLO'; + $destinationClo = $plan[$quick->category_id]['clo_id']; $destinationType = $type?->id ?? $quick->type_id ?? 'none'; $this->line("Quick tag {$quick->id}: CLO {$quick->clo_id} → {$destinationClo}, type {$destinationType}."); } $this->line('Per-item object XP delta: '.(XpScore::getObjectXp($destination->key) - XpScore::getObjectXp($source->key))); $this->line('Whole-photo XP is recalculated using current rules; the per-item delta does not predict that total.'); $this->line('Replacement type: '.($type?->key ?? 'none')); - foreach ($plan as $categoryId => $entry) { - if ($entry['clo_id'] === null) { - $this->line("Would create category_litter_object: category_id={$categoryId}, litter_object_id={$destination->id} ({$entry['category']}/{$destination->key})."); - foreach ($entry['create_types'] as $id) { - $this->line("Would create category_object_types: new destination CLO, litter_object_type_id={$id}."); - } - } else { - $this->line("Destination: {$entry['category']}/{$destination->key}, CLO {$entry['clo_id']}."); - } + foreach ($plan as $entry) { + $this->line("Destination: {$entry['category']}/{$destination->key}, CLO {$entry['clo_id']}."); } $empty = CategoryObject::where('litter_object_id', $source->id)->whereNotIn('category_id', array_keys($plan))->count(); $this->line("Unused source CLOs: {$empty} (informational; no destination required)."); diff --git a/readme/ArtisanCommands.md b/readme/ArtisanCommands.md index f158555ad..ad9505819 100644 --- a/readme/ArtisanCommands.md +++ b/readme/ArtisanCommands.md @@ -64,7 +64,7 @@ These live in `tmp/` and are intended for the v5 migration period only. | Command | Purpose | |---------|---------| | `seed:tags` | Catalogue setup for a new local/test database; not a production migration step | -| `olm:migrate-tag OLD NEW` | Preview one object migration; `--apply` updates data. Optional `--type`, `--create-destination`, `--allow-xp-change`. See [deployment guide](PostTagMigrationClean.md). | +| `olm:migrate-tag OLD NEW` | Preview one object migration; `--apply` updates data. Optional `--type`. Destination CLOs and type associations must already exist; XP differences are informational. See [deployment guide](PostTagMigrationClean.md). | | `tags:verify-for-user-id {user_id}` | Verify remaining tags for a user | --- diff --git a/readme/PostTagMigrationClean.md b/readme/PostTagMigrationClean.md index b4c5a276c..1f4a601b0 100644 --- a/readme/PostTagMigrationClean.md +++ b/readme/PostTagMigrationClean.md @@ -53,7 +53,7 @@ Local database check on 2026-09-14: `olm_postmig_7` records exactly one retired | Old key | New key | Replacement type | Local state | | --- | --- | --- | --- | -| `plasticBags` | `plastic_bag` | None | Retirement recorded at `2026-09-14 19:16:19` (database value); zero old photo-tag records remain. | +| `plasticBags` | `plastic_bag` | None | Local retirement recorded on `2026-09-14`; zero old photo-tag records remain. | The brand configuration and English translations have also been updated for this mapping. `energy_can → can --type=energy` is a proposed later run, not a completed migration. Add further completed mappings to this list only after checking their recorded replacements and remaining data. @@ -72,11 +72,11 @@ php artisan olm:migrate-tag energy_can can --type=energy php artisan olm:migrate-tag energy_can can --type=energy --apply ``` -Optional `--create-destination` creates only missing destination CLOs declared in `TagsConfig`, plus their declared type associations. All those type records must already exist. The preview prints the exact proposed rows. It does not create objects, categories, materials or types, or run seeders. Neither of the first two mappings should need this flag if their approved destination CLOs already exist. +Prepare destination catalogue changes separately in reviewed code before running this command. The destination object, CLO and any permitted type associations must already exist in the target database; changing `TagsConfig` alone does not update an existing database. The command never creates catalogue entries or runs seeders. Plastic bags and energy cans already have their required destinations in the checked local catalogue. Source observations with a null category or an existing type are refused. Category moves and migrations of already-typed observations are follow-up work. Incompatible destination types or saved quick-tag types stop the run before retirement. The same checks run in preview and apply. -`--allow-xp-change` is required when the old and new object keys have different per-item XP under the existing summary calculator. A zero per-item delta is **not** a promise of zero whole-photo XP drift: every affected photo is rescored using current rules, including its other observations. Compare XP during rehearsal before approving a mapping. +The preview reports the per-item XP difference without blocking the migration. Every affected photo is rescored using current rules, including its other observations; XP is not frozen or deferred by this command. A zero per-item delta does not promise zero whole-photo XP drift. Changes to XP rules and any later bulk rescore remain separate work. The latest non-null type recorded in an object replacement chain wins. Without a recorded type, a supplied type must be allowed at the final destination or the API returns 422 atomically. For a retired object submitted without a category, the destination and effective type must identify one category; otherwise the API returns 422. Explicit object-format categories are preserved or rejected, never silently substituted. See [API contract](API.md). @@ -155,6 +155,6 @@ Before production, attach the tested commit, approved arguments, fresh-copy timi - Tested the uncommitted working tree on `fix/tags/migrate-tag`, based on `ac688aa6`, using PHP 8.3.16, local MySQL `olm_test` and Redis database 2. - Full PHPUnit suite: **1,388 tests, 6,997 assertions, zero failures, one skipped**. The existing `TypesCheckerTest::test_it_unlocks_per_type_achievement` skips because its fixture has no types; achievement changes remain follow-up work. - `npm run build` passes, with the existing large-chunk warning. PHP syntax and `git diff --check` pass. -- Covered missing source CLOs, explicit destination creation, dry runs, stale object/CLO submissions, type rules, quick tags, observation/extras/timestamp preservation, school approval, ownerless/deleted photos, retry agreement, chains and actual wide/long CSV output. +- Covered missing source CLOs, dry runs, stale object/CLO submissions, type rules, quick tags, observation/extras/timestamp preservation, school approval, ownerless/deleted photos, retry agreement, chains and actual wide/long CSV output. Automatic destination creation was covered in that earlier run but has since been removed; destinations now require separate preparation. - The interruption test commits 200 photos, fails the next batch, resumes the remaining photo and verifies replay and Redis rebuild without double-counting. A second MySQL connection verifies advisory-lock contention. This is exception-based interruption coverage, not a process-kill rehearsal. - No mounted browser session, concurrent editor execution, final-commit CI or fresh production-copy rehearsal was run. Production and `olm_postmig_6` were not modified. Those deployment checks remain pending. diff --git a/readme/changelog/2026-09-14.md b/readme/changelog/2026-09-14.md index 2b954e786..0cc0631bd 100644 --- a/readme/changelog/2026-09-14.md +++ b/readme/changelog/2026-09-14.md @@ -5,6 +5,7 @@ - Add preview and resumable migration of one old object to its replacement. — v5.13.1 - Report migrated records, quantities, photos, quick tags and remaining records after each migration. — v5.13.1 - Show migration percentage after each committed batch. — v5.13.1 +- Require prepared destinations and report XP differences without an approval flag. — v5.13.1 - Use plastic_bag in brand associations and add its English label. — v5.13.1 - Redirect retired object submissions and saved quick tags to the replacement. — v5.13.1 - Hide retired objects from tag suggestions. — v5.13.1 diff --git a/tests/Feature/Tags/MigrateTagTest.php b/tests/Feature/Tags/MigrateTagTest.php index 95ac987c8..e9836d710 100644 --- a/tests/Feature/Tags/MigrateTagTest.php +++ b/tests/Feature/Tags/MigrateTagTest.php @@ -154,17 +154,19 @@ public function test_apply_preserves_observations_extras_totals_and_existing_new $this->assertEquals(11, Redis::hget(RedisKeys::objects(RedisKeys::global()), $this->new->id)); } - public function test_missing_destination_requires_explicit_creation_and_declaration(): void + public function test_missing_destination_is_refused_until_prepared_separately(): void { $this->destination->delete(); - $this->observation(); + $tag = $this->observation(); + $this->migrate(['--apply' => false], 1); $this->migrate([], 1); $this->assertNull($this->old->fresh()->retired_at); - $this->migrate(['--create-destination' => true, '--apply' => false]); + $this->assertEquals($this->old->id, $tag->fresh()->litter_object_id); $this->assertDatabaseCount('category_litter_object', 0); - $this->migrate(['--create-destination' => true]); + CategoryObject::create(['category_id' => $this->category->id, 'litter_object_id' => $this->new->id]); + $this->migrate(); $this->assertDatabaseCount('category_litter_object', 1); - $this->assertDatabaseHas('category_litter_object', ['litter_object_id' => $this->new->id, 'category_id' => $this->category->id]); + $this->assertEquals($this->new->id, $tag->fresh()->litter_object_id); } public function test_undeclared_destination_is_not_created(): void @@ -172,7 +174,7 @@ public function test_undeclared_destination_is_not_created(): void $this->destination->delete(); $this->new->update(['key' => 'unapproved_destination']); $this->observation(); - $this->migrate(['--create-destination' => true], 1); + $this->migrate([], 1); $this->assertDatabaseCount('category_litter_object', 0); $this->assertNull($this->old->fresh()->retired_at); } @@ -410,37 +412,39 @@ public function test_cycles_missing_targets_and_ambiguous_replacements_are_rejec $this->assertDatabaseCount('photo_tags', 0); } - public function test_creating_a_destination_lists_and_creates_only_declared_existing_types(): void + public function test_xp_difference_is_reported_without_blocking_and_types_must_be_prepared(): void { - $this->destination->delete(); $this->category->update(['key' => 'dumping']); $this->old->update(['key' => 'dumping_small']); $this->new->update(['key' => 'dumping']); - $this->observation(); + $tag = $this->observation(); $small = LitterObjectType::factory()->create(['key' => 'small']); - $this->migrate(['--create-destination' => true, '--type' => 'small', '--allow-xp-change' => true], 1); + $this->migrate(['--type' => 'small'], 1); $this->assertNull($this->old->fresh()->retired_at); - LitterObjectType::factory()->create(['key' => 'medium']); - LitterObjectType::factory()->create(['key' => 'large']); - $options = ['old' => $this->old->key, 'new' => $this->new->key, '--create-destination' => true, '--type' => 'small', '--allow-xp-change' => true]; - $this->artisan('olm:migrate-tag', $options) - ->expectsOutput("Would create category_object_types: new destination CLO, litter_object_type_id={$small->id}.")->assertSuccessful(); $this->assertDatabaseCount('category_object_types', 0); + $this->destination->types()->attach($small->id); + app(GeneratePhotoSummaryService::class)->run($tag->photo); + $this->assertEquals(20, $tag->photo->fresh()->xp); + $options = ['old' => $this->old->key, 'new' => $this->new->key, '--type' => 'small']; + $this->artisan('olm:migrate-tag', $options) + ->expectsOutput('Per-item object XP delta: -9')->assertSuccessful(); + $this->assertEquals(20, $tag->photo->fresh()->xp); $this->migrate($options); - $this->assertDatabaseCount('category_object_types', 3); + $this->assertEquals(2, $tag->photo->fresh()->xp); + $this->assertEquals($small->id, $tag->fresh()->litter_object_type_id); + $this->assertDatabaseCount('category_object_types', 1); $this->assertDatabaseCount('taggables', 0); - $this->assertDatabaseCount('litter_object_types', 3); + $this->assertDatabaseCount('litter_object_types', 1); } - public function test_catalogue_creation_failure_rolls_back_the_retirement_transaction(): void + public function test_retirement_write_failure_rolls_back_the_transaction(): void { - $this->destination->delete(); $tag = $this->observation(); - \Illuminate\Support\Facades\Event::listen('eloquent.created: '.CategoryObject::class, function () { - throw new \RuntimeException('Simulated catalogue write failure.'); + \Illuminate\Support\Facades\Event::listen('eloquent.updated: '.LitterObject::class, function () { + throw new \RuntimeException('Simulated retirement write failure.'); }); - $this->migrate(['--create-destination' => true], 1); - $this->assertDatabaseCount('category_litter_object', 0); + $this->migrate([], 1); + $this->assertDatabaseCount('category_litter_object', 1); $this->assertNull($this->old->fresh()->retired_at); $this->assertEquals($this->old->id, $tag->fresh()->litter_object_id); } From 4544e4ec8cbeaae0d0f2a27ca44dcf7714d87e89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Se=C3=A1n=20L=2E?= Date: Mon, 14 Sep 2026 20:50:56 +0100 Subject: [PATCH 4/5] updates --- CLAUDE.md | 3 +- app/Actions/Tags/AddTagsToPhotoAction.php | 6 ++-- .../Controllers/API/QuickTagsController.php | 4 +-- .../API/Tags/GetTagsController.php | 6 ++-- app/Models/Litter/Tags/LitterObject.php | 13 +++++++ app/Services/Tags/ResolveLitterObject.php | 7 +++- readme/API.md | 4 +-- readme/PostTagMigrationClean.md | 4 ++- readme/Tags.md | 14 ++++++++ readme/changelog/2026-09-14.md | 1 + tests/Feature/Tags/AddNewTagsToPhotosTest.php | 19 ++++------ tests/Feature/Tags/MigrateTagTest.php | 36 +++++++++++++++++++ 12 files changed, 93 insertions(+), 24 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 0fed81f25..a38aaf181 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -138,9 +138,10 @@ Built by a single developer over 17 years. - `VerificationStatus` enum cast is on Photo model — use `->value` for `>=`/`<` comparisons, direct enum for `===` - `Photo.geom` column is binary spatial data — hidden from JSON via `$hidden` array - `photo_tags` table uses FK columns (`category_id`, `litter_object_id`), NOT string columns. All three (`category_litter_object_id`, `category_id`, `litter_object_id`) are **nullable** — extra-tag-only PhotoTags have null CLO -- `AddTagsToPhotoAction` (v5) auto-resolves category from object — frontend need not send category. Brand-only, material-only, and custom-only tags use `createExtraTagOnly()` with null CLO +- `AddTagsToPhotoAction` preserves explicit categories. Without one, prefer an existing `other` CLO, then a sole category; reject ambiguity. Editors should send the recorded category or CLO. Brand-only, material-only, and custom-only tags use `createExtraTagOnly()` with null CLO - Replace tags (`PUT /api/v3/tags`) accepts empty `tags: []` to clear all tags from a photo - Object retirement uses `litter_objects.retired_at`, `merged_into_id` and `merged_into_type_id`; resolve the object replacement before choosing its CLO. `olm:migrate-tag` needs no source CLO or production seeder. See `readme/PostTagMigrationClean.md`. +- `LitterObject::active()` delegates to `retired(false)`; `retired()` selects retired objects. Use `active()` explicitly for catalogue/search/top tags. No global scope: stale submissions, migration retries and historical reads need retired records. See `readme/Tags.md#active-and-retired-objects`. - `TagsConfig` provides helper methods: `buildObjectMap()`, `buildObjectMaps()`, `allMaterialKeys()`, `allTypeKeys()` — use these instead of hardcoding lists - Legacy v1/v2 mobile endpoints removed (2026-03-01) — mobile uses v3 endpoints with CLO format only - `Photo` model has `SoftDeletes` trait but all delete endpoints use `forceDelete()` for hard deletion. Cascading FKs on `photo_tags` (→ `photo_tag_extras`) handle relationship cleanup diff --git a/app/Actions/Tags/AddTagsToPhotoAction.php b/app/Actions/Tags/AddTagsToPhotoAction.php index 992a1306b..03cac7215 100644 --- a/app/Actions/Tags/AddTagsToPhotoAction.php +++ b/app/Actions/Tags/AddTagsToPhotoAction.php @@ -365,7 +365,8 @@ protected function attachCustomTags(int $userId, PhotoTag $photoTag, array $cust /** * Resolve category and object from tag input (legacy format). * Accepts either { id: int } or string key for both. - * Auto-resolves category from object if not explicitly provided. + * - Preserve an explicit category. + * - Otherwise prefer an existing other CLO, then a sole category; reject ambiguity. */ protected function resolveTag(array $tag): array { @@ -387,7 +388,8 @@ protected function resolveTag(array $tag): array if ($object && $object->retired_at === null && ! $category && ! isset($tag['category_id']) && ! isset($tag['category'])) { - $category = $object->categories()->first(); + [$clo] = app(ResolveLitterObject::class)->resolve($object, null); + $category = $clo->category; } } diff --git a/app/Http/Controllers/API/QuickTagsController.php b/app/Http/Controllers/API/QuickTagsController.php index a16bc0676..1b2891546 100644 --- a/app/Http/Controllers/API/QuickTagsController.php +++ b/app/Http/Controllers/API/QuickTagsController.php @@ -5,6 +5,7 @@ use App\Actions\QuickTags\SyncQuickTagsAction; use App\Http\Controllers\Controller; use App\Http\Requests\Api\SyncQuickTagsRequest; +use App\Models\Litter\Tags\LitterObject; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Support\Facades\DB; @@ -56,9 +57,8 @@ public function topTags(Request $request): JsonResponse ->join('photos as p', 'p.id', '=', 'pt.photo_id') ->join('category_litter_object as clo', 'clo.id', '=', 'pt.category_litter_object_id') ->join('categories as c', 'c.id', '=', 'clo.category_id') - ->join('litter_objects as lo', 'lo.id', '=', 'clo.litter_object_id') + ->joinSub(LitterObject::active()->select('id', 'key'), 'lo', 'lo.id', '=', 'clo.litter_object_id') ->leftJoin('litter_object_types as lot', 'lot.id', '=', 'pt.litter_object_type_id') - ->whereNull('lo.retired_at') ->where('p.user_id', $userId) ->whereNotNull('pt.category_litter_object_id') ->select( diff --git a/app/Http/Controllers/API/Tags/GetTagsController.php b/app/Http/Controllers/API/Tags/GetTagsController.php index 007a6c86d..6ed86f90f 100644 --- a/app/Http/Controllers/API/Tags/GetTagsController.php +++ b/app/Http/Controllers/API/Tags/GetTagsController.php @@ -53,7 +53,7 @@ public function getAllTags(): JsonResponse $objectTypesMap = $objectMaps['types']; $objectMaterialsMap = $objectMaps['materials']; - $litterObjects = LitterObject::with(['categories:id,key'])->whereNull('retired_at') + $litterObjects = LitterObject::with(['categories:id,key'])->active() ->whereHas('categories') ->select('id', 'key') ->orderBy('key') @@ -73,7 +73,7 @@ public function getAllTags(): JsonResponse $types = LitterObjectType::select('id', 'key', 'name')->orderBy('key')->get(); $categoryObjects = CategoryObject::select('id', 'category_id', 'litter_object_id') - ->whereHas('litterObject', fn ($q) => $q->whereNull('retired_at'))->get(); + ->whereHas('litterObject', fn ($q) => $q->active())->get(); $categoryObjectTypes = DB::table('category_object_types') ->whereIn('category_litter_object_id', $categoryObjects->pluck('id')) @@ -101,7 +101,7 @@ protected function generateQueryFromRequest (Request $request): array $materialsKeys = $request['materials'] ? explode(',', $request['materials']) : null; $searchQuery = $request['search'] ?? null; - $query = CategoryObject::whereHas('litterObject', fn ($q) => $q->whereNull('retired_at')); + $query = CategoryObject::whereHas('litterObject', fn ($q) => $q->active()); if ($categoryKey) { $query->whereHas('category', function($q) use ($categoryKey) { diff --git a/app/Models/Litter/Tags/LitterObject.php b/app/Models/Litter/Tags/LitterObject.php index 6aa46d1e1..ceee72804 100644 --- a/app/Models/Litter/Tags/LitterObject.php +++ b/app/Models/Litter/Tags/LitterObject.php @@ -3,6 +3,7 @@ namespace App\Models\Litter\Tags; use Illuminate\Database\Eloquent\Model; +use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Relations\BelongsToMany; @@ -18,6 +19,18 @@ class LitterObject extends Model protected $casts = ['retired_at' => 'datetime', 'merged_into_id' => 'integer', 'merged_into_type_id' => 'integer']; + public function scopeRetired(Builder $query, bool $retired = true): Builder + { + $column = $this->qualifyColumn('retired_at'); + + return $retired ? $query->whereNotNull($column) : $query->whereNull($column); + } + + public function scopeActive(Builder $query): Builder + { + return $query->retired(false); + } + public function getRouteKeyName(): string { return 'key'; diff --git a/app/Services/Tags/ResolveLitterObject.php b/app/Services/Tags/ResolveLitterObject.php index 35d6aa1cc..42bd8cd20 100644 --- a/app/Services/Tags/ResolveLitterObject.php +++ b/app/Services/Tags/ResolveLitterObject.php @@ -2,6 +2,7 @@ namespace App\Services\Tags; +use App\Enums\CategoryKey; use App\Models\Litter\Tags\CategoryObject; use App\Models\Litter\Tags\LitterObject; use Illuminate\Validation\ValidationException; @@ -12,6 +13,7 @@ class ResolveLitterObject * - Follow retired objects before choosing the category's CLO. * - Example: other/plasticBags becomes other/plastic_bag, even without an old CLO. * - The latest recorded replacement type wins; otherwise validate the submitted type. + * - Without a category, prefer an existing other CLO; otherwise require one category. * * @return array{0: CategoryObject, 1: ?int} */ @@ -36,7 +38,10 @@ public function resolve(LitterObject $object, ?int $categoryId, ?int $typeId = n } elseif ($typeId !== null) { $choices->whereHas('types', fn ($query) => $query->where('litter_object_types.id', $typeId)); } - $clos = $choices->limit(2)->get(); + $other = $categoryId === null + ? (clone $choices)->whereHas('category', fn ($query) => $query->where('key', CategoryKey::Other->value))->first() + : null; + $clos = $other ? collect([$other]) : $choices->limit(2)->get(); if ($clos->count() !== 1) { throw ValidationException::withMessages(['tags' => $clos->isEmpty() ? 'Category does not contain the replacement object.' : 'Choose a category for this object.']); diff --git a/readme/API.md b/readme/API.md index 2ac5e582b..d974d4a58 100644 --- a/readme/API.md +++ b/readme/API.md @@ -512,11 +512,11 @@ Category is auto-resolved from `category_litter_object_id`. Generates summary, c - Old object IDs/keys and old `category_litter_object_id` values resolve to the current replacement before saving. A retired object need not have a CLO when submitted in object format. - The object's recorded replacement type overrides a stale submitted type. Across a chain, the latest non-null recorded type wins. Without a recorded type, the supplied type must be valid at the final destination. - An explicit object-format `category_id` or `category` is preserved or rejected; the API no longer silently substitutes another category. PUT now validates and retains `category_id`, as POST already does. -- For retired objects without a category, the destination plus effective type must identify a single category. Example: `energy_can → can --type=energy` can identify `softdrinks/can` when only that category permits energy. Ambiguous choices return 422. +- For retired objects without a category, first filter destination CLOs by the effective type when present. Prefer an existing `other` CLO among those choices; otherwise require a single category. Example: `energy_can → can --type=energy` can identify `softdrinks/can` when only that category permits energy. Ambiguous choices return 422. - Unknown objects/categories, unresolved combinations, broken replacement chains and incompatible destination types return 422. Rejected PUTs leave the existing photo, observations and extras intact. The usual response is `{ "message": "...", "errors": { "tags": ["..."] } }`; older unresolved-combination errors may contain structured details in `errors.tags`. - Quick-tag sync resolves old CLOs and validates the final type before replacing presets. Invalid syncs preserve the old presets. GET catalogue/search/top-tags excludes retired objects. -This release does not change the existing first-category inference for **active** object-only submissions. Send a category or CLO to avoid that ambiguity. It also does not make unrelated unresolved observations editable: a full-photo replacement containing one still fails atomically. +For **active** object-only submissions, prefer an existing `other` CLO; otherwise use the object's sole category. If several categories remain, return 422 asking the caller to choose a category. No CLO is created. Editors should always send the recorded category or CLO to preserve it. Unrelated unresolved observations remain uneditable: a full-photo replacement containing one still fails atomically. ### PUT /api/v3/tags — Replace All Tags on Photo diff --git a/readme/PostTagMigrationClean.md b/readme/PostTagMigrationClean.md index 1f4a601b0..2a6f1486e 100644 --- a/readme/PostTagMigrationClean.md +++ b/readme/PostTagMigrationClean.md @@ -25,6 +25,8 @@ Every affected photo gets a regenerated summary using the existing XP rules; `ph Old object submissions and old CLO submissions resolve through the recorded replacement before saving. Retired objects disappear from the catalogue, search and generated top tags. Saved quick tags move to the destination CLO; their other fields stay intact, except an explicitly recorded replacement type overrides their type. +Those discovery queries explicitly use `LitterObject::active()`, which delegates to `retired(false)`. `retired()` selects retired objects. There is no global scope: ordinary lookups must still find `plasticBags` for stale submissions, migration retries and replacement chains. See [object scopes](Tags.md#active-and-retired-objects). + ## What else changes when a key is renamed? The command updates stored data. It does **not** rename references in PHP, JavaScript, translations or the mobile app. Review these references for every approved mapping. @@ -78,7 +80,7 @@ Source observations with a null category or an existing type are refused. Catego The preview reports the per-item XP difference without blocking the migration. Every affected photo is rescored using current rules, including its other observations; XP is not frozen or deferred by this command. A zero per-item delta does not promise zero whole-photo XP drift. Changes to XP rules and any later bulk rescore remain separate work. -The latest non-null type recorded in an object replacement chain wins. Without a recorded type, a supplied type must be allowed at the final destination or the API returns 422 atomically. For a retired object submitted without a category, the destination and effective type must identify one category; otherwise the API returns 422. Explicit object-format categories are preserved or rejected, never silently substituted. See [API contract](API.md). +The latest non-null type recorded in an object replacement chain wins. Without a recorded type, a supplied type must be allowed at the final destination or the API returns 422 atomically. When no category is supplied, prefer an existing `other` CLO, otherwise require a sole category; retired-object choices are first filtered by the effective type. Ambiguous choices return 422. Explicit object-format categories are preserved or rejected, never silently substituted. See [API contract](API.md). Matching retries resume the remaining rows; conflicting object/type arguments fail. A completed A → B can be replayed after B → C. B → C is refused while an earlier migration into B still has source rows or quick tags. The command uses one MySQL advisory lock for the entire apply, with source and destination row locks in ID order during retirement. It checks lock ownership and Redis connectivity before each batch of 200 photos. This does not replace the write freeze. diff --git a/readme/Tags.md b/readme/Tags.md index b773cb97d..f7c5409aa 100644 --- a/readme/Tags.md +++ b/readme/Tags.md @@ -73,6 +73,20 @@ The category+object pairing is identified by a single `category_litter_object_id | `custom_tags_new` | `CustomTagNew` | User-generated custom tags | | `litter_states` | `LitterState` | Physical condition (degraded, micro, macro) — orthogonal, wired via `taggables` | +### Active and retired objects + +```php +LitterObject::active() // Same as retired(false): retired_at is null. +LitterObject::retired() // retired_at is not null. +LitterObject::retired(false) // Objects that have not been retired. +``` + +- Use `active()` explicitly for the tag catalogue, search and generated top tags. +- There is **no global scope** hiding retired objects. Ordinary lookups can find both active and retired records. +- Keep retired objects available to migration retries, replacement-chain resolution and historical observations. +- Example: looking up `plasticBags` must still find the retired record so a stale submission can resolve to `plastic_bag`. +- An ordinary lookup does not authorise saving a retired object. Tag writes must follow its recorded replacement before choosing the CLO. + ### Relationship pivots ```sql diff --git a/readme/changelog/2026-09-14.md b/readme/changelog/2026-09-14.md index 0cc0631bd..d297f0962 100644 --- a/readme/changelog/2026-09-14.md +++ b/readme/changelog/2026-09-14.md @@ -10,6 +10,7 @@ - Redirect retired object submissions and saved quick tags to the replacement. — v5.13.1 - Hide retired objects from tag suggestions. — v5.13.1 - Preserve explicit categories or reject the edit without deleting observations. — v5.13.1 +- Default category-less objects to an existing Other entry, then a sole category; reject ambiguity. — v5.13.1 - Preserve global and location metrics for photos without an owner. — v5.13.1 - Restrict database tests to local test databases and replace server-wide Redis flushing. — v5.13.1 diff --git a/tests/Feature/Tags/AddNewTagsToPhotosTest.php b/tests/Feature/Tags/AddNewTagsToPhotosTest.php index 55882a35b..db85761df 100644 --- a/tests/Feature/Tags/AddNewTagsToPhotosTest.php +++ b/tests/Feature/Tags/AddNewTagsToPhotosTest.php @@ -342,9 +342,9 @@ public function test_single_category_object_resolves_without_category_sent(): vo } /** - * Multi-category object with no category sent falls back to first(). + * - Bottle has multiple categories and no other CLO, so the caller must choose. */ - public function test_multi_category_object_falls_back_when_no_category_sent(): void + public function test_multi_category_object_requires_a_category_when_other_is_unavailable(): void { $this->seed(GenerateTagsSeeder::class); @@ -353,10 +353,10 @@ public function test_multi_category_object_falls_back_when_no_category_sent(): v $photo = $this->createPhotoFromImageAttributes($this->imageAndAttributes, $user); - // "bottle" belongs to "alcohol" and "soft_drinks" + // "bottle" belongs to "alcohol" and "softdrinks". $object = LitterObject::where('key', 'bottle')->first(); - $firstCategory = $object->categories()->first(); - $this->assertNotNull($firstCategory); + $this->assertGreaterThan(1, $object->categories()->count()); + $this->assertFalse($object->categories()->where('key', 'other')->exists()); $response = $this->postJson('/api/v3/tags', [ 'photo_id' => $photo->id, @@ -368,13 +368,8 @@ public function test_multi_category_object_falls_back_when_no_category_sent(): v ], ]); - $response->assertOk(); - - $this->assertDatabaseHas('photo_tags', [ - 'photo_id' => $photo->id, - 'category_id' => $firstCategory->id, - 'litter_object_id' => $object->id, - ]); + $response->assertUnprocessable()->assertJsonValidationErrors('tags'); + $this->assertDatabaseMissing('photo_tags', ['photo_id' => $photo->id]); } /** diff --git a/tests/Feature/Tags/MigrateTagTest.php b/tests/Feature/Tags/MigrateTagTest.php index e9836d710..e36dc1eb3 100644 --- a/tests/Feature/Tags/MigrateTagTest.php +++ b/tests/Feature/Tags/MigrateTagTest.php @@ -305,12 +305,47 @@ public function test_retired_objects_are_absent_from_catalogue_search_and_top_ta $this->observation($photo, ['category_litter_object_id' => $clo->id, 'quantity' => 3]); $this->old->update(['retired_at' => now(), 'merged_into_id' => $this->new->id]); $all = $this->getJson('/api/tags/all')->assertOk(); + $this->assertSame([$this->old->id], LitterObject::retired()->pluck('id')->all()); $this->assertNotContains($this->old->id, array_column($all->json('objects'), 'id')); $this->assertNotContains($clo->id, array_column($all->json('category_objects'), 'id')); $this->getJson('/api/tags?search=plasticBags')->assertOk()->assertJsonCount(0, 'tags'); $this->actingAs($user)->getJson('/api/v3/user/top-tags')->assertOk()->assertJsonCount(0, 'tags'); } + public function test_category_defaults_prefer_other_then_a_sole_category_for_active_and_retired_objects(): void + { + $this->category->update(['key' => 'coffee']); + $other = Category::factory()->create(['key' => 'other']); + $otherClo = CategoryObject::create(['category_id' => $other->id, 'litter_object_id' => $this->new->id]); + $this->old->update(['retired_at' => now(), 'merged_into_id' => $this->new->id]); + $user = User::factory()->create(); + $photo = Photo::factory()->create(['user_id' => $user->id]); + $this->actingAs($user); + + foreach ([$this->new->key, $this->old->key] as $key) { + $payload = ['photo_id' => $photo->id, 'tags' => [['object' => $key, 'quantity' => 2]]]; + $this->putJson('/api/v3/tags', $payload)->assertOk(); + $this->assertEquals($otherClo->id, $photo->photoTags()->sole()->category_litter_object_id); + $payload['tags'][0]['category_id'] = $this->category->id; + $this->putJson('/api/v3/tags', $payload)->assertOk(); + $this->assertEquals($this->destination->id, $photo->photoTags()->sole()->category_litter_object_id); + } + + $otherClo->delete(); + foreach ([$this->new->key, $this->old->key] as $key) { + $this->putJson('/api/v3/tags', ['photo_id' => $photo->id, 'tags' => [['object' => $key, 'quantity' => 2]]])->assertOk(); + $this->assertEquals($this->destination->id, $photo->photoTags()->sole()->category_litter_object_id); + } + + CategoryObject::create(['category_id' => Category::factory()->create(['key' => 'alcohol'])->id, 'litter_object_id' => $this->new->id]); + $before = $photo->photoTags()->sole()->getRawOriginal(); + foreach ([$this->new->key, $this->old->key] as $key) { + $this->putJson('/api/v3/tags', ['photo_id' => $photo->id, 'tags' => [['object' => $key, 'quantity' => 3]]]) + ->assertUnprocessable()->assertJsonValidationErrors('tags'); + $this->assertSame($before, $photo->photoTags()->sole()->getRawOriginal()); + } + } + public function test_chain_and_matching_replay_after_a_second_migration(): void { $tag = $this->observation(); @@ -398,6 +433,7 @@ public function test_recorded_chain_type_is_validated_at_the_final_destination() public function test_cycles_missing_targets_and_ambiguous_replacements_are_rejected(): void { + $this->category->update(['key' => 'coffee']); $user = User::factory()->create(); $photo = Photo::factory()->create(['user_id' => $user->id]); $this->old->update(['retired_at' => now(), 'merged_into_id' => $this->new->id]); From 43c0860f271c8e537fd688e79c6a46a157b1c162 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Se=C3=A1n=20L=2E?= Date: Sun, 11 Oct 2026 16:00:49 +0100 Subject: [PATCH 5/5] latest updates --- readme/PostTagMigrationClean.md | 13 +- readme/audit/RemainingTagMigrations.md | 157 +++++++++++++++++++++++++ tests/Feature/Tags/MigrateTagTest.php | 88 ++++++++++++++ 3 files changed, 253 insertions(+), 5 deletions(-) create mode 100644 readme/audit/RemainingTagMigrations.md diff --git a/readme/PostTagMigrationClean.md b/readme/PostTagMigrationClean.md index 2a6f1486e..a34267ff0 100644 --- a/readme/PostTagMigrationClean.md +++ b/readme/PostTagMigrationClean.md @@ -2,6 +2,8 @@ `olm:migrate-tag OLD NEW` updates one object at a time. This release keeps each observation's category. It does not require a source CLO or run `GenerateTagsSeeder` in production. +See [remaining tag migrations](audit/RemainingTagMigrations.md) for the current local backlog, proposed mappings, disputed destinations and catalogue repairs. The list is a review inventory, not an approved batch execution plan. + ## Plastic bags first ```bash @@ -51,25 +53,26 @@ Before deployment, build the frontend and check the displayed label and a fresh ## Recorded tag updates and seeding -Local database check on 2026-09-14: `olm_postmig_7` records exactly one retired object. This is local execution evidence, not a record of production deployment or completed reconciliation. +Local database check on 2026-10-11: `olm_postmig_7` records two retired objects. This is local execution evidence, not a record of production deployment or completed reconciliation. | Old key | New key | Replacement type | Local state | | --- | --- | --- | --- | | `plasticBags` | `plastic_bag` | None | Local retirement recorded on `2026-09-14`; zero old photo-tag records remain. | +| `energy_can` | `can` | `energy` | Local retirement recorded on `2026-10-11`; zero old photo-tag records remain. SQL checks passed; Redis reconciliation remains outstanding. | -The brand configuration and English translations have also been updated for this mapping. `energy_can → can --type=energy` is a proposed later run, not a completed migration. Add further completed mappings to this list only after checking their recorded replacements and remaining data. +The brand configuration and English translations have been updated for plastic bags. The energy-can run moved 20,498 observations representing 20,931 items locally; its subsequent Redis audit found inconsistent counters. See the [current backlog and completion notes](audit/RemainingTagMigrations.md#completed-locally). Add further completed mappings only after checking their recorded replacements and remaining data, and record any outstanding reconciliation separately. - **Fresh database:** `GenerateTagsSeeder` reads `TagsConfig`, which already declares `other/plastic_bag`. It creates the current object and CLO without needing to create or migrate `plasticBags`. - **Existing database:** run the reviewed `olm:migrate-tag OLD NEW --apply` command separately, with the write freeze and verification described here. The command records the old object's replacement and updates its existing observations. - **Normal seeding:** do not call data-migration commands automatically from `DatabaseSeeder` or `CreateAllTagsSeeder`. A seed would then change observations, summaries, XP and Redis, and a fresh database may not even contain the old object required by the command. -- **Future automation:** if several approved runs need a runner, make it a separate explicit command with structured old/new/type arguments and failure handling. Listing a mapping must not automatically execute it during seeding. No runner is needed for the single completed mapping above. +- **Future automation:** if several approved runs need a runner, make it a separate explicit command with structured old/new/type arguments and failure handling. Listing a mapping must not automatically execute it during seeding. The individual runs above do not require a runner. ## Supported mappings and limits Both objects must already exist. The destination must be active and have a CLO in each source category with observations or saved quick tags. Empty source CLOs do not require destinations. No source CLO is created. ```bash -# Separate, later mapping — review and rehearse independently +# Energy-can mapping — completed locally; review and rehearse separately for production php artisan olm:migrate-tag energy_can can --type=energy php artisan olm:migrate-tag energy_can can --type=energy --apply ``` @@ -132,7 +135,7 @@ The current PR already carries the single-object command, replacement-aware subm ### Later migrations do not each need a new PR -- After plastic bags passes verification, review and rehearse energy cans as a separate command run. Other supported mappings can follow independently. +- Plastic bags and energy cans have been applied locally; production runs still require separate review, rehearsal and reconciliation. Other supported mappings can follow independently from the [remaining migration list](audit/RemainingTagMigrations.md). - A mapping needs code changes only when its destination declarations, translations, brand configuration or required command behaviour are missing. Review those references as described above. - Confirm meaning before execution: for example, whether a legacy size distinction should be retained. Do not turn an unreviewed inventory into an approved execution list. diff --git a/readme/audit/RemainingTagMigrations.md b/readme/audit/RemainingTagMigrations.md new file mode 100644 index 000000000..9e07b85eb --- /dev/null +++ b/readme/audit/RemainingTagMigrations.md @@ -0,0 +1,157 @@ +# Remaining tag migrations + +Local snapshot: **11 October 2026, 15:26 Europe/Dublin**, database **`olm_postmig_7`**, branch commit **`4544e4ec`**. Production counts and completion are unverified. + +**65 object keys remain proposed for remapping across 68 category/object combinations, affecting 156,355 photo-tag records.** Another **4 keys** need catalogue links rather than object migration, affecting **2,614 records**. Together: **69 keys, 72 combinations, 158,969 records, 243,292 litter items and 144,624 distinct photos**. Photo counts overlap across mappings and must not be summed. + +This is a review list, not an approved batch execution plan. Proposed destinations and review decisions come from [the September audit](TagCleanupReview-2026-09-12.csv); record counts and catalogue checks were refreshed from the local database. All currently observed object/category pairs without a CLO are accounted for below. Extra-only observations are excluded. Valid pairs outside that audit are not a newly reviewed taxonomy. + +| Work | Object keys | Category/object combinations | Records | Litter items | +| --- | ---: | ---: | ---: | ---: | +| Mapping approval pending | 48 | 51 | 148,651 | 228,847 | +| Disputed meaning or destination | 17 | 17 | 7,704 | 8,716 | +| Keep key; repair catalogue | 4 | 4 | 2,614 | 5,729 | + +## Completed locally + +| Source | Replacement | Type | Remaining source records | Local retirement recorded | +| --- | --- | --- | ---: | --- | +| `plasticBags` | `plastic_bag` | — | 0 | 14 September 2026 | +| `energy_can` | `can` | `energy` | 0 | 11 October 2026 | + +Both source objects have recorded replacements and no remaining source observations. Energy-can SQL counts, collection totals, extras and aggregate photo XP matched the saved baseline; summary and processed-snapshot checks passed for 20,498 inferred affected photos. However, the subsequent Redis audit found 1,246 mismatched old-object, can-object and energy-type counters across global, location and user scopes. At that audit, the global old-object count was −20,931 instead of zero. Redis reconciliation remains outstanding; do not treat the SQL completion as a fully verified rollout. + +## Proposed mappings that preserve category + +All entries remain pending approval. “Catalogue present” means the destination object, category/object link (CLO), and requested type association exist locally; it is not release approval. The command retires the source object across **all** its categories. Shared sources such as `balloons`, `brokenglass` and `straws` must be reviewed together and migrated once. No remaining audited source observations have an existing type. + +### alcohol + +| Old object | New object | Type | Records | Items | Preparation or decision | +| --- | --- | --- | ---: | ---: | --- | +| `beer_bottle` | `bottle` | `beer` | 7,337 | 18,074 | Catalogue present; mapping approval pending | +| `beer_can` | `can` | `beer` | 19,275 | 26,488 | Catalogue present; mapping approval pending | +| `bottletops` | `bottle_cap` | — | 4,902 | 13,018 | Catalogue present; mapping approval pending | +| `brokenglass` | `broken_glass` | — | 3,370 | 8,015 | Catalogue present; mapping approval pending | +| `spirits_bottle` | `bottle` | `spirits` | 2,661 | 3,457 | Catalogue present; mapping approval pending | +| `wine_bottle` | `bottle` | `wine` | 638 | 863 | Catalogue present; mapping approval pending | + +### marine + +| Old object | New object | Type | Records | Items | Preparation or decision | +| --- | --- | --- | ---: | ---: | --- | +| `balloons` | `balloon` | — | 450 | 635 | Destination CLO missing | +| `buoys` | `buoy` | — | 145 | 189 | Catalogue present; mapping approval pending | +| `fishing_nets` | `fishing_net` | — | 894 | 1,154 | Catalogue present; mapping approval pending | +| `mediumplastics` | `medium_plastic` | — | 1,564 | 4,004 | Destination object missing; Destination CLO missing | +| `shotgun_cartridges` | `shotgun_cartridge` | — | 493 | 725 | Catalogue present; mapping approval pending | +| `straws` | `straw` | — | 175 | 226 | Destination CLO missing | + +### other + +| Old object | New object | Type | Records | Items | Preparation or decision | +| --- | --- | --- | ---: | ---: | --- | +| `bagsLitter` | `bags_litter` | — | 1,041 | 2,239 | Object XP changes by +20,151 total | +| `balloons` | `balloon` | — | 1,730 | 1,856 | Also needs marine/balloon CLO | +| `cableTie` | `cable_tie` | — | 1,021 | 1,203 | Catalogue present; mapping approval pending | +| `overflowingBins` | `overflowing_bin` | — | 517 | 524 | Catalogue present; mapping approval pending | +| `posters` | `poster` | — | 208 | 268 | Catalogue present; mapping approval pending | +| `randomLitter` | `random_litter` | — | 12,821 | 33,801 | Destination object missing; Destination CLO missing | +| `trafficCone` | `traffic_cone` | — | 116 | 136 | Catalogue present; mapping approval pending | + +### sanitary + +| Old object | New object | Type | Records | Items | Preparation or decision | +| --- | --- | --- | ---: | ---: | --- | +| `condoms` | `condom` | — | 190 | 366 | Catalogue present; mapping approval pending | +| `earSwabs` | `ear_swabs` | — | 172 | 294 | Catalogue present; mapping approval pending | +| `wetwipes` | `wipes` | — | 3,326 | 6,052 | Catalogue present; mapping approval pending | + +### smoking + +| Old object | New object | Type | Records | Items | Preparation or decision | +| --- | --- | --- | ---: | ---: | --- | +| `cigarette_box` | `box` | `cigarette` | 9,803 | 10,245 | Catalogue present; mapping approval pending | +| `filters` | `rolling_filter` | — | 603 | 852 | Catalogue present; mapping approval pending | +| `rollingPapers` | `papers` | — | 525 | 546 | Catalogue present; mapping approval pending | +| `tobaccopouch` | `pouch` | `tobacco` | 408 | 434 | Catalogue present; mapping approval pending | +| `vapeOil` | `vape` | `e_liquid_bottle` | 63 | 74 | Catalogue present; mapping approval pending | +| `vapePen` | `vape` | `pen` | 484 | 582 | Catalogue present; mapping approval pending | + +### softdrinks + +| Old object | New object | Type | Records | Items | Preparation or decision | +| --- | --- | --- | ---: | ---: | --- | +| `brokenglass` | `broken_glass` | — | 42 | 145 | Catalogue present; mapping approval pending | +| `fizzy_bottle` | `bottle` | `soda` | 6,448 | 7,831 | Catalogue present; mapping approval pending | +| `iceTea_bottle` | `bottle` | `tea` | 829 | 851 | Catalogue present; mapping approval pending | +| `icedTea_can` | `can` | `iced_tea` | 588 | 607 | Catalogue present; mapping approval pending | +| `juice_bottle` | `bottle` | `juice` | 1,862 | 1,940 | Catalogue present; mapping approval pending | +| `juice_carton` | `carton` | `juice` | 2,243 | 2,327 | Catalogue present; mapping approval pending | +| `milk_bottle` | `bottle` | `milk` | 901 | 935 | Catalogue present; mapping approval pending | +| `milk_carton` | `carton` | `milk` | 827 | 874 | Catalogue present; mapping approval pending | +| `pullRing` | `pull_ring` | — | 758 | 1,168 | Catalogue present; mapping approval pending | +| `soda_can` | `can` | `soda` | 16,462 | 18,034 | Catalogue present; mapping approval pending | +| `sports_bottle` | `bottle` | `sports` | 2,931 | 3,277 | Catalogue present; mapping approval pending | +| `straw_packaging` | `straw_wrapper` | — | 1,039 | 1,111 | Catalogue present; mapping approval pending | +| `straws` | `straw` | — | 7,368 | 8,438 | Also needs marine/straw CLO | +| `water_bottle` | `bottle` | `water` | 17,189 | 21,889 | Catalogue present; mapping approval pending | + +`bagsLitter → bags_litter` changes the object score from 1 to 10 XP per item, implying +20,151 object XP for these 2,239 items. This is not a prediction of the whole-photo XP delta; all affected photos are rescored. The command reports XP differences but does not enforce approval. + +## Proposed category moves + +These 9 mappings require category-move support or a separately reviewed procedure. The current `olm:migrate-tag` command preserves category and cannot implement them. In particular, never retire a shared object globally just to move one category’s observations. + +| Source category/object | Proposed destination category/object | Records | Items | +| --- | --- | ---: | ---: | +| `other/batteries` | `electronics/battery` | 291 | 363 | +| `other/dogshit` | `pets/dogshit` | 1,146 | 1,383 | +| `other/dogshit_in_bag` | `pets/dogshit_in_bag` | 506 | 649 | +| `other/dump` | `dumping/dumping` | 1,875 | 9,733 | +| `other/life_buoy` | `marine/buoy` | 6 | 11 | +| `other/tyre` | `vehicles/tyre` | 202 | 436 | +| `sanitary/facemask` | `medical/face_mask` | 7,742 | 7,910 | +| `sanitary/gloves` | `medical/gloves` | 2,422 | 2,562 | +| `sanitary/sanitiser` | `medical/sanitiser` | 42 | 53 | + +## Disputed mappings + +These 17 suggestions are **not approved for execution**. Preserve the existing meaning until a destination is agreed; generic `other` replacements can discard useful information. + +| Source category/object | Disputed destination | Records | Items | Reason | +| --- | --- | ---: | ---: | --- | +| `art/item` | `art/other` | 42 | 43 | Would replace a specific object with generic other | +| `food/crisp_large` | `food/crisp_packet` | 431 | 469 | May lose size/material detail or change object meaning | +| `food/crisp_small` | `food/crisp_packet` | 4,654 | 5,037 | May lose size/material detail or change object meaning | +| `food/glass_jar` | `food/jar` | 131 | 188 | May lose size/material detail or change object meaning | +| `industrial/chemical` | `industrial/chemical_container` | 27 | 36 | May lose size/material detail or change object meaning | +| `industrial/oil` | `industrial/oil_container` | 57 | 69 | May lose size/material detail or change object meaning | +| `marine/lego` | `other/other` | 3 | 3 | Would replace a specific object with generic other | +| `other/automobile` | `vehicles/car_part` | 794 | 987 | Category and object interpretation need agreement | +| `other/books` | `other/other` | 38 | 71 | Would replace a specific object with generic other | +| `other/ear_plugs` | `sanitary/other` | 26 | 46 | Would replace a specific object with generic other | +| `other/elec_large` | `electronics/other` | 60 | 100 | Would replace a specific object with generic other | +| `other/elec_small` | `electronics/other` | 291 | 385 | Would replace a specific object with generic other | +| `other/hair_tie` | `sanitary/other` | 515 | 546 | Would replace a specific object with generic other | +| `other/magazine` | `other/other` | 93 | 124 | Would replace a specific object with generic other | +| `other/washingUp` | `other/other` | 79 | 102 | Would replace a specific object with generic other | +| `sanitary/menstrual` | `sanitary/sanitary_pad` | 179 | 201 | May lose size/material detail or change object meaning | +| `sanitary/toothpick` | `sanitary/other` | 284 | 309 | Would replace a specific object with generic other | + +## Catalogue repairs without renaming + +These 4 category/object pairs should retain their keys. Their CLO entries are absent locally. Prepare reviewed, targeted catalogue repairs; `olm:migrate-tag OLD OLD` cannot repair them. Do not run a broad production seeder as a shortcut. + +| Category/object | Records | Items | +| --- | ---: | ---: | +| `industrial/plastic` | 666 | 2,937 | +| `marine/bag` | 1,161 | 1,662 | +| `marine/bottle` | 741 | 1,070 | +| `marine/lighters` | 46 | 60 | + +## Refresh and execution + +Refresh this list after each verified mapping. Recount `photo_tags` by category/object, compare those pairs with `category_litter_object`, and inspect recorded retirements plus saved quick tags. Keep the September audit as historical evidence; this list records the current local backlog. + +Before another apply, reconcile Redis, approve the mapping, review application and translation references, and follow the [migration runbook](../PostTagMigrationClean.md#rehearsal-and-production-window). Preview with `php artisan olm:migrate-tag OLD NEW` and add `--type=TYPE` where listed. Add `--apply` only for the separately reviewed run with writes paused. diff --git a/tests/Feature/Tags/MigrateTagTest.php b/tests/Feature/Tags/MigrateTagTest.php index e36dc1eb3..e76c594db 100644 --- a/tests/Feature/Tags/MigrateTagTest.php +++ b/tests/Feature/Tags/MigrateTagTest.php @@ -4,6 +4,7 @@ use App\Console\Commands\Tags\MigrateTag; use App\Exports\CreateCSVExport; +use App\Mail\ExportWithLink; use App\Models\Litter\Tags\BrandList; use App\Models\Litter\Tags\Category; use App\Models\Litter\Tags\CategoryObject; @@ -13,13 +14,16 @@ use App\Models\Litter\Tags\Materials; use App\Models\Litter\Tags\PhotoTag; use App\Models\Photo; +use App\Models\Teams\Team; use App\Models\Users\User; use App\Models\Users\UserQuickTag; use App\Services\Metrics\MetricsService; use App\Services\Redis\RedisKeys; use App\Services\Tags\GeneratePhotoSummaryService; use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Mail; use Illuminate\Support\Facades\Redis; +use Illuminate\Support\Facades\Storage; use Tests\TestCase; class MigrateTagTest extends TestCase @@ -524,6 +528,90 @@ public function test_actual_wide_and_long_csv_use_replacement_keys(): void } } + /** @dataProvider migratedDownloadFormats */ + public function test_download_links_contain_migrated_quantities_for_the_requested_user_or_team(string $scope, string $layout, string $format): void + { + Mail::fake(); + Storage::fake('s3'); + $user = User::factory()->create(); + $teammate = User::factory()->create(); + $team = Team::factory()->create(['leader' => $user->id]); + $user->teams()->attach($team); + $teammate->teams()->attach($team); + + $photo = Photo::factory()->create(['user_id' => $user->id, 'team_id' => $team->id, 'verified' => 2]); + $oldTag = $this->observation($photo); + $oldTag->attachExtraTags([['id' => Materials::factory()->create()->id]], 'material'); + $this->observation($photo, [ + 'litter_object_id' => $this->new->id, + 'category_litter_object_id' => $this->destination->id, + 'quantity' => 5, + ]); + + // User export includes their off-team photo; team export includes another member. + foreach ([ + [$user->id, null, 2, 11], + [$teammate->id, $team->id, 2, 3], + [$teammate->id, null, 2, 13], + [$teammate->id, $team->id, 0, 17], + ] as [$owner, $teamId, $verified, $quantity]) { + $other = Photo::factory()->create(['user_id' => $owner, 'team_id' => $teamId, 'verified' => $verified]); + $this->observation($other, ['quantity' => $quantity]); + } + $this->migrate(); + + $params = ['layout' => $layout, 'format' => $format]; + $this->actingAs($user); + $response = $scope === 'user' + ? $this->getJson('/api/user/profile/download?'.http_build_query($params)) + : $this->postJson('/api/teams/download', $params + ['team_id' => $team->id]); + $response->assertOk()->assertJson(['success' => true]); + + Mail::assertSent(ExportWithLink::class, 1); + Mail::assertSent(ExportWithLink::class, function (ExportWithLink $mail) use ($user, $scope, $layout) { + $this->assertTrue($mail->hasTo($user->email)); + Storage::disk('s3')->assertExists($mail->path); + $csv = Storage::disk('s3')->get($mail->path); + $this->assertStringContainsString('plastic_bag', $csv); + $this->assertStringNotContainsString('plasticBags', $csv); + + $stream = fopen('php://memory', 'r+'); + fwrite($stream, $csv); + rewind($stream); + $headers = fgetcsv($stream); + $quantity = 0; + $photoIds = []; + while (($cells = fgetcsv($stream)) !== false) { + $row = array_combine($headers, $cells); + $photoIds[] = $row[$layout === 'long' ? 'photo_id' : 'id']; + if ($layout === 'wide') { + $quantity += (int) $row['plastic_bag']; + } elseif ($row['object'] === 'plastic_bag' + && $row['material'] === '' && $row['brand'] === '' && $row['custom_tag'] === '') { + // Full-detail CSV repeats the parent object on extra-dimension rows. + $quantity += (int) $row['quantity']; + } + } + fclose($stream); + $this->assertCount(2, array_unique($photoIds)); + $this->assertSame($scope === 'user' ? 18 : 10, $quantity); + + return true; + }); + } + + public static function migratedDownloadFormats(): array + { + return [ + 'user split' => ['user', 'wide', 'split'], + 'user joined' => ['user', 'wide', 'joined'], + 'user full detail' => ['user', 'long', 'split'], + 'team split' => ['team', 'wide', 'split'], + 'team joined' => ['team', 'wide', 'joined'], + 'team full detail' => ['team', 'long', 'split'], + ]; + } + public function test_another_connection_holding_the_lock_blocks_apply(): void { config(['database.connections.migration_lock_test' => config('database.connections.mysql')]);