fix: wrong interface used within mixin by mmahalwy · Pull Request #13685 · sequelize/sequelize

mmahalwy · 2021-11-19T20:35:25Z

Pull Request Checklist

Please make sure to review and check all of these items:

Have you added new tests to prevent regressions?
Does npm run test or npm run test-DIALECT pass with this change (including linting)?
Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
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?
Did you follow the commit message conventions explained in CONTRIBUTING.md?

Description Of Change

Fixes a typo from here: #13678

Todos

mmahalwy · 2021-11-19T20:35:47Z

@sdepold my bad

sdepold · 2021-11-19T20:46:26Z

@sdepold my bad

Seems the build is broken :)

mmahalwy · 2021-11-19T20:51:09Z

@sdepold the funny thing is, this is actually now doing the right thing. Typings failed because of missing create attributes :)

* feat(typescript): create alpha release with ts * ci: add other ts versions to ci (#13686) * fix: wrong interface used within mixin (#13685) * fix(increment): fix key value broken query (#12985) Co-authored-by: Sascha Depold <[email protected]> * fix(upsert): fall back to DO NOTHING if no update key values provided (#13594) * fix(types): add instance member declaration (#13684) * fix(types): add instance member declaration * test(types): add static/instance members test cases Co-authored-by: Sascha Depold <[email protected]> * Create aws-lambda.md (#12642) * fix(docs): add aws-lamda route (#13693) * fix(types): allow override json function with custom return type (#13694) * fix(types): allow override to json function with custom return type * fix(types): remove automatic linter changes Co-authored-by: sander-mol <[email protected]> * ci(docs): add doc generation to checks (#13704) * docs: add logo (#13700) * docs: add logo * fix(docs): logo not show up (#13699) * fix(build): markdownlint * docs(readme): use internal link * docs(index.md): use internal link * docs(index): update logo rendering in docs * Center logo and headline Co-authored-by: Sascha Depold <[email protected]> Co-authored-by: Sascha Depold <[email protected]> * test: fix mocha (#13707) Co-authored-by: Rik Smale <[email protected]> Co-authored-by: Sascha Depold <[email protected]> * test: fix failing stack trace test (#13708) * test: fix failing tests (#13709) * test: fix failing tests due to minification Removes esbuild minification which was causing tests to fail and some changed behavior. * Revert "fix(upsert): fall back to DO NOTHING if no update key values provided (#13594)" This reverts commit 4071378. * fix(upsert): fall back to DO NOTHING if no update key values provided (#13711) * fix(upsert): fall back to DO NOTHING if no update key values provided (#13594) * fix: remove erroneous .length in _.isEmpty * refactor: use includes instead of or operators (#13706) * docs(contributing): add section on adding/updating deps (#13715) * docs(contributing): add Node versions Fixes #13714 * docs(contributing): add section on adding/updating deps * fix(types): add Col to where Ops (#13717) * fix(types): add Col to where Ops * fix(types): tests * fix(data-types): unnecessary warning when getting data with DATE dataTypes (#13712) * fix(data-types): unnecessary error when getting data with DATE dataTypes * fix(data-types): date stringify mariadb * fix(types): add missing schema field to sequelize options Fixes #12606 Co-authored-by: Sascha Depold <[email protected]> * fix(example): fix coordinates format as per GeoJson (#13718) * fix(example): fix coordinates format as per GeoJson * Update data-types.js Co-authored-by: Sascha Depold <[email protected]> * ci(node): use Node 16 instead of 12 (#13703) * ci(node): add Node 14 and 16 to DB tests * ci(node): move linting, test typing and release to Node 16 * ci(docs): use Node 16 for docs * ci(node): run tests on Node 10 and 16 Co-authored-by: Rik Smale <[email protected]> * refactor(postgres): move `clientMinMessages` from general to pg options (#13720) * refactor(postgres): move `clientMinMessages` from general to pg options * refactor(postgres): address review comments * refactor(postgres): address review comments * refactor(postgres): fix pipeline * fix(dialect): try to fix flaky test * fix(dialect): try to fix flaky test Co-authored-by: Jesse Peng <[email protected]> * refactor(build): use rm instead of rmdir for Node 14 and up (#13702) * fix(build): refactor rmdir to rm * refactor(build): only use rm in Node 14 and up * refactor(build): move consts inside function * refactor(build): fix definition Co-authored-by: Rik Smale <[email protected]> Co-authored-by: Sascha Depold <[email protected]> * docs(postgres): warn about deprecated clientMinMessage option (#13727) * docs(postgres): warn about deprecated clientMinMessage option * docs(deprecation-warning): fix typo in deprecation warning * docs(deprecation-warning): fix typo in deprecation warning * meta(deps): upgrade (dev)deps (#13729) Co-authored-by: Rik Smale <[email protected]> * test(integration): mark and fix flaky test (#13735) * feat(dialect): snowflake dialect support (#13406) Co-authored-by: Mohamed El Mahallawy <[email protected]> Co-authored-by: bparan <[email protected]> Co-authored-by: Matthew Blasius <[email protected]> Co-authored-by: WeRDyin <[email protected]> Co-authored-by: Marco Gonzalez <[email protected]> Co-authored-by: Constantin Metz <[email protected]> Co-authored-by: Sander Mol <[email protected]> Co-authored-by: sander-mol <[email protected]> Co-authored-by: Rik Smale <[email protected]> Co-authored-by: Fauzan <[email protected]> Co-authored-by: Rik Smale <[email protected]> Co-authored-by: AllAwesome497 <[email protected]> Co-authored-by: manish kakoti <[email protected]> Co-authored-by: Marco Kerwitz <[email protected]> Co-authored-by: Jesse Peng <[email protected]> Co-authored-by: Jesse Peng <[email protected]>

github-actions · 2021-12-04T18:32:12Z

🎉 This PR is included in version 6.12.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

github-actions · 2021-12-17T19:27:01Z

🎉 This PR is included in version 6.12.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

EmilioCristalli · 2022-01-03T22:56:43Z


  const project = await newUser.createProject({
    name: "first!",
+    ownerId: 123,


we are investigating some issues in our app after upgrading to Sequelize 6.12.0 that might be related to this PR.

Since user-> project association is defined like this:

User.hasMany(Project, { sourceKey: "id", foreignKey: "ownerId", as: "projects" });

Shouldn't ownerId be automatically set when you do await newUser.createProject({ name: "first!" })?

Looking at the code in lib/associations/has-many.js, it seems like the ownerId will even be overwritten with newUser.get('id').

I will try to open a PR with an idea to fix it, but wanted to first confirm this is not expected behavior

@EmilioCristalli issue being that you need to specify the id otherwise it fails TS?

yes.
Since ownerId is going to be automatically set, i think it shouldn't be required by TS. Or even not allowed, since it will be overwritten

yeah, that's a good catch

i just opened this PR #13892 with an idea to fix it

fix: wrong interface used within mixin

58cee70

fix: test typings

d47798b

mmahalwy and others added 2 commits November 19, 2021 15:52

test: add optional field

0b5f1be

Merge branch 'main' into me--fix-name-for-mixins

76fcba0

sdepold merged commit bd3ddf5 into sequelize:main Nov 20, 2021

joe223 pushed a commit to joe223/sequelize that referenced this pull request Nov 22, 2021

fix: wrong interface used within mixin (sequelize#13685)

4587b0b

This was referenced Nov 22, 2021

fix(types): consistent create association values. Fixes #12463 #13640

Closed

Inconsistent typing of Create functions #12463

Closed

github-actions Bot added the released on @v6-beta label Dec 4, 2021

github-actions Bot added the released label Dec 17, 2021

EmilioCristalli reviewed Jan 3, 2022

View reviewed changes

aliatsis pushed a commit to creditiq/sequelize that referenced this pull request Jun 2, 2022

fix: wrong interface used within mixin (sequelize#13685)

34db558

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: wrong interface used within mixin#13685

fix: wrong interface used within mixin#13685
sdepold merged 4 commits intosequelize:mainfrom
mmahalwy:me--fix-name-for-mixins

mmahalwy commented Nov 19, 2021 •

edited

Loading

Uh oh!

mmahalwy commented Nov 19, 2021

Uh oh!

sdepold commented Nov 19, 2021

Uh oh!

mmahalwy commented Nov 19, 2021

Uh oh!

github-actions Bot commented Dec 4, 2021

Uh oh!

github-actions Bot commented Dec 17, 2021

Uh oh!

EmilioCristalli Jan 3, 2022

Uh oh!

mmahalwy Jan 3, 2022

Uh oh!

EmilioCristalli Jan 3, 2022

Uh oh!

mmahalwy Jan 3, 2022

Uh oh!

EmilioCristalli Jan 3, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

mmahalwy commented Nov 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Checklist

Description Of Change

Todos

Uh oh!

mmahalwy commented Nov 19, 2021

Uh oh!

sdepold commented Nov 19, 2021

Uh oh!

mmahalwy commented Nov 19, 2021

Uh oh!

github-actions Bot commented Dec 4, 2021

Uh oh!

github-actions Bot commented Dec 17, 2021

Uh oh!

EmilioCristalli Jan 3, 2022

Choose a reason for hiding this comment

Uh oh!

mmahalwy Jan 3, 2022

Choose a reason for hiding this comment

Uh oh!

EmilioCristalli Jan 3, 2022

Choose a reason for hiding this comment

Uh oh!

mmahalwy Jan 3, 2022

Choose a reason for hiding this comment

Uh oh!

EmilioCristalli Jan 3, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mmahalwy commented Nov 19, 2021 •

edited

Loading