Refactor acceptance criteria into model
linter / quality (pull_request) Successful in 1m14s
tests / ci (8.4) (pull_request) Successful in 1m28s

This commit is contained in:
2026-05-19 22:20:15 +00:00
parent 3ddfcc89f9
commit 7805bd057e
10 changed files with 217 additions and 72 deletions
+2 -1
View File
@@ -55,7 +55,6 @@ class CreateProjectTicket
$ticket = Ticket::query()->create([ $ticket = Ticket::query()->create([
'acceptance_criteria' => TicketAcceptanceCriteriaData::toLegacyText($acceptanceCriteria), 'acceptance_criteria' => TicketAcceptanceCriteriaData::toLegacyText($acceptanceCriteria),
'acceptance_criteria_data' => TicketAcceptanceCriteriaData::forStorage($acceptanceCriteria),
'assignee_user_id' => Arr::get($attributes, 'assignee_user_id'), 'assignee_user_id' => Arr::get($attributes, 'assignee_user_id'),
'author_user_id' => $actor?->id, 'author_user_id' => $actor?->id,
'description' => Arr::get($attributes, 'description'), 'description' => Arr::get($attributes, 'description'),
@@ -69,6 +68,8 @@ class CreateProjectTicket
'story_points' => Arr::get($attributes, 'story_points'), 'story_points' => Arr::get($attributes, 'story_points'),
]); ]);
TicketAcceptanceCriteriaData::syncTicket($ticket, $acceptanceCriteria);
$occurredAt = CarbonImmutable::now(); $occurredAt = CarbonImmutable::now();
if ($actor !== null) { if ($actor !== null) {
@@ -2,6 +2,9 @@
namespace App\Actions\Tickets\Support; namespace App\Actions\Tickets\Support;
use App\Models\AcceptanceCriterion;
use App\Models\Ticket;
use Illuminate\Database\Eloquent\Collection as EloquentCollection;
use Illuminate\Support\Arr; use Illuminate\Support\Arr;
class TicketAcceptanceCriteriaData class TicketAcceptanceCriteriaData
@@ -9,6 +12,7 @@ class TicketAcceptanceCriteriaData
public const MAX_INDENT_LEVEL = 8; public const MAX_INDENT_LEVEL = 8;
/** /**
* @param iterable<AcceptanceCriterion> $criteria
* @return array{ * @return array{
* summary: string|null, * summary: string|null,
* items: array<int, array{ * items: array<int, array{
@@ -18,17 +22,37 @@ class TicketAcceptanceCriteriaData
* }> * }>
* } * }
*/ */
public static function fromStored(mixed $storedData, ?string $legacyText): array public static function fromModels(iterable $criteria, ?string $legacyText): array
{ {
$normalizedStoredData = is_array($storedData) $summary = self::normalize($legacyText)['summary'];
? self::normalize($storedData) $models = ($criteria instanceof EloquentCollection ? $criteria : collect($criteria))
: self::empty(); ->sort(function (AcceptanceCriterion $left, AcceptanceCriterion $right): int {
return [
$left->parent_acceptance_criterion_id ?? 0,
$left->position,
$left->id,
] <=> [
$right->parent_acceptance_criterion_id ?? 0,
$right->position,
$right->id,
];
})
->values();
if (self::hasContent($normalizedStoredData)) { if ($models->isEmpty()) {
return $normalizedStoredData; return self::normalize($legacyText);
} }
return self::normalize($legacyText); /** @var array<int|null, array<int, AcceptanceCriterion>> $grouped */
$grouped = $models->groupBy('parent_acceptance_criterion_id')->all();
$items = [];
self::flattenItems($items, $grouped, null, 0);
return [
'summary' => $summary,
'items' => $items,
];
} }
/** /**
@@ -71,6 +95,7 @@ class TicketAcceptanceCriteriaData
} }
/** /**
* @param Ticket $ticket
* @param array{ * @param array{
* summary: string|null, * summary: string|null,
* items: array<int, array{ * items: array<int, array{
@@ -79,22 +104,39 @@ class TicketAcceptanceCriteriaData
* text: string * text: string
* }> * }>
* } $acceptanceCriteria * } $acceptanceCriteria
* @return array{
* summary: string|null,
* items: array<int, array{
* checked: bool,
* indent: int,
* text: string
* }>
* }|null
*/ */
public static function forStorage(array $acceptanceCriteria): ?array public static function syncTicket(Ticket $ticket, array $acceptanceCriteria): void
{ {
$normalized = self::normalize($acceptanceCriteria); $normalized = self::normalize($acceptanceCriteria);
return self::hasContent($normalized) $ticket->acceptanceCriteria()->delete();
? $normalized
: null; if ($normalized['items'] === []) {
return;
}
$parentStack = [];
$positionsByParent = [];
foreach ($normalized['items'] as $item) {
$indent = min($item['indent'], count($parentStack));
$parentId = $indent === 0 ? null : $parentStack[$indent - 1];
$positionKey = $parentId ?? 'root';
$position = $positionsByParent[$positionKey] ?? 0;
$criterion = $ticket->acceptanceCriteria()->create([
'parent_acceptance_criterion_id' => $parentId,
'position' => $position,
'text' => $item['text'],
'is_checked' => $item['checked'],
]);
$positionsByParent[$positionKey] = $position + 1;
$parentStack[$indent] = $criterion->id;
$parentStack = array_slice($parentStack, 0, $indent + 1);
}
$ticket->unsetRelation('acceptanceCriteria');
} }
/** /**
@@ -154,6 +196,23 @@ class TicketAcceptanceCriteriaData
]; ];
} }
/**
* @param array<int, array{checked: bool, indent: int, text: string}> $items
* @param array<int|string, iterable<AcceptanceCriterion>> $grouped
*/
private static function flattenItems(array &$items, array $grouped, ?int $parentId, int $indent): void
{
foreach ($grouped[$parentId] ?? [] as $criterion) {
$items[] = [
'checked' => $criterion->is_checked,
'indent' => $indent,
'text' => $criterion->text,
];
self::flattenItems($items, $grouped, $criterion->id, $indent + 1);
}
}
/** /**
* @param array<string, mixed> $value * @param array<string, mixed> $value
* @return array{ * @return array{
+11 -16
View File
@@ -52,9 +52,12 @@ class UpdateProjectTicket
array $validated, array $validated,
): bool { ): bool {
$fieldValueAttributes = Arr::get($validated, 'field_values', []); $fieldValueAttributes = Arr::get($validated, 'field_values', []);
$nextAcceptanceCriteria = TicketAcceptanceCriteriaData::normalize(
Arr::get($validated, 'acceptance_criteria'),
);
$ticketAttributes = $this->buildTicketAttributes($ticket, $validated); $ticketAttributes = $this->buildTicketAttributes($ticket, $validated);
return DB::transaction(function () use ($actor, $fieldValueAttributes, $ticket, $ticketAttributes): bool { return DB::transaction(function () use ($actor, $fieldValueAttributes, $nextAcceptanceCriteria, $ticket, $ticketAttributes): bool {
if (! $ticketAttributes['is_blocked']) { if (! $ticketAttributes['is_blocked']) {
$ticketAttributes['blocked_comment'] = null; $ticketAttributes['blocked_comment'] = null;
} }
@@ -62,6 +65,7 @@ class UpdateProjectTicket
$ticket->loadMissing([ $ticket->loadMissing([
'assignee', 'assignee',
'author', 'author',
'acceptanceCriteria',
'fieldValues.ticketFieldDefinition', 'fieldValues.ticketFieldDefinition',
'fieldValues.userValue', 'fieldValues.userValue',
'parentTicket', 'parentTicket',
@@ -85,14 +89,10 @@ class UpdateProjectTicket
/** @var Collection<int, TicketFieldValue> $existingFieldValues */ /** @var Collection<int, TicketFieldValue> $existingFieldValues */
$existingFieldValues = $ticket->fieldValues $existingFieldValues = $ticket->fieldValues
->keyBy('ticket_field_definition_id'); ->keyBy('ticket_field_definition_id');
$currentAcceptanceCriteria = TicketAcceptanceCriteriaData::fromStored( $currentAcceptanceCriteria = TicketAcceptanceCriteriaData::fromModels(
$ticket->acceptance_criteria_data, $ticket->acceptanceCriteria,
$ticket->acceptance_criteria, $ticket->acceptance_criteria,
); );
$nextAcceptanceCriteria = TicketAcceptanceCriteriaData::fromStored(
$ticketAttributes['acceptance_criteria_data'],
$ticketAttributes['acceptance_criteria'],
);
$changes = []; $changes = [];
$summaryLabels = []; $summaryLabels = [];
@@ -198,6 +198,10 @@ class UpdateProjectTicket
$ticket->save(); $ticket->save();
} }
if ($currentAcceptanceCriteria !== $nextAcceptanceCriteria) {
TicketAcceptanceCriteriaData::syncTicket($ticket, $nextAcceptanceCriteria);
}
$fieldValueChangeSet = $this->syncProjectTicketFieldValues->handle( $fieldValueChangeSet = $this->syncProjectTicketFieldValues->handle(
$ticket, $ticket,
$fieldDefinitions, $fieldDefinitions,
@@ -252,14 +256,6 @@ class UpdateProjectTicket
* } $validated * } $validated
* @return array{ * @return array{
* acceptance_criteria: string|null, * acceptance_criteria: string|null,
* acceptance_criteria_data: array{
* summary: string|null,
* items: array<int, array{
* checked: bool,
* indent: int,
* text: string
* }>
* }|null,
* assignee_user_id: int|null, * assignee_user_id: int|null,
* author_user_id: int|null, * author_user_id: int|null,
* blocked_comment: string|null, * blocked_comment: string|null,
@@ -280,7 +276,6 @@ class UpdateProjectTicket
return [ return [
'acceptance_criteria' => TicketAcceptanceCriteriaData::toLegacyText($acceptanceCriteria), 'acceptance_criteria' => TicketAcceptanceCriteriaData::toLegacyText($acceptanceCriteria),
'acceptance_criteria_data' => TicketAcceptanceCriteriaData::forStorage($acceptanceCriteria),
'assignee_user_id' => Arr::exists($validated, 'assignee_user_id') 'assignee_user_id' => Arr::exists($validated, 'assignee_user_id')
? Arr::get($validated, 'assignee_user_id') ? Arr::get($validated, 'assignee_user_id')
: $ticket->assignee_user_id, : $ticket->assignee_user_id,
+4 -2
View File
@@ -17,8 +17,10 @@ class TicketResource extends JsonResource
{ {
return [ return [
'id' => $this->id, 'id' => $this->id,
'acceptance_criteria' => TicketAcceptanceCriteriaData::fromStored( 'acceptance_criteria' => TicketAcceptanceCriteriaData::fromModels(
$this->acceptance_criteria_data, $this->relationLoaded('acceptanceCriteria')
? $this->acceptanceCriteria
: $this->acceptanceCriteria()->get(),
$this->acceptance_criteria, $this->acceptance_criteria,
), ),
'acceptance_criteria_text' => $this->acceptance_criteria, 'acceptance_criteria_text' => $this->acceptance_criteria,
+4 -2
View File
@@ -227,8 +227,10 @@ class KanbanGitMcp
public function updatePayload(Ticket $ticket): array public function updatePayload(Ticket $ticket): array
{ {
return [ return [
'acceptance_criteria' => TicketAcceptanceCriteriaData::fromStored( 'acceptance_criteria' => TicketAcceptanceCriteriaData::fromModels(
$ticket->acceptance_criteria_data, $ticket->relationLoaded('acceptanceCriteria')
? $ticket->acceptanceCriteria
: $ticket->acceptanceCriteria()->get(),
$ticket->acceptance_criteria, $ticket->acceptance_criteria,
), ),
'assignee_user_id' => $ticket->assignee_user_id, 'assignee_user_id' => $ticket->assignee_user_id,
+45
View File
@@ -0,0 +1,45 @@
<?php
namespace App\Models;
use Illuminate\Database\Eloquent\Attributes\Fillable;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\BelongsTo;
use Illuminate\Database\Eloquent\Relations\HasMany;
#[Fillable([
'ticket_id',
'parent_acceptance_criterion_id',
'position',
'text',
'is_checked',
])]
class AcceptanceCriterion extends Model
{
/**
* @return array<string, string>
*/
protected function casts(): array
{
return [
'is_checked' => 'boolean',
];
}
public function children(): HasMany
{
return $this->hasMany(self::class, 'parent_acceptance_criterion_id')
->orderBy('position')
->orderBy('id');
}
public function parent(): BelongsTo
{
return $this->belongsTo(self::class, 'parent_acceptance_criterion_id');
}
public function ticket(): BelongsTo
{
return $this->belongsTo(Ticket::class);
}
}
+7 -2
View File
@@ -11,7 +11,6 @@ use Illuminate\Database\Eloquent\Relations\HasMany;
#[Fillable([ #[Fillable([
'acceptance_criteria', 'acceptance_criteria',
'acceptance_criteria_data',
'assignee_user_id', 'assignee_user_id',
'author_user_id', 'author_user_id',
'blocked_comment', 'blocked_comment',
@@ -38,11 +37,17 @@ class Ticket extends Model
protected function casts(): array protected function casts(): array
{ {
return [ return [
'acceptance_criteria_data' => 'array',
'is_blocked' => 'boolean', 'is_blocked' => 'boolean',
]; ];
} }
public function acceptanceCriteria(): HasMany
{
return $this->hasMany(AcceptanceCriterion::class)
->orderBy('position')
->orderBy('id');
}
public function assignee(): BelongsTo public function assignee(): BelongsTo
{ {
return $this->belongsTo(User::class, 'assignee_user_id'); return $this->belongsTo(User::class, 'assignee_user_id');
@@ -1,28 +0,0 @@
<?php
use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;
return new class extends Migration
{
/**
* Run the migrations.
*/
public function up(): void
{
Schema::table('tickets', function (Blueprint $table): void {
$table->json('acceptance_criteria_data')->nullable();
});
}
/**
* Reverse the migrations.
*/
public function down(): void
{
Schema::table('tickets', function (Blueprint $table): void {
$table->dropColumn('acceptance_criteria_data');
});
}
};
@@ -0,0 +1,39 @@
<?php
use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;
return new class extends Migration
{
/**
* Run the migrations.
*/
public function up(): void
{
Schema::create('acceptance_criteria', function (Blueprint $table): void {
$table->id();
$table->foreignId('ticket_id')
->constrained()
->cascadeOnDelete();
$table->foreignId('parent_acceptance_criterion_id')
->nullable()
->constrained('acceptance_criteria')
->cascadeOnDelete();
$table->unsignedInteger('position')->default(0);
$table->text('text');
$table->boolean('is_checked')->default(false);
$table->timestamps();
$table->index(['ticket_id', 'parent_acceptance_criterion_id', 'position']);
});
}
/**
* Reverse the migrations.
*/
public function down(): void
{
Schema::dropIfExists('acceptance_criteria');
}
};
@@ -527,9 +527,34 @@ class ProjectTicketControllerTest extends TestCase
'ticket_priority_id' => $highPriority->id, 'ticket_priority_id' => $highPriority->id,
]); ]);
$ticket->refresh(); $ticket->refresh()->load('acceptanceCriteria');
$this->assertEquals($updatedAcceptanceCriteria, $ticket->acceptance_criteria_data); $this->assertEquals(
$updatedAcceptanceCriteria,
\App\Actions\Tickets\Support\TicketAcceptanceCriteriaData::fromModels(
$ticket->acceptanceCriteria,
$ticket->acceptance_criteria,
),
);
$this->assertDatabaseHas('acceptance_criteria', [
'ticket_id' => $ticket->id,
'parent_acceptance_criterion_id' => null,
'position' => 0,
'text' => 'Shows success banner',
'is_checked' => false,
]);
$rootCriterionId = \App\Models\AcceptanceCriterion::query()
->where('ticket_id', $ticket->id)
->where('text', 'Shows success banner')
->value('id');
$this->assertNotNull($rootCriterionId);
$this->assertDatabaseHas('acceptance_criteria', [
'ticket_id' => $ticket->id,
'parent_acceptance_criterion_id' => $rootCriterionId,
'position' => 0,
'text' => 'Includes nested confirmations',
'is_checked' => true,
]);
$this->assertDatabaseHas('ticket_field_values', [ $this->assertDatabaseHas('ticket_field_values', [
'ticket_id' => $ticket->id, 'ticket_id' => $ticket->id,
'ticket_field_definition_id' => $reviewerField->id, 'ticket_field_definition_id' => $reviewerField->id,