Skip to content

Commit

Permalink
Remove issue filtering from base Service class
Browse files Browse the repository at this point in the history
We here remove the abstract `get_owner` method and the concrete
`include` method from the base class and copy the `include` method to
the handful of services which make use of it. The `include` method was
only being used in 6 of 26 services, so I think the duplication is
acceptable in exchange for the benefits:

- This eliminates the last instance of bidirectional communication
  between the base classes and the service implementations, which in my
  opinion is an architectural problem.
- Since most of the services don't use this filtering mechanism, it is
  annoying to have to define `get_owner`.
- This filtering mechanism is somewhat legacy from the days when API's
  were generally far less capable. I don't think we should encourage it
  going forward because it is better to implement filtering through the
  API query rather than pulling all data and iterating over it locally,
  and this is a reasonable expectation from third party API's these
  days.

One might suspect that eliminating these NotImplementedError's would
eliminate the warnings from services that do not support
only_if_assigned, but in fact these would only get raised if the service
itself called `self.include`, which they do not. So a user trying to use
these options already would find them silently ignored, which isn't
great but is a problem for another day.
  • Loading branch information
ryneeverett committed Oct 24, 2024
1 parent 1dbde51 commit 3ca4c5b
Show file tree
Hide file tree
Showing 26 changed files with 103 additions and 109 deletions.
8 changes: 0 additions & 8 deletions bugwarrior/docs/contributing/new-service.rst
Original file line number Diff line number Diff line change
Expand Up @@ -187,12 +187,6 @@ Now for the main service class which bugwarrior will invoke to fetch issues.
self.client = GitBugClient(path=self.config.path, port=self.config.port)
def get_owner(self, issue):
# Issue assignment hasn't been implemented in upstream git-bug yet.
# See https://github.com/MichaelMure/git-bug/issues/112.
raise NotImplementedError(
"This service has not implemented support for 'only_if_assigned'.")
def issues(self):
for issue in self.client.get_issues():
comments = issue.pop('comments')
Expand All @@ -209,8 +203,6 @@ Now for the main service class which bugwarrior will invoke to fetch issues.
Here we see two required class attributes (pointing to the classes we previously defined) and two required methods.

The ``get_owner`` method takes an individual issue and returns the "assigned" user, so that bugwarrior can filter issues on this basis. In this case git-bug has not yet implemented this feature, but it generally will just involve returning a value from the ``issue`` dictionary.

The ``issues`` method is a generator which yields individual issue dictionaries.

7. Service Registration
Expand Down
4 changes: 0 additions & 4 deletions bugwarrior/services/activecollab.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,10 +193,6 @@ def _comments(self, issue):
body=comment['body']))
return comments_formatted

def get_owner(self, issue):
if issue['assignee_id']:
return issue['assignee_id']

def annotations(self, issue, issue_obj):
if 'type' not in issue:
# Subtask
Expand Down
5 changes: 0 additions & 5 deletions bugwarrior/services/activecollab2.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,11 +204,6 @@ def __init__(self, *args, **kw):
self.config.projects,
self.config.target)

def get_owner(self, issue):
# TODO
raise NotImplementedError(
"This service has not implemented support for 'only_if_assigned'.")

def issues(self):
# Loop through each project
start = time.time()
Expand Down
4 changes: 0 additions & 4 deletions bugwarrior/services/azuredevops.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,10 +249,6 @@ def issues(self):
issue_obj.extra.update(extra)
yield issue_obj

def get_owner(self, issue):
# Issue filtering is implemented as part of issue aggregation.
pass

@staticmethod
def get_keyring_service(config):
return f"azuredevops://{config.organization}@{config.host}"
18 changes: 0 additions & 18 deletions bugwarrior/services/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,24 +106,6 @@ def build_annotations(self, annotations, url=None):
final.append('@%s - %s' % (author, message))
return final

def include(self, issue):
""" Return true if the issue in question should be included """
if self.config.only_if_assigned:
owner = self.get_owner(issue)
include_owners = [self.config.only_if_assigned]

if self.config.also_unassigned:
include_owners.append(None)

return owner in include_owners

return True

@abc.abstractmethod
def get_owner(self, issue):
""" Override this for filtering on tickets """
raise NotImplementedError()

