Skip to content

Commit

Permalink
Merge pull request #5965 from christianbeeznest/rna-22246
Browse files Browse the repository at this point in the history 
Internal: Fix course template selection and error handling in settings management - refs BT#22246
  • Loading branch information
@NicoDucou
NicoDucou authored Dec 13, 2024
2 parents ac17881 + 9be2926 commit 4be43e5
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 107 deletions.
11 changes: 8 additions & 3 deletions src/CoreBundle/Controller/Admin/SettingsController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,15 +152,20 @@ public function updateSetting(Request $request, AccessUrlHelper $accessUrlHelper
$manager->save($form->getData());
$message = $this->trans('Settings have been successfully updated');
} catch (ValidatorException $validatorException) {
// $message = $this->trans($exception->getMessage(), [], 'validators');
$message = $this->trans($validatorException->getMessage());
$messageType = 'error';
}

$this->addFlash($messageType, $message);
if (!empty($keywordFromGet)) {
return $this->redirect($request->headers->get('referer'));
if (!empty($keyword)) {
return $this->redirectToRoute('chamilo_platform_settings_search', [
'keyword' => $keyword,
]);
}

return $this->redirectToRoute('chamilo_platform_settings', [
'namespace' => $namespace,
]);
}
$schemas = $manager->getSchemas();

Expand Down
27 changes: 19 additions & 8 deletions src/CoreBundle/Form/DataTransformer/ResourceToIdentifierTransformer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,23 +31,34 @@ public function transform($value)
return null;
}

/* @psalm-suppress ArgumentTypeCoercion */
Assert::isInstanceOf($value, $this->repository->getClassName());
if (is_object($value) && method_exists($value, 'getId')) {
return $value;
}

if (is_numeric($value)) {
return $this->repository->find($value);
}

return PropertyAccess::createPropertyAccessor()->getValue($value, $this->identifier);
return $value;
}

