omines / datatables-bundle

DataTables bundle for Symfony
https://omines.github.io/datatables-bundle/
MIT License
251 stars 113 forks source link

Working with PHP Enums #317

Closed craigh closed 7 months ago

craigh commented 7 months ago

I am struggling to create a column where the property is configured as an enum value. To be clear, the database simply stores a varchar but the Doctrine entity defines the property an enum. It appears that this library is attempting to convert the value to a string too early in the process.

Entity:

    #[ORM\Column(length: 2, enumType: USState::class)]
    protected USState $state;

Datatable:

            ->add(
                'state',
                TextColumn::class,
                [
                    'label' => 'State',
                    'render' => fn (USState $state) => $state->value,
                ]
            )

I get an exception:

Object of class App\Enum\USState could not be converted to string

It never reaches the render stage. Any ideas here?

craigh commented 7 months ago
<?php

declare(strict_types=1);

namespace App\Datatables\Column;

use Omines\DataTablesBundle\Column\AbstractColumn;
use Symfony\Component\OptionsResolver\OptionsResolver;

class EnumColumn extends AbstractColumn
{
    public function normalize($value): string
    {
        if (empty($value)) {
            return '';
        }
        if ($value instanceof $this->options['class']) {
            return $value->value;
        }
        throw new \InvalidArgumentException('Class must be of type BackedEnum');
    }

    protected function configureOptions(OptionsResolver $resolver)
    {
        parent::configureOptions($resolver);

        $resolver
            ->setDefault('class', \BackedEnum::class)
            ->setAllowedTypes('class', 'string')
        ;

        return $this;
    }
}
curry684 commented 7 months ago

I was just about to post "You should create an EnumColumn in which you override the normalize method, but I see you figured it out ;)

I'll happily merge a PR adding the EnumColumn in a futureproof way (PHP didn't have enums when we originally developed the bundle).

craigh commented 7 months ago

I'll happily merge a PR adding the EnumColumn in a futureproof way (PHP didn't have enums when we originally developed the bundle).

Since your next version of the bundle (0.8) breaks semver and requires php >=8.1 (and Symfony ^6.3) it seems a future-proof method wouldn't be necessary.

curry684 commented 7 months ago

We're not breaking semver, major version 0 is an exception case on purpose and Composer considers the minors to be breaking.

craigh commented 7 months ago

@curry684 sure 😄 but it could be argued that a project that has been in the public since 2017 and is well used (and liked) and now seeing greater use with the abandonment of sg/datatables probably should be at at least version 1.0.0 for some time. In any case, the new requirements of version 0.8 will not allow us to use it as the project I work on is still at php 8.1 and Symfony 5.4. I guess I am stuck with v0.7 for now. Out off curiosity, what prompted the new requirements?

curry684 commented 7 months ago

Yes, it could be argued, and I'd counter with "feel free to do the required maintenance for a full 1.0 bundle" 😉 That would require BC planning, deprecations, service releases, maintaining old branches and all. I don't have time for that.

Fact is that DataTables.js is technically end of life. It still depends on jQuery, which is for all intents and purposes dead. When we created this bundle over 6 years ago, we fully expected either DataTables to reinvent itself, or a replacement to establish itself. Either way, the bundle would need fundamental refactoring and rewriting, or become obsolete. It's still unclear how long this bundle will have a future.

I'm still maintaining it and planning to keep doing that. I upgraded the static analysis to PHPStan level 8 last month and ditched all PHP5 constructs that were still in there. PHP5 and 7 are end of life, so that's fine. PHPStan level 8 however required all methods to have typed parameters and return values, which is a breaking change to 0.7, so we're releasing a 0.8 for that, which is how Semver tells us to do breaking changes in major version 0. We're fully compliant.

You do not need to upgrade, you can stay on 0.7 for as long as you wish. Once you upgrade your project to Symfony 6.3 or later you can update to 0.8. If you need a change in 0.7, I'll even branch it and release a 0.7.3, all exactly as prescribed by Semver. Whether 0.7 ever has an EnumColumn is up to you at this point, not me.

And if you don't like my release policies, you don't need to use this bundle, just as I don't need to invest time maintaining it. I'm doing that for the public good and because we still use it ourselves in some projects.

curry684 commented 7 months ago

0.7 is branched: https://github.com/omines/datatables-bundle/tree/0.7

Please ensure your PRs target the correct branch before submitting.

craigh commented 7 months ago

I appreciate the discussion. Please don't misunderstand my thoughts for criticism. I appreciate the bundle (and your work maintaining it) and hope it lives on 😄

ftr, apparently datatables can be used without jQuery https://datatables.net/manual/installation (Non-jQuery initialisation)

althaus commented 7 months ago

ftr, apparently datatables can be used without jQuery https://datatables.net/manual/installation (Non-jQuery initialisation)

I'm pretty sure that DT is still using jQuery under the hood. Only the initilization can be done without it.

curry684 commented 7 months ago

I'm pretty sure that DT is still using jQuery under the hood. Only the initilization can be done without it.

Yep, it just bundles it in 'independent' mode, without relying on jQuery globals.

They've been trying literally for over 5 years to get rid of it: https://datatables.net/forums/discussion/51702/non-jquery-version-of-datatables-is-this-gonna-happen