@abc.abstractmethod
def issues(self):
""" Returns a list of Issue instances representing issues from a remote service.
Expand Down
13 changes: 13 additions & 0 deletions bugwarrior/services/bitbucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,19 @@ def get_owner(self, issue):
if assignee is not None:
return assignee.get('username', None)

def include(self, issue):
""" Return true if the issue in question should be included """
if self.config.only_if_assigned:
owner = self.get_owner(issue)
include_owners = [self.config.only_if_assigned]

if self.config.also_unassigned:
include_owners.append(None)

return owner in include_owners

return True

def issues(self):
user = self.config.username
response = self.get_collection('/repositories/' + user + '/')
Expand Down
5 changes: 0 additions & 5 deletions bugwarrior/services/bts.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,6 @@ class BTSService(Service, Client):
ISSUE_CLASS = BTSIssue
CONFIG_SCHEMA = BTSConfig

def get_owner(self, issue):
# TODO
raise NotImplementedError(
"This service has not implemented support for 'only_if_assigned'.")

def _record_for_bug(self, bug):
return {'number': bug.bug_num,
'url': 'https://bugs.debian.org/' + str(bug.bug_num),
Expand Down
13 changes: 13 additions & 0 deletions bugwarrior/services/bz.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,19 @@ def get_keyring_service(config):
def get_owner(self, issue):
return issue['assigned_to']

def include(self, issue):
""" Return true if the issue in question should be included """
if self.config.only_if_assigned:
owner = self.get_owner(issue)
include_owners = [self.config.only_if_assigned]

if self.config.also_unassigned:
include_owners.append(None)

return owner in include_owners

return True

def annotations(self, tag, issue):
base_url = "%s/show_bug.cgi?id=" % self.config.base_uri
long_url = base_url + str(issue['id'])
Expand Down
13 changes: 13 additions & 0 deletions bugwarrior/services/deck.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,19 @@ def __init__(self, *args, **kwargs):
def get_owner(self, issue):
return issue[issue.ASSIGNEE]

def include(self, issue):
""" Return true if the issue in question should be included """
if self.config.only_if_assigned:
owner = self.get_owner(issue)
include_owners = [self.config.only_if_assigned]

if self.config.also_unassigned:
include_owners.append(None)

return owner in include_owners

return True

def filter_boards(self, board):
# include_board_ids takes precedence over exclude_board_ids
if self.config.include_board_ids:
Expand Down
5 changes: 0 additions & 5 deletions bugwarrior/services/gerrit.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,6 @@ def __init__(self, *args, **kw):
def get_keyring_service(config):
return f"gerrit://{config.base_uri}"

def get_owner(self, issue):
# TODO
raise NotImplementedError(
"This service has not implemented support for 'only_if_assigned'.")

def issues(self):
# Construct the whole url by hand here, because otherwise requests will
# percent-encode the ':' characters, which gerrit doesn't like.
Expand Down
6 changes: 0 additions & 6 deletions bugwarrior/services/gitbug.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,6 @@ def __init__(self, *args, **kwargs):
port=self.config.port,
annotation_comments=self.main_config.annotation_comments)

def get_owner(self, issue):
# Issue assignment hasn't been implemented in upstream git-bug yet.
# See https://github.com/MichaelMure/git-bug/issues/112.
raise NotImplementedError(
"This service has not implemented support for 'only_if_assigned'.")

def issues(self):
for issue in self.client.get_issues():
comments = issue.pop('comments')
Expand Down
13 changes: 12 additions & 1 deletion bugwarrior/services/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -439,12 +439,23 @@ def filter_repo_name(self, name):
return True

def include(self, issue):
""" Return true if the issue in question should be included """
if 'pull_request' in issue[1]:
if self.config.exclude_pull_requests:
return False
if not self.config.filter_pull_requests:
return True
return super().include(issue)

if self.config.only_if_assigned:
owner = self.get_owner(issue)
include_owners = [self.config.only_if_assigned]

if self.config.also_unassigned:
include_owners.append(None)

return owner in include_owners

return True

def issues(self):
issues = {}
Expand Down
4 changes: 0 additions & 4 deletions bugwarrior/services/gmail.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,10 +204,6 @@ def annotations(self, issue):
issue_url = issue.extra['url']
return self.build_annotations([(sender, subj)], issue_url)

def get_owner(self, issue):
raise NotImplementedError(
"This service has not implemented support for 'only_if_assigned'.")

def issues(self):
labels = self.get_labels()
for thread in self.get_threads():
Expand Down
5 changes: 0 additions & 5 deletions bugwarrior/services/jira.py
Original file line number Diff line number Diff line change
Expand Up @@ -413,11 +413,6 @@ def __init__(self, *args, **kw):
def get_keyring_service(config):
return f"jira://{config.username}@{config.base_uri}"

def get_owner(self, issue):
# TODO
raise NotImplementedError(
"This service has not implemented support for 'only_if_assigned'.")

def body(self, issue):
body = issue.record.get('fields', {}).get('description')

Expand Down
5 changes: 0 additions & 5 deletions bugwarrior/services/kanboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,6 @@ def issues(self):

yield self.get_issue_for_record(task, extra)

def get_owner(self, issue):
# TODO
raise NotImplementedError(
"This service has not implemented support for 'only_if_assigned'.")

@staticmethod
def get_keyring_service(config):
parsed = urlparse(config.url)
Expand Down
6 changes: 0 additions & 6 deletions bugwarrior/services/logseq.py
Original file line number Diff line number Diff line change
Expand Up @@ -296,12 +296,6 @@ def __init__(self, *args, **kwargs):
filter=filter,
)

def get_owner(self, issue):
# Issue assignment hasn't been implemented yet.
raise NotImplementedError(
"This service has not implemented support for 'only_if_assigned'."
)

def issues(self):
graph_name = self.client.get_graph_name()
for issue in self.client.get_issues():
Expand Down
13 changes: 13 additions & 0 deletions bugwarrior/services/pagure.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,19 @@ def get_owner(self, issue):
if issue[1]['assignee']:
return issue[1]['assignee']['name']

def include(self, issue):
""" Return true if the issue in question should be included """
if self.config.only_if_assigned:
owner = self.get_owner(issue)
include_owners = [self.config.only_if_assigned]

if self.config.also_unassigned:
include_owners.append(None)

return owner in include_owners

return True

def filter_repos(self, repo):
if repo in self.config.exclude_repos:
return False
Expand Down
4 changes: 0 additions & 4 deletions bugwarrior/services/phab.py
Original file line number Diff line number Diff line change
Expand Up @@ -243,10 +243,6 @@ def revisions(self):
}
yield self.get_issue_for_record(diff, extra)

def get_owner(self, issue):
# Issue filtering is implemented as part of issue aggregation.
pass

def issues(self):
yield from self.tasks()
yield from self.revisions()
8 changes: 0 additions & 8 deletions bugwarrior/services/pivotaltracker.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,6 @@ class PivotalTrackerIssue(Issue):

UNIQUE_KEY = (URL,)

def get_owner(self, issue):
_, issue = issue
return issue.get('pivotalowners')

def to_taskwarrior(self):
description = self.record.get('description')
created = self.parse_date(self.record.get('created_at'))
Expand Down Expand Up @@ -148,10 +144,6 @@ def __init__(self, *args, **kwargs):
if self.config.only_if_author:
self.query += f" requester:{self.config.user_id}"

def get_owner(self, issue):
# Issue filtering is implemented as part of the api query.
pass

def annotations(self, annotations, story):
final_annotations = []
if self.main_config.annotation_comments:
Expand Down
4 changes: 0 additions & 4 deletions bugwarrior/services/redmine.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,10 +265,6 @@ def __init__(self, *args, **kw):
def get_keyring_service(config):
return f"redmine://{config.login}@{config.url}/"

def get_owner(self, issue):
# Issue filtering is implemented as part of the api query.
pass

def issues(self):
issues = self.client.find_issues(
self.config.issue_limit, self.config.query, self.config.only_if_assigned)
Expand Down
5 changes: 0 additions & 5 deletions bugwarrior/services/taiga.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,6 @@ def __init__(self, *args, **kw):
def get_keyring_service(config):
return f"taiga://{config.base_uri}"

def get_owner(self, issue):
# TODO
raise NotImplementedError(
"This service has not implemented support for 'only_if_assigned'.")

def _issues(self, userid, task_type, task_type_plural, task_type_short):
log.debug('Getting %s' % task_type_plural)

Expand Down
3 changes: 0 additions & 3 deletions bugwarrior/services/teamwork_projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,6 @@ def get_comments(self, issue):
return self.build_annotations(comment_list, None)
return []

def get_owner(self, issue):
return issue.get_owner()

def issues(self):
response = self.client.call_api("GET", "/tasks.json")
for issue in response["todo-items"]:
Expand Down
13 changes: 13 additions & 0 deletions bugwarrior/services/trac.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,19 @@ def get_owner(self, issue):
tag, issue = issue
return issue.get('owner', None) or None

def include(self, issue):
""" Return true if the issue in question should be included """
if self.config.only_if_assigned:
owner = self.get_owner(issue)
include_owners = [self.config.only_if_assigned]

if self.config.also_unassigned:
include_owners.append(None)

return owner in include_owners

return True

def issues(self):
base_url = "https://" + self.config.base_uri
if self.trac:
Expand Down
4 changes: 0 additions & 4 deletions bugwarrior/services/trello.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,6 @@ class TrelloService(Service, Client):
ISSUE_CLASS = TrelloIssue
CONFIG_SCHEMA = TrelloConfig

def get_owner(self, issue):
# Issue filtering is implemented as part of the api query.
pass

@staticmethod
def get_keyring_service(config):
return f"trello://{config.api_key}@trello.com"
Expand Down
5 changes: 0 additions & 5 deletions bugwarrior/services/youtrack.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,6 @@ def __init__(self, *args, **kw):
def get_keyring_service(config):
return f"youtrack://{config.login}@{config.host}"

def get_owner(self, issue):
# TODO
raise NotImplementedError(
"This service has not implemented support for 'only_if_assigned'.")

def issues(self):
params = {
'query': self.config.query,
Expand Down
Loading

0 comments on commit 3ca4c5b

Please sign in to comment.