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 (GothenburgBitFactory#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 GothenburgBitFactory#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 authored and NexAdn committed Sep 19, 2024
1 parent 953f792 commit 0c4b25b
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 0c4b25b

Please sign in to comment.