jwed/central-ticketing
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

A row the merge bug minted is visible, fixable, and repairable in bulk (#938)

Browse Source 
#920 made a merged ticket read-only on the server and shipped NO data change,
deliberately: its guard reads `status === 'merged'` and not
`merged_into_id !== null`, so the rows the bug had already produced — the
pointer set, the status back to `open` — stayed EDITABLE and could be corrected
in the product instead of in SQL. What it left behind was rows that are
simultaneously live in the All Open queue and merged into another ticket, with
nothing on any screen saying so and nothing offering to fix them.

Both halves of the card ship, because option A is a BEHAVIOUR RULE and a
behaviour rule is a setting:

  B — the office fixes them one at a time. `merged_inconsistent` rides on the
      model (Ticket::$appends), so the list row, the index detail pane and the
      Show header all draw the same amber badge — "merged_into set but status
      open — fix?" — off one predicate rather than three copies of an
      expression. The badge IS the button: it opens a modal that says what will
      change and what will not, and posts to a new `mark-merged` verb that
      writes `status` and nothing else. A new "Merged" view lists every ticket
      folded into another one BY THE POINTER, so the consistent and the broken
      appear side by side; a status filter would show one or the other and
      never the pair worth comparing.

  A — the migration re-marks all of them at once, and it is INERT unless
      `ticketing.auto_repair_merged_on_deploy` is on. It defaults OFF, which is
      also what a box that has never heard of the setting does, so this can
      ship ahead of the operator's decision and change nothing unasked. The
      toggle is on the Automation tab with the live count beside it, because
      nobody should arm a bulk data change without seeing how many rows it
      touches.

`MergedTicketRepair` is one implementation with three callers for one reason:
"which rows are broken" and "what fixing one means" must not drift, or the
badge offers a fix on a row the migration skips.

THE PROD COUNT THE CARD ASKS FOR IS NOT IN HERE, and it could not be: this
worker box holds no ssh private key, so central-prod is unreachable from it and
there is no prod container locally. `php artisan ticketing:repair-merged
--dry-run` is the read-only answer for whoever does have access — it prints the
card's own SELECT COUNT and lists the rows, and `--force` (not the bare form)
is the only spelling that writes.

Repairing NEVER reverts. Messages, participants, assignee and title edits made
to a row while it was wrongly editable are somebody's work and the only copy of
it; there is no record of what the fields held before the bad save, so
"reverting" would invent a previous state rather than restore one.

Proved by tests/Feature/Ticketing/MergedTicketInconsistentRowsTest.php (22
tests) in dashboard-shell, over the fixture the card names — one inconsistent
row beside a genuinely merged one, both produced by the real merge endpoint.
The NEGCTRL half is what keeps it honest: the consistent row and an ordinary
open ticket must come out of every path untouched, and a repaired row must keep
the reply and the rename it was given after the merge.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
central-worker-4
2026-09-04 02:53:23 -07:00
parent 5315afc239
commit 420ef78603
8 changed files with 391 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

133
src/Support/MergedTicketRepair.php Normal file
View File

@@ -0,0 +1,133 @@
<?php
namespace Dashboard\Ticketing\Support;
use App\Models\Setting;
use Dashboard\Ticketing\Models\Ticket;
use Illuminate\Support\Facades\Log;
/**
* REPAIRING THE ROWS THE MERGE BUG MINTED — [central-ticketing] #938.
*
* #920 stopped the bug making new ones: a ticket whose `status` is `merged` is
* read-only server-side, so the Edit form can no longer write `status = open`
* over a row that has already been folded into another ticket. It shipped NO
* data change, on purpose, and its guard reads the status rather than the
* pointer precisely so the rows already carrying the corrupt shape
* (`status = open, merged_into_id = 56`) stay editable and can be corrected
* through the UI instead of by hand in SQL.
*
* This class is the correction, and it is deliberately ONE implementation with
* three callers, because "which rows are broken" and "what fixing one means"
* are the two things that must not drift:
*
* - {@see \Dashboard\Ticketing\Http\Controllers\TicketController::markMerged()}
* — the office fixes one row, from the badge on the ticket header.
* - {@see \Dashboard\Ticketing\Console\Commands\RepairMergedTickets}
* — `--dry-run` is the COUNT, and it is the whole reason the class is not
* just a line inside the migration: the operator has to be able to see how
* many rows there are, and which, before anything writes.
* - the `2026_09_04_000001_repair_inconsistent_merged_tickets` migration
* — every row at once, on deploy, and ONLY when the operator has turned
* {@see SETTING} on.
*
* ── WHAT REPAIRING DOES *NOT* DO ────────────────────────────────────────────
*
* It writes `status = 'merged'` and nothing else. Messages, participants,
* assignee, priority and title are left exactly as they are, including edits
* made to the row AFTER the merge — those are somebody's work, they are the
* only copy, and there is no record of what the fields held before the bad
* save to revert them to. The ticket becoming read-only is the whole change.
*/
class MergedTicketRepair
{
/**
* "Auto-repair inconsistent merged tickets on deploy", and it DEFAULTS OFF.
*
* A deploy that silently rewrites rows nobody asked it to rewrite is the one
* outcome #938 rules out by name, so the migration reads this and no-ops
* when it is unset. Off is therefore the whole behaviour of a box that has
* never heard of this setting, which is what makes shipping the migration
* safe before the operator has decided anything.
*/
public const SETTING = 'ticketing.auto_repair_merged_on_deploy';
/** How many rows are inconsistent right now. The dry run, and it writes nothing. */
public static function count(): int
{
return Ticket::query()->inconsistentlyMerged()->count();
}
/**
* The rows themselves, for a human to look at before deciding.
*
* @return \Illuminate\Database\Eloquent\Collection<int,Ticket>
*/
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;
}
}