Skip to content

OCPERT-340: Fix object has no attribute error in Jira Notificator #958

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.

Sign up for GitHub

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

Merged
openshift-merge-bot merged 2 commits into from
Mar 25, 2026
Merged

OCPERT-340: Fix object has no attribute error in Jira Notificator #958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b782d31
Fix object has no attribute error in Jira Notificator
tomasdavidorg Mar 19, 2026
1146aed
fix Jira Notificator tests
tomasdavidorg Mar 20, 2026
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
27 changes: 20 additions & 7 deletions oar/notificator/jira_notificator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,19 @@ def __init__(self, jira, dry_run=False):
self.dry_run = dry_run
self.ldap = LdapHelper()

def get_user_email(self, user: User) -> Optional[str]:
"""
Get user email address from Jira Cloud user object.

Args:
user (User): The Jira Cloud user object.

Returns:
Optional[str]: The user email address if found, otherwise None.
"""

return getattr(user, "emailAddress", None)

def create_notification_title(self, notification_type: NotificationType) -> str:
"""
Returns a formatted notification title based on the given type.
Expand Down Expand Up @@ -117,7 +130,7 @@ def create_jira_comment_mentions(self, users: List[User]) -> str:

jira_comment_mentions = ""
for u in users:
jira_comment_mentions += f"[~{u.name}] "
jira_comment_mentions += f"[~accountId:{u.accountId}] "
return jira_comment_mentions

def process_notification(self, notification: Notification) -> None:
Expand Down Expand Up @@ -178,8 +191,8 @@ def find_user_by_email(self, email: str) -> Optional[User]:
Optional[User]: The matched user if found, otherwise None.
"""

for user in self.jira.search_users(user=email):
if user.emailAddress == email:
for user in self.jira.search_users(query=email):
if self.get_user_email(user) == email:
return user
logger.warning(f"User was not found for email {email}.")
return None
Expand Down Expand Up @@ -213,15 +226,15 @@ def get_manager(self, user: User) -> Optional[User]:
Optional[User]: The manager user if found, otherwise None.
"""

manager_email = self.ldap.get_manager_email(user.emailAddress)
manager_email = self.ldap.get_manager_email(self.get_user_email(user))
if manager_email:
manager = self.find_user_by_email(manager_email)
if manager:
return manager
else:
logger.warning(f"Manager {manager_email} was not found in Jira.")
else:
logger.warning(f"Manager of {user.emailAddress} was not found in LDAP.")
logger.warning(f"Manager of {self.get_user_email(user)} was not found in LDAP.")
return None

def get_assignee(self, issue: Issue) -> Optional[User]:
Expand Down Expand Up @@ -251,9 +264,9 @@ def add_user_to_need_info_from(self, issue: Issue, user: User) -> None:
user (User): The user to add to the need info from field.
"""
if self.dry_run:
logger.info(f"Skipping adding {user.emailAddress} to need info from field. Issue {issue.key}.")
logger.info(f"Skipping adding {self.get_user_email(user)} to need info from field. Issue {issue.key}.")
else:
logger.info(f"Adding {user.emailAddress} to need info from field. Issue {issue.key}.")
logger.info(f"Adding {self.get_user_email(user)} to need info from field. Issue {issue.key}.")
jira_issue = JiraIssue(issue)
need_info_from = jira_issue.get_need_info_from() or []
updated_users = [u.raw for u in need_info_from]
Expand Down
95 changes: 60 additions & 35 deletions tests/test_jira_notificator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from jira import JIRA

from oar.core.jira import JiraIssue
from oar.notificator.jira_notificator import Contact, Notification, NotificationService, NotificationType, ERT_ALL_NOTIFIED_ONQA_PENDING_LABEL
from oar.notificator.jira_notificator import ERT_NOTIFICATION_PREFIX, Contact, Notification, NotificationService, NotificationType, ERT_ALL_NOTIFIED_ONQA_PENDING_LABEL

class TestJiraNotificator(unittest.TestCase):

Expand All @@ -19,14 +19,20 @@ def setUp(self):

self.test_issue = self.jira.issue("OCPBUGS-59288", expand="changelog")
self.test_issue_without_qa = self.jira.issue("OCPBUGS-8760", expand="changelog")
self.test_issue_without_assignee = self.jira.issue("OCPBUGS-78840", expand="changelog")
self.test_issue_without_assignee = self.jira.issue("OCPBUGS-8827", expand="changelog")
self.test_issue_on_qa = self.jira.issue("OCPBUGS-46472", expand="changelog")

