Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: migrate constraints to typescript #15962

Merged
merged 30 commits into from
Jun 22, 2023
Merged

feat: migrate constraints to typescript #15962

merged 30 commits into from
Jun 22, 2023

Conversation

lohart13
Copy link
Contributor

@lohart13 lohart13 commented Apr 25, 2023 •
edited

Pull Request Checklist

  • Have you added new tests to prevent regressions?
  • If a documentation update is necessary, have you opened a PR to the documentation repository?
  • Did you update the typescript typings accordingly (if applicable)?
  • Does the description below contain a link to an existing issue (Closes #[issue]) or a description of the issue you are solving?
  • Does the name of your PR follow our conventions?

Description Of Change

From some testing, it turned out that the typing's for the constraint functions were either missing, invalid or missing valid option and rather than creating one PR per fix, I have bundled all the changes into this PR.

This closes #15407 as it implements the same fixes plus a few tweaks.

I've also moved MariaDB to its own QueryInterface as it supports removeConstraint however this is not supported in all versions of MySQL.
Also, MariaDB supports check constraints however this is also not supported in all versions of MySQL.

Closes #15964, #16115

@ephys ephys self-requested a review June 18, 2023 11:01
ephys
ephys requested changes Jun 18, 2023
View reviewed changes
Copy link
Member

@ephys ephys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing work! Once again thank you so much :) I only glanced at the tests but I'll re-review them once these few notes are handled

packages/core/src/deferrable.ts Outdated Show resolved Hide resolved
packages/core/src/dialects/abstract/query-interface-typescript.ts Outdated Show resolved Hide resolved
packages/core/src/dialects/abstract/query-generator-typescript.ts Outdated Show resolved Hide resolved
packages/core/src/dialects/abstract/query-interface-typescript.ts Outdated Show resolved Hide resolved
packages/core/src/dialects/abstract/query-interface-typescript.ts Outdated Show resolved Hide resolved
packages/core/src/dialects/mssql/query-generator-typescript.ts Show resolved Hide resolved
packages/core/src/dialects/mysql/query-generator-typescript.ts Show resolved Hide resolved
packages/core/src/dialects/sqlite/query-generator-typescript.ts Outdated Show resolved Hide resolved
packages/core/src/dialects/sqlite/query-interface.js Outdated Show resolved Hide resolved
packages/core/src/transaction.ts Outdated Show resolved Hide resolved
@ephys ephys mentioned this pull request Jun 18, 2023
Merged
5 tasks
@lohart13 lohart13 force-pushed the feat-migrate-constraints branch 2 times, most recently from bc267b9 to 3292c79 Compare June 19, 2023 04:46
@lohart13
Copy link
Contributor Author

Thanks @ephys.
I've gone through and made some changes and left any queries in the unresolved changes.

@WikiRik WikiRik mentioned this pull request Jun 19, 2023
Closed
8 tasks
ephys
ephys requested changes Jun 21, 2023
View reviewed changes
packages/core/test/integration/query-interface/changeColumn.test.js Outdated Show resolved Hide resolved
packages/core/test/integration/query-interface/remove-constraint.test.ts Show resolved Hide resolved
packages/core/test/integration/query-interface/show-constraint.test.ts Outdated Show resolved Hide resolved
packages/core/test/integration/sequelize/truncate.test.ts Outdated Show resolved Hide resolved
packages/core/test/unit/query-generator/add-constraint-query.test.ts Outdated Show resolved Hide resolved
packages/core/test/unit/query-generator/add-constraint-query.test.ts Outdated Show resolved Hide resolved
packages/core/test/unit/query-generator/add-constraint-query.test.ts Outdated Show resolved Hide resolved
packages/core/test/unit/query-generator/add-constraint-query.test.ts Outdated Show resolved Hide resolved
packages/core/test/unit/query-generator/set-constraint-checking-query.test.ts Outdated Show resolved Hide resolved
@lohart13
fix: migrate showConstraint to TypeScript
35efb81
@lohart13
feat: migrate deferConstraints to typescript
98fc6ab
@lohart13
feat: migrate getConstraintSnippet to typescript
35c8205
@lohart13
feat: migrate addConstraint to typescript
eb892a2
@lohart13
feat: migrate removeConstraint to typescript
c15672e
@lohart13
fix(db2): onUpdate is not supported
7987a07
@lohart13
fix(sqlite): conditional add referential actions
5119532
@lohart13
meta: onUpdate not supported by all dialects
6ed99ec
@lohart13
fix(types): updated and exported types
1e1ab48
@lohart13
fix: revert change to AbstractQueryGeneratorTypeScript
d5c8cde
@lohart13
fix: add return type & set query type for constraints
6b0ef33
@lohart13
fix: throw error for field.attribute option
0b6e752
@lohart13
feat: add rootTransaction getter
93bde0b
@lohart13
fix: deferrable is always required for deferConstraints
5dd36f2
@lohart13
feat: merge setDeferred & setImmediate into setConstraintCheckingQuery
01aa84e
@lohart13
feat: rename showConstraint to showConstraints
c74bfaf
@lohart13
fix: updated variables
c9d0c20
@WikiRik WikiRik added this pull request to the merge queue Jun 21, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 21, 2023
@lohart13
Copy link
Contributor Author

lohart13 commented Jun 21, 2023 •
edited

@WikiRik what can be done to fix the flaky test that failed in the merge queue.
I've noted that mariadb latest on both node 16 and 20 have failed at times in the CI due to the same flaky test.

@WikiRik
Copy link
Member

WikiRik commented Jun 21, 2023

@WikiRik what can be done to fix the flaky test that failed in the merge queue.
I've noted that mariadb latest on both node 16 and 18 have failed at times in the CI due to the same flaky test.

Yup, since the upgrade to MariaDB 11.0 this test is very flaky. There's not much to do apart from moving this PR back in the merge queue

@WikiRik WikiRik added this pull request to the merge queue Jun 21, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 21, 2023
@WikiRik WikiRik added this pull request to the merge queue Jun 22, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 22, 2023
@WikiRik WikiRik merged commit 0e17c07 into sequelize:main Jun 22, 2023
48 checks passed
@lohart13 lohart13 deleted the feat-migrate-constraints branch June 22, 2023 06:29
@lohart13 lohart13 mentioned this pull request Jun 22, 2023
Closed
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ephys ephys ephys approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@lohart13 @WikiRik @ephys