fix: add missing fields to 'FindOrCreateType' #12338
Conversation
sonirico
changed the title
Codecov Report
@@ Coverage Diff @@
## master #12338 +/- ##
=======================================
Coverage 96.40% 96.40%
=======================================
Files 95 95
Lines 9116 9116
=======================================
Hits 8788 8788
Misses 328 328 Continue to review full report at Codecov.
|
|
Please add a test here https://github.com/sequelize/sequelize/blob/master/types/test/model.ts |
sushantdhiman
added
the
status: awaiting response
|
@sushantdhiman I've added a handful of tests 👍 |
types/test/model.ts
Outdated
| } | ||
| }) | ||
|
|
||
| if (! created) { |
There was a problem hiding this comment.
These tests are supposed to only test syntax, not actually doing any db operations. I don't think these conditions or checks should be included in this code.
Just add some calls to <Model>.findOrCreate to assert correct typings or move these to e2e if you prefer that
There was a problem hiding this comment.
Yes these tests are not actually executed, they are merely type checked.
There was a problem hiding this comment.
Ok, I have added only the typings because I have seen enough tests for findOrCreate in the integrations folder. Thank you for your guidance!
sushantdhiman
removed
the
status: awaiting response
Pull Request check-list
Please make sure to review and check all of these items:
npm run testornpm run test-DIALECTpass with this change (including linting)?Description of change
This PR includes some typings needed to provide clarification in regards to the usage of
findOrCreatefunctionality, thus theFindOrCreateOptionsinterface has been altered to include thefieldsstring array to whitelist which fields should be persisted as explained in #12325 . Tests are not passing, as well as in master upstream.