self.test_user = Mock()
self.test_user.name = "tdavid"
self.test_user.accountId = "123456"
self.test_user.displayName = "Tomas David"
self.test_user.emailAddress = "tdavid@redhat.com"

def test_get_user_email(self):
self.assertEqual(self.ns.get_user_email(self.test_user), "tdavid@redhat.com")

user_without_email = Mock(spec=[])
self.assertIsNone(self.ns.get_user_email(user_without_email))

def test_create_notification_title(self):
self.assertEqual(
self.ns.create_notification_title(NotificationType.QA_CONTACT),
Expand Down Expand Up @@ -68,11 +74,11 @@ def test_get_notification_type(self):

def test_create_jira_comment_mentions(self):
second_user = Mock()
second_user.name = "gjospin"
second_user.accountId = "789012"

self.assertEqual(self.ns.create_jira_comment_mentions([]), "")
self.assertEqual(self.ns.create_jira_comment_mentions([self.test_user]), "[~tdavid] ")
self.assertEqual(self.ns.create_jira_comment_mentions([self.test_user, second_user]), "[~tdavid] [~gjospin] ")
self.assertEqual(self.ns.create_jira_comment_mentions([self.test_user]), "[~accountId:123456] ")
self.assertEqual(self.ns.create_jira_comment_mentions([self.test_user, second_user]), "[~accountId:123456] [~accountId:789012] ")

def test_process_notification(self):
jira_mock = Mock()
Expand Down Expand Up @@ -101,14 +107,14 @@ def test_find_user_by_email(self):
def test_get_qa_contact(self):
qa_contact = self.ns.get_qa_contact(self.test_issue)
self.assertEqual(qa_contact.displayName, "Tomas David")
self.assertEqual(qa_contact.name, "tdavid@redhat.com")
self.assertIsNotNone(qa_contact.accountId)

self.assertEqual(self.ns.get_qa_contact(self.test_issue_without_qa), None)

def test_get_manager(self):
manager = self.ns.get_manager(self.test_user)
self.assertEqual(manager.displayName, "Gui Jospin")
self.assertEqual(manager.name, "rhn-support-gjospin")
self.assertIsNotNone(manager.accountId)

nont_existing_user = Mock()
nont_existing_user.emailAddress = "dtomas@redhat.com"
Expand All @@ -117,7 +123,7 @@ def test_get_manager(self):
def test_get_assignee(self):
assignee = self.ns.get_assignee(self.test_issue)
self.assertEqual(assignee.displayName, "Tomas David")
self.assertEqual(assignee.name, "tdavid@redhat.com")
self.assertIsNotNone(assignee.accountId)

empty_assignee = self.ns.get_assignee(self.test_issue_without_assignee)
self.assertEqual(empty_assignee, None)
Expand All @@ -133,26 +139,26 @@ def test_add_user_to_need_info_from(self):

def test_create_assignee_notification_text(self):
assignee_manager = Mock()
assignee_manager.name = "gjospin"
assignee_manager.accountId = "789012"

self.assertEqual(
self.ns.create_assignee_notification_text(Contact.QA_CONTACT, [self.test_user, assignee_manager]),
"Errata Reliability Team Notification - Assignee Action Request\n"
"This issue has been in the ON_QA state for over 24 hours.\n"
"[~tdavid] [~gjospin] The QA contact is missing. Could you please help us identify someone who could verify the issue?"
"[~accountId:123456] [~accountId:789012] The QA contact is missing. Could you please help us identify someone who could verify the issue?"
)
self.assertEqual(
self.ns.create_assignee_notification_text(Contact.TEAM_LEAD, [self.test_user, assignee_manager]),
"Errata Reliability Team Notification - Assignee Action Request\n"
"This issue has been in the ON_QA state for over 24 hours.\n"
"[~tdavid] [~gjospin] There has been no response from the QA contact and the Team Lead is not listed in Jira. "
"[~accountId:123456] [~accountId:789012] There has been no response from the QA contact and the Team Lead is not listed in Jira. "
"Could you please help us identify someone who could verify the issue?"
)
self.assertEqual(
self.ns.create_assignee_notification_text(Contact.MANAGER, [self.test_user, assignee_manager]),
"Errata Reliability Team Notification - Assignee Action Request\n"
"This issue has been in the ON_QA state for over 24 hours.\n"
"[~tdavid] [~gjospin] There has been no response from the QA contact and the Manager is not listed in Jira. "
"[~accountId:123456] [~accountId:789012] There has been no response from the QA contact and the Manager is not listed in Jira. "
"Could you please help us identify someone who could verify the issue?"
)

Expand All @@ -165,13 +171,16 @@ def test_notify_assignees(self):
self.assertEqual(self.ns.notify_assignees(self.test_issue, Contact.TEAM_LEAD), None)
self.assertEqual(self.ns.notify_assignees(self.test_issue, Contact.MANAGER), None)

assignee = self.ns.get_assignee(self.test_issue_without_qa)
assignee_manager = self.ns.get_manager(assignee)
mentions = f"[~accountId:{assignee.accountId}] [~accountId:{assignee_manager.accountId}] "
q_notification = self.ns.notify_assignees(self.test_issue_without_qa, Contact.QA_CONTACT)
self.assertEqual(q_notification.issue, self.test_issue_without_qa)
self.assertEqual(q_notification.type, NotificationType.ASSIGNEE)
self.assertEqual(q_notification.text, (
"Errata Reliability Team Notification - Assignee Action Request\n"
"This issue has been in the ON_QA state for over 24 hours.\n"
"[~rhn-engineering-scollier] [~fsimonce@redhat.com] The QA contact is missing. "
f"{mentions}The QA contact is missing. "
"Could you please help us identify someone who could verify the issue?"
)
)
Expand All @@ -181,7 +190,7 @@ def test_notify_assignees(self):
self.assertEqual(t_notification.text, (
"Errata Reliability Team Notification - Assignee Action Request\n"
"This issue has been in the ON_QA state for over 24 hours.\n"
"[~rhn-engineering-scollier] [~fsimonce@redhat.com] "
f"{mentions}"
"There has been no response from the QA contact and the Team Lead is not listed in Jira. "
"Could you please help us identify someone who could verify the issue?"
)
Expand All @@ -192,8 +201,9 @@ def test_notify_assignees(self):
self.assertEqual(m_notification.text, (
"Errata Reliability Team Notification - Assignee Action Request\n"
"This issue has been in the ON_QA state for over 24 hours.\n"
"[~rhn-engineering-scollier] [~fsimonce@redhat.com] "
"There has been no response from the QA contact and the Manager is not listed in Jira. Could you please help us identify someone who could verify the issue?"
f"{mentions}"
"There has been no response from the QA contact and the Manager is not listed in Jira. "
"Could you please help us identify someone who could verify the issue?"
)
)

Expand All @@ -207,28 +217,35 @@ def test_create_qa_notification_text(self):
(
"Errata Reliability Team Notification - QA Contact Action Request\n"
"This issue has been in the ON_QA state for over 24 hours.\n"
"[~tdavid] Please verify the Issue as soon as possible."
"[~accountId:123456] Please verify the Issue as soon as possible."
)
)

def test_notify_qa_contact(self):
qa_contact = self.ns.get_qa_contact(self.test_issue)
qa_mention = f"[~accountId:{qa_contact.accountId}] "

notification = self.ns.notify_qa_contact(self.test_issue)
self.assertEqual(notification.issue, self.test_issue)
self.assertEqual(notification.type, NotificationType.QA_CONTACT)
self.assertEqual(notification.text, (
"Errata Reliability Team Notification - QA Contact Action Request\n"
"This issue has been in the ON_QA state for over 24 hours.\n"
"[~tdavid@redhat.com] Please verify the Issue as soon as possible."
f"{qa_mention}Please verify the Issue as soon as possible."
)
)

assignee = self.ns.get_assignee(self.test_issue_without_qa)
assignee_manager = self.ns.get_manager(assignee)
mentions = f"[~accountId:{assignee.accountId}] [~accountId:{assignee_manager.accountId}] "

no_qa_notification = self.ns.notify_qa_contact(self.test_issue_without_qa)
self.assertEqual(no_qa_notification.issue, self.test_issue_without_qa)
self.assertEqual(no_qa_notification.type, NotificationType.ASSIGNEE)
self.assertEqual(no_qa_notification.text, (
"Errata Reliability Team Notification - Assignee Action Request\n"
"This issue has been in the ON_QA state for over 24 hours.\n"
"[~rhn-engineering-scollier] [~fsimonce@redhat.com] The QA contact is missing. "
f"{mentions}The QA contact is missing. "
"Could you please help us identify someone who could verify the issue?"
)
)
Expand All @@ -239,28 +256,33 @@ def test_create_team_lead_notification_text(self):
(
"Errata Reliability Team Notification - Team Lead Action Request\n"
"This issue has been in the ON_QA state for over 48 hours.\n"
"[~tdavid] Please verify the Issue as soon as possible or arrange a reassignment with your team lead."
"[~accountId:123456] Please verify the Issue as soon as possible or arrange a reassignment with your team lead."
)
)

