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[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 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.

[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.

[t-will-gillis]

Hey @jphamtv You had put a ton of work into this before the projects migration fracked up everything... Were you planning to give this another go?

[jphamtv]

Hey @jphamtv You had put a ton of work into this before the projects migration fracked up everything... Were you planning to give this another go?

Hi @t-will-gillis, thanks for checking in. Yes, I'll update and test later this week. I'll message you when it's ready for your review.

[jphamtv]

Hey @t-will-gillis, I've updated and tested the code on my repo. It's now compatible with the new Projects V2 board and includes updates such as skipping the eligibility check for Admin or Merge team members. I've also revised the testing instructions, with specific changes to Step 8. Please let me know if you need clarification or have questions.

[t-will-gillis]

Hey @jphamtv - whew, just like before this is a ton of work and the project migration did not help so thanks once again for tackling this. I am still reviewing but here are prelim comments. I have it running in my repo but still need to try a couple different scenarios to trigger the comments.

This is going to be very annoying so I will apologize in advance. We have had a string of PRs that have similar functions, for example retrieving issue data and changing the issue’s status. These are pulled out into /utils/query-issue-info.js, /utils/mutate-issue-status.js, and /utils/data/status-field-ids.js, and I am writing issues to refactor “Schedule Friday” and “Issue Trigger” to use these modules and simplify files.

• We might need to discuss over Slack about how to approach this. I have a branch here if you are curious that I have not submitted yet for refactoring preliminary-update-comment.js which I am thinking should be held off until your PR is done, but hoping that you won’t mind to refactor check-complexity-elegibility.js to use /utils/query-issue-info.js, /utils/mutate-issue-status.js, and /utils/data/status-field-ids.js. Let me know what you think.
• Regarding changes to format-comment.js: These look good; however this file is used by four other workflows so we can’t make changes without editing the other files also.
• I imagine this will be OK, but will need to check that the new “Skills Issue” works now that it is replacing the “Pre-work Checklist”

To let you know, that is good info in your description and I edited the Hack for LA’s GitHub Actions (current revision) document to include your code for copying all of HfLA’s labels at once to one’s repo and for generating issues- new Tip 8.

[jphamtv]

Hey @t-will-gillis,

This is going to be very annoying so I will apologize in advance. We have had a string of PRs that have similar functions, for example retrieving issue data and changing the issue’s status. These are pulled out into /utils/query-issue-info.js, /utils/mutate-issue-status.js, and /utils/data/status-field-ids.js, and I am writing issues to refactor “Schedule Friday” and “Issue Trigger” to use these modules and simplify files.

• We might need to discuss over Slack about how to approach this. I have a branch here if you are curious that I have not submitted yet for refactoring preliminary-update-comment.js which I am thinking should be held off until your PR is done, but hoping that you won’t mind to refactor check-complexity-elegibility.js to use /utils/query-issue-info.js, /utils/mutate-issue-status.js, and /utils/data/status-field-ids.js. Let me know what you think.

Thanks for pointing out the new utils. I somehow missed them when updating the code after the migration. I've refactored check-complexity-eligibility.js to use the utils and tested it on my repo. It should be good to go now.

• Regarding changes to format-comment.js: These look good; however this file is used by four other workflows so we can’t make changes without editing the other files also.

I appreciate you letting me know that format-comment.js is used by other workflows. I should have caught this myself. I've reverted the code and included the formatComment function for multiple replacement objects in the check-complexity-eligibility.js file.

• I imagine this will be OK, but will need to check that the new “Skills Issue” works now that it is replacing the “Pre-work Checklist”

The Skills Issue will also work if it continues using the Complexity: Prework label.

To let you know, that is good info in your description and I edited the Hack for LA’s GitHub Actions (current revision) document to include your code for copying all of HfLA’s labels at once to one’s repo and for generating issues- new Tip 8.

Thank you for the feedback on my description, including my code in the wiki, and the recognition!

I updated step 8 in the testing instructions above. Please let me know if you need anything else for this PR or if we need to discuss how to handle the preliminary-update-comment.js file with your recent updates in #7373.

[t-will-gillis]

Hey @jphamtv I have been testing this and wanted to give you an update. It took a lot of tweaking to get the conditions set up where the other conditions were not triggered first, but now have it so that the check-complexity-eligibility.js is running. Unfortunately an error keeps popping up for me that appears to be in the fetchIssuesByAssignee() function (starting on line 127):

Error fetching issues for assignee t-will-gillis TypeError: Cannot read properties of null (reading 'id')

So one or more of the issues in my repo (or possibly a PR?) is returning null values. Link here But I have not been able to track it down yet.

I don’t know if this is a problem with the issues in my repo alone, or if something similar would occur in the live repo. If you have any good ideas, please let me know. Otherwise, it will take me a while to track down.

[jphamtv]

@t-will-gillis - You might be getting the error because you have three Prework checklists assigned to yourself on your repo. Try assigning only one and see if you still get the error.

[t-will-gillis]

@jphamtv Closed two of the three preworks (thank you), still had the issue. Did a little more digging and found that three of my very early issues were returning "null" on "assignee.id" even though of course I was assigned (under "assignees"). Whatefs I removed those three issues and the previous error is no longer happening. will continue...