Merge into trunk : 720563-privacy : Code : Kanban

Status:	Merged
Approved by:	Jamu Kakar on 2011-03-15
Approved revision:	26
Merged at revision:	26
Proposed branch:	lp:~mbp/kanban/720563-privacy
Merge into:	lp:kanban
To merge this branch:	bzr merge lp:~mbp/kanban/720563-privacy
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Jamu Kakar		2011-03-15	Approve on 2011-03-15
Review via email: mp+53386@code.launchpad.net

Description of the change

This makes sure that when a kanban is generated by an unprivileged user, public bugs with private branches attached will still show up. (It should also handle the case of being logged in but not able to see a particular bug.)

This has a workaround for lp bug 735346 which means if any one of the linked branches is private, you can't see any of them at all. Eventually Launchpad should just omit branches we can't see from the list, and the try/except should never fire, but will be harmless.

I've removed the higher-level catchall except blocks; if there are any other private things with similar problems we could add this back in.

This should also mean that if other HTTP errors show up, such as a 503, they'll propagate to the user. (bug 735317)

lp:~mbp/kanban/720563-privacy updated on 2011-03-15

26. By Martin Pool on 2011-03-15

When bug 735332 is fixed, we shouldn't get 401/403 errors on branch collections at all.

So we don't need to catch them at a lower scope.

Revision history for this message

Jamu Kakar (jkakar) wrote on 2011-03-15:

#

[1]

+ merge_proposal_url = ("https://code.launchpad.net/~"
+ + str(merge_proposal.self).split("~")[-1])

Please format this as:

                merge_proposal_url = (
                    "https://code.launchpad.net/~"
                    + str(merge_proposal.self).split("~")[-1])

[2]

+ break
+ break

Please remove one of these.

[3]

+ # Due to <http://pad.lv735346>, at the moment bugs with
+ # any private linked branches hide all of them and raise an error.

The URL here needs a trailing slash after pad.lv, please.

[4]

+ if is_forbidden(e) or is_unauthorized(e):
+ pass

It might be worth calling trace("Skipping forbidden or unauthorized
bug.") here...? Though, I'm not really sure what a user will do about
it, so maybe it's just noise. Anyway, just a thought.

Nice work, +1!

review: Approve

Revision history for this message

Martin Pool (mbp) wrote on 2011-03-16:

#

On 16 March 2011 05:22, Jamu Kakar <email address hidden> wrote:
> Review: Approve
> [1]
>
> + merge_proposal_url = ("https://code.launchpad.net/~"
> + + str(merge_proposal.self).split("~")[-1])
>
> Please format this as:
>
> merge_proposal_url = (
> "https://code.launchpad.net/~"
> + str(merge_proposal.self).split("~")[-1])

Thanks for the prompt review.

That's obviously just moved code, but anyhow: I wonder if there is any
way we can get the right url from lplib rather than hand hacking?

Revision history for this message

Martin Pool (mbp) wrote on 2011-03-16:

#

Yes, we can use the .web_link property <http://blog.launchpad.net/cool-new-stuff/new-web_link-property>

lp:~mbp/kanban/720563-privacy updated on 2011-03-16

27. By Martin Pool on 2011-03-16: Use web_link property rather than hand-hacking of urls
28. By Martin Pool on 2011-03-16: Fix link; give message when skipping private branches

Kanban

Merge lp:~mbp/kanban/720563-privacy into lp:kanban

Commit message

Description of the change

Subscribers