Merge ~twom/launchpad:questions-about-storm into launchpad:master
- Git
- lp:~twom/launchpad
- questions-about-storm
- Merge into master
Proposed by
Tom Wardill
Status: | Merged |
---|---|
Approved by: | Tom Wardill |
Approved revision: | 7d78d8280da98cca0542090e88d4d68aa6e5bf51 |
Merge reported by: | Otto Co-Pilot |
Merged at revision: | not available |
Proposed branch: | ~twom/launchpad:questions-about-storm |
Merge into: | launchpad:master |
Diff against target: |
634 lines (+196/-176) 7 files modified
lib/lp/answers/doc/expiration.txt (+13/-6) lib/lp/answers/doc/workflow.txt (+1/-1) lib/lp/answers/model/faq.py (+3/-2) lib/lp/answers/model/question.py (+145/-164) lib/lp/answers/model/tests/test_question.py (+31/-0) lib/lp/answers/templates/question-portlet-reopenings.pt (+1/-1) lib/lp/testing/factory.py (+2/-2) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Colin Watson (community) | Approve | ||
Review via email: mp+394763@code.launchpad.net |
Commit message
Port Question to Storm
Description of the change
To post a comment you must log in.
- 12eec51... by Tom Wardill
-
Format imports
Revision history for this message
Colin Watson (cjwatson) : | # |
review:
Approve
- e460434... by Tom Wardill
-
Fix template for question reopening
- 4303c98... by Tom Wardill
-
drop datelastqueried parameter to Question
- 80e56cc... by Tom Wardill
-
pytz.UTC
- 7d78d82... by Tom Wardill
-
Fix aother answers doctests
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/lib/lp/answers/doc/expiration.txt b/lib/lp/answers/doc/expiration.txt |
2 | index 5027480..6f1d240 100644 |
3 | --- a/lib/lp/answers/doc/expiration.txt |
4 | +++ b/lib/lp/answers/doc/expiration.txt |
5 | @@ -19,19 +19,26 @@ somebody are subject to expiration. |
6 | |
7 | # Sanity check in case somebody modifies the question sampledata and |
8 | # forget to update this script. |
9 | + >>> from lp.services.database.interfaces import IStore |
10 | >>> from lp.answers.enums import QuestionStatus |
11 | >>> from lp.answers.model.question import Question |
12 | - >>> Question.select('status IN (%i,%i)' % ( |
13 | - ... QuestionStatus.OPEN.value, |
14 | - ... QuestionStatus.NEEDSINFO.value)).count() |
15 | + >>> IStore(Question).find( |
16 | + ... Question, Question.status.is_in( |
17 | + ... (QuestionStatus.OPEN, QuestionStatus.NEEDSINFO))).count() |
18 | 9 |
19 | |
20 | # By default, all open and needs info question should expire. Make |
21 | # sure that no new questions were recently added and will make this |
22 | # test fails in the future. |
23 | - >>> Question.select( |
24 | - ... "datelastresponse >= current_timestamp - interval '15 days' OR " |
25 | - ... "datelastquery >= current_timestamp - interval '15 days'").count() |
26 | + >>> from datetime import datetime, timedelta |
27 | + >>> import pytz |
28 | + >>> from storm.locals import Or |
29 | + >>> interval = datetime.now(pytz.UTC) - timedelta(days=15) |
30 | + >>> IStore(Question).find( |
31 | + ... Question, |
32 | + ... Or( |
33 | + ... Question.datelastresponse >= interval, |
34 | + ... Question.datelastquery >= interval)).count() |
35 | 0 |
36 | |
37 | # We need to massage sample data a little. Since all expiration |
38 | diff --git a/lib/lp/answers/doc/workflow.txt b/lib/lp/answers/doc/workflow.txt |
39 | index 3cabebb..4d2116e 100644 |
40 | --- a/lib/lp/answers/doc/workflow.txt |
41 | +++ b/lib/lp/answers/doc/workflow.txt |
42 | @@ -592,7 +592,7 @@ Users without launchpad.Moderator privileges cannot set the assignee. |
43 | >>> question.assignee = sample_person |
44 | Traceback (most recent call last): |
45 | ... |
46 | - Unauthorized: (<Question ...>, 'assignee', 'launchpad.Append') |
47 | + Unauthorized: (<lp.answers.model.question.Question ...>, 'assignee', 'launchpad.Append') |
48 | |
49 | |
50 | Events |
51 | diff --git a/lib/lp/answers/model/faq.py b/lib/lp/answers/model/faq.py |
52 | index f5ed5ae..70f4648 100644 |
53 | --- a/lib/lp/answers/model/faq.py |
54 | +++ b/lib/lp/answers/model/faq.py |
55 | @@ -20,6 +20,7 @@ from sqlobject import ( |
56 | StringCol, |
57 | ) |
58 | from storm.expr import And |
59 | +from storm.references import ReferenceSet |
60 | from zope.event import notify |
61 | from zope.interface import implementer |
62 | |
63 | @@ -83,8 +84,8 @@ class FAQ(SQLBase): |
64 | dbName='distribution', foreignKey='Distribution', notNull=False, |
65 | default=None) |
66 | |
67 | - related_questions = SQLMultipleJoin( |
68 | - 'Question', joinColumn='faq', orderBy=['Question.datecreated']) |
69 | + related_questions = ReferenceSet( |
70 | + 'id', 'Question.faq_id', order_by=('Question.datecreated')) |
71 | |
72 | @property |
73 | def target(self): |
74 | diff --git a/lib/lp/answers/model/question.py b/lib/lp/answers/model/question.py |
75 | index 9c51e88..f2fa7f4 100644 |
76 | --- a/lib/lp/answers/model/question.py |
77 | +++ b/lib/lp/answers/model/question.py |
78 | @@ -13,7 +13,10 @@ __all__ = [ |
79 | 'QuestionTargetMixin', |
80 | ] |
81 | |
82 | -from datetime import datetime |
83 | +from datetime import ( |
84 | + datetime, |
85 | + timedelta, |
86 | + ) |
87 | from email.utils import make_msgid |
88 | import operator |
89 | |
90 | @@ -28,16 +31,15 @@ from lazr.lifecycle.event import ( |
91 | from lazr.lifecycle.snapshot import Snapshot |
92 | import pytz |
93 | import six |
94 | -from sqlobject import ( |
95 | - ForeignKey, |
96 | - SQLMultipleJoin, |
97 | - SQLObjectNotFound, |
98 | - StringCol, |
99 | +from storm.expr import ( |
100 | + Alias, |
101 | + LeftJoin, |
102 | ) |
103 | -from storm.expr import LeftJoin |
104 | from storm.locals import ( |
105 | And, |
106 | ClassAlias, |
107 | + Count, |
108 | + DateTime, |
109 | Desc, |
110 | Int, |
111 | Join, |
112 | @@ -46,6 +48,7 @@ from storm.locals import ( |
113 | Reference, |
114 | Select, |
115 | Store, |
116 | + Unicode, |
117 | ) |
118 | from storm.references import ReferenceSet |
119 | from zope.component import getUtility |
120 | @@ -89,6 +92,7 @@ from lp.app.interfaces.launchpad import ILaunchpadCelebrities |
121 | from lp.bugs.interfaces.buglink import IBugLinkTarget |
122 | from lp.bugs.interfaces.bugtask import BugTaskStatus |
123 | from lp.bugs.model.buglinktarget import BugLinkTargetMixin |
124 | +from lp.bugs.model.bugtask import BugTask |
125 | from lp.registry.interfaces.distribution import ( |
126 | IDistribution, |
127 | IDistributionSet, |
128 | @@ -111,16 +115,11 @@ from lp.services.database.constants import ( |
129 | DEFAULT, |
130 | UTC_NOW, |
131 | ) |
132 | -from lp.services.database.datetimecol import UtcDateTimeCol |
133 | from lp.services.database.decoratedresultset import DecoratedResultSet |
134 | -from lp.services.database.enumcol import EnumCol |
135 | +from lp.services.database.enumcol import DBEnum |
136 | from lp.services.database.interfaces import IStore |
137 | from lp.services.database.nl_search import nl_phrase_search |
138 | -from lp.services.database.sqlbase import ( |
139 | - cursor, |
140 | - SQLBase, |
141 | - sqlvalues, |
142 | - ) |
143 | +from lp.services.database.stormbase import StormBase |
144 | from lp.services.database.stormexpr import ( |
145 | fti_search, |
146 | rank_by_fti, |
147 | @@ -137,6 +136,7 @@ from lp.services.worlddata.helpers import is_english_variant |
148 | from lp.services.worlddata.interfaces.language import ILanguage |
149 | from lp.services.worlddata.model.language import Language |
150 | from lp.services.xref.interfaces import IXRefSet |
151 | +from lp.services.xref.model import XRef |
152 | |
153 | |
154 | class notify_question_modified: |
155 | @@ -177,50 +177,50 @@ class notify_question_modified: |
156 | |
157 | |
158 | @implementer(IQuestion, IBugLinkTarget) |
159 | -class Question(SQLBase, BugLinkTargetMixin): |
160 | +class Question(StormBase, BugLinkTargetMixin): |
161 | """See `IQuestion`.""" |
162 | |
163 | - _table = 'Question' |
164 | + __storm_table__ = 'Question' |
165 | _defaultOrder = ['-priority', 'datecreated'] |
166 | |
167 | # db field names |
168 | - owner = ForeignKey( |
169 | - dbName='owner', foreignKey='Person', |
170 | - storm_validator=validate_public_person, notNull=True) |
171 | - title = StringCol(notNull=True) |
172 | - description = StringCol(notNull=True) |
173 | - language = ForeignKey( |
174 | - dbName='language', notNull=True, foreignKey='Language') |
175 | - status = EnumCol( |
176 | - schema=QuestionStatus, notNull=True, default=QuestionStatus.OPEN) |
177 | - priority = EnumCol( |
178 | - schema=QuestionPriority, notNull=True, |
179 | - default=QuestionPriority.NORMAL) |
180 | - assignee = ForeignKey( |
181 | - dbName='assignee', notNull=False, foreignKey='Person', |
182 | - storm_validator=validate_public_person, default=None) |
183 | - answerer = ForeignKey( |
184 | - dbName='answerer', notNull=False, foreignKey='Person', |
185 | - storm_validator=validate_public_person, default=None) |
186 | + id = Int(primary=True) |
187 | + owner_id = Int(name='owner', allow_none=False, |
188 | + validator=validate_public_person) |
189 | + owner = Reference(owner_id, 'Person.id') |
190 | + title = Unicode(allow_none=False) |
191 | + description = Unicode(allow_none=False) |
192 | + language_id = Int(name="language", allow_none=False) |
193 | + language = Reference(language_id, 'Language.id') |
194 | + status = DBEnum(name="status", enum=QuestionStatus, allow_none=False, |
195 | + default=QuestionStatus.OPEN) |
196 | + priority = DBEnum(name="priority", enum=QuestionPriority, |
197 | + allow_none=False, default=QuestionPriority.NORMAL) |
198 | + assignee_id = Int(name="assignee", allow_none=True, |
199 | + validator=validate_public_person, default=None) |
200 | + assignee = Reference(assignee_id, 'Person.id') |
201 | + answerer_id = Int(name="answerer", allow_none=True, |
202 | + validator=validate_public_person, default=None) |
203 | + answerer = Reference(answerer_id, 'Person.id') |
204 | answer_id = Int(name='answer', allow_none=True, default=None) |
205 | answer = Reference(answer_id, 'QuestionMessage.id') |
206 | - datecreated = UtcDateTimeCol(notNull=True, default=DEFAULT) |
207 | - datedue = UtcDateTimeCol(notNull=False, default=None) |
208 | - datelastquery = UtcDateTimeCol(notNull=True, default=DEFAULT) |
209 | - datelastresponse = UtcDateTimeCol(notNull=False, default=None) |
210 | - date_solved = UtcDateTimeCol(notNull=False, default=None) |
211 | - product = ForeignKey( |
212 | - dbName='product', foreignKey='Product', notNull=False, default=None) |
213 | - distribution = ForeignKey( |
214 | - dbName='distribution', foreignKey='Distribution', notNull=False, |
215 | - default=None) |
216 | - sourcepackagename = ForeignKey( |
217 | - dbName='sourcepackagename', foreignKey='SourcePackageName', |
218 | - notNull=False, default=None) |
219 | - whiteboard = StringCol(notNull=False, default=None) |
220 | - |
221 | - faq = ForeignKey( |
222 | - dbName='faq', foreignKey='FAQ', notNull=False, default=None) |
223 | + datecreated = DateTime(allow_none=False, default=DEFAULT, tzinfo=pytz.UTC) |
224 | + datedue = DateTime(allow_none=True, default=None, tzinfo=pytz.UTC) |
225 | + datelastquery = DateTime( |
226 | + allow_none=False, default=DEFAULT, tzinfo=pytz.UTC) |
227 | + datelastresponse = DateTime(allow_none=True, default=None, tzinfo=pytz.UTC) |
228 | + date_solved = DateTime(allow_none=True, default=None, tzinfo=pytz.UTC) |
229 | + product_id = Int(name='product', allow_none=True, default=None) |
230 | + product = Reference(product_id, 'Product.id') |
231 | + distribution_id = Int(name='distribution', allow_none=True, default=None) |
232 | + distribution = Reference(distribution_id, 'Distribution.id') |
233 | + sourcepackagename_id = Int( |
234 | + name='sourcepackagename', allow_none=True, default=None) |
235 | + sourcepackagename = Reference(sourcepackagename_id, 'SourcePackageName.id') |
236 | + whiteboard = Unicode(allow_none=True, default=None) |
237 | + |
238 | + faq_id = Int(name='faq', allow_none=True, default=None) |
239 | + faq = Reference(faq_id, 'FAQ.id') |
240 | |
241 | # useful joins |
242 | subscriptions = ReferenceSet( |
243 | @@ -238,6 +238,18 @@ class Question(SQLBase, BugLinkTargetMixin): |
244 | 'id', 'QuestionReopening.question_id', |
245 | order_by='QuestionReopening.datecreated') |
246 | |
247 | + def __init__(self, title, description, owner, product, distribution, |
248 | + language, sourcepackagename, datecreated): |
249 | + self.title = title |
250 | + self.description = description |
251 | + self.owner = owner |
252 | + self.product = product |
253 | + self.distribution = distribution |
254 | + self.sourcepackagename = sourcepackagename |
255 | + self.datecreated = datecreated |
256 | + self.language = language |
257 | + self.datelastquery = datecreated |
258 | + |
259 | @property |
260 | def messages(self): |
261 | return list(self._messages) |
262 | @@ -735,29 +747,28 @@ class QuestionSet: |
263 | # This query joins to bugtasks that are not BugTaskStatus.INVALID |
264 | # because there are many bugtasks to one question. A question is |
265 | # included when BugTask.status IS NULL. |
266 | - return Question.select(""" |
267 | - id in (SELECT Question.id |
268 | - FROM Question |
269 | - LEFT OUTER JOIN XRef ON ( |
270 | - XRef.from_type = 'question' |
271 | - AND XRef.from_id_int = Question.id |
272 | - AND XRef.to_type = 'bug') |
273 | - LEFT OUTER JOIN BugTask ON ( |
274 | - BugTask.bug = XRef.to_id_int |
275 | - AND BugTask.status != %s) |
276 | - WHERE |
277 | - Question.status IN (%s, %s) |
278 | - AND (Question.datelastresponse IS NULL |
279 | - OR Question.datelastresponse < (CURRENT_TIMESTAMP |
280 | - AT TIME ZONE 'UTC' - interval '%s days')) |
281 | - AND Question.datelastquery < (CURRENT_TIMESTAMP |
282 | - AT TIME ZONE 'UTC' - interval '%s days') |
283 | - AND Question.assignee IS NULL |
284 | - AND BugTask.status IS NULL) |
285 | - """ % sqlvalues( |
286 | - BugTaskStatus.INVALID, |
287 | - QuestionStatus.OPEN, QuestionStatus.NEEDSINFO, |
288 | - days_before_expiration, days_before_expiration)) |
289 | + origin = [ |
290 | + Question, |
291 | + LeftJoin(XRef, And( |
292 | + XRef.from_type == u'question', |
293 | + XRef.from_id_int == Question.id, |
294 | + XRef.to_type == u'bug')), |
295 | + LeftJoin(BugTask, And( |
296 | + BugTask.bug == XRef.to_id_int, |
297 | + BugTask._status != BugTaskStatus.INVALID)), |
298 | + ] |
299 | + expiry = datetime.now(pytz.UTC) - timedelta( |
300 | + days=days_before_expiration) |
301 | + return IStore(Question).using(*origin).find( |
302 | + Question, |
303 | + Question.status.is_in( |
304 | + (QuestionStatus.OPEN, QuestionStatus.NEEDSINFO)), |
305 | + Or( |
306 | + Question.datelastresponse == None, |
307 | + Question.datelastresponse < expiry), |
308 | + Question.datelastquery < expiry, |
309 | + Question.assignee == None, |
310 | + BugTask._status == None).config(distinct=True) |
311 | |
312 | def searchQuestions(self, search_text=None, language=None, |
313 | status=QUESTION_STATUS_DEFAULT_SEARCH, sort=None): |
314 | @@ -773,32 +784,33 @@ class QuestionSet: |
315 | |
316 | def getMostActiveProjects(self, limit=5): |
317 | """See `IQuestionSet`.""" |
318 | - cur = cursor() |
319 | - cur.execute(""" |
320 | - SELECT product, distribution, count(*) AS "question_count" |
321 | - FROM ( |
322 | - SELECT product, distribution |
323 | - FROM Question |
324 | - LEFT OUTER JOIN Product ON (Question.product = Product.id) |
325 | - LEFT OUTER JOIN Distribution ON ( |
326 | - Question.distribution = Distribution.id) |
327 | - WHERE |
328 | - ((Product.answers_usage = %s AND Product.active) |
329 | - OR Distribution.answers_usage = %s) |
330 | - AND Question.datecreated > ( |
331 | - current_timestamp -interval '60 days') |
332 | - LIMIT 5000 |
333 | - ) AS "RecentQuestions" |
334 | - GROUP BY product, distribution |
335 | - ORDER BY question_count DESC |
336 | - LIMIT %s |
337 | - """ % sqlvalues( |
338 | - ServiceUsage.LAUNCHPAD, ServiceUsage.LAUNCHPAD, limit)) |
339 | + |
340 | + from lp.registry.model.product import Product |
341 | + from lp.registry.model.distribution import Distribution |
342 | + |
343 | + time_cutoff = datetime.now(pytz.UTC) - timedelta(days=60) |
344 | + question_count = Alias(Count()) |
345 | + |
346 | + origin = (Question, |
347 | + LeftJoin(Product, Question.product_id == Product.id), |
348 | + LeftJoin(Distribution, |
349 | + Question.distribution_id == Distribution.id)) |
350 | + |
351 | + results = IStore(Question).using(*origin).find( |
352 | + (Question.product_id, Question.distribution_id, question_count), |
353 | + Or( |
354 | + And(Product._answers_usage == ServiceUsage.LAUNCHPAD, |
355 | + Product.active), |
356 | + Distribution._answers_usage == ServiceUsage.LAUNCHPAD), |
357 | + Question.datecreated > time_cutoff |
358 | + ).group_by( |
359 | + Question.product_id, Question.distribution_id |
360 | + ).order_by(Desc(question_count))[:limit] |
361 | |
362 | projects = [] |
363 | product_set = getUtility(IProductSet) |
364 | distribution_set = getUtility(IDistributionSet) |
365 | - for product_id, distribution_id, ignored in cur.fetchall(): |
366 | + for product_id, distribution_id, _ in results: |
367 | if product_id: |
368 | projects.append(product_set.get(product_id)) |
369 | elif distribution_id: |
370 | @@ -820,8 +832,7 @@ class QuestionSet: |
371 | question = Question( |
372 | title=title, description=description, owner=owner, |
373 | product=product, distribution=distribution, language=language, |
374 | - sourcepackagename=sourcepackagename, datecreated=datecreated, |
375 | - datelastquery=datecreated) |
376 | + sourcepackagename=sourcepackagename, datecreated=datecreated) |
377 | |
378 | # Subscribe the submitter |
379 | question.subscribe(owner) |
380 | @@ -830,10 +841,9 @@ class QuestionSet: |
381 | |
382 | def get(self, question_id, default=None): |
383 | """See `IQuestionSet`.""" |
384 | - try: |
385 | - return Question.get(question_id) |
386 | - except SQLObjectNotFound: |
387 | - return default |
388 | + store = IStore(Question) |
389 | + question = store.get(Question, question_id) |
390 | + return question or default |
391 | |
392 | def getOpenQuestionCountByPackages(self, packages): |
393 | """See `IQuestionSet`.""" |
394 | @@ -861,27 +871,17 @@ class QuestionSet: |
395 | package.sourcepackagename.id for package in packages] |
396 | open_statuses = [QuestionStatus.OPEN, QuestionStatus.NEEDSINFO] |
397 | |
398 | - query = """ |
399 | - SELECT Question.distribution, |
400 | - Question.sourcepackagename, |
401 | - COUNT(*) AS open_questions |
402 | - FROM Question |
403 | - WHERE Question.status IN %(open_statuses)s |
404 | - AND Question.sourcepackagename IN %(package_names)s |
405 | - AND Question.distribution = %(distribution)s |
406 | - GROUP BY Question.distribution, Question.sourcepackagename |
407 | - """ % sqlvalues( |
408 | - open_statuses=open_statuses, |
409 | - package_names=package_name_ids, |
410 | - distribution=distribution, |
411 | - ) |
412 | - cur = cursor() |
413 | - cur.execute(query) |
414 | + results = IStore(Question).find( |
415 | + (Question.distribution_id, Question.sourcepackagename_id, Count()), |
416 | + Question.status.is_in(open_statuses), |
417 | + Question.sourcepackagename_id.is_in(package_name_ids), |
418 | + Question.distribution == distribution, |
419 | + ).group_by(Question.distribution_id, Question.sourcepackagename_id) |
420 | sourcepackagename_set = getUtility(ISourcePackageNameSet) |
421 | # Only packages with open questions are included in the query |
422 | # result, so initialize each package to 0. |
423 | counts = dict((package, 0) for package in packages) |
424 | - for distro_id, spn_id, open_questions in cur.fetchall(): |
425 | + for distro_id, spn_id, open_questions in results: |
426 | # The SourcePackageNames here should already be pre-fetched, |
427 | # so that .get(spn_id) won't issue a DB query. |
428 | sourcepackagename = sourcepackagename_set.get(spn_id) |
429 | @@ -1024,7 +1024,7 @@ class QuestionSearch: |
430 | QuestionMessage.owner == self.needs_attention_from))) |
431 | |
432 | if self.language: |
433 | - constraints.append(Question.languageID.is_in( |
434 | + constraints.append(Question.language_id.is_in( |
435 | [language.id for language in self.language])) |
436 | |
437 | return constraints |
438 | @@ -1161,7 +1161,7 @@ class QuestionTargetSearch(QuestionSearch): |
439 | supported_languages = ( |
440 | self.unsupported_target.getSupportedLanguages()) |
441 | constraints.append( |
442 | - Not(Question.languageID.is_in( |
443 | + Not(Question.language_id.is_in( |
444 | [language.id for language in supported_languages]))) |
445 | |
446 | return constraints |
447 | @@ -1310,7 +1310,7 @@ class QuestionTargetMixin: |
448 | datecreated=bug.datecreated) |
449 | # Give the datelastresponse a current datetime, otherwise the |
450 | # Launchpad Janitor would quickly expire questions made from old bugs. |
451 | - question.datelastresponse = datetime.now(pytz.timezone('UTC')) |
452 | + question.datelastresponse = datetime.now(pytz.UTC) |
453 | # Directly create the BugLink so that users do not receive duplicate |
454 | # messages about the bug. |
455 | question.createBugLink(bug) |
456 | @@ -1327,9 +1327,9 @@ class QuestionTargetMixin: |
457 | |
458 | def getQuestion(self, question_id): |
459 | """See `IQuestionTarget`.""" |
460 | - try: |
461 | - question = Question.get(question_id) |
462 | - except SQLObjectNotFound: |
463 | + question = IStore(Question).find( |
464 | + Question, Question.id == question_id).one() |
465 | + if not question: |
466 | return None |
467 | # Verify that the question is actually for this target. |
468 | if not self.questionIsForTarget(question): |
469 | @@ -1349,18 +1349,11 @@ class QuestionTargetMixin: |
470 | |
471 | def getQuestionLanguages(self): |
472 | """See `IQuestionTarget`.""" |
473 | - constraints = ['Language.id = Question.language'] |
474 | - targets = self.getTargetTypes() |
475 | - for column, target in targets.items(): |
476 | - if target is None: |
477 | - constraint = "Question." + column + " IS NULL" |
478 | - else: |
479 | - constraint = "Question." + column + " = %s" % sqlvalues( |
480 | - target) |
481 | - constraints.append(constraint) |
482 | - return set(Language.select( |
483 | - ' AND '.join(constraints), |
484 | - clauseTables=['Question'], distinct=True)) |
485 | + query = [Language.id == Question.language_id] |
486 | + for column, target in self.getTargetTypes().items(): |
487 | + query.append(getattr(Question, column) == target) |
488 | + results = IStore(Question).find(Language, *query).config(distinct=True) |
489 | + return results |
490 | |
491 | @property |
492 | def _store(self): |
493 | @@ -1461,44 +1454,32 @@ class QuestionTargetMixin: |
494 | Store.of(answer_contact).flush() |
495 | return True |
496 | |
497 | - def _selectPersonFromAnswerContacts(self, constraints, clause_tables): |
498 | - """Return the Persons or Teams who are AnswerContacts.""" |
499 | - constraints.append("""Person.id = AnswerContact.person""") |
500 | - clause_tables.append('AnswerContact') |
501 | - # Avoid a circular import of Person, which imports the mixin. |
502 | - from lp.registry.model.person import Person |
503 | - return Person.select( |
504 | - " AND ".join(constraints), clauseTables=clause_tables, |
505 | - orderBy=['display_name'], distinct=True) |
506 | - |
507 | def getAnswerContactsForLanguage(self, language): |
508 | """See `IQuestionTarget`.""" |
509 | + from lp.registry.model.person import PersonLanguage, Person |
510 | assert language is not None, ( |
511 | "The language cannot be None when selecting answer contacts.") |
512 | - constraints = [] |
513 | + query = [] |
514 | targets = self.getTargetTypes() |
515 | for column, target in targets.items(): |
516 | - if target is None: |
517 | - constraint = "AnswerContact." + column + " IS NULL" |
518 | - else: |
519 | - constraint = "AnswerContact." + column + " = %s" % sqlvalues( |
520 | - target) |
521 | - constraints.append(constraint) |
522 | - |
523 | - constraints.append(""" |
524 | - AnswerContact.person = PersonLanguage.person AND |
525 | - PersonLanguage.Language = Language.id""") |
526 | + query.append(getattr(AnswerContact, column) == target) |
527 | + query.append( |
528 | + And( |
529 | + AnswerContact.person == PersonLanguage.person_id, |
530 | + PersonLanguage.language_id == Language.id)) |
531 | # XXX sinzui 2007-07-12 bug=125545: |
532 | # Using a LIKE constraint is suboptimal. We would not need this |
533 | # if-else clause if variant languages knew their parent language. |
534 | if language.code == 'en': |
535 | - constraints.append(""" |
536 | - Language.code LIKE %s""" % sqlvalues('%s%%' % language.code)) |
537 | + query.append(Language.code.startswith(language.code)) |
538 | else: |
539 | - constraints.append(""" |
540 | - Language.id = %s""" % sqlvalues(language)) |
541 | - return list((self._selectPersonFromAnswerContacts( |
542 | - constraints, ['PersonLanguage', 'Language']))) |
543 | + query.append(Language.id == language.id) |
544 | + |
545 | + query.append(AnswerContact.person == Person.id) |
546 | + results = IStore(Person).find( |
547 | + Person, *query).config(distinct=True).order_by( |
548 | + "Person.displayname") |
549 | + return results |
550 | |
551 | def getAnswerContactRecipients(self, language): |
552 | """See `IQuestionTarget`.""" |
553 | diff --git a/lib/lp/answers/model/tests/test_question.py b/lib/lp/answers/model/tests/test_question.py |
554 | index b13665c..0687f00 100644 |
555 | --- a/lib/lp/answers/model/tests/test_question.py |
556 | +++ b/lib/lp/answers/model/tests/test_question.py |
557 | @@ -2,11 +2,22 @@ |
558 | # GNU Affero General Public License version 3 (see the file LICENSE). |
559 | |
560 | from __future__ import absolute_import, print_function, unicode_literals |
561 | +from datetime import ( |
562 | + datetime, |
563 | + timedelta, |
564 | + ) |
565 | + |
566 | +import pytz |
567 | + |
568 | +from lp.services import database |
569 | + |
570 | |
571 | __metaclass__ = type |
572 | |
573 | from zope.component import getUtility |
574 | +from zope.security.proxy import removeSecurityProxy |
575 | |
576 | +from lp.answers.interfaces.questioncollection import IQuestionSet |
577 | from lp.services.worlddata.interfaces.language import ILanguageSet |
578 | from lp.testing import ( |
579 | person_logged_in, |
580 | @@ -101,3 +112,23 @@ class TestQuestionInDirectSubscribers(TestCaseWithFactory): |
581 | |
582 | # Check the results. |
583 | self.assertEqual([spanish_person], question.getIndirectSubscribers()) |
584 | + |
585 | + |
586 | +class TestQuestionSet(TestCaseWithFactory): |
587 | + |
588 | + layer = DatabaseFunctionalLayer |
589 | + |
590 | + def test_expiredQuestions(self): |
591 | + question = self.factory.makeQuestion() |
592 | + removeSecurityProxy(question).datelastquery = datetime.now( |
593 | + pytz.UTC) - timedelta(days=5) |
594 | + |
595 | + question_set = getUtility(IQuestionSet) |
596 | + expired = question_set.findExpiredQuestions(3) |
597 | + self.assertIn(question, expired) |
598 | + |
599 | + def test_expiredQuestionsDoesNotExpire(self): |
600 | + question = self.factory.makeQuestion() |
601 | + question_set = getUtility(IQuestionSet) |
602 | + expired = question_set.findExpiredQuestions(3) |
603 | + self.assertNotIn(question, expired) |
604 | diff --git a/lib/lp/answers/templates/question-portlet-reopenings.pt b/lib/lp/answers/templates/question-portlet-reopenings.pt |
605 | index 73e5ddc..ad459ec 100644 |
606 | --- a/lib/lp/answers/templates/question-portlet-reopenings.pt |
607 | +++ b/lib/lp/answers/templates/question-portlet-reopenings.pt |
608 | @@ -4,7 +4,7 @@ |
609 | xmlns:i18n="http://xml.zope.org/namespaces/i18n" |
610 | omit-tag=""> |
611 | <div id="portlet-reopenings" |
612 | - tal:condition="context/reopenings"> |
613 | + tal:condition="not: context/reopenings/is_empty"> |
614 | <h2>This question was reopened</h2> |
615 | |
616 | <ul> |
617 | diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py |
618 | index 14e917d..f49a04e 100644 |
619 | --- a/lib/lp/testing/factory.py |
620 | +++ b/lib/lp/testing/factory.py |
621 | @@ -2387,11 +2387,11 @@ class BareLaunchpadObjectFactory(ObjectFactory): |
622 | if target is None: |
623 | target = self.makeProduct() |
624 | if title is None: |
625 | - title = self.getUniqueString('title') |
626 | + title = self.getUniqueUnicode('title') |
627 | if owner is None: |
628 | owner = target.owner |
629 | if description is None: |
630 | - description = self.getUniqueString('description') |
631 | + description = self.getUniqueUnicode('description') |
632 | with person_logged_in(owner): |
633 | question = target.newQuestion( |
634 | owner=owner, title=title, description=description, **kwargs) |