Skip to content

Fix object has no attribute error in Jira Notificator#958

Open
tomasdavidorg wants to merge 1 commit intoopenshift:mainfrom
tomasdavidorg:OCPERT-340
Open

Fix object has no attribute error in Jira Notificator#958
tomasdavidorg wants to merge 1 commit intoopenshift:mainfrom
tomasdavidorg:OCPERT-340

Conversation

@tomasdavidorg
Copy link
Contributor

@tomasdavidorg tomasdavidorg requested a review from rioliu-rh March 19, 2026 15:21
@openshift-ci openshift-ci bot requested review from LuboTerifaj and ming1013 March 19, 2026 15:21
@openshift-ci
Copy link

openshift-ci bot commented Mar 19, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign rioliu-rh for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai
Copy link

coderabbitai bot commented Mar 19, 2026

Walkthrough

This pull request refactors email access in the Jira notificator by introducing a centralized get_user_email() method to safely read user email addresses. It also updates Jira mention formatting from name-based to accountId-based references, changes a user search API parameter, and applies the new method across multiple functions.

Changes

Cohort / File(s) Summary
Core Implementation
oar/notificator/jira_notificator.py
Added NotificationService.get_user_email() method for safe email access. Updated Jira mention formatting from [~{u.name}] to [~accountId:{u.accountId}]. Changed user search parameter from user=email to query=email. Refactored four functions to use the new centralized email getter instead of direct emailAddress access.
Test Coverage
tests/test_jira_notificator.py
Added unit test test_get_user_email() to verify the new method returns email when present and None when absent.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.

OpenGrep is compatible with Semgrep configurations. Add an opengrep.yml or semgrep.yml configuration file to your project to enable OpenGrep analysis.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
tests/test_jira_notificator.py (2)

75-81: ⚠️ Potential issue | 🔴 Critical

Test expects old mention format, but production now uses accountId.

The production code changed to [~accountId:{u.accountId}] format, but this test still expects [~tdavid] (the .name format). The test mock also doesn't set accountId. This test will fail.

🐛 Fix test to match new format
     def test_create_jira_comment_mentions(self):
         second_user = Mock()
-        second_user.name = "gjospin"
+        second_user.accountId = "account-id-2"

         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:test-account-id] ")
+        self.assertEqual(self.ns.create_jira_comment_mentions([self.test_user, second_user]), "[~accountId:test-account-id] [~accountId:account-id-2] ")

Also add accountId to the test_user setup in setUp:

         self.test_user.name = "tdavid"
         self.test_user.displayName = "Tomas David"
         self.test_user.emailAddress = "tdavid@redhat.com"
+        self.test_user.accountId = "test-account-id"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_jira_notificator.py` around lines 75 - 81, The test
test_create_jira_comment_mentions expects the old name-based mention format but
production uses accountId; update the test to set accountId on the mocks (e.g.,
set self.test_user.accountId and second_user.accountId in setUp or within the
test) and change the asserted strings to the new format produced by
create_jira_comment_mentions (e.g., "[~accountId:{...}] " for each user) so the
expectations match the current create_jira_comment_mentions output.

140-163: ⚠️ Potential issue | 🔴 Critical

Multiple tests still expect the old [~name] mention format.

test_create_assignee_notification_text and several other tests expect strings like [~tdavid] [~gjospin], but production now outputs [~accountId:...]. These mocks need accountId attributes and assertions need updating to match the new format.

Affected tests: test_create_assignee_notification_text, test_notify_assignees, test_create_qa_notification_text, test_notify_qa_contact, test_create_team_lead_notification_text, test_notify_team_lead, test_create_manager_notification_text, test_notify_manager, test_check_issue_and_notify_responsible_people.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_jira_notificator.py` around lines 140 - 163, The tests (e.g.,
test_create_assignee_notification_text) still expect the old Jira mention format
"[~name]" while the production code now emits "[~accountId:...]", so update the
mocks and assertions: add an accountId attribute to the mock assignee/QA/team
lead/manager objects used in tests and change expected strings in assertions to
the new "[~accountId:...]" format (or build expected mentions dynamically from
the mock.accountId values) for the tested functions
create_assignee_notification_text, create_qa_notification_text,
create_team_lead_notification_text, create_manager_notification_text and the
notification tests notify_assignees, notify_qa_contact, notify_team_lead,
notify_manager, test_check_issue_and_notify_responsible_people.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@tests/test_jira_notificator.py`:
- Around line 75-81: The test test_create_jira_comment_mentions expects the old
name-based mention format but production uses accountId; update the test to set
accountId on the mocks (e.g., set self.test_user.accountId and
second_user.accountId in setUp or within the test) and change the asserted
strings to the new format produced by create_jira_comment_mentions (e.g.,
"[~accountId:{...}] " for each user) so the expectations match the current
create_jira_comment_mentions output.
- Around line 140-163: The tests (e.g., test_create_assignee_notification_text)
still expect the old Jira mention format "[~name]" while the production code now
emits "[~accountId:...]", so update the mocks and assertions: add an accountId
attribute to the mock assignee/QA/team lead/manager objects used in tests and
change expected strings in assertions to the new "[~accountId:...]" format (or
build expected mentions dynamically from the mock.accountId values) for the
tested functions create_assignee_notification_text, create_qa_notification_text,
create_team_lead_notification_text, create_manager_notification_text and the
notification tests notify_assignees, notify_qa_contact, notify_team_lead,
notify_manager, test_check_issue_and_notify_responsible_people.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a66225b0-2622-47eb-b8c7-ddae54d4cea4

📥 Commits

Reviewing files that changed from the base of the PR and between 8886a64 and b782d31.

📒 Files selected for processing (2)
  • oar/notificator/jira_notificator.py
  • tests/test_jira_notificator.py

@openshift-ci
Copy link

openshift-ci bot commented Mar 19, 2026

@tomasdavidorg: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@tomasdavidorg
Copy link
Contributor Author

🤔 need to fix the test cases. I will take a look tomorrow.

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.

1 participant