Skip to content

Commit 8fb61d5

Browse files
committed
Break TaskConstructor out of base Issue class.
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.)
1 parent da88cbb commit 8fb61d5

27 files changed

+126
-98
lines changed

bugwarrior/collect.py

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
1+
import copy
12
import logging
23
import multiprocessing
34
import time
45

6+
from jinja2 import Template
57
from pkg_resources import iter_entry_points
68

9+
from taskw.task import Task
10+
711
log = logging.getLogger(__name__)
812

913
# Sentinels for process completion status
@@ -88,7 +92,7 @@ def aggregate_issues(conf, main_section, debug):
8892
while currently_running > 0:
8993
issue = queue.get(True)
9094
try:
91-
yield issue.get_taskwarrior_record()
95+
yield TaskConstructor(issue).get_taskwarrior_record()
9296
except AttributeError:
9397
if isinstance(issue, tuple):
9498
currently_running -= 1
@@ -100,3 +104,50 @@ def aggregate_issues(conf, main_section, debug):
100104
yield issue
101105

102106
log.info("Done aggregating remote issues.")
107+
108+
109+
class TaskConstructor:
110+
""" Construct a taskwarrior task from a foreign record. """
111+
112+
def __init__(self, issue):
113+
self.issue = issue
114+
115+
def get_added_tags(self):
116+
added_tags = []
117+
for tag in self.issue.config.add_tags:
118+
tag = Template(tag).render(self.get_template_context())
119+
if tag:
120+
added_tags.append(tag)
121+
122+
return added_tags
123+
124+
def get_taskwarrior_record(self, refined=True) -> dict:
125+
if not getattr(self, '_taskwarrior_record', None):
126+
self._taskwarrior_record = self.issue.to_taskwarrior()
127+
record = copy.deepcopy(self._taskwarrior_record)
128+
if refined:
129+
record = self.refine_record(record)
130+
if 'tags' not in record:
131+
record['tags'] = []
132+
if refined:
133+
record['tags'].extend(self.get_added_tags())
134+
return record
135+
136+
def get_template_context(self):
137+
context = (
138+
self.get_taskwarrior_record(refined=False).copy()
139+
)
140+
context.update(self.issue.extra)
141+
context.update({
142+
'description': self.issue.get_default_description(),
143+
})
144+
return context
145+
146+
def refine_record(self, record):
147+
for field in Task.FIELDS.keys():
148+
if field in self.issue.config.templates:
149+
template = Template(self.issue.config.templates[field])
150+
record[field] = template.render(self.get_template_context())
151+
elif hasattr(self.issue, 'get_default_%s' % field):
152+
record[field] = getattr(self.issue, 'get_default_%s' % field)()
153+
return record

bugwarrior/docs/contributing/new-service.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ Create a test file and implement at least the minimal service tests by inheritin
264264
265265
expected = { ... }
266266
267-
self.assertEqual(issue.get_taskwarrior_record(), expected)
267+
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)
268268
269269
9. Documentation
270270
------------------

bugwarrior/services/__init__.py

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,11 @@
11
import abc
2-
import copy
32
import re
43

54
from dateutil.parser import parse as parse_date
65
from dateutil.tz import tzlocal
76
from jinja2 import Template
87
import pytz
98

10-
from taskw.task import Task
11-
129
from bugwarrior.config import schema, secrets
1310
from bugwarrior.db import MARKUP, URLShortener
1411

