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

Add hasChanges to AR #10

Open
Auramel opened this issue May 30, 2018 · 6 comments
Open

Add hasChanges to AR #10

Auramel opened this issue May 30, 2018 · 6 comments
Labels
status:ready for adoption Feel free to implement this issue. type:enhancement Enhancement
Milestone

Comments

@Auramel
Copy link

Auramel commented May 30, 2018

Hi! I have idea - add method to \yii\db\ActiveRecord. I think it conveniently.

/**
 * @return bool
 */
public function hasChanges(): bool
{
    return sizeof($this->getDirtyAttributes()) > 0;
}

And, maybe, rename dirtyAttributes() to changes() or changedAttributes()?

@samdark
Copy link
Member

samdark commented May 30, 2018

Sounds good to me.

@rob006
Copy link

rob006 commented Jun 2, 2018

I would like to remind you that AR is is not a final class but only a base for user models - every new method reserves some name and may make naming things harder for the end user. I'm pretty sure that many people already use hasChanges() for they own purpose and many others would like to do this in future.

@CedricYii
Copy link
Contributor

CedricYii commented Jun 8, 2018

I would rather avoid to change naming logic, and keep "dirty"'s one.

I would go for BaseActiveRecord::isDirty().

However BaseActiveRecord::isAttributeChanged() should be BaseActiveRecord::isAttributeDirty() to be consistent with markAttributeDirty() and getDirtyAttributes().

@samdark
Copy link
Member

samdark commented Jun 12, 2018

@rob006 and @CedricYii both have a point.

@armpogart
Copy link

Yep, I'm also for consistent naming and in that case add isDirty won't be a big problem as it's not so common to use isDirty method name in your models. So I'm for renaming isAttributeChanged to isAttributeDirty and adding isDirty. (or it could be vice versa, renaming everything to changed and changes)

If the proposal ok, I can make the PR.

@samdark
Copy link
Member

samdark commented Jul 7, 2018

@armpogart please do. Dirty sounds better because @rob006 arguments.

@samdark samdark transferred this issue from yiisoft/yii2 Nov 3, 2018
@samdark samdark added type:enhancement Enhancement status:ready for adoption Feel free to implement this issue. labels Nov 3, 2018
@Tigrov Tigrov added this to the 1.0.0 milestone Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:ready for adoption Feel free to implement this issue. type:enhancement Enhancement
Projects
None yet
Development

No branches or pull requests

6 participants