Skip to content

Conversation

@simonLeary42
Copy link
Member

@simonLeary42 simonLeary42 commented Jan 28, 2026

Uses the same plus addressing scheme as unity-course.php, so the owner mail for a course group is identical to one of the manager mails.

image

The current addressing scheme omits the organization, so name conflicts are possible though unlikely. For courses this is fine, but if we start letting normal PI groups have managers, name conflicts will become more likely. If someday we update PI groups so that one user can own multiple groups, we will have to solve this problem.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates group email notifications so that both group owners and group managers receive key lifecycle emails (creation, approval/denial, membership changes, etc.).

Changes:

  • Updated all relevant UnityGroup methods to send notification emails to the owner and all managers using a new helper, getOwnerAndManagerMails().
  • Adjusted the context data for PI group request handling to include owner/manager emails and reused the new helper in multiple notification flows.
  • Added a private utility method in UnityGroup to collect, deduplicate, and sort the owner’s and managers’ email addresses.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@simonLeary42 simonLeary42 removed the request for review from bryank-cs January 28, 2026 17:29
@simonLeary42 simonLeary42 marked this pull request as draft January 28, 2026 17:29
@simonLeary42 simonLeary42 marked this pull request as ready for review January 28, 2026 17:41
@simonLeary42 simonLeary42 merged commit 7eca19c into main Jan 29, 2026
3 checks passed
@simonLeary42 simonLeary42 deleted the mail-managers branch January 29, 2026 15:43
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