search

dotkom / monoweb

Future version of Online, NTNU's informatics student association's website
https://web.online.ntnu.no/
MIT License
19 stars 2 forks source link

wipe database as first step when running fixtures script #832

Closed henrikskog closed 8 months ago

henrikskog commented 8 months ago

When running migrate-with-fixtures you want to run this on a clean slate database, or else you will get conflicts with existing data. This adds an option to the migrator to wipe the database as the first thing that happens, and changes the migrate-with-fixtures script to use it.

henrikskog commented 8 months ago

Yeah I see your point. I guess I am a bit biased in how I have only used this command in combination with deleting my database, but I strongly agree with how deleting the database should never come as a surprise when running a script. What do you think about it now?

Now I add a new script instead:

    "migrate-with-fixtures": "dotenv -e .env -- turbo run migrate -- latest --with-fixtures && cd packages/db && pnpm generate-types",
    "migrate-with-fixtures-with-db-wipe": "dotenv -e .env -- turbo run migrate -- latest --with-fixtures --with-db-wipe && cd packages/db && pnpm generate-types",
junlarsen commented 8 months ago

Can't we just do onConflict((eb) => eb.doNotihng())) or whatever it is?

Alternatively, update the conflicting fields to take the new value?

henrikskog commented 8 months ago

Aaaa that's what that was for! Yeah we could do that. I removed all of that in https://github.com/dotkom/monoweb/pull/817 oops 😅

However, as this just adds a new script I don't think there is any harm in merging this?

junlarsen commented 8 months ago

Aaaa that's what that was for! Yeah we could do that. I removed all of that in #817 oops 😅

However, as this just adds a new script I don't think there is any harm in merging this?

I think I agree with @henrikhorluck I don't think running any fixtures should ever drop the entire database, even if it's just adding a new command.

If this is a problem you often come across, I think it's easier for you to just drop the database yourself with psql or your favorite database viewer

junlarsen commented 8 months ago

I'm happy to merge in the onConflict changes though!

henrikskog commented 8 months ago

Fair enough guys :( The problem is really actually solved by just adding the 

    .onConflict((conflict) => conflict.doNothing())

I'll create a new PR and close this