feat: add automatic timestamps for various tables

[iPurpl3x]

Changes proposed in this pull request:
Database migrations that add timestamps to various tables:

• groups
• group_user
• group_permission
• tags
• discussion_tag
• post_mentions_post
• post_mentions_user

The timestamps are not really used anywhere in Flarum, but can be very handy for analytics purposes.

This PR is kind of a follow-up of flarum/likes#28

Reviewers should focus on:
The migrations run without error and don't cause any unwanted side-effects (including performance aspects).

Necessity

• Has the problem that is being solved here been clearly explained?
• If applicable, have various options for solving this problem been considered?
• For core PRs, does this need to be in core, or could it be in an extension?
• Are we willing to maintain this for years / potentially forever?

Confirmed

• Frontend changes: tested on a local Flarum installation.
• Backend changes: tests are green (run composer test).
• Core developer confirmed locally this works as intended.
• Tests have been added, or are not appropriate here.

Required changes:

• Related documentation PR: (Remove if irrelevant)
• Related core extension PRs: (Remove if irrelevant)

[askvortsov1]

Haven't tested locally, but makes sense!

[SychO9]

Could you please for every timestamp add it to the relevant model's $dates array property for proper casting?

also very minor but there seems to be a space in the filename:
2022_05_20_000005_add_created_at_to_ post_mentions_post_table.php

[davwheat]

The only issue with this is that it would set created_at to the time the migration is run, which isn't accurate and could pollute statistics.

Would it not be better to set to null initially, then update the default value to be the current time?

[iPurpl3x]

@SychO9 Sure, I added the casts (for the tables that have corresponding models) and fixed the file name (good that you saw that one!)

@davwheat I have thought of that. The thing is that:

IF we choose to update the value of the column automatically in the DB (and not use Laravel, which might slightly impact performance) then the DEFAULT is going to be current_timestamp() so I would need to run a second migration that sets all the old values to NULL after the column(s) have been added. Do you want me to do that ?

[davwheat]

In my opinion, running two migrations to do that (can they be merged into two steps of one migration? I have no idea) would be the best option.

I'd be interested to hear the opinions of other maintainers though.

[iPurpl3x]

@davwheat If I don't use the \Flarum\Database\Migration helper, and write the migration by hand, I might be able to merge it all into one file.

I also would like to hear opinions of others...

[SychO9]

How about creating the columns first, then modifying them to use current (and use current on update where appropriate). That avoids having to modify all rows.

use Illuminate\Database\Schema\Blueprint;
use Illuminate\Database\Schema\Builder;

return [
    'up' => function (Builder $schema) {
        $schema->table('groups', function (Blueprint $table) {
            $table->timestamp('created_at')->nullable();
        });

        // do this manually because dbal doesn't recognize timestamp columns
        $connection = $schema->getConnection();
        $prefix = $connection->getTablePrefix();
        $connection->statement("ALTER TABLE $prefix`groups` MODIFY created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP");
    },

    'down' => function (Builder $schema) {
        $schema->table('groups', function (Blueprint $table) {
            $table->dropColumn('created_at');
        });
    }
];

Also it might be good to have a comment on each migration that mentions the columns were added for third-party analytical purposes.

[iPurpl3x]

Ah yes, that's pretty clever @SychO9, I will try that I keep you updated.

Agreed for the comments.

[iPurpl3x]

I have now updated all the migrations, using the idea of @SychO9 to makes that all existing rows in the DB stay with NULL on the timestamps and don't get a false value of the time when the migration is done.

Hopefully that makes this PR now mergeable 🤞🏻

[davwheat]

Tested locally. Seems to work as intended on non-prefixed DBs.