@@ -202,27 +199,6 @@ def get_tags_from_labels(self,
202199

203200
return tags
204201

205-
def get_added_tags(self):
206-
added_tags = []
207-
for tag in self.config.add_tags:
208-
tag = Template(tag).render(self.get_template_context())
209-
if tag:
210-
added_tags.append(tag)
211-
212-
return added_tags
213-
214-
def get_taskwarrior_record(self, refined=True) -> dict:
215-
if not getattr(self, '_taskwarrior_record', None):
216-
self._taskwarrior_record = self.to_taskwarrior()
217-
record = copy.deepcopy(self._taskwarrior_record)
218-
if refined:
219-
record = self.refine_record(record)
220-
if 'tags' not in record:
221-
record['tags'] = []
222-
if refined:
223-
record['tags'].extend(self.get_added_tags())
224-
return record
225-
226202
def get_priority(self):
227203
return self.PRIORITY_MAP.get(
228204
self.record.get('priority'),
@@ -272,25 +248,6 @@ def build_default_description(
272248
url,
273249
)
274250

275-
def get_template_context(self):
276-
context = (
277-
self.get_taskwarrior_record(refined=False).copy()
278-
)
279-
context.update(self.extra)
280-
context.update({
281-
'description': self.get_default_description(),
282-
})
283-
return context
284-
285-
def refine_record(self, record):
286-
for field in Task.FIELDS.keys():
287-
if field in self.config.templates:
288-
template = Template(self.config.templates[field])
289-
record[field] = template.render(self.get_template_context())
290-
elif hasattr(self, 'get_default_%s' % field):
291-
record[field] = getattr(self, 'get_default_%s' % field)()
292-
return record
293-
294251
@property
295252
def record(self):
296253
return self._foreign_record
@@ -299,15 +256,6 @@ def record(self):
299256
def extra(self):
300257
return self._extra
301258

302-
def __str__(self):
303-
return '%s: %s' % (
304-
self.config.target,
305-
self.get_taskwarrior_record()['description']
306-
)
307-
308-
def __repr__(self):
309-
return '<%s>' % str(self)
310-
311259

312260
class ServiceClient:
313261
""" Abstract class responsible for making requests to service API's. """

tests/test_activecollab.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import pypandoc
66
import pytz
77

8+
from bugwarrior.collect import TaskConstructor
89
from bugwarrior.services.activecollab import (
910
ActiveCollabService
1011
)
@@ -141,4 +142,4 @@ def test_issues(self):
141142
'project': 'something',
142143
'tags': []}
143144

144-
self.assertEqual(issue.get_taskwarrior_record(), expected)
145+
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)

tests/test_activecollab2.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import pytz
55
import responses
66

7+
from bugwarrior.collect import TaskConstructor
78
from bugwarrior.services.activecollab2 import ActiveCollab2Service
89

910
from .base import ServiceTest, AbstractServiceTest
@@ -97,4 +98,4 @@ def test_issues(self):
9798
'project': 'something',
9899
'tags': []}
99100

100-
self.assertEqual(issue.get_taskwarrior_record(), expected)
101+
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)

tests/test_azuredevops.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
from dateutil.tz.tz import tzutc
55

6+
from bugwarrior.collect import TaskConstructor
67
from bugwarrior.services.azuredevops import (
78
AzureDevopsService,
89
striphtml,
@@ -230,7 +231,7 @@ def test_issues(self):
230231
"tags": []
231232
}
232233
issue = next(self.service.issues())
233-
self.assertEqual(issue.get_taskwarrior_record(), expected)
234+
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)
234235

235236
def test_issues_wiql_filter(self):
236237
expected = {
@@ -255,4 +256,4 @@ def test_issues_wiql_filter(self):
255256
}
256257
service = self.get_service(config_overrides={'wiql_filter': 'something'})
257258
issue = next(service.issues())
258-
self.assertEqual(issue.get_taskwarrior_record(), expected)
259+
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)

tests/test_bitbucket.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import responses
22

3+
from bugwarrior.collect import TaskConstructor
34
from bugwarrior.services.bitbucket import BitbucketService
45

56
from .base import ServiceTest, AbstractServiceTest
@@ -104,7 +105,7 @@ def test_issues(self):
104105
'project': 'somerepo',
105106
'tags': []}
106107

107-
self.assertEqual(issue.get_taskwarrior_record(), expected_issue)
108+
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected_issue)
108109

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

119-
self.assertEqual(pr.get_taskwarrior_record(), expected_pr)
120+
self.assertEqual(TaskConstructor(pr).get_taskwarrior_record(), expected_pr)
120121

121122
def test_get_owner(self):
122123
issue = {

tests/test_bts.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import sys
22
from unittest import mock, SkipTest
33

4+
from bugwarrior.collect import TaskConstructor
5+
46
if sys.version_info >= (3, 11):
57
raise SkipTest(
68
"Python-3.11+ not supported. "
@@ -89,4 +91,4 @@ def test_issues(self):
8991
'btsstatus': 'pending',
9092
'tags': []}
9193

92-
self.assertEqual(issue.get_taskwarrior_record(), expected)
94+
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)

tests/test_bugzilla.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
from unittest import mock
33
from collections import namedtuple
44

5+
from bugwarrior.collect import TaskConstructor
56
from bugwarrior.services.bz import BugzillaService
67

78
from .base import ConfigTest, ServiceTest, AbstractServiceTest
@@ -155,7 +156,7 @@ def test_issues(self):
155156
'project': 'Something',
156157
'tags': []}
157158

