fix(mssql): returning data for bulkUpdate

[ShaharHD]

Pull Request check-list

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

• Does npm run test or npm run test-DIALECT pass with this change (including linting)?
• Does the description below contain a link to an existing issue (Closes #[issue]) or a description of the issue you are solving?
• Have you added new tests to prevent regressions?
• Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
• Did you update the typescript typings accordingly (if applicable)?
• Did you follow the commit message conventions explained in CONTRIBUTING.md?

Description of change

Fix for the MSSQL dialect to return changed records when using bulkUpdate with returning: true

• bulkUpdate will now return the updates if options.returning is true.
• Fix documentation for model.update

This is in relation to #12410 as requested

[codecov]

Codecov Report

Merging #12413 into master will decrease coverage by 0.03%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##           master   #12413      +/-   ##
==========================================
- Coverage   96.44%   96.41%   -0.04%     
==========================================
  Files          95       95              
  Lines        9114     9116       +2     
==========================================
- Hits         8790     8789       -1     
- Misses        324      327       +3     

Impacted Files	Coverage Δ
lib/dialects/mssql/query.js	96.15% <50.00%> (-0.52%)	⬇️
lib/dialects/postgres/connection-manager.js	94.40% <0.00%> (-1.40%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 77b5fe6...ac476a7. Read the comment docs.

[sushantdhiman]

No tests for this change? Is this already covered by previous tests

[ShaharHD]

No tests for this change? Is this already covered by previous tests

You do have tests for returning: true I can modify those tests (in the mssql) to verify the updated data is returned.

I can see something similar on the postgres tests - but the postgres tests seems way more complex than what you have to MSSQL.
I can add a simple test which shows with returning: true to replicate the one test from postgres on bulkUpdate

[sushantdhiman]

Here is the relevant test file

sequelize/test/integration/model/update.test.js

Line 120 in 77b5fe6

if (_.get(current.dialect.supports, 'returnValues.returning')) {

You need to modify if check to this

if (current.dialect.supports.returnValues) {

[ShaharHD]

You need to modify if check to this

if (current.dialect.supports.returnValues) {

@sunshinewyin I've kept the same syntax as the old test, so I used:

if (_.get(current.dialect.supports, 'returnValues.output')) {

Look for should output the updated record in the tests.
Note the firstAcc.reload() as MSSQL only returns updated fields, so I've added this to still complete the test as the postgres one does.

It seems the tests now fails on something unrelated to my fix (malformed URL of some sort)

[sushantdhiman]

🎉 This PR is included in version 6.1.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