Merge lp:~thekorn/launchpad/make_iperson_ihasbugs into lp:launchpad
| Status: | Merged |
|---|---|
| Approved by: | Gavin Panella on 2010-02-12 |
| Approved revision: | not available |
| Merge reported by: | Gavin Panella |
| Merged at revision: | not available |
| Proposed branch: | lp:~thekorn/launchpad/make_iperson_ihasbugs |
| Merge into: | lp:launchpad |
| Diff against target: |
975 lines (+372/-88) 22 files modified
lib/canonical/launchpad/interfaces/_schema_circular_imports.py (+55/-2) lib/canonical/launchpad/interfaces/message.py (+4/-6) lib/lp/bugs/doc/bugtask-search.txt (+12/-1) lib/lp/bugs/interfaces/bug.py (+5/-6) lib/lp/bugs/interfaces/bugtarget.py (+20/-18) lib/lp/bugs/interfaces/bugtask.py (+9/-2) lib/lp/bugs/interfaces/bugtracker.py (+1/-2) lib/lp/bugs/interfaces/bugwatch.py (+1/-2) lib/lp/bugs/model/bugtask.py (+46/-2) lib/lp/bugs/stories/webservice/xx-bug.txt (+74/-4) lib/lp/registry/configure.zcml (+2/-0) lib/lp/registry/interfaces/distribution.py (+0/-7) lib/lp/registry/interfaces/distributionsourcepackage.py (+4/-3) lib/lp/registry/interfaces/distroseries.py (+5/-3) lib/lp/registry/interfaces/milestone.py (+3/-2) lib/lp/registry/interfaces/person.py (+2/-13) lib/lp/registry/interfaces/product.py (+0/-7) lib/lp/registry/interfaces/productseries.py (+3/-2) lib/lp/registry/interfaces/project.py (+3/-2) lib/lp/registry/interfaces/sourcepackage.py (+3/-2) lib/lp/registry/model/person.py (+18/-2) lib/lp/registry/tests/test_person.py (+102/-0) |
| To merge this branch: | bzr merge lp:~thekorn/launchpad/make_iperson_ihasbugs |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Gavin Panella (community) | 2010-02-03 | Approve on 2010-02-12 | |
| Markus Korn (community) | Resubmit on 2010-02-12 | ||
| Eleanor Berger (community) | code | 2010-02-04 | Approve on 2010-02-11 |
|
Review via email:
|
|||
Commit Message
Person.
| Markus Korn (thekorn) wrote : | # |
| Gavin Panella (allenap) wrote : | # |
Hi Markus,
I've added some comments to the diff below. It looks pretty good,
thank you so much for doing this.
Because I worked closely on you on this branch, I'm going to ask
someone else to review it too.
Gavin.
> === modified file 'lib/canonical/
> --- lib/canonical/
> +++ lib/canonical/
> @@ -24,11 +24,11 @@
>
> from lp.registry.
> IStructuralSubs
> -from lp.bugs.
> +from lp.bugs.
> from lp.bugs.
> from lp.bugs.
> from lp.bugs.
> -from lp.bugs.
> +from lp.bugs.
> from lp.soyuz.
> BuildStatus, IBuild)
> from lp.soyuz.
> @@ -69,6 +69,11 @@
> from lp.soyuz.
> IPackageUpload, PackageUploadCu
> from lp.registry.
> +from canonical.
> + IIndexedMessage, IMessage, IUserToUserEmail)
> +
> +from lp.bugs.
> +from lp.bugs.
>
>
> IBranch[
> @@ -312,5 +317,53 @@
> patch_reference
> IStructuralSubs
> IStructuralSubs
> -
> +
> IBuildBase[
> +
> +# IHasBugs
> +patch_
> + IHasBugs, 'searchTasks', 'assignee', IPerson)
> +patch_
> + IHasBugs, 'searchTasks', 'bug_reporter', IPerson)
> +patch_
> + IHasBugs, 'searchTasks', 'bug_supervisor', IPerson)
> +patch_
> + IHasBugs, 'searchTasks', 'bug_commenter', IPerson)
> +patch_
> + IHasBugs, 'searchTasks', 'bug_subscriber', IPerson)
> +patch_
> + IHasBugs, 'searchTasks', 'owner', IPerson)
> +patch_
> + IHasBugs, 'searchTasks', 'affected_user', IPerson)
> +
> +# IBugTask
> +patch_
> +
> +# IBugWatch
> +patch_
> +
> +# IIndexedMessage
> +patch_
> +
> +# IMessage
> +patch_
> +
> +# IUserToUserEmail
> +patch_
> +patch_
> +
> +# IBug
> +patch_
> + IBug, 'addNomination', 'target', IBugTarget)
> +patch_
> + IBug, 'canBeNominated
> +patch_
> +...
| Eleanor Berger (intellectronica) wrote : | # |
Markus (and Gavin), thanks a lot for picking this up. Excellent branch! I have no comments beyond what Gavin already mentioned in his review. I'm leaving the branch in NEEDS FIXING, since you'll need to fix them and then ask one of us to merge it on your behalf, but this shouldn't be much work.
- 10114. By Markus Korn on 2010-02-04
-
fixed typo
- 10115. By Markus Korn on 2010-02-04
-
started to address some comments of the review
- 10116. By Markus Korn on 2010-02-08
-
merged devel
- 10117. By Markus Korn on 2010-02-10
-
* merged devel
* fixed conflicting base classes of IPersonPublic - 10118. By Markus Korn on 2010-02-10
-
* Added IllegalRelatedB
ugTasksParams Exception which translates to a 400
HTTP error in the webservice API and will be raised in cases of invalid
queries for related tasks - 10119. By Markus Korn on 2010-02-10
-
* moved creation of BugTaskSearchParams for Person.
searchTasks( ) to an
external function.
* added more comments to document the behaviour of this function
* raise IllegalRelatedBugTasksParams exception in case it is impossible to
execute Person.searchTasks( ) because of invalid number of parameters - 10120. By Markus Korn on 2010-02-10
-
* moved doctest for Person.
searchTasks( ) to xx-bug.txt - 10121. By Markus Korn on 2010-02-10
-
* added unittests for getRelatedBugTa
sksParams
| Markus Korn (thekorn) wrote : | # |
Thanks for your review Gavin, I tried to address as many points as possible in the recent commits, most importantly I moved the logic to create search parameters into a separate function, added a few more descriptive comments/
| Eleanor Berger (intellectronica) wrote : | # |
Very nice. r=me.
Set the commit message and I'll land this on your behalf.
Thanks a lot for working on this!
| Gavin Panella (allenap) wrote : | # |
Hi Markus,
I've gone through each revision in turn since the last review; I
couldn't figure out how to get a cumulative diff without pulling in
all the merges from devel.
There are still a few things left to do to get this ready for landing,
but they're not huge, and I've done some of them (a few more unit
tests). It's really close though.
Gavin.
Revision 10114:
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -65,7 +65,7 @@
> == Person bugs ==
>
> To get all related tasks to a person call searchTasks() on the person
> -pbject:
> +object:
>
> >>> from canonical.
> >>> user = getUtility(
>
Revision 10115:
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -31,7 +31,6 @@
> import random
> import re
> import weakref
> -import copy
>
> from zope.lifecycleevent import ObjectCreatedEvent
> from zope.interface import alsoProvides, implementer, implements
> @@ -838,16 +837,20 @@
>
> def searchTasks(self, search_params, *args, **kwargs):
> """See `IHasBugs`."""
> - if search_params is None and not args:
> + if search_params is None and len(args) == 0:
> # this method is called via webapi directly
> - args = list()
> + args = []
> for key in ('assignee', 'bug_subscriber', 'owner', 'bug_commenter'):
> - if kwargs.get(key, None) is None:
> - arguments = copy.copy(kwargs)
> + # all these parameter default to None
> +
> + if kwargs.get(key) is None:
> +
> + arguments = kwargs.copy()
> arguments[key] = self
> if key == 'owner':
> - # Specify both owner and bug_reporter to try to prevent the same
> - # bug (but different tasks) being displayed.
> + # Specify both owner and bug_reporter to try to
> + # prevent the same bug (but different tasks)
> + # being displayed.
> # see `PersonRelatedB
> arguments[
> args.append(
>
Revision 10118:
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -25,6 +25,7 @@
> 'ITeamContactAd
> 'ITeamCreation',
> 'ITeamReassignm
> + 'IllegalRelated
> 'ImmutableVisib
> 'InvalidName',
> 'JoinNotAllowed',
> @@ -2057,6 +2058,12 @@
> webservice_
>
>
> +class IllegalRelatedB
> + """Exception raised when trying to ov...
- 10122. By Markus Korn on 2010-02-11
-
fixed typo in docstring of IllegalRelatedB
ugTasksParams - 10123. By Markus Korn on 2010-02-11
-
* renamed `getRelatedBugT
asksParams` into
`get_related_ bugtasks_ search_ params` and moved this function to
lp.bugs.model.bugtask
* checking type of context argument of get_related_bugtasks_ search_ params( )
* - 10124. By Markus Korn on 2010-02-11
-
* moved IllegalRelatedB
ugTasksParams to lp.bugs. interfaces. bugtask - 10125. By Markus Korn on 2010-02-11
-
* added webservice API test for IllegalRelatedB
ugTasksParams - 10126. By Markus Korn on 2010-02-11
-
* Merged Gavin Panella's changes to the unittests for the related bug task
search. - 10127. By Markus Korn on 2010-02-12
-
added more detailed doctests to Person.
searchTasks( )
| Markus Korn (thekorn) wrote : | # |
With r10127 I *think* I've addressed all of Gavin's comments
- 10128. By Markus Korn on 2010-02-12
-
removed trailing whitespaces from lib/canonical/
launchpad/ interfaces/ _schema_ circular_ imports. py - 10129. By Markus Korn on 2010-02-12
-
removed trailing whitespaces from lib/lp/
bugs/doc/ bugtask- search. txt - 10130. By Markus Korn on 2010-02-12
-
removed trailing whitespaces from lib/lp/
bugs/interfaces /bugtask. py - 10131. By Markus Korn on 2010-02-12
-
removed trailing whitespaces from lib/lp/
bugs/stories/ webservice/ xx-bug. txt - 10132. By Markus Korn on 2010-02-12
-
merged with devel
- 10133. By Markus Korn on 2010-02-12
-
removed trailing whitespaces from lib/lp/
registry/ interfaces/ distributionsou rcepackage. py - 10134. By Markus Korn on 2010-02-12
-
removed trailing whitespaces from lib/lp/
registry/ interfaces/ distroseries. py
| Markus Korn (thekorn) wrote : | # |
fixed bunch of trailing whitespaces.
Now I get messages from `make lint` for code I did not even touch directly
| Gavin Panella (allenap) wrote : | # |
As discussed on IRC earlier, you accidentally merged db-devel instead of devel at revision 10132. My branch lp:~allenap/launchpad/make_iperson_ihasbugs fixes this and also removes the remaining trailing white-space from the branch.
Right, I'll get it landed now :)
| Gavin Panella (allenap) wrote : | # |
Merged via lp:~allenap/launchpad/make_iperson_ihasbugs.

This is the first step to fix bug 282178 (making Person.searchTasks available over the webservice API)
This branch includes three different types of changes: searchTasks( ) in a way that it can be called without BugTaskSearchParams and only by giving keyword arguments. Person. searchTasks( ) returns the same list of tasks which is shown as "related Bugs" in bugs.launchpad. net/~username.
* implemented Person.
* as a person cannot have official_bug_tags this field is moved away from IHasBugs - thanks to Gavin Panella for helping me with the related changes.
* added testcases for the internal API and the webservice API
The seconds step to fix the bug mentioned above will be to add (alias like) methods to IPerson (searchCommente dTasks( ), searchSubscribe dTasks( ) ...)