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: Loading teams for team option of author filter/validator #713

Merged
merged 1 commit into from
May 12, 2023

Conversation

robinmayerhofer
Copy link
Contributor

Fix loading teams for team option of author filter/validator.

Context

My previous PR #710 added an author validator. However, the API classes (mocked in the tests) access the context, and the context in lib/validators/options_processor/options/team.js is not the one I expected (it's a special validator context, different from the context in lib/filters/options_processor/options/team.js (correct one).

API calls are mocked for unit tests, but they log information taken from the context, and this doesn't work with the validator context.

Changes

I introduced a new function in lib/validators/options_processor/teams.js and moved most of the logic there (shared for filter/validator), and removed the options_processor/options/team.js classes because we don't always have the correct context there.
This method is called prior to processing the other options (and potentially results in an early return if unsuccessful), and then the team key is simply removed from the options.

This follows the pattern for similar cases, for example, see lib/validators/changeset.js#L58

@codecov
Copy link

codecov bot commented May 12, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (d9fb002) 92.90% compared to head (62266c2) 92.91%.

❗ Current head 62266c2 differs from pull request most recent head c4de8d1. Consider uploading reports for the commit c4de8d1 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #713   +/-   ##
=======================================
  Coverage   92.90%   92.91%           
=======================================
  Files         112      110    -2     
  Lines        2877     2878    +1     
  Branches      576      578    +2     
=======================================
+ Hits         2673     2674    +1     
  Misses        185      185           
  Partials       19       19           
Impacted Files Coverage Δ
lib/filters/author.js 100.00% <100.00%> (ø)
lib/validators/author.js 100.00% <100.00%> (ø)
lib/validators/options_processor/teams.js 65.30% <100.00%> (+12.52%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@robinmayerhofer
Copy link
Contributor Author

@shine2lay could you also check out this PR? Thx!

@shine2lay shine2lay merged commit 590a40f into mergeability:master May 12, 2023
@github-actions
Copy link

🎉 This PR is included in version 2.17.5 🎉

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