Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add GitHub Action to update project field with priority. #80931

Closed
wants to merge 3 commits into from

Conversation

john-legg
Copy link
Contributor

Adds a new GitHub Action to update the Priority field of our new board whenever a [Pri] label is added to an issue. This field is needed to group and sort issues by priority on the board, which will tremendously help the triaging workflow.

Notes

Due to the many quirks of GitHub actions, I had to use a few workarounds, which made the code a bit clunkier than I'd like. I'll put some comments describing some of them. I'm open to any suggestions to make it better!

Testing Instructions

  • Unfortunately, this isn't testable until it's merged. However, I did test this out on my own repository and it works fine.

Pre-merge Checklist

  • Has the general commit checklist been followed? (PCYsg-hS-p2)
  • Have you written new tests for your changes?
  • Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
  • Have you checked for TypeScript, React or other console errors?
  • Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
  • Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
  • For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-ajp-p2)?

@john-legg john-legg requested a review from a team as a code owner August 22, 2023 17:43
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 22, 2023
@github-actions
Copy link

github-actions bot commented Aug 22, 2023

@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

uses: tibdex/github-app-token@v1
with:
app_id: ${{ secrets.APP_ID }}
private_key: ${{ secrets.APP_PEM }}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The standard GitHub token does not have the scope needed to access projects, so a new token has to be created. It was recommended in this doc to create a GitHub App for organization projects rather than a personal access token. I created one easily in my own repo, but I don't have the permission to do so in the Automattic organization.

Is that a permission I can get or is it not recommended? Is there already a token out there than has read and write permissions for organization projects, because that's all I need

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that a permission I can get or is it not recommended? Is there already a token out there than has read and write permissions for organization projects, because that's all I need

It looks like there might already be one? Looking at this post: p3topS-193-p2. I see a secret configured in wp-calypso secrets called PROJECT_AUTOMATION_TOKEN that was added 5 days after that post, so I'm guessing it is likely the same secret and might work? cc @jeherve to see if he knows! That would certainly be easier than having to get the powers-that-be manage a new app install! 😅

Either way, if specific GitHub secret isn't it, we could certainly make a new one with the token created from that post. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nice! Great catch 👏 Based on the post, it sounds exactly like what I'm looking for! I'll wait for Jeremy to chime in though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, that's what it's for! :)

runs-on: ubuntu-latest

# Only runs if a priority label is added.
if: contains(github.event.label.name, '[Pri]')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only runs when [Pri] labels are added, which cuts down on unnecessary runs. I originally wanted to also skip the entire action if the issue wasn't in the project, but there doesn't seem to be an easy way to get that info from a single issue event without iterating through the entire project board first 😢 It's not a dealbreaker though! It won't cause any chaos, just extra runs.

Also, I really wish I could use the LABEL_PREFIX environment variable I created above here, but it's just one of those weird GitHub Actions quirks again.

Copy link
Contributor

@dpasque dpasque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome John, and off to a such a great start! 😄

I left a few responses and questions -- let me know what you think! 👍

