Webhook call returns unsupported event

[brbmi]

Hi again,
I am running into an issue when doing a pull request. I am not prompted to sign the CLA., I debugged this to the array webhooks in src/server/app.js. That array is empty, so when github calls cla-assistant with the pull request, it looks up the event in the array and finds nothing and fails the call with an Unsupported event error. I looked in app.js for where the array is loaded, pretty much how it is done with api, but I did not find it. After the call to bootstrap('webhooks', callback); the array is still empty. So am I missing something here? But the bottom line is that whenever I do a pull request I don't get the prompt to sign the CLA. Please tell me what I am doing wrong here, if the problem is on my end.

[mpierre0220]

Hi, I opened this issue with another ID. I needed another ID to test the pull request flow. This is still related to deploying cla-assistant on Cloud Foundry. I made a change to app.js that loads the webhooks array and this seems to take care of the "Unsupported event" error.
This is the fix I made in src/srver/app.js line 83
} else if (files === 'webhooks'){
webhooks[path.basename(f,'.js')] = require(f);
}
I don't know if this array is loaded elsewhere but while debugging it was empty on the call to
/github/webhooks/:repo. Please let me know if I am missing something here.

On the other hand I am still not comfortable with the flow. 1) There is no prompt to sign the CLA. I have to click on a link that says "checks passed" then to "details," that's when I finally have a screen that shows the CLA and asks to sign it. Is this supposed to be the flow? I don't see where the signing of the CLA is being enforced. These are the screens I get:

and the CLA signature page which only shows when you click on details

2. Once you sign a CLA for a project, are you supposed to be prompted to sign for each pull request or not? I had that impression when I read the docs. But I might read too much into this.

Appreciate your answer on this

[KharitonOff]

Hi, thank you very much for pointing to this problem. Your fix is absolutely correct.
CLA assistant comments only once on a new PR in case the contributor hasn't signed CLA yet. All other PRs of the user get only green status (without any comment from CLA assistant). This is also the case in your screenshot: the green checkmark shows that the user has already signed the CLA. Because of the bug you identified there was no comment on the initial PR, sorry for that.
Does it answer all your questions?

[mpierre0220]

Hi, my remaining question is whether the person doing the PR is expected to click on Check to go sign the CLA in the case he has not signed it yet. It would seem that upon receiving the PR request and seeing that the CLA is not signed, cla-assistant should present the CLA to the person for him to sign it and it should inform Github on the status of signing the PR. I don't know if Github enforces CLA signing and makes routing the PR to the repository owners contingent on the existence of a signed CLA. I need to look into this but at least I think for a contributor for whom a CLA is not signed for a particular repository, he should be presented with CLA for him to sign it. Let me know if this functionality exists and I will test it with a new user.

If CLA signing enforcement is not done by default, then I think that Github should provide an option to repository owners to flip a bit that would make it mandatory for a contributor to sign the CLA before being allowed to do a PR to their particular repository.

[KharitonOff]

No, the person doing the PR is expected to click either on "CLA not signed yet" icon or on details link in status bar (which is open if there is a yellow or red status) to go to sign the CLA.

after signing the CLA the comment gets changed to

There is no enforcement in the way you described it. What you can do is "protect" your branch, choosing the status of CLA assistant as mandatory one. Doing so, you'll be able to merge only PRs with green CLA assistant status.

[mpierre0220]

Excellent response. Now that I have the fix in, I will test this flow. I will let you know if there is any issue. Your response has been very helpful. Thanks a lot. Once I go though the flow and evrything checks out, I will close this issue.

[mpierre0220]

Hi I have protected the branch as you have suggested. At first, I would not get the screenshots you mentioned above. It would say that it is waiting for the CLA to be signed, then when I refresh it would say that the CLA is signed without prompting me to sign. I kept playing with it, creating new ids and, adding them to cla-assistant, cloning a repo, modifying it, doing pull requests. Now I am receiving the screen shots you mention but after I sign the CLA it returns me to the repo where I initiated the PR, but it still says that the CLA is not signed. If this rings a bell, then suggest what I am doing wrong, if not I can continue to look at it.

On a different, unrelated issue....when you add a new id to cla-assistant, github does a ping to cla-assistant to ensure that things are setup properly. The ping was failing with an "Unsupported event" error... after I look at it, I think you should have a ping.js file in the same directory as pull_request.js...unless I am missing something. I went ahead and created this file and that seemed to have fixed the ping problem...

Here is its content:

//////////////////////////////////////////////////////////////////////////////////////////////
// Github Webhook Ping Handler
//////////////////////////////////////////////////////////////////////////////////////////////

module.exports = function (req, res) {
res.status(200).send('OK');
};

You can tell whether anything else is needed in that file.

Please let me know anything you think on the other issue.

[KharitonOff]

Regarding your first point (and having in mind what I've seen as I checked some your new users and forks and their PRs 😉 ) I would check whether the user who signs the CLA is the same user who made a commit. There is a difference between an author (person who opens a PR) and a committer. CLA assistant checks committers of a PR and not it's author. So as soon all committers of the PR have signed the CLA the comment should become green.

To the second point: good catch 👍 This is one of the points that still should be done. You are welcome to create a PR with your solution.

[mpierre0220]

Great ;-) I will be careful to distinguish between PR creators and committers.... really careful ;-)
I will create another PR to cla-assistant this PM with the ping.js fix. Thanks for your help.

[brbmi]

Looks like everything has snapped into place. In all my tests, I was committing under the id that installed git on my system even if I had forked the repo under a new id. I had to actually do git config --global --replace-all user.name and user.email with that of the new user so the committer could change. But I believe that my original scenario allowed me to uncover the bug with the empty webhook array... I have a hunch that committing under same id that owns the repo may have masked the problem. Anyways I think I got it. I will close this issue. I will now to the PR for for ping.

[brbmi]

Closed!