This means that you must first make sure the current migrations have been applied in one way or another.
IMHO, it would be useful to clarify why the other old migrations did not create any issue and why not all of them show in the migrations table after a success, say after we renamed the two migrations with a suffix .bck and successfully run the others using bin/console mautic:update:apply --finish.
I would feel for confident about the proposed change, if I understood this.
I looked further at the code to understand why after having applied the old migrations they are not added in the table migrations. I considered the specific case of Version20190326190241. The preUp method of the migration checks in the database schema if it was already applied and if it was, it throws a SkipMigration exception, which has for consequence that the mysql statement is not committed and, in particular, the table migrations is not modified. A similar preUp method exists for the other migrations. Given that explanation, I see no problem with the proposed change. The code assumes the migrations are already executed and it logically does not alter the table migrations. On our side, we know the table is not complete, so we make it complete it. It’s fine.
Yet, there might be room for further improvements. The code seems robust with another level of checks to see whether the migration was executed. If it was, it is not executed again. The question is then why this other level of checks is not working properly with the two migrations that create the issue. The migrations table should have the already executed migrations, but the code should not bug because some of them are missing.
The preUp method of the four years old migration Version20211020114811.php detects that it was not yet applied ! Futhermore, when the migration is applied, it throws an exception. The same thing must be the case with Version20230621074925.php. This is not really a bug, because the table migrations should contain these migrations. The problem must be that subsequent migrations have modified the schema in such a way that this very old migration cannot recognize anymore that it has been applied and when it is incorrectly applied again, it messes up things. Still, I will open an issue about this, because there might be a better way to manage that situation.
Hello dominic.mayers,
Thanks for the detailed analysis. I do appreciate your efforts.
My coding skills with Symfony and Doctrine is currently limited so I have to follow your guidance. However I have extensive knowledge of other programming languages and on SQL type relational databases in a commercial setting.
Database schema changes between app versions is very important. It also very important to know what the database schema should actually be as well so a DBA can manually fix the database if necessary. If this info was public I would have corrected the database myself and carried on using Mautic long ago.
Yes opening a case for the developers to look at this issue is a good idea as some installs don’t encounter this problem and some do which is surprising and it needs resolving by those that have more knowledge of Mautic than me.
If you are correct in that the two migrations that are causing this error are no longer necessary for Mautic then the developers should make that determination and remove them from the new releases published on github. Or if they are necessary the code should be fixed accordingly by the developers on subsequent releases.
I thank you for all your efforts.
Thanks.
This suggests a good idea. Migrations that have been applied in previous updates are kept in the committed folder app/migrations for other purposes than having to apply them again in the current update. That’s ok. Your point is that the new release should clearly know which migrations should be applied and ignore the ones that have been already applied, which is just common sense. The problem with the current code is that the new release does not have the information and relies on the table migrations in the database to determine which migrations to apply. The new release should not have to rely on the database for that purpose. For other purposes, perhaps the table migrations and the old migrations could be useful, but it should be much simpler for a release that needs to apply the current migrations.The release should assume that the old migrations have already been applied and completely ignore them.