Coming to your response. I can see that what you are saying and I partially agree with that. We have tried same apporach mentioned here for post schema change method. But even that also is not running when we are enabling the app. Due to that when we deploy our app on new instance we have to do app reset after enabling the app which is deleting all tables, remove migration records and re-run the migrations.
So, due to our app reset command we can’t do the schema change in changeSchema method. As we know in NC21 all change schema methods (changeSchema method of all migrations) are running first then postSchemaChange when we run migrate command then if we do a schema change lets say add a new column then it will fail because the table where we want to add the new column is not created yet. I hope you understand what I’m trying to say here.
To be very short, postSchemaChange method is not executing when we are enabling the app.
I used changeSchema method only to add new column.
There is still an issue with that. If I use columnDefinition and use AFTER to add new column after a existing table then there is an issue when I enable the app for 1st time then I’m getting following error:
Due to this issue we decided to use postSchemaMethod so that if we install the app in new instance we don’t face this issue.
If I enable the app and then add new migration to add new column using changeSchema then on running the app:update command we are getting
Nextcloud or one of the apps require upgrade - only a limited number of commands are available
You may use your browser or the occ upgrade command to do the upgrade
This is understanble. So, When I updated the app via UI then app is getting updated properly without any issues.
Now my only question is what should we do for fresh install? As there we can’t add the new column at a proper place in table. Which is not good from the point of view of a good database design.
you say the type is integer but then in the definition you say VARCHAR(50)
in columnDefinition you say NULL and in the option you define notnull = true
you try to specify a second autoincrement column which is not supported
Another (common) mistake is: Migrations should assume the previous state and only check against their current action:
You currently check if the table exists => Assume it does
You currently always add the column => Check if it exists
Also you should not care about the order of your columns as it is not supported with all databases nextcloud supports (that is exactly why this abstraction was added and you shouldn’t use columnDefinition directly unless you really know what you are doing)
With all this your second migration should look like this:
Also note that in your first table you have column defined as notnull with default set to an empty string. Oracle saves empty strings as null and therefor might choke on that column. I would therefor recommend to make it nullable:
Thank you for detailed explaination. That really helped with our issue. I will be giving a PR to udpate the documentation associated to that as there it’s not mentioned what you said in your very 1st reply.
But now I have doubt regarding postSchemaChange method execution. Please check the following scenario.
If XYZ enables the APP but later he/she disables the APP (Please note the APP here is private APP which will not be avaliable in nextcloud app store). So later when we update the APP version in which we have some migrations(using postSchemaChange method for transformation). Now XYZ replaces the old APP files with new APP files. Now if XYZ enables the APP it automatically updates the APP to latest version. Keeping this scenario in mind please proceed further.
When I looked into the app:enable command code specifically I can see that from there we are calling installetAppmethod which is defined at only one place.(Can check screenshot below)
Then I looked into installerApp method. There is a check for database.xml file if that exists use that otherwise run migrate method. As we are not using the database.xml file for our app it’s obvious that migrate method will be called.
Please note that here in migrate we are sending ‘lastest’ and true. In migrate app I can see that from there we are calling executeStep method which executing the migrations. There we are passing the values latest and true.
There we can clearly see that pre and post schema methods will only execute when the $schemaOnly value is false. But in case of app:enable it is always send as true.
For updating an app you usually do not disable and remove it/reenable it, but instead you just bump the version number in info.xml . Then the next page visit or occ upgrade can be used to run all the necessary migrations which did not yet get executed.
I just enabled the debugger and stepped through it and … thanks for pointing this out. There is indeed a bug that migrations are accidentally not ran fully if there is a previous version installed already. Patch is at: