Skip to content

Commit

Permalink
Break TaskConstructor out of base Issue class.
Browse files Browse the repository at this point in the history
In preparation for stabilizing the base classes (#791), I'm moving these
methods which child classes never need to call to a separate class in
the collect module.

We *could* leave them where they are and just make them private methods,
but at this point I think slimming down the base classes as much as
possible is our best bet to being disciplined about their stabilization.
This not only removes lines of code but I believe the entire class of
would-be private methods.

I also removed the __str__ and __repr__ methods, which depended on the
get_taskwarrior_record method. These used to be used in logging messages
in db.synchronize but since #1037 db.synchronize is receiving dict's and
not Issue's. (The usages in that method are presently mixed between
logging `issue` and logging `issue['description']`. The logs could be
improved by switching the remainder to the latter.)
  • Loading branch information
ryneeverett committed Aug 30, 2024
1 parent 25e9983 commit 2a041f4
Show file tree
Hide file tree
Showing 27 changed files with 126 additions and 98 deletions.
53 changes: 52 additions & 1 deletion bugwarrior/collect.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import copy
import logging
import multiprocessing
import time

from jinja2 import Template
from pkg_resources import iter_entry_points

from taskw.task import Task

log = logging.getLogger(__name__)

# Sentinels for process completion status
Expand Down Expand Up @@ -88,7 +92,7 @@ def aggregate_issues(conf, main_section, debug):
while currently_running > 0:
issue = queue.get(True)
try:
yield issue.get_taskwarrior_record()
yield TaskConstructor(issue).get_taskwarrior_record()
except AttributeError:
if isinstance(issue, tuple):
currently_running -= 1
Expand All @@ -100,3 +104,50 @@ def aggregate_issues(conf, main_section, debug):
yield issue

log.info("Done aggregating remote issues.")


class TaskConstructor:
""" Construct a taskwarrior task from a foreign record. """

def __init__(self, issue):
self.issue = issue

def get_added_tags(self):
added_tags = []
for tag in self.issue.config.add_tags:
tag = Template(tag).render(self.get_template_context())
if tag:
added_tags.append(tag)

return added_tags

def get_taskwarrior_record(self, refined=True) -> dict:
if not getattr(self, '_taskwarrior_record', None):
self._taskwarrior_record = self.issue.to_taskwarrior()
record = copy.deepcopy(self._taskwarrior_record)
if refined:
record = self.refine_record(record)
if 'tags' not in record:
record['tags'] = []
if refined:
record['tags'].extend(self.get_added_tags())
return record

def get_template_context(self):
context = (
self.get_taskwarrior_record(refined=False).copy()
)
context.update(self.issue.extra)
context.update({
'description': self.issue.get_default_description(),
})
return context

def refine_record(self, record):
for field in Task.FIELDS.keys():
if field in self.issue.config.templates:
template = Template(self.issue.config.templates[field])
record[field] = template.render(self.get_template_context())
elif hasattr(self.issue, 'get_default_%s' % field):
record[field] = getattr(self.issue, 'get_default_%s' % field)()
return record
2 changes: 1 addition & 1 deletion bugwarrior/docs/contributing/new-service.rst
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ Create a test file and implement at least the minimal service tests by inheritin
expected = { ... }
self.assertEqual(issue.get_taskwarrior_record(), expected)
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)
9. Documentation
------------------
Expand Down
52 changes: 0 additions & 52 deletions bugwarrior/services/__init__.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
import abc
import copy
import re

from dateutil.parser import parse as parse_date
from dateutil.tz import tzlocal
from jinja2 import Template
import pytz

from taskw.task import Task

from bugwarrior.config import schema, secrets
from bugwarrior.db import MARKUP, URLShortener

Expand Down Expand Up @@ -193,27 +190,6 @@ def get_tags_from_labels(self,

return tags

def get_added_tags(self):
added_tags = []
for tag in self.config.add_tags:
tag = Template(tag).render(self.get_template_context())
if tag:
added_tags.append(tag)

return added_tags

def get_taskwarrior_record(self, refined=True) -> dict:
if not getattr(self, '_taskwarrior_record', None):
self._taskwarrior_record = self.to_taskwarrior()
record = copy.deepcopy(self._taskwarrior_record)
if refined:
record = self.refine_record(record)
if 'tags' not in record:
record['tags'] = []
if refined:
record['tags'].extend(self.get_added_tags())
return record

def get_priority(self):
return self.PRIORITY_MAP.get(
self.record.get('priority'),
Expand Down Expand Up @@ -263,25 +239,6 @@ def build_default_description(
url,
)

def get_template_context(self):
context = (
self.get_taskwarrior_record(refined=False).copy()
)
context.update(self.extra)
context.update({
'description': self.get_default_description(),
})
return context

def refine_record(self, record):
for field in Task.FIELDS.keys():
if field in self.config.templates:
template = Template(self.config.templates[field])
record[field] = template.render(self.get_template_context())
elif hasattr(self, 'get_default_%s' % field):
record[field] = getattr(self, 'get_default_%s' % field)()
return record

@property
def record(self):
return self._foreign_record
Expand All @@ -290,15 +247,6 @@ def record(self):
def extra(self):
return self._extra

def __str__(self):
return '%s: %s' % (
self.config.target,
self.get_taskwarrior_record()['description']
)

def __repr__(self):
return '<%s>' % str(self)


class ServiceClient:
""" Abstract class responsible for making requests to service API's. """
Expand Down
3 changes: 2 additions & 1 deletion tests/test_activecollab.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import pypandoc
import pytz

from bugwarrior.collect import TaskConstructor
from bugwarrior.services.activecollab import (
ActiveCollabService
)
Expand Down Expand Up @@ -141,4 +142,4 @@ def test_issues(self):
'project': 'something',
'tags': []}

self.assertEqual(issue.get_taskwarrior_record(), expected)
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)
3 changes: 2 additions & 1 deletion tests/test_activecollab2.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import pytz
import responses

from bugwarrior.collect import TaskConstructor
from bugwarrior.services.activecollab2 import ActiveCollab2Service

from .base import ServiceTest, AbstractServiceTest
Expand Down Expand Up @@ -97,4 +98,4 @@ def test_issues(self):
'project': 'something',
'tags': []}

self.assertEqual(issue.get_taskwarrior_record(), expected)
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)
5 changes: 3 additions & 2 deletions tests/test_azuredevops.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

from dateutil.tz.tz import tzutc

from bugwarrior.collect import TaskConstructor
from bugwarrior.services.azuredevops import (
AzureDevopsService,
striphtml,
Expand Down Expand Up @@ -230,7 +231,7 @@ def test_issues(self):
"tags": []
}
issue = next(self.service.issues())
self.assertEqual(issue.get_taskwarrior_record(), expected)
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)

def test_issues_wiql_filter(self):
expected = {
Expand All @@ -255,4 +256,4 @@ def test_issues_wiql_filter(self):
}
service = self.get_service(config_overrides={'wiql_filter': 'something'})
issue = next(service.issues())
self.assertEqual(issue.get_taskwarrior_record(), expected)
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)
5 changes: 3 additions & 2 deletions tests/test_bitbucket.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import responses

from bugwarrior.collect import TaskConstructor
from bugwarrior.services.bitbucket import BitbucketService

from .base import ServiceTest, AbstractServiceTest
Expand Down Expand Up @@ -104,7 +105,7 @@ def test_issues(self):
'project': 'somerepo',
'tags': []}

self.assertEqual(issue.get_taskwarrior_record(), expected_issue)
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected_issue)

expected_pr = {
'annotations': ['@nobody - Some comment.'],
Expand All @@ -116,7 +117,7 @@ def test_issues(self):
'project': 'somerepo',
'tags': []}

self.assertEqual(pr.get_taskwarrior_record(), expected_pr)
self.assertEqual(TaskConstructor(pr).get_taskwarrior_record(), expected_pr)

def test_get_owner(self):
issue = {
Expand Down
4 changes: 3 additions & 1 deletion tests/test_bts.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import sys
from unittest import mock, SkipTest

from bugwarrior.collect import TaskConstructor

if sys.version_info >= (3, 11):
raise SkipTest(
"Python-3.11+ not supported. "
Expand Down Expand Up @@ -89,4 +91,4 @@ def test_issues(self):
'btsstatus': 'pending',
'tags': []}

self.assertEqual(issue.get_taskwarrior_record(), expected)
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)
9 changes: 5 additions & 4 deletions tests/test_bugzilla.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from unittest import mock
from collections import namedtuple

from bugwarrior.collect import TaskConstructor
from bugwarrior.services.bz import BugzillaService

from .base import ConfigTest, ServiceTest, AbstractServiceTest
Expand Down Expand Up @@ -155,7 +156,7 @@ def test_issues(self):
'project': 'Something',
'tags': []}

self.assertEqual(issue.get_taskwarrior_record(), expected)
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)

def test_only_if_assigned(self):
with mock.patch('bugzilla.Bugzilla'):
Expand Down Expand Up @@ -206,7 +207,7 @@ def test_only_if_assigned(self):
'project': 'Something',
'tags': []}

self.assertEqual(next(issues).get_taskwarrior_record(), expected)
self.assertEqual(TaskConstructor(next(issues)).get_taskwarrior_record(), expected)

# Only one issue is assigned.
self.assertRaises(StopIteration, lambda: next(issues))
Expand Down Expand Up @@ -246,9 +247,9 @@ def test_also_unassigned(self):

issues = self.service.issues()

self.assertIn(next(issues).get_taskwarrior_record()['bugzillabugid'],
self.assertIn(TaskConstructor(next(issues)).get_taskwarrior_record()['bugzillabugid'],
[1234567, 1234568])
self.assertIn(next(issues).get_taskwarrior_record()['bugzillabugid'],
self.assertIn(TaskConstructor(next(issues)).get_taskwarrior_record()['bugzillabugid'],
[1234567, 1234568])
# Only two issues are assigned to the user or unassigned.
self.assertRaises(StopIteration, lambda: next(issues))
4 changes: 3 additions & 1 deletion tests/test_deck.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
import pydantic
from dateutil.tz import tzutc

from bugwarrior.collect import TaskConstructor
from bugwarrior.services.deck import NextcloudDeckClient, NextcloudDeckService

from .base import AbstractServiceTest, ServiceTest


Expand Down Expand Up @@ -157,7 +159,7 @@ def test_issues(self):
'tags': ['Later']
}

self.assertEqual(issue.get_taskwarrior_record(), expected)
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)

def test_filter_boards_include(self):
self.config['deck']['include_board_ids'] = '5'
Expand Down
6 changes: 4 additions & 2 deletions tests/test_gerrit.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

import responses

from bugwarrior.collect import TaskConstructor
from bugwarrior.services.gerrit import GerritService

from .base import ServiceTest, AbstractServiceTest


Expand Down Expand Up @@ -82,7 +84,7 @@ def test_work_in_progress(self):
'project': 'nova',
'tags': []}

self.assertEqual(issue.get_taskwarrior_record(), expected)
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)

@responses.activate
def test_issues(self):
Expand All @@ -107,4 +109,4 @@ def test_issues(self):
'project': 'nova',
'tags': []}

self.assertEqual(issue.get_taskwarrior_record(), expected)
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)
3 changes: 2 additions & 1 deletion tests/test_gitbug.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import dateutil
import pydantic

from bugwarrior.collect import TaskConstructor
from bugwarrior.services.gitbug import GitBugClient, GitBugConfig, GitBugService

from .base import AbstractServiceTest, ConfigTest, ServiceTest
Expand Down Expand Up @@ -82,7 +83,7 @@ def test_issues(self):
'tags': []
}

self.assertEqual(issue.get_taskwarrior_record(), expected)
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)


class TestGitBugConfig(ConfigTest):
Expand Down
7 changes: 4 additions & 3 deletions tests/test_github.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import pytz
import responses

from bugwarrior.collect import TaskConstructor
from bugwarrior.services.github import (
GithubConfig, GithubService, GithubClient)

Expand Down Expand Up @@ -85,7 +86,7 @@ def test_draft(self):
'tags': []
}

self.assertEqual(issue.get_taskwarrior_record(), expected)
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)

def test_to_taskwarrior(self):
service = self.get_mock_service(GithubService, config_overrides={
Expand Down Expand Up @@ -178,7 +179,7 @@ def test_issues(self):
'project': 'arbitrary_repo',
'tags': []}

self.assertEqual(issue.get_taskwarrior_record(), expected)
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)


class TestGithubIssueQuery(AbstractServiceTest, ServiceTest):
Expand Down Expand Up @@ -238,7 +239,7 @@ def test_issues(self):
'project': 'arbitrary_repo',
'tags': []}

self.assertEqual(issue.get_taskwarrior_record(), expected)
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)


class TestGithubService(ServiceTest):
Expand Down
Loading

0 comments on commit 2a041f4

Please sign in to comment.