158-
self.assertEqual(issue.get_taskwarrior_record(), expected)
159+
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)
159160

160161
def test_only_if_assigned(self):
161162
with mock.patch('bugzilla.Bugzilla'):
@@ -206,7 +207,7 @@ def test_only_if_assigned(self):
206207
'project': 'Something',
207208
'tags': []}
208209

209-
self.assertEqual(next(issues).get_taskwarrior_record(), expected)
210+
self.assertEqual(TaskConstructor(next(issues)).get_taskwarrior_record(), expected)
210211

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

247248
issues = self.service.issues()
248249

249-
self.assertIn(next(issues).get_taskwarrior_record()['bugzillabugid'],
250+
self.assertIn(TaskConstructor(next(issues)).get_taskwarrior_record()['bugzillabugid'],
250251
[1234567, 1234568])
251-
self.assertIn(next(issues).get_taskwarrior_record()['bugzillabugid'],
252+
self.assertIn(TaskConstructor(next(issues)).get_taskwarrior_record()['bugzillabugid'],
252253
[1234567, 1234568])
253254
# Only two issues are assigned to the user or unassigned.
254255
self.assertRaises(StopIteration, lambda: next(issues))

tests/test_deck.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
import pydantic
55
from dateutil.tz import tzutc
66

7+
from bugwarrior.collect import TaskConstructor
78
from bugwarrior.services.deck import NextcloudDeckClient, NextcloudDeckService
9+
810
from .base import AbstractServiceTest, ServiceTest
911

1012

@@ -157,7 +159,7 @@ def test_issues(self):
157159
'tags': ['Later']
158160
}
159161

160-
self.assertEqual(issue.get_taskwarrior_record(), expected)
162+
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)
161163

162164
def test_filter_boards_include(self):
163165
self.config['deck']['include_board_ids'] = '5'

tests/test_gerrit.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22

33
import responses
44

5+
from bugwarrior.collect import TaskConstructor
56
from bugwarrior.services.gerrit import GerritService
7+
68
from .base import ServiceTest, AbstractServiceTest
79

810

@@ -82,7 +84,7 @@ def test_work_in_progress(self):
8284
'project': 'nova',
8385
'tags': []}
8486

85-
self.assertEqual(issue.get_taskwarrior_record(), expected)
87+
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)
8688

8789
@responses.activate
8890
def test_issues(self):
@@ -107,4 +109,4 @@ def test_issues(self):
107109
'project': 'nova',
108110
'tags': []}
109111

110-
self.assertEqual(issue.get_taskwarrior_record(), expected)
112+
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)

tests/test_gitbug.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import dateutil
55
import pydantic
66

7+
from bugwarrior.collect import TaskConstructor
78
from bugwarrior.services.gitbug import GitBugClient, GitBugConfig, GitBugService
89

910
from .base import AbstractServiceTest, ConfigTest, ServiceTest
@@ -82,7 +83,7 @@ def test_issues(self):
8283
'tags': []
8384
}
8485

85-
self.assertEqual(issue.get_taskwarrior_record(), expected)
86+
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)
8687

8788

8889
class TestGitBugConfig(ConfigTest):

tests/test_github.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import pytz
55
import responses
66

7+
from bugwarrior.collect import TaskConstructor
78
from bugwarrior.services.github import (
89
GithubConfig, GithubService, GithubClient)
910

@@ -85,7 +86,7 @@ def test_draft(self):
8586
'tags': []
8687
}
8788

88-
self.assertEqual(issue.get_taskwarrior_record(), expected)
89+
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)
8990

9091
def test_to_taskwarrior(self):
9192
service = self.get_mock_service(GithubService, config_overrides={
@@ -178,7 +179,7 @@ def test_issues(self):
178179
'project': 'arbitrary_repo',
179180
'tags': []}
180181

181-
self.assertEqual(issue.get_taskwarrior_record(), expected)
182+
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)
182183

183184

184185
class TestGithubIssueQuery(AbstractServiceTest, ServiceTest):
@@ -238,7 +239,7 @@ def test_issues(self):
238239
'project': 'arbitrary_repo',
239240
'tags': []}
240241

241-
self.assertEqual(issue.get_taskwarrior_record(), expected)
242+
self.assertEqual(TaskConstructor(issue).get_taskwarrior_record(), expected)
242243

243244

244245
class TestGithubService(ServiceTest):

0 commit comments

Comments
 (0)