Skip to content

Update tou_version and tou_acceptances migration#1028

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.

Sign up for GitHub

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

Closed
dati18 wants to merge 7 commits into from
Closed

Update tou_version and tou_acceptances migration #1028

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6a3589f
Refactor UserTermsOfUseAcceptance and TermsOfUseVersion Models
dati18 Dec 29, 2025
e0f554c
add comment on booted() hook
dati18 Dec 29, 2025
a630e3a
fix linting error
dati18 Dec 29, 2025
3754c00
Tied tou_acceptances and tou_versions with a foreign key
dati18 Dec 29, 2025
e103876
rename tou_acceptances migration so tou_versions run first
dati18 Dec 29, 2025
28de5d5
Create new migrations, delete old ones
dati18 Jan 6, 2026
61a7904
remove unnecessary booted() method
dati18 Jan 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions app/TermsOfUseVersion.php
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -13,8 +13,14 @@
class TermsOfUseVersion extends Model {
use HasFactory;

protected $primaryKey = 'version';

protected $keyType = 'string';

protected $table = 'tou_versions';

public $incrementing = false;

const FIELDS = [
'version',
'active',
Expand Down
1 change: 0 additions & 1 deletion ...ations/2025_10_14_091126_tou_versions.php → ...rations/2026_01_06_131233_tou_version.php
View file
Open in desktop
Comment thread
dati18 marked this conversation as resolved.

@outdooracornoutdooracornJan 7, 2026•
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it is renaming and modifying an existing database migration. Each change to the database schema requires its own migration. If you delete or modify an existing migration, you will likely break Laravel's migration tracking.

In this case, I expect that:

  • Trying to roll back the 2025_10_14_091126_tou_versions.php migration will fail, as that file no longer exists
  • When trying to apply the 2026_01_06_131233_tou_version.php migration, it would fail due to the tou_versions table already existing. Did this succeed when you applied this migration locally?

This PR should contain a new migration file that makes the required changes to the existingtou_versions and tou_acceptances tables. See https://laravel.com/docs/10.x/migrations#updating-tables. All previous migration files should remain untouched.

I also recommend reading up on how Laravel's database migration process works. Laravel's docs on Database Migrations is probably a good place to start.

Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,6 @@
*/
public function up(): void {
Schema::create('tou_versions', function (Blueprint $table) {
$table->id();
$table->string('version')->unique();

@outdooracornoutdooracornJan 7, 2026•
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, make the column a CHAR instead of VARCHAR:

Schema::table('tou_versions', function (Blueprint $table) {
....
$table->char('version', 10)->change();
....
}

@outdooracornoutdooracornJan 7, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember to make the version column the new primary key!

$table->boolean('active')->default(false);
$table->timestamps();
Expand Down
7 changes: 6 additions & 1 deletion ...ons/2025_09_29_194758_tou_acceptances.php → ...ons/2026_01_06_131319_tou_acceptances.php
View file
Open in desktop
Comment thread
dati18 marked this conversation as resolved.

@outdooracornoutdooracornJan 7, 2026•
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, each time the database schema is changed, a new migration is required.

Original file line numberDiff line numberDiff line change
Expand Up@@ -12,11 +12,16 @@ public function up(): void {
Schema::create('tou_acceptances', function (Blueprint $table) {
$table->id();
$table->unsignedInteger('user_id');
$table->string('tou_version', 10);
$table->string('tou_version');

@outdooracornoutdooracornJan 7, 2026•
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How come you've removed the $length parameter from the string() method? This will now use the default length of 255 ([1][2]) when we know the length will always be 10 characters.

As hinted at in #977 (review), I think we should use char(10) to create a fixed width CHAR column. We don't need the flexibility that a VARCHAR column provides, so the slight performance improvement and data efficiency would be good to benefit from.

You'll probably have to do something like this to update the column's data type:

Schema::table('tou_acceptances', function (Blueprint $table) {
....
$table->char('tou_version', 10)->change();
....
}

$table->timestamp('tou_accepted_at');
$table->timestamps();
$table->unique(['user_id', 'tou_version']);
$table->foreign('user_id')->references('id')->on('users')->cascadeOnDelete();
$table->foreign('tou_version')
->references('version')
->on('tou_versions')
->cascadeOnUpdate()
->restrictOnDelete();
});
}

Expand Down
Loading