Merge ~lgp171188/launchpad:allow-distribution-bug-supervisor-to-create-structural-subscriptions into launchpad:master

The intent of that additional check is to prevent random people from accidentally subscribing themselves to the flood of mail that is Ubuntu's bugs. I don't think we want to change that, and I don't think we want to relax the rule that people can't subscribe other unrelated people or teams -- ~ubuntu-bugcontrol is a fairly broad set of people.

Though reading further I see that that check would still be implemented in userCanAlterSubscription. Never mind me.

William,

> The intent of that additional check is to prevent random people from accidentally subscribing themselves to the flood of mail that is Ubuntu's bugs. I don't think we want to change that, and I don't think we want to relax the rule that people can't subscribe other unrelated people or teams -- ~ubuntu-bugcontrol is a fairly broad set of people.

In https://git.launchpad.net/launchpad/tree/lib/lp/bugs/model/structuralsubscription.py#n425, the comment explains exactly what I have implemented in this MP even though the current implementation doesn't match it - it checks the 'subscribed' person instead of the 'subscribed_by' person.

I implemented this change based on the feedback from Colin in comment #3 of https://answers.launchpad.net/launchpad/+question/707908.

Please let me know if I misunderstood something here.

Right, I think this is largely simple confusion between `subscriber` and `subscribed_by` - I don't feel that "only the bug supervisor or its members can be subscribed to the distribution's bugs" is a rule that's coherent with the rest of Launchpad, as opposed to "only the bug supervisor or its members can subscribe anyone to the distribution's bugs" which is what the comment says but not what the implementation does. Ironically, the person who asked the question that prompted this wrote this code to start with.

I don't like the location of the tests here, though. Looking through `tig blame` for the original code, I find that it came along with the addition of `lp.bugs.tests.test_structuralsubscriptiontarget.TestStructuralSubscriptionForDistro`, which still has several tests that overlap with the test cases you've added here (they just unfortunately didn't keep as clear a distinction between the subscriber and the person doing the subscribing as might have helped to avoid this problem). Could you move your tests there and rework them so that they use the same style as those existing tests?

Colin, while trying to port my existing tests to the `TestStructuralSubscriptionForDistro` test class, I found that fixing the checks in `userCanAlterBugSubscription()` like I have done is insufficient to allow the subscription mentioned in the linked question to happen. This is because, `addBugSubscription()` calls `userCanAlterBugSubscription()` and that returns `True` because of my changes in this MP. It then calls `addSubscription()` which, in turn, calls `userCanAlterSubscription()` and that doesn't allow this subscription to happen.

I am unsure of tweaking `userCanAlterSubscription()` since that _could_ violate the restriction that William mentioned in his first comment. So what can I do here?