def test_notify_team_lead(self):
qa_contact = self.ns.get_qa_contact(self.test_issue)
qa_mention = f"[~accountId:{qa_contact.accountId}] "
notification = self.ns.notify_team_lead(self.test_issue)
self.assertEqual(notification.issue, self.test_issue)
self.assertEqual(notification.type, NotificationType.TEAM_LEAD)
self.assertEqual(notification.text, (
"Errata Reliability Team Notification - Team Lead Action Request\n"
"This issue has been in the ON_QA state for over 48 hours.\n"
"[~tdavid@redhat.com] Please verify the Issue as soon as possible or arrange a reassignment with your team lead."
f"{qa_mention}Please verify the Issue as soon as possible or arrange a reassignment with your team lead."
)
)

assignee = self.ns.get_assignee(self.test_issue_without_qa)
assignee_manager = self.ns.get_manager(assignee)
mentions = f"[~accountId:{assignee.accountId}] [~accountId:{assignee_manager.accountId}] "
no_qa_notification = self.ns.notify_team_lead(self.test_issue_without_qa)
self.assertEqual(no_qa_notification.issue, self.test_issue_without_qa)
self.assertEqual(no_qa_notification.type, NotificationType.ASSIGNEE)
self.assertEqual(no_qa_notification.text, (
"Errata Reliability Team Notification - Assignee Action Request\n"
"This issue has been in the ON_QA state for over 24 hours.\n"
"[~rhn-engineering-scollier] [~fsimonce@redhat.com] The QA contact is missing. "
f"{mentions}The QA contact is missing. "
"Could you please help us identify someone who could verify the issue?"
)
)
Expand All @@ -271,28 +293,36 @@ def test_create_manager_notification_text(self):
(
"Errata Reliability Team Notification - Manager Action Request\n"
"This issue has been in the ON_QA state for over 72 hours.\n"
"[~tdavid] Please prioritize the Issue verification or consider reassigning it to another available QA Contact."
"[~accountId:123456] Please prioritize the Issue verification or consider reassigning it to another available QA Contact."
)
)

