Is Danger always making a new comment?

[orta]

I keep seeing notifications in Slack about new comments by Danger, when they should be being edited.

[jmenestr]

I am also seeing this. I'm using 9.2.2 and currently see a new comment.

I can inspect each comment's markdown and see the same dangerid for each comment.

I was also under the impression that it should search all the comments for ones with the same dangerid and update them, rather than create a new one.

[tamlenguyen]

Same for me, adding a new comment every time

[nick28]

Same for me in Bitbucket Cloud, any updates or any leads how can we solve this?

[orta]

Each platform has a updateOrCreateComment which must not be doing what we expect

• danger-js/source/platforms/BitBucketCloud.ts

  Line 100 in 6ae125d

  async updateOrCreateComment(dangerID: string, newComment: string): Promise<string | undefined> {

• danger-js/source/platforms/github/comms/issueCommenter.ts

  Line 133 in 6ae125d

  updateOrCreateComment: async (dangerID: string, newComment: string): Promise<string | undefined> => {

[HelloCore]

Same for me in Bitbucket Cloud, any updates or any leads how can we solve this?

For Bitbucket Cloud, we need DANGER_BITBUCKETCLOUD_UUID to detect the owner of the comment.
Could you check your UUID?
It has to be this format {zzzzzzzz-zzzz-zzzz-zzzz-zzzzzzzzzzzz}. (38 characters including brackets).

[orta]

That doesn't feel right to me - the bot should be able to ask for it's own user ID via the API and it likely does

[HelloCore]

Yes, that's right, I've never thought about that before.
I'm going to work on this.

[nick28]

Same for me in Bitbucket Cloud, any updates or any leads how can we solve this?

For Bitbucket Cloud, we need DANGER_BITBUCKETCLOUD_UUID to detect the owner of the comment.
Could you check your UUID?
It has to be this format {zzzzzzzz-zzzz-zzzz-zzzz-zzzzzzzzzzzz}. (38 characters including brackets).

I had set it as mentioned in the documentation. And yes it is in the format as mentioned by you

[HelloCore]

Same for me in Bitbucket Cloud, any updates or any leads how can we solve this?

For Bitbucket Cloud, we need DANGER_BITBUCKETCLOUD_UUID to detect the owner of the comment.
Could you check your UUID?
It has to be this format {zzzzzzzz-zzzz-zzzz-zzzz-zzzzzzzzzzzz}. (38 characters including brackets).

I had set it as mentioned in the documentation. And yes it is in the format as mentioned by you

We've just released version 9.2.9, could you remove DANGER_BITBUCKETCLOUD_UUID and try this version?
You might need to create the new credentials with Read Pull requests and Read Account.

I suspect that this issue would have something to do with brackets, some CI might encode it. At 9.2.9, we fetch UUID from Bitbucket Cloud Instead, and it works fine for me.

[nick28]

Same for me in Bitbucket Cloud, any updates or any leads how can we solve this?

For Bitbucket Cloud, we need DANGER_BITBUCKETCLOUD_UUID to detect the owner of the comment.
Could you check your UUID?
It has to be this format {zzzzzzzz-zzzz-zzzz-zzzz-zzzzzzzzzzzz}. (38 characters including brackets).

I had set it as mentioned in the documentation. And yes it is in the format as mentioned by you

We've just released version 9.2.9, could you remove DANGER_BITBUCKETCLOUD_UUID and try this version?
You might need to create the new credentials with Read Pull requests and Read Account.

I suspect that this issue would have something to do with brackets, some CI might encode it. At 9.2.9, we fetch UUID from Bitbucket Cloud Instead, and it works fine for me.

Seems to be fixed and danger is now updating the comments in bitbucket cloud. Tested with DANGER_FAKE_CI="YEP"

[ghost]

It still creates new comments on Github

[mokagio]

Same as @zhongliangwang, I'm seeing new comments on GitHub as well.

I made a dedicated repo to reproduce 👉 mokagio/danger-new-comments-example#3.

I tried to fetch Danger JS from master but I couldn't get it to work.

[mokagio]

I made a new test, mokagio/danger-new-comments-example#5, with a Dangerfile looking like this, to remove any kind of custom logic that could have resulted in the issue.

import {warn} from "danger";

warn("This demo warning was generated on " + new Date());

Same result.

[mokagio]

FWIW, the Ruby version doesn't have this issue. See mokagio/danger-new-comments-example#6.

[mokagio]

I did a bit more investigation, and saw that Danger JS also makes multiple comments when given file and line.

[orta]

Interesting! Yeah, that check and update code was done before inline comments were added, and it makes sense why I rarely see it (I don't use multiline anywhere)

My bet? the check to find the existing main comment might be hitting these and thinking there isn't an issue to update

[f-meloni]

I think the reason why is doing multiple comments is that you are using

 GITHUB_TOKEN: ${{ secrets.MOKAGIO_BOT_GITHUB_TOKEN }}

Try

 GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

Actions doesn't allow you to call https://api.github.com/user, that is what danger-js uses to get the current user.

(this is the response you get)

Request failed [403]: https://api.github.com/user
Response: {
 "message": "Resource not accessible by integration",
 "documentation_url": "https://developer.github.com/v3/users/#get-the-authenticated-user"
}

This is why the user id is hardcoded, then if you don't use the actions user danger can not understand which comment is the danger comment and edit it.
I wonder how danger ruby does this.

[mokagio]

Hey @f-meloni, you're spot on! Once I changed the action GITHUB_TOKEN value to secrets.GITHUB_TOKEN it worked as expected. 🎉 🎉 Thanks a lot! 🙏

Clearly, my understanding of what that token should have been was wrong. Or rather, I didn't spend enough time reading the docs. I though that the GITHUB_TOKEN we talk about in the Danger + GitHub Action guide was just the arbitrary name used to pass the token one has to generate if they want to use Danger. But actually, GITHUB_TOKEN refers to the token GitHub provides a token that you can use to authenticate on behalf of GitHub Actions. 🤦‍♂️

What I might do is open a PR to add the link to the GITHUB_TOKEN documentation in the Danger + GitHub Actions section. This way future me won't make the same mistake again. 🤞

Thanks again folks! ❤️

[mokagio]

Hey @f-meloni, in your comment you mention

This is why the user id is hardcoded

When you have time, could you point me to where in the code this happens? I don't understand how it ends up that even if I pass an --id, the default Danger value is used. Thanks ❤️ .

[f-meloni]

What is hardcoded is the user id, not the danger id.
I use --id often, then I think that works well, how are you setting it?

[f-meloni]

Anyway, this is the code

https://github.com/danger/danger-js/blob/master/source/platforms/github/GitHubAPI.ts#L133-L136

[mokagio]

What is hardcoded is the user id, not the danger id.
Oh gotcha. That's why I couldn't find it. Makes sense.

I use --id often, then I think that works well, how are you setting it?

I use the --id parameter too.

It works well if I have Danger locally, but I'm having troubles with the GitHub Action.

I've been experimenting with this yesterday. I'm probably doing something wrong, but it seems to me the Danger GitHub Action doesn't read the id.

See this PR here for example. I'm calling danger ci in the workflow like this:

- name: Run Consistency Checks
  uses: danger/danger-js@9.1.8
  with:
    args: |
      --dangerfile Automattic/peril-settings/org/pr/ios-macos.ts \
      --id danger_id_same
  env:
    GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

But the resulting Danger comment has id danger-id-Danger.

[orta]

Looks to me like the docker image for an action isn't respecting the args?

Step 9/10 : RUN cd /usr/src/danger &&   yarn &&   yarn run build:fast &&   chmod +x distribution/commands/danger.js &&   ln -s $(pwd)/distribution/commands/danger.js /usr/bin/danger

Was it definitely calling your dangerfile?

[mokagio]

Was it definitely calling your dangerfile?

Yes, it was. I made a commit to trigger a failure comment from Danger, and Danger posted it.

[rohit-gohri]

There are 2 ways one can provide Github Token to danger when using it on Github Actions:

1. GITHUB_TOKEN: Temporary bot token generated by Github (https://help.github.com/en/actions/configuring-and-managing-workflows/authenticating-with-the-github_token)
2. DANGER_GITHUB_API_TOKEN: Personal access token for a user, manually added to secrets

When updating/deleting comments only the 1st case is being considered in the Github Actions env and the userId is never considered, instead a hardcoded ID is returned.

danger-js/source/platforms/github/GitHubAPI.ts

Lines 133 to 136 in 5ad5fd9

	const useGitHubActionsID = process.env["GITHUB_WORKFLOW"]
	if (useGitHubActionsID) {
	return 41898282
	}

I think we should do that only if both the conditions are true, i.e.:

• CI is Github Actions (env.GITHUB_WORKFLOW is set)
• Token is in GITHUB_TOKEN and not in DANGER_GITHUB_API_TOKEN

This would allow people to use method 2. (for whatever reasons) and it would still work properly.

There would still be one case where it will fail and we probably can't detect it, just probably mention in docs not to use it this way

 GITHUB_TOKEN: ${{ secrets.MOKAGIO_BOT_GITHUB_TOKEN }}

@f-meloni @orta

[orta]

Yep, seems reasonable