Skip to content

Leave downstream assignment to owner when upstream is clear #380

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
ralphbean merged 1 commit into from
Oct 29, 2025
Merged

Leave downstream assignment to owner when upstream is clear #380

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 8 additions & 4 deletions sync2jira/downstream_issue.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ def assign_user(
log.warning(
"Unable to assign %s from upstream assignees %s in %s",
downstream.key,
str([a.get("fullname", "<no-fullname>") for a in issue.assignee]),
str([a.get("fullname", a.get("login", "<name>")) for a in issue.assignee]),
issue.url,
)

Expand Down Expand Up @@ -906,15 +906,19 @@ def _update_assignee(client, existing, issue, overwrite):
existing.fields.assignee, "displayName"
)
if overwrite:
if ds_exists and us_exists:
if not ds_exists:
# Let assign_user() figure out what to do.
update = True
elif us_exists:
# Overwrite the downstream assignment only if it is different from
# the upstream one.
un = issue.assignee[0]["fullname"]
dn = existing.fields.assignee.displayName
update = un != dn and remove_diacritics(un) != dn
else:
# Let assign_user() figure out what to do.
update = True
# Without an upstream owner, update only if the downstream is not
# assigned to the project owner.
update = issue.downstream.get("owner") != existing.fields.assignee.name
else:
# We're not overwriting, so call assign_user() only if the downstream
# doesn't already have an assignment.
Expand Down
132 changes: 94 additions & 38 deletions tests/test_downstream_issue.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1092,44 +1092,97 @@ def test_update_assignee_all(self, mock_client, mock_assign_user):
expected_results = iter(
(
# - overwrite = True
# - downstream assignee exists
None, # upstream assignee exists and assignments are equal: not called
None, # upstream assignee exists and assignments differ only in diacritics: not called
False, # upstream assignee exists and assignments are different: called with remove_all=False
True, # upstream assignee has a fullname of None: called with remove_all=True
True, # upstream assignee does not exist: called with remove_all=True
True, # upstream assignee is an empty list: called with remove_all=True
# - downstream assignee is set
# - upstream assignee exists and assignments are equal: not called
(11, None),
# - upstream assignee exists and assignments differ only in diacritics: not called
(12, None),
# - upstream assignee exists and assignments are different: called with remove_all=False
(13, False),
# - upstream assignee has a fullname of None: called with remove_all=True
(14, True),
# - upstream assignee does not exist: called with remove_all=True
(15, True),
# - upstream assignee is an empty list: called with remove_all=True
(16, True),
# - downstream assignee is owner
# - upstream assignee exists and assignments are different: called with remove_all=False
(21, False),
# - upstream assignee exists and assignments are different: called with remove_all=False
(22, False),
# - upstream assignee exists and assignments are different: called with remove_all=False
(23, False),
# - upstream assignee has a fullname of None: not called (already assigned to owner)
(24, None),
# - upstream assignee does not exist: not called (already assigned to owner)
(25, None),
# - upstream assignee is an empty list: not called (already assigned to owner)
(26, None),
# - downstream assignee does not exist
False, # upstream assignee exists: called with remove_all=False
False, # upstream assignee exists: called with remove_all=False
False, # upstream assignee exists: called with remove_all=False
False, # upstream assignee has a fullname of None: called with remove_all=False
False, # upstream assignee does not exist: called with remove_all=False
False, # upstream assignee is an empty list: called with remove_all=False
# - upstream assignee exists: called with remove_all=False
(31, False),
# - upstream assignee exists: called with remove_all=False
(32, False),
# - upstream assignee exists: called with remove_all=False
(33, False),
# - upstream assignee has a fullname of None: called with remove_all=False
(34, False),
# - upstream assignee does not exist: called with remove_all=False
(35, False),
# - upstream assignee is an empty list: called with remove_all=False
(36, False),
# - overwrite = False
# - downstream assignee exists:
None, # upstream assignee exists and assignments are equal: not called
None, # upstream assignee exists and assignments differ only in diacritics: not called
None, # upstream assignee exists and assignments are different: not called
None, # upstream assignee has a fullname of None: not called
None, # upstream assignee does not exist: not called
None, # upstream assignee is an empty list: not called
# - downstream assignee is set:
# - upstream assignee exists and assignments are equal: not called
(41, None),
# - upstream assignee exists and assignments differ only in diacritics: not called
(42, None),
# - upstream assignee exists and assignments are different: not called
(43, None),
# - upstream assignee has a fullname of None: not called
(44, None),
# - upstream assignee does not exist: not called
(45, None),
# - upstream assignee is an empty list: not called
(46, None),
# - downstream assignee is owner
# - upstream assignee exists and assignments are different: not called
(51, None),
# - upstream assignee exists and assignments are different: not called
(52, None),
# - upstream assignee exists and assignments are different: not called
(53, None),
# - upstream assignee has a fullname of None: not called
(54, None),
# - upstream assignee does not exist: not called
(55, None),
# - upstream assignee is an empty list: not called
(56, None),
# - downstream assignee does not exist
False, # upstream assignee exists: called with remove_all=False
False, # upstream assignee exists: called with remove_all=False
False, # upstream assignee exists: called with remove_all=False
False, # upstream assignee has a fullname of None: called with remove_all=False
False, # upstream assignee does not exist: called with remove_all=False
False, # upstream assignee is an empty list: called with remove_all=False
# - upstream assignee exists: called with remove_all=False
(61, False),
# - upstream assignee exists: called with remove_all=False
(62, False),
# - upstream assignee exists: called with remove_all=False
(63, False),
# - upstream assignee has a fullname of None: called with remove_all=False
(64, False),
# - upstream assignee does not exist: called with remove_all=False
(65, False),
# - upstream assignee is an empty list: called with remove_all=False
(66, False),
)
)
match = "Erik"
owner = self.mock_issue.downstream["owner"]
for overwrite in (True, False):
for ds in (match, None):
for ds in (match, owner, None):
if ds is None:
delattr(self.mock_downstream.fields.assignee, "displayName")
delattr(self.mock_downstream.fields.assignee, "name")
else:
setattr(self.mock_downstream.fields.assignee, "displayName", match)
setattr(self.mock_downstream.fields.assignee, "displayName", ds)
setattr(self.mock_downstream.fields.assignee, "name", ds)

for us in (
[{"fullname": match}],
Expand All @@ -1140,6 +1193,7 @@ def test_update_assignee_all(self, mock_client, mock_assign_user):
[],
):
self.mock_issue.assignee = us
scenario, expected_result = next(expected_results)

d._update_assignee(
client=mock_client,
Expand All @@ -1149,16 +1203,18 @@ def test_update_assignee_all(self, mock_client, mock_assign_user):
)

# Check that the call was made correctly
expected_result = next(expected_results)
if expected_result is None:
mock_assign_user.assert_not_called()
else:
mock_assign_user.assert_called_with(
mock_client,
self.mock_issue,
self.mock_downstream,
remove_all=expected_result,
)
try:
if expected_result is None:
mock_assign_user.assert_not_called()
else:
mock_assign_user.assert_called_with(
mock_client,
self.mock_issue,
self.mock_downstream,
remove_all=expected_result,
)
except AssertionError as e:
raise AssertionError(f"Failed scenario {scenario}: {e}")
mock_assign_user.reset_mock()

@mock.patch(PATH + "verify_tags")
Expand Down