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

Use Waiting for: Product Owner label to send triage notifications #492

Merged

Conversation

hubertdeng123
Copy link
Member

Uses Waiting for: Product Owner instead of Status: Untriaged to send slack notifications

@hubertdeng123 hubertdeng123 changed the base branch from main to hubertdeng123/save-due-by-dates-for-issues-in-project June 16, 2023 21:40
@codecov
Copy link

codecov bot commented Jun 16, 2023

Codecov Report

Patch coverage: 83.78% and project coverage change: +0.29 🎉

Comparison is base (8d7b327) 83.07% compared to head (688f7d5) 83.37%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #492      +/-   ##
==========================================
+ Coverage   83.07%   83.37%   +0.29%     
==========================================
  Files          96       96              
  Lines        2523     2520       -3     
  Branches      493      490       -3     
==========================================
+ Hits         2096     2101       +5     
+ Misses        421      412       -9     
- Partials        6        7       +1     
Impacted Files Coverage Δ
src/config/index.ts 97.84% <75.00%> (-1.06%) ⬇️
src/webhooks/pubsub/slackNotifications.ts 72.83% <80.00%> (-0.37%) ⬇️
src/utils/githubEventHelpers.ts 89.39% <90.00%> (+22.72%) ⬆️
src/brain/issueLabelHandler/route.ts 97.36% <100.00%> (-0.62%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@hubertdeng123 hubertdeng123 force-pushed the hubertdeng123/use-waiting-for-labels-triage-notifications branch from e214f71 to b865b19 Compare June 20, 2023 18:40
@hubertdeng123 hubertdeng123 force-pushed the hubertdeng123/save-due-by-dates-for-issues-in-project branch from d8690e3 to 16e8174 Compare June 21, 2023 18:52
Base automatically changed from hubertdeng123/save-due-by-dates-for-issues-in-project to main June 21, 2023 19:48
node(id: "${issueNodeId}") {
... on ProjectV2Item {
id
fieldValueByName(name: "Respond By") {
Copy link
Member

Choose a reason for hiding this comment

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

Do we have to query by name? If we query by id then it would be immutable, maybe?

Copy link
Member

@chadwhitacre chadwhitacre Jun 21, 2023

Choose a reason for hiding this comment

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

I guess I like having the flexibility to tinker with the names of the project board and fields without messing up the automation, if possible.

Copy link
Member Author

Choose a reason for hiding this comment

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

I also do like that, but I don't think it's possible. Played around with this using the GH graphql explorer and it's only allowing me to input a field value by name
Screenshot 2023-06-21 at 2 43 50 PM

Copy link
Member

Choose a reason for hiding this comment

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

Do you think it's worth it to use fieldValues and iterate over all of them to find the value by id? Less efficient but more resilient. I've already changed the name of the field in prod, happened days/weeks ago. 🐭

Screenshot 2023-06-21 at 5 59 02 PM

Copy link
Member Author

Choose a reason for hiding this comment

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

This is what using fieldValues looks like

const query = `query{
    node(id: "${issueNodeId}") {
      ... on ProjectV2Item {
        id
        fieldValues(first: 50) {
          nodes {
            ... on ProjectV2ItemFieldTextValue {
              id
              text
              field {
                ... on ProjectV2Field {
                  id
                  name
                }
              }
            }
          }
        }
      }
    }
  }`;

  const data = {
    issueNodeId,
  };
  const response = await sendQuery(query, data, octokit);
  // When the response due date is empty, the node doesn't exist so we default to empty string
  const issueDueDateInfoNode = response?.node.fieldValues.nodes.find(item => item.field?.id === RESPONSE_DUE_DATE_FIELD_ID) || "";
  return issueDueDateInfoNode.text;

Honestly, I really don't like relying on this line here...

  const issueDueDateInfoNode = response?.node.fieldValues.nodes.find(item => item.field?.id === RESPONSE_DUE_DATE_FIELD_ID) || "";```

Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

not all items on the project board have the same fields

Say more about this? Surprises me if true.

Copy link
Member Author

@hubertdeng123 hubertdeng123 Jun 22, 2023

Choose a reason for hiding this comment

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

Say more about this? Surprises me if true.

Er, I meant to say number of fieldValues. Say there are 6 fields for an item on a project board. If the due date field is an empty string, GH graphql api returns 5 nodes in the fieldValues.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, gotcha. Yeah, that's weird. :-/

Copy link
Member Author

Choose a reason for hiding this comment

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

Implemented the changes above. @chadwhitacre are there any other concerns for this to go in?

Copy link
Member

Choose a reason for hiding this comment

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

Let's add a comment explaining why we're using fieldValues then I'm good to go. 👍

@hubertdeng123 hubertdeng123 merged commit 94396e5 into main Jun 23, 2023
@hubertdeng123 hubertdeng123 deleted the hubertdeng123/use-waiting-for-labels-triage-notifications branch June 23, 2023 04:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants