Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUGFIX: Properly handle object identity DTOs in select box editor #3482

Merged
merged 1 commit into from
Dec 18, 2023

Conversation

grebaldi
Copy link
Contributor

@grebaldi grebaldi commented May 3, 2023

fixes: #2553

The Problem

A node property value may be persisted as an object identity DTO:

{
  "__identity": "bef64ff9-4c58-4104-bee6-1395eff1e303",
  "__type": "Vendor\\Package\\Type"
}

If this is the case and the property is configured to be edited via select box, the persisted serialized object identity DTO is not recognized by the selectbox.

Reproduction

You'll need to prepare the following files:

[Insert Path To Package]/Configuration/NodeTypes.[Insert Some Content Node Type Name].yaml:

'Vendor.Site:Content.SomeContent':
  # ...
  properties:

    tag:
      type: Neos\Media\Domain\Model\Tag
      defaultValue: null
      ui:
        label: 'Tag'
        reloadIfChanged: true
        inspector:
          group: example
          editorOptions:
            allowEmpty: true

    heckLetsAddABunchOfMoreTags:
      type: array<Neos\Media\Domain\Model\Tag>
      ui:
        label: 'Too many tags 😱'
        inspector:
          group: example
          editorOptions:
            multiple: true

☝️ This one's to have an example with two select box editors covering both the single-select and the multi-select case. You may add this to any existing node type or create a new one (can be a document as well).

[Insert Path To Package]/Configuration/Settings.Neos.Neos.yaml:

Neos:
  Neos:
    userInterface:
      inspector:
        dataTypes:
          Neos\Media\Domain\Model\Tag:
            typeConverter: Neos\Neos\TypeConverter\EntityToIdentityConverter
            editor: Neos.Neos/Inspector/Editors/SelectBoxEditor
            editorOptions:
              dataSourceIdentifier: 'tag-data-source'
          array<Neos\Media\Domain\Model\Tag>:
            typeConverter: Neos\Flow\Property\TypeConverter\TypedArrayConverter
            editor: 'Neos.Neos/Inspector/Editors/SelectBoxEditor'
            editorOptions:
              dataSourceIdentifier: 'tag-data-source'

☝️ This one's to achieve default type conversion and editor assignment for Neos\Media\Domain\Model\Tag and it's array counterpart. Funny side note: #2553 seems to suggest that this is actually supposed to be the default. During my research I was unable to confirm this, but it would make sense :)

[Insert Path To Package]/Classes/Application/DataSource/TagDataSource.php:

<?php

/*
 * This script belongs to the package "Vendor.Site".
 *
 * This package is Open Source Software. For the full copyright and license
 * information, please view the LICENSE file which was distributed with this
 * source code.
 */

declare(strict_types=1);

namespace Vendor\Site\Application\DataSource;

use Neos\Flow\Annotations as Flow;
use Neos\Neos\Service\DataSource\AbstractDataSource;
use Neos\ContentRepository\Domain\Model\NodeInterface;
use Neos\Flow\Persistence\Doctrine\PersistenceManager;
use Neos\Media\Domain\Repository\TagRepository;

#[Flow\Scope("singleton")]
final class TagDataSource extends AbstractDataSource
{
    /**
     * @var string
     */
    static protected $identifier = 'tag-data-source';

    /**
     * @Flow\Inject(lazy=false)
     * @var TagRepository
     */
    protected $tagRepository;

    /**
     * @Flow\Inject(lazy=false)
     * @var PersistenceManager
     */
    protected $persistenceManager;

    /**
     * Get data
     *
     * {@inheritdoc}
     */
    public function getData(NodeInterface $node = NULL, array $arguments = [])
    {
        $tags = $this->tagRepository->findAll();
        $result = [];

        foreach ($tags as $tag) {
            $result[] = [
                'value' => $this->persistenceManager->getIdentifierByObject($tag),
                'icon' => 'tag',
                'label' => $tag->getLabel()
            ];
        }

        return $result;
    }
}

☝️ This is the data source that provides our select boxes from above with options straight from the Media package's tag repository.

For reproduction, just add a node of the type from above and see what happens if you set the select box to a value and then reload.

The solution

It's a bit of a lazy solution, but I couldn't think of anything more sophisticated that wouldn't break the select box editor. The gist is: I added a function that pre-processes incoming select box editor values and converts any detected object identity DTO down to it's string-based identity.

@github-actions github-actions bot added Bug Label to mark the change as bugfix 7.3 labels May 3, 2023
@grebaldi grebaldi linked an issue May 3, 2023 that may be closed by this pull request
@Sebobo
Copy link
Member

Sebobo commented May 5, 2023

Very nice, will take a look.
I had actually hoped to add DataSources for Tags and Collections already in 8.3 but wasn't able to finish it on time :(
But it would be great to add this to the Media UI as an interim solution. Together with this fix and the new AssetsHelper in 8.3 it should be much nicer to work with assets based on their tags and collections.

@Sebobo Sebobo self-requested a review May 5, 2023 07:00
@mhsdesign mhsdesign self-requested a review June 8, 2023 07:38
@mhsdesign
Copy link
Member

What is needed for this to be merged? Is it ready for review?

@grebaldi
Copy link
Contributor Author

grebaldi commented Jul 3, 2023

It's ready :) Just needs review

Copy link
Member

@Sebobo Sebobo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx, works as expected with your example 🙂

I'm fine with refactoring the amazingly called new method at a later point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
7.3 Bug Label to mark the change as bugfix next-patch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SelectBoxEditor with ModelDataSource cannot be set to "null"
4 participants