Add trigger for user membership in a team #374
Conversation
gagoar
force-pushed
the
gg/feature-346
branch
from
9e3d30f
to
1353fbf
Compare
Merging into a non-master branch, so omitting most of the PR template. #374 has the relevant context. This PR fixes a bad import and fixes some logic in the isMemberOf matcher. Also adds 404s as an accepted response.
| @@ -65,7 +65,7 @@ describe('rules', () => { | |||
| }; | |||
| const rules = new Rules(rule); | |||
|
|
|||
| expect(await rules.getMatchingRules(files, event)).toMatchInlineSnapshot('MatchingRules []'); | |||
| expect(await rules.getMatchingRules(files, event, {} as OctokitClient)).toMatchInlineSnapshot('MatchingRules []'); | |||
There was a problem hiding this comment.
I should store the mocked object and then re-use it.
Codecov Report
@@ Coverage Diff @@
## master #374 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 8 9 +1
Lines 344 373 +29
Branches 54 59 +5
=========================================
+ Hits 344 373 +29
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| const results = await Promise.all( | ||
| membershipChecks.map((membership) => queue.add(() => client.teams.getMembershipForUserInOrg(membership))) | ||
| ).catch(catchHandler(debug)); | ||
|
|
||
| return (results as Record<string, unknown>[]).some((response: Record<string, unknown>) => { | ||
| return isGetMembershipForUserInOrg(response) && response.data.state == ACTIVE_STATE; | ||
| }); |
There was a problem hiding this comment.
I think this bit of code is problematic, but you'll have to verify for me.
If I don't belong in one of the teams which we check for membership, then catchHandler will have to handle a 404 not found. This needs to be added to:
use-herald-action/src/util/constants.ts
Lines 52 to 54 in cb11e2c
Furthermore, if any of the promises reject, I'm not sure that results is an array anymore, which is where I run into the error complaining that results.some is not a function. I think at that point, results is simply the value of the promise which rejected.
FYI even after I added 404 to the AllowedHttpErrors enum, I am still seeing the results.some is not a function error.
There was a problem hiding this comment.
mm, how is that not happening on the code?
Co-authored-by: Simon Liu <siliu@coursera.org>
|
Kudos, SonarCloud Quality Gate passed!
|
Issue Reference: fixes #346
Implements feature request #346
Motivation and Context
Useful to create custom rules like adding a label when a member is part of a team, among other things, to trigger a workflow.
it is a really useful filter when adding ppl to a review or commenting, for instance, checking if the person is on a team, might reduce adding a comment to that PR. etc.
How Has This Been Tested?
unit tests.
Checklist: