Implemented hiding of outdated bot comments - #1976

[iancooperman]

Fixes #1976

What changes did you make?

• Added code that hides bot "update" comments older than 7 days.

Why did you make the changes (we will use this info to test)?

• To improve readability of issue histories.

[github-actions[bot]]

Want to review this pull request? Take a look at this documentation for a step by step guide!

From your project repository, check out a new branch and test the changes.

git checkout -b iancooperman-ga-hide-old-comments-1976 gh-pages
git pull https://github.com/iancooperman/hackforla-website.git ga-hide-old-comments-1976

---

Note that CONTRIBUTING.md cannot previewed locally; rather it should be previewed at this URL:

https://github.com/iancooperman/website/blob/ga-hide-old-comments-1976/CONTRIBUTING.md  

[roslynwythe]

@iancooperman Please see the CodeQL alert/annotation above. Does the log entry indeed depend on a user-provided value? If you feel that the alert is a false positive, please leave a comment with your reasoning. Please do not dismiss the alert. If the alert is legitimate, would you be willing to sanitize the log entry as described in "show more" ? Thank you for taking up this large and complex issue!

[roslynwythe]

• Hi @iancooperman thank you for completing this challenging issue. I'm currently reviewing another open PR, #6542 which modifies the same file github-actions/trigger-schedule/add-update-label-weekly/add-label.js and when that review is complete I will use the same test environment to review this one.

[roslynwythe]

@iancooperman Please advise, what are the scopes required for secrets.HFLA_PROJECT_BOARD_TOKEN ? Also prior to merge I assume that I need to generate the token/secret - should that be done from the HackforLABot account?

Also I noticed that workflow_dispatch was added in schedule-fri-0700.yml. Was that just for testing or is the intention to retain it is ? Is there any risk in allowing that workflow to be manually triggered?

[iancooperman]

@iancooperman Please advise, what are the scopes required for secrets.HFLA_PROJECT_BOARD_TOKEN ? Also prior to merge I assume that I need to generate the token/secret - should that be done from the HackforLABot account?

Also I noticed that workflow_dispatch was added in schedule-fri-0700.yml. Was that just for testing or is the intention to retain it is ? Is there any risk in allowing that workflow to be manually triggered?

@roslynwythe It appears I forgot to revert the name of that token back to what it's called in the canonical repo, as noted in the docs. I'll go ahead and do that.

As for workflow_dispatch, that was purely for testing purposes, but I was hoping we could keep it so the next person having to edit this has one less hoop to jump through when setting up their testing environment. Manually triggering will cause the bot to create a duplicate comment, leading to there being multiple bot comments that aren't minimized (because they're not 7+ days old yet).

[roslynwythe]

@iancooperman thank you for the explanation regarding the HACK_FOR_LA_BOT secret.

@t-will-gillis Ian has suggested retaining workflow_dispatch in schedule-daily-1100.yml in order to make future testing more convenient. Do you approve of that change?

[iancooperman]

@iancooperman @t-will-gillis - The wiki does refer to secrets.HACK_FOR_LA_BOT however that name does not appear in the list of action secrets for the repository, so I believe it is outdated information. I apologize for the confusion. There is a token with a similar name secrets.HACKFORLA_BOT_PA_TOKEN which is in use in our workflows for GitHub API requests so I suggest using that one, if Will agrees.

@roslynwythe I've made the replacement. I'll be on a flight during tonight's dev meeting, so unfortunately we won't be able to discuss further then.

[iancooperman]

@roslynwythe

Based on the log you linked, it appears that at least some of the comments are being minimized, as the response from the GraphQL API is saying so. But there are so many comments that the secondary rate limits on the API are being run into. The docs on API best practices suggest waiting at least one second between each request. Should I proceed with implementing the recommended delay?

As for the scopes, here's what I have selected:

[roslynwythe]

@iancooperman I suggest implementing the wait to avoid rate-limiting and also minimizing the scopes of the token so that we can keep privledges to a minimum.

[t-will-gillis]

Started reviewing, hope to finish e.o.d. 4/21

[iancooperman]

@roslynwythe I can probably have the wait done by this coming Sunday. I won't have much availability until after Thursday though because I have another, rather large, obligation to prepare for.

@t-will-gillis isTimelineOutdated() seemed like a convenient place to put all my code because it already loops through every event on an issue, including comments. But I do agree that there are probably cleaner ways to go about it.

[roslynwythe]

@t-will-gillis @iancooperman Please advise, should we add the "repo" scope to HACKFORLA_BOT_PA_TOKEN, or should we define a new token for use in this PR?

[t-will-gillis]

@roslynwythe @iancooperman I think that the GitHub literature says the the best practice is to only give a token the scopes that it needs and nothing more, which would be an argument to create a new token. I can go ahead and create one if this is what we want to do. For the secret's name HACKFORLA_REPO_TOKEN to follow a similar naming pattern as for the other tokens?

[t-will-gillis]

@iancooperman

@t-will-gillis isTimelineOutdated() seemed like a convenient place to put all my code because it already loops through every event on an issue, including comments. But I do agree that there are probably cleaner ways to go about it.

Yes, that makes sense since it is already looping through the issue, as you mentioned.

[roslynwythe]

@roslynwythe @iancooperman I think that the GitHub literature says the the best practice is to only give a token the scopes that it needs and nothing more, which would be an argument to create a new token. I can go ahead and create one if this is what we want to do. For the secret's name HACKFORLA_REPO_TOKEN to follow a similar naming pattern as for the other tokens?

Yes I agree. Thanks

[iancooperman]

@roslynwythe Sorry it's taken so long, but I've added the wait. Go ahead and try it out again when you get the chance.

[roslynwythe]

@iancooperman Using a PAT with the "repo" group of scopes in my fork of the repo, all of the "please add update" bot comments older than 7 days were hidden successfully. Thank you for adding the wait and your patience with changing the token. Prior to merge we need to create HACKFORLA_REPO_TOKEN. Please advise @t-will-gillis, if you would like me to create that secret.

[t-will-gillis]

Hi @roslynwythe and @iancooperman because of another automation's requirements, the HACKFORLA_ADMIN_TOKEN also needed the "repo" scope. It now has the following scopes so I believe it will work for this automation:

• repo
• admin:org_hook
• write:org

@iancooperman sorry for the delay- I will try to look at this tonight.

[iancooperman]

@t-will-gillis @roslynwythe I just changed the name of the secret in schedule-fri-0700.yml. Anything else I can do?

[iancooperman]

Hi @iancooperman

• The file ga-hide-old-comments-1976.code-workspace is included in the commit - what is this file?

• In add-label.js:

  • please confirm whether the variable at line 118 is being used, if not please remove
  • there are extra spaces on line 142. please remove
  • extra line at 189, please remove
  • to prevent future CodeQL alerts, please add semi-colon at end of line 154

Thanks

ETA: 5/16 EoD Availability: 5/16 6pm-9pm

[iancooperman]

@t-will-gillis I've made the requested changes in add-label.js. ga-hide-old-comments-1976.code-workspace was mistakenly pushed when I was exploring VS Code's workspace settings. It has now been deleted.

[Thinking-Panda]

Hi @roslynwythe Whenever possible please add you availability and ETA for this PR. Thanks!