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

Fix annotation for entity field mapped to enum type. #959

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

ADmad
Copy link
Member

@ADmad ADmad commented Nov 2, 2023

Closes #958

@ADmad ADmad added this to the 3.x (CakePHP 5) milestone Nov 2, 2023
@othercorey
Copy link
Member

Looks good, but will let @LordSimal confirm the type is built properly.

@LordSimal
Copy link
Contributor

LGTM 👍🏻

@LordSimal LordSimal merged commit 369cfb1 into 3.x Nov 2, 2023
6 checks passed
@LordSimal LordSimal deleted the enum-entity-annotation branch November 2, 2023 15:55

default:
if (str_starts_with($type, 'enum-')) {
$dbType = TypeFactory::build($type);
Copy link
Member

Choose a reason for hiding this comment

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

@LordSimal Won't this throw an exception if the type isn't found? Is that ok for a catch-all?

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, we might wanna use try catch

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not really sure this situation is possible.
Getting to this case where the cake type string starts with enum- (basically) requires it being generated by

EnumType::from(MyEnum::class)

So that DBType needs to be present in the TypeFactory

@dereuromark
Copy link
Member

For IdeHelper I did dereuromark/cakephp-ide-helper#333
Just waiting for a release here to constraint to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Baked enum docblock wrong
4 participants