console: down migrations improvements (close #3503, #4988)

[soorajshankar]

Description

Down migration of a renamed column fails because of the order of down migration was not reversed.

Changelog

• CHANGELOG.md is updated with user-facing content relevant to this PR.

Affected components

• Console

Related Issues

close #3503, #4988

Solution and Design

schemaChangesDown and schemaChangesDown were getting stacked on the same order. I have reversed schemaChangesDown just before calling makeMigrationCall.
Now even if the column is renamed, the down migration works correctly.

This PR will also, add SQL comments in down schema when there is no down schema changes

-- Could not auto-generate a down migration.
-- Please write an appropriate down migration for the SQL below:
--   DROP TABLE orders;

• Refactored some of the code to give better code readability

Steps to test and verify

1. Go to the Hasura Data module.
2. Do multiple edits on a column (eg. try renaming it, changing the comment, and toggle nullable)
3. Click on save button.
4. Validate the order of the up and down migrations from the network tab.

[hasura-bot]

Review app for commit 1f316be deployed to Heroku: https://hge-ci-pull-4790.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4790-1f316bed

[soorajshankar]

Previous behavior.

1. Here the last up-migration command is setting column name to ttt
2. but in down migration, the first command is trying to SET NOT NULL on the table with name test. (at that time there is no table with name test)

** note the image shows up query at the bottom :)

[rikinsk]

@soorajshankar This looks good.

Can we use this PR to fix all such issues of incorrect down migrations statement ordering. There definitely could be more which havent been reported.

Also can we standardize the pattern of ensuring the down migrations statements order across all usages. I like this style of adding up and down migration statements in the same order and then reversing the order of the down migration statements at the end cc: @beerose

[hasura-bot]

Review app for commit b87b8a7 deployed to Heroku: https://hge-ci-pull-4790.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4790-b87b8a7b

[rikinsk]

@soorajshankar A gentle reminder on the previous comment. it would be nice if this could be made consistent across the codebase.

I also noticed an issue while squashing migrations that if a migration does not have an equivalent down migration, that information gets lost once the migrations are squashed. So can we ensure that we add an sql comment -- no down migration created for <migration name> as a down migration in case there are no down migrations passed in any case. (This happens in cases such as a drop table as it is very hard to create the equivalent create table statement required in the down migration due to the many dependent objects a table can have)

[beerose]

Also can we standardize the pattern of ensuring the down migrations statements order across all usages. I like this style of adding up and down migration statements in the same order and then reversing the order of the down migration statements at the end

I think that all migration-related code can be extracted to its own module and maybe even unit tested. This way it'll be easier to enforce some behaviors and consistency. Also, some repeated parts like calling getRunSqlQuery could be hidden inside of this module. For example, we could have smaller functions that are returning { upMigration, downMigration } objects.

@soorajshankar We can discuss the details and brainstorm some ideas on how to approach this.

[hasura-bot]

Review app for commit da21269 deployed to Heroku: https://hge-ci-pull-4790.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4790-da21269d

[hasura-bot]

Review app for commit d6b0f0d deployed to Heroku: https://hge-ci-pull-4790.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4790-d6b0f0db

[hasura-bot]

Review app for commit 7c53a78 deployed to Heroku: https://hge-ci-pull-4790.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4790-7c53a782

[hasura-bot]

Review app for commit 680afdc deployed to Heroku: https://hge-ci-pull-4790.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4790-680afdc2

Erroneous approval

[kodiakhq]

This PR currently has a merge conflict. Please resolve this and then re-add the auto-update-auto-merge label.

[rikinsk]

Also seems like while creating a migration on Raw SQL page, a down.sql with a comment does not get created

@soorajshankar I still dont see a down migration with a comment created when running sql from raw sql page.

[rikinsk]

down migration with comment not created for sql run in raw sql page

[hasura-bot]

Review app for commit 3190c58 deployed to Heroku: https://hge-ci-pull-4790.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4790-3190c586