`tickets.merged_into_id` is `nullOnDelete()` and `destroy()` was a one-liner, so deleting the surviving ticket of a merge blanked the pointer on every ticket merged into it and left `status = 'merged'` behind. The row that came out was read-only for good (#920 guards on the status), carried a banner naming a ticket that no longer existed, reported `merged_inconsistent = false` — the flag that exists to catch exactly this — and dropped out of the Merged view, which was scoped by the pointer, leaving it visible only under Resolved. `StrandedMerge::handleTargetDeletion()` runs inside destroy()'s transaction, before the row goes: sources are RE-POINTED at the doomed ticket's own merge target when it has one (the messages really did travel that far up the chain), otherwise UN-MERGED to `closed` — out of the work queues, as they were, but editable, reopenable and answerable again — with a system note naming what was deleted and why. #938's dangling pointers are simply cleared: those rows are somebody's open work. The flash names every ticket that changed, and the delete confirm names them before the click instead of the generic "cannot be undone". The shape prod already holds gets a name: `scopeStrandedMerge()`, reported as `merged_inconsistent` (now the union of both broken shapes) plus `merged_stranded` so a page can tell which fix to offer, listed by the Merged view via `scopeEverMerged()`, and released one at a time by `unmerge`. #938's repair scope is deliberately untouched — it writes `status = 'merged'`, which a stranded row already has, and matching it would break its idempotency. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
252 lines
10 KiB
PHP
252 lines
10 KiB
PHP
<?php
|
|
|
|
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;
|
|
|
|
class Ticket extends Model
|
|
{
|
|
/** Set only by TicketController::merge(); never offered by any form. */
|
|
public const STATUS_MERGED = 'merged';
|
|
|
|
/**
|
|
* WHERE AN UN-MERGED TICKET LANDS — [central-ticketing] #1567.
|
|
*
|
|
* A merge is a way of closing a ticket, so a ticket that is un-merged because
|
|
* its target was deleted goes to `closed` and not to `open`: it stays out of
|
|
* the All Open / Mine / Unassigned work queues, exactly as it was before, and
|
|
* the only thing that changes is that it is no longer read-only, so an agent
|
|
* can reopen it, reply to it or reassign it. Un-merging to `open` would push
|
|
* somebody's tidied-away ticket back into a live queue without being asked.
|
|
*
|
|
* It is one of the six values the `tickets.status` ENUM allows and it is
|
|
* `is_closed` in `ticketing_statuses`, so both the column and every count
|
|
* that reads `$closedSlugs` already understand it.
|
|
*/
|
|
public const STATUS_UNMERGED = 'closed';
|
|
|
|
/**
|
|
* What every surface says when a write to a merged ticket is refused. The Show
|
|
* page has displayed this sentence since merge shipped; #920 was that it was
|
|
* only ever a sentence, with no server-side guard behind it.
|
|
*/
|
|
public const READ_ONLY_REASON = 'This ticket has been merged and is read-only.';
|
|
|
|
protected $fillable = [
|
|
'number', 'group_id', 'submitter_id', 'assigned_to', 'project_id',
|
|
'title', 'description', 'status', 'priority_id', 'due_date', 'merged_into_id',
|
|
];
|
|
|
|
// `date:Y-m-d`, not bare `date`. The column is a DATE and every UI that shows
|
|
// it is an <input type="date">, which accepts exactly `Y-m-d` and silently
|
|
// renders anything else as an EMPTY field. A bare `date` cast serializes to
|
|
// `2026-09-15T00:00:00.000000Z`, so a due date that had been stored perfectly
|
|
// well came back as a blank input on the edit form, the show page and the
|
|
// index pane — indistinguishable from "it was never saved", which is how this
|
|
// was reported (a QA sweep read `input.value === ''` as silent data loss).
|
|
//
|
|
// The write path was never at fault: the column, the `$fillable` entry and
|
|
// both `validate()` rules were correct throughout, and re-submitting the ISO
|
|
// string (which Show.vue's meta form does, because it seeds itself from this
|
|
// 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.
|
|
*
|
|
* #1567 added `merged_stranded` beside it, for the same reason: there are now
|
|
* TWO ways a merge can fail to add up and they need different offers, so the
|
|
* page is handed which one it is rather than working it out from the columns.
|
|
*/
|
|
protected $appends = ['merged_inconsistent', 'merged_stranded'];
|
|
|
|
protected $casts = [
|
|
'due_date' => 'date:Y-m-d',
|
|
];
|
|
|
|
public function group(): BelongsTo
|
|
{
|
|
return $this->belongsTo(TicketingGroup::class, 'group_id');
|
|
}
|
|
|
|
public function priority(): BelongsTo
|
|
{
|
|
return $this->belongsTo(PriorityLevel::class, 'priority_id');
|
|
}
|
|
|
|
public function project(): BelongsTo
|
|
{
|
|
return $this->belongsTo(TicketingProject::class, 'project_id');
|
|
}
|
|
|
|
public function messages(): HasMany
|
|
{
|
|
return $this->hasMany(TicketMessage::class, 'ticket_id')->orderBy('created_at');
|
|
}
|
|
|
|
public function attachments(): HasMany
|
|
{
|
|
return $this->hasMany(TicketAttachment::class, 'ticket_id');
|
|
}
|
|
|
|
public function views(): HasMany
|
|
{
|
|
return $this->hasMany(TicketView::class, 'ticket_id')->orderBy('viewed_at');
|
|
}
|
|
|
|
public function participants(): HasMany
|
|
{
|
|
return $this->hasMany(TicketParticipant::class, 'ticket_id');
|
|
}
|
|
|
|
public function mergedInto(): BelongsTo
|
|
{
|
|
return $this->belongsTo(Ticket::class, 'merged_into_id');
|
|
}
|
|
|
|
/**
|
|
* A merged ticket is read-only — its messages and attachments now live on the
|
|
* ticket it was merged into, so it is a stub kept for its reference number and
|
|
* the audit trail, not a ticket anyone can still work.
|
|
*
|
|
* `status === 'merged'` and NOT `merged_into_id !== null`, deliberately. #920
|
|
* minted rows with `status = 'open', merged_into_id = 56` — the very rows this
|
|
* guard exists to prevent — and reading the id would make each of those
|
|
* permanently uneditable, i.e. a row the product will not let anyone correct.
|
|
* The status is the state; the id is the pointer that goes with it.
|
|
*/
|
|
public function isMerged(): bool
|
|
{
|
|
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();
|
|
}
|
|
|
|
/**
|
|
* THE OTHER DIRECTION — [central-ticketing] #1567.
|
|
*
|
|
* `status = 'merged'` with NOTHING to point at. `merged_into_id` carries
|
|
* `nullOnDelete()`, so deleting the ticket that others had been merged INTO
|
|
* used to blank their pointer and leave the status behind: a row that
|
|
* {@see isMerged()} still calls read-only, whose banner names a ticket that
|
|
* no longer exists, and which the Merged view — scoped by the pointer — could
|
|
* not list. Nothing in the product could act on it.
|
|
*
|
|
* `destroy()` no longer mints these ({@see \Dashboard\Ticketing\Support\StrandedMerge}),
|
|
* but the rows already made are still out there, so the shape needs a name:
|
|
* this is what the badge offers to un-merge and what the Merged view now
|
|
* includes.
|
|
*
|
|
* Deliberately NOT folded into {@see scopeInconsistentlyMerged()}: that scope
|
|
* is what #938's repair writes `status = 'merged'` over, and a stranded row
|
|
* already HAS that status — repairing it would match forever and write
|
|
* nothing, which is exactly the idempotency the repair guarantees.
|
|
*/
|
|
public function scopeStrandedMerge(Builder $query): Builder
|
|
{
|
|
return $query->where('status', self::STATUS_MERGED)
|
|
->whereNull('merged_into_id');
|
|
}
|
|
|
|
/** The row-level twin of {@see scopeStrandedMerge()}. */
|
|
public function isStrandedMerge(): bool
|
|
{
|
|
return $this->isMerged() && $this->merged_into_id === null;
|
|
}
|
|
|
|
/**
|
|
* EITHER shape of a merge that does not add up — #938's and #1567's.
|
|
*
|
|
* The two are mutually exclusive by construction (one has a pointer and the
|
|
* wrong status, the other has the right status and no pointer), and the
|
|
* badge, the sidebar's warning count and `merged_inconsistent` all want the
|
|
* union: "this row's status and its pointer disagree, whichever way round".
|
|
* The repair scopes stay separate because the fixes differ.
|
|
*/
|
|
public function scopeMergeInconsistent(Builder $query): Builder
|
|
{
|
|
return $query->where(function (Builder $q) {
|
|
$q->where(fn (Builder $i) => $i->inconsistentlyMerged())
|
|
->orWhere(fn (Builder $s) => $s->strandedMerge());
|
|
});
|
|
}
|
|
|
|
/**
|
|
* Every row that carries the merge shape AT ALL — what the Merged view lists.
|
|
*
|
|
* Pointer set (whatever the status: #938's rows included) or status `merged`
|
|
* (whether or not the pointer survived: #1567's stranded rows included).
|
|
* Scoping the view by the pointer alone dropped a stranded row out of the one
|
|
* view whose whole job is showing merges, leaving it visible only under
|
|
* Resolved — so a reader looking for merged tickets never found it.
|
|
*/
|
|
public function scopeEverMerged(Builder $query): Builder
|
|
{
|
|
return $query->where(function (Builder $q) {
|
|
$q->whereNotNull('merged_into_id')
|
|
->orWhere('status', self::STATUS_MERGED);
|
|
});
|
|
}
|
|
|
|
/**
|
|
* @see $appends — the same answer, for the page.
|
|
*
|
|
* #1567 widened this to the union: a stranded row is inconsistent in the
|
|
* plainest sense (it says it is merged into nothing), and the flag existing
|
|
* while reading `false` on exactly that row was half of what the card
|
|
* reported.
|
|
*/
|
|
public function getMergedInconsistentAttribute(): bool
|
|
{
|
|
return $this->isInconsistentlyMerged() || $this->isStrandedMerge();
|
|
}
|
|
|
|
/**
|
|
* @see $appends — WHICH of the two shapes, for a page that must offer
|
|
* different fixes. `merged_inconsistent` says something is wrong;
|
|
* `merged_stranded` says the target is gone, so the offer is "un-merge"
|
|
* rather than #938's "mark merged".
|
|
*/
|
|
public function getMergedStrandedAttribute(): bool
|
|
{
|
|
return $this->isStrandedMerge();
|
|
}
|
|
}
|