Check Developer's Complexity Allowance for Issue Assignments

[jphamtv]

Fixes #4442

What changes did you make?

• Added new file github-actions/trigger-issue/add-preliminary-comment/developer-complexity-reminder.md
• Added new file github-actions/trigger-issue/add-preliminary-comment/check-complexity-eligibility.js and created script that checks for complexity allowance and handles actions if issue exceeds complexity allowance
• Updated github-actions/utils/format-comment.js to work with single or multiple string replacements
• In the file github-actions/trigger-issue/preliminary-update-comment.js file:
  • Imported checkComplexityEligibility function module
  • Added condition logic to skip posting the preliminary update comment if the issue is not within complexity allowance (check-complexity-eligibility.js script will handle posting a comment)
  • Updated commentObject in the makeComment function to work with the updated format-comment.js update above
  • Cleaned up formatting to maintain consistency and readability

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

• We want to create a GitHub action script to check and notify when developers assign themselves to too many issues of specified complexity (up to medium sized issue).
• New developers for the website team are expected to follow the chain of complexity progression when assigning themselves to an issue: two good first issues, one small issue, one medium issue, and at least one large issue. We make an exception to this progression for developers who apply both front end and back end roles on their prework issues.

Information For Testing

1. Create a copy of the Project Board by following the directions outlined in Tip 6: Creating your own Project Board.

2. Create a personal access token that will allow the automation to work on your duplicated project board by following the directions outlined in Tip 7: Using Personal Access Tokens to test in your own Project Board.

3. Copy Hack for LA's website repo labels into your repo using GH CLI. This command will clone the labels from the source repository to your own repository. Install the tool, login, then use this command (replace "your-username/your-repo" with your repo's information):

   gh label clone hackforla/website --repo your-username/your-repo
   

4. To quickly create test issues on your repository's project board, use the GitHub CLI and run the script provided in this repository: https://github.com/jphamtv/create-issues-script

   Follow the instructions in the script's README file to set it up and execute it.

5. Please note that the script skips checking for complexity when the creator of an issue assigns themselves to that issue. To properly test the complexity eligibility checks, you'll need to create a separate GitHub test account.

6. Use the test account to self-assign the "Pre-work Checklist" issue and other issues created by the script. This will ensure that the complexity eligibility checks are triggered correctly.

7. Update the secret tokens in the following workflow files:

   • github/workflows/issue-trigger.yml
     • Line 21: Replace the secret token with your own.
     • Line 46 (optional): Replace the secret token if needed.
   • github/workflows/move-closed-issues.yaml
     • Line 24 (optional): Replace the secret token if needed.

8. Change the column ID constants in the handleIssueComplexityNotPermitted function of the github-actions/trigger-issue/add-preliminary-comment/check-complexity-eligibility.js file (lines 373 and 374) to match your repo project board's column ID. You can get the ID number from the column's URL link.

9. In your fork of the website repository, go to "Settings" and change the default branch from gh-pages to this PR's branch.

10. Self-assign the Pre-work Checklist with your test account and then close the issue. It should automatically move to the QA column. This is to make sure it gets moved to the In Progress column after the script re-opens it.

11. Move issues to the Prioritized Backlog and self-assign issues to test the criteria described in the "Your script should check the following" Action Item.

[github-actions]

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 jphamtv-check-complexity-eligibility-4442 gh-pages
git pull https://github.com/jphamtv/hackforla-website.git check-complexity-eligibility-4442

[LRenDO]

Hi @t-will-gillis! Thanks for reviewing this issue! When you have a minute, please add your ETA and Availability. Thanks!

[t-will-gillis]

Availability: Thurs & Fri
eta: 5/17 e.o.d.

[t-will-gillis]

need to push back my timeframe
eta now: 5/19 e.o.d.

[t-will-gillis]

Hey @jphamtv Awesome work on this! There is a lot to review- I am still in the middle of testing and going through the workflow but these are some questions, comments, conversations to start with. I likely will add to this, and some of the questions / comments might turn out to be irrelevant, which is why this is only a review comment for now.

