Skip to content

Commit 9fc7a4b

Browse files
webbnhralphbean
webbnh
authored and
ralphbean
committed
Leave downstream assignment to owner when upstream is clear
1 parent fb908a3 commit 9fc7a4b

File tree

2 files changed

+102
-42
lines changed

2 files changed

+102
-42
lines changed

‎sync2jira/downstream_issue.py‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ def assign_user(
449449
log.warning(
450450
"Unable to assign %s from upstream assignees %s in %s",
451451
downstream.key,
452-
str([a.get("fullname", "<no-fullname>") for a in issue.assignee]),
452+
str([a.get("fullname", a.get("login", "<name>")) for a in issue.assignee]),
453453
issue.url,
454454
)
455455

@@ -906,15 +906,19 @@ def _update_assignee(client, existing, issue, overwrite):
906906
existing.fields.assignee, "displayName"
907907
)
908908
if overwrite:
909-
if ds_exists and us_exists:
909+
if not ds_exists:
910+
# Let assign_user() figure out what to do.
911+
update = True
912+
elif us_exists:
910913
# Overwrite the downstream assignment only if it is different from
911914
# the upstream one.
912915
un = issue.assignee[0]["fullname"]
913916
dn = existing.fields.assignee.displayName
914917
update = un != dn and remove_diacritics(un) != dn
915918
else:
916-
# Let assign_user() figure out what to do.
917-
update = True
919+
# Without an upstream owner, update only if the downstream is not
920+
# assigned to the project owner.
921+
update = issue.downstream.get("owner") != existing.fields.assignee.name
918922
else:
919923
# We're not overwriting, so call assign_user() only if the downstream
920924
# doesn't already have an assignment.

‎tests/test_downstream_issue.py‎

