search

fuel / orm

Fuel PHP Framework - Fuel v1.x ORM
http://fuelphp.com/docs/packages/orm/intro.html
151 stars 95 forks source link

Why destroying override setting is expected? #374

Closed takayukioda closed 9 years ago

takayukioda commented 9 years ago

This is related to PR #373 .

I have one question that I couldn't understand; why destroying the override setting is expected behavior? I can understand that when I create new common model, I should use empty array to enable the override. Though, there are some packages, such as Auth_User class in Auth package, are not defined by empty array ( https://github.com/fuel/auth/blob/1.8/develop/classes/model/auth/user.php#L38 ). If it's on purpose, what is the benefit of disable override setting? Is there any security reason?

What I wanted to do was to use Overser_Typing for User class (which is extended class of Auth_User) for strict comparison instead of casting user's id every time. So, it is a bit annoying to cast every time, but can be solve without changing OrmModel I'm just curious about the benefit of not allowing to override the settings.

Thank you.

WanWizard commented 9 years ago

That is indeed not correct, it should be

'id' => array(),

as @stevewest already mentioned.

emlynwest commented 9 years ago

Given that this has already been explained I am closing this issue.

emlynwest commented 9 years ago

The ability to easily override or extend properties is a known limitation with the v1 orm, something that is being addressed in v2. At the moment you either have to copy the $_properties array and edit as desired or make changes in the Model's public static _init() method.

WanWizard commented 9 years ago

Proper property definition for the Auth models is in the making. ;)

takayukioda commented 9 years ago

@stevewest Sorry for asking same question and thank you for you kind reply. By read your comment, I realized that my problem occurred because I tried not simply copying the $_properties but merge with extended properties by array_merge. Thank you for the idea to use public static _init(), I couldn't come up with that; I will fix my code with your solution, thanks.

@WanWizard Thank you for the speedy fix!