Probesys / bileto

The ergonomic ticketing tool for managing your Help Desk.
https://bileto.fr
GNU Affero General Public License v3.0
44 stars 2 forks source link

Improve the process to generate migrations #230

Open marien-probesys opened 1 year ago

marien-probesys commented 1 year ago

Today, generating migrations is tedious: https://github.com/Probesys/bileto/blob/main/docs/developers/entity.md#migrations

Things could be improved in incremental steps:

  1. test on the CI that new migrations are not to generate (problem: there is always a new PGSQL migration to generate, see https://github.com/doctrine/migrations/issues/1196)
  2. create a volume with docker-compose so we don't lose data when generating the migrations for MariaDB
  3. improve the switch between databases (there are few ideas to take from wallabag, see https://doc.wallabag.org/en/developer/docker.html#switch-dbms and https://github.com/wallabag/wallabag/blob/master/docker-compose.yml)
  4. create the migrations in one command: I didn't found a standard way to do it with Doctrine and I'm very surprised there are none (I mean, one of the main advantage of an ORM is to query a database without worrying its type. Also, other ORMs like the one of Ruby on Rails is able to handle it, so why not Doctrine?)
marien-probesys commented 10 months ago

I spent some time on the first point today, but I'm stuck at several levels and I don't want to spend more time on the subject.

First, it's quite easy to bypass the initial issue which is that CREATE SCHEMA public; is added to all the down() methods of the generated migrations. See this patch.

Unfortunately, Foundry doesn't work well with this solution and fails because the public schema already exists; meaning that all the tests fail :shrug:

Also, I tried to rewrite the tests about the migrations. I was able to test that no new migration was to be generated, but I realized that testing the rollbacks wasn't working as expected. Indeed, the current migrations version is already set to 0 (i.e. no migration applied). By reorganizing the test and making sure to apply the migrations before the rollbacks, it doesn't work better because of how Doctrine works. Indeed, in the Doctrine/Migrations/Version/DbalExecutor class, Doctrine retrieves the SQL queries to run for each migration. Unfortunately, these queries are never reset. So the queries from the initial doctrine:migrations:migrate command are still present in memory. It means that when running the rollback command, Doctrine will first try to execute the up() queries of the last migration. It fails because the column/table/whatever already exists in the database. A solution would be to execute this test outside of PHP.

marien-probesys commented 10 months ago

Mounting volumes is not required as we rarely remove the containers. At least it's not a problem in this case (changing the database doesn't delete the data in the containers). I also removed the mention of reseting the database when generating a migration for MariaDB: it should not be needed.

marien-probesys commented 10 months ago

Since https://github.com/Probesys/bileto/pull/392, it's easier to switch the database thanks to the --profile option of Docker Compose. The last improvement would be to determine automatically the correct DATABASE_URL configuration, but I'm not sure how to do that easily.