diff --git a/migrations/Version20260903120000.php b/migrations/Version20260903120000.php new file mode 100644 index 000000000..d9c730301 --- /dev/null +++ b/migrations/Version20260903120000.php @@ -0,0 +1,46 @@ +addSql('ALTER TABLE part_custom_states ADD color VARCHAR(20) DEFAULT NULL'); + } + + public function mySQLDown(Schema $schema): void + { + $this->addSql('ALTER TABLE part_custom_states DROP COLUMN color'); + } + + public function sqLiteUp(Schema $schema): void + { + $this->addSql('ALTER TABLE part_custom_states ADD COLUMN color VARCHAR(20) DEFAULT NULL'); + } + + public function sqLiteDown(Schema $schema): void + { + $this->addSql('ALTER TABLE part_custom_states DROP COLUMN color'); + } + + public function postgreSQLUp(Schema $schema): void + { + $this->addSql('ALTER TABLE part_custom_states ADD color VARCHAR(20) DEFAULT NULL'); + } + + public function postgreSQLDown(Schema $schema): void + { + $this->addSql('ALTER TABLE part_custom_states DROP COLUMN color'); + } +} diff --git a/src/DataTables/Helpers/PartDataTableHelper.php b/src/DataTables/Helpers/PartDataTableHelper.php index 065e1201c..80a962e06 100644 --- a/src/DataTables/Helpers/PartDataTableHelper.php +++ b/src/DataTables/Helpers/PartDataTableHelper.php @@ -27,6 +27,7 @@ use App\Entity\ProjectSystem\Project; use App\Entity\Attachments\Attachment; use App\Entity\Parts\Part; +use App\Entity\Parts\PartCustomState; use App\Services\Attachments\AttachmentURLGenerator; use App\Services\Attachments\PartPreviewGenerator; use App\Services\EntityURLGenerator; @@ -170,6 +171,22 @@ public function renderEdaStatus(Part $context): string return sprintf('%s', $editUrl, $statusIcon); } + /** + * Renders the custom state of a part as the colored badge it is configured with. + * Returns an empty string if the part has no custom state. + */ + public function renderPartCustomState(?PartCustomState $state): string + { + if ($state === null) { + return ''; + } + + return sprintf('%s', + htmlspecialchars($state->getBadgeClass()), + htmlspecialchars($state->getName()) + ); + } + public function renderAmount(Part $context): string { $amount = $context->getAmountSum(); diff --git a/src/DataTables/PartsDataTable.php b/src/DataTables/PartsDataTable.php index c15468de5..a43a1939e 100644 --- a/src/DataTables/PartsDataTable.php +++ b/src/DataTables/PartsDataTable.php @@ -198,18 +198,11 @@ public function configure(DataTable $dataTable, array $options): void return $tmp; } ]) - ->add('partCustomState', TextColumn::class, [ + ->add('partCustomState', HTMLColumn::class, [ 'label' => $this->translator->trans('part.table.partCustomState'), 'orderField' => 'NATSORT(_partCustomState.name)', - 'data' => function(Part $context): string { - $partCustomState = $context->getPartCustomState(); - - if ($partCustomState === null) { - return ''; - } - - return $partCustomState->getName(); - } + 'data' => fn(Part $context): string + => $this->partDataTableHelper->renderPartCustomState($context->getPartCustomState()), ]) ->add('addedDate', LocaleDateTimeColumn::class, [ 'label' => $this->translator->trans('part.table.addedDate'), diff --git a/src/DataTables/ProjectBomEntriesDataTable.php b/src/DataTables/ProjectBomEntriesDataTable.php index 0f7af87c9..3245d097e 100644 --- a/src/DataTables/ProjectBomEntriesDataTable.php +++ b/src/DataTables/ProjectBomEntriesDataTable.php @@ -177,6 +177,14 @@ public function configure(DataTable $dataTable, array $options): void }, ]) + ->add('partCustomState', HTMLColumn::class, [ + 'label' => $this->translator->trans('part.table.partCustomState'), + 'orderField' => 'NATSORT(partCustomState.name)', + 'visible' => false, + 'data' => fn (ProjectBOMEntry $context): string + => $this->partDataTableHelper->renderPartCustomState($context->getPart()?->getPartCustomState()), + ]) + ->add('mountnames', HTMLColumn::class, [ 'label' => 'project.bom.mountnames', 'data' => function (ProjectBOMEntry $context) { diff --git a/src/Entity/Parts/PartCustomState.php b/src/Entity/Parts/PartCustomState.php index 29a96c007..396f7100d 100644 --- a/src/Entity/Parts/PartCustomState.php +++ b/src/Entity/Parts/PartCustomState.php @@ -50,6 +50,7 @@ use App\State\Mcp\ListStructuralElementsProcessor; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\Collections\Collection; +use Doctrine\DBAL\Types\Types; use Doctrine\ORM\Mapping as ORM; use Symfony\Component\Serializer\Attribute\Groups; use Symfony\Component\Validator\Constraints as Assert; @@ -108,6 +109,14 @@ class PartCustomState extends AbstractPartsContainingDBElement #[Groups(['part_custom_state:read', 'part_custom_state:write', 'full', 'import'])] protected string $comment = ''; + /** + * @var PartCustomStateColor|null The semantic color this state is rendered as a badge with. + * Null keeps the default, uncolored appearance Part-DB used before this field existed. + */ + #[ORM\Column(type: Types::STRING, length: 20, nullable: true, enumType: PartCustomStateColor::class)] + #[Groups(['part_custom_state:read', 'part_custom_state:write', 'full', 'import'])] + protected ?PartCustomStateColor $color = null; + #[ORM\OneToMany(targetEntity: self::class, mappedBy: 'parent', cascade: ['persist'])] #[ORM\OrderBy(['name' => 'ASC'])] protected Collection $children; @@ -152,4 +161,26 @@ public function __construct() $this->attachments = new ArrayCollection(); $this->parameters = new ArrayCollection(); } + + public function getColor(): ?PartCustomStateColor + { + return $this->color; + } + + public function setColor(?PartCustomStateColor $color): self + { + $this->color = $color; + + return $this; + } + + /** + * Returns the CSS class this state is rendered as a badge with, everywhere it is shown. + * Without a configured color this is the color Part-DB used before the color existed, so an unconfigured + * state keeps looking exactly the way it did. + */ + public function getBadgeClass(): string + { + return $this->color?->toBadgeClass() ?? 'bg-primary'; + } } diff --git a/src/Entity/Parts/PartCustomStateColor.php b/src/Entity/Parts/PartCustomStateColor.php new file mode 100644 index 000000000..d5093aed1 --- /dev/null +++ b/src/Entity/Parts/PartCustomStateColor.php @@ -0,0 +1,53 @@ +. + */ + +declare(strict_types=1); + +namespace App\Entity\Parts; + +/** + * The semantic Bootstrap color a PartCustomState can be rendered with. + * This is a closed whitelist: no free-form CSS classes or colors can be stored. + */ +enum PartCustomStateColor: string +{ + case PRIMARY = 'primary'; + case SECONDARY = 'secondary'; + case INFO = 'info'; + case SUCCESS = 'success'; + case WARNING = 'warning'; + case DANGER = 'danger'; + case LIGHT = 'light'; + case DARK = 'dark'; + + public function toTranslationKey(): string + { + return 'part_custom_state.color.' . $this->value; + } + + /** + * Maps this color to the fixed Bootstrap badge class it is rendered with. + * This is the only place that translates a stored color into a CSS class. + */ + public function toBadgeClass(): string + { + return 'text-bg-' . $this->value; + } +} diff --git a/src/Form/AdminPages/PartCustomStateAdminForm.php b/src/Form/AdminPages/PartCustomStateAdminForm.php index b8bb2815e..cba043e9c 100644 --- a/src/Form/AdminPages/PartCustomStateAdminForm.php +++ b/src/Form/AdminPages/PartCustomStateAdminForm.php @@ -22,6 +22,29 @@ namespace App\Form\AdminPages; +use App\Entity\Base\AbstractNamedDBElement; +use App\Entity\Parts\PartCustomState; +use App\Entity\Parts\PartCustomStateColor; +use Symfony\Component\Form\Extension\Core\Type\EnumType; +use Symfony\Component\Form\FormBuilderInterface; + class PartCustomStateAdminForm extends BaseEntityAdminForm { + protected function additionalFormElements(FormBuilderInterface $builder, array $options, AbstractNamedDBElement $entity): void + { + if (!$entity instanceof PartCustomState) { + return; + } + + $is_new = null === $entity->getID(); + + $builder->add('color', EnumType::class, [ + 'class' => PartCustomStateColor::class, + 'choice_label' => fn (PartCustomStateColor $color) => $color->toTranslationKey(), + 'required' => false, + 'label' => 'part_custom_state.color.label', + 'help' => 'part_custom_state.color.help', + 'disabled' => !$this->security->isGranted($is_new ? 'create' : 'edit', $entity), + ]); + } } diff --git a/src/Serializer/APIPlatform/SkippableItemNormalizer.php b/src/Serializer/APIPlatform/SkippableItemNormalizer.php index 618736152..169520e14 100644 --- a/src/Serializer/APIPlatform/SkippableItemNormalizer.php +++ b/src/Serializer/APIPlatform/SkippableItemNormalizer.php @@ -26,6 +26,7 @@ use ApiPlatform\Metadata\Exception\InvalidArgumentException; use ApiPlatform\Metadata\Exception\ItemNotFoundException; use ApiPlatform\Metadata\IriConverterInterface; +use ApiPlatform\Metadata\ResourceClassResolverInterface; use ApiPlatform\Serializer\ItemNormalizer; use Symfony\Component\DependencyInjection\Attribute\AsDecorator; use Symfony\Component\Serializer\Exception\NotNormalizableValueException; @@ -53,6 +54,7 @@ class SkippableItemNormalizer implements NormalizerInterface, DenormalizerInterf public function __construct( private readonly ItemNormalizer $inner, private readonly IriConverterInterface $iriConverter, + private readonly ResourceClassResolverInterface $resourceClassResolver, ) { } @@ -63,7 +65,13 @@ public function denormalize(mixed $data, string $type, ?string $format = null, a // check (line 271). For abstract resource classes with a discriminator map (e.g. Attachment), this // fails because the array has no _type key. Fix by resolving IRI strings directly. // See: https://github.com/Part-DB/Part-DB-server/issues/1370 - if (is_string($data) || (is_array($data) && isset($data['@id']) && is_string($data['@id']))) { + // + // $type must actually be an API resource for this to make sense: this normalizer also runs for plain + // value objects (e.g. backed enums) nested inside a resource, and a string value there is the enum's + // scalar value, not an IRI - treating it as one silently swallows the value instead of letting the + // regular (enum) normalizer handle it. + if ($this->resourceClassResolver->isResourceClass($type) + && (is_string($data) || (is_array($data) && isset($data['@id']) && is_string($data['@id'])))) { if (is_array($data)) { $iri = $data['@id']; } else { diff --git a/templates/admin/part_custom_state_admin.html.twig b/templates/admin/part_custom_state_admin.html.twig index 9d8576468..dac799e00 100644 --- a/templates/admin/part_custom_state_admin.html.twig +++ b/templates/admin/part_custom_state_admin.html.twig @@ -12,3 +12,7 @@ {% trans %}part_custom_state.new{% endtrans %} {% endblock %} +{% block additional_controls %} + {{ form_row(form.color) }} +{% endblock %} + diff --git a/templates/parts/info/_sidebar.html.twig b/templates/parts/info/_sidebar.html.twig index 120602418..06d2a3b3e 100644 --- a/templates/parts/info/_sidebar.html.twig +++ b/templates/parts/info/_sidebar.html.twig @@ -47,7 +47,7 @@ {% if part.partCustomState is not null %}
- {{ part.partCustomState.name }} + {{ part.partCustomState.name }} {% if part.partCustomState is not null and part.partCustomState.masterPictureAttachment and attachment_manager.fileExisting(part.partCustomState.masterPictureAttachment) %}
diff --git a/tests/API/Endpoints/PartCustomStateEndpointTest.php b/tests/API/Endpoints/PartCustomStateEndpointTest.php index 8d1253f30..def1d991a 100644 --- a/tests/API/Endpoints/PartCustomStateEndpointTest.php +++ b/tests/API/Endpoints/PartCustomStateEndpointTest.php @@ -66,4 +66,22 @@ public function testDeleteItem(): void { $this->_testDeleteItem(4); } + + public function testColorIsWritableAndReadable(): void + { + $this->_testPatchItem(5, [ + 'color' => 'warning', + ]); + self::assertJsonContains([ + 'color' => 'warning', + ]); + + //Unsetting the color must be possible again (back to the default, uncolored appearance). + //Like every other null-valued field in this API, an unset color is omitted from the response entirely + //rather than being serialized as an explicit "color": null (see e.g. InfoProviderEndpointTest). + $response = $this->_testPatchItem(5, [ + 'color' => null, + ]); + self::assertArrayNotHasKey('color', json_decode($response->getContent(), true)); + } } diff --git a/tests/Entity/Parts/PartCustomStateColorTest.php b/tests/Entity/Parts/PartCustomStateColorTest.php new file mode 100644 index 000000000..708e1fe4c --- /dev/null +++ b/tests/Entity/Parts/PartCustomStateColorTest.php @@ -0,0 +1,88 @@ +. + */ +namespace App\Tests\Entity\Parts; + +use App\Entity\Parts\PartCustomState; +use App\Entity\Parts\PartCustomStateColor; +use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\TestCase; + +final class PartCustomStateColorTest extends TestCase +{ + public static function colorProvider(): array + { + return [ + [PartCustomStateColor::PRIMARY, 'text-bg-primary'], + [PartCustomStateColor::SECONDARY, 'text-bg-secondary'], + [PartCustomStateColor::INFO, 'text-bg-info'], + [PartCustomStateColor::SUCCESS, 'text-bg-success'], + [PartCustomStateColor::WARNING, 'text-bg-warning'], + [PartCustomStateColor::DANGER, 'text-bg-danger'], + [PartCustomStateColor::LIGHT, 'text-bg-light'], + [PartCustomStateColor::DARK, 'text-bg-dark'], + ]; + } + + #[DataProvider('colorProvider')] + public function testToBadgeClassMapsEveryColorToAFixedClass(PartCustomStateColor $color, string $expectedClass): void + { + $this->assertSame($expectedClass, $color->toBadgeClass()); + } + + public function testUnknownColorValueIsRejected(): void + { + $this->assertNull(PartCustomStateColor::tryFrom('not-a-real-color')); + } + + public function testPartCustomStateDefaultsToNoColor(): void + { + $state = new PartCustomState(); + + $this->assertNull($state->getColor()); + } + + public function testPartCustomStateColorCanBeSetAndRetrieved(): void + { + $state = new PartCustomState(); + $state->setColor(PartCustomStateColor::WARNING); + + $this->assertSame(PartCustomStateColor::WARNING, $state->getColor()); + + $state->setColor(null); + $this->assertNull($state->getColor()); + } + + public function testBadgeClassFallsBackToThePreviousAppearance(): void + { + //A state without a configured color has to keep looking exactly the way it did before colors existed + $this->assertSame('bg-primary', (new PartCustomState())->getBadgeClass()); + } + + #[DataProvider('colorProvider')] + public function testBadgeClassUsesTheConfiguredColor(PartCustomStateColor $color, string $expectedClass): void + { + $state = (new PartCustomState())->setColor($color); + + $this->assertSame($expectedClass, $state->getBadgeClass()); + } +} diff --git a/tests/Serializer/APIPlatform/SkippableItemNormalizerTest.php b/tests/Serializer/APIPlatform/SkippableItemNormalizerTest.php new file mode 100644 index 000000000..83e3e71ab --- /dev/null +++ b/tests/Serializer/APIPlatform/SkippableItemNormalizerTest.php @@ -0,0 +1,111 @@ +. + */ +namespace App\Tests\Serializer\APIPlatform; + +use ApiPlatform\Metadata\IriConverterInterface; +use ApiPlatform\Metadata\ResourceClassResolverInterface; +use ApiPlatform\Serializer\ItemNormalizer; +use App\Entity\Attachments\Attachment; +use App\Serializer\APIPlatform\SkippableItemNormalizer; +use PHPUnit\Framework\TestCase; + +final class SkippableItemNormalizerTest extends TestCase +{ + private ItemNormalizer&\PHPUnit\Framework\MockObject\MockObject $inner; + private IriConverterInterface&\PHPUnit\Framework\MockObject\MockObject $iriConverter; + private ResourceClassResolverInterface&\PHPUnit\Framework\MockObject\MockObject $resourceClassResolver; + private SkippableItemNormalizer $normalizer; + + protected function setUp(): void + { + $this->inner = $this->createMock(ItemNormalizer::class); + $this->iriConverter = $this->createMock(IriConverterInterface::class); + $this->resourceClassResolver = $this->createMock(ResourceClassResolverInterface::class); + $this->normalizer = new SkippableItemNormalizer($this->inner, $this->iriConverter, $this->resourceClassResolver); + } + + public function testStringIsResolvedAsIriForResourceClass(): void + { + //Regression guard for https://github.com/Part-DB/Part-DB-server/issues/1370: an IRI string for a resource + //class (like Attachment, which has a discriminator map) must still be resolved via the IriConverter. + $this->resourceClassResolver->method('isResourceClass')->with(Attachment::class)->willReturn(true); + + $attachment = $this->createMock(Attachment::class); + $this->iriConverter->expects($this->once()) + ->method('getResourceFromIri') + ->with('/api/attachments/1') + ->willReturn($attachment); + + $result = $this->normalizer->denormalize('/api/attachments/1', Attachment::class); + + $this->assertSame($attachment, $result); + } + + public function testStringIsNotResolvedAsIriForNonResourceClass(): void + { + //A backed enum (or any other plain value object) is not an API resource, so a string value denormalized + //into it is the enum's scalar value, not an IRI - it must be passed through to the inner normalizer + //(which delegates to Symfony's BackedEnumNormalizer) instead of being swallowed by a failed IRI lookup. + $this->resourceClassResolver->method('isResourceClass')->willReturn(false); + + $this->iriConverter->expects($this->never())->method('getResourceFromIri'); + $this->inner->expects($this->once()) + ->method('denormalize') + ->with('warning', 'SomeEnum', null, []) + ->willReturn('warning-denormalized'); + + $result = $this->normalizer->denormalize('warning', 'SomeEnum'); + + $this->assertSame('warning-denormalized', $result); + } + + public function testArrayWithIriIsResolvedForResourceClass(): void + { + $this->resourceClassResolver->method('isResourceClass')->with(Attachment::class)->willReturn(true); + + $attachment = $this->createMock(Attachment::class); + $this->iriConverter->expects($this->once()) + ->method('getResourceFromIri') + ->with('/api/attachments/1') + ->willReturn($attachment); + + $result = $this->normalizer->denormalize(['@id' => '/api/attachments/1'], Attachment::class); + + $this->assertSame($attachment, $result); + } + + public function testArrayWithoutIriIsPassedThrough(): void + { + $this->resourceClassResolver->method('isResourceClass')->willReturn(true); + + $this->iriConverter->expects($this->never())->method('getResourceFromIri'); + $this->inner->expects($this->once()) + ->method('denormalize') + ->with(['name' => 'Test'], Attachment::class, null, []) + ->willReturn('denormalized'); + + $result = $this->normalizer->denormalize(['name' => 'Test'], Attachment::class); + + $this->assertSame('denormalized', $result); + } +} diff --git a/translations/messages.en.xlf b/translations/messages.en.xlf index f7a9d4e9f..e738a576a 100644 --- a/translations/messages.en.xlf +++ b/translations/messages.en.xlf @@ -15110,5 +15110,65 @@ Buerklin-API Authentication server: Edit BOM entry #%id% of project + + + part_custom_state.color.label + Color + + + + + part_custom_state.color.help + Optional semantic color this custom state is shown with wherever it appears as a badge (e.g. on the part detail page and in part tables). Without a color, the previous default appearance is used. + + + + + part_custom_state.color.primary + Primary + + + + + part_custom_state.color.secondary + Secondary + + + + + part_custom_state.color.info + Info + + + + + part_custom_state.color.success + Success + + + + + part_custom_state.color.warning + Warning + + + + + part_custom_state.color.danger + Danger + + + + + part_custom_state.color.light + Light + + + + + part_custom_state.color.dark + Dark + +