Launchpad itself

Merge lp:~stevenk/launchpad/fix-branch-subscribe-open-team into lp:launchpad

  1. fix-branch-subscribe-open-team
  2. Merge into devel
Proposed by Steve Kowalik on 2012-06-21
Status: Merged
Approved by: Steve Kowalik on 2012-06-21
Approved revision: no longer in the source branch.
Merged at revision: 15469
Proposed branch: lp:~stevenk/launchpad/fix-branch-subscribe-open-team
Merge into: lp:launchpad
Diff against target: 0 lines
To merge this branch: bzr merge lp:~stevenk/launchpad/fix-branch-subscribe-open-team
Reviewer Review Type Date Requested Status
Ian Booth (community) Approve on 2012-06-21
Launchpad code reviewers from Canonical code 2012-06-21 Pending
Review via email: mp+111326@code.launchpad.net

This proposal supersedes a proposal from 2012-06-13.

Commit Message

Use a validate() method on Branch:+addsubscriber so that users can see the error about subscribing an open or delegated team to a private branch rather than it redirecting back to Branch:+index and hiding the error.

Description of the Change

This branch is a follow-up to some of the behaviour introduced in https://code.launchpad.net/~stevenk/launchpad/branch-subscribe-aag/+merge/108881. That branch changed Branch.subscribe() to raise an exception if an open or delegated team was trying to be subscribed to a private branch. I had changed the browser code to catch the exception and call setFieldError(), but since next_url was unilaterally set, Zope happily redirected back to Branch:+index, and it looked like nothing happened.

I have removed most of that code and made use of a validate() method, which is the proper pattern.

To post a comment you must log in.
Curtis Hovey (sinzui) wrote : Posted in a previous version of this proposal

Thank you.

review: Approve (code)
Ian Booth (wallyworld) wrote :

Looks good, I assume the existing test coverage is adequate.

review: Approve

Preview Diff

Empty