Lines changed: 94 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1092,44 +1092,97 @@ def test_update_assignee_all(self, mock_client, mock_assign_user):
10921092
expected_results = iter(
10931093
(
10941094
# - overwrite = True
1095-
# - downstream assignee exists
1096-
None, # upstream assignee exists and assignments are equal: not called
1097-
None, # upstream assignee exists and assignments differ only in diacritics: not called
1098-
False, # upstream assignee exists and assignments are different: called with remove_all=False
1099-
True, # upstream assignee has a fullname of None: called with remove_all=True
1100-
True, # upstream assignee does not exist: called with remove_all=True
1101-
True, # upstream assignee is an empty list: called with remove_all=True
1095+
# - downstream assignee is set
1096+
# - upstream assignee exists and assignments are equal: not called
1097+
(11, None),
1098+
# - upstream assignee exists and assignments differ only in diacritics: not called
1099+
(12, None),
1100+
# - upstream assignee exists and assignments are different: called with remove_all=False
1101+
(13, False),
1102+
# - upstream assignee has a fullname of None: called with remove_all=True
1103+
(14, True),
1104+
# - upstream assignee does not exist: called with remove_all=True
1105+
(15, True),
1106+
# - upstream assignee is an empty list: called with remove_all=True
1107+
(16, True),
1108+
# - downstream assignee is owner
1109+
# - upstream assignee exists and assignments are different: called with remove_all=False
1110+
(21, False),
1111+
# - upstream assignee exists and assignments are different: called with remove_all=False
1112+
(22, False),
1113+
# - upstream assignee exists and assignments are different: called with remove_all=False
1114+
(23, False),
1115+
# - upstream assignee has a fullname of None: not called (already assigned to owner)
1116+
(24, None),
1117+
# - upstream assignee does not exist: not called (already assigned to owner)
1118+
(25, None),
1119+
# - upstream assignee is an empty list: not called (already assigned to owner)
1120+
(26, None),
11021121
# - downstream assignee does not exist
1103-
False, # upstream assignee exists: called with remove_all=False
1104-
False, # upstream assignee exists: called with remove_all=False
1105-
False, # upstream assignee exists: called with remove_all=False
1106-
False, # upstream assignee has a fullname of None: called with remove_all=False
1107-
False, # upstream assignee does not exist: called with remove_all=False
1108-
False, # upstream assignee is an empty list: called with remove_all=False
1122+
# - upstream assignee exists: called with remove_all=False
1123+
(31, False),
1124+
# - upstream assignee exists: called with remove_all=False
1125+
(32, False),
1126+
# - upstream assignee exists: called with remove_all=False
1127+
(33, False),
1128+
# - upstream assignee has a fullname of None: called with remove_all=False
1129+
(34, False),
1130+
# - upstream assignee does not exist: called with remove_all=False
1131+
(35, False),
1132+
# - upstream assignee is an empty list: called with remove_all=False
1133+
(36, False),
11091134
# - overwrite = False
1110-
# - downstream assignee exists:
1111-
None, # upstream assignee exists and assignments are equal: not called
1112-
None, # upstream assignee exists and assignments differ only in diacritics: not called
1113-
None, # upstream assignee exists and assignments are different: not called
1114-
None, # upstream assignee has a fullname of None: not called
1115-
None, # upstream assignee does not exist: not called
1116-
None, # upstream assignee is an empty list: not called
1135+
# - downstream assignee is set:
1136+
# - upstream assignee exists and assignments are equal: not called
1137+
(41, None),
1138+
# - upstream assignee exists and assignments differ only in diacritics: not called
1139+
(42, None),
1140+
# - upstream assignee exists and assignments are different: not called
1141+
(43, None),
1142+
# - upstream assignee has a fullname of None: not called
1143+
(44, None),
1144+
# - upstream assignee does not exist: not called
1145+
(45, None),
1146+
# - upstream assignee is an empty list: not called
1147+
(46, None),
1148+
# - downstream assignee is owner
1149+
# - upstream assignee exists and assignments are different: not called
1150+
(51, None),
1151+
# - upstream assignee exists and assignments are different: not called
1152+
(52, None),
1153+
# - upstream assignee exists and assignments are different: not called
1154+
(53, None),
1155+
# - upstream assignee has a fullname of None: not called
1156+
(54, None),
1157+
# - upstream assignee does not exist: not called
1158+
(55, None),
1159+
# - upstream assignee is an empty list: not called
1160+
(56, None),
11171161
# - downstream assignee does not exist
1118-
False, # upstream assignee exists: called with remove_all=False
1119-
False, # upstream assignee exists: called with remove_all=False
1120-
False, # upstream assignee exists: called with remove_all=False
1121-
False, # upstream assignee has a fullname of None: called with remove_all=False
1122-
False, # upstream assignee does not exist: called with remove_all=False
1123-
False, # upstream assignee is an empty list: called with remove_all=False
1162+
# - upstream assignee exists: called with remove_all=False
1163+
(61, False),
1164+
# - upstream assignee exists: called with remove_all=False
1165+
(62, False),
1166+
# - upstream assignee exists: called with remove_all=False
1167+
(63, False),
1168+
# - upstream assignee has a fullname of None: called with remove_all=False
1169+
(64, False),
1170+
# - upstream assignee does not exist: called with remove_all=False
1171+
(65, False),
1172+
# - upstream assignee is an empty list: called with remove_all=False
1173+
(66, False),
11241174
)
11251175
)
11261176
match = "Erik"
1177+
owner = self.mock_issue.downstream["owner"]
11271178
for overwrite in (True, False):
1128-
for ds in (match, None):
1179+
for ds in (match, owner, None):
11291180
if ds is None:
11301181
delattr(self.mock_downstream.fields.assignee, "displayName")
1182+
delattr(self.mock_downstream.fields.assignee, "name")
11311183
else:
1132-
setattr(self.mock_downstream.fields.assignee, "displayName", match)
1184+
setattr(self.mock_downstream.fields.assignee, "displayName", ds)
1185+
setattr(self.mock_downstream.fields.assignee, "name", ds)
11331186

11341187
for us in (
11351188
[{"fullname": match}],
@@ -1140,6 +1193,7 @@ def test_update_assignee_all(self, mock_client, mock_assign_user):
11401193
[],
11411194
):
11421195
self.mock_issue.assignee = us
1196+
scenario, expected_result = next(expected_results)
11431197

11441198
d._update_assignee(
11451199
client=mock_client,
@@ -1149,16 +1203,18 @@ def test_update_assignee_all(self, mock_client, mock_assign_user):
11491203
)
11501204

11511205
# Check that the call was made correctly
1152-
expected_result = next(expected_results)
1153-
if expected_result is None:
1154-
mock_assign_user.assert_not_called()
1155-
else:
1156-
mock_assign_user.assert_called_with(
1157-
mock_client,
1158-
self.mock_issue,
1159-
self.mock_downstream,
1160-
remove_all=expected_result,
1161-
)
1206+
try:
1207+
if expected_result is None:
1208+
mock_assign_user.assert_not_called()
1209+
else:
1210+
mock_assign_user.assert_called_with(
1211+
mock_client,
1212+
self.mock_issue,
1213+
self.mock_downstream,
1214+
remove_all=expected_result,
1215+
)
1216+
except AssertionError as e:
1217+
raise AssertionError(f"Failed scenario {scenario}: {e}")
11621218
mock_assign_user.reset_mock()
11631219

11641220
@mock.patch(PATH + "verify_tags")

0 commit comments

Comments
 (0)