comment lgtm in gitlab but always returns "you cannot LGTM your own PR."

[soulseen]

when I installed lighthouse witn provider gitlab, and it running well. But I comment lgtm in Merge Requests, it always returns

you cannot LGTM your own PR.

In response to this:

/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the jenkins-x/lighthouse repository.

---

No matter what user created MR, or what user commented on LGTM, the bot always returns it above.

what's more

I found the cause is Comment author same with MergeReuests author. ref https://github.com/jenkins-x/lighthouse/blob/master/pkg/plugins/lgtm/lgtm.go#L267

and the root cause is in jenkins-x/go-scm, the func convertMergeRequestCommentHook is used the same user to struct PullRequest and Comment

My team has used the lighthouse in a deep way, what are the roadmap of the lighthouse and the next step improvement? and I can provide some help.

[slimm609]

I am seeing this same issue on gitlab.

[slimm609]

according to https://docs.gitlab.com/ee/user/project/integrations/webhooks.html#comment-on-merge-request

it looks like using object_attributes.author_id and merge_request.author_id would be the proper way to verify if the comment author and merge request author are the same

[soulseen]

according to https://docs.gitlab.com/ee/user/project/integrations/webhooks.html#comment-on-merge-request

it looks like using object_attributes.author_id and merge_request.author_id would be the proper way to verify if the comment author and merge request author are the same

agree

[jstrachan]

anyone fancy creating the PR on go-scm to use the correct user id field?

[jeremyrickard]

Hey @jstrachan i think I have some time this week to take a look (unless @soulseen wants to contribute)

[jstrachan]

@jeremyrickard that would be totally awesome! ping us on slack if you get stuck with anything

[slimm609]

@jeremyrickard and I talked about it today for a bit, it looks like the fix can be fully solved in go-scm gitlab driver and shouldn't need any changes from lighthouse/lgtm (besides being rebuilt to pick up the changes)

[jstrachan]

@slimm609 awesome! incidentally whenever go-scm is released there's an automated PR on lighthouse and a new release

[slimm609]

We have a "hacky" fix working to make sure it would fix the issue properly. @soulseen if you want to test in a test environment, you can pull slimm609/test-jx:webhook5 from dockerhub and make sure it works.

It will also log the commenter ID and the author ID to the logs of the webhook container.

Its a little more work to fix it properly but we know how to fix

[soulseen]

@slimm609 wonderful! I used it in test env, and it works well.

[jenkins-x-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
Provide feedback via https://jenkins-x.io/community.
/lifecycle stale

[jeremyrickard]

/remove-lifecycle-stale

[slimm609]

/remove-lifecycle stale

[jeremyrickard]

I've opened a PR in go-scm to address this. Essentially, when the convertMergeRequestCommentHook method runs, the previous code was just building up a single user object. Really need to fetch both the comment user and the MR user and set them appropriately, to ensure that things are handled properly. This PR adds logic to fetch the user objects (adds a reference to the gitlab user service to do so).

[jstrachan]

I think this is all fixed now? let me know if not and we can reopen