Merge lp://staging/~wgrant/launchpad/sensible-validate_target into lp://staging/launchpad

Proposed by William Grant
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: 13514
Proposed branch: lp://staging/~wgrant/launchpad/sensible-validate_target
Merge into: lp://staging/launchpad
Diff against target: 763 lines (+284/-248)
8 files modified
lib/canonical/launchpad/doc/validation.txt (+0/-105)
lib/canonical/launchpad/interfaces/validation.py (+0/-98)
lib/lp/bugs/browser/bugalsoaffects.py (+20/-22)
lib/lp/bugs/browser/bugtask.py (+16/-13)
lib/lp/bugs/browser/tests/test_bugtask.py (+2/-1)
lib/lp/bugs/model/bugtask.py (+65/-7)
lib/lp/bugs/model/tests/test_bugtask.py (+177/-0)
lib/lp/bugs/stories/bug-also-affects/xx-bug-also-affects.txt (+4/-2)
To merge this branch: bzr merge lp://staging/~wgrant/launchpad/sensible-validate_target
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code Approve
Review via email: mp+69175@code.staging.launchpad.net

Commit message

Replace validate_distrotask and valid_upstreamtask with a more general and sensible validate_target.

Description of the change

This branch continues my ruthless campaign to move BugTask.target handling completely into the model. It takes validate_distrotask and valid_upstreamtask, combining them into a single validate_target which can be easily called by transitionToTarget. It now lives in lp.bugs, and has extensive unittests rather than doctests.

To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :

This branch looks good to land. I have a comment that relates to my own recent work. Maybe your thoughts will help us to make a method faster.

> === modified file 'lib/lp/bugs/model/bugtask.py'
> --- lib/lp/bugs/model/bugtask.py 2011-07-22 04:50:51 +0000
> +++ lib/lp/bugs/model/bugtask.py 2011-07-25 23:06:42 +0000
...
> @@ -439,6 +437,65 @@
> return validate_target_attribute(self, attr, value)
>
>
> +def validate_target(bug, target):
> + """Validate a bugtask target against a bug's existing tasks.
> +
> + Checks that no conflicting tasks already exist.
> + """
> + if bug.getBugTask(target):
> + raise IllegalTarget(
> + "A fix for this bug has already been requested for %s"
> + % target.displayname)
> +
> + if IDistributionSourcePackage.providedBy(target):
> + # If the distribution has at least one series, check that the
> + # source package has been published in the distribution.
> + if (target.sourcepackagename is not None and
> + len(target.distribution.series) > 0):
> + try:
> + target.distribution.guessPublishedSourcePackageName(
> + target.sourcepackagename.name)

I believe this is the method that contributes to bugs
https://bugs.launchpad.net/launchpad/+bug/618366 and
https://bugs.launchpad.net/launchpad/+bug/721643

I tried to avoid using it. Do you know why guessPublishedSourcePackageName()
requires a string and does the extra work to get the spn. We already have
an spn half the time. I also want to avoid calling this method if the method
that provided the spn can be trusted to have provide one that was published
in the distro.

Ah, but this method is smart enough to verify the branch for the package.
I like that.

The binary name is in part the fault of the existing vocabs. I hope
Steven's work will make this path in the method obsolete, or at least very
rare.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.