public function reverseTransform($value)
{
if (null === $value) {
if (null === $value || '' === $value) {
return null;
}

$resource = $this->repository->findOneBy([
$this->identifier => $value,
]);
if (is_object($value) && method_exists($value, 'getId')) {

Check failure on line 51 in src/CoreBundle/Form/DataTransformer/ResourceToIdentifierTransformer.php

View workflow job for this annotation

GitHub Actions / PHP 8.2 Test on ubuntu-latest

NoValue 

src/CoreBundle/Form/DataTransformer/ResourceToIdentifierTransformer.php:51:48: NoValue: All possible types for this argument were invalidated - This may be dead code (see https://psalm.dev/179)
return $value;

Check failure on line 52 in src/CoreBundle/Form/DataTransformer/ResourceToIdentifierTransformer.php

View workflow job for this annotation

GitHub Actions / PHP 8.2 Test on ubuntu-latest

NoValue 

src/CoreBundle/Form/DataTransformer/ResourceToIdentifierTransformer.php:52:13: NoValue: All possible types for this return were invalidated - This may be dead code (see https://psalm.dev/179)
}

$resource = $this->repository->find($value);
if (null === $resource) {
throw new TransformationFailedException(\sprintf('Object "%s" with identifier "%s"="%s" does not exist.', $this->repository->getClassName(), $this->identifier, $value));
throw new TransformationFailedException(sprintf(
'Object "%s" with identifier "%s" does not exist.',
$this->repository->getClassName(),
$value
));
}

return $resource;
Expand Down
10 changes: 6 additions & 4 deletions src/CoreBundle/Settings/CourseSettingsSchema.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,10 @@ public function buildSettings(AbstractSettingsBuilder $builder): void
'course_hide_tools',
new ArrayToIdentifierTransformer()
)
/* ->setTransformer(
->setTransformer(
'course_creation_use_template',
new ResourceToIdentifierTransformer($this->getRepository())
)*/
new ResourceToIdentifierTransformer($this->getRepository(), 'id')
)
;

$allowedTypes = [
Expand Down Expand Up @@ -229,7 +229,9 @@ public function buildForm(FormBuilderInterface $builder): void
'class' => Course::class,
'placeholder' => 'Choose ...',
'empty_data' => null,
'data' => null,
'choice_label' => 'title',
'choice_value' => 'id',
'required' => false,
]
)
->add('hide_scorm_export_link', YesNoType::class)
Expand Down
118 changes: 26 additions & 92 deletions src/CoreBundle/Settings/SettingsManager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,13 +259,7 @@ public function load(string $schemaAlias, ?string $namespace = null, bool $ignor

foreach ($settingsBuilder->getTransformers() as $parameter => $transformer) {
if (\array_key_exists($parameter, $parameters)) {
if ('course_creation_use_template' === $parameter) {
if (empty($parameters[$parameter])) {
$parameters[$parameter] = null;
}
} else {
$parameters[$parameter] = $transformer->reverseTransform($parameters[$parameter]);
}
$parameters[$parameter] = $transformer->reverseTransform($parameters[$parameter]);
}
}

Expand All @@ -290,10 +284,8 @@ public function update(SettingsInterface $settings): void
// 2. Is defined as an array in class DocumentSettingsSchema
// 3. Add transformer for that variable "ArrayToIdentifierTransformer"
// 4. Here we recover the transformer and convert the array to string
foreach ($settingsBuilder->getTransformers() as $parameter => $transformer) {
if (\array_key_exists($parameter, $parameters)) {
$parameters[$parameter] = $transformer->transform($parameters[$parameter]);
}
foreach ($parameters as $parameter => $value) {
$parameters[$parameter] = $this->transformToString($value);
}

$settings->setParameters($parameters);
Expand Down Expand Up @@ -329,11 +321,6 @@ public function update(SettingsInterface $settings): void
->setAccessUrlLocked(1)
;

// @var ConstraintViolationListInterface $errors
/*$errors = $this->validator->validate($parameter);
if (0 < $errors->count()) {
throw new ValidatorException($errors->get(0)->getMessage());
}*/
$this->manager->persist($parameter);
}
}
Expand All @@ -359,10 +346,8 @@ public function save(SettingsInterface $settings): void
// 2. Is defined as an array in class DocumentSettingsSchema
// 3. Add transformer for that variable "ArrayToIdentifierTransformer"
// 4. Here we recover the transformer and convert the array to string
foreach ($settingsBuilder->getTransformers() as $parameter => $transformer) {
if (\array_key_exists($parameter, $parameters)) {
$parameters[$parameter] = $transformer->transform($parameters[$parameter]);
}
foreach ($parameters as $parameter => $value) {
$parameters[$parameter] = $this->transformToString($value);
}
$settings->setParameters($parameters);
$persistedParameters = $this->repository->findBy([
Expand All @@ -373,12 +358,6 @@ public function save(SettingsInterface $settings): void
$persistedParametersMap[$parameter->getVariable()] = $parameter;
}

// @var SettingsEvent $event
/*$event = $this->eventDispatcher->dispatch(
SettingsEvent::PRE_SAVE,
new SettingsEvent($settings, $parameters)
);*/

$url = $this->getUrl();
$simpleCategoryName = str_replace('chamilo_core.settings.', '', $namespace);

Expand All @@ -397,77 +376,11 @@ public function save(SettingsInterface $settings): void
->setAccessUrlLocked(1)
;

// @var ConstraintViolationListInterface $errors
/*$errors = $this->validator->validate($parameter);
if (0 < $errors->count()) {
throw new ValidatorException($errors->get(0)->getMessage());
}*/
$this->manager->persist($parameter);
}
$this->manager->persist($parameter);
}

$this->manager->flush();

// $schemaAlias = $settings->getSchemaAlias();
// $schemaAliasChamilo = str_replace('chamilo_core.settings.', '', $schemaAlias);
//
// $schema = $this->schemaRegistry->get($schemaAlias);
//
// $settingsBuilder = new SettingsBuilder();
// $schema->buildSettings($settingsBuilder);
//
// $parameters = $settingsBuilder->resolve($settings->getParameters());
//
// foreach ($settingsBuilder->getTransformers() as $parameter => $transformer) {
// if (array_key_exists($parameter, $parameters)) {
// $parameters[$parameter] = $transformer->transform($parameters[$parameter]);
// }
// }
//
// /** @var \Sylius\Bundle\SettingsBundle\Event\SettingsEvent $event */
// $event = $this->eventDispatcher->dispatch(
// SettingsEvent::PRE_SAVE,
// new SettingsEvent($settings)
// );
//
// /** @var SettingsCurrent $url */
// $url = $event->getSettings()->getAccessUrl();
//
// foreach ($parameters as $name => $value) {
// if (isset($persistedParametersMap[$name])) {
// if ($value instanceof Course) {
// $value = $value->getId();
// }
// $persistedParametersMap[$name]->setValue($value);
// } else {
// $setting = new Settings();
// $setting->setSchemaAlias($schemaAlias);
//
// $setting
// ->setNamespace($schemaAliasChamilo)
// ->setName($name)
// ->setValue($value)
// ->setUrl($url)
// ->setAccessUrlLocked(0)
// ->setAccessUrlChangeable(1)
// ;
//
// /** @var ConstraintViolationListInterface $errors */
// /*$errors = $this->->validate($parameter);
// if (0 < $errors->count()) {
// throw new ValidatorException($errors->get(0)->getMessage());
// }*/
// $this->manager->persist($setting);
// $this->manager->flush();
// }
// }
/*$parameters = $settingsBuilder->resolve($settings->getParameters());
* $settings->setParameters($parameters);
* $this->eventDispatcher->dispatch(SettingsEvent::PRE_SAVE, new SettingsEvent($settings));
* $this->manager->persist($settings);
* $this->manager->flush();
* $this->eventDispatcher->dispatch(SettingsEvent::POST_SAVE, new SettingsEvent($settings));*/
}

/**
Expand Down Expand Up @@ -1078,4 +991,25 @@ private function fixCategory($variable, $defaultCategory)

return $settings[$variable] ?? $defaultCategory;
}

private function transformToString($value): string
{
if (is_array($value)) {
return implode(',', $value);
}

if ($value instanceof Course) {
return (string) $value->getId();
}

if (is_bool($value)) {
return $value ? 'true' : 'false';
}

if (is_null($value)) {
return '';
}

return (string) $value;
}
}

0 comments on commit 4be43e5

Please sign in to comment.