Code review comment for lp:~gmb/launchpad/launchpadformify-subscribe-view-bug-651240

Looks good. Took me a while before I realised that it's mostly moved
code. Because of that, this is landable as-is, though my comments
below still apply.

[1]

+ if referer and referer not in ('localhost', self.request.getURL()):

Implicit booleanism. You could express this in another way:

        if referer in (None, '', 'localhost', self.request.getURL()):
            next_url = canonical_url(self.context)
        else:
            next_url = referer

[2]

+ persons_for_user = {}
+ person_count = 0
+ bug = self.context.bug
+ for person in bug.getSubscribersForPerson(self.user):
+ if person.id not in persons_for_user:
+ persons_for_user[person.id] = person
+ person_count += 1
+
+ self._subscriber_count_for_current_user = person_count
+ return persons_for_user.values()

I think that person_count is equivalent to len(persons_for_user).

Having said that, _subscriber_count_for_current_user doesn't appear to
be used anywhere.

Also, based on how it's used, _subscribers_for_current_user could be
simplified to:

    @cachedproperty
    def _subscribers_for_current_user(self):
        """Return a dict of the subscribers for the current user."""
        return set(bug.getSubscribersForPerson(self.user))

[3]

+ 'Unsubscribe <a href="%s">%s</a> from this bug' % (
+ canonical_url(person),

The canonical_url(person) may need to be cgi.escape()d too.

[4]

+ def _isSubscriptionRequest(self):
+ """Return True if the form contains subscribe/unsubscribe input."""
+ return (
+ self.user and
+ self.request.method == 'POST' and
+ 'cancel' not in self.request.form and
+ self.widgets['subscription'].hasValidInput())

Could this be replaced by referring to
self.subscribe_action.submitted()?

Actually, it doesn't seem to be used anymore.

Okay, I'll stop going through this stuff so finely as I now see that
it's mostly moved code.