correct handling of notifications if member domain has no admins

[havetisyan]

Description

addresses bug #3111

Contribution Checklist:

• The pull request does not introduce any breaking changes
• I have read the contribution guidelines.
• Create an issue and link to the pull request.

Attach Screenshots (Optional)

[gemini-code-assist]

Summary of Changes

Hello @havetisyan, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves a critical bug in the notification system where group and role member expiry reminders were not correctly handled for domains that lacked explicit administrators. The changes ensure that even in such scenarios, the relevant role administrators receive their notifications, improving the reliability and completeness of the Athenz notification service.

Highlights

• Bug Fix: Notification Handling for Domains Without Admins: Addressed a bug where notification processing for group and role member expiry reminders would fail or be skipped if the associated member domain did not have any direct administrators.
• Introduction of _NOTIFY-SKIP-PRINCIPAL_: A special internal principal, _NOTIFY-SKIP-PRINCIPAL_, has been introduced. This principal is used as a placeholder when a domain has no direct administrators, allowing the notification consolidation process to continue and ensure that role administrators still receive relevant notifications.
• Refined Notification Generation Logic: The logic for generating actual notification objects has been updated to explicitly check for and skip the _NOTIFY-SKIP-PRINCIPAL_, preventing the creation of notifications for this internal placeholder while ensuring notifications for actual role admins are processed.
• Comprehensive Test Coverage: New test cases have been added to GroupMemberExpiryNotificationTaskTest and RoleMemberNotificationCommonTest to validate the correct behavior of notification processing when domains lack administrators, ensuring the fix works as expected.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request addresses an issue where notifications were not being sent for members in domains without administrators. The approach of using a special _NOTIFY-SKIP-PRINCIPAL_ to ensure role/group admin notifications are still processed is clever.

However, the implementation appears to be incomplete. The fix is only applied when notifications are consolidated by principal (ConsolidatedBy.PRINCIPAL). The corresponding methods for domain-based consolidation (consolidateRoleMembersByDomain and consolidateGroupMembersByDomain) have not been updated with the same logic. This means the original bug likely still exists in that scenario. A test case in RoleMemberNotificationCommonTest.java confirms this discrepancy in behavior.

Additionally, the NOTIFY_SKIP_PRINCIPAL constant is duplicated in two separate classes, which should be centralized for better maintainability.