I'm currently writing a module that requires knowing when various things were created (things that we don't necessarily know about ahead of time). That basically involves guessing the column / object key where the created time is stored. Almost universally across core and other modules, "created" is used for the timestamp things were created and "timestamp" is used for more ambiguous times (such as if the same column keeps track of both the time the entity was created and changed). Additionally "changed" is used in core and usually in contrib for the column representing the timestamp when an entity was updated. Personally I much prefer "last_updated" or simply "updated" over "changed," but I don't choose these things. Also I care a lot less about that column because it doesn't particularly affect what I'm doing. :-P

Anyway, the reason I'm bringing this up is because in order to get the timestamp I currently have to do this:

function EXAMPLE_get_timestamp($data) {
  // Practically everything uses either "timestamp" or "created." User Relationships uses created_at and Userpoints uses time_stamp.
  foreach (array('timestamp', 'created', 'created_at', 'time_stamp') as $key) {
    if (isset($data->$key)) {
      return $data->$key;
    }
  }
  // If we don't find anything, return the current time. This will be accurate except when reading existing data.
  return time();
}

Here is what I would like to do instead:

  return (isset($data->timestamp) ? $data->timestamp : (isset($data->created) ? $data->created : time()));

My preference for ternary syntax aside, I believe there is an argument to be made for consistency, especially consistency with core. I don't really expect this to happen in a stable branch, but 7.x-1.x is still in alpha... ;-) Something to keep in mind for future branches anyway.

Comments

berdir’s picture

I'd be ok with this change if you can provide a patch... :)

icecreamyou’s picture

Status: Active » Needs review
StatusFileSize
new7.82 KB

Sure.

To create this patch I ran:

grep -lr -e 'created_at' * | xargs sed -i 's/created_at/created/g'
grep -lr -e 'updated_at' * | xargs sed -i 's/updated_at/changed/g'
git diff > ur-standardize-date-columns-1230790.patch
berdir’s picture

Note that grep won't write the update function for you ;)

Let's see what the tests say...

icecreamyou’s picture

StatusFileSize
new8.51 KB

Note that grep won't write the update function for you ;)

Oh, right. Well, boo. It should. :-P

Here's a version with an update function.

berdir’s picture

Status: Needs review » Needs work
+++ b/user_relationships.installundefined
@@ -102,3 +102,21 @@ function user_relationships_update_7001() {
+
+/**
+ * Change column names from created_at and updated_at to created and changed.
+ */
+function user_relationships_update_7003() {
+  db_change_field(
+    'user_relationships',
+    'created_at',
+    'created', ¶
+    array('type' => 'int', 'unsigned' => TRUE, 'not null' => TRUE, 'default' => 0)
+  );
+  db_change_field(
+    'user_relationships',
+    'updated_at',
+    'changed', ¶
+    array('type' => 'int', 'unsigned' => TRUE, 'not null' => TRUE, 'default' => 0)
+  );

Trailing spaces after changed/created.

Note that there are also indexes on these columns which must be dropped before the rename and re-added again (db_change_field as a $new_keys argument..) as explained on http://api.drupal.org/api/drupal/includes--database--database.inc/functi....

icecreamyou’s picture

Status: Needs work » Needs review
StatusFileSize
new8.51 KB

Note that there are also indexes on these columns

None that I can see. The schema for {user_relationships} looks like this:

    'primary key' => array('requester_id', 'requestee_id', 'rtid'),
    'indexes' => array(
      'requester_id' => array('requester_id'),
      'requestee_id' => array('requestee_id'),
      'rtid' => array('rtid'),
      'rid' => array('rid'),
    ),

No indexes on created/changed there.

Attached patch with whitespace fixes.

berdir’s picture

You are correct, my mistake.

berdir’s picture

Status: Needs review » Fixed

Thanks for the patch, commited and pushed!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.