CTimeStampBehavior columns not set immediately after save() #406

Closed
mikehaertl opened this Issue Feb 26, 2012 · 10 comments

Comments

Projects
None yet
4 participants
@mikehaertl

I'd like to come back to this google code issue:
http://code.google.com/p/yii/issues/detail?id=2104

If you save a record and have CTimestampBehavior attached, it will by default set the timestamp columns to CDbExpressions. So if you read out this value in the same request you'll not get what you expect.

I know, that we can circumvent this by either refreshing the AR or by setting timestampExpression. I consider both to be a workaround. The behavior should work transparently and not require so much extra consideration or configuration.

A solid fix would be, to set the right value on the PHP side, at least for those column types which can be autodetected (DATE, DATETIME and TIMESTAMP).

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Feb 26, 2012

Member

Think this is more a general issue, not limited to CTimestampBehavior.
This should be done for all CDbExpressions set to a value, so after save it would be the value that's in the database.
But I am not sure how to implement this without causing confusion due to unexpected behavior in complex situations.
Thought about some solutions, but all of them are too messy to post them here, can't find a good one...

Member

cebe commented Feb 26, 2012

Think this is more a general issue, not limited to CTimestampBehavior.
This should be done for all CDbExpressions set to a value, so after save it would be the value that's in the database.
But I am not sure how to implement this without causing confusion due to unexpected behavior in complex situations.
Thought about some solutions, but all of them are too messy to post them here, can't find a good one...

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Feb 26, 2012

Member

One could do a $this->owner->refresh() in CTimeStampBehavior afterSave(), but I'm still not sure if this is a good solution. If you have a create action and you redirect to view after save, you do not need that extra reload.

Member

cebe commented Feb 26, 2012

One could do a $this->owner->refresh() in CTimeStampBehavior afterSave(), but I'm still not sure if this is a good solution. If you have a create action and you redirect to view after save, you do not need that extra reload.

@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Feb 27, 2012

Member

Refresh will produce extra db query that's generally not desired if you don't need data immediately after saving.

Member

samdark commented Feb 27, 2012

Refresh will produce extra db query that's generally not desired if you don't need data immediately after saving.

@mikehaertl

This comment has been minimized.

Show comment
Hide comment
@mikehaertl

mikehaertl Feb 27, 2012

I agree that this is a general problem of all CDbExpression. But that does not mean, that CTimestampBehavior can not be fixed. It does not have to use such an expression at all. The less classes we have which rely on a DB expression the better.

I agree that this is a general problem of all CDbExpression. But that does not mean, that CTimestampBehavior can not be fixed. It does not have to use such an expression at all. The less classes we have which rely on a DB expression the better.

@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Feb 27, 2012

Member

Well, you have an option to use timestamps while configuring it. As for not using db expression it can cause BC and other problems if timezone is different at DB server.

Member

samdark commented Feb 27, 2012

Well, you have an option to use timestamps while configuring it. As for not using db expression it can cause BC and other problems if timezone is different at DB server.

@mikehaertl

This comment has been minimized.

Show comment
Hide comment
@mikehaertl

mikehaertl Feb 27, 2012

Ok, that's a point. Still it's a pitty that it requires so much extra configuration. Guess i'll not use CTimestampBehavior anymore and even if it's breaking DRY i'll prefer to use the simple "raw" approach instead:

<?php
public function beforeSave()
{
    $this->changed_at=date('Y-m-d H:i:s');
    if($this->isNewRecord)
        $this->created_at=$this->changed_at;
}

Ok, that's a point. Still it's a pitty that it requires so much extra configuration. Guess i'll not use CTimestampBehavior anymore and even if it's breaking DRY i'll prefer to use the simple "raw" approach instead:

<?php
public function beforeSave()
{
    $this->changed_at=date('Y-m-d H:i:s');
    if($this->isNewRecord)
        $this->created_at=$this->changed_at;
}
@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Feb 27, 2012

Member

You could do exactly that in a behavior, so it will be DRY.

Member

cebe commented Feb 27, 2012

You could do exactly that in a behavior, so it will be DRY.

@mikehaertl

This comment has been minimized.

Show comment
Hide comment
@mikehaertl

mikehaertl Feb 27, 2012

I know ... :)

I know ... :)

@samdark samdark closed this Feb 28, 2012

@PrplHaz4

This comment has been minimized.

Show comment
Hide comment
@PrplHaz4

PrplHaz4 Feb 28, 2012

@mikehaertl how is that different from what the existing CTimestampBehavior does? just curious, as I've implemented this pattern as well, and am not using the CTimestampBehavior for the mere fact that I also like to store CreatedBy and UpdatedBy at the same time... (which I would've thought is pretty common...)

@mikehaertl how is that different from what the existing CTimestampBehavior does? just curious, as I've implemented this pattern as well, and am not using the CTimestampBehavior for the mere fact that I also like to store CreatedBy and UpdatedBy at the same time... (which I would've thought is pretty common...)

@mikehaertl

This comment has been minimized.

Show comment
Hide comment
@mikehaertl

mikehaertl Feb 29, 2012

@PrplHaz4 See above for how CTimestampBehavior uses CDbExpressions instead of setting a raw value. BTW you can enable setUpdateOnCreate and it will also set update time when creating a record.

@PrplHaz4 See above for how CTimestampBehavior uses CDbExpressions instead of setting a raw value. BTW you can enable setUpdateOnCreate and it will also set update time when creating a record.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment