Code review comment for lp:~bac/launchpad/bug-602773

Hi Brad,

I'm glad you worked did this, since it doesn't look like fun, but it's very important. I'm wondering if the valid_person storm_validator is even necessary, since it appears to only to check whether IPerson.providedBy() is true. We can discuss this more when I get back from lunch. I'm marking it needs-fixing until then.

I have just one comment below.

-Edwin

>=== modified file 'lib/lp/registry/vocabularies.py'
>--- lib/lp/registry/vocabularies.py 2010-06-04 09:31:21 +0000
>+++ lib/lp/registry/vocabularies.py 2010-07-28 15:12:59 +0000
>@@ -442,8 +442,7 @@
> private_query = AND(
> Not(Person.teamowner == None),
> OR(
>- Person.visibility == PersonVisibility.PRIVATE,
>- Person.visibility == PersonVisibility.PRIVATE_MEMBERSHIP))
>+ Person.visibility == PersonVisibility.PRIVATE))
>

You can get rid of the OR() since it only has a single argument.
I'm surprised that it doesn't raise an exception.

>
> else:
> private_query = AND(
> TeamParticipation.person == logged_in_user.id,