Disable Self Assigns #13
Conversation
|
Code looks good. Can you prove it works as expected with a QA test? Also I would've expected some unit tests although I'm not sure if we have been enforcing them for plugins. @gentlementlegen rfc |
|
I would like to see associated tests and a QA run somewhere. Also, maybe this should be configurable through the plugin configuration if we want users to be able to assign themselves again or not. |
| issue_number: issueNumber.id, | ||
| }); | ||
|
|
||
| return issueEvents.data.some((event) => event.event === "unassigned" && event.event === senderLogin); |
There was a problem hiding this comment.
I'm sceptical that event.event can be both unassigned and senderLogin, QA would be ideal
There was a problem hiding this comment.
Just noticed that this fails on the test CI. I suppose I should be more cautious of reviews from mobile as it is not easy to see these details sometimes.
| export async function wasPreviouslyUnassigned( | ||
| context: Context, | ||
| senderLogin: Context["payload"]["sender"], | ||
| issueNumber: Context["payload"]["issue"] |
There was a problem hiding this comment.
naming convention for these args is off slightly so it's being used in error in the return statement below I think
|
@gentlementlegen & @Keyrxng this isn't ready for reviews yet. I sent it in so I can save my progress that's why I left it on draft. @0x4007 you should probably revert your approval 😅😅 |
|
Normally I wouldn't have reviewed but it was because there was an approval on it I checked it too, I assumed you were waiting till you get the other one through first |
| issue_number: issueNumber.id, | ||
| }); | ||
|
|
||
| return issueEvents.data.some((event) => event.event === "unassigned" && event.event === senderLogin); |
There was a problem hiding this comment.
Just noticed that this fails on the test CI. I suppose I should be more cautious of reviews from mobile as it is not easy to see these details sometimes.
Resolves #2