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.

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

flattenObject function throws uncaught error when it recursively flat… #10293

Merged

Conversation

@skellertor
Copy link
Contributor

@skellertor skellertor commented Dec 24, 2018

…tens objects and a value is null. Added a check to make sure the value is not null before recursively calling the flattenObject function again.

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 follow the commit message conventions explained in CONTRIBUTING.md?

Description of change

Added an additional check for null when flattening objects in the util.js file. Since null is a typeof object, the function would call itself and try to use the Object.keys function on null, which would then throw an error that was covering a validation error.

…tens objects and a value is null. Added a check to make sure the value is not null before recursively calling the flattenObject function again.
@SimonSchick
Copy link
Member

@SimonSchick SimonSchick commented Dec 24, 2018

Can you add a test for this?

@codecov
Copy link

@codecov codecov bot commented Dec 26, 2018

Codecov Report

Merging #10293 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10293      +/-   ##
==========================================
+ Coverage   96.27%   96.28%   +0.01%     
==========================================
  Files          68       68              
  Lines        9772     9772              
==========================================
+ Hits         9408     9409       +1     
+ Misses        364      363       -1
Impacted Files Coverage Δ
lib/utils.js 98.62% <100%> (+0.34%) ⬆️

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 9c0f041...e61d08e. Read the comment docs.

@skellertor
Copy link
Contributor Author

@skellertor skellertor commented Dec 26, 2018

@SimonSchick Tests written

@sushantdhiman
Copy link
Contributor

@sushantdhiman sushantdhiman commented Jan 2, 2019

Thanks @skellertor

@sushantdhiman sushantdhiman merged commit ecb0868 into sequelize:master Jan 2, 2019
4 checks passed
4 checks passed
codecov/patch 100% of diff hit (target 96.27%)
Details
codecov/project 96.28% (+0.01%) compared to 9c0f041
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants