diff --git a/database/migrations/2026_09_04_000001_repair_inconsistent_merged_tickets.php b/database/migrations/2026_09_04_000001_repair_inconsistent_merged_tickets.php new file mode 100644 index 0000000..7becc6e --- /dev/null +++ b/database/migrations/2026_09_04_000001_repair_inconsistent_merged_tickets.php @@ -0,0 +1,73 @@ +line(sprintf( + 'SELECT COUNT(*) FROM tickets WHERE merged_into_id IS NOT NULL AND status <> \'merged\' => %d', + $rows->count(), + )); + + if ($rows->isEmpty()) { + $this->info('Nothing is inconsistent. No repair to make.'); + + return 0; + } + + $this->table( + ['id', 'number', 'status', 'merged_into_id', 'updated_at'], + $rows->map(fn ($t) => [ + $t->id, $t->number, $t->status, $t->merged_into_id, (string) $t->updated_at, + ])->all(), + ); + + if (! $this->option('force')) { + $this->comment('Dry run — nothing was written. Re-run with --force to re-mark these as merged.'); + + return 0; + } + + $changed = MergedTicketRepair::repairAll('artisan ticketing:repair-merged'); + + $this->info("Re-marked {$changed} ticket(s) as merged. Their messages and participants are untouched."); + + return 0; + } +} diff --git a/src/Http/Controllers/TicketController.php b/src/Http/Controllers/TicketController.php index 318d7a3..f5d50bc 100644 --- a/src/Http/Controllers/TicketController.php +++ b/src/Http/Controllers/TicketController.php @@ -13,6 +13,7 @@ use Dashboard\Ticketing\Models\TicketMessage; use Dashboard\Ticketing\Models\TicketParticipant; use Dashboard\Ticketing\Models\TicketStatus; use Dashboard\Ticketing\Models\TicketView; +use Dashboard\Ticketing\Support\MergedTicketRepair; use Illuminate\Http\Request; use Illuminate\Routing\Controller; use Illuminate\Support\Facades\Auth; @@ -147,6 +148,11 @@ class TicketController extends Controller 'unassigned' => (clone $baseQuery)->whereNull('assigned_to')->whereNotIn('status', $closedSlugs)->count(), 'pending' => (clone $baseQuery)->where('status', 'pending')->count(), 'resolved' => (clone $baseQuery)->whereIn('status', $closedSlugs)->count(), + // #938: the pointer, not the status — this view exists to show the + // rows where the two disagree, so filtering by `status = merged` + // would hide exactly the ones worth looking at. + 'merged' => (clone $baseQuery)->whereNotNull('merged_into_id')->count(), + 'merged_inconsistent' => (clone $baseQuery)->inconsistentlyMerged()->count(), ] : [ 'all' => (clone $baseQuery)->count(), @@ -154,6 +160,8 @@ class TicketController extends Controller 'unassigned' => 0, 'pending' => (clone $baseQuery)->where('status', 'pending')->count(), 'resolved' => (clone $baseQuery)->whereIn('status', $closedSlugs)->count(), + 'merged' => 0, + 'merged_inconsistent' => 0, ]; $query = Ticket::with(['group', 'priority', 'project']); @@ -181,6 +189,12 @@ class TicketController extends Controller $query->where('status', 'pending'); } elseif ($request->filter === 'resolved') { $query->whereIn('status', $closedSlugs); + } elseif ($request->filter === 'merged') { + // Every ticket that has been folded into another one, whatever its + // status says — which is what makes this the place the office + // finds the rows #938 is about. `merged` is a closed slug, so the + // whereNotIn below would drop the consistent half of the list. + $query->whereNotNull('merged_into_id'); } elseif (!$request->filled('status')) { $query->whereNotIn('status', $closedSlugs); } @@ -616,7 +630,7 @@ class TicketController extends Controller { $validated = $request->validate([ 'view_mode' => 'sometimes|in:list,kanban', - 'filter' => 'sometimes|in:all,mine,unassigned,pending,resolved', + 'filter' => 'sometimes|in:all,mine,unassigned,pending,resolved,merged', ]); if (empty($validated)) { @@ -700,6 +714,42 @@ class TicketController extends Controller ->with('success', "Ticket {$source->number} merged in."); } + /** + * MARK ONE INCONSISTENT ROW AS MERGED — option B of [central-ticketing] #938. + * + * The rows the merge bug minted carry `merged_into_id` while their status + * says `open`. #920 left them editable ON PURPOSE, so that they could be + * corrected in the product; this is the correction, and until it existed the + * only way to make one was to type the right status into the Edit form and + * know that `merged` was not in the list. + * + * Note what it is NOT: it is not `update()` with a status of `merged`, which + * `refuseIfMerged()` would have to be weakened to allow, and it does not + * touch messages, participants or any field somebody edited after the merge. + * It writes one column, on a row that is provably in the broken shape, and + * it is a no-op on anything else — so a double-click, a stale page and a row + * a colleague fixed thirty seconds ago all land somewhere harmless. + */ + public function markMerged(Ticket $ticket) + { + if (! $this->isAgent($ticket->group_id)) { + abort(403); + } + + if (! $ticket->isInconsistentlyMerged()) { + // Deliberately not an error: the button is only rendered on a row in + // the broken shape, so arriving here means somebody else has already + // fixed it, and being told so is the right answer. + return redirect()->route('ticketing.show', $ticket) + ->with('success', 'That ticket is already consistent — nothing to fix.'); + } + + MergedTicketRepair::repair($ticket, 'user '.Auth::id()); + + return redirect()->route('ticketing.show', $ticket) + ->with('success', "Ticket {$ticket->number} is marked merged and is read-only again."); + } + /** * Split this ticket — create a copy with a new number, same metadata, duplicated messages. */ diff --git a/src/Http/Controllers/TicketingSettingsController.php b/src/Http/Controllers/TicketingSettingsController.php index 5786a4f..cd5720f 100644 --- a/src/Http/Controllers/TicketingSettingsController.php +++ b/src/Http/Controllers/TicketingSettingsController.php @@ -9,6 +9,7 @@ use Dashboard\Ticketing\Models\TicketingAgentAccess; use Dashboard\Ticketing\Models\TicketingGroup; use Dashboard\Ticketing\Models\TicketingProject; use Dashboard\Ticketing\Models\TicketStatus; +use Dashboard\Ticketing\Support\MergedTicketRepair; use Illuminate\Http\Request; use Illuminate\Routing\Controller; use Illuminate\Support\Facades\Auth; @@ -121,6 +122,11 @@ class TicketingSettingsController extends Controller 'isBootstrap' => $isBootstrap, 'isSiteAdmin' => $this->isSiteAdmin(), 'autoCloseDays' => (int) Setting::get('ticketing.auto_close_days', 0), + // #938 — the deploy-time repair, and how many rows it would touch. The + // count is what turns an abstract toggle into a decision somebody can + // actually make, so it is shown beside it whether the toggle is on or off. + 'autoRepairMerged' => MergedTicketRepair::enabledOnDeploy(), + 'inconsistentMergedCount' => MergedTicketRepair::count(), ]); } @@ -575,10 +581,20 @@ class TicketingSettingsController extends Controller $validated = $request->validate([ 'auto_close_days' => 'required|integer|min:0|max:365', + 'auto_repair_merged_on_deploy' => 'sometimes|boolean', ]); Setting::set('ticketing.auto_close_days', $validated['auto_close_days']); + // #938 option A is a behaviour rule, so it is a setting, and it defaults + // off: a deploy must never rewrite rows nobody asked it to rewrite. + if (array_key_exists('auto_repair_merged_on_deploy', $validated)) { + Setting::set( + MergedTicketRepair::SETTING, + $validated['auto_repair_merged_on_deploy'] ? '1' : '0', + ); + } + return back()->with('success', 'Automation settings saved.'); } } diff --git a/src/Models/Ticket.php b/src/Models/Ticket.php index 872a25b..435c10f 100644 --- a/src/Models/Ticket.php +++ b/src/Models/Ticket.php @@ -2,6 +2,7 @@ namespace Dashboard\Ticketing\Models; +use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\Relations\HasMany; @@ -37,6 +38,19 @@ class Ticket extends Model // value) still stores the right day — measured in UTC and in // America/New_York. So nothing was ever lost in the database; the value // simply had no way of reaching the screen. + /** + * `merged_inconsistent` rides on every serialized ticket — #938. + * + * The badge and its "Mark merged" button are rendered per ROW, on the index + * list, in the detail pane and on the Show header, so the page needs the + * answer for each ticket it draws. Handing it the model's own predicate + * rather than letting three Vue files each write + * `t.merged_into_id && t.status !== 'merged'` is what keeps the client's idea + * of "inconsistent" from drifting away from the scope the repair uses. + * It costs no query — both columns are already loaded. + */ + protected $appends = ['merged_inconsistent']; + protected $casts = [ 'due_date' => 'date:Y-m-d', ]; @@ -96,4 +110,38 @@ class Ticket extends Model { return $this->status === self::STATUS_MERGED; } + + /** + * THE SHAPE THE MERGE BUG MINTED — [central-ticketing] #938. + * + * `merged_into_id` points at a target while `status` is something other than + * `merged`: the row was merged, and then a save through the unguarded Edit + * form (#920) put the status back. #920 stopped new ones being made and + * deliberately left the existing ones EDITABLE — `isMerged()` reads the + * status, not the pointer — so that the office can correct one rather than + * needing SQL. This is the predicate that finds them. + * + * It is one definition and three callers read it: the badge that offers the + * fix, the `mark-merged` verb that applies it one row at a time, and the + * repair migration that applies it to all of them at once. Two spellings of + * "inconsistent" that drifted apart would mean a badge on a row the repair + * skips. + */ + public function scopeInconsistentlyMerged(Builder $query): Builder + { + return $query->whereNotNull('merged_into_id') + ->where('status', '!=', self::STATUS_MERGED); + } + + /** The row-level twin of {@see scopeInconsistentlyMerged()}. */ + public function isInconsistentlyMerged(): bool + { + return $this->merged_into_id !== null && ! $this->isMerged(); + } + + /** @see $appends — the same answer, for the page. */ + public function getMergedInconsistentAttribute(): bool + { + return $this->isInconsistentlyMerged(); + } } diff --git a/src/Support/MergedTicketRepair.php b/src/Support/MergedTicketRepair.php new file mode 100644 index 0000000..8621f33 --- /dev/null +++ b/src/Support/MergedTicketRepair.php @@ -0,0 +1,133 @@ +inconsistentlyMerged()->count(); + } + + /** + * The rows themselves, for a human to look at before deciding. + * + * @return \Illuminate\Database\Eloquent\Collection + */ + public static function rows() + { + return Ticket::query()->inconsistentlyMerged()->orderBy('id')->get(); + } + + /** Has the operator asked for the deploy-time repair? */ + public static function enabledOnDeploy(): bool + { + return filter_var(Setting::get(self::SETTING, '0'), FILTER_VALIDATE_BOOLEAN); + } + + /** + * Re-mark every inconsistent row as merged. Returns how many rows changed. + * + * IDEMPOTENT, and it is the scope that makes it so rather than a flag: the + * second run matches nothing, because the first run's rows no longer satisfy + * `status <> 'merged'`. So a migration that is re-run, a command run twice + * and a deploy that repeats all answer 0 and write nothing. + */ + public static function repairAll(string $by): int + { + $numbers = Ticket::query()->inconsistentlyMerged()->pluck('number')->all(); + + if ($numbers === []) { + Log::info("[ticketing] #938 merged-ticket repair ({$by}): nothing to repair."); + + return 0; + } + + $changed = Ticket::query()->inconsistentlyMerged()->update([ + 'status' => Ticket::STATUS_MERGED, + ]); + + // The numbers, not just the count. These rows are being made read-only, + // and "which ones" is the question the office will ask afterwards. + Log::info(sprintf( + '[ticketing] #938 merged-ticket repair (%s): re-marked %d ticket(s) as merged — %s', + $by, + $changed, + implode(', ', $numbers), + )); + + return $changed; + } + + /** + * Re-mark ONE row. Returns false when it was not inconsistent to begin with, + * so a double-clicked button and a stale page are both no-ops rather than a + * second write. + */ + public static function repair(Ticket $ticket, string $by): bool + { + if (! $ticket->isInconsistentlyMerged()) { + return false; + } + + $ticket->forceFill(['status' => Ticket::STATUS_MERGED])->save(); + + Log::info(sprintf( + '[ticketing] #938 merged-ticket repair (%s): re-marked %s as merged (merged into #%d).', + $by, + $ticket->number, + $ticket->merged_into_id, + )); + + return true; + } +} diff --git a/src/TicketingServiceProvider.php b/src/TicketingServiceProvider.php index 3167a72..c50c6e6 100644 --- a/src/TicketingServiceProvider.php +++ b/src/TicketingServiceProvider.php @@ -26,6 +26,7 @@ class TicketingServiceProvider extends ServiceProvider if ($this->app->runningInConsole()) { $this->commands([ \Dashboard\Ticketing\Console\Commands\AutoCloseTickets::class, + \Dashboard\Ticketing\Console\Commands\RepairMergedTickets::class, ]); } } diff --git a/src/routes/ticketing.php b/src/routes/ticketing.php index 6017c1f..312a53e 100644 --- a/src/routes/ticketing.php +++ b/src/routes/ticketing.php @@ -76,6 +76,8 @@ Route::middleware(['web', 'auth', 'app.access:ticketing']) Route::post('/{ticket}/view', [TicketController::class, 'recordView']) ->name('tickets.view'); Route::post('/{ticket}/merge', [TicketController::class, 'merge']) ->name('merge'); Route::post('/{ticket}/split', [TicketController::class, 'split']) ->name('split'); + // #938 — the one-click fix for a row the merge bug left inconsistent. + Route::post('/{ticket}/mark-merged', [TicketController::class, 'markMerged']) ->name('mark-merged'); Route::post('/{ticket}/participants', [TicketController::class, 'addParticipant']) ->name('participants.store'); Route::delete('/{ticket}/participants/{userId}', [TicketController::class, 'removeParticipant'])->name('participants.destroy'); Route::get('/{ticket}/users/search', [TicketController::class, 'searchUsers']) ->name('users.search');