• First, thanks for enforcing some styling as well as adding semi-colons in preliminary-update-column.js! (and prevent future CodeQL alerts)

• These are out of scope, but line 52...var i..., i -= 1, eventdescriptions

• Nice find with the "GH CLI"! I had already copied the labels using the API, but this is faster and easier. This should be added to Tip 6 on the Latest edits to Hack for LA's GitHub Actions.

• Bravo with your create-issues-script! Disclosure: I have not actually used it yet. However, it makes perfect sense to just automatically spit out 6-8 issues. We (or I) should have done this already, so this is great.

• In check-complexity-eligibility.js:

  • Lines 373 and 374, conversation: how are you planning to handle the column id's?
  • Also, should "Prioritized Backlog" be included since often the dev forgets to move the issue? ( Tested this: I assigned myself from "Prioritized Backlog" and still was unassigned. )
  • Line 162: conversation only: does this need page turning? Or maybe the assumption is that if the dev has over 100 self-assigned issues we don't need to worry...
  • This is something to look at: Line 176-182, the workflow was initially crashing for me and returning Error: Assignee's Pre-Work Checklist not found in assigned issues. but since the Pre-Work Checklist exists, it looks like the search found an issue where at least one of the elements had a null value, causing the mistaken error message.

  

  • Conversation: in general, are Merge Team, Leads, devs who have 'completed the ladder' going to be excluded from this functionality?
  • I can see that 4442 requests it, but why is the "Pre-work Checklist" reopening, and why to "New Issue Approval"?
  • (This might be part of the plan where eventually everyone's Prework becomes a record for everything that they have worked on)

To be continued...

So again, still want to keep looking through this and might post more and/or answer my own questions, but great work!

[jphamtv]

Hey @t-will-gillis, thanks for the review and feedback so far.

• In check-complexity-eligibility.js:

  • Lines 373 and 374, conversation: how are you planning to handle the column id's?

I'm not sure I fully understand your question. Could you please clarify what you mean by "handle the column id's"? The variables declared lines 373 and 374 are used on line 397 and line 414 to handle moving the issue back to the New Issue Approval column and the Pre-work Checklist to the In Progress column.

• Line 162: conversation only: does this need page turning? Or maybe the assumption is that if the dev has over 100 self-assigned issues we don't need to worry...

You're right, this is a great catch. My assumption is also that if a developer has over 100 self-assigned issues, we don't need to worry about page turning. But direction: 'asc', property needs to be included in the query to return the issues sorted in ascending order, as the default is descending order. This way the oldest issues are returned first.

• This is something to look at: Line 176-182, the workflow was initially crashing for me and returning Error: Assignee's Pre-Work Checklist not found in assigned issues. but since the Pre-Work Checklist exists, it looks like the search found an issue where at least one of the elements had a null value, causing the mistaken error message.

Is this a persistent error or did it occur before the Prework Checklist was self-assigned? I think this error may happen when a Pre-work Checklist is first opened but not yet assigned, which could be the case during the onboarding process for new members.

To prevent this, I can add a fallback to handle null value on the assignee ID specifically for the onboarding scenario. So the error won't happen when a new member's Pre-work Checklist is opened but not yet assigned.

After the onboarding process is complete, this shouldn't be an issue because developers who are ready to start working on issues will have the Pre-work Checklist issue assigned to them. Let me know your thoughts on this approach, and if you have any further suggestions.

• Conversation: in general, are Merge Team, Leads, devs who have 'completed the ladder' going to be excluded from this functionality?

Currently, the script doesn't exclude Merge Team members, Leads, or developers who have completed the ladder from the complexity check. I could add functionality to check if the assignee is a member of the website-admin or website-merge teams and skip the complexity eligibility check if this is the case.

Regarding developers who have completed the ladder, we currently don't track this information anywhere and would need to explore a way to identify developers to exclude them from the complexity eligibility check.

• I can see that 4442 requests it, but why is the "Pre-work Checklist" reopening, and why to "New Issue Approval"?
• (This might be part of the plan where eventually everyone's Prework becomes a record for everything that they have worked on)

The moving of the "Pre-work Checklist" issue to the "New Issue Approval" column when it is reopened is handled by a GitHub Action outside of this script. This GitHub Action moves all opened or reopened issues to the "New Issue Approval" column. The check-complexity-eligibility.js script is responsible for moving the Pre-work Checklist to the "In Progress" column after it's reopened.

That's my understanding as well of being part of the plan to have the Pre-work Checklist become a record for everything. When I brought it up at the Dev/PM meeting, the decision was to move it to the "In Progress" column.

Thanks again for your valuable feedback and questions so far. Please let me know if you have any further questions or if there's anything else I can clarify.

[t-will-gillis]

Hey @jphamtv Sheez I lost track of this issue, sorry.

• About the column ids: What I was thinking about is schedule-fri-0700.yml and how we are using a secret to hide the IN_PROGRESS_COLUMN_ID. But... I am not sure why this workflow would be using this secret here, since it is very easy to find the column id without even having access to HfLA.

  • My only thought is that maybe(?) we use a secret id so that if someone clones the repo and is careless, they won't mistakenly refer to a column in our Project Board. But that person would not have a valid access token so I am not sure why we would care.
  • So when I asked about lines 373-374 I was wondering if you were going to use secrets to hide the column ids, but unless someone knows a good reason it probably is not needed.

• About the page turning: Agreed, we don't need more than 100 results. But great call about direction: 'asc'!

• About lines 176-182: In my tests, I have had an assigned "Prework Checklist". I found that if I changed the per_page: 20 on Line 162, everything works fine, but when per_page: 100 there is an error> HOWEVER this could be that somewhere in my repo, one of my test issues has something weird going. This likely is not worth worrying about unless the same error pops up somewhere else.

  

• About excluding Merge Team, Leads, devs who have completed the ladder, etc.: ugh- Since it is checking for the dev's "Pre-work Checklist" anyway, maybe your workflow can check whether the dev's prework has a label that says to the effect that "this person completed the ladder and should excluded from these checks".

[t-will-gillis]

Hi @jphamtv -

Bonnie asked that I include our conversations from Slack- This is a summary of what is going on with the migration, etc. and is from our previous conversation 6/17/24:
You might have seen the Sunset Notice on the Project Board. GitHub is discontinuing Projects (classic) and before August Hack for LA needs to migrate to the Projects Beta. After talking with Bonnie and the team, doing some testing, and checking the calendar, we have decided that the Website is going to migrate to the new Projects Beta on Monday, June 24th.

Sorry for the short notice- this is happening very quickly. Each of you have currently submitted a PR that involves GitHub Actions and wanted to let you know that we will need to hold off merging any new GHAs until after the migration. To make this interesting, also need to let you know that after migration, any of the existing API calls using a ‘column’ reference will no longer work because Projects Beta is not structured the same- the new term is ‘status’ and it is not interchangeable. Some other REST API endpoints that will be deprecated soon are here.

So for all of you, after migration your workflows will need to be tested using Projects Beta. If you already installed Projects (classic) on your repo you can migrate to Projects Beta for early testing. (FYI after migrating you can reopen Projects (classic)- but do that immediately if you want to keep it because that option disappeared for me after a day.)

For any workflows that currently involve moving an issue to or from a ‘column’, this should be doable using GraphQL, ProjectV2, and Status. (I have not done this yet- still learning!) And/or we can discuss other options.

Finally, one last thing I notice is that for all of you there are parts of your issue that overlap with the other two’s issues. As one example, all three of your issues include posting messages to a member’s “Pre-work Checklist”. Unfortunately, this should have been caught earlier so that each issue was worked on after the other was completed, so that you would not each have a different solution to the same problem. (I could also be wrong and maybe there will not be any conflicts.)

[t-will-gillis]

From 6/24 Slack
Hey everyone- we have migrated to Projects Beta. As I mentioned before we made revisions to all workflows that previously involved 'columns' including removing all workflow jobs that used the alex-page automation since these no longer work. "Issue Trigger", "Pull Request Trigger", and "Schedule-Friday" plus related js files had the biggest changes.