Simplify acceptance criteria checklist model
linter / quality (pull_request) Successful in 1m16s
tests / ci (8.4) (pull_request) Successful in 1m30s

This commit is contained in:
2026-05-19 22:44:28 +00:00
parent 7805bd057e
commit 689db9e41b
16 changed files with 140 additions and 363 deletions
+9 -12
View File
@@ -4,7 +4,7 @@ namespace App\Actions\Tickets;
use App\Actions\Notifications\BuildTicketCreatedPayload;
use App\Actions\Notifications\TicketCreated;
use App\Actions\Tickets\Support\TicketAcceptanceCriteriaData;
use App\Actions\Tickets\Support\AcceptanceCriteriaChecklist;
use App\Enums\TicketWorklogEntryType;
use App\Models\Project;
use App\Models\Ticket;
@@ -23,14 +23,11 @@ class CreateProjectTicket
/**
* @param array{
* acceptance_criteria: array{
* summary: string|null,
* items: array<int, array{
* checked: bool,
* indent: int,
* text: string
* }>
* },
* acceptance_criteria: array<int, array{
* checked: bool,
* indent: int,
* text: string
* }>,
* assignee_user_id?: int|null,
* description: string|null,
* story_points?: int|null,
@@ -43,7 +40,7 @@ class CreateProjectTicket
public function handle(Project $project, array $attributes, ?User $actor = null): Ticket
{
return DB::transaction(function () use ($actor, $attributes, $project): Ticket {
$acceptanceCriteria = TicketAcceptanceCriteriaData::normalize(
$acceptanceCriteria = AcceptanceCriteriaChecklist::normalize(
Arr::get($attributes, 'acceptance_criteria'),
);
$nextSequenceNumber = (int) Ticket::query()
@@ -54,7 +51,7 @@ class CreateProjectTicket
->findOrFail(Arr::get($attributes, 'ticket_status_id'));
$ticket = Ticket::query()->create([
'acceptance_criteria' => TicketAcceptanceCriteriaData::toLegacyText($acceptanceCriteria),
'acceptance_criteria' => AcceptanceCriteriaChecklist::toText($acceptanceCriteria),
'assignee_user_id' => Arr::get($attributes, 'assignee_user_id'),
'author_user_id' => $actor?->id,
'description' => Arr::get($attributes, 'description'),
@@ -68,7 +65,7 @@ class CreateProjectTicket
'story_points' => Arr::get($attributes, 'story_points'),
]);
TicketAcceptanceCriteriaData::syncTicket($ticket, $acceptanceCriteria);
AcceptanceCriteriaChecklist::syncTicket($ticket, $acceptanceCriteria);
$occurredAt = CarbonImmutable::now();
@@ -7,24 +7,16 @@ use App\Models\Ticket;
use Illuminate\Database\Eloquent\Collection as EloquentCollection;
use Illuminate\Support\Arr;
class TicketAcceptanceCriteriaData
class AcceptanceCriteriaChecklist
{
public const MAX_INDENT_LEVEL = 8;
/**
* @param iterable<AcceptanceCriterion> $criteria
* @return array{
* summary: string|null,
* items: array<int, array{
* checked: bool,
* indent: int,
* text: string
* }>
* }
* @return array<int, array{checked: bool, indent: int, text: string}>
*/
public static function fromModels(iterable $criteria, ?string $legacyText): array
public static function fromModels(iterable $criteria, ?string $legacyText = null): array
{
$summary = self::normalize($legacyText)['summary'];
$models = ($criteria instanceof EloquentCollection ? $criteria : collect($criteria))
->sort(function (AcceptanceCriterion $left, AcceptanceCriterion $right): int {
return [
@@ -43,27 +35,17 @@ class TicketAcceptanceCriteriaData
return self::normalize($legacyText);
}
/** @var array<int|null, array<int, AcceptanceCriterion>> $grouped */
/** @var array<int|string, iterable<AcceptanceCriterion>> $grouped */
$grouped = $models->groupBy('parent_acceptance_criterion_id')->all();
$items = [];
self::flattenItems($items, $grouped, null, 0);
return [
'summary' => $summary,
'items' => $items,
];
return $items;
}
/**
* @return array{
* summary: string|null,
* items: array<int, array{
* checked: bool,
* indent: int,
* text: string
* }>
* }
* @return array<int, array{checked: bool, indent: int, text: string}>
*/
public static function normalize(mixed $value): array
{
@@ -75,50 +57,27 @@ class TicketAcceptanceCriteriaData
return self::normalizeFromString($value);
}
return self::empty();
}
/**
* @param array{
* summary: string|null,
* items: array<int, array{
* checked: bool,
* indent: int,
* text: string
* }>
* } $acceptanceCriteria
*/
public static function hasContent(array $acceptanceCriteria): bool
{
return $acceptanceCriteria['summary'] !== null
|| $acceptanceCriteria['items'] !== [];
return [];
}
/**
* @param Ticket $ticket
* @param array{
* summary: string|null,
* items: array<int, array{
* checked: bool,
* indent: int,
* text: string
* }>
* } $acceptanceCriteria
* @param array<int, array{checked: bool, indent: int, text: string}> $items
*/
public static function syncTicket(Ticket $ticket, array $acceptanceCriteria): void
public static function syncTicket(Ticket $ticket, array $items): void
{
$normalized = self::normalize($acceptanceCriteria);
$normalized = self::normalize($items);
$ticket->acceptanceCriteria()->delete();
if ($normalized['items'] === []) {
if ($normalized === []) {
return;
}
$parentStack = [];
$positionsByParent = [];
foreach ($normalized['items'] as $item) {
foreach ($normalized as $item) {
$indent = min($item['indent'], count($parentStack));
$parentId = $indent === 0 ? null : $parentStack[$indent - 1];
$positionKey = $parentId ?? 'root';
@@ -140,60 +99,25 @@ class TicketAcceptanceCriteriaData
}
/**
* @param array{
* summary: string|null,
* items: array<int, array{
* checked: bool,
* indent: int,
* text: string
* }>
* } $acceptanceCriteria
* @param array<int, array{checked: bool, indent: int, text: string}> $items
*/
public static function toLegacyText(array $acceptanceCriteria): ?string
public static function toText(array $items): ?string
{
$normalized = self::normalize($acceptanceCriteria);
$lines = [];
$normalized = self::normalize($items);
if ($normalized['summary'] !== null) {
$lines[] = $normalized['summary'];
if ($normalized === []) {
return null;
}
if ($normalized['items'] !== []) {
if ($normalized['summary'] !== null) {
$lines[] = '';
}
foreach ($normalized['items'] as $item) {
$lines[] = sprintf(
'%s- [%s] %s',
str_repeat(' ', $item['indent']),
$item['checked'] ? 'x' : ' ',
$item['text'],
);
}
}
$legacyText = trim(implode("\n", $lines));
return $legacyText === '' ? null : $legacyText;
}
/**
* @return array{
* summary: string|null,
* items: array<int, array{
* checked: bool,
* indent: int,
* text: string
* }>
* }
*/
private static function empty(): array
{
return [
'summary' => null,
'items' => [],
];
return implode("\n", array_map(
static fn (array $item): string => sprintf(
'%s- [%s] %s',
str_repeat(' ', $item['indent']),
$item['checked'] ? 'x' : ' ',
$item['text'],
),
$normalized,
));
}
/**
@@ -214,23 +138,15 @@ class TicketAcceptanceCriteriaData
}
/**
* @param array<string, mixed> $value
* @return array{
* summary: string|null,
* items: array<int, array{
* checked: bool,
* indent: int,
* text: string
* }>
* }
* @param array<int|string, mixed> $value
* @return array<int, array{checked: bool, indent: int, text: string}>
*/
private static function normalizeFromArray(array $value): array
{
$summary = trim((string) Arr::get($value, 'summary', ''));
$rawItems = Arr::get($value, 'items', []);
$rawItems = Arr::isAssoc($value) ? Arr::get($value, 'items', []) : $value;
if (! is_array($rawItems)) {
$rawItems = [];
return [];
}
$items = [];
@@ -246,78 +162,37 @@ class TicketAcceptanceCriteriaData
continue;
}
$rawChecked = Arr::get($rawItem, 'checked', false);
$rawIndent = Arr::get($rawItem, 'indent', 0);
$items[] = [
'checked' => filter_var($rawChecked, FILTER_VALIDATE_BOOLEAN),
'indent' => self::clampIndent((int) $rawIndent),
'checked' => filter_var(Arr::get($rawItem, 'checked', false), FILTER_VALIDATE_BOOLEAN),
'indent' => self::clampIndent((int) Arr::get($rawItem, 'indent', 0)),
'text' => $text,
];
}
return [
'summary' => $summary === '' ? null : $summary,
'items' => $items,
];
return $items;
}
/**
* @return array{
* summary: string|null,
* items: array<int, array{
* checked: bool,
* indent: int,
* text: string
* }>
* }
* @return array<int, array{checked: bool, indent: int, text: string}>
*/
private static function normalizeFromString(?string $value): array
{
$text = trim((string) $value);
if ($text === '') {
return self::empty();
return [];
}
$lines = preg_split('/\R/u', $text);
if (! is_array($lines)) {
return [
'summary' => $text,
'items' => [],
];
return [];
}
$summary = null;
$items = [];
foreach ($lines as $line) {
$trimmedLine = trim($line);
if ($trimmedLine === '') {
continue;
}
if ($summary === null) {
$summary = $trimmedLine;
continue;
}
$parsedItem = self::parseChecklistItem($line);
if ($parsedItem === null) {
continue;
}
$items[] = $parsedItem;
}
return [
'summary' => $summary,
'items' => $items,
];
return array_values(array_filter(array_map(
static fn (string $line): ?array => self::parseChecklistItem($line),
$lines,
)));
}
/**
@@ -348,6 +223,8 @@ class TicketAcceptanceCriteriaData
$content = trim((string) Arr::get($checkboxMatches, 'text', ''));
} elseif (preg_match('/^[-*+]\s+(?<text>.+)$/u', $content, $bulletMatches)) {
$content = trim((string) Arr::get($bulletMatches, 'text', ''));
} else {
return null;
}
if ($content === '') {
+18 -30
View File
@@ -2,7 +2,7 @@
namespace App\Actions\Tickets;
use App\Actions\Tickets\Support\TicketAcceptanceCriteriaData;
use App\Actions\Tickets\Support\AcceptanceCriteriaChecklist;
use App\Enums\TicketWorklogEntryType;
use App\Models\Ticket;
use App\Models\TicketFieldDefinition;
@@ -21,14 +21,11 @@ class UpdateProjectTicket
/**
* @param array{
* acceptance_criteria: array{
* summary: string|null,
* items: array<int, array{
* checked: bool,
* indent: int,
* text: string
* }>
* },
* acceptance_criteria: array<int, array{
* checked: bool,
* indent: int,
* text: string
* }>,
* assignee_user_id?: int|null,
* author_user_id?: int|null,
* blocked_comment: string|null,
@@ -52,7 +49,7 @@ class UpdateProjectTicket
array $validated,
): bool {
$fieldValueAttributes = Arr::get($validated, 'field_values', []);
$nextAcceptanceCriteria = TicketAcceptanceCriteriaData::normalize(
$nextAcceptanceCriteria = AcceptanceCriteriaChecklist::normalize(
Arr::get($validated, 'acceptance_criteria'),
);
$ticketAttributes = $this->buildTicketAttributes($ticket, $validated);
@@ -89,7 +86,7 @@ class UpdateProjectTicket
/** @var Collection<int, TicketFieldValue> $existingFieldValues */
$existingFieldValues = $ticket->fieldValues
->keyBy('ticket_field_definition_id');
$currentAcceptanceCriteria = TicketAcceptanceCriteriaData::fromModels(
$currentAcceptanceCriteria = AcceptanceCriteriaChecklist::fromModels(
$ticket->acceptanceCriteria,
$ticket->acceptance_criteria,
);
@@ -199,7 +196,7 @@ class UpdateProjectTicket
}
if ($currentAcceptanceCriteria !== $nextAcceptanceCriteria) {
TicketAcceptanceCriteriaData::syncTicket($ticket, $nextAcceptanceCriteria);
AcceptanceCriteriaChecklist::syncTicket($ticket, $nextAcceptanceCriteria);
}
$fieldValueChangeSet = $this->syncProjectTicketFieldValues->handle(
@@ -235,14 +232,11 @@ class UpdateProjectTicket
/**
* @param array{
* acceptance_criteria: array{
* summary: string|null,
* items: array<int, array{
* checked: bool,
* indent: int,
* text: string
* }>
* },
* acceptance_criteria: array<int, array{
* checked: bool,
* indent: int,
* text: string
* }>,
* assignee_user_id?: int|null,
* author_user_id?: int|null,
* blocked_comment: string|null,
@@ -270,12 +264,12 @@ class UpdateProjectTicket
*/
private function buildTicketAttributes(Ticket $ticket, array $validated): array
{
$acceptanceCriteria = TicketAcceptanceCriteriaData::normalize(
$acceptanceCriteria = AcceptanceCriteriaChecklist::normalize(
Arr::get($validated, 'acceptance_criteria'),
);
return [
'acceptance_criteria' => TicketAcceptanceCriteriaData::toLegacyText($acceptanceCriteria),
'acceptance_criteria' => AcceptanceCriteriaChecklist::toText($acceptanceCriteria),
'assignee_user_id' => Arr::exists($validated, 'assignee_user_id')
? Arr::get($validated, 'assignee_user_id')
: $ticket->assignee_user_id,
@@ -298,14 +292,8 @@ class UpdateProjectTicket
/**
* @param array<string, mixed> $changes
* @param array<int, string> $summaryLabels
* @param array{
* summary: string|null,
* items: array<int, array{checked: bool, indent: int, text: string}>
* } $currentValue
* @param array{
* summary: string|null,
* items: array<int, array{checked: bool, indent: int, text: string}>
* } $newValue
* @param array<int, array{checked: bool, indent: int, text: string}> $currentValue
* @param array<int, array{checked: bool, indent: int, text: string}> $newValue
*/
private function trackAcceptanceCriteriaChange(
array &$changes,
@@ -2,7 +2,7 @@
namespace App\Http\Requests;
use App\Actions\Tickets\Support\TicketAcceptanceCriteriaData;
use App\Actions\Tickets\Support\AcceptanceCriteriaChecklist;
use App\Models\Project;
use Illuminate\Contracts\Validation\ValidationRule;
use Illuminate\Foundation\Http\FormRequest;
@@ -24,7 +24,7 @@ class StoreProjectTicketRequest extends FormRequest
protected function prepareForValidation(): void
{
$this->merge([
'acceptance_criteria' => TicketAcceptanceCriteriaData::normalize(
'acceptance_criteria' => AcceptanceCriteriaChecklist::normalize(
$this->input('acceptance_criteria'),
),
]);
@@ -42,12 +42,10 @@ class StoreProjectTicketRequest extends FormRequest
return [
'acceptance_criteria' => ['nullable', 'array'],
'acceptance_criteria.summary' => ['nullable', 'string', 'max:255'],
'acceptance_criteria.items' => ['array'],
'acceptance_criteria.items.*' => ['array'],
'acceptance_criteria.items.*.text' => ['required', 'string'],
'acceptance_criteria.items.*.checked' => ['required', 'boolean'],
'acceptance_criteria.items.*.indent' => ['required', 'integer', 'min:0', 'max:'.TicketAcceptanceCriteriaData::MAX_INDENT_LEVEL],
'acceptance_criteria.*' => ['array'],
'acceptance_criteria.*.text' => ['required', 'string'],
'acceptance_criteria.*.checked' => ['required', 'boolean'],
'acceptance_criteria.*.indent' => ['required', 'integer', 'min:0', 'max:'.AcceptanceCriteriaChecklist::MAX_INDENT_LEVEL],
'assignee_user_id' => ['nullable', 'integer', Rule::exists('users', 'id')],
'description' => ['nullable', 'string'],
'ticket_priority_id' => [
@@ -3,7 +3,7 @@
namespace App\Http\Requests;
use App\Actions\Permissions\PermissionService;
use App\Actions\Tickets\Support\TicketAcceptanceCriteriaData;
use App\Actions\Tickets\Support\AcceptanceCriteriaChecklist;
use App\Enums\Permission;
use App\Models\Project;
use App\Models\Ticket;
@@ -30,7 +30,7 @@ class UpdateProjectTicketRequest extends FormRequest
protected function prepareForValidation(): void
{
$this->merge([
'acceptance_criteria' => TicketAcceptanceCriteriaData::normalize(
'acceptance_criteria' => AcceptanceCriteriaChecklist::normalize(
$this->input('acceptance_criteria'),
),
]);
@@ -48,12 +48,10 @@ class UpdateProjectTicketRequest extends FormRequest
return [
'acceptance_criteria' => ['nullable', 'array'],
'acceptance_criteria.summary' => ['nullable', 'string', 'max:255'],
'acceptance_criteria.items' => ['array'],
'acceptance_criteria.items.*' => ['array'],
'acceptance_criteria.items.*.text' => ['required', 'string'],
'acceptance_criteria.items.*.checked' => ['required', 'boolean'],
'acceptance_criteria.items.*.indent' => ['required', 'integer', 'min:0', 'max:'.TicketAcceptanceCriteriaData::MAX_INDENT_LEVEL],
'acceptance_criteria.*' => ['array'],
'acceptance_criteria.*.text' => ['required', 'string'],
'acceptance_criteria.*.checked' => ['required', 'boolean'],
'acceptance_criteria.*.indent' => ['required', 'integer', 'min:0', 'max:'.AcceptanceCriteriaChecklist::MAX_INDENT_LEVEL],
'assignee_user_id' => ['nullable', 'integer', Rule::exists('users', 'id')],
'author_user_id' => ['nullable', 'integer', Rule::exists('users', 'id')],
'blocked_comment' => ['nullable', 'string'],
+2 -3
View File
@@ -2,7 +2,7 @@
namespace App\Http\Resources;
use App\Actions\Tickets\Support\TicketAcceptanceCriteriaData;
use App\Actions\Tickets\Support\AcceptanceCriteriaChecklist;
use Illuminate\Http\Request;
use Illuminate\Http\Resources\Json\JsonResource;
@@ -17,13 +17,12 @@ class TicketResource extends JsonResource
{
return [
'id' => $this->id,
'acceptance_criteria' => TicketAcceptanceCriteriaData::fromModels(
'acceptance_criteria' => AcceptanceCriteriaChecklist::fromModels(
$this->relationLoaded('acceptanceCriteria')
? $this->acceptanceCriteria
: $this->acceptanceCriteria()->get(),
$this->acceptance_criteria,
),
'acceptance_criteria_text' => $this->acceptance_criteria,
'assignee' => $this->whenLoaded(
'assignee',
fn (): ?array => $this->assignee === null
+3 -6
View File
@@ -3,7 +3,7 @@
namespace App\Mcp\Support;
use App\Actions\Permissions\PermissionService;
use App\Actions\Tickets\Support\TicketAcceptanceCriteriaData;
use App\Actions\Tickets\Support\AcceptanceCriteriaChecklist;
use App\Enums\Permission;
use App\Http\Resources\ProjectResource;
use App\Http\Resources\TicketResource;
@@ -208,10 +208,7 @@ class KanbanGitMcp
/**
* @return array{
* acceptance_criteria: array{
* summary: string|null,
* items: array<int, array{checked: bool, indent: int, text: string}>
* },
* acceptance_criteria: array<int, array{checked: bool, indent: int, text: string}>,
* assignee_user_id: int|null,
* author_user_id: int|null,
* blocked_comment: string|null,
@@ -227,7 +224,7 @@ class KanbanGitMcp
public function updatePayload(Ticket $ticket): array
{
return [
'acceptance_criteria' => TicketAcceptanceCriteriaData::fromModels(
'acceptance_criteria' => AcceptanceCriteriaChecklist::fromModels(
$ticket->relationLoaded('acceptanceCriteria')
? $ticket->acceptanceCriteria
: $ticket->acceptanceCriteria()->get(),