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

fix(validation): updated upgradeDowngradeHistory Grades #684

Merged
merged 7 commits into from
Sep 27, 2023

Conversation

z3nful
Copy link
Contributor

@z3nful z3nful commented Sep 26, 2023

Changes

  • Added missing possible entries
  • in very rare cases, currentPrice is not returned in FinancialData, switched it to optional

Type

  • New Module
  • Other New Feature
  • Validation Fix
  • Other Bugfix
  • Docs
  • Chore/other

Comments/notes

@codecov
Copy link

codecov bot commented Sep 26, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (edaec4b) 93.05% compared to head (4a51889) 93.05%.
Report is 1 commits behind head on devel.

Additional details and impacted files
@@           Coverage Diff           @@
##            devel     #684   +/-   ##
=======================================
  Coverage   93.05%   93.05%           
=======================================
  Files          25       25           
  Lines         662      662           
  Branches      225      225           
=======================================
  Hits          616      616           
  Misses         46       46           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gadicc
Copy link
Owner

gadicc commented Sep 27, 2023

Awesome! Thanks, @z3nful, for another one of these 🙏

Noted the "Not a typo, how it was returned from API" 😂 Yahoo 🤷‍♂️

I'm assuming the BUy and HOld is the same story, I would just duplicate the same comment again to make this clear... and so that no one accidentally "fixes" this in the future on our side.

Otherwise looks great and thanks as usual!

@z3nful
Copy link
Contributor Author

z3nful commented Sep 27, 2023

Np : )

I just added the note to the other 2 as well. It seems like 'SEll' could logically also be returned at some point, but I checked that module against 10k different stocks and its possible that these typos are just years old and have had some type of validation built in since then on Yahoos side.

@gadicc gadicc merged commit fe6de09 into gadicc:devel Sep 27, 2023
2 checks passed
@gadicc
Copy link
Owner

gadicc commented Sep 27, 2023

Awesome, thanks so much! 🙏

Yeah a big part of the validation is basically crowdsourcing community knowledge around weird API responses and this is certainly one of them! And the typescript integration makes this so great... the compiler will now warn people if they accidentally correctly spell these things and help them spell it in a way will keep their code working as expected!

gadicc pushed a commit that referenced this pull request Sep 27, 2023
## [2.7.1](v2.7.0...v2.7.1) (2023-09-27)

### Bug Fixes

* **deps:** update dependency node-fetch to v2.7.0 ([26d53e3](26d53e3))
* **validation:** updated upgradeDowngradeHistory Grades ([#684](#684)) ([fe6de09](fe6de09))
@gadicc
Copy link
Owner

gadicc commented Sep 27, 2023

🎉 This PR is included in version 2.7.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

2 participants