Remove legacy acceptance criteria storage
This commit is contained in:
@@ -51,7 +51,6 @@ class CreateProjectTicket
|
||||
->findOrFail(Arr::get($attributes, 'ticket_status_id'));
|
||||
|
||||
$ticket = Ticket::query()->create([
|
||||
'acceptance_criteria' => AcceptanceCriteriaChecklist::toText($acceptanceCriteria),
|
||||
'assignee_user_id' => Arr::get($attributes, 'assignee_user_id'),
|
||||
'author_user_id' => $actor?->id,
|
||||
'description' => Arr::get($attributes, 'description'),
|
||||
|
||||
@@ -15,7 +15,7 @@ class AcceptanceCriteriaChecklist
|
||||
* @param iterable<AcceptanceCriterion> $criteria
|
||||
* @return array<int, array{checked: bool, indent: int, text: string}>
|
||||
*/
|
||||
public static function fromModels(iterable $criteria, ?string $legacyText = null): array
|
||||
public static function fromModels(iterable $criteria): array
|
||||
{
|
||||
$models = ($criteria instanceof EloquentCollection ? $criteria : collect($criteria))
|
||||
->sort(function (AcceptanceCriterion $left, AcceptanceCriterion $right): int {
|
||||
@@ -32,7 +32,7 @@ class AcceptanceCriteriaChecklist
|
||||
->values();
|
||||
|
||||
if ($models->isEmpty()) {
|
||||
return self::normalize($legacyText);
|
||||
return [];
|
||||
}
|
||||
|
||||
/** @var array<int|string, iterable<AcceptanceCriterion>> $grouped */
|
||||
@@ -53,10 +53,6 @@ class AcceptanceCriteriaChecklist
|
||||
return self::normalizeFromArray($value);
|
||||
}
|
||||
|
||||
if (is_string($value) || $value === null) {
|
||||
return self::normalizeFromString($value);
|
||||
}
|
||||
|
||||
return [];
|
||||
}
|
||||
|
||||
@@ -97,29 +93,6 @@ class AcceptanceCriteriaChecklist
|
||||
|
||||
$ticket->unsetRelation('acceptanceCriteria');
|
||||
}
|
||||
|
||||
/**
|
||||
* @param array<int, array{checked: bool, indent: int, text: string}> $items
|
||||
*/
|
||||
public static function toText(array $items): ?string
|
||||
{
|
||||
$normalized = self::normalize($items);
|
||||
|
||||
if ($normalized === []) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return implode("\n", array_map(
|
||||
static fn (array $item): string => sprintf(
|
||||
'%s- [%s] %s',
|
||||
str_repeat(' ', $item['indent']),
|
||||
$item['checked'] ? 'x' : ' ',
|
||||
$item['text'],
|
||||
),
|
||||
$normalized,
|
||||
));
|
||||
}
|
||||
|
||||
/**
|
||||
* @param array<int, array{checked: bool, indent: int, text: string}> $items
|
||||
* @param array<int|string, iterable<AcceptanceCriterion>> $grouped
|
||||
@@ -143,15 +116,9 @@ class AcceptanceCriteriaChecklist
|
||||
*/
|
||||
private static function normalizeFromArray(array $value): array
|
||||
{
|
||||
$rawItems = Arr::isAssoc($value) ? Arr::get($value, 'items', []) : $value;
|
||||
|
||||
if (! is_array($rawItems)) {
|
||||
return [];
|
||||
}
|
||||
|
||||
$items = [];
|
||||
|
||||
foreach ($rawItems as $rawItem) {
|
||||
foreach ($value as $rawItem) {
|
||||
if (! is_array($rawItem)) {
|
||||
continue;
|
||||
}
|
||||
@@ -172,79 +139,6 @@ class AcceptanceCriteriaChecklist
|
||||
return $items;
|
||||
}
|
||||
|
||||
/**
|
||||
* @return array<int, array{checked: bool, indent: int, text: string}>
|
||||
*/
|
||||
private static function normalizeFromString(?string $value): array
|
||||
{
|
||||
$text = trim((string) $value);
|
||||
|
||||
if ($text === '') {
|
||||
return [];
|
||||
}
|
||||
|
||||
$lines = preg_split('/\R/u', $text);
|
||||
|
||||
if (! is_array($lines)) {
|
||||
return [];
|
||||
}
|
||||
|
||||
return array_values(array_filter(array_map(
|
||||
static fn (string $line): ?array => self::parseChecklistItem($line),
|
||||
$lines,
|
||||
)));
|
||||
}
|
||||
|
||||
/**
|
||||
* @return array{checked: bool, indent: int, text: string}|null
|
||||
*/
|
||||
private static function parseChecklistItem(string $line): ?array
|
||||
{
|
||||
$line = rtrim(str_replace("\t", ' ', $line));
|
||||
|
||||
if ($line === '') {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (! preg_match('/^(?<indent>\s*)(?<content>.+)$/u', $line, $matches)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
$content = trim((string) Arr::get($matches, 'content', ''));
|
||||
|
||||
if ($content === '') {
|
||||
return null;
|
||||
}
|
||||
|
||||
$checked = false;
|
||||
|
||||
if (preg_match('/^(?:[-*+]\s+)?\[(?<state>[xX ])\]\s*(?<text>.+)$/u', $content, $checkboxMatches)) {
|
||||
$checked = strtolower((string) Arr::get($checkboxMatches, 'state', '')) === 'x';
|
||||
$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 === '') {
|
||||
return null;
|
||||
}
|
||||
|
||||
return [
|
||||
'checked' => $checked,
|
||||
'indent' => self::indentLevel((string) Arr::get($matches, 'indent', '')),
|
||||
'text' => $content,
|
||||
];
|
||||
}
|
||||
|
||||
private static function indentLevel(string $leadingWhitespace): int
|
||||
{
|
||||
$normalizedWhitespace = str_replace("\t", ' ', $leadingWhitespace);
|
||||
|
||||
return self::clampIndent(intdiv(strlen($normalizedWhitespace), 2));
|
||||
}
|
||||
|
||||
private static function clampIndent(int $indent): int
|
||||
{
|
||||
return max(0, min(self::MAX_INDENT_LEVEL, $indent));
|
||||
|
||||
@@ -86,10 +86,7 @@ class UpdateProjectTicket
|
||||
/** @var Collection<int, TicketFieldValue> $existingFieldValues */
|
||||
$existingFieldValues = $ticket->fieldValues
|
||||
->keyBy('ticket_field_definition_id');
|
||||
$currentAcceptanceCriteria = AcceptanceCriteriaChecklist::fromModels(
|
||||
$ticket->acceptanceCriteria,
|
||||
$ticket->acceptance_criteria,
|
||||
);
|
||||
$currentAcceptanceCriteria = AcceptanceCriteriaChecklist::fromModels($ticket->acceptanceCriteria);
|
||||
|
||||
$changes = [];
|
||||
$summaryLabels = [];
|
||||
@@ -249,7 +246,6 @@ class UpdateProjectTicket
|
||||
* title: string
|
||||
* } $validated
|
||||
* @return array{
|
||||
* acceptance_criteria: string|null,
|
||||
* assignee_user_id: int|null,
|
||||
* author_user_id: int|null,
|
||||
* blocked_comment: string|null,
|
||||
@@ -264,12 +260,7 @@ class UpdateProjectTicket
|
||||
*/
|
||||
private function buildTicketAttributes(Ticket $ticket, array $validated): array
|
||||
{
|
||||
$acceptanceCriteria = AcceptanceCriteriaChecklist::normalize(
|
||||
Arr::get($validated, 'acceptance_criteria'),
|
||||
);
|
||||
|
||||
return [
|
||||
'acceptance_criteria' => AcceptanceCriteriaChecklist::toText($acceptanceCriteria),
|
||||
'assignee_user_id' => Arr::exists($validated, 'assignee_user_id')
|
||||
? Arr::get($validated, 'assignee_user_id')
|
||||
: $ticket->assignee_user_id,
|
||||
|
||||
@@ -23,9 +23,15 @@ class StoreProjectTicketRequest extends FormRequest
|
||||
*/
|
||||
protected function prepareForValidation(): void
|
||||
{
|
||||
$acceptanceCriteria = $this->input('acceptance_criteria');
|
||||
|
||||
if ($acceptanceCriteria !== null && ! is_array($acceptanceCriteria)) {
|
||||
return;
|
||||
}
|
||||
|
||||
$this->merge([
|
||||
'acceptance_criteria' => AcceptanceCriteriaChecklist::normalize(
|
||||
$this->input('acceptance_criteria'),
|
||||
$acceptanceCriteria,
|
||||
),
|
||||
]);
|
||||
}
|
||||
|
||||
@@ -29,9 +29,15 @@ class UpdateProjectTicketRequest extends FormRequest
|
||||
*/
|
||||
protected function prepareForValidation(): void
|
||||
{
|
||||
$acceptanceCriteria = $this->input('acceptance_criteria');
|
||||
|
||||
if ($acceptanceCriteria !== null && ! is_array($acceptanceCriteria)) {
|
||||
return;
|
||||
}
|
||||
|
||||
$this->merge([
|
||||
'acceptance_criteria' => AcceptanceCriteriaChecklist::normalize(
|
||||
$this->input('acceptance_criteria'),
|
||||
$acceptanceCriteria,
|
||||
),
|
||||
]);
|
||||
}
|
||||
|
||||
@@ -21,7 +21,6 @@ class TicketResource extends JsonResource
|
||||
$this->relationLoaded('acceptanceCriteria')
|
||||
? $this->acceptanceCriteria
|
||||
: $this->acceptanceCriteria()->get(),
|
||||
$this->acceptance_criteria,
|
||||
),
|
||||
'assignee' => $this->whenLoaded(
|
||||
'assignee',
|
||||
|
||||
@@ -228,7 +228,6 @@ class KanbanGitMcp
|
||||
$ticket->relationLoaded('acceptanceCriteria')
|
||||
? $ticket->acceptanceCriteria
|
||||
: $ticket->acceptanceCriteria()->get(),
|
||||
$ticket->acceptance_criteria,
|
||||
),
|
||||
'assignee_user_id' => $ticket->assignee_user_id,
|
||||
'author_user_id' => $ticket->author_user_id,
|
||||
|
||||
@@ -10,7 +10,6 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo;
|
||||
use Illuminate\Database\Eloquent\Relations\HasMany;
|
||||
|
||||
#[Fillable([
|
||||
'acceptance_criteria',
|
||||
'assignee_user_id',
|
||||
'author_user_id',
|
||||
'blocked_comment',
|
||||
|
||||
@@ -22,7 +22,6 @@ class TicketFactory extends Factory
|
||||
public function definition(): array
|
||||
{
|
||||
return [
|
||||
'acceptance_criteria' => fake()->paragraph(),
|
||||
'assignee_user_id' => null,
|
||||
'author_user_id' => User::factory(),
|
||||
'blocked_comment' => null,
|
||||
|
||||
@@ -26,7 +26,6 @@ return new class extends Migration
|
||||
$table->string('ticket_key')->unique();
|
||||
$table->string('title');
|
||||
$table->text('description')->nullable();
|
||||
$table->text('acceptance_criteria')->nullable();
|
||||
$table->unsignedInteger('story_points')->nullable()->default(null);
|
||||
$table->timestamps();
|
||||
|
||||
|
||||
@@ -0,0 +1,36 @@
|
||||
<?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
|
||||
{
|
||||
if (! Schema::hasColumn('tickets', 'acceptance_criteria')) {
|
||||
return;
|
||||
}
|
||||
|
||||
Schema::table('tickets', function (Blueprint $table): void {
|
||||
$table->dropColumn('acceptance_criteria');
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Reverse the migrations.
|
||||
*/
|
||||
public function down(): void
|
||||
{
|
||||
if (Schema::hasColumn('tickets', 'acceptance_criteria')) {
|
||||
return;
|
||||
}
|
||||
|
||||
Schema::table('tickets', function (Blueprint $table): void {
|
||||
$table->text('acceptance_criteria')->nullable()->after('description');
|
||||
});
|
||||
}
|
||||
};
|
||||
@@ -42,7 +42,11 @@ class TicketSeeder extends Seeder
|
||||
->firstOrFail();
|
||||
|
||||
$createProjectTicket->handle($project, [
|
||||
'acceptance_criteria' => 'The seeded ticket should be visible in the backlog list.',
|
||||
'acceptance_criteria' => [[
|
||||
'checked' => false,
|
||||
'indent' => 0,
|
||||
'text' => 'The seeded ticket should be visible in the backlog list.',
|
||||
]],
|
||||
'assignee_user_id' => $assignee?->id,
|
||||
'description' => 'Seeded backlog item for demonstrating the first ticket workflow.',
|
||||
'story_points' => 3,
|
||||
@@ -53,7 +57,11 @@ class TicketSeeder extends Seeder
|
||||
], $author);
|
||||
|
||||
$createProjectTicket->handle($project, [
|
||||
'acceptance_criteria' => 'Duplicate titles must be allowed for project-local ticket keys.',
|
||||
'acceptance_criteria' => [[
|
||||
'checked' => false,
|
||||
'indent' => 0,
|
||||
'text' => 'Duplicate titles must be allowed for project-local ticket keys.',
|
||||
]],
|
||||
'assignee_user_id' => null,
|
||||
'description' => 'Second seeded backlog item using the same title on purpose.',
|
||||
'story_points' => 5,
|
||||
|
||||
@@ -136,7 +136,7 @@ class TicketApiTest extends TestCase
|
||||
|
||||
$this->withApplicationKey($writer)
|
||||
->patchJson(route('api.projects.tickets.update', [$project, $ticket]), [
|
||||
'acceptance_criteria' => $ticket->acceptance_criteria,
|
||||
'acceptance_criteria' => [],
|
||||
'blocked_comment' => $ticket->blocked_comment,
|
||||
'description' => $ticket->description,
|
||||
'is_blocked' => $ticket->is_blocked,
|
||||
|
||||
@@ -110,7 +110,11 @@ class TicketCreatedNotificationDispatchTest extends TestCase
|
||||
|
||||
$response = $this->actingAs($user)
|
||||
->post(route('projects.tickets.store', $project), [
|
||||
'acceptance_criteria' => '- [ ] Ship it.',
|
||||
'acceptance_criteria' => [[
|
||||
'checked' => false,
|
||||
'indent' => 0,
|
||||
'text' => 'Ship it.',
|
||||
]],
|
||||
'assignee_user_id' => null,
|
||||
'description' => 'Created from notification dispatch test.',
|
||||
'story_points' => 3,
|
||||
|
||||
@@ -84,7 +84,11 @@ class ProjectBacklogTest extends TestCase
|
||||
->post(route('projects.tickets.store', $project), [
|
||||
'title' => 'New ticket',
|
||||
'description' => 'Description',
|
||||
'acceptance_criteria' => '- [ ] Criteria',
|
||||
'acceptance_criteria' => [[
|
||||
'checked' => false,
|
||||
'indent' => 0,
|
||||
'text' => 'Criteria',
|
||||
]],
|
||||
'ticket_type_id' => $project->ticketTypes()->firstOrFail()->id,
|
||||
'ticket_priority_id' => $priority->id,
|
||||
'ticket_status_id' => $backlogStatus->id,
|
||||
@@ -118,7 +122,11 @@ class ProjectBacklogTest extends TestCase
|
||||
$payload = [
|
||||
'title' => 'Same title',
|
||||
'description' => 'Description',
|
||||
'acceptance_criteria' => '- [ ] Criteria',
|
||||
'acceptance_criteria' => [[
|
||||
'checked' => false,
|
||||
'indent' => 0,
|
||||
'text' => 'Criteria',
|
||||
]],
|
||||
'assignee_user_id' => $assignee->id,
|
||||
'story_points' => 3,
|
||||
'ticket_type_id' => $project->ticketTypes()->firstOrFail()->id,
|
||||
@@ -192,7 +200,11 @@ class ProjectBacklogTest extends TestCase
|
||||
->post(route('projects.tickets.store', $project), [
|
||||
'title' => 'New ticket',
|
||||
'description' => 'Description',
|
||||
'acceptance_criteria' => '- [ ] Criteria',
|
||||
'acceptance_criteria' => [[
|
||||
'checked' => false,
|
||||
'indent' => 0,
|
||||
'text' => 'Criteria',
|
||||
]],
|
||||
'ticket_type_id' => $project->ticketTypes()->firstOrFail()->id,
|
||||
'ticket_priority_id' => $project->ticketPriorities()->firstOrFail()->id,
|
||||
'ticket_status_id' => $boardStatus->id,
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
namespace Tests\Feature\Projects;
|
||||
|
||||
use App\Actions\ProjectTicketMetadata\InitializeProjectTicketMetadata;
|
||||
use App\Models\AcceptanceCriterion;
|
||||
use App\Models\BoardColumn;
|
||||
use App\Models\Project;
|
||||
use App\Models\RepositoryConnection;
|
||||
@@ -56,10 +57,7 @@ class ProjectTicketControllerTest extends TestCase
|
||||
'position' => 1,
|
||||
]);
|
||||
|
||||
$legacyAcceptanceCriteria = "- [ ] User can sign in\n - [x] Nested verification";
|
||||
|
||||
$ticket = Ticket::factory()->create([
|
||||
'acceptance_criteria' => $legacyAcceptanceCriteria,
|
||||
'assignee_user_id' => $assignee->id,
|
||||
'author_user_id' => $author->id,
|
||||
'project_id' => $project->id,
|
||||
@@ -70,6 +68,20 @@ class ProjectTicketControllerTest extends TestCase
|
||||
'ticket_type_id' => $this->nonParentType($project)->id,
|
||||
'title' => 'Ticket detail test',
|
||||
]);
|
||||
$rootCriterion = AcceptanceCriterion::query()->create([
|
||||
'ticket_id' => $ticket->id,
|
||||
'parent_acceptance_criterion_id' => null,
|
||||
'position' => 0,
|
||||
'text' => 'User can sign in',
|
||||
'is_checked' => false,
|
||||
]);
|
||||
AcceptanceCriterion::query()->create([
|
||||
'ticket_id' => $ticket->id,
|
||||
'parent_acceptance_criterion_id' => $rootCriterion->id,
|
||||
'position' => 0,
|
||||
'text' => 'Nested verification',
|
||||
'is_checked' => true,
|
||||
]);
|
||||
$linkedTicket = Ticket::factory()->create([
|
||||
'author_user_id' => $author->id,
|
||||
'project_id' => $project->id,
|
||||
@@ -348,7 +360,11 @@ class ProjectTicketControllerTest extends TestCase
|
||||
|
||||
$this->actingAs($editor)
|
||||
->post(route('projects.tickets.store', $project), [
|
||||
'acceptance_criteria' => '- [ ] The ticket exists.',
|
||||
'acceptance_criteria' => [[
|
||||
'checked' => false,
|
||||
'indent' => 0,
|
||||
'text' => 'The ticket exists.',
|
||||
]],
|
||||
'assignee_user_id' => $assignee->id,
|
||||
'description' => 'Created from the dedicated page.',
|
||||
'story_points' => 5,
|
||||
@@ -446,7 +462,6 @@ class ProjectTicketControllerTest extends TestCase
|
||||
]);
|
||||
|
||||
$ticket = Ticket::factory()->create([
|
||||
'acceptance_criteria' => '- [ ] Old acceptance criteria',
|
||||
'author_user_id' => $author->id,
|
||||
'description' => 'Old description',
|
||||
'project_id' => $project->id,
|
||||
@@ -515,7 +530,6 @@ class ProjectTicketControllerTest extends TestCase
|
||||
'id' => $ticket->id,
|
||||
'title' => 'Updated title',
|
||||
'description' => 'Updated description',
|
||||
'acceptance_criteria' => "- [ ] Shows success banner\n - [x] Includes nested confirmations",
|
||||
'assignee_user_id' => $assignee->id,
|
||||
'story_points' => 8,
|
||||
'ticket_type_id' => $this->parentType($project)->id,
|
||||
@@ -526,10 +540,7 @@ class ProjectTicketControllerTest extends TestCase
|
||||
|
||||
$this->assertEquals(
|
||||
$updatedAcceptanceCriteria,
|
||||
\App\Actions\Tickets\Support\AcceptanceCriteriaChecklist::fromModels(
|
||||
$ticket->acceptanceCriteria,
|
||||
$ticket->acceptance_criteria,
|
||||
),
|
||||
\App\Actions\Tickets\Support\AcceptanceCriteriaChecklist::fromModels($ticket->acceptanceCriteria),
|
||||
);
|
||||
$this->assertDatabaseHas('acceptance_criteria', [
|
||||
'ticket_id' => $ticket->id,
|
||||
@@ -968,7 +979,9 @@ class ProjectTicketControllerTest extends TestCase
|
||||
private function updatePayload(Ticket $ticket, array $overrides = []): array
|
||||
{
|
||||
return array_merge([
|
||||
'acceptance_criteria' => $ticket->acceptance_criteria,
|
||||
'acceptance_criteria' => \App\Actions\Tickets\Support\AcceptanceCriteriaChecklist::fromModels(
|
||||
$ticket->acceptanceCriteria()->get(),
|
||||
),
|
||||
'assignee_user_id' => $ticket->assignee_user_id,
|
||||
'author_user_id' => $ticket->author_user_id,
|
||||
'blocked_comment' => $ticket->blocked_comment,
|
||||
|
||||
Reference in New Issue
Block a user