label="${{github.event.label.name}}"
priority=${label#"$prefix"}
echo "PRIORITY_LABEL=$priority" >> $GITHUB_ENV
echo $PRIORITY_LABEL
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question (non-blocking): What's the point of this line? As I understand, I don't think you actually have a value here because you haven't actually set a variable named PRIORITY_LABEL for this local script, just set it up for the next one? And do we need this here, or is it kind of for debugging?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this could probably use some clean up and renaming. I'm really just trying to get the actual priority without the prefix (Low, Normal, etc.) and save it as an environment variable to use on the next step when updating the project field. That last line is for debugging haha, whoops! 😅 I'll take that out.

Comment on lines 12 to 15
LABEL_PREFIX: "[Pri] "
PROJECT_NUMBER: 391
PROJECT_FIELD: "Priority"
ORGANIZATION: "Automattic"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Praise: Mmm, I like how you pull all the static variables we need in one place here -- makes it super easy for future updates! 😄

uses: tibdex/github-app-token@v1
with:
app_id: ${{ secrets.APP_ID }}
private_key: ${{ secrets.APP_PEM }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that a permission I can get or is it not recommended? Is there already a token out there than has read and write permissions for organization projects, because that's all I need

It looks like there might already be one? Looking at this post: p3topS-193-p2. I see a secret configured in wp-calypso secrets called PROJECT_AUTOMATION_TOKEN that was added 5 days after that post, so I'm guessing it is likely the same secret and might work? cc @jeherve to see if he knows! That would certainly be easier than having to get the powers-that-be manage a new app install! 😅

Either way, if specific GitHub secret isn't it, we could certainly make a new one with the token created from that post. 👍

echo $PRIORITY_LABEL

# Update the project field with the priority.
# Note: if this step fails, the issue is most likely not in the project.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion (maybe blocking?): Bahhh, this is tough... But I think we may want to avoid this if at all possible. Having steps fail in a non-meaningful way (or in a way that could be predicted and avoided) is probably a state we want to avoid.

I think our best bet is having the first step in this workflow be an API call that checks and evaluates if the current issue is on our project. To do that, we can use either curl or the gh CLI (as they do in the sample docs). Then, all the other steps would only run if the output of that first step is "true" or "1" or something like that.

The API to check that is GraphQL and I know it can be a bit mind-numbing if you're not experienced with that API design (like meeeeee lolol 😅 ), but I think it's worth it in this case!

Let me know how I can help with that -- I'd be glad to go dig through the API docs and try to spin up that checking step! 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think our best bet is having the first step in this workflow be an API call that checks and evaluates if the current issue is on our project.

Totally agree! I had an old attempt at this from yesterday that failed miserably haha. I'll try to resurrect it and see what went wrong. Thanks for your suggestion!

steps:

# Checks if the issue is already in the project. If not, the next steps will be skipped.
- name: Check if issue is in project
Copy link
Contributor Author

@john-legg john-legg Aug 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think our best bet is having the first step in this workflow be an API call that checks and evaluates if the current issue is on our project.

So after a lot of trial and error, I was able to cobble something together 😅 The GraphQL piece iterates through all projects the issue has been added to, looks for a match on the project number, and stores a boolean value in a new variable ISSUE_IN_PROJECT, which we use later in the conditional statements.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! 🚀

# Checks if the issue is already in the project. If not, the next steps will be skipped.
- name: Check if issue is in project
env:
GITHUB_TOKEN: ${{ secrets.PROJECT_AUTOMATION_TOKEN }}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see a secret configured in wp-calypso secrets called PROJECT_AUTOMATION_TOKEN

Thanks again for discovering this! I removed the token generation step and replaced it with the token you mentioned. I feel like this has all the permissions I need 👍

@john-legg
Copy link
Contributor Author

john-legg commented Aug 24, 2023

@dpasque Thanks again for your helpful suggestions! I just pushed a bunch of changes that should address everything. Please take a look whenever you have a moment! I'll be AFK tomorrow, so I'll probably wait to merge on Monday (assuming it looks okay).

I also refactored a few things to make the naming less priority label-focused since I'll be updating the script to handle other labels like [Type] in the next PR.

Copy link
Contributor

@dpasque dpasque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job John! 🥳 Left a tiny optional comment, but otherwise I think this looks GTG to me!

steps:

# Checks if the issue is already in the project. If not, the next steps will be skipped.
- name: Check if issue is in project
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion (non-blocking): If you want, you could skip this step too if the label doesn't have the right prefix! Not required though of course. 😄

steps:

# Checks if the issue is already in the project. If not, the next steps will be skipped.
- name: Check if issue is in project
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! 🚀

Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this would be useful in multiple Automattic repos (Calypso, Jetpack, Themes, and more), I think it would make sense to have this part of the existing repo gardening GitHub action instead of having it be a new action altogether.

The "Triage issues" task already runs whenever an issue is labeled, so I think it could be extended to make changes to a project as well as labels:
https://github.com/Automattic/jetpack/tree/dc4128083248033728c76b687b7c9006500a9c86/projects/github-actions/repo-gardening/src/tasks/triage-issues

We already have logic in there to fetch labels for an issue, so part of the job is done. You would then run a graphql query from inside the task with octokit.graphql.

I talk a bit about how this can be done in this post:
https://jeremy.hu/github-actions-javascript-action-part-3/

You could also check this PR (that never got merged) for an example of how to use GraphQL inside the existing action:
Automattic/jetpack#24862

I think this material should allow you to move you action to the existing repo gardening action, while keeping all the functionality you've developed (which is going to be super handy!).

@john-legg
Copy link
Contributor Author

Closing this since I'm converting this to be included in the existing repo gardening GitHub action that Jeremy mentioned. I'll be opening a new PR for it soon!

@john-legg john-legg closed this Sep 11, 2023
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Sep 11, 2023
jeherve added a commit to Automattic/jetpack that referenced this pull request Oct 4, 2023
This replaces Automattic/wp-calypso#80931

This is a first version of the new task. Eventually, I believe we should be able to add more automations in there.

For now, here is what it does:

- It is triggered every time an issue is labeled.
- It looks if there are any priority labels in the issue (either added when labeling, or existing in the issue).
- It looks for information about our company GitHub project board.
- It checks if our issue is already on the board.
- If so, it updates the priority field on that board to match the priority label that was found in the issue.
jeherve added a commit that referenced this pull request Oct 4, 2023
This task was created in Automattic/jetpack#33469

Related discussion: pciE2j-2u3-p2#comment-2359

Replaces #80931
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants