launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bugtasksearch-interfaces-2 into lp:launchpad with lp:~wgrant/launchpad/bugtasksearch-interfaces-1 as a prerequisite.

Requested reviews:
  William Grant (wgrant)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bugtasksearch-interfaces-2/+merge/118468

Months ago I split the core bugtask search model code out of lp.bugs.model.bugtask into lp.bugs.model.bugtasksearch. This branch is the third in the interface split, moving most of the remnants across. model.bugtask.get_related_bugtasks_search_params becomes interfaces.bugtask.get_person_bugtasks_search_params, more accurately reflecting its function and purpose, and a related exception moves too.
-- 
https://code.launchpad.net/~wgrant/launchpad/bugtasksearch-interfaces-2/+merge/118468
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

=== modified file 'lib/lp/bugs/interfaces/bugtask.py'
--- lib/lp/bugs/interfaces/bugtask.py	2012-08-07 03:54:21 +0000
+++ lib/lp/bugs/interfaces/bugtask.py	2012-08-07 03:54:21 +0000
@@ -24,7 +24,6 @@
     'IBugTaskDelta',
     'IBugTaskSet',
     'ICreateQuestionFromBugTaskForm',
-    'IllegalRelatedBugTasksParams',
     'IllegalTarget',
     'INominationsReviewTableBatchNavigator',
     'IRemoveQuestionFromBugTaskForm',
@@ -378,12 +377,6 @@
     """Exception raised when trying to set an illegal bug task target."""
 
 
-@error_status(httplib.BAD_REQUEST)
-class IllegalRelatedBugTasksParams(Exception):
-    """Exception raised when trying to overwrite all relevant parameters
-    in a search for related bug tasks"""
-
-
 class IBugTaskDelete(Interface):
     """An interface for operations allowed with the Delete permission."""
     @export_destructor_operation()

=== modified file 'lib/lp/bugs/interfaces/bugtasksearch.py'
--- lib/lp/bugs/interfaces/bugtasksearch.py	2012-08-07 03:54:21 +0000
+++ lib/lp/bugs/interfaces/bugtasksearch.py	2012-08-07 03:54:21 +0000
@@ -11,18 +11,22 @@
     'BugTagsSearchCombinator',
     'BugTaskSearchParams',
     'DEFAULT_SEARCH_BUGTASK_STATUSES_FOR_DISPLAY',
+    'get_person_bugtasks_search_params',
     'IBugTaskSearch',
+    'IllegalRelatedBugTasksParams',
     'IFrontPageBugTaskSearch',
     'IPersonBugTaskSearch',
     'IUpstreamProductBugTaskSearch',
     ]
 
 import collections
+import httplib
 
 from lazr.enum import (
     EnumeratedType,
     Item,
     )
+from lazr.restful.declarations import error_status
 from zope.interface import Interface
 from zope.schema import (
     Bool,
@@ -53,6 +57,12 @@
 from lp.soyuz.interfaces.component import IComponent
 
 
+@error_status(httplib.BAD_REQUEST)
+class IllegalRelatedBugTasksParams(Exception):
+    """Exception raised when trying to overwrite all relevant parameters
+    in a search for related bug tasks"""
+
+
 class BugBranchSearch(EnumeratedType):
     """Bug branch search option.
 
@@ -647,3 +657,48 @@
     scope = Choice(
         title=u"Search Scope", required=False,
         vocabulary="DistributionOrProductOrProjectGroup")
+
+
+def get_person_bugtasks_search_params(user, context, **kwargs):
+    """Returns a list of `BugTaskSearchParams` which can be used to
+    search for all tasks related to a user given by `context`.
+
+    Which tasks are related to a user?
+      * the user has to be either assignee or owner of this task
+        OR
+      * the user has to be subscriber or commenter to the underlying bug
+        OR
+      * the user is reporter of the underlying bug, but this condition
+        is automatically fulfilled by the first one as each new bug
+        always get one task owned by the bug reporter
+    """
+    from lp.registry.interfaces.person import IPerson
+    assert IPerson.providedBy(context), "Context argument needs to be IPerson"
+    relevant_fields = ('assignee', 'bug_subscriber', 'owner', 'bug_commenter',
+                       'structural_subscriber')
+    search_params = []
+    for key in relevant_fields:
+        # all these parameter default to None
+        user_param = kwargs.get(key)
+        if user_param is None or user_param == context:
+            # we are only creating a `BugTaskSearchParams` object if
+            # the field is None or equal to the context
+            arguments = kwargs.copy()
+            arguments[key] = context
+            if key == 'owner':
+                # Specify both owner and bug_reporter to try to
+                # prevent the same bug (but different tasks)
+                # being displayed.
+                # see `PersonRelatedBugTaskSearchListingView.searchUnbatched`
+                arguments['bug_reporter'] = context
+            search_params.append(
+                BugTaskSearchParams.fromSearchForm(user, **arguments))
+    if len(search_params) == 0:
+        # unable to search for related tasks to user_context because user
+        # modified the query in an invalid way by overwriting all user
+        # related parameters
+        raise IllegalRelatedBugTasksParams(
+            ('Cannot search for related tasks to \'%s\', at least one '
+             'of these parameter has to be empty: %s'
+                % (context.name, ", ".join(relevant_fields))))
+    return search_params

=== modified file 'lib/lp/bugs/interfaces/webservice.py'
--- lib/lp/bugs/interfaces/webservice.py	2011-12-22 16:19:11 +0000
+++ lib/lp/bugs/interfaces/webservice.py	2012-08-07 03:54:21 +0000
@@ -63,13 +63,13 @@
     )
 from lp.bugs.interfaces.bugtask import (
     IBugTask,
-    IllegalRelatedBugTasksParams,
     IllegalTarget,
     UserCannotEditBugTaskAssignee,
     UserCannotEditBugTaskImportance,
     UserCannotEditBugTaskMilestone,
     UserCannotEditBugTaskStatus,
     )
+from lp.bugs.interfaces.bugtasksearch import IllegalRelatedBugTasksParams
 from lp.bugs.interfaces.bugtracker import (
     IBugTracker,
     IBugTrackerComponent,

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2012-08-07 03:54:21 +0000
+++ lib/lp/bugs/model/bugtask.py	2012-08-07 03:54:21 +0000
@@ -15,7 +15,6 @@
     'bugtask_sort_key',
     'bug_target_from_key',
     'bug_target_to_key',
-    'get_related_bugtasks_search_params',
     'validate_new_target',
     'validate_target',
     ]
@@ -87,7 +86,6 @@
     IBugTask,
     IBugTaskDelta,
     IBugTaskSet,
-    IllegalRelatedBugTasksParams,
     IllegalTarget,
     normalize_bugtask_status,
     RESOLVED_BUGTASK_STATUSES,
@@ -112,7 +110,6 @@
 from lp.registry.interfaces.milestone import IMilestoneSet
 from lp.registry.interfaces.milestonetag import IProjectGroupMilestoneTag
 from lp.registry.interfaces.person import (
-    IPerson,
     validate_person,
     validate_public_person,
     )
@@ -196,50 +193,6 @@
         distroseries_name, sourcepackage_name)
 
 
-def get_related_bugtasks_search_params(user, context, **kwargs):
-    """Returns a list of `BugTaskSearchParams` which can be used to
-    search for all tasks related to a user given by `context`.
-
-    Which tasks are related to a user?
-      * the user has to be either assignee or owner of this task
-        OR
-      * the user has to be subscriber or commenter to the underlying bug
-        OR
-      * the user is reporter of the underlying bug, but this condition
-        is automatically fulfilled by the first one as each new bug
-        always get one task owned by the bug reporter
-    """
-    assert IPerson.providedBy(context), "Context argument needs to be IPerson"
-    relevant_fields = ('assignee', 'bug_subscriber', 'owner', 'bug_commenter',
-                       'structural_subscriber')
-    search_params = []
-    for key in relevant_fields:
-        # all these parameter default to None
-        user_param = kwargs.get(key)
-        if user_param is None or user_param == context:
-            # we are only creating a `BugTaskSearchParams` object if
-            # the field is None or equal to the context
-            arguments = kwargs.copy()
-            arguments[key] = context
-            if key == 'owner':
-                # Specify both owner and bug_reporter to try to
-                # prevent the same bug (but different tasks)
-                # being displayed.
-                # see `PersonRelatedBugTaskSearchListingView.searchUnbatched`
-                arguments['bug_reporter'] = context
-            search_params.append(
-                BugTaskSearchParams.fromSearchForm(user, **arguments))
-    if len(search_params) == 0:
-        # unable to search for related tasks to user_context because user
-        # modified the query in an invalid way by overwriting all user
-        # related parameters
-        raise IllegalRelatedBugTasksParams(
-            ('Cannot search for related tasks to \'%s\', at least one '
-             'of these parameter has to be empty: %s'
-                % (context.name, ", ".join(relevant_fields))))
-    return search_params
-
-
 def bug_target_from_key(product, productseries, distribution, distroseries,
                         sourcepackagename):
     """Returns the IBugTarget defined by the given DB column values."""

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2012-08-07 03:54:21 +0000
+++ lib/lp/registry/model/person.py	2012-08-07 03:54:21 +0000
@@ -137,7 +137,7 @@
 from lp.bugs.interfaces.bugtask import IBugTaskSet
 from lp.bugs.interfaces.bugtasksearch import BugTaskSearchParams
 from lp.bugs.model.bugtarget import HasBugsBase
-from lp.bugs.model.bugtask import get_related_bugtasks_search_params
+from lp.bugs.interfaces.bugtasksearch import get_person_bugtasks_search_params
 from lp.bugs.model.structuralsubscription import StructuralSubscription
 from lp.code.interfaces.branchcollection import (
     IAllBranches,
@@ -1028,7 +1028,7 @@
             # calling this method on a Person object directly via the
             # webservice API means searching for user related tasks
             user = kwargs.pop('user')
-            search_params = get_related_bugtasks_search_params(
+            search_params = get_person_bugtasks_search_params(
                 user, self, **kwargs)
             return getUtility(IBugTaskSet).search(
                 *search_params, prejoins=prejoins)

=== modified file 'lib/lp/registry/tests/test_person.py'
--- lib/lp/registry/tests/test_person.py	2012-07-26 15:03:37 +0000
+++ lib/lp/registry/tests/test_person.py	2012-08-07 03:54:21 +0000
@@ -26,7 +26,7 @@
 from lp.blueprints.model.specification import Specification
 from lp.bugs.interfaces.bugtask import IllegalRelatedBugTasksParams
 from lp.bugs.model.bug import Bug
-from lp.bugs.model.bugtask import get_related_bugtasks_search_params
+from lp.bugs.interfaces.bugtasksearch import get_person_bugtasks_search_params
 from lp.registry.enums import InformationType
 from lp.registry.errors import PrivatePersonLinkageError
 from lp.registry.interfaces.karma import IKarmaCacheManager
@@ -785,11 +785,11 @@
         self.failUnlessEqual(structural_subscriber,
                              params.structural_subscriber)
 
-    def test_get_related_bugtasks_search_params(self):
-        # With no specified options, get_related_bugtasks_search_params()
+    def test_get_person_bugtasks_search_params(self):
+        # With no specified options, get_person_bugtasks_search_params()
         # returns 5 BugTaskSearchParams objects, each with a different
         # user field set.
-        search_params = get_related_bugtasks_search_params(
+        search_params = get_person_bugtasks_search_params(
             self.user, self.context)
         self.assertEqual(len(search_params), 5)
         self.checkUserFields(
@@ -803,10 +803,10 @@
         self.checkUserFields(
             search_params[4], structural_subscriber=self.context)
 
-    def test_get_related_bugtasks_search_params_with_assignee(self):
-        # With assignee specified, get_related_bugtasks_search_params()
+    def test_get_person_bugtasks_search_params_with_assignee(self):
+        # With assignee specified, get_person_bugtasks_search_params()
         # returns 4 BugTaskSearchParams objects.
-        search_params = get_related_bugtasks_search_params(
+        search_params = get_person_bugtasks_search_params(
             self.user, self.context, assignee=self.user)
         self.assertEqual(len(search_params), 4)
         self.checkUserFields(
@@ -820,10 +820,10 @@
             search_params[3], assignee=self.user,
             structural_subscriber=self.context)
 
-    def test_get_related_bugtasks_search_params_with_owner(self):
-        # With owner specified, get_related_bugtasks_search_params() returns
+    def test_get_person_bugtasks_search_params_with_owner(self):
+        # With owner specified, get_person_bugtasks_search_params() returns
         # 4 BugTaskSearchParams objects.
-        search_params = get_related_bugtasks_search_params(
+        search_params = get_person_bugtasks_search_params(
             self.user, self.context, owner=self.user)
         self.assertEqual(len(search_params), 4)
         self.checkUserFields(
@@ -836,11 +836,11 @@
             search_params[3], owner=self.user,
             structural_subscriber=self.context)
 
-    def test_get_related_bugtasks_search_params_with_bug_reporter(self):
-        # With bug reporter specified, get_related_bugtasks_search_params()
+    def test_get_person_bugtasks_search_params_with_bug_reporter(self):
+        # With bug reporter specified, get_person_bugtasks_search_params()
         # returns 4 BugTaskSearchParams objects, but the bug reporter
         # is overwritten in one instance.
-        search_params = get_related_bugtasks_search_params(
+        search_params = get_person_bugtasks_search_params(
             self.user, self.context, bug_reporter=self.user)
         self.assertEqual(len(search_params), 5)
         self.checkUserFields(
@@ -861,19 +861,19 @@
             search_params[4], bug_reporter=self.user,
             structural_subscriber=self.context)
 
-    def test_get_related_bugtasks_search_params_illegal(self):
+    def test_get_person_bugtasks_search_params_illegal(self):
         self.assertRaises(
             IllegalRelatedBugTasksParams,
-            get_related_bugtasks_search_params, self.user, self.context,
+            get_person_bugtasks_search_params, self.user, self.context,
             assignee=self.user, owner=self.user, bug_commenter=self.user,
             bug_subscriber=self.user, structural_subscriber=self.user)
 
-    def test_get_related_bugtasks_search_params_illegal_context(self):
+    def test_get_person_bugtasks_search_params_illegal_context(self):
         # in case the `context` argument is not  of type IPerson an
         # AssertionError is raised
         self.assertRaises(
             AssertionError,
-            get_related_bugtasks_search_params, self.user, "Username",
+            get_person_bugtasks_search_params, self.user, "Username",
             assignee=self.user)