Skip to content

fix(types): enhance having parameters #10733

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

Merged
merged 2 commits into from
Apr 9, 2019
Merged

fix(types): enhance having parameters #10733

merged 2 commits into from
Apr 9, 2019

Conversation

samalexander
Copy link
Contributor

@samalexander samalexander commented Apr 9, 2019

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

Description of change

At present, using the where function to build a having parameter for FindOptions produces a typescript error. As an abbreviated example, the following...

Model.findAll({
   having: where(fn('max', col('"Model".created_at')), {
     [Op.lt]: subYears(new Date(), 1)
   }),
});

would produce the error...
Type 'Where' is not assignable to type 'WhereAttributeHash'.

However, when casted to an any type, it produces a valid SQL query.

Therefore, I have updated the supported types for the having parameter to accept the Where type.

Fixes #10734.

@codecov
Copy link

codecov bot commented Apr 9, 2019

Codecov Report

Merging #10733 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #10733   +/-   ##
=======================================
  Coverage   96.31%   96.31%           
=======================================
  Files          93       93           
  Lines        8980     8980           
=======================================
  Hits         8649     8649           
  Misses        331      331

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 88228d7...7c5ef70. Read the comment docs.

Copy link
Contributor

@SimonSchick SimonSchick left a comment

Choose a reason for hiding this comment

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

Lol nice one with the documentation.

@sushantdhiman sushantdhiman merged commit f6dd943 into sequelize:master Apr 9, 2019
@sushantdhiman
Copy link
Contributor

🎉 This PR is included in version 5.2.14 🎉

The release is available on:

Your semantic-release bot 📦🚀

@samalexander samalexander deleted the patch-1 branch April 9, 2019 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Typescript issue with aggregation in having clause
3 participants