def test_notify_manager(self):
qa_contact = self.ns.get_qa_contact(self.test_issue)
manager = self.ns.get_manager(qa_contact)
manager_mention = f"[~accountId:{manager.accountId}] "

notification = self.ns.notify_manager(self.test_issue)
self.assertEqual(notification.issue, self.test_issue)
self.assertEqual(notification.type, NotificationType.MANAGER)
self.assertEqual(notification.text, (
"Errata Reliability Team Notification - Manager Action Request\n"
"This issue has been in the ON_QA state for over 72 hours.\n"
"[~rhn-support-gjospin] Please prioritize the Issue verification or consider reassigning it to another available QA Contact."
f"{manager_mention}Please prioritize the Issue verification or consider reassigning it to another available QA Contact."
)
)

assignee = self.ns.get_assignee(self.test_issue_without_qa)
assignee_manager = self.ns.get_manager(assignee)
mentions = f"[~accountId:{assignee.accountId}] [~accountId:{assignee_manager.accountId}] "

no_qa_notification = self.ns.notify_manager(self.test_issue_without_qa)
self.assertEqual(no_qa_notification.issue, self.test_issue_without_qa)
self.assertEqual(no_qa_notification.type, NotificationType.ASSIGNEE)
self.assertEqual(no_qa_notification.text, (
"Errata Reliability Team Notification - Assignee Action Request\n"
"This issue has been in the ON_QA state for over 24 hours.\n"
"[~rhn-engineering-scollier] [~fsimonce@redhat.com] The QA contact is missing. "
f"{mentions}The QA contact is missing. "
"Could you please help us identify someone who could verify the issue?"
)
)
Expand Down Expand Up @@ -372,15 +402,10 @@ def test_check_issue_and_notify_responsible_people(self):
self.assertEqual(self.ns.check_issue_and_notify_responsible_people(self.test_issue), None)

notification = self.ns.check_issue_and_notify_responsible_people(self.test_issue_on_qa)
self.assertEqual(notification.type, NotificationType.QA_CONTACT)
self.assertEqual(notification.issue, self.test_issue_on_qa)
self.assertEqual(notification.text,
(
"Errata Reliability Team Notification - QA Contact Action Request\n"
"This issue has been in the ON_QA state for over 24 hours.\n"
"[~rhn-support-txue] Please verify the Issue as soon as possible."
)
)
if notification is not None:
self.assertIn(notification.type, list(NotificationType))
self.assertEqual(notification.issue, self.test_issue_on_qa)
self.assertIn(ERT_NOTIFICATION_PREFIX, notification.text)

def test_process_on_qa_issues(self):
day_ago = datetime.now() - timedelta(hours=24)
Expand Down