yii2 icon indicating copy to clipboard operation
yii2 copied to clipboard

Extend support for PHP's enum

Open glpzzz opened this issue 11 months ago • 4 comments

Q A
Is bugfix? ? (was failing when the value of the field was an enum)
New feature? ✔️
Breaks BC?
Fixed issues

If the value of the model attribute is an enum,then it fails with Object of class common\enums\Frequency could not be converted to string due to several places where the value is tried to be used directly as an string. E.g checking the selected value against the options to set the "selected" or "checked" one.

Also, the client validation of the RangeValidator was failing when the items in range are the enum cases as in:

public function rules(){
    return [
        ['attribute', 'in', 'range' => MyEnum::cases()],
    ];
}

This is work in progress yet. Trying to find other places which could require changes...

glpzzz avatar Jan 27 '25 20:01 glpzzz

Codecov Report

Attention: Patch coverage is 10.00000% with 9 lines in your changes missing coverage. Please review.

Project coverage is 64.83%. Comparing base (d146307) to head (17d7e68). Report is 30 commits behind head on master.

Files with missing lines Patch % Lines
framework/validators/RangeValidator.php 0.00% 5 Missing :warning:
framework/helpers/BaseHtml.php 20.00% 4 Missing :warning:
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #20318      +/-   ##
============================================
- Coverage     64.84%   64.83%   -0.02%     
- Complexity    11433    11439       +6     
============================================
  Files           431      431              
  Lines         37187    37197      +10     
============================================
+ Hits          24115    24116       +1     
- Misses        13072    13081       +9     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Jan 27 '25 20:01 codecov[bot]

In general, what about creating "enum" rule?

public function rules(){
    return [
        ['attribute', 'in', 'range' => MyEnum::cases()],

        // vs
        ['attribute', 'in', 'enum' => MyEnum::class, 'target' => 'values'], // attribute is one of enum values, default
        ['attribute', 'in', 'enum' => MyEnum::class, 'target' => 'names'], // support enum names as well

        // implement the last keys with the original approach instead
        ['attribute', 'in', 'range' => array_map(fn(MyEnum $case) => $case->name, MyEnum::cases())],
    ];
}

xepozz avatar Jan 29 '25 11:01 xepozz

I don't think this:

['attribute', 'in', 'enum' => MyEnum::class, 'target' => 'values'], // attribute is one of enum values, default
['attribute', 'in', 'enum' => MyEnum::class, 'target' => 'names'], // support enum names as well

is necessary as it can be achieved with the current RangeValidator just by using an array_map as you did in the last example in your comment. That validating that the value is one of the names (string) or one of the values (string|int) of the enum.

The first one:

['attribute', 'in', 'range' => MyEnum::cases()],

validates that the actual value on the attribute is an instance of the enum, which is the intention.

The current implementation supports the enum range validation on server side. It only fails on client side as the provided values in range must be converted to string and enums can not be directly converted to strings.

glpzzz avatar Jan 29 '25 15:01 glpzzz

I'm adding tests to this PR:

This works:

<?php

/**
 * @link https://www.yiiframework.com/
 * @copyright Copyright (c) 2008 Yii Software LLC
 * @license https://www.yiiframework.com/license/
 */

namespace yiiunit\data\enums;

enum StatusEnum: int
{
    case ACTIVE = 1;
    case DELETED = 2;
    case INACTIVE = 0;
}
<?php

/**
 * @link https://www.yiiframework.com/
 * @copyright Copyright (c) 2008 Yii Software LLC
 * @license https://www.yiiframework.com/license/
 */

namespace yiiunit\data\enums;

enum ColorEnum
{
    case BLUE;
    case GREEN;
    case RED;
}
/**
 * @requires PHP >= 8.1
 */
public function testGetAttributeValueWithBackedEnum(): void
{
    $model = new HtmlTestModel();
    $model->name = StatusEnum::ACTIVE;

    $this->assertSame(
        1,
        Html::getAttributeValue($model, 'name'),
        "Backed enum value should be returned by 'getAttributeValue()' method.",
    );
}

/**
 * @requires PHP >= 8.1
 */
public function testGetAttributeValueWithUnitEnum(): void
{
    $model = new HtmlTestModel();
    $model->name = ColorEnum::RED;

    $this->assertSame(
        'RED',
        Html::getAttributeValue($model, 'name'),
        "Unit enum name should be returned by 'getAttributeValue()' method.",
    );
}
    
/**
  * @requires PHP >= 8.1
  */
public function testValidateValueWithBackedEnum(): void
{
    $val = new RangeValidator(['range' => StatusEnum::cases()]);

    $this->assertTrue($val->validate(StatusEnum::ACTIVE));
    $this->assertTrue($val->validate(StatusEnum::INACTIVE));
    $this->assertFalse($val->validate(5));
}

Now the big question should the validator validate the scalar of the enum case?

/**
  * @requires PHP >= 8.1
  */
public function testValidateValueWithBackedEnum(): void
{
    $val = new RangeValidator(['range' => StatusEnum::cases()]);

    $this->assertTrue($val->validate(1)); // it should be `true`, currently it doesn't work because it compares with the case?
}

terabytesoftw avatar Oct 15 '25 10:10 terabytesoftw