Code review comment for lp:~adeuring/launchpad/bug-39674-lfa-editable

Looks good, thank you!

What do you think of this instead of the try/except blocks on the test?

    self.assertRaises(setattr(self.bug_attachment.title, 'whatever'), Unauthorized)
    self.assertRaises(self.lfa_with_parent.restricted, True), Unauthorized)

It's worth noting some of the other ideas we discussed and rejected in the course of the branch.

- Set a __parent__ on the library object in the database. The problems are that library objects may have multiple parents in theory; and, more importantly, they may have multiple *types* of parents, and making a reference to an arbitrary object is not trivial in Launchpad at the moment given our table structures. This would be my preferred solution in the long run.

- Set a __parent__ on the object during traversal. The mechanism implemented here could be used for that, but would be more heavyweight than would be needed--you could just slam a transient __parent__ on the file alias instance. Problems: we didn't care for this pattern because it had hidden fragility. The current approach clearly exposes the situation for what it is. This alternative is the one that I wonder if it would have been better, but I'm happy with the call that we made.

- Make a method on the file alias to change the restricted attribute that takes a security context and then handles security checks itself. I wanted to keep security handled by the usual machinery, and I wanted to keep security concerns out of the model code.

- Use another sort of mediator, like a function. I wanted to keep security handled by the usual machinery. Using the approach we have here keeps us in the usual patterns.