Code review comment for lp:~brian-murray/launchpad/limited-subscriptions-page

Revision history for this message
Guilherme Salgado (salgado) wrote :

Hi Brian,

It's great to see that work is being done on this long-awaited feature, but I was expecting to see some mockups for what we expect this page to look like and how it'd work. Something similar to what the Soyuz team is doing with derivative distributions. Do you know if there's a LEP for this? Or maybe just some mockups? I think it's important to work on those (maybe with help from the Design team, as described in https://dev.launchpad.net/UI/Design) for this feature, as the current approach may not scale well once we start adding the other types of items that a person can subscribe to this page.

Regardless of that, here's some feedback on the current implementation.

In the long term I'm sure we want to use ajax to unsubscribe without leaving the page, and given how easy it is not much more work to do so, I think it'd make sense to do that from the start. That would probably make an undo functionality more important, but I think we can live without that for now as that'd complicate things a bit more.

Also, upon clicking the unsubscribe link, you're taken to the bug's +subscribe page, and clicking Cancel there will take you to the bug's +index page rather than to +subscriptions as one would expect. This wouldn't be a problem if we were using ajax.

The page should not include the unsubscribe buttons when looking at somebody else's page, as the only thing you can do is unsubscribe yourself but not the person whose page you're looking at. Either that or make the page not available for other users.

For teams, the unsubscribe link takes you to the bug's +subscribe page but if you're not currently subscribed to that bug, the default option (and the heading) is for you to subscribe yourself rather than to unsubscribe the team. When you are subscribed, the default options is to unsubscribe yourself. This is a bit confusing, but we could avoid this completely by using ajax to unsubscribe directly from +subscriptions.

review: Abstain (ui*)

« Back to merge proposal