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: Ensure allowlist is also checked in the context of issue #30

Merged

Conversation

jcfr
Copy link
Contributor

@jcfr jcfr commented May 9, 2024

While working on using the action in the context of issue comments, I realized specifying allowlist was not respected.

This pull request moves the call to isAllowed (in src/functions/prechecks.js) so that it is done earlier (just after validPermissions and before running the pull request specific logic)

The test has also been updated accordingly.

@jcfr jcfr requested a review from GrantBirki as a code owner May 9, 2024 17:50
@jcfr
Copy link
Contributor Author

jcfr commented May 9, 2024

cc: @GrantBirki @ronnnnn

@GrantBirki GrantBirki added the bug Something isn't working label May 9, 2024
@GrantBirki GrantBirki merged commit d975bd0 into github:main May 9, 2024
4 checks passed
@jcfr jcfr deleted the support-allowlist-for-both-issue-and-pr branch May 9, 2024 18:18
@jcfr
Copy link
Contributor Author

jcfr commented May 9, 2024

@GrantBirki Thanks for the quick turnaround 🚀

@GrantBirki
Copy link
Member

@jcfr You're very welcome! Thank you for this fix 🎉

Your changes have been pulled into the latest release of this Action at v1.2.0 and also by using the primary v1 tag. Enjoy!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants