Skip to content

Add PolicySeeder#1178

Open
tarrow wants to merge 9 commits into
mainfrom
addPolicySeed
Open

Add PolicySeeder#1178
tarrow wants to merge 9 commits into
mainfrom
addPolicySeed

Conversation

@tarrow

@tarrow tarrow commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Creates a new Seeder for easily creating Policy locally. This is not automatically run with php artisan db:seed since I intend to follow up with separating out Seeders into those we may wish to run only in test or dev and those we may wish to run in Prod

Bug: T430090

outdooracorn and others added 9 commits June 24, 2026 21:32
We are using the built in Eqloquent timestamp method to introduce nullable timestamp
columns.

The framework seems to have discussed these issues at length[1] and determined that
this is the best setup to prevent issues with ON UPDATE CURRENT_TIMESTAMP behaviour.

[1] laravel/framework#12490
For nice to have's like Factory and Seeder we should do in a
follow-up.

Still need to come to a decision on casting the timestamps;
particularly the built in.
Run php artisan ide-helper:models --reset "App\PolicyAcceptance" "App\Policy"
Creates a new Seeder for easily creating Policy locally.
This is not automatically run with `php artisan db:seed`
since I intend to follow up with separating out Seeders
into those we may wish to run only in test or dev and those we
may wish to run in Prod

Bug: T430090
dati18
dati18 previously approved these changes Jun 25, 2026

@outdooracorn outdooracorn left a comment

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.

It might be good for the PR description / commit message to include:

  • how you can run this seeder (php artisan db:seed --class=PolicySeeder), as well as why it won't be run with php artisan db:seed.
  • that you might need to run composer dump-autoload in order for the db:seed command to know that the PolicySeeder class exists.

'active_from' => CarbonImmutable::createFromDate(2026, 06, 01),
'content_vue_file' => 'terms-of-use/example.vue',
]
);

@outdooracorn outdooracorn Jun 29, 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.

I believe this will create a new policy, or fail if the policy already exists. If multiple seeders are run in the future, I think a failed Seeder will prevent subsequent seeders from running, which is likely not what we want.

From my quick searching, it's often recommended to create seeders in an idempotent way (i.e. running the seeder multiple times will result in the same database state).

https://oneuptime.com/blog/post/2026-02-03-laravel-database-seeding/view recommends using firstOrCreate() or updateOrCreate() methods on the model to make seeders idempotent.

Alternatively/additionally, you could use the Policy::truncate(); method to clear the table and reset the index (I haven't thought much about how best to handle existing constraints, though).

Policy::create(
[
'policy_type' => 'terms-of-use',
'active_from' => CarbonImmutable::createFromDate(2026, 06, 01),

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.

Creating a CarbonImmutable instance here works, but makes it trickier to use firstOrCreate() or updateOrCreate() methods (see other comment as to why you would want to use those methods). This is because (emphasis mine):

Create a Carbon instance from just a date. The time portion is set to now.

And explained using tinker:

$ dc exec -it api php artisan tinker
> use Carbon\CarbonImmutable;

> $a = CarbonImmutable::createFromDate(2026, 06, 01);
= Carbon\CarbonImmutable @1780325575 {#8373
    date: 2026-06-01 14:52:55.291326 UTC (+00:00),
  }

> $b = CarbonImmutable::createFromDate(2026, 06, 01);
= Carbon\CarbonImmutable @1780325594 {#8399
    date: 2026-06-01 14:53:14.547747 UTC (+00:00),
  }

> $a === $b
= false

> $a == $b
= false

> $a->equalTo($b)
= false

To address this you could either:

  • use a date string (e.g. 'active_from' => '2026-06-01',)
  • use the CarbonImmutable::createMidnightDate() method
> $a = CarbonImmutable::createMidnightDate(2026, 06, 01);
= Carbon\CarbonImmutable @1780272000 {#8474
    date: 2026-06-01 00:00:00.0 UTC (+00:00),
  }

> $b = CarbonImmutable::createMidnightDate(2026, 06, 01);
= Carbon\CarbonImmutable @1780272000 {#8451
    date: 2026-06-01 00:00:00.0 UTC (+00:00),
  }

> $a === $b
= false

> $a == $b
= true

> $a->equalTo($b)
= true

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.

I did some more testing of creating CarbonImmutable objects while writing a test for another PR and thought I would paste the findings here as well.

echo CarbonImmutable::create(2026, 06, 01);              // 2026-06-01 00:00:00
echo CarbonImmutable::createFromDate(2026, 06, 01);      // 2026-06-01 16:01:49
echo CarbonImmutable::createMidnightDate(2026, 06, 01);  // 2026-06-01 00:00:00
echo CarbonImmutable::create(2026, 06, 01, 12);          // 2026-06-01 12:00:00

Comment on lines +10 to +18
public function run() {
Policy::create(
[
'policy_type' => 'terms-of-use',
'active_from' => CarbonImmutable::createFromDate(2026, 06, 01),
'content_vue_file' => 'terms-of-use/example.vue',
]
);
}

@outdooracorn outdooracorn Jun 29, 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.

For testing purposes, would it be useful to have more than one current policy? Maybe also a superseded policy? I'm also happy if we add these at a later stage, though.

Base automatically changed from create-generic-policy-schemas to main July 1, 2026 11:53
@dati18 dati18 dismissed their stale review July 1, 2026 11:53

The base branch was changed.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants