From 6c03f9f15e2647b31cba1f456531bbc94b8fab2b Mon Sep 17 00:00:00 2001 From: Matt Date: Mon, 17 Aug 2026 00:09:45 +0200 Subject: [PATCH 1/5] Add global parameter definitions with choice inputs --- .../parameters_autocomplete_controller.js | 104 +++++- config/permissions.yaml | 7 +- migrations/Version20260816190000.php | 170 +++++++++ .../ParameterDefinitionDeleteProcessor.php | 58 ++++ .../AdminPages/BaseAdminController.php | 70 ++-- .../ParameterDefinitionController.php | 92 +++++ src/Controller/TypeaheadController.php | 51 +++ src/Entity/LogSystem/LogTargetType.php | 5 +- src/Entity/Parameters/AbstractParameter.php | 213 +++++++++++- src/Entity/Parameters/ParameterDefinition.php | 322 ++++++++++++++++++ src/Entity/UserSystem/PermissionData.php | 2 +- src/Form/AdminPages/BaseEntityAdminForm.php | 41 +-- .../ParameterDefinitionAdminForm.php | 84 +++++ src/Form/Filters/LogFilterType.php | 1 + src/Form/ParameterType.php | 139 +++++++- .../ParameterDefinitionRepository.php | 62 ++++ src/Repository/ParameterRepository.php | 11 + src/Security/Voter/StructureVoter.php | 4 +- src/Services/ElementTypes.php | 5 + src/Services/EntityURLGenerator.php | 7 + src/Services/LogSystem/TimeTravel.php | 16 + src/Services/Trees/ToolsTreeBuilder.php | 7 + .../UserSystem/PermissionPresetsHelper.php | 1 + .../UserSystem/PermissionSchemaUpdater.php | 20 ++ templates/admin/base_admin.html.twig | 22 +- .../parameter_definition_admin.html.twig | 24 ++ .../parts/edit/edit_form_styles.html.twig | 11 +- .../ParameterDefinitionsEndpointTest.php | 118 +++++++ .../API/Endpoints/ParametersEndpointTest.php | 57 +++- .../ParameterDefinitionControllerTest.php | 282 +++++++++++++++ tests/Controller/TypeaheadControllerTest.php | 64 ++++ .../ParameterDefinitionSchemaTest.php | 47 +++ tests/Entity/LogSystem/LogTargetTypeTest.php | 2 + .../ParameterDefinitionDoctrineTest.php | 131 +++++++ .../Parameters/ParameterDefinitionTest.php | 128 +++++++ tests/Form/ParameterTypeTest.php | 222 ++++++++++++ .../ParameterDefinitionMigrationTest.php | 92 +++++ .../ImportExportSystem/EntityExporterTest.php | 30 ++ .../ImportExportSystem/EntityImporterTest.php | 26 ++ tests/Services/LogSystem/TimeTravelTest.php | 69 ++++ .../PermissionPresetsHelperTest.php | 22 ++ .../PermissionSchemaUpdaterTest.php | 16 + translations/messages.en.xlf | 42 +++ translations/messages.fr.xlf | 42 +++ translations/validators.en.xlf | 3 + translations/validators.fr.xlf | 3 + 46 files changed, 2849 insertions(+), 96 deletions(-) create mode 100644 migrations/Version20260816190000.php create mode 100644 src/ApiPlatform/ParameterDefinitionDeleteProcessor.php create mode 100644 src/Controller/AdminPages/ParameterDefinitionController.php create mode 100644 src/Entity/Parameters/ParameterDefinition.php create mode 100644 src/Form/AdminPages/ParameterDefinitionAdminForm.php create mode 100644 src/Repository/ParameterDefinitionRepository.php create mode 100644 templates/admin/parameter_definition_admin.html.twig create mode 100644 tests/API/Endpoints/ParameterDefinitionsEndpointTest.php create mode 100644 tests/Controller/AdminPages/ParameterDefinitionControllerTest.php create mode 100644 tests/Doctrine/ParameterDefinitionSchemaTest.php create mode 100644 tests/Entity/Parameters/ParameterDefinitionDoctrineTest.php create mode 100644 tests/Entity/Parameters/ParameterDefinitionTest.php create mode 100644 tests/Form/ParameterTypeTest.php create mode 100644 tests/Migration/ParameterDefinitionMigrationTest.php diff --git a/assets/controllers/pages/parameters_autocomplete_controller.js b/assets/controllers/pages/parameters_autocomplete_controller.js index 8a1e34f72..2b193cd85 100644 --- a/assets/controllers/pages/parameters_autocomplete_controller.js +++ b/assets/controllers/pages/parameters_autocomplete_controller.js @@ -38,9 +38,10 @@ export default class extends Controller url: String, } - static targets = ["name", "symbol", "unit"] + static targets = ["name", "symbol", "unit", "valueText", "definition"] _tomSelect; + _initialized = false; onItemAdd(value, item) { //Retrieve the unit and symbol from the item @@ -57,6 +58,91 @@ export default class extends Controller //Trigger input event to update the preview this.unitTarget.dispatchEvent(new Event('input')); } + + // TomSelect emits onItemAdd for the value already present while initializing an existing row. The server has + // rendered that row from its persisted definition, so only an explicit user selection may change the link. + if (!this._initialized || !this.hasDefinitionTarget || !this.hasValueTextTarget) { + return; + } + + const definitionId = item.dataset.definitionId; + if (definitionId === undefined || !/^\d+$/.test(definitionId) || Number(definitionId) < 1) { + this.setDefinition(null); + this.applyInputDefinition('text', []); + + return; + } + + let choices = []; + if (item.dataset.choices) { + try { + choices = JSON.parse(item.dataset.choices); + } catch (_) { + choices = []; + } + } + + this.setDefinition(definitionId, item.dataset.definitionName ?? value); + this.applyInputDefinition(item.dataset.inputType ?? 'text', choices); + } + + onItemRemove() { + if (!this._initialized || !this.hasDefinitionTarget || !this.hasValueTextTarget) { + return; + } + + this.setDefinition(null); + this.applyInputDefinition('text', []); + } + + setDefinition(definitionId, name = '') { + this.definitionTarget.replaceChildren(); + + const emptyOption = new Option('', ''); + this.definitionTarget.add(emptyOption); + + if (definitionId !== null) { + const option = new Option(name, definitionId, true, true); + this.definitionTarget.add(option); + this.definitionTarget.value = definitionId; + } else { + this.definitionTarget.value = ''; + } + + this.definitionTarget.dispatchEvent(new Event('change', {bubbles: true})); + } + + applyInputDefinition(inputType, choices) { + const oldElement = this.valueTextTarget; + const currentValue = oldElement.value; + const useChoice = inputType === 'choice' && Array.isArray(choices); + const newElement = document.createElement(useChoice ? 'select' : 'input'); + + for (const attribute of oldElement.attributes) { + if (attribute.name !== 'type') { + newElement.setAttribute(attribute.name, attribute.value); + } + } + + if (useChoice) { + newElement.classList.remove('form-control', 'form-control-sm'); + newElement.classList.add('form-select', 'form-select-sm'); + newElement.add(new Option('', '')); + + for (const choice of choices) { + newElement.add(new Option(choice, choice)); + } + + newElement.value = choices.includes(currentValue) ? currentValue : ''; + } else { + newElement.type = 'text'; + newElement.classList.remove('form-select', 'form-select-sm'); + newElement.classList.add('form-control', 'form-control-sm'); + newElement.value = currentValue; + } + + oldElement.replaceWith(newElement); + newElement.dispatchEvent(new Event('change', {bubbles: true})); } connect() { @@ -80,6 +166,10 @@ export default class extends Controller valueField: "name", clearAfterSelect: true, onItemAdd: this.onItemAdd.bind(this), + onItemRemove: this.onItemRemove.bind(this), + onInitialize: () => { + this._initialized = true; + }, render: { option: (data, escape) => { let tmp = '
' @@ -110,6 +200,16 @@ export default class extends Controller if (data.symbol !== undefined) { element.dataset.symbol = data.symbol; } + if (data.definition_id !== undefined) { + element.dataset.definitionId = data.definition_id; + element.dataset.definitionName = data.name; + } + if (data.input_type !== undefined) { + element.dataset.inputType = data.input_type; + } + if (data.choices !== undefined) { + element.dataset.choices = JSON.stringify(data.choices); + } return element.outerHTML; } @@ -138,6 +238,6 @@ export default class extends Controller disconnect() { super.disconnect(); //Destroy the TomSelect instance - this._tomSelect.destroy(); + this._tomSelect?.destroy(); } } diff --git a/config/permissions.yaml b/config/permissions.yaml index a7a35c0ae..a65c3f560 100644 --- a/config/permissions.yaml +++ b/config/permissions.yaml @@ -24,7 +24,8 @@ perms: # Here comes a list with all Permission names (they have a perm_[name] co label: "perm.read" # If a part can be read by a user, he can also see all the datastructures (except devices) alsoSet: ['storelocations.read', 'footprints.read', 'categories.read', 'suppliers.read', 'manufacturers.read', - 'currencies.read', 'attachment_types.read', 'measurement_units.read', 'part_custom_states.read'] + 'currencies.read', 'attachment_types.read', 'measurement_units.read', 'part_custom_states.read', + 'parameter_definitions.read'] apiTokenRole: ROLE_API_READ_ONLY edit: label: "perm.edit" @@ -140,6 +141,10 @@ perms: # Here comes a list with all Permission names (they have a perm_[name] co <<: *PART_CONTAINING label: "[[Part_custom_state]]" + parameter_definitions: + <<: *PART_CONTAINING + label: "[[Parameter_definition]]" + tools: label: "perm.part.tools" operations: diff --git a/migrations/Version20260816190000.php b/migrations/Version20260816190000.php new file mode 100644 index 000000000..5c1f44ec2 --- /dev/null +++ b/migrations/Version20260816190000.php @@ -0,0 +1,170 @@ +addSql(<<<'SQL' + CREATE TABLE parameter_definitions ( + id INT AUTO_INCREMENT NOT NULL, + name VARCHAR(255) NOT NULL, + normalized_name VARCHAR(255) NOT NULL, + input_type VARCHAR(16) DEFAULT 'text' NOT NULL, + choices JSON DEFAULT NULL, + symbol VARCHAR(20) NOT NULL, + unit VARCHAR(50) NOT NULL, + last_modified DATETIME DEFAULT CURRENT_TIMESTAMP NOT NULL, + datetime_added DATETIME DEFAULT CURRENT_TIMESTAMP NOT NULL, + UNIQUE INDEX parameter_definition_normalized_name_unique (normalized_name), + INDEX parameter_definition_name_idx (name), + PRIMARY KEY (id) + ) DEFAULT CHARACTER SET utf8mb4 COLLATE `utf8mb4_unicode_ci` + SQL); + $this->addSql('ALTER TABLE parameters ADD definition_id INT DEFAULT NULL'); + $this->addSql('ALTER TABLE parameters ADD CONSTRAINT FK_69348FED11EA911 FOREIGN KEY (definition_id) REFERENCES parameter_definitions (id) ON DELETE RESTRICT'); + $this->addSql('CREATE INDEX IDX_69348FED11EA911 ON parameters (definition_id)'); + $this->addSql('CREATE INDEX parameter_definition_value_idx ON parameters (definition_id, value_text, type, element_id)'); + } + + public function mySQLDown(Schema $schema): void + { + $this->addSql('ALTER TABLE parameters DROP FOREIGN KEY FK_69348FED11EA911'); + $this->addSql('DROP INDEX IDX_69348FED11EA911 ON parameters'); + $this->addSql('DROP INDEX parameter_definition_value_idx ON parameters'); + $this->addSql('ALTER TABLE parameters DROP definition_id'); + $this->addSql('DROP TABLE parameter_definitions'); + } + + public function sqLiteUp(Schema $schema): void + { + $this->addSql(<<<'SQL' + CREATE TABLE parameter_definitions ( + id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, + name VARCHAR(255) NOT NULL, + normalized_name VARCHAR(255) NOT NULL, + input_type VARCHAR(16) DEFAULT 'text' NOT NULL, + choices CLOB DEFAULT NULL, + symbol VARCHAR(20) NOT NULL, + unit VARCHAR(50) NOT NULL, + last_modified DATETIME DEFAULT CURRENT_TIMESTAMP NOT NULL, + datetime_added DATETIME DEFAULT CURRENT_TIMESTAMP NOT NULL + ) + SQL); + $this->addSql('CREATE INDEX parameter_definition_name_idx ON parameter_definitions (name)'); + $this->addSql('CREATE UNIQUE INDEX parameter_definition_normalized_name_unique ON parameter_definitions (normalized_name)'); + + $this->addSql('CREATE TEMPORARY TABLE __temp__parameters AS SELECT id, symbol, value_min, value_typical, value_max, unit, value_text, param_group, name, last_modified, datetime_added, type, element_id, eda_visibility, eda_symbol_visibility FROM parameters'); + $this->addSql('DROP TABLE parameters'); + $this->addSql(<<<'SQL' + CREATE TABLE parameters ( + id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, + symbol VARCHAR(255) NOT NULL, + value_min DOUBLE PRECISION DEFAULT NULL, + value_typical DOUBLE PRECISION DEFAULT NULL, + value_max DOUBLE PRECISION DEFAULT NULL, + unit VARCHAR(255) NOT NULL, + value_text VARCHAR(255) NOT NULL, + param_group VARCHAR(255) NOT NULL, + name VARCHAR(255) NOT NULL, + last_modified DATETIME DEFAULT CURRENT_TIMESTAMP NOT NULL, + datetime_added DATETIME DEFAULT CURRENT_TIMESTAMP NOT NULL, + type SMALLINT NOT NULL, + element_id INTEGER NOT NULL, + eda_visibility BOOLEAN DEFAULT NULL, + eda_symbol_visibility BOOLEAN DEFAULT NULL, + definition_id INTEGER DEFAULT NULL, + CONSTRAINT FK_69348FED11EA911 FOREIGN KEY (definition_id) REFERENCES parameter_definitions (id) ON DELETE RESTRICT NOT DEFERRABLE INITIALLY IMMEDIATE + ) + SQL); + $this->addSql('INSERT INTO parameters (id, symbol, value_min, value_typical, value_max, unit, value_text, param_group, name, last_modified, datetime_added, type, element_id, eda_visibility, eda_symbol_visibility) SELECT id, symbol, value_min, value_typical, value_max, unit, value_text, param_group, name, last_modified, datetime_added, type, element_id, eda_visibility, eda_symbol_visibility FROM __temp__parameters'); + $this->addSql('DROP TABLE __temp__parameters'); + $this->createSQLiteParameterIndexes(); + } + + public function sqLiteDown(Schema $schema): void + { + $this->addSql('CREATE TEMPORARY TABLE __temp__parameters AS SELECT id, symbol, value_min, value_typical, value_max, unit, value_text, param_group, name, last_modified, datetime_added, type, element_id, eda_visibility, eda_symbol_visibility FROM parameters'); + $this->addSql('DROP TABLE parameters'); + $this->addSql(<<<'SQL' + CREATE TABLE parameters ( + id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, + symbol VARCHAR(255) NOT NULL, + value_min DOUBLE PRECISION DEFAULT NULL, + value_typical DOUBLE PRECISION DEFAULT NULL, + value_max DOUBLE PRECISION DEFAULT NULL, + unit VARCHAR(255) NOT NULL, + value_text VARCHAR(255) NOT NULL, + param_group VARCHAR(255) NOT NULL, + name VARCHAR(255) NOT NULL, + last_modified DATETIME DEFAULT CURRENT_TIMESTAMP NOT NULL, + datetime_added DATETIME DEFAULT CURRENT_TIMESTAMP NOT NULL, + type SMALLINT NOT NULL, + element_id INTEGER NOT NULL, + eda_visibility BOOLEAN DEFAULT NULL, + eda_symbol_visibility BOOLEAN DEFAULT NULL + ) + SQL); + $this->addSql('INSERT INTO parameters (id, symbol, value_min, value_typical, value_max, unit, value_text, param_group, name, last_modified, datetime_added, type, element_id, eda_visibility, eda_symbol_visibility) SELECT id, symbol, value_min, value_typical, value_max, unit, value_text, param_group, name, last_modified, datetime_added, type, element_id, eda_visibility, eda_symbol_visibility FROM __temp__parameters'); + $this->addSql('DROP TABLE __temp__parameters'); + $this->addSql('CREATE INDEX parameter_type_element_idx ON parameters (type, element_id)'); + $this->addSql('CREATE INDEX parameter_group_idx ON parameters (param_group)'); + $this->addSql('CREATE INDEX parameter_name_idx ON parameters (name)'); + $this->addSql('CREATE INDEX IDX_69348FE1F1F2A24 ON parameters (element_id)'); + $this->addSql('DROP TABLE parameter_definitions'); + } + + public function postgreSQLUp(Schema $schema): void + { + $this->addSql(<<<'SQL' + CREATE TABLE parameter_definitions ( + id INT GENERATED BY DEFAULT AS IDENTITY NOT NULL, + name VARCHAR(255) NOT NULL, + normalized_name VARCHAR(255) NOT NULL, + input_type VARCHAR(16) DEFAULT 'text' NOT NULL, + choices JSON DEFAULT NULL, + symbol VARCHAR(20) NOT NULL, + unit VARCHAR(50) NOT NULL, + last_modified TIMESTAMP(0) WITHOUT TIME ZONE DEFAULT CURRENT_TIMESTAMP NOT NULL, + datetime_added TIMESTAMP(0) WITHOUT TIME ZONE DEFAULT CURRENT_TIMESTAMP NOT NULL, + PRIMARY KEY (id) + ) + SQL); + $this->addSql('CREATE INDEX parameter_definition_name_idx ON parameter_definitions (name)'); + $this->addSql('CREATE UNIQUE INDEX parameter_definition_normalized_name_unique ON parameter_definitions (normalized_name)'); + $this->addSql('ALTER TABLE parameters ADD definition_id INT DEFAULT NULL'); + $this->addSql('ALTER TABLE parameters ADD CONSTRAINT FK_69348FED11EA911 FOREIGN KEY (definition_id) REFERENCES parameter_definitions (id) ON DELETE RESTRICT NOT DEFERRABLE INITIALLY IMMEDIATE'); + $this->addSql('CREATE INDEX IDX_69348FED11EA911 ON parameters (definition_id)'); + $this->addSql('CREATE INDEX parameter_definition_value_idx ON parameters (definition_id, value_text, type, element_id)'); + } + + public function postgreSQLDown(Schema $schema): void + { + $this->addSql('ALTER TABLE parameters DROP CONSTRAINT FK_69348FED11EA911'); + $this->addSql('DROP INDEX IDX_69348FED11EA911'); + $this->addSql('DROP INDEX parameter_definition_value_idx'); + $this->addSql('ALTER TABLE parameters DROP definition_id'); + $this->addSql('DROP TABLE parameter_definitions'); + } + + private function createSQLiteParameterIndexes(): void + { + $this->addSql('CREATE INDEX parameter_type_element_idx ON parameters (type, element_id)'); + $this->addSql('CREATE INDEX parameter_group_idx ON parameters (param_group)'); + $this->addSql('CREATE INDEX parameter_name_idx ON parameters (name)'); + $this->addSql('CREATE INDEX IDX_69348FE1F1F2A24 ON parameters (element_id)'); + $this->addSql('CREATE INDEX IDX_69348FED11EA911 ON parameters (definition_id)'); + $this->addSql('CREATE INDEX parameter_definition_value_idx ON parameters (definition_id, value_text, type, element_id)'); + } +} diff --git a/src/ApiPlatform/ParameterDefinitionDeleteProcessor.php b/src/ApiPlatform/ParameterDefinitionDeleteProcessor.php new file mode 100644 index 000000000..cca2879b2 --- /dev/null +++ b/src/ApiPlatform/ParameterDefinitionDeleteProcessor.php @@ -0,0 +1,58 @@ + + */ +final readonly class ParameterDefinitionDeleteProcessor implements ProcessorInterface +{ + public function __construct( + private ManagerRegistry $managerRegistry, + #[Autowire(service: 'api_platform.doctrine.orm.state.remove_processor')] + private ProcessorInterface $removeProcessor, + ) { + } + + public function process(mixed $data, Operation $operation, array $uriVariables = [], array $context = []): void + { + if (!$data instanceof ParameterDefinition) { + throw new \InvalidArgumentException('Expected a parameter definition.'); + } + + $repository = $this->managerRegistry->getRepository(AbstractParameter::class); + if (!$repository instanceof ParameterRepository) { + throw new \LogicException('The abstract parameter repository is not configured correctly.'); + } + + if ($repository->countByDefinition($data) > 0) { + throw new ConflictHttpException('This parameter definition is still in use and cannot be deleted.'); + } + + $this->removeProcessor->process($data, $operation, $uriVariables, $context); + } +} diff --git a/src/Controller/AdminPages/BaseAdminController.php b/src/Controller/AdminPages/BaseAdminController.php index b7c25229f..eb523878d 100644 --- a/src/Controller/AdminPages/BaseAdminController.php +++ b/src/Controller/AdminPages/BaseAdminController.php @@ -88,8 +88,8 @@ public function __construct(protected TranslatorInterface $translator, protected throw new InvalidArgumentException('You have to override the $entity_class, $form_class, $route_base and $twig_template value in your subclasss!'); } - if ('' === $this->attachment_class || !is_a($this->attachment_class, Attachment::class, true)) { - throw new InvalidArgumentException('You have to override the $attachment_class value with a valid Attachment class in your subclass!'); + if ('' !== $this->attachment_class && !is_a($this->attachment_class, Attachment::class, true)) { + throw new InvalidArgumentException('The $attachment_class value must be empty or a valid Attachment class!'); } if ('' === $this->parameter_class || ($this->parameter_class && !is_a($this->parameter_class, AbstractParameter::class, true))) { @@ -171,21 +171,23 @@ protected function _edit(AbstractNamedDBElement $entity, Request $request, Entit if ($form->isSubmitted() && $form->isValid()) { if ($this->additionalActionEdit($form, $entity)) { //Upload passed files - $attachments = $form['attachments']; - foreach ($attachments as $attachment) { - /** @var FormInterface $attachment */ - try { - $this->attachmentSubmitHandler->handleUpload( - $attachment->getData(), - AttachmentUpload::fromAttachmentForm($attachment) - ); - } catch (AttachmentDownloadException $attachmentDownloadException) { - $this->addFlash( - 'error', - $this->translator->trans( - 'attachment.download_failed' - ).' '.$attachmentDownloadException->getMessage() - ); + if ($form->has('attachments')) { + $attachments = $form['attachments']; + foreach ($attachments as $attachment) { + /** @var FormInterface $attachment */ + try { + $this->attachmentSubmitHandler->handleUpload( + $attachment->getData(), + AttachmentUpload::fromAttachmentForm($attachment) + ); + } catch (AttachmentDownloadException $attachmentDownloadException) { + $this->addFlash( + 'error', + $this->translator->trans( + 'attachment.download_failed' + ).' '.$attachmentDownloadException->getMessage() + ); + } } } @@ -268,22 +270,24 @@ protected function _new(Request $request, EntityManagerInterface $em, EntityImpo //Perform additional actions if ($form->isSubmitted() && $form->isValid() && $this->additionalActionNew($form, $new_entity)) { //Upload passed files - $attachments = $form['attachments']; - foreach ($attachments as $attachment) { - /** @var FormInterface $attachment */ - - try { - $this->attachmentSubmitHandler->handleUpload( - $attachment->getData(), - AttachmentUpload::fromAttachmentForm($attachment) - ); - } catch (AttachmentDownloadException $attachmentDownloadException) { - $this->addFlash( - 'error', - $this->translator->trans( - 'attachment.download_failed' - ).' '.$attachmentDownloadException->getMessage() - ); + if ($form->has('attachments')) { + $attachments = $form['attachments']; + foreach ($attachments as $attachment) { + /** @var FormInterface $attachment */ + + try { + $this->attachmentSubmitHandler->handleUpload( + $attachment->getData(), + AttachmentUpload::fromAttachmentForm($attachment) + ); + } catch (AttachmentDownloadException $attachmentDownloadException) { + $this->addFlash( + 'error', + $this->translator->trans( + 'attachment.download_failed' + ).' '.$attachmentDownloadException->getMessage() + ); + } } } diff --git a/src/Controller/AdminPages/ParameterDefinitionController.php b/src/Controller/AdminPages/ParameterDefinitionController.php new file mode 100644 index 000000000..d619f02c4 --- /dev/null +++ b/src/Controller/AdminPages/ParameterDefinitionController.php @@ -0,0 +1,92 @@ +_delete($request, $entity, $recursionHelper); + } + + #[Route(path: '/{id}/edit/{timestamp}', name: 'parameter_definition_edit', requirements: ['id' => '\d+'])] + #[Route(path: '/{id}', requirements: ['id' => '\d+'])] + public function edit(ParameterDefinition $entity, Request $request, EntityManagerInterface $em, ?string $timestamp = null): Response + { + return $this->_edit($entity, $request, $em, $timestamp); + } + + #[Route(path: '/new', name: 'parameter_definition_new')] + #[Route(path: '/{id}/clone', name: 'parameter_definition_clone')] + #[Route(path: '/')] + public function new(Request $request, EntityManagerInterface $em, EntityImporter $importer, ?ParameterDefinition $entity = null): Response + { + return $this->_new($request, $em, $importer, $entity); + } + + #[Route(path: '/export', name: 'parameter_definition_export_all')] + public function exportAll(EntityManagerInterface $em, EntityExporter $exporter, Request $request): Response + { + return $this->_exportAll($em, $exporter, $request); + } + + #[Route(path: '/{id}/export', name: 'parameter_definition_export')] + public function exportEntity(ParameterDefinition $entity, EntityExporter $exporter, Request $request): Response + { + return $this->_exportEntity($entity, $exporter, $request); + } + + protected function deleteCheck(AbstractNamedDBElement $entity): bool + { + if ($entity instanceof ParameterDefinition) { + $repository = $this->entityManager->getRepository(AbstractParameter::class); + if (!$repository instanceof ParameterRepository) { + throw new \LogicException('The abstract parameter repository is not configured correctly.'); + } + + if ($repository->countByDefinition($entity) > 0) { + $this->addFlash('error', 'parameter_definition.delete.in_use'); + + return false; + } + } + + return parent::deleteCheck($entity); + } +} diff --git a/src/Controller/TypeaheadController.php b/src/Controller/TypeaheadController.php index f7e15b6da..4477e7ca7 100644 --- a/src/Controller/TypeaheadController.php +++ b/src/Controller/TypeaheadController.php @@ -30,6 +30,7 @@ use App\Entity\Parameters\GroupParameter; use App\Entity\Parameters\ManufacturerParameter; use App\Entity\Parameters\MeasurementUnitParameter; +use App\Entity\Parameters\ParameterDefinition; use App\Entity\Parameters\PartParameter; use App\Entity\Parameters\ProjectParameter; use App\Entity\Parameters\StorageLocationParameter; @@ -176,9 +177,59 @@ public function parameters(string $type, EntityManagerInterface $entityManager, $data = $repository->autocompleteParamName($query); + if (PartParameter::class === $class) { + $definition_repository = $entityManager->getRepository(ParameterDefinition::class); + $data = $this->mergeParameterDefinitionSuggestions( + $definition_repository->autocompleteForParameterEditor($query), + $data, + ); + } + return new JsonResponse($data); } + /** + * Global definitions take precedence over legacy parameter suggestions with the same case-insensitive name. + * + * @param list|null + * }> $definitions + * @param array $legacy_parameters + * @return list> + */ + private function mergeParameterDefinitionSuggestions(array $definitions, array $legacy_parameters): array + { + $result = []; + $known_names = []; + + foreach ($definitions as $definition) { + $definition['choices'] ??= []; + $result[] = $definition; + $known_names[mb_strtolower(trim($definition['name']))] = true; + } + + foreach ($legacy_parameters as $legacy_parameter) { + if (50 <= count($result)) { + break; + } + + $normalized_name = mb_strtolower(trim($legacy_parameter['name'])); + if (isset($known_names[$normalized_name])) { + continue; + } + + $result[] = $legacy_parameter; + $known_names[$normalized_name] = true; + } + + return array_slice($result, 0, 50); + } + #[Route(path: '/tags/search/{query}', name: 'typeahead_tags', requirements: ['query' => '.+'])] public function tags(string $query, TagFinder $finder): JsonResponse { diff --git a/src/Entity/LogSystem/LogTargetType.php b/src/Entity/LogSystem/LogTargetType.php index 3b2d8682a..cb3284983 100644 --- a/src/Entity/LogSystem/LogTargetType.php +++ b/src/Entity/LogSystem/LogTargetType.php @@ -28,6 +28,7 @@ use App\Entity\InfoProviderSystem\BulkInfoProviderImportJobPart; use App\Entity\LabelSystem\LabelProfile; use App\Entity\Parameters\AbstractParameter; +use App\Entity\Parameters\ParameterDefinition; use App\Entity\Parts\Category; use App\Entity\Parts\Footprint; use App\Entity\Parts\Manufacturer; @@ -73,6 +74,7 @@ enum LogTargetType: int case BULK_INFO_PROVIDER_IMPORT_JOB = 21; case BULK_INFO_PROVIDER_IMPORT_JOB_PART = 22; case PART_CUSTOM_STATE = 23; + case PARAMETER_DEFINITION = 24; /** * Returns the class name of the target type or null if the target type is NONE. @@ -104,7 +106,8 @@ public function toClass(): ?string self::PART_ASSOCIATION => PartAssociation::class, self::BULK_INFO_PROVIDER_IMPORT_JOB => BulkInfoProviderImportJob::class, self::BULK_INFO_PROVIDER_IMPORT_JOB_PART => BulkInfoProviderImportJobPart::class, - self::PART_CUSTOM_STATE => PartCustomState::class + self::PART_CUSTOM_STATE => PartCustomState::class, + self::PARAMETER_DEFINITION => ParameterDefinition::class, }; } diff --git a/src/Entity/Parameters/AbstractParameter.php b/src/Entity/Parameters/AbstractParameter.php index 48aa40bd8..5c52ed0cd 100644 --- a/src/Entity/Parameters/AbstractParameter.php +++ b/src/Entity/Parameters/AbstractParameter.php @@ -46,6 +46,7 @@ use ApiPlatform\Doctrine\Orm\Filter\OrderFilter; use ApiPlatform\Doctrine\Orm\Filter\RangeFilter; use ApiPlatform\Metadata\ApiFilter; +use ApiPlatform\Metadata\ApiProperty; use ApiPlatform\Metadata\ApiResource; use ApiPlatform\Metadata\Delete; use ApiPlatform\Metadata\Get; @@ -64,6 +65,7 @@ use Symfony\Component\Serializer\Annotation\SerializedName; use Symfony\Component\Serializer\Attribute\DiscriminatorMap; use Symfony\Component\Validator\Constraints as Assert; +use Symfony\Component\Validator\Context\ExecutionContextInterface; use function sprintf; @@ -79,6 +81,7 @@ #[ORM\Index(columns: ['name'], name: 'parameter_name_idx')] #[ORM\Index(columns: ['param_group'], name: 'parameter_group_idx')] #[ORM\Index(columns: ['type', 'element_id'], name: 'parameter_type_element_idx')] +#[ORM\Index(columns: ['definition_id', 'value_text', 'type', 'element_id'], name: 'parameter_definition_value_idx')] #[ApiResource( shortName: 'Parameter', operations: [ @@ -98,7 +101,6 @@ #[DiscriminatorMap(typeProperty: '_type', mapping: self::API_DISCRIMINATOR_MAP)] abstract class AbstractParameter extends AbstractNamedDBElement implements UniqueValidatableInterface { - /* * The discriminator map used for API platform. The key should be the same as the api platform short type (the @type JSONLD field). */ @@ -164,6 +166,16 @@ abstract class AbstractParameter extends AbstractNamedDBElement implements Uniqu #[Assert\Length(max: 255)] protected string $value_text = ''; + /** + * Optional global definition. Name, symbol and unit remain persisted snapshots for historical views, while + * input type and choices are always read from the linked definition. + */ + #[ApiProperty(readableLink: false, writableLink: false)] + #[Groups(['parameter:read', 'parameter:write'])] + #[ORM\ManyToOne(targetEntity: ParameterDefinition::class, inversedBy: 'parameter_usages')] + #[ORM\JoinColumn(name: 'definition_id', nullable: true, onDelete: 'RESTRICT')] + protected ?ParameterDefinition $definition = null; + /** * @var string the group this parameter belongs to */ @@ -218,6 +230,86 @@ public function getElement(): ?AbstractDBElement return $this->element; } + /** + * Returns the optional global definition linked to this parameter. + */ + public function getDefinition(): ?ParameterDefinition + { + return $this->definition; + } + + /** + * Links a definition and captures its current name, symbol and unit as historical snapshots. + */ + public function setDefinition(?ParameterDefinition $definition): self + { + $this->synchronizeDefinitionReference($definition); + if ($definition instanceof ParameterDefinition) { + $this->refreshSnapshotFromDefinition(); + + if (ParameterDefinition::INPUT_TYPE_CHOICE === $definition->getInputType() && '' !== $this->value_text) { + $canonical_choice = $definition->findCanonicalChoice($this->value_text); + if (null !== $canonical_choice) { + $this->value_text = $canonical_choice; + } + } + } + + return $this; + } + + /** + * Restores only the historical association without replacing the independently persisted metadata snapshots. + * This is intended for the TimeTravel infrastructure. + */ + public function restoreDefinitionReference(?ParameterDefinition $definition): self + { + $this->synchronizeDefinitionReference($definition); + + return $this; + } + + private function synchronizeDefinitionReference(?ParameterDefinition $definition): void + { + if ($this->definition === $definition) { + $definition?->addParameterUsage($this); + + return; + } + + $previous_definition = $this->definition; + $this->definition = $definition; + + $previous_definition?->removeParameterUsage($this); + $definition?->addParameterUsage($this); + } + + /** + * Explicitly refreshes the persisted name, symbol and unit snapshots from the linked definition. + */ + public function refreshSnapshotFromDefinition(): self + { + if (!$this->definition instanceof ParameterDefinition) { + return $this; + } + + $this->name = $this->definition->getName(); + $this->symbol = $this->definition->getSymbol(); + $this->unit = $this->definition->getUnit(); + + return $this; + } + + public function getSnapshotName(): string + { + return $this->name; + } + + public function getEffectiveName(): string + { + return $this->definition?->getName() ?? $this->name; + } + /** * Return a formatted string version of the values of the string. * Based on the set values it can return something like this: 34 V (12 V ... 50 V) [Text]. @@ -225,6 +317,19 @@ public function getElement(): ?AbstractDBElement #[Groups(['parameter:read', 'full'])] #[SerializedName('formatted')] public function getFormattedValue(bool $latex_formatted = false): string + { + return $this->formatValue($this->unit, $latex_formatted); + } + + /** + * Formats the current value using metadata from the linked definition when available. + */ + public function getEffectiveFormattedValue(bool $latex_formatted = false): string + { + return $this->formatValue($this->getEffectiveUnit(), $latex_formatted); + } + + private function formatValue(string $unit, bool $latex_formatted): string { //If we just only have text value, return early if (null === $this->value_typical && null === $this->value_min && null === $this->value_max) { @@ -234,7 +339,7 @@ public function getFormattedValue(bool $latex_formatted = false): string $str = ''; $bracket_opened = false; if ($this->value_typical !== null) { - $str .= $this->getValueTypicalWithUnit($latex_formatted); + $str .= $this->formatWithExplicitUnit($this->value_typical, $unit, with_latex: $latex_formatted); if ($this->value_min || $this->value_max) { $bracket_opened = true; $str .= ' ('; @@ -242,11 +347,12 @@ public function getFormattedValue(bool $latex_formatted = false): string } if ($this->value_max !== null && $this->value_min !== null) { - $str .= $this->getValueMinWithUnit($latex_formatted).' ... '.$this->getValueMaxWithUnit($latex_formatted); + $str .= $this->formatWithExplicitUnit($this->value_min, $unit, with_latex: $latex_formatted).' ... ' + .$this->formatWithExplicitUnit($this->value_max, $unit, with_latex: $latex_formatted); } elseif ($this->value_max !== null) { - $str .= 'max. '.$this->getValueMaxWithUnit($latex_formatted); + $str .= 'max. '.$this->formatWithExplicitUnit($this->value_max, $unit, with_latex: $latex_formatted); } elseif ($this->value_min !== null) { - $str .= 'min. '.$this->getValueMinWithUnit($latex_formatted); + $str .= 'min. '.$this->formatWithExplicitUnit($this->value_min, $unit, with_latex: $latex_formatted); } //Add closing bracket @@ -317,6 +423,16 @@ public function getSymbol(): string return $this->symbol; } + public function getSnapshotSymbol(): string + { + return $this->symbol; + } + + public function getEffectiveSymbol(): string + { + return $this->definition?->getSymbol() ?? $this->symbol; + } + /** * Sets the mathematical symbol for this specification (e.g. "V_CB"). * @@ -422,6 +538,16 @@ public function getUnit(): string return $this->unit; } + public function getSnapshotUnit(): string + { + return $this->unit; + } + + public function getEffectiveUnit(): string + { + return $this->definition?->getUnit() ?? $this->unit; + } + /** * Sets the unit used by the value. * @@ -449,29 +575,69 @@ public function getValueText(): string */ public function setValueText(string $value_text): self { + if ($this->definition instanceof ParameterDefinition + && ParameterDefinition::INPUT_TYPE_CHOICE === $this->definition->getInputType() + && '' !== $value_text) { + $canonical_choice = $this->definition->findCanonicalChoice($value_text); + if (null !== $canonical_choice) { + $value_text = $canonical_choice; + } + } + $this->value_text = $value_text; return $this; } + #[Groups(['parameter:read'])] + #[SerializedName('input_type')] + public function getEffectiveInputType(): string + { + return $this->definition?->getInputType() ?? ParameterDefinition::INPUT_TYPE_TEXT; + } + + /** @return list */ + #[Groups(['parameter:read'])] + #[SerializedName('choices')] + public function getEffectiveChoices(): array + { + return $this->definition?->getChoices() ?? []; + } + + public function hasEffectiveChoices(): bool + { + return ParameterDefinition::INPUT_TYPE_CHOICE === $this->getEffectiveInputType() + && [] !== $this->getEffectiveChoices(); + } + + public function getEffectiveChoicesText(): string + { + return implode("\n", $this->getEffectiveChoices()); + } + /** * Return a string representation and (if possible) with its unit. */ protected function formatWithUnit(float $value, string $format = '%g', bool $with_latex = false): string + { + return $this->formatWithExplicitUnit($value, $this->unit, $format, $with_latex); + } + + private function formatWithExplicitUnit(float $value, string $unit, string $format = '%g', bool $with_latex = false): string { $str = sprintf($format, $value); - if ($this->unit !== '') { + if ('' !== $unit) { if (!$with_latex) { - $unit = $this->unit; + $formatted_unit = $unit; } else { //Escape the percentage sign for convenience (as latex uses it as comment and it is often used in units) - $escaped = preg_replace('/\\\\?%/', "\\\\%", $this->unit); + $escaped = preg_replace('/\\\\?%/', "\\\\%", $unit); - $unit = '$\mathrm{'.$escaped.'}$'; + $formatted_unit = '$\mathrm{'.$escaped.'}$'; } - return $str.' '.$unit; + return $str.' '.$formatted_unit; } return $str; @@ -516,8 +682,33 @@ public function setEdaSymbolVisibility(?bool $eda_symbol_visibility): self return $this; } + #[Assert\Callback] + public function validateDefinitionUsage(ExecutionContextInterface $context): void + { + if (!$this->definition instanceof ParameterDefinition + || ParameterDefinition::INPUT_TYPE_CHOICE !== $this->definition->getInputType() + || '' === $this->value_text) { + return; + } + + $canonical_choice = $this->definition->findCanonicalChoice($this->value_text); + if (null === $canonical_choice) { + $context->buildViolation('The selected value is not part of the linked parameter definition.') + ->atPath('value_text') + ->addViolation(); + + return; + } + + if ($canonical_choice !== $this->value_text) { + $context->buildViolation('The selected value does not use the canonical spelling from the linked parameter definition.') + ->atPath('value_text') + ->addViolation(); + } + } + public function getComparableFields(): array { - return ['name' => $this->name, 'group' => $this->group, 'element' => $this->element?->getId()]; + return ['name' => $this->getEffectiveName(), 'group' => $this->group, 'element' => $this->element?->getId()]; } } diff --git a/src/Entity/Parameters/ParameterDefinition.php b/src/Entity/Parameters/ParameterDefinition.php new file mode 100644 index 000000000..d4e521e6e --- /dev/null +++ b/src/Entity/Parameters/ParameterDefinition.php @@ -0,0 +1,322 @@ + ['parameter_definition:read', 'api:basic:read'], 'openapi_definition_name' => 'Read'], + denormalizationContext: ['groups' => ['parameter_definition:write', 'api:basic:write'], 'openapi_definition_name' => 'Write'], +)] +#[ApiFilter(PropertyFilter::class)] +#[ApiFilter(LikeFilter::class, properties: ['name', 'symbol', 'unit'])] +#[ApiFilter(DateFilter::class, strategy: DateFilterInterface::EXCLUDE_NULL)] +#[ApiFilter(OrderFilter::class, properties: ['name', 'id', 'addedDate', 'lastModified'])] +class ParameterDefinition extends AbstractNamedDBElement +{ + public const INPUT_TYPE_TEXT = 'text'; + public const INPUT_TYPE_CHOICE = 'choice'; + public const MAX_CHOICE_LENGTH = 255; + + #[ORM\Column(type: Types::STRING, length: 255)] + private string $normalized_name = ''; + + #[ORM\Column(type: Types::STRING, length: 16, options: ['default' => self::INPUT_TYPE_TEXT])] + #[Assert\Choice(choices: [self::INPUT_TYPE_TEXT, self::INPUT_TYPE_CHOICE])] + #[Groups(['full', 'import', 'parameter_definition:read', 'parameter_definition:write'])] + private string $input_type = self::INPUT_TYPE_TEXT; + + /** @var list|null */ + #[ORM\Column(type: Types::JSON, nullable: true)] + #[Groups(['full', 'import', 'parameter_definition:read', 'parameter_definition:write'])] + private ?array $choices = null; + + #[ORM\Column(type: Types::STRING, length: 20)] + #[Assert\Length(max: 20)] + #[Groups(['full', 'import', 'parameter_definition:read', 'parameter_definition:write'])] + private string $symbol = ''; + + #[ORM\Column(type: Types::STRING, length: 50)] + #[Assert\Length(max: 50)] + #[Groups(['full', 'import', 'parameter_definition:read', 'parameter_definition:write'])] + private string $unit = ''; + + /** @var Collection */ + #[ORM\OneToMany(mappedBy: 'definition', targetEntity: AbstractParameter::class)] + private Collection $parameter_usages; + + public function __construct() + { + $this->parameter_usages = new ArrayCollection(); + } + + public function setName(string $new_name): self + { + $new_name = trim($new_name); + parent::setName($new_name); + $this->normalized_name = self::normalize($new_name); + + return $this; + } + + #[ORM\PrePersist] + #[ORM\PreUpdate] + public function updateNormalizedName(): void + { + $this->normalized_name = self::normalize($this->name); + } + + public function getNormalizedName(): string + { + return $this->normalized_name; + } + + public function getInputType(): string + { + return $this->input_type; + } + + public function setInputType(string $input_type): self + { + if (!in_array($input_type, [self::INPUT_TYPE_TEXT, self::INPUT_TYPE_CHOICE], true)) { + throw new InvalidArgumentException(sprintf('Unsupported parameter input type "%s".', $input_type)); + } + + $this->input_type = $input_type; + if (self::INPUT_TYPE_TEXT === $input_type) { + $this->choices = null; + } + + return $this; + } + + /** @return list */ + public function getChoices(): array + { + return $this->choices ?? []; + } + + /** @param list|null $choices */ + public function setChoices(?array $choices): self + { + $canonical_choices = self::canonicalizeChoices($choices ?? []); + $this->choices = [] === $canonical_choices ? null : $canonical_choices; + + return $this; + } + + public function getChoicesText(): string + { + return implode("\n", $this->getChoices()); + } + + public function setChoicesText(?string $choices_text): self + { + if (null === $choices_text || '' === trim($choices_text)) { + return $this->setChoices(null); + } + + return $this->setChoices(preg_split('/\R/', $choices_text) ?: []); + } + + public function addChoice(string $choice): string + { + if (self::INPUT_TYPE_CHOICE !== $this->input_type) { + throw new LogicException('Choices can only be added to a choice parameter definition.'); + } + + $choice = trim($choice); + if ('' === $choice) { + throw new InvalidArgumentException('A parameter choice must not be empty.'); + } + if (mb_strlen($choice) > self::MAX_CHOICE_LENGTH) { + throw new InvalidArgumentException(sprintf('A parameter choice must not exceed %d characters.', self::MAX_CHOICE_LENGTH)); + } + + $canonical_choice = $this->findCanonicalChoice($choice); + if (null !== $canonical_choice) { + return $canonical_choice; + } + + $choices = $this->getChoices(); + $choices[] = $choice; + $this->choices = $choices; + + return $choice; + } + + public function findCanonicalChoice(string $choice): ?string + { + $normalized_choice = self::normalize($choice); + foreach ($this->getChoices() as $canonical_choice) { + if (self::normalize($canonical_choice) === $normalized_choice) { + return $canonical_choice; + } + } + + return null; + } + + public function getSymbol(): string + { + return $this->symbol; + } + + public function setSymbol(string $symbol): self + { + $this->symbol = $symbol; + + return $this; + } + + public function getUnit(): string + { + return $this->unit; + } + + public function setUnit(string $unit): self + { + $this->unit = $unit; + + return $this; + } + + /** @return Collection */ + public function getParameterUsages(): Collection + { + return $this->parameter_usages; + } + + public function addParameterUsage(AbstractParameter $parameter): self + { + if (!$this->parameter_usages->contains($parameter)) { + $this->parameter_usages->add($parameter); + } + + if ($parameter->getDefinition() !== $this) { + $parameter->setDefinition($this); + } + + return $this; + } + + public function removeParameterUsage(AbstractParameter $parameter): self + { + $this->parameter_usages->removeElement($parameter); + + if ($parameter->getDefinition() === $this) { + $parameter->setDefinition(null); + } + + return $this; + } + + #[Assert\Callback] + public function validateChoices(ExecutionContextInterface $context): void + { + if (self::INPUT_TYPE_TEXT === $this->input_type && [] !== $this->getChoices()) { + $context->buildViolation('A text parameter definition must not contain choices.') + ->atPath('choices') + ->addViolation(); + } + + foreach ($this->getChoices() as $choice) { + if (mb_strlen($choice) > self::MAX_CHOICE_LENGTH) { + $context->buildViolation(sprintf('A parameter choice must not exceed %d characters.', self::MAX_CHOICE_LENGTH)) + ->atPath('choices') + ->addViolation(); + } + } + } + + /** + * @param list $choices + * @return list + */ + private static function canonicalizeChoices(array $choices): array + { + $canonical_choices = []; + $seen_choices = []; + + foreach ($choices as $choice) { + if (!is_string($choice)) { + throw new InvalidArgumentException('Parameter choices must be strings.'); + } + + $choice = trim($choice); + if ('' === $choice) { + continue; + } + if (mb_strlen($choice) > self::MAX_CHOICE_LENGTH) { + throw new InvalidArgumentException(sprintf('A parameter choice must not exceed %d characters.', self::MAX_CHOICE_LENGTH)); + } + + $normalized_choice = self::normalize($choice); + if (isset($seen_choices[$normalized_choice])) { + continue; + } + + $seen_choices[$normalized_choice] = true; + $canonical_choices[] = $choice; + } + + return $canonical_choices; + } + + private static function normalize(string $value): string + { + return mb_strtolower(trim($value)); + } +} diff --git a/src/Entity/UserSystem/PermissionData.php b/src/Entity/UserSystem/PermissionData.php index b7d1ff8f5..337e3c38e 100644 --- a/src/Entity/UserSystem/PermissionData.php +++ b/src/Entity/UserSystem/PermissionData.php @@ -43,7 +43,7 @@ final class PermissionData implements \JsonSerializable /** * The current schema version of the permission data */ - public const CURRENT_SCHEMA_VERSION = 4; + public const CURRENT_SCHEMA_VERSION = 5; /** * Creates a new Permission Data Instance using the given data. diff --git a/src/Form/AdminPages/BaseEntityAdminForm.php b/src/Form/AdminPages/BaseEntityAdminForm.php index 54cb04069..5491c456b 100644 --- a/src/Form/AdminPages/BaseEntityAdminForm.php +++ b/src/Form/AdminPages/BaseEntityAdminForm.php @@ -56,6 +56,7 @@ public function configureOptions(OptionsResolver $resolver): void parent::configureOptions($resolver); $resolver->setRequired('attachment_class'); $resolver->setRequired('parameter_class'); + $resolver->setAllowedTypes('attachment_class', 'string'); $resolver->setAllowedTypes('parameter_class', ['string', 'null']); $resolver->setDefaults([ @@ -135,26 +136,28 @@ public function buildForm(FormBuilderInterface $builder, array $options): void $this->additionalFormElements($builder, $options, $entity); - //Attachment section - $builder->add('attachments', CollectionType::class, [ - 'entry_type' => AttachmentFormType::class, - 'allow_add' => true, - 'allow_delete' => true, - 'label' => false, - 'reindex_enable' => true, - 'disabled' => !$this->security->isGranted($is_new ? 'create' : 'edit', $entity), - 'entry_options' => [ - 'data_class' => $options['attachment_class'], - ], - 'by_reference' => false, - ]); + if ('' !== $options['attachment_class']) { + //Attachment section + $builder->add('attachments', CollectionType::class, [ + 'entry_type' => AttachmentFormType::class, + 'allow_add' => true, + 'allow_delete' => true, + 'label' => false, + 'reindex_enable' => true, + 'disabled' => !$this->security->isGranted($is_new ? 'create' : 'edit', $entity), + 'entry_options' => [ + 'data_class' => $options['attachment_class'], + ], + 'by_reference' => false, + ]); - $builder->add('master_picture_attachment', MasterPictureAttachmentType::class, [ - 'required' => false, - 'disabled' => !$this->security->isGranted($is_new ? 'create' : 'edit', $entity), - 'label' => 'part.edit.master_attachment', - 'entity' => $entity, - ]); + $builder->add('master_picture_attachment', MasterPictureAttachmentType::class, [ + 'required' => false, + 'disabled' => !$this->security->isGranted($is_new ? 'create' : 'edit', $entity), + 'label' => 'part.edit.master_attachment', + 'entity' => $entity, + ]); + } $builder->add('log_comment', TextType::class, [ 'label' => 'edit.log_comment', diff --git a/src/Form/AdminPages/ParameterDefinitionAdminForm.php b/src/Form/AdminPages/ParameterDefinitionAdminForm.php new file mode 100644 index 000000000..a0b6e870b --- /dev/null +++ b/src/Form/AdminPages/ParameterDefinitionAdminForm.php @@ -0,0 +1,84 @@ +getID(); + $disabled = !$this->security->isGranted($is_new ? 'create' : 'edit', $entity); + + $builder + ->add('symbol', TextType::class, [ + 'required' => false, + 'empty_data' => '', + 'label' => 'parameter_definition.symbol', + 'disabled' => $disabled, + ]) + ->add('unit', TextType::class, [ + 'required' => false, + 'empty_data' => '', + 'label' => 'parameter_definition.unit', + 'disabled' => $disabled, + ]) + ->add('input_type', ChoiceType::class, [ + 'label' => 'parameter_definition.input_type', + 'choices' => [ + 'parameter_definition.input_type.text' => ParameterDefinition::INPUT_TYPE_TEXT, + 'parameter_definition.input_type.choice' => ParameterDefinition::INPUT_TYPE_CHOICE, + ], + 'disabled' => $disabled, + ]) + ->add('choices_text', TextareaType::class, [ + 'mapped' => false, + 'data' => $entity->getChoicesText(), + 'required' => false, + 'empty_data' => '', + 'label' => 'parameter_definition.choices', + 'help' => 'parameter_definition.choices.help', + 'attr' => ['rows' => 8], + 'disabled' => $disabled, + ]); + + $builder->addEventListener(FormEvents::SUBMIT, static function (FormEvent $event): void { + $definition = $event->getData(); + if (!$definition instanceof ParameterDefinition) { + return; + } + + if (ParameterDefinition::INPUT_TYPE_CHOICE === $definition->getInputType()) { + $choices_text = $event->getForm()->get('choices_text')->getData(); + $definition->setChoicesText(is_string($choices_text) ? $choices_text : null); + } else { + $definition->setChoices(null); + } + }); + } +} diff --git a/src/Form/Filters/LogFilterType.php b/src/Form/Filters/LogFilterType.php index 30abf723d..12ec595d6 100644 --- a/src/Form/Filters/LogFilterType.php +++ b/src/Form/Filters/LogFilterType.php @@ -126,6 +126,7 @@ public function buildForm(FormBuilderInterface $builder, array $options): void LogTargetType::PRICEDETAIL => 'pricedetail.label', LogTargetType::MEASUREMENT_UNIT => 'measurement_unit.label', LogTargetType::PARAMETER => 'parameter.label', + LogTargetType::PARAMETER_DEFINITION => 'parameter_definition.label', LogTargetType::LABEL_PROFILE => 'label_profile.label', LogTargetType::PART_ASSOCIATION => 'part_association.label', LogTargetType::BULK_INFO_PROVIDER_IMPORT_JOB => 'bulk_info_provider_import_job.label', diff --git a/src/Form/ParameterType.php b/src/Form/ParameterType.php index bf37fca4a..22e1033ee 100644 --- a/src/Form/ParameterType.php +++ b/src/Form/ParameterType.php @@ -50,33 +50,53 @@ use App\Entity\Parameters\GroupParameter; use App\Entity\Parameters\ManufacturerParameter; use App\Entity\Parameters\PartParameter; +use App\Entity\Parameters\ParameterDefinition; use App\Entity\Parameters\StorageLocationParameter; use App\Entity\Parameters\SupplierParameter; use App\Entity\Parts\MeasurementUnit; use App\Form\Type\ExponentialNumberType; use App\Form\Type\TriStateCheckboxType; +use Doctrine\ORM\EntityManagerInterface; +use Symfony\Bridge\Doctrine\Form\Type\EntityType; use Symfony\Component\Form\AbstractType; +use Symfony\Component\Form\Event\PreSetDataEvent; use Symfony\Component\Form\Extension\Core\Type\CheckboxType; +use Symfony\Component\Form\Extension\Core\Type\ChoiceType; use Symfony\Component\Form\Extension\Core\Type\NumberType; use Symfony\Component\Form\Extension\Core\Type\TextType; +use Symfony\Component\Form\FormEvent; use Symfony\Component\Form\FormBuilderInterface; +use Symfony\Component\Form\FormEvents; use Symfony\Component\Form\FormInterface; use Symfony\Component\Form\FormView; use Symfony\Component\OptionsResolver\OptionsResolver; class ParameterType extends AbstractType { + public function __construct(private readonly EntityManagerInterface $entity_manager) + { + } + public function buildForm(FormBuilderInterface $builder, array $options): void { - $builder->add('name', TextType::class, [ + $parameter = $builder->getData(); + $linked_part_parameter = $parameter instanceof PartParameter + && $parameter->getDefinition() instanceof ParameterDefinition; + + $name_options = [ 'label' => false, 'empty_data' => '', 'attr' => [ 'placeholder' => 'parameters.name.placeholder', 'class' => 'form-control-sm', ], - ]); - $builder->add('symbol', TextType::class, [ + ]; + if ($linked_part_parameter) { + $name_options['data'] = $parameter->getEffectiveName(); + } + $builder->add('name', TextType::class, $name_options); + + $symbol_options = [ 'label' => false, 'required' => false, 'empty_data' => '', @@ -85,16 +105,25 @@ public function buildForm(FormBuilderInterface $builder, array $options): void 'class' => 'form-control-sm', 'style' => 'max-width: 12ch;', ], - ]); - $builder->add('value_text', TextType::class, [ - 'label' => false, - 'required' => false, - 'empty_data' => '', - 'attr' => [ - 'placeholder' => 'parameters.text.placeholder', - 'class' => 'form-control-sm', - ], - ]); + ]; + if ($linked_part_parameter) { + $symbol_options['data'] = $parameter->getEffectiveSymbol(); + } + $builder->add('symbol', TextType::class, $symbol_options); + + $builder->addEventListener( + FormEvents::PRE_SET_DATA, + function (PreSetDataEvent $event): void { + $parameter = $event->getData(); + $this->addValueTextField( + $event->getForm(), + $parameter instanceof PartParameter + ? $parameter->getEffectiveInputType() + : ParameterDefinition::INPUT_TYPE_TEXT, + $parameter instanceof PartParameter ? $parameter->getEffectiveChoices() : [], + ); + } + ); $builder->add('value_max', ExponentialNumberType::class, [ 'label' => false, @@ -129,7 +158,7 @@ public function buildForm(FormBuilderInterface $builder, array $options): void 'style' => 'max-width: 25ch;', ], ]); - $builder->add('unit', TextType::class, [ + $unit_options = [ 'label' => false, 'required' => false, 'empty_data' => '', @@ -138,7 +167,11 @@ public function buildForm(FormBuilderInterface $builder, array $options): void 'class' => 'form-control-sm', 'style' => 'max-width: 8ch;', ], - ]); + ]; + if ($linked_part_parameter) { + $unit_options['data'] = $parameter->getEffectiveUnit(); + } + $builder->add('unit', TextType::class, $unit_options); $builder->add('group', TextType::class, [ 'label' => false, @@ -149,9 +182,49 @@ public function buildForm(FormBuilderInterface $builder, array $options): void 'class' => 'form-control-sm', ], ]); - // Only show the EDA visibility field for part parameters, as it has no function for other entities if ($options['data_class'] === PartParameter::class) { + $builder->add('definition', EntityType::class, [ + 'class' => ParameterDefinition::class, + 'choice_label' => 'name', + 'choice_lazy' => true, + 'label' => false, + 'required' => false, + 'placeholder' => '', + 'attr' => [ + 'class' => 'd-none', + ], + ]); + + $builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event): void { + $submitted_data = $event->getData(); + $definition = null; + + if (is_array($submitted_data)) { + $definition_id = filter_var( + $submitted_data['definition'] ?? null, + FILTER_VALIDATE_INT, + ['options' => ['min_range' => 1]], + ); + if (false !== $definition_id) { + $definition = $this->entity_manager->find(ParameterDefinition::class, $definition_id); + } + } + + $this->addValueTextField( + $event->getForm(), + $definition?->getInputType() ?? ParameterDefinition::INPUT_TYPE_TEXT, + $definition?->getChoices() ?? [], + ); + }); + + $builder->addEventListener(FormEvents::SUBMIT, static function (FormEvent $event): void { + $parameter = $event->getData(); + if ($parameter instanceof PartParameter && $parameter->getDefinition() instanceof ParameterDefinition) { + $parameter->refreshSnapshotFromDefinition(); + } + }); + $builder->add('eda_visibility', TriStateCheckboxType::class, [ 'label' => false, 'required' => false, @@ -161,6 +234,7 @@ public function buildForm(FormBuilderInterface $builder, array $options): void 'label' => false, 'required' => false, ]); + } } @@ -196,4 +270,37 @@ public function configureOptions(OptionsResolver $resolver): void 'data_class' => AbstractParameter::class, ]); } + + /** @param list $choices */ + private function addValueTextField(FormInterface $form, string $input_type, array $choices): void + { + if (ParameterDefinition::INPUT_TYPE_CHOICE === $input_type) { + $choice_map = []; + foreach ($choices as $choice) { + $choice_map[$choice] = $choice; + } + + $form->add('value_text', ChoiceType::class, [ + 'label' => false, + 'required' => false, + 'placeholder' => '', + 'choices' => $choice_map, + 'attr' => [ + 'class' => 'form-select-sm', + ], + ]); + + return; + } + + $form->add('value_text', TextType::class, [ + 'label' => false, + 'required' => false, + 'empty_data' => '', + 'attr' => [ + 'placeholder' => 'parameters.text.placeholder', + 'class' => 'form-control-sm', + ], + ]); + } } diff --git a/src/Repository/ParameterDefinitionRepository.php b/src/Repository/ParameterDefinitionRepository.php new file mode 100644 index 000000000..18385054e --- /dev/null +++ b/src/Repository/ParameterDefinitionRepository.php @@ -0,0 +1,62 @@ + + */ +class ParameterDefinitionRepository extends NamedDBElementRepository +{ + /** + * @return list|null + * }> + */ + public function autocompleteForParameterEditor(string $name, int $max_results = 50): array + { + /** @var list|null + * }> $result + */ + $result = $this->createQueryBuilder('definition') + ->select('definition.id AS definition_id') + ->addSelect('definition.name AS name') + ->addSelect('definition.symbol AS symbol') + ->addSelect('definition.unit AS unit') + ->addSelect('definition.input_type AS input_type') + ->addSelect('definition.choices AS choices') + ->where('ILIKE(definition.name, :name) = TRUE') + ->setParameter('name', '%'.$name.'%') + ->orderBy('definition.name', 'ASC') + ->setMaxResults($max_results) + ->getQuery() + ->getArrayResult(); + + return $result; + } +} diff --git a/src/Repository/ParameterRepository.php b/src/Repository/ParameterRepository.php index 6c6c867d6..de423ae25 100644 --- a/src/Repository/ParameterRepository.php +++ b/src/Repository/ParameterRepository.php @@ -23,6 +23,7 @@ namespace App\Repository; use App\Entity\Parameters\AbstractParameter; +use App\Entity\Parameters\ParameterDefinition; /** * @template TEntityClass of AbstractParameter @@ -30,6 +31,16 @@ */ class ParameterRepository extends DBElementRepository { + public function countByDefinition(ParameterDefinition $definition): int + { + return (int) $this->createQueryBuilder('parameter') + ->select('COUNT(parameter.id)') + ->where('parameter.definition = :definition') + ->setParameter('definition', $definition) + ->getQuery() + ->getSingleScalarResult(); + } + /** * Find parameters using a parameter name * @param string $name The name to search for diff --git a/src/Security/Voter/StructureVoter.php b/src/Security/Voter/StructureVoter.php index 16d38e058..ab03173c6 100644 --- a/src/Security/Voter/StructureVoter.php +++ b/src/Security/Voter/StructureVoter.php @@ -24,6 +24,7 @@ use App\Entity\Attachments\AttachmentType; use App\Entity\Parts\PartCustomState; +use App\Entity\Parameters\ParameterDefinition; use App\Entity\ProjectSystem\Project; use App\Entity\Parts\Category; use App\Entity\Parts\Footprint; @@ -40,7 +41,7 @@ use function is_object; /** - * @phpstan-extends Voter + * @phpstan-extends Voter */ final class StructureVoter extends Voter { @@ -55,6 +56,7 @@ final class StructureVoter extends Voter Currency::class => 'currencies', MeasurementUnit::class => 'measurement_units', PartCustomState::class => 'part_custom_states', + ParameterDefinition::class => 'parameter_definitions', ]; public function __construct(private readonly VoterHelper $helper) diff --git a/src/Services/ElementTypes.php b/src/Services/ElementTypes.php index 6ce8f8514..f1a9f0554 100644 --- a/src/Services/ElementTypes.php +++ b/src/Services/ElementTypes.php @@ -29,6 +29,7 @@ use App\Entity\InfoProviderSystem\BulkInfoProviderImportJobPart; use App\Entity\LabelSystem\LabelProfile; use App\Entity\Parameters\AbstractParameter; +use App\Entity\Parameters\ParameterDefinition; use App\Entity\Parts\Category; use App\Entity\Parts\Footprint; use App\Entity\Parts\Manufacturer; @@ -70,6 +71,7 @@ enum ElementTypes: string implements TranslatableInterface case GROUP = "group"; case USER = "user"; case PARAMETER = "parameter"; + case PARAMETER_DEFINITION = "parameter_definition"; case LABEL_PROFILE = "label_profile"; case PART_ASSOCIATION = "part_association"; case BULK_INFO_PROVIDER_IMPORT_JOB = "bulk_info_provider_import_job"; @@ -96,6 +98,7 @@ enum ElementTypes: string implements TranslatableInterface Group::class => self::GROUP, User::class => self::USER, AbstractParameter::class => self::PARAMETER, + ParameterDefinition::class => self::PARAMETER_DEFINITION, LabelProfile::class => self::LABEL_PROFILE, PartAssociation::class => self::PART_ASSOCIATION, BulkInfoProviderImportJob::class => self::BULK_INFO_PROVIDER_IMPORT_JOB, @@ -127,6 +130,7 @@ public function getDefaultLabelKey(): string self::GROUP => 'group.label', self::USER => 'user.label', self::PARAMETER => 'parameter.label', + self::PARAMETER_DEFINITION => 'parameter_definition.label', self::LABEL_PROFILE => 'label_profile.label', self::PART_ASSOCIATION => 'part_association.label', self::BULK_INFO_PROVIDER_IMPORT_JOB => 'bulk_info_provider_import_job.label', @@ -156,6 +160,7 @@ public function getDefaultPluralLabelKey(): string self::GROUP => 'group.labelp', self::USER => 'user.labelp', self::PARAMETER => 'parameter.labelp', + self::PARAMETER_DEFINITION => 'parameter_definition.labelp', self::LABEL_PROFILE => 'label_profile.labelp', self::PART_ASSOCIATION => 'part_association.labelp', self::BULK_INFO_PROVIDER_IMPORT_JOB => 'bulk_info_provider_import_job.labelp', diff --git a/src/Services/EntityURLGenerator.php b/src/Services/EntityURLGenerator.php index 91e271cc0..cb84f3b96 100644 --- a/src/Services/EntityURLGenerator.php +++ b/src/Services/EntityURLGenerator.php @@ -27,6 +27,7 @@ use App\Entity\Attachments\PartAttachment; use App\Entity\Base\AbstractDBElement; use App\Entity\Parameters\PartParameter; +use App\Entity\Parameters\ParameterDefinition; use App\Entity\Parts\PartCustomState; use App\Entity\ProjectSystem\Project; use App\Entity\LabelSystem\LabelProfile; @@ -109,6 +110,7 @@ public function timeTravelURL(AbstractDBElement $entity, \DateTimeInterface $dat Group::class => 'group_edit', LabelProfile::class => 'label_profile_edit', PartCustomState::class => 'part_custom_state_edit', + ParameterDefinition::class => 'parameter_definition_edit', ]; try { @@ -216,6 +218,7 @@ public function infoURL(AbstractDBElement $entity): string Group::class => 'group_edit', LabelProfile::class => 'label_profile_edit', PartCustomState::class => 'part_custom_state_edit', + ParameterDefinition::class => 'parameter_definition_edit', ]; return $this->urlGenerator->generate($this->mapToController($map, $entity), ['id' => $entity->getID()]); @@ -247,6 +250,7 @@ public function editURL(AbstractDBElement $entity): string Group::class => 'group_edit', LabelProfile::class => 'label_profile_edit', PartCustomState::class => 'part_custom_state_edit', + ParameterDefinition::class => 'parameter_definition_edit', ]; return $this->urlGenerator->generate($this->mapToController($map, $entity), ['id' => $entity->getID()]); @@ -279,6 +283,7 @@ public function createURL(AbstractDBElement|string $entity): string Group::class => 'group_new', LabelProfile::class => 'label_profile_new', PartCustomState::class => 'part_custom_state_new', + ParameterDefinition::class => 'parameter_definition_new', ]; return $this->urlGenerator->generate($this->mapToController($map, $entity)); @@ -311,6 +316,7 @@ public function cloneURL(AbstractDBElement $entity): string Group::class => 'group_clone', LabelProfile::class => 'label_profile_clone', PartCustomState::class => 'part_custom_state_clone', + ParameterDefinition::class => 'parameter_definition_clone', ]; return $this->urlGenerator->generate($this->mapToController($map, $entity), ['id' => $entity->getID()]); @@ -357,6 +363,7 @@ public function deleteURL(AbstractDBElement $entity): string Group::class => 'group_delete', LabelProfile::class => 'label_profile_delete', PartCustomState::class => 'part_custom_state_delete', + ParameterDefinition::class => 'parameter_definition_delete', ]; return $this->urlGenerator->generate($this->mapToController($map, $entity), ['id' => $entity->getID()]); diff --git a/src/Services/LogSystem/TimeTravel.php b/src/Services/LogSystem/TimeTravel.php index 79d9f8e20..7649b8afd 100644 --- a/src/Services/LogSystem/TimeTravel.php +++ b/src/Services/LogSystem/TimeTravel.php @@ -30,6 +30,8 @@ use App\Entity\LogSystem\AbstractLogEntry; use App\Entity\LogSystem\CollectionElementDeleted; use App\Entity\LogSystem\ElementEditedLogEntry; +use App\Entity\Parameters\AbstractParameter; +use App\Entity\Parameters\ParameterDefinition; use App\Repository\LogEntryRepository; use Brick\Math\BigDecimal; use DateTime; @@ -135,6 +137,9 @@ public function revertEntityToTimestamp(AbstractDBElement $element, \DateTimeInt if ( ($element instanceof AbstractStructuralDBElement && ('parts' === $field || 'children' === $field)) || ($element instanceof AttachmentType && 'attachments' === $field) + //applyEntry() restores the parameter's historical definition reference. Only skip recursively + //reverting the shared global definition object itself; historical metadata comes from snapshots. + || ($element instanceof AbstractParameter && 'definition' === $field) ) { continue; } @@ -239,6 +244,17 @@ public function applyEntry(AbstractDBElement $element, TimeTravelInterface $logE $this->setField($element, $field, $data); } if ($metadata->hasAssociation($field)) { + if ($element instanceof AbstractParameter && 'definition' === $field) { + if (null === $data) { + $element->restoreDefinitionReference(null); + } elseif (is_array($data) && isset($data['@id'])) { + $definition = $this->em->getReference(ParameterDefinition::class, $data['@id']); + $element->restoreDefinitionReference($definition); + } + + continue; + } + $mapping = $metadata->getAssociationMapping($field); $target_class = $mapping['targetEntity']; //Try to extract the old ID: diff --git a/src/Services/Trees/ToolsTreeBuilder.php b/src/Services/Trees/ToolsTreeBuilder.php index 97792aa81..1b0be7a95 100644 --- a/src/Services/Trees/ToolsTreeBuilder.php +++ b/src/Services/Trees/ToolsTreeBuilder.php @@ -32,6 +32,7 @@ use App\Entity\Parts\PartCustomState; use App\Entity\Parts\StorageLocation; use App\Entity\Parts\Supplier; +use App\Entity\Parameters\ParameterDefinition; use App\Entity\PriceInformations\Currency; use App\Entity\ProjectSystem\Project; use App\Entity\UserSystem\Group; @@ -232,6 +233,12 @@ protected function getEditNodes(): array $this->urlGenerator->generate('measurement_unit_new') ))->setIcon('fa-fw fa-treeview fa-solid fa-balance-scale'); } + if ($this->security->isGranted('read', new ParameterDefinition())) { + $nodes[] = (new TreeViewNode( + $this->elementTypeNameGenerator->typeLabelPlural(ParameterDefinition::class), + $this->urlGenerator->generate('parameter_definition_new') + ))->setIcon('fa-fw fa-treeview fa-solid fa-list-check'); + } if ($this->security->isGranted('read', new LabelProfile())) { $nodes[] = (new TreeViewNode( $this->elementTypeNameGenerator->typeLabelPlural(LabelProfile::class), diff --git a/src/Services/UserSystem/PermissionPresetsHelper.php b/src/Services/UserSystem/PermissionPresetsHelper.php index 6da9a0040..1025c0be9 100644 --- a/src/Services/UserSystem/PermissionPresetsHelper.php +++ b/src/Services/UserSystem/PermissionPresetsHelper.php @@ -108,6 +108,7 @@ private function admin(HasPermissionsInterface $perm_holder): void $this->permissionResolver->setAllOperationsOfPermission($perm_holder, 'part_custom_states', PermissionData::ALLOW); $this->permissionResolver->setAllOperationsOfPermission($perm_holder, 'suppliers', PermissionData::ALLOW); $this->permissionResolver->setAllOperationsOfPermission($perm_holder, 'projects', PermissionData::ALLOW); + $this->permissionResolver->setAllOperationsOfPermission($perm_holder, 'parameter_definitions', PermissionData::ALLOW); //Allow to change system settings $this->permissionResolver->setPermission($perm_holder, 'config', 'change_system_settings', PermissionData::ALLOW); diff --git a/src/Services/UserSystem/PermissionSchemaUpdater.php b/src/Services/UserSystem/PermissionSchemaUpdater.php index fd85ee7ca..8da515a5d 100644 --- a/src/Services/UserSystem/PermissionSchemaUpdater.php +++ b/src/Services/UserSystem/PermissionSchemaUpdater.php @@ -173,4 +173,24 @@ private function upgradeSchemaToVersion4(HasPermissionsInterface $holder): void $permissions->setPermissionValue('parts_stock', 'stocktake', $new_value); } } + + private function upgradeSchemaToVersion5(HasPermissionsInterface $holder): void //@phpstan-ignore-line This is called via reflection + { + $permissions = $holder->getPermissions(); + + if (!$permissions->isAnyOperationOfPermissionSet('parameter_definitions')) { + // Definitions are required for displaying and using parameters, but managing + // the global library remains restricted to existing system administrators. + $permissions->setPermissionValue( + 'parameter_definitions', + 'read', + $permissions->getPermissionValue('parts', 'read') + ); + + $management_value = $permissions->getPermissionValue('config', 'change_system_settings'); + foreach (['edit', 'create', 'delete', 'show_history', 'revert_element', 'import'] as $operation) { + $permissions->setPermissionValue('parameter_definitions', $operation, $management_value); + } + } + } } diff --git a/templates/admin/base_admin.html.twig b/templates/admin/base_admin.html.twig index f19f4c440..6135d7da6 100644 --- a/templates/admin/base_admin.html.twig +++ b/templates/admin/base_admin.html.twig @@ -83,9 +83,11 @@ {% trans %}admin.common{% endtrans %} {% block additional_pills %}{% endblock %} -
  • - {% trans %}admin.attachments{% endtrans %} -
    • + {% if form.attachments is defined %} +
  • + {% trans %}admin.attachments{% endtrans %} +
    • + {% endif %} {% if entity.parameters is defined and showParameters == true %}
  • {% trans %}admin.parameters{% endtrans %} @@ -114,12 +116,14 @@
    • {% block additional_panes %}{% endblock %} -
    - {% include "admin/_attachments.html.twig" %} - {% block master_picture_block %} - {{ form_row(form.master_picture_attachment) }} - {% endblock %} -
    + {% if form.attachments is defined %} +
    + {% include "admin/_attachments.html.twig" %} + {% block master_picture_block %} + {{ form_row(form.master_picture_attachment) }} + {% endblock %} +
    + {% endif %} {% if entity.parameters is defined %}
    diff --git a/templates/admin/parameter_definition_admin.html.twig b/templates/admin/parameter_definition_admin.html.twig new file mode 100644 index 000000000..c66b86b04 --- /dev/null +++ b/templates/admin/parameter_definition_admin.html.twig @@ -0,0 +1,24 @@ +{% extends "admin/base_admin.html.twig" %} + +{% block card_title %} + {{ type_label_p(entity) }} +{% endblock %} + +{% block edit_title %} + {% trans %}parameter_definition.edit{% endtrans %}: {{ entity.name }} +{% endblock %} + +{% block new_title %} + {% trans %}parameter_definition.new{% endtrans %} +{% endblock %} + +{% block preview_picture %}{% endblock %} + +{% block additional_controls %} + {{ form_row(form.symbol) }} + {{ form_row(form.unit) }} + {{ form_row(form.input_type) }} + {{ form_row(form.choices_text) }} +{% endblock %} + +{% block comment %}{% endblock %} diff --git a/templates/parts/edit/edit_form_styles.html.twig b/templates/parts/edit/edit_form_styles.html.twig index 765f200c3..817ca7670 100644 --- a/templates/parts/edit/edit_form_styles.html.twig +++ b/templates/parts/edit/edit_form_styles.html.twig @@ -78,7 +78,16 @@ {{ form_widget(form.value_typical) }}{{ form_errors(form.value_typical) }} {{ form_widget(form.value_max) }}{{ form_errors(form.value_max) }} {{ form_widget(form.unit, {"attr": {"data-pages--parameters-autocomplete-target": "unit", "data-pages--latex-preview-target": "input"}}) }}{{ form_errors(form.unit) }} - {{ form_widget(form.value_text) }}{{ form_errors(form.value_text) }} + + {{ form_widget(form.value_text, {"attr": {"data-pages--parameters-autocomplete-target": "valueText"}}) }} + {{ form_errors(form.value_text) }} + + {% if form.definition is defined %} + {{ form_widget(form.definition, {"attr": {"data-pages--parameters-autocomplete-target": "definition"}}) }} + {{ form_errors(form.definition) }} + {% endif %} + + {{ form_widget(form.group) }}{{ form_errors(form.group) }} {% if form.eda_visibility is defined %} {{ form_widget(form.eda_visibility) }} diff --git a/tests/API/Endpoints/ParameterDefinitionsEndpointTest.php b/tests/API/Endpoints/ParameterDefinitionsEndpointTest.php new file mode 100644 index 000000000..d09c1d075 --- /dev/null +++ b/tests/API/Endpoints/ParameterDefinitionsEndpointTest.php @@ -0,0 +1,118 @@ +createDefinitionClient(); + $client->request('GET', self::BASE_PATH); + + self::assertResponseIsSuccessful(); + self::assertResponseHeaderSame('content-type', 'application/ld+json; charset=utf-8'); + } + + public function testCrudLifecycleAndChoiceCanonicalization(): void + { + $client = $this->createDefinitionClient(); + $response = $client->request('POST', self::BASE_PATH, [ + 'json' => [ + 'name' => 'API dielectric definition', + 'symbol' => 'D', + 'unit' => 'kind', + 'input_type' => ParameterDefinition::INPUT_TYPE_CHOICE, + 'choices' => ['X7R', ' x7r ', '', 'X5R'], + ], + ]); + self::assertResponseIsSuccessful(); + $id = $response->toArray(true)['id']; + self::assertIsInt($id); + self::assertJsonContains([ + 'name' => 'API dielectric definition', + 'input_type' => ParameterDefinition::INPUT_TYPE_CHOICE, + 'choices' => ['X7R', 'X5R'], + ]); + + $client->request('GET', self::BASE_PATH.'/'.$id); + self::assertResponseIsSuccessful(); + $client->request('PATCH', self::BASE_PATH.'/'.$id, [ + 'json' => [ + 'name' => 'API dielectric type', + 'choices' => ['C0G', 'NP0'], + ], + 'headers' => ['Content-Type' => 'application/merge-patch+json'], + ]); + self::assertResponseIsSuccessful(); + self::assertJsonContains([ + 'name' => 'API dielectric type', + 'choices' => ['C0G', 'NP0'], + ]); + + $client->request('DELETE', self::BASE_PATH.'/'.$id); + self::assertResponseIsSuccessful(); + } + + public function testReadOnlyTokenCannotCreateDefinition(): void + { + $this->createDefinitionClient(APITokenFixtures::TOKEN_READONLY)->request('POST', self::BASE_PATH, [ + 'json' => [ + 'name' => 'Forbidden API definition', + 'input_type' => ParameterDefinition::INPUT_TYPE_TEXT, + ], + ]); + + self::assertResponseStatusCodeSame(403); + } + + public function testUsedDefinitionCannotBeDeletedThroughApi(): void + { + $client = $this->createDefinitionClient(); + $definition = (new ParameterDefinition())->setName('Used API definition'); + $parameter = (new PartParameter())->setDefinition($definition)->setValueText('value'); + $category = (new Category())->setName('Used API definition category'); + $part = (new Part()) + ->setName('Used API definition part') + ->setCategory($category) + ->addParameter($parameter); + $em = self::getContainer()->get(EntityManagerInterface::class); + $em->persist($definition); + $em->persist($category); + $em->persist($part); + $em->flush(); + $id = $definition->getID(); + self::assertNotNull($id); + + $client->request('DELETE', self::BASE_PATH.'/'.$id); + + self::assertResponseStatusCodeSame(409); + self::assertJsonContains(['detail' => 'This parameter definition is still in use and cannot be deleted.']); + } + + private function createDefinitionClient(string $token = APITokenFixtures::TOKEN_ADMIN): Client + { + $client = self::createAuthenticatedClient($token); + $em = self::getContainer()->get(EntityManagerInterface::class); + $admin = $em->getRepository(User::class)->findOneBy(['name' => 'admin']); + self::assertInstanceOf(User::class, $admin); + self::getContainer()->get(PermissionSchemaUpdater::class)->userUpgradeSchemaRecursively($admin); + $em->flush(); + + return $client; + } +} diff --git a/tests/API/Endpoints/ParametersEndpointTest.php b/tests/API/Endpoints/ParametersEndpointTest.php index 323fc7b86..4bacb7b1f 100644 --- a/tests/API/Endpoints/ParametersEndpointTest.php +++ b/tests/API/Endpoints/ParametersEndpointTest.php @@ -23,6 +23,11 @@ namespace App\Tests\API\Endpoints; +use App\Entity\Parameters\ParameterDefinition; +use App\Entity\UserSystem\User; +use App\Services\UserSystem\PermissionSchemaUpdater; +use Doctrine\ORM\EntityManagerInterface; + final class ParametersEndpointTest extends CrudEndpointTestCase { @@ -59,4 +64,54 @@ public function testElementLifecycle(): void //Check if we can delete the item $this->_testDeleteItem($id); } -} \ No newline at end of file + + public function testEffectiveDefinitionMetadataIsExposedWithoutDuplicatedWritableFields(): void + { + $client = self::createAuthenticatedClient(); + $definition = (new ParameterDefinition()) + ->setName('API parameter dielectric') + ->setInputType(ParameterDefinition::INPUT_TYPE_CHOICE) + ->setChoices(['C0G', 'X7R']); + $entity_manager = self::getContainer()->get(EntityManagerInterface::class); + $admin = $entity_manager->getRepository(User::class)->findOneBy(['name' => 'admin']); + self::assertInstanceOf(User::class, $admin); + self::getContainer()->get(PermissionSchemaUpdater::class)->userUpgradeSchemaRecursively($admin); + $entity_manager->flush(); + $entity_manager->persist($definition); + $entity_manager->flush(); + $definition_id = $definition->getID(); + self::assertNotNull($definition_id); + + $response = $client->request('POST', $this->getBasePath(), [ + 'json' => [ + 'name' => 'API parameter dielectric', + 'element' => '/api/parts/1', + 'definition' => '/api/parameter_definitions/'.$definition_id, + 'value_text' => 'X7R', + ], + ]); + self::assertResponseIsSuccessful(); + + self::assertJsonContains([ + 'definition' => '/api/parameter_definitions/'.$definition_id, + 'input_type' => ParameterDefinition::INPUT_TYPE_CHOICE, + 'choices' => ['C0G', 'X7R'], + 'value_text' => 'X7R', + ]); + } + + public function testParameterWithoutDefinitionRemainsFreeText(): void + { + $response = $this->_testPostItem([ + 'name' => 'API free text parameter', + 'element' => '/api/parts/1', + 'value_text' => 'arbitrary value', + ]); + + self::assertJsonContains([ + 'input_type' => ParameterDefinition::INPUT_TYPE_TEXT, + 'choices' => [], + 'value_text' => 'arbitrary value', + ]); + } +} diff --git a/tests/Controller/AdminPages/ParameterDefinitionControllerTest.php b/tests/Controller/AdminPages/ParameterDefinitionControllerTest.php new file mode 100644 index 000000000..d9cb854c1 --- /dev/null +++ b/tests/Controller/AdminPages/ParameterDefinitionControllerTest.php @@ -0,0 +1,282 @@ + ['noread', false]; + yield 'read-only user' => ['anonymous', true]; + yield 'part editor' => ['user', true]; + yield 'administrator' => ['admin', true]; + } + + #[DataProvider('readAccessProvider')] + public function testAdminReadAccessUsesDedicatedPermissions(string $username, bool $allowed): void + { + $client = $this->createAuthenticatedClient($username); + $client->request('GET', '/en/parameter_definition/new'); + + self::assertSame($allowed, $client->getResponse()->isSuccessful()); + self::assertSame(!$allowed, $client->getResponse()->isForbidden()); + } + + public function testPartEditorCanReadButCannotManageDefinitions(): void + { + $client = $this->createAuthenticatedClient('user'); + $crawler = $client->request('GET', '/en/parameter_definition/new'); + + self::assertResponseIsSuccessful(); + self::assertSame(1, $crawler->filter('#parameter_definition_admin_form_name[disabled]')->count()); + self::assertSame(1, $crawler->filter('#parameter_definition_admin_form_save[disabled]')->count()); + } + + public function testToolsTreeContainsParameterDefinitionsEntry(): void + { + $client = $this->createAuthenticatedClient(); + $client->request('GET', '/en/tree/tools'); + + self::assertResponseIsSuccessful(); + $tree = json_decode((string) $client->getResponse()->getContent(), true, flags: JSON_THROW_ON_ERROR); + self::assertStringContainsString('/en/parameter_definition/new', json_encode($tree, JSON_UNESCAPED_SLASHES | JSON_THROW_ON_ERROR)); + } + + public function testCreateTextDefinitionClearsSubmittedChoices(): void + { + $client = $this->createAuthenticatedClient(); + $crawler = $client->request('GET', '/en/parameter_definition/new'); + self::assertResponseIsSuccessful(); + self::assertSame(0, $crawler->filter('a[href="#attachments"]')->count()); + + $crawler = $this->submitDefinitionForm($client, $crawler, [ + 'name' => 'Controller text definition', + 'input_type' => ParameterDefinition::INPUT_TYPE_TEXT, + 'choices_text' => "Ignored\nvalues", + ]); + + self::assertTrue( + $client->getResponse()->isRedirect(), + sprintf( + 'HTTP %d: %s', + $client->getResponse()->getStatusCode(), + implode(' | ', $crawler->filter('.invalid-feedback, form[name="parameter_definition_admin_form"] li')->each(static fn (Crawler $node): string => $node->text())), + ), + ); + $definition = $this->findDefinition('Controller text definition'); + self::assertSame(ParameterDefinition::INPUT_TYPE_TEXT, $definition->getInputType()); + self::assertSame([], $definition->getChoices()); + } + + public function testCreateChoiceDefinitionCanonicalizesChoices(): void + { + $client = $this->createAuthenticatedClient(); + $crawler = $client->request('GET', '/en/parameter_definition/new'); + + $this->submitDefinitionForm($client, $crawler, [ + 'name' => 'Controller choice definition', + 'symbol' => 'D', + 'unit' => 'kind', + 'input_type' => ParameterDefinition::INPUT_TYPE_CHOICE, + 'choices_text' => " X7R \nx7r\n\nX7r\nX5R ", + ]); + + self::assertResponseRedirects(); + $definition = $this->findDefinition('Controller choice definition'); + self::assertSame(['X7R', 'X5R'], $definition->getChoices()); + self::assertSame('D', $definition->getSymbol()); + self::assertSame('kind', $definition->getUnit()); + } + + public function testPersistedDefinitionAppearsImmediatelyInAdminTree(): void + { + $client = $this->createAuthenticatedClient(); + + // Opening the creation page primes the generic tree cache before the definition exists. + $crawler = $client->request('GET', '/en/parameter_definition/new'); + self::assertResponseIsSuccessful(); + + $this->submitDefinitionForm($client, $crawler, [ + 'name' => 'Visible admin tree definition', + 'input_type' => ParameterDefinition::INPUT_TYPE_CHOICE, + 'choices_text' => "C0G\nX7R", + ]); + self::assertResponseRedirects(); + + $crawler = $client->followRedirect(); + self::assertResponseIsSuccessful(); + $tree_data = $crawler->filter('[data-controller="elements--tree"]')->attr('data-tree-data'); + self::assertNotNull($tree_data); + $tree = json_decode($tree_data, true, flags: JSON_THROW_ON_ERROR); + + self::assertStringContainsString( + 'Visible admin tree definition', + json_encode($tree, JSON_UNESCAPED_SLASHES | JSON_THROW_ON_ERROR), + ); + } + + public function testEditDefinitionCreatesHistoryEntry(): void + { + $client = $this->createAuthenticatedClient(); + $definition = (new ParameterDefinition()) + ->setName('Controller history definition') + ->setInputType(ParameterDefinition::INPUT_TYPE_CHOICE) + ->setChoices(['C0G', 'X7R']); + $em = $this->entityManager(); + $em->persist($definition); + $em->flush(); + $id = $definition->getID(); + self::assertNotNull($id); + + $crawler = $client->request('GET', '/en/parameter_definition/'.$id.'/edit'); + self::assertResponseIsSuccessful(); + self::assertSame(1, $crawler->filter('a[href="#history"]')->count()); + + $this->submitDefinitionForm($client, $crawler, [ + 'name' => 'Controller renamed definition', + 'input_type' => ParameterDefinition::INPUT_TYPE_CHOICE, + 'choices_text' => "C0G\nX7R\nX5R", + ]); + self::assertResponseIsSuccessful(); + + $em->clear(); + $reloaded = $em->find(ParameterDefinition::class, $id); + self::assertInstanceOf(ParameterDefinition::class, $reloaded); + self::assertSame('Controller renamed definition', $reloaded->getName()); + self::assertSame(['C0G', 'X7R', 'X5R'], $reloaded->getChoices()); + + $log_repository = $em->getRepository(AbstractLogEntry::class); + self::assertInstanceOf(LogEntryRepository::class, $log_repository); + self::assertNotEmpty(array_filter( + $log_repository->getElementHistory($reloaded), + static fn (AbstractLogEntry $entry): bool => $entry instanceof ElementEditedLogEntry, + )); + } + + public function testCaseInsensitiveDuplicateNameIsAFormError(): void + { + $client = $this->createAuthenticatedClient(); + $em = $this->entityManager(); + $em->persist((new ParameterDefinition())->setName('Controller Unique Name')); + $em->flush(); + + $crawler = $client->request('GET', '/en/parameter_definition/new'); + $crawler = $this->submitDefinitionForm($client, $crawler, [ + 'name' => 'controller unique name', + 'input_type' => ParameterDefinition::INPUT_TYPE_TEXT, + ]); + + self::assertResponseStatusCodeSame(422); + self::assertStringContainsString( + 'A parameter definition with this name already exists.', + $crawler->filter('body')->text(), + ); + } + + public function testUnusedDefinitionCanBeDeleted(): void + { + $client = $this->createAuthenticatedClient(); + $definition = (new ParameterDefinition())->setName('Controller unused deletion'); + $em = $this->entityManager(); + $em->persist($definition); + $em->flush(); + $id = $definition->getID(); + self::assertNotNull($id); + + $this->deleteDefinition($client, $id); + + self::assertResponseRedirects('/en/parameter_definition/new'); + $em->clear(); + self::assertNull($em->find(ParameterDefinition::class, $id)); + } + + public function testUsedDefinitionDeletionIsRefusedCleanly(): void + { + $client = $this->createAuthenticatedClient(); + $definition = (new ParameterDefinition())->setName('Controller used deletion'); + $parameter = (new PartParameter())->setDefinition($definition)->setValueText('value'); + $category = (new Category())->setName('Controller deletion category'); + $part = (new Part()) + ->setName('Controller deletion part') + ->setCategory($category) + ->addParameter($parameter); + $em = $this->entityManager(); + $em->persist($definition); + $em->persist($category); + $em->persist($part); + $em->flush(); + $id = $definition->getID(); + self::assertNotNull($id); + + $this->deleteDefinition($client, $id); + + self::assertResponseRedirects('/en/parameter_definition/'.$id.'/edit'); + $crawler = $client->followRedirect(); + self::assertStringContainsString( + 'This parameter definition is still in use and cannot be deleted.', + $crawler->filter('body')->text(), + ); + $em->clear(); + self::assertInstanceOf(ParameterDefinition::class, $em->find(ParameterDefinition::class, $id)); + } + + private function createAuthenticatedClient(string $username = 'admin'): KernelBrowser + { + return static::createClient([], [ + 'PHP_AUTH_USER' => $username, + 'PHP_AUTH_PW' => 'test', + ]); + } + + /** @param array $values */ + private function submitDefinitionForm(KernelBrowser $client, Crawler $crawler, array $values): Crawler + { + $form = $crawler->filter('form[name="parameter_definition_admin_form"]')->form(); + $submitted = []; + foreach ($values as $field => $value) { + $submitted['parameter_definition_admin_form['.$field.']'] = $value; + } + + return $client->submit($form, $submitted); + } + + private function deleteDefinition(KernelBrowser $client, int $id): void + { + $crawler = $client->request('GET', '/en/parameter_definition/'.$id.'/edit'); + self::assertResponseIsSuccessful(); + $delete_form = $crawler->filter('form[action="/en/parameter_definition/'.$id.'"]')->form(); + $client->submit($delete_form); + } + + private function findDefinition(string $name): ParameterDefinition + { + $definition = $this->entityManager()->getRepository(ParameterDefinition::class) + ->findOneBy(['normalized_name' => mb_strtolower($name)]); + self::assertInstanceOf(ParameterDefinition::class, $definition); + + return $definition; + } + + private function entityManager(): EntityManagerInterface + { + return self::getContainer()->get(EntityManagerInterface::class); + } +} diff --git a/tests/Controller/TypeaheadControllerTest.php b/tests/Controller/TypeaheadControllerTest.php index ce2747fae..15cfac5eb 100644 --- a/tests/Controller/TypeaheadControllerTest.php +++ b/tests/Controller/TypeaheadControllerTest.php @@ -23,6 +23,7 @@ namespace App\Tests\Controller; +use App\Entity\Parameters\ParameterDefinition; use App\Entity\UserSystem\User; use Doctrine\ORM\EntityManagerInterface; use PHPUnit\Framework\Attributes\DataProvider; @@ -124,6 +125,54 @@ public function testPartsSearchReturnsArrayWithExpectedKeys(): void } } + public function testPartParameterAutocompleteExposesChoiceDefinitionMetadata(): void + { + $client = $this->loginClient('admin'); + $definition = (new ParameterDefinition()) + ->setName('Checkpoint 3 dielectric') + ->setSymbol('D') + ->setUnit('grade') + ->setInputType(ParameterDefinition::INPUT_TYPE_CHOICE) + ->setChoices(['X7R', 'X5R', 'C0G']); + $em = static::getContainer()->get(EntityManagerInterface::class); + $em->persist($definition); + $em->flush(); + + $client->request('GET', '/en/typeahead/parameters/part/search/Checkpoint%203%20dielectric'); + + self::assertResponseIsSuccessful(); + $data = json_decode($client->getResponse()->getContent(), true, flags: JSON_THROW_ON_ERROR); + self::assertIsArray($data); + $suggestion = $this->findSuggestion($data, 'Checkpoint 3 dielectric'); + self::assertIsArray($suggestion); + self::assertSame($definition->getID(), $suggestion['definition_id']); + self::assertSame('D', $suggestion['symbol']); + self::assertSame('grade', $suggestion['unit']); + self::assertSame(ParameterDefinition::INPUT_TYPE_CHOICE, $suggestion['input_type']); + self::assertSame(['X7R', 'X5R', 'C0G'], $suggestion['choices']); + } + + public function testPartParameterAutocompleteExposesTextDefinitionWithoutChoices(): void + { + $client = $this->loginClient('admin'); + $definition = (new ParameterDefinition()) + ->setName('Checkpoint 3 manufacturer code') + ->setInputType(ParameterDefinition::INPUT_TYPE_TEXT); + $em = static::getContainer()->get(EntityManagerInterface::class); + $em->persist($definition); + $em->flush(); + + $client->request('GET', '/en/typeahead/parameters/part/search/Checkpoint%203%20manufacturer%20code'); + + self::assertResponseIsSuccessful(); + $data = json_decode($client->getResponse()->getContent(), true, flags: JSON_THROW_ON_ERROR); + self::assertIsArray($data); + $suggestion = $this->findSuggestion($data, 'Checkpoint 3 manufacturer code'); + self::assertIsArray($suggestion); + self::assertSame(ParameterDefinition::INPUT_TYPE_TEXT, $suggestion['input_type']); + self::assertSame([], $suggestion['choices']); + } + // ----------------------------------------------------------------------- // Access control // ----------------------------------------------------------------------- @@ -159,4 +208,19 @@ public function testEditorUserCanAccess(string $url): void $this->assertResponseIsSuccessful(); } + + /** + * @param array $suggestions + * @return array|null + */ + private function findSuggestion(array $suggestions, string $name): ?array + { + foreach ($suggestions as $suggestion) { + if (is_array($suggestion) && $name === ($suggestion['name'] ?? null)) { + return $suggestion; + } + } + + return null; + } } diff --git a/tests/Doctrine/ParameterDefinitionSchemaTest.php b/tests/Doctrine/ParameterDefinitionSchemaTest.php new file mode 100644 index 000000000..bc4419cc7 --- /dev/null +++ b/tests/Doctrine/ParameterDefinitionSchemaTest.php @@ -0,0 +1,47 @@ +get(Connection::class); + $schema_manager = $connection->createSchemaManager(); + + self::assertTrue($schema_manager->tablesExist(['parameter_definitions'])); + $definition_table = $schema_manager->introspectTable('parameter_definitions'); + self::assertTrue($definition_table->hasColumn('normalized_name')); + self::assertTrue($definition_table->hasColumn('input_type')); + self::assertTrue($definition_table->hasColumn('choices')); + self::assertTrue($definition_table->hasIndex('parameter_definition_normalized_name_unique')); + self::assertTrue($definition_table->getIndex('parameter_definition_normalized_name_unique')->isUnique()); + + $parameter_table = $schema_manager->introspectTable('parameters'); + self::assertFalse($parameter_table->hasColumn('input_type')); + self::assertFalse($parameter_table->hasColumn('choices')); + self::assertTrue($parameter_table->hasColumn('definition_id')); + self::assertTrue($parameter_table->getColumn('definition_id')->getNotnull() === false); + self::assertTrue($parameter_table->hasIndex('parameter_definition_value_idx')); + self::assertSame( + ['definition_id', 'value_text', 'type', 'element_id'], + $parameter_table->getIndex('parameter_definition_value_idx')->getColumns(), + ); + + $foreign_keys = array_values(array_filter( + $parameter_table->getForeignKeys(), + static fn ($foreign_key): bool => ['definition_id'] === $foreign_key->getLocalColumns(), + )); + self::assertCount(1, $foreign_keys); + self::assertSame('parameter_definitions', $foreign_keys[0]->getForeignTableName()); + } +} diff --git a/tests/Entity/LogSystem/LogTargetTypeTest.php b/tests/Entity/LogSystem/LogTargetTypeTest.php index 06e2ead1d..597ed9872 100644 --- a/tests/Entity/LogSystem/LogTargetTypeTest.php +++ b/tests/Entity/LogSystem/LogTargetTypeTest.php @@ -25,6 +25,7 @@ use App\Entity\Attachments\Attachment; use App\Entity\Attachments\PartAttachment; use App\Entity\LogSystem\LogTargetType; +use App\Entity\Parameters\ParameterDefinition; use App\Entity\Parameters\PartParameter; use App\Entity\Parts\Category; use App\Entity\UserSystem\User; @@ -54,6 +55,7 @@ public function testFromElementClass(): void //Test creation from subclass $this->assertSame(LogTargetType::ATTACHMENT, LogTargetType::fromElementClass(new PartAttachment())); $this->assertSame(LogTargetType::PARAMETER, LogTargetType::fromElementClass(new PartParameter())); + $this->assertSame(LogTargetType::PARAMETER_DEFINITION, LogTargetType::fromElementClass(new ParameterDefinition())); } public function testFromElementClassInvalid(): void diff --git a/tests/Entity/Parameters/ParameterDefinitionDoctrineTest.php b/tests/Entity/Parameters/ParameterDefinitionDoctrineTest.php new file mode 100644 index 000000000..fef6d6f3a --- /dev/null +++ b/tests/Entity/Parameters/ParameterDefinitionDoctrineTest.php @@ -0,0 +1,131 @@ +entityManager = self::getContainer()->get(EntityManagerInterface::class); + } + + public function testDefinitionRelationAndVisibleSnapshotsArePersisted(): void + { + $definition = (new ParameterDefinition()) + ->setName('Doctrine dielectric') + ->setSymbol('D') + ->setInputType(ParameterDefinition::INPUT_TYPE_CHOICE) + ->setChoices(['C0G', 'X7R']); + $parameter = (new PartParameter())->setDefinition($definition)->setValueText('X7R'); + $category = (new Category())->setName('Definition relation category'); + $part = (new Part()) + ->setName('Definition relation part') + ->setCategory($category) + ->addParameter($parameter); + + $this->entityManager->persist($definition); + $this->entityManager->persist($category); + $this->entityManager->persist($part); + $this->entityManager->flush(); + $part_id = $part->getID(); + $definition_id = $definition->getID(); + self::assertNotNull($part_id); + self::assertNotNull($definition_id); + + $this->entityManager->clear(); + $reloaded_part = $this->entityManager->find(Part::class, $part_id); + self::assertInstanceOf(Part::class, $reloaded_part); + $reloaded_parameter = $reloaded_part->getParameters()->first(); + self::assertInstanceOf(PartParameter::class, $reloaded_parameter); + self::assertSame($definition_id, $reloaded_parameter->getDefinition()?->getID()); + self::assertSame('Doctrine dielectric', $reloaded_parameter->getSnapshotName()); + self::assertSame(['C0G', 'X7R'], $reloaded_parameter->getEffectiveChoices()); + + $reloaded_definition = $reloaded_parameter->getDefinition(); + self::assertInstanceOf(ParameterDefinition::class, $reloaded_definition); + $reloaded_definition->setName('Doctrine dielectric type')->setChoices(['C0G', 'X7R', 'X5R']); + $this->entityManager->flush(); + $this->entityManager->clear(); + + $reloaded_part = $this->entityManager->find(Part::class, $part_id); + self::assertInstanceOf(Part::class, $reloaded_part); + $reloaded_parameter = $reloaded_part->getParameters()->first(); + self::assertInstanceOf(PartParameter::class, $reloaded_parameter); + self::assertSame('Doctrine dielectric', $reloaded_parameter->getSnapshotName()); + self::assertSame('Doctrine dielectric type', $reloaded_parameter->getEffectiveName()); + self::assertSame(['C0G', 'X7R', 'X5R'], $reloaded_parameter->getEffectiveChoices()); + } + + public function testLinkedChoiceValueIsValidatedAgainstTheDefinition(): void + { + $definition = (new ParameterDefinition()) + ->setName('Validated dielectric') + ->setInputType(ParameterDefinition::INPUT_TYPE_CHOICE) + ->setChoices(['C0G', 'X7R']); + $parameter = (new PartParameter())->setDefinition($definition)->setValueText('Y5V'); + + $validator = self::getContainer()->get(ValidatorInterface::class); + $violations = $validator->validate($parameter); + + self::assertGreaterThan(0, $violations->count()); + self::assertSame('value_text', $violations[0]->getPropertyPath()); + } + + public function testDefinitionAssociationMetadataIsNullableAndRestrictive(): void + { + $metadata = $this->entityManager->getClassMetadata(AbstractParameter::class); + $mapping = $metadata->getAssociationMapping('definition'); + + self::assertTrue($mapping['joinColumns'][0]->nullable); + self::assertSame('RESTRICT', $mapping['joinColumns'][0]->onDelete); + self::assertSame(ParameterDefinition::class, $mapping['targetEntity']); + } + + public function testNormalizedNameLifecycleCallbackRunsAfterDirectNameRestoration(): void + { + $metadata = $this->entityManager->getClassMetadata(ParameterDefinition::class); + self::assertContains('updateNormalizedName', $metadata->getLifecycleCallbacks(Events::prePersist)); + self::assertContains('updateNormalizedName', $metadata->getLifecycleCallbacks(Events::preUpdate)); + + $definition = (new ParameterDefinition())->setName('Lifecycle original name'); + $this->entityManager->persist($definition); + $this->entityManager->flush(); + + (new \ReflectionClass($definition))->getProperty('name')->setValue($definition, 'Lifecycle Restored Name'); + $this->entityManager->flush(); + $definition_id = $definition->getID(); + self::assertNotNull($definition_id); + + $this->entityManager->clear(); + $reloaded_definition = $this->entityManager->find(ParameterDefinition::class, $definition_id); + self::assertInstanceOf(ParameterDefinition::class, $reloaded_definition); + self::assertSame('Lifecycle Restored Name', $reloaded_definition->getName()); + self::assertSame('lifecycle restored name', $reloaded_definition->getNormalizedName()); + } + + public function testCompleteDoctrineMappingIsValid(): void + { + $errors = (new SchemaValidator($this->entityManager))->validateMapping(); + + self::assertSame([], $errors, var_export($errors, true)); + } +} diff --git a/tests/Entity/Parameters/ParameterDefinitionTest.php b/tests/Entity/Parameters/ParameterDefinitionTest.php new file mode 100644 index 000000000..0ae9af1bf --- /dev/null +++ b/tests/Entity/Parameters/ParameterDefinitionTest.php @@ -0,0 +1,128 @@ +setInputType(ParameterDefinition::INPUT_TYPE_CHOICE) + ->setChoicesText(" X7R \r\nx7r\n\nX7r\n X5R \n"); + + self::assertSame(['X7R', 'X5R'], $definition->getChoices()); + self::assertSame("X7R\nX5R", $definition->getChoicesText()); + } + + public function testAddingAChoiceReusesTheExistingCanonicalSpelling(): void + { + $definition = (new ParameterDefinition()) + ->setInputType(ParameterDefinition::INPUT_TYPE_CHOICE) + ->setChoices(['X7R']); + + self::assertSame('X7R', $definition->addChoice(' x7r ')); + self::assertSame(['X7R'], $definition->getChoices()); + self::assertSame('X5R', $definition->addChoice(' X5R ')); + self::assertSame(['X7R', 'X5R'], $definition->getChoices()); + } + + public function testTextDefinitionsDoNotKeepChoices(): void + { + $definition = (new ParameterDefinition()) + ->setInputType(ParameterDefinition::INPUT_TYPE_CHOICE) + ->setChoices(['X7R']) + ->setInputType(ParameterDefinition::INPUT_TYPE_TEXT); + + self::assertSame([], $definition->getChoices()); + $this->expectException(LogicException::class); + $definition->addChoice('X5R'); + } + + public function testUnsupportedInputTypeIsRejected(): void + { + $this->expectException(InvalidArgumentException::class); + (new ParameterDefinition())->setInputType('unsupported'); + } + + public function testNameIsTrimmedAndNormalized(): void + { + $definition = (new ParameterDefinition())->setName(' DiElEcTrIc '); + + self::assertSame('DiElEcTrIc', $definition->getName()); + self::assertSame('dielectric', $definition->getNormalizedName()); + } + + public function testSnapshotAndEffectiveMetadataStayExplicitlySeparated(): void + { + $definition = (new ParameterDefinition()) + ->setName('Dielectric') + ->setSymbol('D') + ->setUnit('legacy-unit') + ->setInputType(ParameterDefinition::INPUT_TYPE_CHOICE) + ->setChoices(['C0G', 'X7R']); + $parameter = (new PartParameter())->setDefinition($definition)->setValueText('x7r'); + + self::assertSame('Dielectric', $parameter->getName()); + self::assertSame('Dielectric', $parameter->getSnapshotName()); + self::assertSame(['C0G', 'X7R'], $parameter->getEffectiveChoices()); + self::assertSame('X7R', $parameter->getValueText()); + + $definition + ->setName('Dielectric type') + ->setSymbol('DT') + ->setUnit('current-unit') + ->setChoices(['C0G', 'X7R', 'X5R']); + + self::assertSame('Dielectric', $parameter->getSnapshotName()); + self::assertSame('D', $parameter->getSnapshotSymbol()); + self::assertSame('legacy-unit', $parameter->getSnapshotUnit()); + self::assertSame('Dielectric type', $parameter->getEffectiveName()); + self::assertSame('DT', $parameter->getEffectiveSymbol()); + self::assertSame('current-unit', $parameter->getEffectiveUnit()); + self::assertSame(ParameterDefinition::INPUT_TYPE_CHOICE, $parameter->getEffectiveInputType()); + self::assertSame(['C0G', 'X7R', 'X5R'], $parameter->getEffectiveChoices()); + } + + public function testLegacyParameterWithoutDefinitionUsesFreeTextMetadata(): void + { + $parameter = (new PartParameter()) + ->setName('Legacy') + ->setSymbol('L') + ->setUnit('V') + ->setValueText('free-form value'); + + self::assertNull($parameter->getDefinition()); + self::assertSame($parameter->getSnapshotName(), $parameter->getEffectiveName()); + self::assertSame($parameter->getSnapshotSymbol(), $parameter->getEffectiveSymbol()); + self::assertSame($parameter->getSnapshotUnit(), $parameter->getEffectiveUnit()); + self::assertSame(ParameterDefinition::INPUT_TYPE_TEXT, $parameter->getEffectiveInputType()); + self::assertSame([], $parameter->getEffectiveChoices()); + self::assertSame('free-form value', $parameter->getValueText()); + } + + public function testDefinitionRelationIsSynchronizedInMemory(): void + { + $first_definition = (new ParameterDefinition())->setName('First definition'); + $second_definition = (new ParameterDefinition())->setName('Second definition'); + $parameter = new PartParameter(); + + $parameter->setDefinition($first_definition); + self::assertTrue($first_definition->getParameterUsages()->contains($parameter)); + + $parameter->setDefinition($second_definition); + self::assertFalse($first_definition->getParameterUsages()->contains($parameter)); + self::assertTrue($second_definition->getParameterUsages()->contains($parameter)); + + $parameter->setDefinition(null); + self::assertFalse($second_definition->getParameterUsages()->contains($parameter)); + self::assertNull($parameter->getDefinition()); + } +} diff --git a/tests/Form/ParameterTypeTest.php b/tests/Form/ParameterTypeTest.php new file mode 100644 index 000000000..2bfe6107a --- /dev/null +++ b/tests/Form/ParameterTypeTest.php @@ -0,0 +1,222 @@ +entity_manager = static::getContainer()->get(EntityManagerInterface::class); + $this->form_factory = static::getContainer()->get(FormFactoryInterface::class); + } + + public function testChoiceDefinitionIsLinkedAndRestrictsSubmittedValue(): void + { + $definition = $this->createDefinition( + 'Form dielectric', + ParameterDefinition::INPUT_TYPE_CHOICE, + ['X7R', 'X5R', 'C0G'], + ); + $parameter = new PartParameter(); + $form = $this->createParameterForm($parameter); + + $form->submit($this->submission($definition, 'X7R')); + + self::assertTrue($form->isSynchronized()); + self::assertTrue($form->isValid(), (string) $form->getErrors(true)); + self::assertSame($definition, $parameter->getDefinition()); + self::assertSame('X7R', $parameter->getValueText()); + self::assertSame('Form dielectric', $parameter->getSnapshotName()); + self::assertInstanceOf(ChoiceType::class, $form->get('value_text')->getConfig()->getType()->getInnerType()); + + $invalid_parameter = new PartParameter(); + $invalid_form = $this->createParameterForm($invalid_parameter); + $invalid_form->submit($this->submission($definition, 'Tantalum')); + + self::assertFalse($invalid_form->isValid()); + } + + public function testTextDefinitionKeepsFreeTextInput(): void + { + $definition = $this->createDefinition('Form manufacturer code', ParameterDefinition::INPUT_TYPE_TEXT); + $parameter = new PartParameter(); + $form = $this->createParameterForm($parameter); + + $form->submit($this->submission($definition, 'free-form code')); + + self::assertTrue($form->isValid(), (string) $form->getErrors(true)); + self::assertSame($definition, $parameter->getDefinition()); + self::assertSame('free-form code', $parameter->getValueText()); + self::assertInstanceOf(TextType::class, $form->get('value_text')->getConfig()->getType()->getInnerType()); + } + + public function testParameterWithoutDefinitionKeepsLegacyTextBehavior(): void + { + $parameter = new PartParameter(); + $form = $this->createParameterForm($parameter); + + $form->submit($this->submission(null, 'legacy free text', 'My custom parameter')); + + self::assertTrue($form->isValid(), (string) $form->getErrors(true)); + self::assertNull($parameter->getDefinition()); + self::assertSame('My custom parameter', $parameter->getName()); + self::assertSame('legacy free text', $parameter->getValueText()); + self::assertInstanceOf(TextType::class, $form->get('value_text')->getConfig()->getType()->getInnerType()); + } + + public function testDefinitionTransitionsRebuildValueFieldWithoutResidualValidation(): void + { + $choice_definition = $this->createDefinition( + 'Form transition choice', + ParameterDefinition::INPUT_TYPE_CHOICE, + ['X7R', 'C0G'], + ); + $text_definition = $this->createDefinition('Form transition text', ParameterDefinition::INPUT_TYPE_TEXT); + $parameter = (new PartParameter()) + ->setDefinition($choice_definition) + ->setValueText('X7R'); + + $text_form = $this->createParameterForm($parameter); + self::assertInstanceOf(ChoiceType::class, $text_form->get('value_text')->getConfig()->getType()->getInnerType()); + $text_form->submit($this->submission($text_definition, 'now free text')); + + self::assertTrue($text_form->isValid(), (string) $text_form->getErrors(true)); + self::assertSame($text_definition, $parameter->getDefinition()); + self::assertSame('now free text', $parameter->getValueText()); + self::assertInstanceOf(TextType::class, $text_form->get('value_text')->getConfig()->getType()->getInnerType()); + + $choice_form = $this->createParameterForm($parameter); + $choice_form->submit($this->submission($choice_definition, 'C0G')); + + self::assertTrue($choice_form->isValid(), (string) $choice_form->getErrors(true)); + self::assertSame($choice_definition, $parameter->getDefinition()); + self::assertSame('C0G', $parameter->getValueText()); + self::assertInstanceOf(ChoiceType::class, $choice_form->get('value_text')->getConfig()->getType()->getInnerType()); + } + + public function testPersistedChoiceParameterReopensWithItsSelectedChoice(): void + { + $definition = $this->createDefinition( + 'Form persisted dielectric', + ParameterDefinition::INPUT_TYPE_CHOICE, + ['X7R', 'X5R', 'C0G'], + ); + $category = (new Category())->setName('Form persisted category'); + $part = (new Part())->setName('Form persisted part')->setCategory($category); + $parameter = new PartParameter(); + $part->addParameter($parameter); + + $form = $this->createParameterForm($parameter); + $form->submit($this->submission($definition, 'X7R')); + self::assertTrue($form->isValid(), (string) $form->getErrors(true)); + + $this->entity_manager->persist($category); + $this->entity_manager->persist($part); + $this->entity_manager->flush(); + $parameter_id = $parameter->getID(); + $this->entity_manager->clear(); + + $reloaded = $this->entity_manager->find(PartParameter::class, $parameter_id); + self::assertInstanceOf(PartParameter::class, $reloaded); + $reopened_form = $this->createParameterForm($reloaded); + + self::assertInstanceOf(ChoiceType::class, $reopened_form->get('value_text')->getConfig()->getType()->getInnerType()); + self::assertSame('X7R', $reopened_form->get('value_text')->getData()); + self::assertSame('X7R', $reopened_form->createView()['value_text']->vars['value']); + } + + public function testLinkedParameterEditorUsesCurrentDefinitionMetadataWithoutChangingSnapshots(): void + { + $definition = $this->createDefinition('Original form name', ParameterDefinition::INPUT_TYPE_TEXT); + $definition->setSymbol('old')->setUnit('old-unit'); + $parameter = (new PartParameter())->setDefinition($definition); + + $definition + ->setName('Current form name') + ->setSymbol('new') + ->setUnit('new-unit'); + + $form = $this->createParameterForm($parameter); + + self::assertSame('Original form name', $parameter->getSnapshotName()); + self::assertSame('old', $parameter->getSnapshotSymbol()); + self::assertSame('old-unit', $parameter->getSnapshotUnit()); + self::assertSame('Current form name', $form->get('name')->getData()); + self::assertSame('new', $form->get('symbol')->getData()); + self::assertSame('new-unit', $form->get('unit')->getData()); + } + + /** @param list|null $choices */ + private function createDefinition(string $name, string $input_type, ?array $choices = null): ParameterDefinition + { + $definition = (new ParameterDefinition()) + ->setName($name) + ->setInputType($input_type) + ->setChoices($choices); + $this->entity_manager->persist($definition); + $this->entity_manager->flush(); + + return $definition; + } + + private function createParameterForm(PartParameter $parameter): FormInterface + { + return $this->form_factory->create(ParameterType::class, $parameter, [ + 'data_class' => PartParameter::class, + 'csrf_protection' => false, + ]); + } + + /** @return array */ + private function submission( + ?ParameterDefinition $definition, + string $value_text, + ?string $name = null, + ): array { + return [ + 'name' => $name ?? $definition?->getName() ?? '', + 'symbol' => $definition?->getSymbol() ?? '', + 'value_min' => '', + 'value_typical' => '', + 'value_max' => '', + 'unit' => $definition?->getUnit() ?? '', + 'value_text' => $value_text, + 'group' => '', + 'definition' => $definition instanceof ParameterDefinition ? (string) $definition->getID() : '', + 'eda_visibility' => '', + 'eda_symbol_visibility' => '', + ]; + } +} diff --git a/tests/Migration/ParameterDefinitionMigrationTest.php b/tests/Migration/ParameterDefinitionMigrationTest.php new file mode 100644 index 000000000..c667e3d5d --- /dev/null +++ b/tests/Migration/ParameterDefinitionMigrationTest.php @@ -0,0 +1,92 @@ + [new MySQLPlatform(), 'JSON DEFAULT NULL', 'INT AUTO_INCREMENT NOT NULL']; + yield 'postgresql' => [new PostgreSQLPlatform(), 'JSON DEFAULT NULL', 'GENERATED BY DEFAULT AS IDENTITY']; + yield 'sqlite' => [new SQLitePlatform(), 'choices CLOB DEFAULT NULL', 'INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL']; + } + + #[DataProvider('platformProvider')] + public function testUpMigrationContainsPortableDefinitionAndRelationSql( + AbstractPlatform $platform, + string $expected_json_type, + string $expected_identity, + ): void { + $sql = $this->migrationSql($platform, true); + + self::assertStringContainsString('CREATE TABLE parameter_definitions', $sql); + self::assertStringContainsString($expected_json_type, $sql); + self::assertStringContainsString($expected_identity, $sql); + self::assertStringContainsString('definition_id', $sql); + self::assertStringContainsString('ON DELETE RESTRICT', $sql); + self::assertStringContainsString('parameter_definition_value_idx', $sql); + if ($platform instanceof SQLitePlatform) { + self::assertStringNotContainsString('eda_symbol_visibility, input_type', $sql); + self::assertStringNotContainsString('input_type VARCHAR(16)', $this->parameterTableSql($sql)); + self::assertStringNotContainsString('choices CLOB', $this->parameterTableSql($sql)); + } + } + + #[DataProvider('platformProvider')] + public function testDownMigrationRemovesRelationBeforeDefinitionTable( + AbstractPlatform $platform, + string $_expected_json_type, + string $_expected_identity, + ): void { + $sql = $this->migrationSql($platform, false); + + self::assertStringContainsString('parameters', $sql); + self::assertStringContainsString('DROP TABLE parameter_definitions', $sql); + self::assertStringEndsWith('DROP TABLE parameter_definitions', trim($sql)); + if ($platform instanceof SQLitePlatform) { + self::assertStringNotContainsString('input_type VARCHAR(16)', $this->parameterTableSql($sql)); + self::assertStringNotContainsString('choices CLOB', $this->parameterTableSql($sql)); + } + } + + private function migrationSql(AbstractPlatform $platform, bool $up): string + { + $connection = $this->createMock(Connection::class); + $connection->method('getDatabasePlatform')->willReturn($platform); + $migration = new Version20260816190000($connection, new NullLogger()); + + if ($up) { + $migration->up(new Schema()); + } else { + $migration->down(new Schema()); + } + + return implode("\n", array_map( + static fn ($query): string => $query->getStatement(), + $migration->getSql(), + )); + } + + private function parameterTableSql(string $sql): string + { + $start = strpos($sql, 'CREATE TABLE parameters'); + self::assertNotFalse($start); + + return substr($sql, $start, (int) strpos($sql, ')', $start) - $start + 1); + } +} diff --git a/tests/Services/ImportExportSystem/EntityExporterTest.php b/tests/Services/ImportExportSystem/EntityExporterTest.php index 2c400102a..3338a04ee 100644 --- a/tests/Services/ImportExportSystem/EntityExporterTest.php +++ b/tests/Services/ImportExportSystem/EntityExporterTest.php @@ -23,6 +23,9 @@ namespace App\Tests\Services\ImportExportSystem; use App\Entity\Parts\Category; +use App\Entity\Parameters\ParameterDefinition; +use App\Entity\Parameters\PartParameter; +use App\Entity\Parts\Part; use App\Services\ImportExportSystem\EntityExporter; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; use Symfony\Component\HttpFoundation\Request; @@ -64,6 +67,33 @@ public function testExportStructuralEntities(): void $json_with_children); } + public function testFullPartExportDoesNotDuplicateDefinitionMetadataIntoParameters(): void + { + $definition = (new ParameterDefinition()) + ->setName('Export dielectric') + ->setInputType(ParameterDefinition::INPUT_TYPE_CHOICE) + ->setChoices(['C0G', 'X7R']); + $parameter = (new PartParameter()) + ->setDefinition($definition) + ->setValueText('X7R'); + $part = (new Part()) + ->setName('Export definition part') + ->setCategory((new Category())->setName('Export definition category')) + ->addParameter($parameter); + + $data = json_decode( + $this->service->exportEntities($part, ['format' => 'json', 'level' => 'full']), + true, + flags: JSON_THROW_ON_ERROR, + ); + $exported_parameter = $data[0]['parameters'][0]; + + self::assertSame('X7R', $exported_parameter['value_text']); + self::assertArrayNotHasKey('definition', $exported_parameter); + self::assertArrayNotHasKey('input_type', $exported_parameter); + self::assertArrayNotHasKey('choices', $exported_parameter); + } + public function testExportEntityFromRequest(): void { $entities = $this->getEntities(); diff --git a/tests/Services/ImportExportSystem/EntityImporterTest.php b/tests/Services/ImportExportSystem/EntityImporterTest.php index 29804dc6d..6c3ec06e8 100644 --- a/tests/Services/ImportExportSystem/EntityImporterTest.php +++ b/tests/Services/ImportExportSystem/EntityImporterTest.php @@ -29,6 +29,8 @@ use App\Entity\LabelSystem\LabelProfile; use App\Entity\Parts\Category; use App\Entity\Parts\Part; +use App\Entity\Parameters\ParameterDefinition; +use App\Entity\Parameters\PartParameter; use App\Entity\ProjectSystem\Project; use App\Entity\UserSystem\User; use App\Services\ImportExportSystem\EntityImporter; @@ -350,6 +352,30 @@ public function testImportStringParts(): void $this->assertSame('test,test2', $results[0]->getTags()); } + public function testImportedParameterWithoutDefinitionRemainsFreeText(): void + { + $category = (new Category())->setName('Imported parameter category'); + $errors = []; + $results = $this->service->importString( + '[{"name":"Imported parameter part","parameters":[{"_type":"Part","name":"Dielectric","value_text":"X7R"}]}]', + [ + 'class' => Part::class, + 'format' => 'json', + 'part_category' => $category, + ], + $errors, + ); + + self::assertEmpty($errors); + self::assertCount(1, $results); + $parameter = $results[0]->getParameters()->first(); + self::assertInstanceOf(PartParameter::class, $parameter); + self::assertNull($parameter->getDefinition()); + self::assertSame(ParameterDefinition::INPUT_TYPE_TEXT, $parameter->getEffectiveInputType()); + self::assertSame([], $parameter->getEffectiveChoices()); + self::assertSame('X7R', $parameter->getValueText()); + } + public function testImportAcceptsNestedEdaInfoColumns(): void { //The API / JSON export writes EDA columns as "eda_info.kicad_symbol"; re-importing such a diff --git a/tests/Services/LogSystem/TimeTravelTest.php b/tests/Services/LogSystem/TimeTravelTest.php index 9b51592d1..4d6b5c52a 100644 --- a/tests/Services/LogSystem/TimeTravelTest.php +++ b/tests/Services/LogSystem/TimeTravelTest.php @@ -23,6 +23,8 @@ namespace App\Tests\Services\LogSystem; use App\Entity\LogSystem\ElementEditedLogEntry; +use App\Entity\Parameters\ParameterDefinition; +use App\Entity\Parameters\PartParameter; use App\Entity\Parts\Category; use App\Services\LogSystem\TimeTravel; use Doctrine\ORM\EntityManagerInterface; @@ -79,4 +81,71 @@ public function testRevertEntityToTimestamp(): void //The category with 1 should have the name 'Test' at this timestamp $this->assertEquals('Test', $category->getName()); } + + public function testApplyingHistoricalParameterDataRestoresVisibleSnapshotsButKeepsCurrentEffectiveDefinition(): void + { + $definition = (new ParameterDefinition()) + ->setName('Dielectric type') + ->setInputType(ParameterDefinition::INPUT_TYPE_CHOICE) + ->setChoices(['C0G', 'X7R', 'X5R']); + $parameter = (new PartParameter())->setDefinition($definition); + (new \ReflectionClass($parameter))->getProperty('id')->setValue($parameter, 1001); + + $log_entry = new ElementEditedLogEntry($parameter); + $log_entry->setOldData([ + 'name' => 'Dielectric', + ]); + + $this->service->applyEntry($parameter, $log_entry); + + self::assertSame('Dielectric', $parameter->getSnapshotName()); + self::assertSame('Dielectric type', $parameter->getEffectiveName()); + self::assertSame(ParameterDefinition::INPUT_TYPE_CHOICE, $parameter->getEffectiveInputType()); + self::assertSame(['C0G', 'X7R', 'X5R'], $parameter->getEffectiveChoices()); + } + + public function testApplyingHistoricalParameterDataRestoresNullDefinitionReference(): void + { + $definition = (new ParameterDefinition())->setName('Current definition'); + $parameter = (new PartParameter())->setDefinition($definition); + (new \ReflectionClass($parameter))->getProperty('id')->setValue($parameter, 1002); + + $log_entry = new ElementEditedLogEntry($parameter); + $log_entry->setOldData([ + 'definition' => null, + 'name' => 'Historical ad hoc parameter', + ]); + + $this->service->applyEntry($parameter, $log_entry); + + self::assertNull($parameter->getDefinition()); + self::assertFalse($definition->getParameterUsages()->contains($parameter)); + self::assertSame('Historical ad hoc parameter', $parameter->getSnapshotName()); + self::assertSame('Historical ad hoc parameter', $parameter->getEffectiveName()); + } + + public function testApplyingHistoricalParameterDataRestoresPreviousDefinitionReference(): void + { + $previous_definition = (new ParameterDefinition())->setName('Previous definition'); + $current_definition = (new ParameterDefinition())->setName('Current replacement definition'); + $this->em->persist($previous_definition); + $this->em->persist($current_definition); + $this->em->flush(); + + $parameter = (new PartParameter())->setDefinition($current_definition); + (new \ReflectionClass($parameter))->getProperty('id')->setValue($parameter, 1003); + $log_entry = new ElementEditedLogEntry($parameter); + $log_entry->setOldData([ + 'definition' => ['@id' => $previous_definition->getID()], + 'name' => 'Historical snapshot name', + ]); + + $this->service->applyEntry($parameter, $log_entry); + + self::assertSame($previous_definition, $parameter->getDefinition()); + self::assertTrue($previous_definition->getParameterUsages()->contains($parameter)); + self::assertFalse($current_definition->getParameterUsages()->contains($parameter)); + self::assertSame('Historical snapshot name', $parameter->getSnapshotName()); + self::assertSame('Previous definition', $parameter->getEffectiveName()); + } } diff --git a/tests/Services/UserSystem/PermissionPresetsHelperTest.php b/tests/Services/UserSystem/PermissionPresetsHelperTest.php index c141997a9..f2c3eae81 100644 --- a/tests/Services/UserSystem/PermissionPresetsHelperTest.php +++ b/tests/Services/UserSystem/PermissionPresetsHelperTest.php @@ -78,6 +78,7 @@ public function testReadOnlyPresetAllowsPartsRead(): void self::$service->applyPreset($user, PermissionPresetsHelper::PRESET_READ_ONLY); $this->assertTrue(self::$permissionManager->dontInherit($user, 'parts', 'read')); + $this->assertTrue(self::$permissionManager->dontInherit($user, 'parameter_definitions', 'read')); } public function testReadOnlyPresetDoesNotAllowPartsCreate(): void @@ -96,6 +97,27 @@ public function testUnknownPresetThrowsException(): void self::$service->applyPreset($this->createUser(), 'non_existent_preset'); } + public function testEditorCanReadButCannotManageParameterDefinitions(): void + { + $user = $this->createUser(); + self::$service->applyPreset($user, PermissionPresetsHelper::PRESET_EDITOR); + + $this->assertTrue(self::$permissionManager->dontInherit($user, 'parameter_definitions', 'read')); + $this->assertNotTrue(self::$permissionManager->dontInherit($user, 'parameter_definitions', 'edit')); + $this->assertNotTrue(self::$permissionManager->dontInherit($user, 'parameter_definitions', 'create')); + } + + public function testAdminCanManageParameterDefinitions(): void + { + $user = $this->createUser(); + self::$service->applyPreset($user, PermissionPresetsHelper::PRESET_ADMIN); + + $this->assertTrue(self::$permissionManager->dontInherit($user, 'parameter_definitions', 'read')); + $this->assertTrue(self::$permissionManager->dontInherit($user, 'parameter_definitions', 'edit')); + $this->assertTrue(self::$permissionManager->dontInherit($user, 'parameter_definitions', 'create')); + $this->assertTrue(self::$permissionManager->dontInherit($user, 'parameter_definitions', 'delete')); + } + public function testApplyPresetReturnsTheSameUser(): void { $user = $this->createUser(); diff --git a/tests/Services/UserSystem/PermissionSchemaUpdaterTest.php b/tests/Services/UserSystem/PermissionSchemaUpdaterTest.php index 738ff6496..1bf0338e2 100644 --- a/tests/Services/UserSystem/PermissionSchemaUpdaterTest.php +++ b/tests/Services/UserSystem/PermissionSchemaUpdaterTest.php @@ -123,4 +123,20 @@ public function testUpgradeSchemaToVersion3(): void self::assertTrue($this->service->upgradeSchema($user, 3)); self::assertSame(PermissionData::ALLOW, $user->getPermissions()->getPermissionValue('system', 'show_updates')); } + + public function testUpgradeSchemaToVersion5KeepsDefinitionManagementAdministrative(): void + { + $perm_data = new PermissionData(); + $perm_data->setSchemaVersion(4); + $perm_data->setPermissionValue('parts', 'read', PermissionData::ALLOW); + $perm_data->setPermissionValue('parts', 'edit', PermissionData::ALLOW); + $perm_data->setPermissionValue('config', 'change_system_settings', PermissionData::DISALLOW); + $user = new TestPermissionHolder($perm_data); + + self::assertTrue($this->service->upgradeSchema($user, 5)); + self::assertSame(PermissionData::ALLOW, $perm_data->getPermissionValue('parameter_definitions', 'read')); + self::assertSame(PermissionData::DISALLOW, $perm_data->getPermissionValue('parameter_definitions', 'edit')); + self::assertSame(PermissionData::DISALLOW, $perm_data->getPermissionValue('parameter_definitions', 'create')); + self::assertSame(PermissionData::DISALLOW, $perm_data->getPermissionValue('parameter_definitions', 'delete')); + } } diff --git a/translations/messages.en.xlf b/translations/messages.en.xlf index fa3998b54..dc00baa69 100644 --- a/translations/messages.en.xlf +++ b/translations/messages.en.xlf @@ -3638,6 +3638,12 @@ If you have done this incorrectly or if a computer is no longer trusted, you can Parameter + + + parameter_definition.label + Parameter definition + + label_profile.label @@ -14261,5 +14267,41 @@ Buerklin-API Authentication server: OAuth2 client updated successfully! + + + parameter_definition.labelp + Parameters + + + + parameter_definition.editEdit parameter definition + + + parameter_definition.newNew parameter definition + + + parameter_definition.symbolSymbol + + + parameter_definition.unitUnit + + + parameter_definition.input_typeInput type + + + parameter_definition.input_type.textText + + + parameter_definition.input_type.choiceChoice + + + parameter_definition.choicesChoices + + + parameter_definition.choices.helpEnter one choice per line. This field is ignored and cleared when the input type is Text. + + + parameter_definition.delete.in_useThis parameter definition is still in use and cannot be deleted. + diff --git a/translations/messages.fr.xlf b/translations/messages.fr.xlf index 49b7ca032..3747d8088 100644 --- a/translations/messages.fr.xlf +++ b/translations/messages.fr.xlf @@ -3614,6 +3614,12 @@ Si vous avez fait cela de manière incorrecte ou si un ordinateur n'est plus fia Caractéristique + + + parameter_definition.label + Définition de paramètre + + label_profile.label @@ -12954,5 +12960,41 @@ Serveur d'authentification Buerklin-API : 10 requêtes/min par adresse IPErreur de mapping : vérifier si vous avez sélectionné les bons délimiteurs + + + parameter_definition.labelp + Paramètres + + + + parameter_definition.editModifier la définition de paramètre + + + parameter_definition.newNouvelle définition de paramètre + + + parameter_definition.symbolSymbole + + + parameter_definition.unitUnité + + + parameter_definition.input_typeType de saisie + + + parameter_definition.input_type.textTexte + + + parameter_definition.input_type.choiceChoix + + + parameter_definition.choicesChoix + + + parameter_definition.choices.helpSaisissez un choix par ligne. Ce champ est ignoré et vidé lorsque le type de saisie est Texte. + + + parameter_definition.delete.in_useCette définition de paramètre est encore utilisée et ne peut pas être supprimée. + diff --git a/translations/validators.en.xlf b/translations/validators.en.xlf index 76a6a7d8f..47a7f8aa7 100644 --- a/translations/validators.en.xlf +++ b/translations/validators.en.xlf @@ -271,5 +271,8 @@ If you specify an info provider, you also need to provide a provider ID, or remove both. + + parameter_definition.name_uniqueA parameter definition with this name already exists. + diff --git a/translations/validators.fr.xlf b/translations/validators.fr.xlf index 2a9364193..a48456fea 100644 --- a/translations/validators.fr.xlf +++ b/translations/validators.fr.xlf @@ -253,5 +253,8 @@ Cela n'est pas un GTIN / EAN valide ! + + parameter_definition.name_uniqueUne définition de paramètre portant ce nom existe déjà. + From 2fc432ac15e1dddfc3ff94b5821f398571ae0dbc Mon Sep 17 00:00:00 2001 From: Matt Date: Mon, 17 Aug 2026 21:35:22 +0200 Subject: [PATCH 2/5] Improve global choice parameter editing --- .../elements/collection_type_controller.js | 34 ++ .../parameters_autocomplete_controller.js | 217 +++++++- assets/js/tab_remember.js | 54 +- .../form_reset_handler/form_reset_handler.js | 10 +- src/Controller/PartController.php | 6 + src/Entity/Parameters/AbstractParameter.php | 38 +- src/Entity/Parameters/ParameterDefinition.php | 9 +- src/Entity/Parameters/ParametersTrait.php | 4 +- src/Entity/Parameters/PartParameter.php | 2 +- src/Form/ParameterType.php | 102 +++- src/Repository/ParameterRepository.php | 25 + src/Services/LogSystem/TimeTravel.php | 6 +- .../PendingParameterChoiceApplier.php | 66 +++ .../parts/edit/edit_form_styles.html.twig | 6 +- .../ParameterDefinitionsEndpointTest.php | 20 + .../Parameters/ParameterDefinitionTest.php | 10 + tests/Entity/Parts/PartTest.php | 19 + tests/Form/ParameterTypeTest.php | 519 ++++++++++++++++++ tests/Services/LogSystem/TimeTravelTest.php | 34 ++ translations/frontend.en.xlf | 9 + translations/frontend.fr.xlf | 9 + translations/validators.en.xlf | 21 + translations/validators.fr.xlf | 21 + 23 files changed, 1190 insertions(+), 51 deletions(-) create mode 100644 src/Services/Parameters/PendingParameterChoiceApplier.php diff --git a/assets/controllers/elements/collection_type_controller.js b/assets/controllers/elements/collection_type_controller.js index caeb41229..e6decbdf8 100644 --- a/assets/controllers/elements/collection_type_controller.js +++ b/assets/controllers/elements/collection_type_controller.js @@ -32,6 +32,40 @@ export default class extends Controller { static targets = ["target"]; + connect() { + // Native form reset only restores controls that still exist in the DOM. Keep the initial collection structure + // so persisted rows removed by the user can be recreated and rows added from the prototype can be discarded. + this._initialTarget = this.targetTarget.cloneNode(true); + this._form = this.element.closest('form'); + if (this._form) { + this._resetHandler = this.onFormReset.bind(this); + this._form.addEventListener('reset', this._resetHandler); + } + } + + disconnect() { + clearTimeout(this._resetTimer); + if (this._form && this._resetHandler) { + this._form.removeEventListener('reset', this._resetHandler); + } + } + + onFormReset() { + clearTimeout(this._resetTimer); + // Wait until the browser has completed its native value reset before rebuilding the collection structure. + this._resetTimer = setTimeout(() => this.restoreInitialStructure(), 0); + } + + restoreInitialStructure() { + if (!this._initialTarget || !this.element.isConnected) { + return; + } + + const restoredTarget = this._initialTarget.cloneNode(true); + this.targetTarget.replaceChildren(...restoredTarget.childNodes); + this.targetTarget.dispatchEvent(new CustomEvent("collection:reset", {bubbles: true})); + } + /** * Decodes escaped HTML entities * @param {string} input diff --git a/assets/controllers/pages/parameters_autocomplete_controller.js b/assets/controllers/pages/parameters_autocomplete_controller.js index 2b193cd85..c485e34a2 100644 --- a/assets/controllers/pages/parameters_autocomplete_controller.js +++ b/assets/controllers/pages/parameters_autocomplete_controller.js @@ -20,6 +20,7 @@ import {Controller} from "@hotwired/stimulus"; import TomSelect from "tom-select"; import katex from "katex"; +import {trans} from "../../translator"; import "katex/dist/katex.css"; @@ -38,10 +39,16 @@ export default class extends Controller url: String, } - static targets = ["name", "symbol", "unit", "valueText", "definition"] + static targets = ["name", "symbol", "unit", "valueText", "definition", "newChoiceValue"] _tomSelect; + _valueTomSelect; _initialized = false; + _resetting = false; + _initialState; + _form; + _resetHandler; + _resetTimer; onItemAdd(value, item) { //Retrieve the unit and symbol from the item @@ -61,7 +68,7 @@ export default class extends Controller // TomSelect emits onItemAdd for the value already present while initializing an existing row. The server has // rendered that row from its persisted definition, so only an explicit user selection may change the link. - if (!this._initialized || !this.hasDefinitionTarget || !this.hasValueTextTarget) { + if (this._resetting || !this._initialized || !this.hasDefinitionTarget || !this.hasValueTextTarget) { return; } @@ -69,6 +76,7 @@ export default class extends Controller if (definitionId === undefined || !/^\d+$/.test(definitionId) || Number(definitionId) < 1) { this.setDefinition(null); this.applyInputDefinition('text', []); + this.setLinkedFieldState(false); return; } @@ -84,15 +92,17 @@ export default class extends Controller this.setDefinition(definitionId, item.dataset.definitionName ?? value); this.applyInputDefinition(item.dataset.inputType ?? 'text', choices); + this.setLinkedFieldState(true); } onItemRemove() { - if (!this._initialized || !this.hasDefinitionTarget || !this.hasValueTextTarget) { + if (this._resetting || !this._initialized || !this.hasDefinitionTarget || !this.hasValueTextTarget) { return; } this.setDefinition(null); this.applyInputDefinition('text', []); + this.setLinkedFieldState(false); } setDefinition(definitionId, name = '') { @@ -112,9 +122,10 @@ export default class extends Controller this.definitionTarget.dispatchEvent(new Event('change', {bubbles: true})); } - applyInputDefinition(inputType, choices) { + applyInputDefinition(inputType, choices, restoredValue = undefined) { + this.destroyValueTomSelect(); const oldElement = this.valueTextTarget; - const currentValue = oldElement.value; + const currentValue = restoredValue ?? oldElement.value; const useChoice = inputType === 'choice' && Array.isArray(choices); const newElement = document.createElement(useChoice ? 'select' : 'input'); @@ -142,10 +153,196 @@ export default class extends Controller } oldElement.replaceWith(newElement); + this.clearPendingChoice(); + if (useChoice) { + this.setupValueTomSelect(newElement); + } newElement.dispatchEvent(new Event('change', {bubbles: true})); } + setLinkedFieldState(linked) { + if (this.hasSymbolTarget) { + this.symbolTarget.readOnly = linked; + } + if (this.hasUnitTarget) { + this.unitTarget.readOnly = linked; + } + } + + normalizeChoice(value) { + return value.trim().toLocaleLowerCase(); + } + + findCanonicalChoice(value) { + const normalized = this.normalizeChoice(value); + if (normalized === '' || !this._valueTomSelect) { + return null; + } + + for (const option of Object.values(this._valueTomSelect.options)) { + if (!option.pending_choice && this.normalizeChoice(String(option.value ?? '')) === normalized) { + return String(option.value); + } + } + + return null; + } + + clearPendingChoice() { + if (this.hasNewChoiceValueTarget) { + this.newChoiceValueTarget.value = ''; + } + } + + setupValueTomSelect(element) { + const canAddChoice = element.dataset.canAddChoice === 'true'; + const pendingChoice = this.hasNewChoiceValueTarget ? this.newChoiceValueTarget.value.trim() : ''; + const options = Array.from(element.options).map(option => ({ + value: option.value, + text: option.text, + pending_choice: pendingChoice !== '' && option.value === pendingChoice, + })); + + this._valueTomSelect = new TomSelect(element, { + plugins: { + 'clear_button': {}, + 'form_reset_handler': {}, + }, + options, + items: element.value === '' ? [] : [element.value], + valueField: 'value', + labelField: 'text', + searchField: 'text', + maxItems: 1, + allowEmptyOption: true, + placeholder: trans('parameter.choice.nothing_selected'), + createOnBlur: false, + selectOnTab: true, + createFilter: input => canAddChoice + && this.normalizeChoice(input) !== '' + && this.findCanonicalChoice(input) === null, + create: canAddChoice ? (input, callback) => { + const choice = input.trim(); + if (choice === '' || this.findCanonicalChoice(choice) !== null) { + callback(false); + return; + } + + callback({value: choice, text: choice, pending_choice: true}); + } : false, + onType: input => { + const canonical = this.findCanonicalChoice(input); + if (canonical !== null && canonical !== input) { + this._valueTomSelect.setTextboxValue(canonical); + this._valueTomSelect.refreshOptions(false); + } + }, + onItemAdd: value => { + // Initial items are added while the TomSelect constructor is still running, before the instance has + // been assigned to _valueTomSelect. + const option = this._valueTomSelect?.options[value] + ?? options.find(candidate => candidate.value === value); + if (this.hasNewChoiceValueTarget) { + this.newChoiceValueTarget.value = option?.pending_choice ? String(option.value) : ''; + } + }, + onItemRemove: () => this.clearPendingChoice(), + render: { + option_create: (data, escape) => '
    ' + + escape(trans('parameter.choice.add_new', {'%value%': data.input})) + + ' ' + escape(trans('parameter.choice.new')) + '
    ', + item: (data, escape) => '
    ' + escape(data.text) + + (data.pending_choice + ? ' ' + escape(trans('parameter.choice.new')) + '' + : '') + + '
    ', + }, + }); + } + + destroyValueTomSelect() { + this._valueTomSelect?.destroy(); + this._valueTomSelect = undefined; + } + + captureInitialState() { + const valueElement = this.valueTextTarget; + const isChoice = valueElement.tagName === 'SELECT'; + const definitionId = this.hasDefinitionTarget ? this.definitionTarget.value : ''; + const selectedDefinition = this.hasDefinitionTarget + ? this.definitionTarget.options[this.definitionTarget.selectedIndex] + : null; + + this._initialState = { + name: this.nameTarget.value, + symbol: this.hasSymbolTarget ? this.symbolTarget.value : '', + unit: this.hasUnitTarget ? this.unitTarget.value : '', + definitionId, + definitionName: selectedDefinition?.text ?? this.nameTarget.value, + inputType: isChoice ? 'choice' : 'text', + choices: isChoice + ? Array.from(valueElement.options).filter(option => option.value !== '').map(option => option.value) + : [], + value: valueElement.value, + }; + } + + onFormReset() { + this._resetting = true; + clearTimeout(this._resetTimer); + + // The browser restores native form controls after the reset event. Rebuild the composite TomSelect state on + // the next task, once both the native reset and the individual TomSelect reset handlers have completed. + this._resetTimer = setTimeout(() => this.restoreInitialState(), 0); + } + + restoreInitialState() { + if (!this._initialState || !this.element.isConnected) { + this._resetting = false; + return; + } + + const state = this._initialState; + this.clearPendingChoice(); + + if (state.name === '') { + this._tomSelect.clear(true); + } else { + if (!this._tomSelect.options[state.name]) { + this._tomSelect.addOption({name: state.name}); + } + this._tomSelect.setValue(state.name, true); + } + + if (this.hasSymbolTarget) { + this.symbolTarget.value = state.symbol; + this.symbolTarget.dispatchEvent(new Event('input')); + } + if (this.hasUnitTarget) { + this.unitTarget.value = state.unit; + this.unitTarget.dispatchEvent(new Event('input')); + } + + if (this.hasDefinitionTarget) { + this.setDefinition( + state.definitionId === '' ? null : state.definitionId, + state.definitionName + ); + } + this.applyInputDefinition(state.inputType, state.choices, state.value); + this.setLinkedFieldState(state.definitionId !== ''); + this._resetting = false; + } + connect() { + this.captureInitialState(); + this._form = this.nameTarget.form; + if (this._form) { + this._resetHandler = this.onFormReset.bind(this); + // Capture phase ensures callbacks emitted by the TomSelect reset plugins see _resetting=true. + this._form.addEventListener('reset', this._resetHandler, true); + } + const settings = { plugins: { 'autoselect_typed': {}, @@ -210,7 +407,6 @@ export default class extends Controller if (data.choices !== undefined) { element.dataset.choices = JSON.stringify(data.choices); } - return element.outerHTML; } } @@ -233,11 +429,20 @@ export default class extends Controller } this._tomSelect = new TomSelect(this.nameTarget, settings); + this.setLinkedFieldState(this.hasDefinitionTarget && this.definitionTarget.value !== ''); + if (this.hasValueTextTarget && this.valueTextTarget.tagName === 'SELECT') { + this.setupValueTomSelect(this.valueTextTarget); + } } disconnect() { super.disconnect(); + clearTimeout(this._resetTimer); + if (this._form && this._resetHandler) { + this._form.removeEventListener('reset', this._resetHandler, true); + } //Destroy the TomSelect instance this._tomSelect?.destroy(); + this.destroyValueTomSelect(); } } diff --git a/assets/js/tab_remember.js b/assets/js/tab_remember.js index 1bf35db5c..1b175f6e2 100644 --- a/assets/js/tab_remember.js +++ b/assets/js/tab_remember.js @@ -45,17 +45,23 @@ class TabRememberHelper { return; } - //Find the first offending element and show it - //Symfony validation errors can occur on multiple types + this.revealFirstValidationError(); + } + + revealFirstValidationError() { + // Symfony validation errors can occur on inputs or as standalone error blocks. const inputErrors = document.getElementsByClassName('is-invalid'); const blockErrors = document.getElementsByClassName('form-error-message'); - const merged = [...inputErrors, ...blockErrors]; + const firstElement = [...inputErrors, ...blockErrors][0] ?? null; - const first_element = merged[0] ?? null; - if(first_element) { - this.revealElementOnTab(first_element); - this.revealElementInCollapse(first_element); + if (!firstElement) { + return false; } + + this.revealElementOnTab(firstElement); + this.revealElementInCollapse(firstElement); + + return true; } /** @@ -102,21 +108,23 @@ class TabRememberHelper { } onLoad(event) { - //Determine which tab should be shown (use hash if specified, otherwise use localstorage) - let activeTab = null; - if (location.hash) { - activeTab = document.querySelector('[href=\'' + location.hash + '\']'); - } else if (localStorage.getItem('activeTab')) { - activeTab = document.querySelector('[href="' + localStorage.getItem('activeTab') + '"]'); - } - - if (activeTab) { - - //Reveal our tab selector (needed for nested tabs) - this.revealElementOnTab(activeTab); - - //Finally show the active tab itself - Tab.getOrCreateInstance(activeTab).show(); + // Validation errors take precedence over the remembered tab after a full-page invalid form response. + if (!this.revealFirstValidationError()) { + //Determine which tab should be shown (use hash if specified, otherwise use localstorage) + let activeTab = null; + if (location.hash) { + activeTab = document.querySelector('[href=\'' + location.hash + '\']'); + } else if (localStorage.getItem('activeTab')) { + activeTab = document.querySelector('[href="' + localStorage.getItem('activeTab') + '"]'); + } + + if (activeTab) { + //Reveal our tab selector (needed for nested tabs) + this.revealElementOnTab(activeTab); + + //Finally show the active tab itself + Tab.getOrCreateInstance(activeTab).show(); + } } //Register listener for tab change @@ -137,4 +145,4 @@ class TabRememberHelper { } -export default new TabRememberHelper(); \ No newline at end of file +export default new TabRememberHelper(); diff --git a/assets/tomselect/form_reset_handler/form_reset_handler.js b/assets/tomselect/form_reset_handler/form_reset_handler.js index c944d352e..ad5d56879 100644 --- a/assets/tomselect/form_reset_handler/form_reset_handler.js +++ b/assets/tomselect/form_reset_handler/form_reset_handler.js @@ -37,10 +37,14 @@ export default function form_reset_handler() { // leaving data-default-value unset and breaking the dirty check for blank defaults. input.dataset.defaultValue = input.value; - if (input.form) { - input.form.addEventListener('reset', () => { + const form = input.form; + if (form) { + const resetHandler = () => { input.value = input.dataset.defaultValue ?? ''; self.sync(); - }); + }; + + form.addEventListener('reset', resetHandler); + self.on('destroy', () => form.removeEventListener('reset', resetHandler)); } } diff --git a/src/Controller/PartController.php b/src/Controller/PartController.php index c4c0e5260..84c480b9e 100644 --- a/src/Controller/PartController.php +++ b/src/Controller/PartController.php @@ -46,6 +46,7 @@ use App\Services\LogSystem\HistoryHelper; use App\Services\LogSystem\TimeTravel; use App\Services\Parameters\ParameterExtractor; +use App\Services\Parameters\PendingParameterChoiceApplier; use App\Services\Parts\PartLotWithdrawAddHelper; use App\Services\Parts\PricedetailHelper; use App\Services\ProjectSystem\ProjectBuildPartHelper; @@ -81,6 +82,7 @@ public function __construct( private readonly EventCommentHelper $commentHelper, private readonly PartInfoSettings $partInfoSettings, private readonly IpnSuggestSettings $ipnSuggestSettings, + private readonly PendingParameterChoiceApplier $pendingParameterChoiceApplier, ) { } @@ -468,6 +470,10 @@ private function renderPartForm(string $mode, Request $request, Part $data, arra $this->commentHelper->setMessage($form['log_comment']->getData()); + // Apply definition changes only after the complete Part form is valid. The following flush persists the + // Part and its definition changes atomically. + $this->pendingParameterChoiceApplier->apply($new_part); + $this->em->persist($new_part); //When we are in merge mode, we have to remove the other part diff --git a/src/Entity/Parameters/AbstractParameter.php b/src/Entity/Parameters/AbstractParameter.php index 5c52ed0cd..c4e9223b0 100644 --- a/src/Entity/Parameters/AbstractParameter.php +++ b/src/Entity/Parameters/AbstractParameter.php @@ -176,6 +176,12 @@ abstract class AbstractParameter extends AbstractNamedDBElement implements Uniqu #[ORM\JoinColumn(name: 'definition_id', nullable: true, onDelete: 'RESTRICT')] protected ?ParameterDefinition $definition = null; + /** + * A choice explicitly requested from the Part editor. This is deliberately not persisted: the definition is + * updated only after the complete Part form has passed validation. + */ + private ?string $pending_definition_choice = null; + /** * @var string the group this parameter belongs to */ @@ -573,8 +579,10 @@ public function getValueText(): string * * @return $this */ - public function setValueText(string $value_text): self + public function setValueText(?string $value_text): self { + $value_text ??= ''; + if ($this->definition instanceof ParameterDefinition && ParameterDefinition::INPUT_TYPE_CHOICE === $this->definition->getInputType() && '' !== $value_text) { @@ -589,6 +597,26 @@ public function setValueText(string $value_text): self return $this; } + public function requestPendingDefinitionChoice(?string $choice): self + { + $choice = null === $choice ? '' : trim($choice); + $this->pending_definition_choice = '' === $choice ? null : $choice; + + return $this; + } + + public function getPendingDefinitionChoice(): ?string + { + return $this->pending_definition_choice; + } + + public function clearPendingDefinitionChoice(): self + { + $this->pending_definition_choice = null; + + return $this; + } + #[Groups(['parameter:read'])] #[SerializedName('input_type')] public function getEffectiveInputType(): string @@ -693,7 +721,11 @@ public function validateDefinitionUsage(ExecutionContextInterface $context): voi $canonical_choice = $this->definition->findCanonicalChoice($this->value_text); if (null === $canonical_choice) { - $context->buildViolation('The selected value is not part of the linked parameter definition.') + if ($this->pending_definition_choice === $this->value_text) { + return; + } + + $context->buildViolation('parameter.validator.value_not_allowed') ->atPath('value_text') ->addViolation(); @@ -701,7 +733,7 @@ public function validateDefinitionUsage(ExecutionContextInterface $context): voi } if ($canonical_choice !== $this->value_text) { - $context->buildViolation('The selected value does not use the canonical spelling from the linked parameter definition.') + $context->buildViolation('parameter.validator.value_not_canonical') ->atPath('value_text') ->addViolation(); } diff --git a/src/Entity/Parameters/ParameterDefinition.php b/src/Entity/Parameters/ParameterDefinition.php index d4e521e6e..3e049736b 100644 --- a/src/Entity/Parameters/ParameterDefinition.php +++ b/src/Entity/Parameters/ParameterDefinition.php @@ -267,14 +267,15 @@ public function removeParameterUsage(AbstractParameter $parameter): self public function validateChoices(ExecutionContextInterface $context): void { if (self::INPUT_TYPE_TEXT === $this->input_type && [] !== $this->getChoices()) { - $context->buildViolation('A text parameter definition must not contain choices.') + $context->buildViolation('parameter_definition.validator.text_has_choices') ->atPath('choices') ->addViolation(); } foreach ($this->getChoices() as $choice) { if (mb_strlen($choice) > self::MAX_CHOICE_LENGTH) { - $context->buildViolation(sprintf('A parameter choice must not exceed %d characters.', self::MAX_CHOICE_LENGTH)) + $context->buildViolation('parameter_definition.validator.choice_too_long') + ->setParameter('{{ limit }}', (string) self::MAX_CHOICE_LENGTH) ->atPath('choices') ->addViolation(); } @@ -299,10 +300,6 @@ private static function canonicalizeChoices(array $choices): array if ('' === $choice) { continue; } - if (mb_strlen($choice) > self::MAX_CHOICE_LENGTH) { - throw new InvalidArgumentException(sprintf('A parameter choice must not exceed %d characters.', self::MAX_CHOICE_LENGTH)); - } - $normalized_choice = self::normalize($choice); if (isset($seen_choices[$normalized_choice])) { continue; diff --git a/src/Entity/Parameters/ParametersTrait.php b/src/Entity/Parameters/ParametersTrait.php index 2ccaa7639..49538b7b9 100644 --- a/src/Entity/Parameters/ParametersTrait.php +++ b/src/Entity/Parameters/ParametersTrait.php @@ -87,7 +87,9 @@ public function addParameter(AbstractParameter $parameter): self */ public function removeParameter(AbstractParameter $parameter): self { - $this->parameters->removeElement($parameter); + if ($this->parameters->removeElement($parameter)) { + $parameter->setDefinition(null); + } return $this; } diff --git a/src/Entity/Parameters/PartParameter.php b/src/Entity/Parameters/PartParameter.php index 91b51c007..76ac61edb 100644 --- a/src/Entity/Parameters/PartParameter.php +++ b/src/Entity/Parameters/PartParameter.php @@ -52,7 +52,7 @@ /** * @see \App\Tests\Entity\Parameters\PartParameterTest */ -#[UniqueEntity(fields: ['name', 'group', 'element'])] +#[UniqueEntity(fields: ['name', 'group', 'element'], repositoryMethod: 'findActiveForUniqueValidation')] #[ORM\Entity(repositoryClass: ParameterRepository::class)] class PartParameter extends AbstractParameter { diff --git a/src/Form/ParameterType.php b/src/Form/ParameterType.php index 22e1033ee..90578540f 100644 --- a/src/Form/ParameterType.php +++ b/src/Form/ParameterType.php @@ -58,13 +58,16 @@ use App\Form\Type\TriStateCheckboxType; use Doctrine\ORM\EntityManagerInterface; use Symfony\Bridge\Doctrine\Form\Type\EntityType; +use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\Form\AbstractType; use Symfony\Component\Form\Event\PreSetDataEvent; use Symfony\Component\Form\Extension\Core\Type\CheckboxType; use Symfony\Component\Form\Extension\Core\Type\ChoiceType; +use Symfony\Component\Form\Extension\Core\Type\HiddenType; use Symfony\Component\Form\Extension\Core\Type\NumberType; use Symfony\Component\Form\Extension\Core\Type\TextType; use Symfony\Component\Form\FormEvent; +use Symfony\Component\Form\FormError; use Symfony\Component\Form\FormBuilderInterface; use Symfony\Component\Form\FormEvents; use Symfony\Component\Form\FormInterface; @@ -73,8 +76,10 @@ class ParameterType extends AbstractType { - public function __construct(private readonly EntityManagerInterface $entity_manager) - { + public function __construct( + private readonly EntityManagerInterface $entity_manager, + private readonly Security $security, + ) { } public function buildForm(FormBuilderInterface $builder, array $options): void @@ -108,6 +113,7 @@ public function buildForm(FormBuilderInterface $builder, array $options): void ]; if ($linked_part_parameter) { $symbol_options['data'] = $parameter->getEffectiveSymbol(); + $symbol_options['attr']['readonly'] = true; } $builder->add('symbol', TextType::class, $symbol_options); @@ -170,6 +176,7 @@ function (PreSetDataEvent $event): void { ]; if ($linked_part_parameter) { $unit_options['data'] = $parameter->getEffectiveUnit(); + $unit_options['attr']['readonly'] = true; } $builder->add('unit', TextType::class, $unit_options); @@ -196,9 +203,16 @@ function (PreSetDataEvent $event): void { ], ]); + $builder->add('new_choice_value', HiddenType::class, [ + 'mapped' => false, + 'required' => false, + 'empty_data' => '', + ]); + $builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event): void { $submitted_data = $event->getData(); $definition = null; + $pending_choice = ''; if (is_array($submitted_data)) { $definition_id = filter_var( @@ -209,18 +223,87 @@ function (PreSetDataEvent $event): void { if (false !== $definition_id) { $definition = $this->entity_manager->find(ParameterDefinition::class, $definition_id); } + + if ($definition instanceof ParameterDefinition + && ParameterDefinition::INPUT_TYPE_CHOICE === $definition->getInputType()) { + $submitted_value = trim((string) ($submitted_data['value_text'] ?? '')); + $pending_choice = trim((string) ($submitted_data['new_choice_value'] ?? '')); + $canonical_choice = $definition->findCanonicalChoice($submitted_value); + + if (null !== $canonical_choice) { + $submitted_data['value_text'] = $canonical_choice; + $submitted_data['new_choice_value'] = ''; + $pending_choice = ''; + } elseif ('' === $submitted_value) { + $submitted_data['value_text'] = ''; + $submitted_data['new_choice_value'] = ''; + $pending_choice = ''; + } elseif ('' !== $pending_choice) { + $submitted_data['value_text'] = $submitted_value; + if (mb_strtolower($pending_choice) === mb_strtolower($submitted_value)) { + $pending_choice = $submitted_value; + $submitted_data['new_choice_value'] = $pending_choice; + } else { + $submitted_data['new_choice_value'] = ''; + $pending_choice = ''; + } + } + + $event->setData($submitted_data); + } } + $choices = $definition?->getChoices() ?? []; + if ('' !== $pending_choice && !in_array($pending_choice, $choices, true)) { + $choices[] = $pending_choice; + } $this->addValueTextField( $event->getForm(), $definition?->getInputType() ?? ParameterDefinition::INPUT_TYPE_TEXT, - $definition?->getChoices() ?? [], + $choices, ); }); - $builder->addEventListener(FormEvents::SUBMIT, static function (FormEvent $event): void { + $builder->addEventListener(FormEvents::SUBMIT, function (FormEvent $event): void { $parameter = $event->getData(); - if ($parameter instanceof PartParameter && $parameter->getDefinition() instanceof ParameterDefinition) { + if (!$parameter instanceof PartParameter) { + return; + } + + $parameter->clearPendingDefinitionChoice(); + $definition = $parameter->getDefinition(); + $pending_choice = trim((string) $event->getForm()->get('new_choice_value')->getData()); + + if ('' !== $pending_choice) { + $error = null; + $visible_value = trim($parameter->getValueText()); + $canonical_choice = $definition?->findCanonicalChoice($visible_value); + + if (!$definition instanceof ParameterDefinition + || ParameterDefinition::INPUT_TYPE_CHOICE !== $definition->getInputType()) { + $error = 'parameter.validator.new_choice_requires_choice_definition'; + } elseif (null !== $canonical_choice) { + $parameter->setValueText($canonical_choice); + } elseif ($pending_choice !== $visible_value) { + $error = 'parameter.validator.new_choice_value_mismatch'; + } elseif (mb_strlen($pending_choice) > ParameterDefinition::MAX_CHOICE_LENGTH) { + $error = 'parameter_definition.validator.choice_too_long'; + } elseif (!$this->security->isGranted('edit', $definition)) { + $error = 'parameter.validator.new_choice_forbidden'; + } else { + $parameter->requestPendingDefinitionChoice($pending_choice); + } + + if (null !== $error) { + $event->getForm()->get('value_text')->addError(new FormError( + $error, + $error, + ['{{ limit }}' => (string) ParameterDefinition::MAX_CHOICE_LENGTH], + )); + } + } + + if ($definition instanceof ParameterDefinition) { $parameter->refreshSnapshotFromDefinition(); } }); @@ -274,6 +357,8 @@ public function configureOptions(OptionsResolver $resolver): void /** @param list $choices */ private function addValueTextField(FormInterface $form, string $input_type, array $choices): void { + $can_add_choice = $this->security->isGranted('edit', ParameterDefinition::class) ? 'true' : 'false'; + if (ParameterDefinition::INPUT_TYPE_CHOICE === $input_type) { $choice_map = []; foreach ($choices as $choice) { @@ -284,9 +369,15 @@ private function addValueTextField(FormInterface $form, string $input_type, arra 'label' => false, 'required' => false, 'placeholder' => '', + 'empty_data' => '', 'choices' => $choice_map, + 'translation_domain' => 'validators', 'attr' => [ 'class' => 'form-select-sm', + // The parameter autocomplete controller owns this TomSelect instance. Defining an empty + // controller prevents the global choice_widget theme from initializing elements--select first. + 'data-controller' => '', + 'data-can-add-choice' => $can_add_choice, ], ]); @@ -300,6 +391,7 @@ private function addValueTextField(FormInterface $form, string $input_type, arra 'attr' => [ 'placeholder' => 'parameters.text.placeholder', 'class' => 'form-control-sm', + 'data-can-add-choice' => $can_add_choice, ], ]); } diff --git a/src/Repository/ParameterRepository.php b/src/Repository/ParameterRepository.php index de423ae25..dcb545239 100644 --- a/src/Repository/ParameterRepository.php +++ b/src/Repository/ParameterRepository.php @@ -24,6 +24,8 @@ use App\Entity\Parameters\AbstractParameter; use App\Entity\Parameters\ParameterDefinition; +use App\Entity\Parameters\PartParameter; +use App\Entity\Parts\Part; /** * @template TEntityClass of AbstractParameter @@ -31,6 +33,29 @@ */ class ParameterRepository extends DBElementRepository { + /** + * UniqueEntity runs before Doctrine flushes orphan removals. Ignore a persisted PartParameter only when it has + * already been removed from its owning Part's active collection; active database matches remain conflicts. + * + * @param array $criteria + * @return list + */ + public function findActiveForUniqueValidation(array $criteria): array + { + return array_values(array_filter( + $this->findBy($criteria), + static function (AbstractParameter $parameter): bool { + if (!$parameter instanceof PartParameter) { + return true; + } + + $part = $parameter->getElement(); + + return !$part instanceof Part || $part->getParameters()->contains($parameter); + }, + )); + } + public function countByDefinition(ParameterDefinition $definition): int { return (int) $this->createQueryBuilder('parameter') diff --git a/src/Services/LogSystem/TimeTravel.php b/src/Services/LogSystem/TimeTravel.php index 7649b8afd..f33131d69 100644 --- a/src/Services/LogSystem/TimeTravel.php +++ b/src/Services/LogSystem/TimeTravel.php @@ -248,8 +248,10 @@ public function applyEntry(AbstractDBElement $element, TimeTravelInterface $logE if (null === $data) { $element->restoreDefinitionReference(null); } elseif (is_array($data) && isset($data['@id'])) { - $definition = $this->em->getReference(ParameterDefinition::class, $data['@id']); - $element->restoreDefinitionReference($definition); + $definition = $this->em->find(ParameterDefinition::class, $data['@id']); + $element->restoreDefinitionReference( + $definition instanceof ParameterDefinition ? $definition : null + ); } continue; diff --git a/src/Services/Parameters/PendingParameterChoiceApplier.php b/src/Services/Parameters/PendingParameterChoiceApplier.php new file mode 100644 index 000000000..caaa6434a --- /dev/null +++ b/src/Services/Parameters/PendingParameterChoiceApplier.php @@ -0,0 +1,66 @@ +getParameters() as $parameter) { + if (!$parameter instanceof PartParameter) { + continue; + } + + $pending_choice = $parameter->getPendingDefinitionChoice(); + if (null === $pending_choice) { + continue; + } + + $definition = $parameter->getDefinition(); + $pending_choice = trim($pending_choice); + + if (!$definition instanceof ParameterDefinition + || ParameterDefinition::INPUT_TYPE_CHOICE !== $definition->getInputType()) { + throw new LogicException('A pending choice requires a linked Choice parameter definition.'); + } + if ('' === $pending_choice || mb_strlen($pending_choice) > ParameterDefinition::MAX_CHOICE_LENGTH) { + throw new LogicException('The pending parameter choice is invalid.'); + } + if ($pending_choice !== trim($parameter->getValueText())) { + throw new LogicException('The pending choice does not match the parameter value.'); + } + if (!$this->security->isGranted('edit', $definition)) { + throw new AccessDeniedException('Editing this parameter definition is not allowed.'); + } + + $canonical_choice = $definition->addChoice($pending_choice); + $parameter + ->setValueText($canonical_choice) + ->clearPendingDefinitionChoice(); + } + } +} diff --git a/templates/parts/edit/edit_form_styles.html.twig b/templates/parts/edit/edit_form_styles.html.twig index 817ca7670..3aa4d8fa6 100644 --- a/templates/parts/edit/edit_form_styles.html.twig +++ b/templates/parts/edit/edit_form_styles.html.twig @@ -71,7 +71,9 @@ {% block parameter_widget %} {% import 'components/collection_type.macro.html.twig' as collection %} - + {{ form_widget(form.name, {"attr": {"data-pages--parameters-autocomplete-target": "name"}}) }}{{ form_errors(form.name) }} {{ form_widget(form.symbol, {"attr": {"data-pages--parameters-autocomplete-target": "symbol", "data-pages--latex-preview-target": "input"}}) }}{{ form_errors(form.symbol) }} {{ form_widget(form.value_min) }}{{ form_errors(form.value_min) }} @@ -85,6 +87,8 @@ {% if form.definition is defined %} {{ form_widget(form.definition, {"attr": {"data-pages--parameters-autocomplete-target": "definition"}}) }} {{ form_errors(form.definition) }} + {{ form_widget(form.new_choice_value, {"attr": {"data-pages--parameters-autocomplete-target": "newChoiceValue"}}) }} + {{ form_errors(form.new_choice_value) }} {% endif %} diff --git a/tests/API/Endpoints/ParameterDefinitionsEndpointTest.php b/tests/API/Endpoints/ParameterDefinitionsEndpointTest.php index d09c1d075..d3edc8584 100644 --- a/tests/API/Endpoints/ParameterDefinitionsEndpointTest.php +++ b/tests/API/Endpoints/ParameterDefinitionsEndpointTest.php @@ -80,6 +80,26 @@ public function testReadOnlyTokenCannotCreateDefinition(): void self::assertResponseStatusCodeSame(403); } + public function testOverlongChoiceReturnsValidationErrorInsteadOfServerError(): void + { + $client = $this->createDefinitionClient(); + $client->request('POST', self::BASE_PATH, [ + 'json' => [ + 'name' => 'API overlong choice definition', + 'input_type' => ParameterDefinition::INPUT_TYPE_CHOICE, + 'choices' => [str_repeat('X', ParameterDefinition::MAX_CHOICE_LENGTH + 1)], + ], + ]); + + self::assertResponseStatusCodeSame(422); + self::assertJsonContains([ + 'violations' => [[ + 'propertyPath' => 'choices', + 'message' => 'A parameter choice must not exceed 255 characters.', + ]], + ]); + } + public function testUsedDefinitionCannotBeDeletedThroughApi(): void { $client = $this->createDefinitionClient(); diff --git a/tests/Entity/Parameters/ParameterDefinitionTest.php b/tests/Entity/Parameters/ParameterDefinitionTest.php index 0ae9af1bf..e73f0ba22 100644 --- a/tests/Entity/Parameters/ParameterDefinitionTest.php +++ b/tests/Entity/Parameters/ParameterDefinitionTest.php @@ -52,6 +52,16 @@ public function testUnsupportedInputTypeIsRejected(): void (new ParameterDefinition())->setInputType('unsupported'); } + public function testOverlongChoiceIsCanonicalizedWithoutDomainException(): void + { + $choice = str_repeat('X', ParameterDefinition::MAX_CHOICE_LENGTH + 1); + $definition = (new ParameterDefinition()) + ->setInputType(ParameterDefinition::INPUT_TYPE_CHOICE) + ->setChoices([$choice]); + + self::assertSame([$choice], $definition->getChoices()); + } + public function testNameIsTrimmedAndNormalized(): void { $definition = (new ParameterDefinition())->setName(' DiElEcTrIc '); diff --git a/tests/Entity/Parts/PartTest.php b/tests/Entity/Parts/PartTest.php index e855c3402..0e93d4dee 100644 --- a/tests/Entity/Parts/PartTest.php +++ b/tests/Entity/Parts/PartTest.php @@ -22,6 +22,8 @@ namespace App\Tests\Entity\Parts; +use App\Entity\Parameters\ParameterDefinition; +use App\Entity\Parameters\PartParameter; use App\Entity\Parts\MeasurementUnit; use App\Entity\Parts\Part; use App\Entity\Parts\PartLot; @@ -48,6 +50,23 @@ public function testAddRemovePartLot(): void $this->assertTrue($part->getPartLots()->isEmpty()); } + public function testRemovingForeignParameterDoesNotUnlinkItsDefinition(): void + { + $part_a = new Part(); + $part_b = new Part(); + $definition = (new ParameterDefinition())->setName('Foreign parameter definition'); + $parameter = (new PartParameter())->setDefinition($definition); + $part_b->addParameter($parameter); + + $part_a->removeParameter($parameter); + + self::assertFalse($part_a->getParameters()->contains($parameter)); + self::assertTrue($part_b->getParameters()->contains($parameter)); + self::assertSame($part_b, $parameter->getElement()); + self::assertSame($definition, $parameter->getDefinition()); + self::assertTrue($definition->getParameterUsages()->contains($parameter)); + } + public function testGetSetMinamount(): void { $part = new Part(); diff --git a/tests/Form/ParameterTypeTest.php b/tests/Form/ParameterTypeTest.php index 2bfe6107a..216dcff10 100644 --- a/tests/Form/ParameterTypeTest.php +++ b/tests/Form/ParameterTypeTest.php @@ -19,14 +19,24 @@ use App\Entity\Parameters\PartParameter; use App\Entity\Parts\Category; use App\Entity\Parts\Part; +use App\Entity\UserSystem\User; use App\Form\ParameterType; +use App\Services\Parameters\PendingParameterChoiceApplier; +use App\Services\UserSystem\PermissionSchemaUpdater; +use App\Validator\Constraints\UniqueObjectCollection; use Doctrine\ORM\EntityManagerInterface; use PHPUnit\Framework\Attributes\Group; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; use Symfony\Component\Form\Extension\Core\Type\ChoiceType; +use Symfony\Component\Form\Extension\Core\Type\FormType; use Symfony\Component\Form\Extension\Core\Type\TextType; use Symfony\Component\Form\FormFactoryInterface; use Symfony\Component\Form\FormInterface; +use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; +use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; +use Symfony\Component\Security\Core\Exception\AccessDeniedException; +use Symfony\Component\Validator\Constraints\NotBlank; +use Symfony\Component\Validator\Validator\ValidatorInterface; #[Group('DB')] #[Group('slow')] @@ -34,12 +44,14 @@ final class ParameterTypeTest extends KernelTestCase { private EntityManagerInterface $entity_manager; private FormFactoryInterface $form_factory; + private ValidatorInterface $validator; protected function setUp(): void { self::bootKernel(); $this->entity_manager = static::getContainer()->get(EntityManagerInterface::class); $this->form_factory = static::getContainer()->get(FormFactoryInterface::class); + $this->validator = static::getContainer()->get(ValidatorInterface::class); } public function testChoiceDefinitionIsLinkedAndRestrictsSubmittedValue(): void @@ -157,6 +169,481 @@ public function testPersistedChoiceParameterReopensWithItsSelectedChoice(): void self::assertSame('X7R', $reopened_form->createView()['value_text']->vars['value']); } + public function testEmptyChoiceCanBeSubmittedAndReopenedWithoutChangingDefinition(): void + { + $this->loginAs('admin'); + $definition = $this->createDefinition( + 'Form empty dielectric', + ParameterDefinition::INPUT_TYPE_CHOICE, + ['X7R', 'X5R'], + ); + $category = (new Category())->setName('Form empty category'); + $part = (new Part())->setName('Form empty part')->setCategory($category); + $parameter = new PartParameter(); + $part->addParameter($parameter); + + $form = $this->createParameterForm($parameter); + $form->submit($this->submission($definition, '')); + + self::assertTrue($form->isSynchronized()); + self::assertTrue($form->isValid(), (string) $form->getErrors(true)); + self::assertSame('', $parameter->getValueText()); + self::assertSame($definition, $parameter->getDefinition()); + self::assertSame(['X7R', 'X5R'], $definition->getChoices()); + + $this->entity_manager->persist($category); + $this->entity_manager->persist($part); + $this->entity_manager->flush(); + $parameter_id = $parameter->getID(); + $this->entity_manager->clear(); + + $reloaded = $this->entity_manager->find(PartParameter::class, $parameter_id); + self::assertInstanceOf(PartParameter::class, $reloaded); + self::assertSame('', $reloaded->getValueText()); + self::assertInstanceOf(ParameterDefinition::class, $reloaded->getDefinition()); + self::assertSame(['X7R', 'X5R'], $reloaded->getDefinition()->getChoices()); + $reopened_form = $this->createParameterForm($reloaded); + self::assertSame('', $reopened_form->get('value_text')->getData()); + self::assertSame( + 'true', + $reopened_form->get('value_text')->getConfig()->getOption('attr')['data-can-add-choice'], + ); + self::assertSame('', $reopened_form->get('value_text')->getConfig()->getOption('attr')['data-controller']); + + $selected_parameter = (new PartParameter())->setDefinition($reloaded->getDefinition())->setValueText('X7R'); + $selected_form = $this->createParameterForm($selected_parameter); + self::assertSame('X7R', $selected_form->get('value_text')->getData()); + self::assertSame( + 'true', + $selected_form->get('value_text')->getConfig()->getOption('attr')['data-can-add-choice'], + ); + self::assertSame('', $selected_form->get('value_text')->getConfig()->getOption('attr')['data-controller']); + } + + public function testExistingChoiceCanBeClearedAndReopened(): void + { + $definition = $this->createDefinition( + 'Form cleared dielectric', + ParameterDefinition::INPUT_TYPE_CHOICE, + ['X7R', 'X5R'], + ); + $category = (new Category())->setName('Form cleared category'); + $part = (new Part())->setName('Form cleared part')->setCategory($category); + $parameter = (new PartParameter())->setDefinition($definition)->setValueText('X7R'); + $part->addParameter($parameter); + $this->entity_manager->persist($category); + $this->entity_manager->persist($part); + $this->entity_manager->flush(); + + $form = $this->createParameterForm($parameter); + $form->submit($this->submission($definition, '')); + self::assertTrue($form->isValid(), (string) $form->getErrors(true)); + $this->entity_manager->flush(); + $parameter_id = $parameter->getID(); + $this->entity_manager->clear(); + + $reloaded = $this->entity_manager->find(PartParameter::class, $parameter_id); + self::assertInstanceOf(PartParameter::class, $reloaded); + self::assertSame('', $reloaded->getValueText()); + self::assertSame(['X7R', 'X5R'], $reloaded->getDefinition()?->getChoices()); + } + + public function testNewChoiceStaysPendingUntilSuccessfulPartSave(): void + { + $this->loginAs('admin'); + $definition = $this->createDefinition( + 'Form pending dielectric', + ParameterDefinition::INPUT_TYPE_CHOICE, + ['X7R', 'X5R'], + ); + $category = (new Category())->setName('Form pending category'); + $part = (new Part())->setName('Form pending part')->setCategory($category); + $parameter = new PartParameter(); + $part->addParameter($parameter); + + $form = $this->createParameterForm($parameter); + $form->submit($this->submission($definition, 'X6S', new_choice_value: ' X6S ')); + + self::assertTrue($form->isValid(), (string) $form->getErrors(true)); + self::assertSame('X6S', $parameter->getPendingDefinitionChoice()); + self::assertSame(['X7R', 'X5R'], $definition->getChoices(), 'The form alone must not mutate the definition.'); + + static::getContainer()->get(PendingParameterChoiceApplier::class)->apply($part); + self::assertSame(['X7R', 'X5R', 'X6S'], $definition->getChoices()); + self::assertSame('X6S', $parameter->getValueText()); + self::assertNull($parameter->getPendingDefinitionChoice()); + + $this->entity_manager->persist($category); + $this->entity_manager->persist($part); + $this->entity_manager->flush(); + $parameter_id = $parameter->getID(); + $this->entity_manager->clear(); + + $reloaded = $this->entity_manager->find(PartParameter::class, $parameter_id); + self::assertInstanceOf(PartParameter::class, $reloaded); + self::assertSame('X6S', $reloaded->getValueText()); + self::assertContains('X6S', $reloaded->getDefinition()?->getChoices() ?? []); + } + + public function testCanonicalAndBlankPendingValuesNeverCreateChoices(): void + { + $this->loginAs('admin'); + $definition = $this->createDefinition( + 'Form canonical dielectric', + ParameterDefinition::INPUT_TYPE_CHOICE, + ['X7R'], + ); + + $canonical_parameter = new PartParameter(); + $canonical_form = $this->createParameterForm($canonical_parameter); + $canonical_form->submit($this->submission($definition, ' x7r ', new_choice_value: ' x7r ')); + self::assertTrue($canonical_form->isValid(), (string) $canonical_form->getErrors(true)); + self::assertSame('X7R', $canonical_parameter->getValueText()); + self::assertNull($canonical_parameter->getPendingDefinitionChoice()); + + $blank_parameter = new PartParameter(); + $blank_form = $this->createParameterForm($blank_parameter); + $blank_form->submit($this->submission($definition, ' ', new_choice_value: ' ')); + self::assertTrue($blank_form->isValid(), (string) $blank_form->getErrors(true)); + self::assertSame('', $blank_parameter->getValueText()); + self::assertNull($blank_parameter->getPendingDefinitionChoice()); + self::assertSame(['X7R'], $definition->getChoices()); + } + + public function testStalePendingForExistingChoiceIsClearedWithoutApplyingDefinition(): void + { + $this->loginAs('admin'); + $definition = $this->createDefinition( + 'Form stale pending dielectric', + ParameterDefinition::INPUT_TYPE_CHOICE, + ['X7R', 'X5R'], + ); + $parameter = new PartParameter(); + $part = (new Part())->addParameter($parameter); + $form = $this->createParameterForm($parameter); + + $form->submit($this->submission($definition, 'X7R', new_choice_value: ' x7r ')); + + self::assertTrue($form->isValid(), (string) $form->getErrors(true)); + self::assertSame('X7R', $parameter->getValueText()); + self::assertNull($parameter->getPendingDefinitionChoice()); + self::assertEmpty($form->get('new_choice_value')->getData()); + + static::getContainer()->get(PendingParameterChoiceApplier::class)->apply($part); + self::assertSame(['X7R', 'X5R'], $definition->getChoices()); + } + + public function testStalePendingCannotOverrideAnotherVisibleExistingChoice(): void + { + $this->loginAs('admin'); + $definition = $this->createDefinition( + 'Form divergent stale pending dielectric', + ParameterDefinition::INPUT_TYPE_CHOICE, + ['X7R', 'X5R'], + ); + $parameter = new PartParameter(); + $part = (new Part())->addParameter($parameter); + $form = $this->createParameterForm($parameter); + + $form->submit($this->submission($definition, 'X5R', new_choice_value: 'x7r')); + + self::assertTrue($form->isValid(), (string) $form->getErrors(true)); + self::assertSame('X5R', $parameter->getValueText()); + self::assertNull($parameter->getPendingDefinitionChoice()); + self::assertEmpty($form->get('new_choice_value')->getData()); + + static::getContainer()->get(PendingParameterChoiceApplier::class)->apply($part); + self::assertSame(['X7R', 'X5R'], $definition->getChoices()); + } + + public function testRepeatedSubmissionOfNewlySavedChoiceDoesNotAddItAgain(): void + { + $this->loginAs('admin'); + $definition = $this->createDefinition( + 'Form repeated pending dielectric', + ParameterDefinition::INPUT_TYPE_CHOICE, + ['X7R'], + ); + $parameter = new PartParameter(); + $part = (new Part())->addParameter($parameter); + + $first_form = $this->createParameterForm($parameter); + $first_form->submit($this->submission($definition, 'X6S', new_choice_value: 'X6S')); + self::assertTrue($first_form->isValid(), (string) $first_form->getErrors(true)); + static::getContainer()->get(PendingParameterChoiceApplier::class)->apply($part); + self::assertSame(['X7R', 'X6S'], $definition->getChoices()); + + $second_form = $this->createParameterForm($parameter); + $second_form->submit($this->submission($definition, 'X6S', new_choice_value: 'X6S')); + self::assertTrue($second_form->isValid(), (string) $second_form->getErrors(true)); + self::assertNull($parameter->getPendingDefinitionChoice()); + static::getContainer()->get(PendingParameterChoiceApplier::class)->apply($part); + self::assertSame(['X7R', 'X6S'], $definition->getChoices()); + } + + public function testUserWithoutDefinitionEditPermissionCanUseExistingOrEmptyButNotNewChoice(): void + { + $this->loginAs('user'); + $definition = $this->createDefinition( + 'Form permission dielectric', + ParameterDefinition::INPUT_TYPE_CHOICE, + ['X7R'], + ); + + foreach (['X7R', ''] as $value) { + $form = $this->createParameterForm(new PartParameter()); + self::assertSame( + 'false', + $form->get('value_text')->getConfig()->getOption('attr')['data-can-add-choice'], + ); + $form->submit($this->submission($definition, $value)); + self::assertTrue($form->isValid(), (string) $form->getErrors(true)); + } + + $parameter = new PartParameter(); + $form = $this->createParameterForm($parameter); + $form->submit($this->submission($definition, 'X6S', new_choice_value: 'X6S')); + + self::assertFalse($form->isValid()); + self::assertSame(['X7R'], $definition->getChoices()); + self::assertNull($parameter->getPendingDefinitionChoice()); + } + + public function testPendingChoiceDoesNotMutateDefinitionWhenAnotherFormFieldIsInvalid(): void + { + $this->loginAs('admin'); + $definition = $this->createDefinition( + 'Form invalid-root dielectric', + ParameterDefinition::INPUT_TYPE_CHOICE, + ['X7R'], + ); + $parameter = new PartParameter(); + $part = (new Part())->addParameter($parameter); + $form = $this->form_factory->createBuilder(FormType::class, ['parameter' => $parameter]) + ->add('parameter', ParameterType::class, [ + 'data_class' => PartParameter::class, + 'csrf_protection' => false, + ]) + ->add('required_field', TextType::class, [ + 'mapped' => false, + 'constraints' => [new NotBlank()], + ]) + ->getForm(); + + $form->submit([ + 'parameter' => $this->submission($definition, 'X6S', new_choice_value: 'X6S'), + 'required_field' => '', + ]); + + self::assertFalse($form->isValid()); + self::assertSame('X6S', $parameter->getPendingDefinitionChoice()); + // This is the same guard used by PartController: the applier is never called for an invalid root form. + if ($form->isValid()) { + static::getContainer()->get(PendingParameterChoiceApplier::class)->apply($part); + } + self::assertSame(['X7R'], $definition->getChoices()); + } + + public function testPendingChoiceApplierEnforcesDefinitionEditPermission(): void + { + $this->loginAs('user'); + $definition = $this->createDefinition( + 'Form forged pending dielectric', + ParameterDefinition::INPUT_TYPE_CHOICE, + ['X7R'], + ); + $parameter = (new PartParameter()) + ->setDefinition($definition) + ->setValueText('X6S') + ->requestPendingDefinitionChoice('X6S'); + $part = (new Part())->addParameter($parameter); + + try { + static::getContainer()->get(PendingParameterChoiceApplier::class)->apply($part); + self::fail('A forged pending choice must be denied.'); + } catch (AccessDeniedException) { + self::assertSame(['X7R'], $definition->getChoices()); + self::assertSame('X6S', $parameter->getPendingDefinitionChoice()); + } + } + + public function testPersistedChoiceCanBeRemovedAndRecreatedInTheSameUnitOfWork(): void + { + [$definition, $part, $old_parameter] = $this->createPersistedChoiceParameter( + 'Form same request dielectric', + 'X7R', + ); + $old_parameter_id = $old_parameter->getID(); + + $part->removeParameter($old_parameter); + $new_parameter = new PartParameter(); + $part->addParameter($new_parameter); + $form = $this->createParameterForm($new_parameter); + $form->submit($this->submission($definition, 'X7R')); + + self::assertTrue($form->isValid(), (string) $form->getErrors(true)); + self::assertFalse($part->getParameters()->contains($old_parameter)); + self::assertTrue($part->getParameters()->contains($new_parameter)); + self::assertInstanceOf(PartParameter::class, $this->entity_manager->find(PartParameter::class, $old_parameter_id)); + self::assertNull($old_parameter->getDefinition()); + self::assertFalse($definition->getParameterUsages()->contains($old_parameter)); + self::assertTrue($definition->getParameterUsages()->contains($new_parameter)); + self::assertSame('X7R', $new_parameter->getValueText()); + self::assertNull($new_parameter->getPendingDefinitionChoice()); + self::assertEmpty($form->get('new_choice_value')->getData()); + self::assertCount(0, $this->validator->validate($part)); + + static::getContainer()->get(PendingParameterChoiceApplier::class)->apply($part); + self::assertSame(['X7R', 'X5R'], $definition->getChoices()); + $this->entity_manager->persist($part); + $this->entity_manager->flush(); + $new_parameter_id = $new_parameter->getID(); + $part_id = $part->getID(); + $this->entity_manager->clear(); + + $reloaded_part = $this->entity_manager->find(Part::class, $part_id); + self::assertInstanceOf(Part::class, $reloaded_part); + self::assertCount(1, $reloaded_part->getParameters()); + $reloaded_parameter = $reloaded_part->getParameters()->first(); + self::assertInstanceOf(PartParameter::class, $reloaded_parameter); + self::assertSame($new_parameter_id, $reloaded_parameter->getID()); + self::assertSame('X7R', $reloaded_parameter->getValueText()); + self::assertNull($this->entity_manager->find(PartParameter::class, $old_parameter_id)); + } + + public function testPersistedChoiceCanBeRemovedAndRecreatedAfterAnIntermediateFlush(): void + { + [$definition, $part, $old_parameter] = $this->createPersistedChoiceParameter( + 'Form separate requests dielectric', + 'X7R', + ); + $old_parameter_id = $old_parameter->getID(); + $part_id = $part->getID(); + + $part->removeParameter($old_parameter); + $this->entity_manager->flush(); + self::assertNull($this->entity_manager->find(PartParameter::class, $old_parameter_id)); + $this->entity_manager->clear(); + + $part = $this->entity_manager->find(Part::class, $part_id); + $definition = $this->entity_manager->find(ParameterDefinition::class, $definition->getID()); + self::assertInstanceOf(Part::class, $part); + self::assertInstanceOf(ParameterDefinition::class, $definition); + $new_parameter = new PartParameter(); + $part->addParameter($new_parameter); + $form = $this->createParameterForm($new_parameter); + $form->submit($this->submission($definition, 'X7R')); + + self::assertTrue($form->isValid(), (string) $form->getErrors(true)); + self::assertCount(0, $this->validator->validate($part)); + $this->entity_manager->persist($part); + $this->entity_manager->flush(); + $this->entity_manager->clear(); + + $reloaded_part = $this->entity_manager->find(Part::class, $part_id); + self::assertInstanceOf(Part::class, $reloaded_part); + self::assertCount(1, $reloaded_part->getParameters()); + $reloaded_parameter = $reloaded_part->getParameters()->first(); + self::assertInstanceOf(PartParameter::class, $reloaded_parameter); + self::assertSame('X7R', $reloaded_parameter->getValueText()); + } + + public function testTwoSimultaneouslyActiveParametersWithTheSameNameRemainInvalid(): void + { + [$definition, $part] = $this->createPersistedChoiceParameter('Form active duplicate dielectric', 'X7R'); + $duplicate = new PartParameter(); + $part->addParameter($duplicate); + $form = $this->createParameterForm($duplicate); + $form->submit($this->submission($definition, 'X5R')); + + self::assertFalse($form->isValid(), 'UniqueEntity must reject a second active persisted name/group.'); + $violations = $this->validator->validate($part); + self::assertGreaterThan(0, $violations->count()); + $collection_violation_found = false; + foreach ($violations as $violation) { + if ($violation->getCode() === UniqueObjectCollection::IS_NOT_UNIQUE) { + $collection_violation_found = true; + break; + } + } + self::assertTrue($collection_violation_found, 'The active collection duplicate protection must remain enabled.'); + } + + public function testChoiceCanBeRemovedAndRecreatedEmptyInTheSameUnitOfWork(): void + { + [$definition, $part, $old_parameter] = $this->createPersistedChoiceParameter( + 'Form replacement empty dielectric', + '', + ); + + $part->removeParameter($old_parameter); + $new_parameter = new PartParameter(); + $part->addParameter($new_parameter); + $form = $this->createParameterForm($new_parameter); + $form->submit($this->submission($definition, '')); + + self::assertTrue($form->isValid(), (string) $form->getErrors(true)); + self::assertCount(0, $this->validator->validate($part)); + self::assertSame('', $new_parameter->getValueText()); + self::assertNull($new_parameter->getPendingDefinitionChoice()); + $this->entity_manager->persist($part); + $this->entity_manager->flush(); + self::assertCount(1, $part->getParameters()); + } + + public function testChoiceCanBeRemovedAndRecreatedWithAnotherExistingChoice(): void + { + [$definition, $part, $old_parameter] = $this->createPersistedChoiceParameter( + 'Form replacement other dielectric', + 'X7R', + ); + + $part->removeParameter($old_parameter); + $new_parameter = new PartParameter(); + $part->addParameter($new_parameter); + $form = $this->createParameterForm($new_parameter); + $form->submit($this->submission($definition, 'X5R')); + + self::assertTrue($form->isValid(), (string) $form->getErrors(true)); + self::assertCount(0, $this->validator->validate($part)); + self::assertSame('X5R', $new_parameter->getValueText()); + self::assertNull($new_parameter->getPendingDefinitionChoice()); + $this->entity_manager->persist($part); + $this->entity_manager->flush(); + self::assertCount(1, $part->getParameters()); + } + + public function testChoiceCanBeRemovedAndRecreatedWithOnePendingNewChoice(): void + { + $this->loginAs('admin'); + [$definition, $part, $old_parameter] = $this->createPersistedChoiceParameter( + 'Form replacement new dielectric', + 'X7R', + ); + + $part->removeParameter($old_parameter); + $new_parameter = new PartParameter(); + $part->addParameter($new_parameter); + $form = $this->createParameterForm($new_parameter); + $form->submit($this->submission($definition, 'X6R', new_choice_value: 'X6R')); + + self::assertTrue($form->isValid(), (string) $form->getErrors(true)); + self::assertCount(0, $this->validator->validate($part)); + self::assertSame(['X7R', 'X5R'], $definition->getChoices()); + self::assertSame('X6R', $new_parameter->getPendingDefinitionChoice()); + + static::getContainer()->get(PendingParameterChoiceApplier::class)->apply($part); + self::assertSame(['X7R', 'X5R', 'X6R'], $definition->getChoices()); + self::assertSame(1, array_count_values($definition->getChoices())['X6R']); + self::assertNull($new_parameter->getPendingDefinitionChoice()); + $this->entity_manager->persist($part); + $this->entity_manager->flush(); + self::assertCount(1, $part->getParameters()); + $saved_parameter = $part->getParameters()->first(); + self::assertInstanceOf(PartParameter::class, $saved_parameter); + self::assertSame('X6R', $saved_parameter->getValueText()); + } + public function testLinkedParameterEditorUsesCurrentDefinitionMetadataWithoutChangingSnapshots(): void { $definition = $this->createDefinition('Original form name', ParameterDefinition::INPUT_TYPE_TEXT); @@ -176,6 +663,8 @@ public function testLinkedParameterEditorUsesCurrentDefinitionMetadataWithoutCha self::assertSame('Current form name', $form->get('name')->getData()); self::assertSame('new', $form->get('symbol')->getData()); self::assertSame('new-unit', $form->get('unit')->getData()); + self::assertTrue($form->get('symbol')->getConfig()->getOption('attr')['readonly']); + self::assertTrue($form->get('unit')->getConfig()->getOption('attr')['readonly']); } /** @param list|null $choices */ @@ -199,11 +688,40 @@ private function createParameterForm(PartParameter $parameter): FormInterface ]); } + /** @return array{ParameterDefinition, Part, PartParameter} */ + private function createPersistedChoiceParameter(string $definition_name, string $value): array + { + $definition = $this->createDefinition( + $definition_name, + ParameterDefinition::INPUT_TYPE_CHOICE, + ['X7R', 'X5R'], + ); + $category = (new Category())->setName($definition_name . ' category'); + $part = (new Part())->setName($definition_name . ' part')->setCategory($category); + $parameter = (new PartParameter())->setDefinition($definition)->setValueText($value); + $part->addParameter($parameter); + $this->entity_manager->persist($category); + $this->entity_manager->persist($part); + $this->entity_manager->flush(); + + return [$definition, $part, $parameter]; + } + + private function loginAs(string $username): void + { + $user = $this->entity_manager->getRepository(User::class)->findOneBy(['name' => $username]); + self::assertInstanceOf(User::class, $user); + static::getContainer()->get(PermissionSchemaUpdater::class)->userUpgradeSchemaRecursively($user); + $this->entity_manager->flush(); + static::getContainer()->get(TokenStorageInterface::class)->setToken(new UsernamePasswordToken($user, 'main')); + } + /** @return array */ private function submission( ?ParameterDefinition $definition, string $value_text, ?string $name = null, + string $new_choice_value = '', ): array { return [ 'name' => $name ?? $definition?->getName() ?? '', @@ -215,6 +733,7 @@ private function submission( 'value_text' => $value_text, 'group' => '', 'definition' => $definition instanceof ParameterDefinition ? (string) $definition->getID() : '', + 'new_choice_value' => $new_choice_value, 'eda_visibility' => '', 'eda_symbol_visibility' => '', ]; diff --git a/tests/Services/LogSystem/TimeTravelTest.php b/tests/Services/LogSystem/TimeTravelTest.php index 4d6b5c52a..a36a9d43c 100644 --- a/tests/Services/LogSystem/TimeTravelTest.php +++ b/tests/Services/LogSystem/TimeTravelTest.php @@ -148,4 +148,38 @@ public function testApplyingHistoricalParameterDataRestoresPreviousDefinitionRef self::assertSame('Historical snapshot name', $parameter->getSnapshotName()); self::assertSame('Previous definition', $parameter->getEffectiveName()); } + + public function testMissingHistoricalDefinitionFallsBackToSnapshots(): void + { + $deleted_definition = (new ParameterDefinition())->setName('Deleted historical definition'); + $this->em->persist($deleted_definition); + $this->em->flush(); + $deleted_definition_id = $deleted_definition->getID(); + self::assertNotNull($deleted_definition_id); + $this->em->remove($deleted_definition); + $this->em->flush(); + + $parameter = (new PartParameter()) + ->setName('Current snapshot') + ->setSymbol('S') + ->setUnit('V'); + (new \ReflectionClass($parameter))->getProperty('id')->setValue($parameter, 1004); + + $log_entry = new ElementEditedLogEntry($parameter); + $log_entry->setOldData([ + 'definition' => ['@id' => $deleted_definition_id], + 'name' => 'Historical dielectric', + 'symbol' => 'D', + 'unit' => 'grade', + ]); + + $this->service->applyEntry($parameter, $log_entry); + + self::assertNull($parameter->getDefinition()); + self::assertSame('Historical dielectric', $parameter->getEffectiveName()); + self::assertSame('D', $parameter->getEffectiveSymbol()); + self::assertSame('grade', $parameter->getEffectiveUnit()); + self::assertSame(ParameterDefinition::INPUT_TYPE_TEXT, $parameter->getEffectiveInputType()); + self::assertSame([], $parameter->getEffectiveChoices()); + } } diff --git a/translations/frontend.en.xlf b/translations/frontend.en.xlf index d00994931..48f2f08b5 100644 --- a/translations/frontend.en.xlf +++ b/translations/frontend.en.xlf @@ -79,5 +79,14 @@ No + + parameter.choice.add_newAdd "%value%" + + + parameter.choice.newNEW + + + parameter.choice.nothing_selectedNothing selected + diff --git a/translations/frontend.fr.xlf b/translations/frontend.fr.xlf index 2a4590523..3ee81157a 100644 --- a/translations/frontend.fr.xlf +++ b/translations/frontend.fr.xlf @@ -73,5 +73,14 @@ Non + + parameter.choice.add_newAjouter « %value% » + + + parameter.choice.newNOUVEAU + + + parameter.choice.nothing_selectedAucune sélection + diff --git a/translations/validators.en.xlf b/translations/validators.en.xlf index 47a7f8aa7..5cf43c095 100644 --- a/translations/validators.en.xlf +++ b/translations/validators.en.xlf @@ -274,5 +274,26 @@ parameter_definition.name_uniqueA parameter definition with this name already exists. + + parameter.validator.value_not_allowedThe selected value is not part of the linked parameter definition. + + + parameter.validator.value_not_canonicalThe selected value does not use the canonical spelling from the linked parameter definition. + + + parameter.validator.new_choice_requires_choice_definitionA new value can only be added to a linked Choice parameter definition. + + + parameter.validator.new_choice_value_mismatchThe requested new value does not match the selected parameter value. + + + parameter.validator.new_choice_forbiddenYou are not allowed to add a value to this parameter definition. + + + parameter_definition.validator.text_has_choicesA text parameter definition must not contain choices. + + + parameter_definition.validator.choice_too_longA parameter choice must not exceed {{ limit }} characters. + diff --git a/translations/validators.fr.xlf b/translations/validators.fr.xlf index a48456fea..07980daa6 100644 --- a/translations/validators.fr.xlf +++ b/translations/validators.fr.xlf @@ -256,5 +256,26 @@ parameter_definition.name_uniqueUne définition de paramètre portant ce nom existe déjà. + + parameter.validator.value_not_allowedLa valeur sélectionnée ne fait pas partie de la définition de paramètre liée. + + + parameter.validator.value_not_canonicalLa valeur sélectionnée n'utilise pas l'écriture canonique de la définition de paramètre liée. + + + parameter.validator.new_choice_requires_choice_definitionUne nouvelle valeur ne peut être ajoutée qu'à une définition de paramètre Choice liée. + + + parameter.validator.new_choice_value_mismatchLa nouvelle valeur demandée ne correspond pas à la valeur de paramètre sélectionnée. + + + parameter.validator.new_choice_forbiddenVous n'êtes pas autorisé à ajouter une valeur à cette définition de paramètre. + + + parameter_definition.validator.text_has_choicesUne définition de paramètre texte ne doit pas contenir de choix. + + + parameter_definition.validator.choice_too_longUn choix de paramètre ne doit pas dépasser {{ limit }} caractères. + From 5dd48ab2e234c3df11c936bb40bd69cab50a1a42 Mon Sep 17 00:00:00 2001 From: Matt Date: Mon, 24 Aug 2026 21:56:59 +0200 Subject: [PATCH 3/5] Add global parameter filters to part search --- .../parameter_constraint_controller.js | 318 +++++++++++++++ .../Constraints/Part/ParameterConstraint.php | 92 ++++- .../ParameterChoiceConstraintType.php | 76 ++++ .../Constraints/ParameterConstraintType.php | 70 +++- templates/form/filter_types_layout.html.twig | 54 ++- .../Part/ParameterConstraintDoctrineTest.php | 307 +++++++++++++++ .../ParameterConstraintTypeTest.php | 366 ++++++++++++++++++ 7 files changed, 1270 insertions(+), 13 deletions(-) create mode 100644 assets/controllers/filters/parameter_constraint_controller.js create mode 100644 src/Form/Filters/Constraints/ParameterChoiceConstraintType.php create mode 100644 tests/DataTables/Filters/Constraints/Part/ParameterConstraintDoctrineTest.php create mode 100644 tests/Form/Filters/Constraints/ParameterConstraintTypeTest.php diff --git a/assets/controllers/filters/parameter_constraint_controller.js b/assets/controllers/filters/parameter_constraint_controller.js new file mode 100644 index 000000000..f251b4325 --- /dev/null +++ b/assets/controllers/filters/parameter_constraint_controller.js @@ -0,0 +1,318 @@ +/* + * This file is part of Part-DB (https://github.com/Part-DB/Part-DB-symfony). + * + * Copyright (C) 2019 - 2026 Jan Böhmer (https://github.com/jbtronics) + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + */ + +import {Controller} from '@hotwired/stimulus'; +import TomSelect from 'tom-select'; +import {trans} from '../../translator'; +import 'tom-select/dist/css/tom-select.bootstrap5.css'; +import '../../css/components/tom-select_extensions.css'; + +/* stimulusFetch: 'lazy' */ +export default class extends Controller { + static values = { + url: String, + initialInputType: String, + initialChoices: Array, + }; + + static targets = [ + 'name', + 'definition', + 'symbol', + 'symbolContainer', + 'unit', + 'unitContainer', + 'numericContainer', + 'valueOperator', + 'valueText', + ]; + + connect() { + this._initialized = false; + this._restoring = false; + this._initialState = this.captureState(); + this._form = this.nameTarget.form; + this._resetHandler = this.onFormReset.bind(this); + this._form?.addEventListener('reset', this._resetHandler); + + this.setupNameTomSelect(); + + const linked = this.definitionTarget.value !== ''; + this.setAdHocFieldState(linked, linked); + this.configureCurrentValueEditor(linked ? this.initialInputTypeValue : 'text'); + } + + disconnect() { + clearTimeout(this._resetTimer); + this._form?.removeEventListener('reset', this._resetHandler); + this._nameTomSelect?.destroy(); + this.destroyValueTomSelect(); + } + + setupNameTomSelect() { + const settings = { + plugins: ['clear_button', 'restore_on_backspace'], + persistent: false, + maxItems: 1, + delimiter: 'VERY_L0NG_D€LIMITER_WHICH_WILL_NEVER_BE_ENCOUNTERED_IN_A_STRING', + createOnBlur: true, + selectOnTab: true, + create: true, + searchField: 'name', + valueField: 'name', + labelField: 'name', + clearAfterSelect: true, + onItemAdd: value => this.onNameItemAdd(value), + onItemRemove: () => this.onNameItemRemove(), + onInitialize: () => { + this._initialized = true; + }, + render: { + option: (data, escape) => { + let details = ''; + if (data.symbol) { + details += escape(data.symbol); + } + if (data.unit) { + details += (details === '' ? '' : ' ') + '[' + escape(data.unit) + ']'; + } + + return '
    ' + escape(data.name) + '' + + (details === '' ? '' : '
    ' + details + '') + + '
    '; + }, + item: (data, escape) => '
    ' + escape(data.name) + '
    ', + }, + }; + + if (this.hasUrlValue) { + const baseUrl = this.urlValue; + settings.load = (query, callback) => { + fetch(baseUrl.replace('__QUERY__', encodeURIComponent(query))) + .then(response => response.json()) + .then(data => callback(data)) + .catch(() => callback()); + }; + } + + this._nameTomSelect = new TomSelect(this.nameTarget, settings); + } + + onNameItemAdd(value) { + if (!this._initialized || this._restoring) { + return; + } + + const suggestion = this._nameTomSelect.options[value] ?? {}; + const definitionId = Number(suggestion.definition_id); + if (Number.isInteger(definitionId) && definitionId > 0) { + this.enterDefinitionMode({ + id: definitionId, + name: suggestion.name ?? value, + inputType: suggestion.input_type === 'choice' ? 'choice' : 'text', + choices: Array.isArray(suggestion.choices) ? suggestion.choices : [], + }); + return; + } + + this.enterAdHocMode(suggestion); + } + + onNameItemRemove() { + if (!this._initialized || this._restoring) { + return; + } + + this.enterAdHocMode(); + } + + enterDefinitionMode(definition) { + this.setDefinition(definition.id, definition.name); + this.setAdHocFieldState(true, true); + this.replaceValueEditor(definition.inputType, definition.choices, '', ''); + } + + enterAdHocMode(suggestion = {}) { + this.setDefinition(null); + this.setAdHocFieldState(false, true); + this.replaceValueEditor('text', [], '', ''); + + this.symbolTarget.value = suggestion.symbol ?? ''; + this.unitTarget.value = suggestion.unit ?? ''; + this.symbolTarget.dispatchEvent(new Event('input', {bubbles: true})); + this.unitTarget.dispatchEvent(new Event('input', {bubbles: true})); + } + + setDefinition(id, name = '') { + if (id === null) { + this.definitionTarget.value = ''; + } else { + const value = String(id); + if (!Array.from(this.definitionTarget.options).some(option => option.value === value)) { + this.definitionTarget.add(new Option(name, value)); + } + this.definitionTarget.value = value; + } + + this.definitionTarget.dispatchEvent(new Event('change', {bubbles: true})); + } + + setAdHocFieldState(linked, clear) { + for (const container of [this.symbolContainerTarget, this.numericContainerTarget, this.unitContainerTarget]) { + container.classList.toggle('d-none', linked); + } + + const fields = [ + this.symbolTarget, + ...this.numericContainerTarget.querySelectorAll('input, select'), + this.unitTarget, + ]; + + for (const field of fields) { + if (clear) { + field.value = ''; + if (field.tomselect) { + field.tomselect.clear(true); + field.tomselect.sync(); + } + } + + field.disabled = linked; + if (field.tomselect) { + if (linked) { + field.tomselect.disable(); + } else { + field.tomselect.enable(); + } + } + } + + if (clear) { + this.symbolTarget.dispatchEvent(new Event('input', {bubbles: true})); + this.unitTarget.dispatchEvent(new Event('input', {bubbles: true})); + } + } + + configureCurrentValueEditor(inputType) { + const choiceMode = inputType === 'choice' && this.valueTextTarget.tagName === 'SELECT'; + this.valueOperatorTarget.classList.toggle('d-none', choiceMode); + this.valueOperatorTarget.value = choiceMode ? '=' : this.valueOperatorTarget.value; + + if (choiceMode) { + this.setupValueTomSelect(this.valueTextTarget); + } + } + + replaceValueEditor(inputType, choices, value, operator) { + this.destroyValueTomSelect(); + const oldElement = this.valueTextTarget; + const choiceMode = inputType === 'choice'; + const newElement = document.createElement(choiceMode ? 'select' : 'input'); + + for (const attribute of oldElement.attributes) { + if (attribute.name !== 'type' && attribute.name !== 'class') { + newElement.setAttribute(attribute.name, attribute.value); + } + } + newElement.setAttribute('data-controller', ''); + + if (choiceMode) { + newElement.className = 'form-select'; + newElement.add(new Option('', '')); + for (const choice of choices) { + newElement.add(new Option(choice, choice)); + } + newElement.value = choices.includes(value) ? value : ''; + } else { + newElement.type = 'search'; + newElement.className = 'form-control'; + newElement.value = value; + } + + oldElement.replaceWith(newElement); + this.valueOperatorTarget.classList.toggle('d-none', choiceMode); + this.valueOperatorTarget.value = choiceMode ? '=' : operator; + + if (choiceMode) { + this.setupValueTomSelect(newElement); + } + } + + setupValueTomSelect(element) { + if (element.tomselect) { + element.tomselect.destroy(); + } + + this._valueTomSelect = new TomSelect(element, { + plugins: ['clear_button'], + allowEmptyOption: true, + create: false, + maxItems: 1, + selectOnTab: true, + placeholder: trans('parameter.choice.nothing_selected'), + }); + } + + destroyValueTomSelect() { + this._valueTomSelect?.destroy(); + this._valueTomSelect = undefined; + } + + captureState() { + return { + name: this.nameTarget.value, + definitionId: this.definitionTarget.value, + definitionName: this.definitionTarget.selectedOptions[0]?.text ?? this.nameTarget.value, + inputType: this.initialInputTypeValue, + choices: [...this.initialChoicesValue], + symbol: this.symbolTarget.value, + unit: this.unitTarget.value, + numeric: Array.from(this.numericContainerTarget.querySelectorAll('input, select')).map(field => field.value), + value: this.valueTextTarget.value, + operator: this.valueOperatorTarget.value, + }; + } + + onFormReset() { + clearTimeout(this._resetTimer); + this._resetTimer = setTimeout(() => this.restoreInitialState(), 0); + } + + restoreInitialState() { + if (!this.element.isConnected) { + return; + } + + const state = this._initialState; + this._restoring = true; + this._nameTomSelect.clear(true); + if (state.name !== '') { + if (!this._nameTomSelect.options[state.name]) { + this._nameTomSelect.addOption({name: state.name}); + } + this._nameTomSelect.setValue(state.name, true); + } + this.setDefinition(state.definitionId === '' ? null : state.definitionId, state.definitionName); + + this.symbolTarget.value = state.symbol; + this.unitTarget.value = state.unit; + const numericFields = this.numericContainerTarget.querySelectorAll('input, select'); + numericFields.forEach((field, index) => { + field.value = state.numeric[index] ?? ''; + field.tomselect?.sync(); + }); + + this.replaceValueEditor(state.inputType, state.choices, state.value, state.operator); + const linked = state.definitionId !== ''; + this.setAdHocFieldState(linked, linked); + this._restoring = false; + } +} diff --git a/src/DataTables/Filters/Constraints/Part/ParameterConstraint.php b/src/DataTables/Filters/Constraints/Part/ParameterConstraint.php index e68dd989f..2fbefd1c9 100644 --- a/src/DataTables/Filters/Constraints/Part/ParameterConstraint.php +++ b/src/DataTables/Filters/Constraints/Part/ParameterConstraint.php @@ -24,6 +24,7 @@ use App\DataTables\Filters\Constraints\AbstractConstraint; use App\DataTables\Filters\Constraints\TextConstraint; +use App\Entity\Parameters\ParameterDefinition; use App\Entity\Parameters\PartParameter; use Doctrine\ORM\QueryBuilder; @@ -35,6 +36,8 @@ class ParameterConstraint extends AbstractConstraint protected string $unit = ''; + protected ?ParameterDefinition $definition = null; + protected TextConstraint $value_text; protected ParameterValueConstraint $value; @@ -54,11 +57,29 @@ public function __construct() public function isEnabled(): bool { - return true; + if ($this->definition instanceof ParameterDefinition) { + if (ParameterDefinition::INPUT_TYPE_CHOICE === $this->definition->getInputType()) { + return $this->value_text->isEnabled() + && '=' === $this->value_text->getOperator() + && '' !== trim((string) $this->value_text->getValue()); + } + + return $this->value_text->isEnabled() || $this->value->isEnabled(); + } + + return '' !== trim($this->name) + || '' !== trim($this->symbol) + || '' !== trim($this->unit) + || $this->value_text->isEnabled() + || $this->value->isEnabled(); } public function apply(QueryBuilder $queryBuilder): void { + if (!$this->isEnabled()) { + return; + } + //Create a new qb to build the subquery $subqb = new QueryBuilder($queryBuilder->getEntityManager()); @@ -68,7 +89,48 @@ public function apply(QueryBuilder $queryBuilder): void ->from(PartParameter::class, $this->alias) ->where($this->alias . '.element = part'); - if ($this->name !== '') { + $value_text_applied = false; + if ($this->definition instanceof ParameterDefinition) { + $definition_param = $this->generateParameterIdentifier('params.definition'); + $definition_name_param = $this->generateParameterIdentifier('params.definition_name'); + + if (ParameterDefinition::INPUT_TYPE_CHOICE === $this->definition->getInputType() + && '=' === $this->value_text->getOperator() + && $this->value_text->isEnabled()) { + $value_param = $this->generateParameterIdentifier('params.value_text'); + $legacy_value_param = $this->generateParameterIdentifier('params.legacy_value_text'); + $subqb->andWhere(sprintf( + '((%1$s.definition = :%2$s AND %1$s.value_text = :%4$s) OR ' + .'(%1$s.definition IS NULL AND ILIKE(TRIM(%1$s.name), :%3$s) = TRUE ' + .'AND ILIKE(TRIM(%1$s.value_text), :%5$s) = TRUE))', + $this->alias, + $definition_param, + $definition_name_param, + $value_param, + $legacy_value_param, + )); + $subqb->setParameter($value_param, $this->value_text->getValue()); + $subqb->setParameter( + $legacy_value_param, + $this->escapeLikeValue(trim((string) $this->value_text->getValue())), + ); + $value_text_applied = true; + } else { + $subqb->andWhere(sprintf( + '(%1$s.definition = :%2$s OR ' + .'(%1$s.definition IS NULL AND ILIKE(TRIM(%1$s.name), :%3$s) = TRUE))', + $this->alias, + $definition_param, + $definition_name_param, + )); + } + + $subqb->setParameter($definition_param, $this->definition); + $subqb->setParameter( + $definition_name_param, + $this->escapeLikeValue(trim($this->definition->getName())), + ); + } elseif ($this->name !== '') { $paramName = $this->generateParameterIdentifier('params.name'); $subqb->andWhere($this->alias . '.name = :' . $paramName); $queryBuilder->setParameter($paramName, $this->name); @@ -87,8 +149,13 @@ public function apply(QueryBuilder $queryBuilder): void } //Apply all subfilters - $this->value_text->apply($subqb); - $this->value->apply($subqb); + if (!$value_text_applied) { + $this->value_text->apply($subqb); + } + if (!$this->definition instanceof ParameterDefinition + || ParameterDefinition::INPUT_TYPE_CHOICE !== $this->definition->getInputType()) { + $this->value->apply($subqb); + } //Copy all parameters from the subquery to the main query //We can not use setParameters here, as this would override the exiting paramaters in queryBuilder @@ -132,6 +199,23 @@ public function setUnit(string $unit): ParameterConstraint return $this; } + public function getDefinition(): ?ParameterDefinition + { + return $this->definition; + } + + public function setDefinition(?ParameterDefinition $definition): ParameterConstraint + { + $this->definition = $definition; + + return $this; + } + + private function escapeLikeValue(string $value): string + { + return str_replace(['\\', '%', '_'], ['\\\\', '\\%', '\\_'], $value); + } + public function getValueText(): TextConstraint { return $this->value_text; diff --git a/src/Form/Filters/Constraints/ParameterChoiceConstraintType.php b/src/Form/Filters/Constraints/ParameterChoiceConstraintType.php new file mode 100644 index 000000000..48d68f72a --- /dev/null +++ b/src/Form/Filters/Constraints/ParameterChoiceConstraintType.php @@ -0,0 +1,76 @@ +setRequired('parameter_choices'); + $resolver->setAllowedTypes('parameter_choices', 'array'); + } + + public function buildForm(FormBuilderInterface $builder, array $options): void + { + $choices = []; + foreach ($options['parameter_choices'] as $choice) { + $choices[$choice] = $choice; + } + + $builder->add('value', ChoiceType::class, [ + 'choices' => $choices, + 'choice_translation_domain' => false, + 'required' => false, + 'placeholder' => 'selectpicker.nothing_selected', + 'empty_data' => '', + ]); + + $builder->addEventListener(FormEvents::PRE_SET_DATA, static function (FormEvent $event): void { + $constraint = $event->getData(); + if ($constraint instanceof TextConstraint) { + $constraint->setOperator('='); + } + }); + + $builder->addEventListener(FormEvents::PRE_SUBMIT, static function (FormEvent $event): void { + $submitted_data = $event->getData(); + if (is_array($submitted_data)) { + $submitted_data['operator'] = '='; + $event->setData($submitted_data); + } + }); + + $builder->addEventListener(FormEvents::SUBMIT, static function (FormEvent $event): void { + $constraint = $event->getData(); + if ($constraint instanceof TextConstraint) { + $constraint->setOperator('='); + } + }); + } + + public function getParent(): string + { + return TextConstraintType::class; + } +} diff --git a/src/Form/Filters/Constraints/ParameterConstraintType.php b/src/Form/Filters/Constraints/ParameterConstraintType.php index 3c3b396d9..a65f88348 100644 --- a/src/Form/Filters/Constraints/ParameterConstraintType.php +++ b/src/Form/Filters/Constraints/ParameterConstraintType.php @@ -23,16 +23,24 @@ namespace App\Form\Filters\Constraints; use App\DataTables\Filters\Constraints\Part\ParameterConstraint; +use App\Entity\Parameters\ParameterDefinition; +use Doctrine\ORM\EntityManagerInterface; +use Symfony\Bridge\Doctrine\Form\Type\EntityType; use Symfony\Component\Form\AbstractType; use Symfony\Component\Form\Extension\Core\Type\SearchType; use Symfony\Component\Form\Extension\Core\Type\TextType; use Symfony\Component\Form\FormBuilderInterface; use Symfony\Component\Form\FormEvent; use Symfony\Component\Form\FormEvents; +use Symfony\Component\Form\FormInterface; use Symfony\Component\OptionsResolver\OptionsResolver; class ParameterConstraintType extends AbstractType { + public function __construct(private readonly EntityManagerInterface $entity_manager) + { + } + public function configureOptions(OptionsResolver $resolver): void { $resolver->setDefaults([ @@ -44,8 +52,16 @@ public function configureOptions(OptionsResolver $resolver): void public function buildForm(FormBuilderInterface $builder, array $options): void { + $builder->add('definition', EntityType::class, [ + 'class' => ParameterDefinition::class, + 'choice_label' => 'name', + 'required' => false, + 'placeholder' => '', + ]); + $builder->add('name', TextType::class, [ 'required' => false, + 'empty_data' => '', ]); $builder->add('unit', SearchType::class, [ @@ -71,12 +87,62 @@ public function buildForm(FormBuilderInterface $builder, array $options): void * arguments. * Ensure that the data is never null, but use an empty ParameterConstraint instead */ - $builder->addEventListener(FormEvents::PRE_SET_DATA, function (FormEvent $event) { + $builder->addEventListener(FormEvents::PRE_SET_DATA, function (FormEvent $event): void { $data = $event->getData(); if ($data === null) { - $event->setData(new ParameterConstraint()); + $data = new ParameterConstraint(); + $event->setData($data); } + + $this->addValueTextField( + $event->getForm(), + $data instanceof ParameterConstraint ? $data->getDefinition() : null, + ); }); + + $builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event): void { + $submitted_data = $event->getData(); + $definition = null; + + if (is_array($submitted_data)) { + $definition_id = filter_var( + $submitted_data['definition'] ?? null, + FILTER_VALIDATE_INT, + ['options' => ['min_range' => 1]], + ); + if (false !== $definition_id) { + $definition = $this->entity_manager->find(ParameterDefinition::class, $definition_id); + } + + if ($definition instanceof ParameterDefinition) { + $submitted_data['name'] = $definition->getName(); + $submitted_data['symbol'] = ''; + $submitted_data['unit'] = ''; + $submitted_data['value'] = [ + 'operator' => '', + 'value1' => '', + 'value2' => '', + ]; + $event->setData($submitted_data); + } + } + + $this->addValueTextField($event->getForm(), $definition); + }); + } + + private function addValueTextField(FormInterface $form, ?ParameterDefinition $definition): void + { + if ($definition instanceof ParameterDefinition + && ParameterDefinition::INPUT_TYPE_CHOICE === $definition->getInputType()) { + $form->add('value_text', ParameterChoiceConstraintType::class, [ + 'parameter_choices' => $definition->getChoices(), + ]); + + return; + } + + $form->add('value_text', TextConstraintType::class); } } diff --git a/templates/form/filter_types_layout.html.twig b/templates/form/filter_types_layout.html.twig index cae9e3ea8..8013df3c8 100644 --- a/templates/form/filter_types_layout.html.twig +++ b/templates/form/filter_types_layout.html.twig @@ -50,12 +50,52 @@ {% block parameter_constraint_widget %} {% import 'components/collection_type.macro.html.twig' as collection %} - - {{ form_widget(form.name, {"attr": {"data-pages--parameters-autocomplete-target": "name"}}) }} - {{ form_widget(form.symbol, {"attr": {"data-pages--parameters-autocomplete-target": "symbol", "data-pages--latex-preview-target": "input"}}) }} - {{ form_widget(form.value) }} - {{ form_widget(form.unit, {"attr": {"data-pages--parameters-autocomplete-target": "unit", "data-pages--latex-preview-target": "input"}}) }} - {{ form_widget(form.value_text) }} + {% set definition = form.definition.vars.data %} + {% set initial_input_type = definition ? definition.inputType : 'text' %} + + + {{ form_widget(form.name, {"attr": {"data-filters--parameter-constraint-target": "name"}}) }} + {{ form_widget(form.definition, {"attr": { + "class": "d-none", + "data-controller": "", + "data-filters--parameter-constraint-target": "definition" + }}) }} + {{ form_errors(form.definition) }} + + + {{ form_widget(form.symbol, {"attr": { + "data-filters--parameter-constraint-target": "symbol", + "data-pages--latex-preview-target": "input" + }}) }} + + + {{ form_widget(form.value) }} + + {{ form_widget(form.unit, {"attr": { + "data-filters--parameter-constraint-target": "unit", + "data-pages--latex-preview-target": "input" + }}) }} + + + +
    + {{ form_widget(form.value_text.operator, {"attr": { + "class": initial_input_type == 'choice' ? 'd-none' : '', + "data-controller": "", + "data-filters--parameter-constraint-target": "valueOperator" + }}) }} + {{ form_widget(form.value_text.value, {"attr": { + "data-controller": "", + "data-filters--parameter-constraint-target": "valueText" + }}) }} +
    + {{ form_errors(form.value_text.operator) }} + {{ form_errors(form.value_text.value) }} +