Skip to content

Commit

Permalink
Pick lint from downstream_issue.py (#262)
Browse files Browse the repository at this point in the history
  • Loading branch information
@webbnh
webbnh authored Dec 9, 2024
1 parent cbe4e6d commit 0b2592c
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 43 deletions.
90 changes: 48 additions & 42 deletions sync2jira/downstream_issue.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import logging
import operator
import re
from typing import Optional

# 3rd Party Modules
import arrow
Expand Down Expand Up @@ -140,10 +141,10 @@ def _matching_jira_issue_query(client, issue, config, free=False):
if free:
query += ' and statusCategory != Done'
# Query the JIRA client and store the results
results_of_query = client.search_issues(query)
results_of_query: jira.client.ResultList = client.search_issues(query)
if len(results_of_query) > 1:
final_results = []
# TODO: there is pagure-specifc code in here that handles the case where a dropped issue's URL is
# TODO: there is pagure-specific code in here that handles the case where a dropped issue's URL is
# re-used by an issue opened later. i.e. pagure re-uses IDs
for result in results_of_query:
description = result.fields.description or ""
Expand All @@ -156,9 +157,9 @@ def _matching_jira_issue_query(client, issue, config, free=False):
else:
# Else search returned a linked issue
final_results.append(search)
# If that's not the case, check if they have the same upstream title
# Upstream username/repo can change if repos are merged
elif re.search(r"\[[a-zA-Z0-9!@#$%^&*()_+\-=\[\]{};':\\|,.<>\/?]*\] "
# If that's not the case, check if they have the same upstream title.
# Upstream username/repo can change if repos are merged.
elif re.search(r"\[[a-zA-Z0-9!@#$%^&*()_+\-=\[\]{};':\\|,.<>/?]*] "
+ issue.upstream_title,
result.fields.summary):
search = check_comments_for_duplicate(client, result,
Expand Down Expand Up @@ -279,10 +280,10 @@ def alert_user_of_duplicate_issues(issue, final_result, results_of_query,
admin_template.append({'name': ret[0].displayName, 'email': ret[0].emailAddress})

# Create and send email
templateLoader = jinja2.FileSystemLoader(
template_loader = jinja2.FileSystemLoader(
searchpath='usr/local/src/sync2jira/sync2jira/')
templateEnv = jinja2.Environment(loader=templateLoader, autoescape=True)
template = templateEnv.get_template('email_template.jinja')
template_env = jinja2.Environment(loader=template_loader, autoescape=True)
template = template_env.get_template('email_template.jinja')
html_text = template.render(user=user,
admins=admin_template,
issue=issue,
Expand Down Expand Up @@ -321,7 +322,7 @@ def check_comments_for_duplicate(client, result, username):
Checks comment of JIRA issue to see if it has been
marked as a duplicate.
:param jira.client.JIRA client: JIRA client)
:param jira.client.JIRA client: JIRA client
:param jira.resource.Issue result: JIRA issue
:param string username: Username of JIRA user
:returns: True if duplicate comment was not found or JIRA issue if \
Expand Down Expand Up @@ -354,7 +355,7 @@ def _find_comment_in_jira(comment, j_comments):
# return the item
return item
if str(comment['id']) in item.raw['body']:
# The comment id's match, if they dont have the same body,
# The comment id's match, if they don't have the same body,
# we need to edit the comment
if item.raw['body'] != formatted_comment:
# We need to update the comment
Expand Down Expand Up @@ -407,7 +408,7 @@ def _get_existing_jira_issue(client, issue, config):
return None


def _get_existing_jira_issue_legacy(client, issue, config):
def _get_existing_jira_issue_legacy(client, issue):
"""
This is our old way of matching issues: use the special url field.
This will be phased out and removed in a future release.
Expand Down Expand Up @@ -441,7 +442,7 @@ def attach_link(client, downstream, remote_link):
modified_desc = downstream.fields.description + " "

# This is crazy. Querying for application links requires admin perms which
# we don't have, so duckpunch the client to think it has already made the
# we don't have, so duck-punch the client to think it has already made the
# query.
client._applicationlinks = [] # pylint: disable=protected-access

Expand Down Expand Up @@ -475,7 +476,7 @@ def _upgrade_jira_issue(client, downstream, issue, config):

def assign_user(client, issue, downstream, remove_all=False):
"""
Attempts to assigns a JIRA issue to the correct
Attempts to assign a JIRA issue to the correct
user based on the issue.
:param jira.client.JIRA client: JIRA Client
Expand All @@ -492,14 +493,14 @@ def assign_user(client, issue, downstream, remove_all=False):
return

# JIRA only supports one assignee
# If we have more than one assignee (i.e. from Github)
# If we have more than one assignee (i.e. from GitHub)
# assign the issue to the first user (i.e. issue.assignee[0])

# First we need to find the user

# See if any of the upstream users has full names available. Not all do.
def assignee_fullname(issue):
for assignee in issue.assignee:
def assignee_fullname(u_issue):
for assignee in u_issue.assignee:
if assignee['fullname']:
return assignee['fullname']
return None
Expand Down Expand Up @@ -540,14 +541,14 @@ def change_status(client, downstream, status, issue):
:param sync2jira.intermediary.Issue issue: Issue object
"""
transitions = client.transitions(downstream)
id = ''
tid = ''
for t in transitions:
if t['name'] and status.upper() == str(t['name']).upper():
id = int(t['id'])
tid = int(t['id'])
break
if id:
if tid:
try:
client.transition_issue(downstream, id)
client.transition_issue(downstream, tid)
log.info('Updated downstream to %s status for issue %s',
status, issue.title)
except JIRAError:
Expand Down Expand Up @@ -577,8 +578,8 @@ def _get_preferred_issue_types(config, issue):
# }
type_list = []

map = config['sync2jira'].get('map', {})
conf = map.get('github', {}).get(issue.upstream, {})
cmap = config['sync2jira'].get('map', {})
conf = cmap.get('github', {}).get(issue.upstream, {})

# we consider the issue_types mapping if it exists. If it does, exclude all other logic.
if 'issue_types' in conf:
Expand Down Expand Up @@ -654,7 +655,7 @@ def _create_jira_issue(client, issue, config):
name_map = {field['name']: field['id'] for field in all_fields}
if issue.downstream.get('epic-link'):
# Try to get and update the custom field
custom_field = name_map.get('Epic Link')
custom_field: Optional[str] = name_map.get('Epic Link')
if custom_field:
try:
downstream.update({custom_field: issue.downstream['epic-link']})
Expand Down Expand Up @@ -724,7 +725,7 @@ def _label_matching(jira_labels, issue_labels):

def _update_jira_issue(existing, issue, client, config):
"""
Updates an existing JIRA issue (i.e. tags, assignee, comments etc).
Updates an existing JIRA issue (i.e. tags, assignee, comments, etc.).
:param jira.resources.Issue existing: Existing JIRA issue that was found
:param sync2jira.intermediary.Issue issue: Upstream issue we're pulling data from
Expand All @@ -738,8 +739,8 @@ def _update_jira_issue(existing, issue, client, config):
# Get a list of what the user wants to update for the upstream issue
updates = issue.downstream.get('issue_updates', [])

# Update relevant data if needed
# If the user has specified nothing
# Update relevant data if needed.
# If the user has specified nothing, just return.
if not updates:
return

Expand Down Expand Up @@ -875,9 +876,9 @@ def _update_fixVersion(updates, existing, issue, client):
for version in existing.fields.fixVersions:
fix_version.append({'name': version.name})

# Github does not allow for multiple fixVersions (milestones)
# GitHub does not allow for multiple fixVersions (milestones)
# But JIRA does, that is why we're looping here. Hopefully one
# Day Github will support multiple fixVersions :0
# day GitHub will support multiple fixVersions.
for version in issue.fixVersion:
if version is not None:
# Update the fixVersion only if it's already not in JIRA
Expand Down Expand Up @@ -954,7 +955,7 @@ def _update_jira_labels(issue, labels):
Do this only if the current labels would change.
:param jira.resource.Issue issue: Jira issue to be updated
:param list<strings> labels: Lables to be applied on the issue
:param list<strings> labels: Labels to be applied on the issue
:returns: None
"""
_labels = sorted(labels)
Expand All @@ -974,6 +975,7 @@ def _update_github_project_fields(client, existing, issue,
:param jira.resource.Issue existing: Existing JIRA issue
:param sync2jira.intermediary.Issue issue: Upstream issue
:param dict github_project_fields: Fields representing GitHub project item fields in GitHub and Jira
:param dict config: configuration options
"""

default_jira_fields = config['sync2jira'].get('default_jira_fields', {})
Expand Down Expand Up @@ -1130,7 +1132,7 @@ def _update_on_close(existing, issue, updates):
}
}
},
...
...,
]
:param jira.resource.Issue existing: existing Jira issue
Expand All @@ -1156,7 +1158,7 @@ def _update_on_close(existing, issue, updates):
updated_labels = list(
set(existing.fields.labels).union(set(on_close_updates['apply_labels']))
)
log.info("Applying 'on_close' labels to downstrem Jira issue")
log.info("Applying 'on_close' labels to downstream Jira issue")
_update_jira_labels(existing, updated_labels)


Expand All @@ -1176,7 +1178,7 @@ def verify_tags(tags):

def sync_with_jira(issue, config):
"""
Attempts to sync a upstream issue with JIRA (i.e. by finding
Attempts to sync an upstream issue with JIRA (i.e. by finding
an existing issue or creating a new one).
:param sync2jira.intermediary.Issue issue: Issue object
Expand Down Expand Up @@ -1225,20 +1227,20 @@ def sync_with_jira(issue, config):
# - If we can't find it, create it.
# - If we can find it, upgrade it to the new method.
log.info("Looking for matching downstream issue via legacy method.")
match = _get_existing_jira_issue_legacy(client, issue, config)
match = _get_existing_jira_issue_legacy(client, issue)
if not match:
_create_jira_issue(client, issue, config)
else:
_upgrade_jira_issue(client, match, issue, config)


def _close_as_duplicate(client, duplicate, keeper, config):
def _close_as_duplicate(client: jira.client, duplicate, keeper, config):
"""
Helper function to close an issue as a duplicate.
:param jira.client client: JIRA Client
:param jira.resources.Issue duplicate: Duplicate JIRA Issue
:param jira.resources.Issue keeper: JIRA issue to keep
:param jira.resources.Issue keeper: the JIRA issue to keep
:param Dict config: Config dict
:returns: Nothing
"""
Expand All @@ -1248,8 +1250,7 @@ def _close_as_duplicate(client, duplicate, keeper, config):
return

# Find the id of some dropped or done state.
transitions = client.transitions(duplicate)
transitions = dict((t['name'], t['id']) for t in transitions)
transitions = {t['name']: t['id'] for t in client.transitions(duplicate)}
closed = None
preferences = ['Dropped', 'Reject', 'Done', 'Closed', 'Closed (2)', ]
for preference in preferences:
Expand All @@ -1259,28 +1260,33 @@ def _close_as_duplicate(client, duplicate, keeper, config):

text = 'Marking as duplicate of %s' % keeper.key
if any(text in comment.body for comment in client.comments(duplicate)):
log.info("Skipping comment. Already present.")
log.info("Skipping comment in duplicate. Already present.")
else:
client.add_comment(duplicate, text)

text = '%s is a duplicate.' % duplicate.key
if any(text in comment.body for comment in client.comments(keeper)):
log.info("Skipping comment. Already present.")
log.info("Skipping comment original. Already present.")
else:
client.add_comment(keeper, text)

if closed:
try:
client.transition_issue(duplicate, closed, resolution={'name': 'Duplicate'})
except Exception as e:
if "Field 'resolution' cannot be set" in e.response.text:
if ('response' in dir(e) and 'text' in dir(e.response)
and "Field 'resolution' cannot be set" in e.response.text):
# Try closing without a specific resolution.
try:
client.transition_issue(duplicate, closed)
except Exception:
log.exception("Failed to close %r", duplicate.permalink())
log.exception(
"Failed to close %r without a resolution",
duplicate.permalink())
else:
log.exception("Failed to close %r", duplicate.permalink())
log.exception(
"Failed to close %r with a resolution of 'Duplicate'",
duplicate.permalink())
else:
log.warning("Unable to find close transition for %r", duplicate.key)

Expand Down
2 changes: 1 addition & 1 deletion tests/test_downstream_issue.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ class MockIssue(object):
# Ensure that we get results back from the jira client.
target1 = "target1"
client.return_value.search_issues = mock.MagicMock(return_value=[target1])
result = d._get_existing_jira_issue_legacy(jira.client.JIRA(), issue, config)
result = d._get_existing_jira_issue_legacy(jira.client.JIRA(), issue)
assert result == target1

client.return_value.search_issues.assert_called_once_with(
Expand Down

0 comments on commit 0b2592c

Please sign in to comment.