Skip to content

Notify multiple issues 4505#6969

Merged
t-will-gillis merged 111 commits intohackforla:gh-pagesfrom
moazDev1:notify-multiple-issues-4505
Jul 15, 2024
Merged

Notify multiple issues 4505#6969
t-will-gillis merged 111 commits intohackforla:gh-pagesfrom
moazDev1:notify-multiple-issues-4505

Conversation

@moazDev1
Copy link
Member

@moazDev1 moazDev1 commented Jun 6, 2024

Fixes #4505

What changes did you make?

  • All Project Board related functions have been updated to align with ProjectV2
  • Added multiple-issue-reminder.md file
  • Modified preliminary-update-comment.js:
    • Added functionality to check if the developer is assigned to another open issue. If so, the script will:
      • Post a comment using the template message in multiple-issue-reminder.md
      • Unassign the dev from the issue
      • Add a "ready for dev lead" label
      • Update issue's status to "New Issue Approval"
    • The code exempt any developer who is part of Admin or Merge teams
    • The code exempt any other issue that applies the following conditions:
      • Any issues with label "Complexity: Prework"
      • Any Agenda issues
      • Any issues in the "Emergent Requests" or "New Issue Approval" status
    • Refactored existing code:
      • Split the code into distinct functions to improve readability and maintainability.

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

  • Ensuring developers are not assigned to multiple issues simultaneously helps maintain focus on a single task and allows other developers to find available issues to work on.

Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)

No visual changes

Moaz Ali and others added 27 commits April 19, 2024 20:00
…verify-pr-author-3906

Re-sync with upstream repository
@github-actions
Copy link

github-actions bot commented Jun 6, 2024

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 moazDev1-notify-multiple-issues-4505 gh-pages
git pull https://github.com/moazDev1/website.git notify-multiple-issues-4505

@github-actions github-actions bot added role: back end/devOps Tasks for back-end developers Complexity: Large labels Jun 6, 2024
@moazDev1 moazDev1 force-pushed the notify-multiple-issues-4505 branch from 7348515 to 7c13624 Compare July 12, 2024 08:15
@moazDev1 moazDev1 force-pushed the notify-multiple-issues-4505 branch from f5dac7d to e8fa628 Compare July 12, 2024 08:25
@moazDev1
Copy link
Member Author

Hey @t-will-gillis, thanks for notices.

  • Thanks for refactoring memberOfAdminOrMergeTeam() - just need to add the import statement at top ;-)

    Sorry for this oversight, I usually work on a separate branch to test changes on my forked repo before integrating them here. I'll make sure to be more careful next time.

  • Also, I think this conditional should occur before assignee = await getLatestAssignee(); (no reason to check for latest assignee if shouldPost === false)

    Done

  • Regarding the "try - catch" statements: I know that this is what we are supposed to do as developers, the argument being ...

    I removed all the try/catch blocks, but I believe using throw error instead of console.log will trigger the error in the GitHub Actions console as you wanted. I made a throw error version in this branch. Please let me know if this works for you, so I can apply the changes to this branch as well. I believe using try/catch will make it much easier to track the exact location of errors as our codebase grows.

  • The workflow seems to change the status to "In progress (actively working)" in many instances when it definitely should not: ...

    My bad, I tried to automate the process of changing status to "In progress" as many developers forgot to do it. I think I can fix this by adding another condition but I'll revert everything to the original state and remove it for now.

  • I notice that there is a lot of repeated code in the main() and makeComment() functions- these should be integrated.

    I just removed makeComment() and combined its functionality into createComment(). I also refactored the main() function so I don't use redundant code. The makeComment() function was meant to make comment but it was holding the whole old code making it really ambiguous and hard to refactor.

@moazDev1 moazDev1 requested a review from t-will-gillis July 12, 2024 09:20
Copy link
Member

@t-will-gillis t-will-gillis left a comment

Choose a reason for hiding this comment

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

Hey @moazDev1 so close! Thanks for the latest round of changes.

  • Thank you for your reasoning and explanation regarding throw error- I appreciate it. I tested your branch and created a purposeful error: The error was logged and the workflow halted- perfect.
  • Thanks for consolidating the main functions. I think this is much cleaner now.
  • I tested this for multiple combos of team member/non-team member, statuses, special labels, etc. and everything appears to be working as intended.
  • I ran the tests without the token on line 55- I believe this token is not needed and can be removed.

Please go ahead with the throw error, and I'll mark as "Comment" in the meantime.

Changing this to Complexity: Extra Large also.

@t-will-gillis t-will-gillis added size: 8pt Can be done in 31-48 hours Complexity: Extra Large and removed Complexity: Large size: 5pt Can be done in 19-30 hours labels Jul 14, 2024
@moazDev1 moazDev1 force-pushed the notify-multiple-issues-4505 branch from 94728e4 to 4846758 Compare July 15, 2024 00:48
@moazDev1
Copy link
Member Author

Hey @t-will-gillis I just added back try/catch blocks and removed the unnecessary token.

Thanks for upgrading my PR complexity, I appreciate it :)

@moazDev1 moazDev1 requested a review from t-will-gillis July 15, 2024 01:10
Copy link
Member

@t-will-gillis t-will-gillis left a comment

Choose a reason for hiding this comment

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

Hey @moazDev1 Fantastic-I did a final test and it appears that everything is functioning as intended. As we discussed, there is a new token added to HfLA HACKFORLA_GRAPHQL_TOKEN with project and repo permissions, because based on the testing, these seem to be the permissions needed.

Thank you again for all of your work on this.

Details
  workflow number assignee post initial status final status initial label(s) final label(s) message
OK 1112 self same New Issue Approval same none n/a draft-label-reminder.md
OK 1114 self same New Issue Approval same Draft same prelim-update-comment.md
OK 1116 dummy unassigned New Issue Approval same none n/a unassign-from-nia.md
OK 1117 dummy unassigned Prioritized Backlog New Issue Approval none ready for dev lead multi-issue-reminder.md
OK 1121 dummy same In Progress same Feature: Agenda same prelim-update-comment.md
OK 1124 dummy same In Progress same Complex: Pre-work same prelim-update-comment.md
OK 1135 dummy same Emergent Requests same none n/a prelim-update-comment.md

@t-will-gillis t-will-gillis merged commit 828694e into hackforla:gh-pages Jul 15, 2024
@moazDev1 moazDev1 deleted the notify-multiple-issues-4505 branch July 29, 2024 02:10
@ExperimentsInHonesty ExperimentsInHonesty added the Lang: GHA GitHub Actions label Mar 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complexity: Extra Large Feature: Board/GitHub Maintenance Project board maintenance that we have to do repeatedly Feature: Refactor GHA Refactoring GitHub actions to fit latest architectural norms Lang: GHA GitHub Actions role: back end/devOps Tasks for back-end developers size: 8pt Can be done in 31-48 hours

Projects

Development

Successfully merging this pull request may close these issues.

GitHub Actions: Notify developers if they are working on multiple issues at once

4 participants