Code review comment for lp:~sinzui/launchpad/closed-teams-0

Hi Curtis. Some incredibly minor comments, +1.

[1]

         js = js_template % simplejson.dumps(args)
         # If the YUI widget or javascript is not supported in the browser,
         # it will degrade to being this "Find..." link instead of the
@@ -154,8 +163,7 @@
             css = ''
         return ('<span class="%s">(<a id="%s" href="/people/">'
                 'Find&hellip;</a>)</span>'
- '\n<script>\n%s\n</script>'
- ) % (css, self.show_widget_id, js)
+ '\n<script>\n%s\n</script>') % (css, self.show_widget_id, js)

It's not part of your change, but using simplejson.dumps() can be
hazardous for embedding javascript into <script> tags. See:

  http://code.google.com/p/simplejson/issues/detail?id=66

Instead the following should be used:

  simplejson.dumps(args, cls=simplejson.encoder.JSONEncoderForHTML)

Or:

  simplejson.encoder.JSONEncoderForHTML().encode(obj)

[2]

+ Y.lp.registry.team.setup_add_member_handler(
+ '${step_title}');

The lack of escaping and all that jazz makes me a little uneasy, but
it's not user-supplied so I'm not too worried.

[3]

+ def test_open_team_cannot_be_a_member_or_a_closed_team(self):

s/or/of/

[4]

+ def test_open_team_can_be_a_member_or_an_open_team(self):

s/or/of/

[5]

+ return 'Search for a restricted or moderated team or person'

This could be parsed as:

  Search for a (restricted or moderated) (team or person)

Perhaps the following would be less ambiguous:

  Search for a restricted team, a moderated team, or a person

?

[6]

+ Person.subscriptionpolicy != TeamSubscriptionPolicy.OPEN)

Maybe it's worth taking a defensive approach here (to someone adding
another TeamSubscriptionPolicy member) instead:

  In(Person.subscriptionpolicy, (
      TeamSubscriptionPolicy.RESTRICTED,
      TeamSubscriptionPolicy.MODERATED))

The same clause appears twice.