Fixed error when setting Oracle column to not null

[aidenfoxx]

Normalized migrations so Oracle lines up with other schemas, and also fixed down migrations which didn't re-apply NOT NULL constraint.

[aidenfoxx]

So after testing a fresh install I do get the error:

[Error] alter table "directus_webhooks" modify ("collections" NOT NULL) - ORA-01442: column to be modified to NOT NULL is already NOT NULL

I tried testing with SQLite to see if this was the case for other schemas, and was surprised to discover [Error] .dropNullable is not supported for SQLite.. I'm setting this PR to draft until I figure out what is going on with this migration.

[aidenfoxx]

Okay, so I see the issue. When I updated the migration 20210312A-webhooks-collections-text.ts that allowed Oracle to install, I retained the NOT NULL constraint. I didn't consider that Knex wouldn't retain the NOT NULL constraint when changing the field type: table.text('collections').alter();.

But to that point, the "down" migration in 20210312A-webhooks-collections-text.ts is also incorrect as it does not re-enable the NOT NULL constraint. The same applies to 20201105B-change-webhook-url-type.ts.

So I think the fix is to:

• Ignore Oracle for this migration, since it is already NOT NULL
• Update the down methods of 20210312A-webhooks-collections-text.ts and 20201105B-change-webhook-url-type.ts to set the columns back to NOT NULL
• Add a new migration to set directus_webhooks.url to NOT NULL

Does that sound alright @rijkvanzanten?

[rijkvanzanten]

I tried testing with SQLite to see if this was the case for other schemas, and was surprised to discover [Error] .dropNullable is not supported for SQLite.. I'm setting this PR to draft until I figure out what is going on with this migration.

Woooooooooooo @nickrum we found a gap in the SQLite implementation 😵 🔫

I didn't consider that Knex wouldn't retain the NOT NULL constraint when changing the field type: table.text('collections').alter();.
But to that point, the "down" migration in 20210312A-webhooks-collections-text.ts is also incorrect as it does not re-enable the NOT NULL constraint. The same applies to 20201105B-change-webhook-url-type.ts.

Ohhhhhh interesting

Update the down methods of 20210312A-webhooks-collections-text.ts and 20201105B-change-webhook-url-type.ts to set the columns back to NOT NULL

While it's technically not really kosher to retroactively change migrations, I think this is enough of an edge case in oracle specifically to do it. I'd go with this direction 👍🏻

[nickrum]

Woooooooooooo @nickrum we found a gap in the SQLite implementation 😵 🔫

Yeah, those functions have only been added a few weeks ago. Shouldn't be too hard to add SQLite support. I can take care of that if you want, @aidenfoxx.

[aidenfoxx]

@nickrum I mentioned to their dev that I'd take a look this weekend if I get the chance. But if you don't see a PR by Monday then you're free to jump on it (assuming you've not already started). 😄

It shouldn't be required for this fix though since Oracle is already NOT NULL.

[aidenfoxx]

• Updated the migrations 20210312A-webhooks-collections-text.ts and 20201105B-change-webhook-url-type.ts to re-apply the NOT NULL constraint on down
• Skip the 20210907A-webhooks-collections-not-null.ts migration for Oracle since it's already NOT NULL due to my incompetence 😝
• Added new migration to set directus_webhooks.url to NOT NULL as I believe it should be

Tested migrations up and down against MySQL, SQLite and Oracle.

[aidenfoxx]

Also, just a heads up @rijkvanzanten and @nickrum. I added setNullable and dropNullable support to SQLite, so we can start using those methods once that's released.

knex/knex#4684