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

Fix #4594 : Issue Triaging Action Added #5591

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions .github/workflows/issue_triage.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
name: Issue Triage and Management

on:
issues:
types: [opened, labeled, assigned]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it could be beneficial to trigger actions for label removal as well. This way, we can ensure that the needs-triage label is added as soon as any required label is missing, which will help us better track and manage such issues.


permissions:
issues: write

jobs:
triage-on-issue-open:
runs-on: ubuntu-latest
if: github.event.sender.login != 'github-actions'
steps:
- name: Reply to Issue
if: ${{ github.event.action == 'opened' }}
uses: peter-evans/create-or-update-comment@v4
with:
issue-number: ${{ github.event.issue.number }}
body: |
Thanks for filing the issue! We’ll review it shortly and route it to the correct team.

- name: Check Labels for Proper Triaging
id: check-labels
uses: actions/github-script@v6
with:
script: |
const issue = await github.rest.issues.get({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: context.issue.number
});
const labels = issue.data.labels.map(label => label.name);
const requiredLabels = {
types: ['bug', 'enhancement', 'good first issue'],
impacts: ['Impact: High', 'Impact: Medium', 'Impact: Low'],
work: ['Work: High', 'Work: Medium', 'Work: Low'],
};
const hasTypeLabel = labels.some(label => requiredLabels.types.includes(label));
const hasImpactLabel = labels.some(label => requiredLabels.impacts.includes(label));
const hasWorkLabel = labels.some(label => requiredLabels.work.includes(label));
const needsTriage = !(hasTypeLabel && hasImpactLabel && hasWorkLabel);
Comment on lines +39 to +42
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is provided in the issue description,

'exactly one of each type of these labels is assigned: issue type, work, and impact.'

However, the current implementation seems to check for at least one of each type and not exactly one of each type, which could allow multiple labels of the same type. It would be helpful to adjust the logic to ensure exactly one label of each type, as required.

core.setOutput('needs-triage', needsTriage);

- name: Manage Needs Triage Label
uses: actions/github-script@v6
with:
script: |
const needsTriage = '${{ steps.check-labels.outputs.needs-triage }}' === 'true';
if (needsTriage) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

As per the issue requirements, two conditions must be met for triaging:

the workflow should verify that exactly one open project is assigned to the issue, and that exactly one of each type of these labels is assigned: issue type, work, and impact. If any of these criteria fail to be met, the issue should be automatically labeled with "needs triage." If all of them are met, "needs triage" should be removed.

  1. The issue should be assigned to exactly one project
  2. Exactly one label of each type must be assigned.

Both (all) conditions need to be satisfied to remove the needs-triage label. If either is missing, the issue should remain labeled as needs-triage. Therefore, please ensure along with label checks the project assignment condition is also verified to handle needs-triage correctly.

await github.rest.issues.addLabels({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: context.issue.number,
labels: ['needs-triage']
});
} else {
try {
await github.rest.issues.removeLabel({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: context.issue.number,
name: 'needs-triage'
});
} catch (error) {
if (error.status !== 404) {
throw error;
}
}
}

67 changes: 67 additions & 0 deletions .github/workflows/project_assigned_verify.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
name: Verify Project Assignment
Copy link
Collaborator

Choose a reason for hiding this comment

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

I just wanted to know if it's absolutely necessary to have two separate workflows now that project verification is also part of triaging. Since both workflows share the same triggers, I believe the conditional checks should suffice. However, please let me know if it's essential to keep them separate.


on:
issues:
types: [labeled]

permissions:
issues: write

jobs:
verify-project-assignment:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you also explore if there's a way to address the additional requirement mentioned in the comment,

ie. automatically move issues into "In progress" when someone self-assigns.

runs-on: ubuntu-latest
if: github.event.sender.login != 'github-actions'
steps:
- name: Check if Label is an Impact Label
id: check-impact-label
run: |
if [[ "${{ github.event.label.name }}" == "Impact: High" ]] || \
[[ "${{ github.event.label.name }}" == "Impact: Medium" ]] || \
[[ "${{ github.event.label.name }}" == "Impact: Low" ]]; then
echo "impact_label=true" >> $GITHUB_ENV
else
echo "impact_label=false" >> $GITHUB_ENV
fi
Comment on lines +15 to +24
Copy link
Collaborator

Choose a reason for hiding this comment

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

It makes sense to conditionally check these to prevent re-runs. However, I was thinking it might be better to check the types rather than impacts, since the impact label might not be assigned by default or could be missed during triaging. Using types could increase the likelihood of properly triggering the condition. WDYT?


- name: Verify Project Assignment
if: env.impact_label == 'true'
uses: actions/github-script@v6
with:
script: |
const issueNumber = context.issue.number;

# Fetch all open projects in the repository
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# Fetch all open projects in the repository
// Fetch all open projects in the repository

Thanks for making this change to the comment syntax. However, I believe it should be reverted to //. While YAML files use # for comments, this section is within a JavaScript script block, where // is the convention. I also discussed this with Adhiambo, so reverting it back should be fine.

const projects = await github.rest.projects.listForRepo({
owner: context.repo.owner,
repo: context.repo.repo,
});

# Filter to get only open projects
const assignedProjects = projects.data.filter(project => project.state === 'open');
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure if this is fully implemented. It retrieves the project list and checks if it's open, but the requirement is to verify if the issue is assigned to a project, which would necessitate checking if the issue appears in the project list. From your demonstration, I'm unsure of its verification.

Could you please implement this fully, and it would be great if you could provide demonstrations for different scenarios, such as,

  • assigning the issue to 0, 2, or more projects and posting a comment
  • when assigning it to just 1 project, where the comment could be skipped.


if (assignedProjects.length !== 1) {
console.warn(`Issue must be assigned to exactly one open project. Found: ${assignedProjects.length}`);

# Add a comment to notify about incorrect project assignment
const existingComments = await github.rest.issues.listComments({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: issueNumber,
});

const commentAlreadyExists = existingComments.data.some(comment =>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is nice to prevent duplicate comments. However, I suggest comparing the entire current comment body with the most recent gentle reminder comment. For example, consider the cases:

  • Current comment: 'Assigned to 0 project(s)', Previous comment: None - Post comment: 'Assigned to 0 project(s)'
  • Current comment: 'Assigned to 1 project(s)' - Skip posting
  • Current comment: 'Assigned to 2 project(s)', Previous comment: 'Assigned to 0 project(s)' - Post comment: 'Assigned to 2 project(s)'

Ref: comment_coverage.yml for checking with most recent comment

The reason for this is that if an issue is reassigned to different projects at different periods, it may end up in multiple or no projects, and no reminder would be posted to handle those changes.

@adhiamboperes, I would like to confirm the requirement with you before proceeding. Should we post a comment only once or whenever there's a change? Also, when should the project assigned reminder be posted - when issue is created or when labels are added (issue is triaged)?

comment.body.includes('Gentle Reminder: This issue must be assigned to exactly one open project')
);

if (!commentAlreadyExists) {
await github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: issueNumber,
body: `Gentle Reminder: This issue must be assigned to exactly one open project. Currently, ${assignedProjects.length} projects are assigned. Thanks.`,
});
}
} else {
console.log(`Valid project assignment: ${assignedProjects[0].name}`);
}

Loading