launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02951
[Merge] lp:~wgrant/launchpad/unuse-nullbugtask into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/unuse-nullbugtask into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #589701 in Launchpad itself: "It's still possible to hit a NullBugTask"
https://bugs.launchpad.net/launchpad/+bug/589701
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/unuse-nullbugtask/+merge/53363
NullBugTask was created to provide a button to report a bug in a new context, if the bug was navigated to below a context for which a task did not yet exist. In early 2009 this behaviour was largely removed; non-existent tasks now redirect to the default task instead.
But one case was missed: DistributionSourcePackages where there is a task for the Distribution or another DistributionSourcePackage for the same distro. BugTargetTraversalMixin.traverse_bug returns a NullBugTask, but BugTaskView.initialise allows the null task through without redirecting.
This branch fixes that case by redirecting during traversal, preventing a NullBugTask from ever being encountered. This allows us to remove a lot of special cases, mostly trivially. One non-trivial case is BugTaskTraversal.traverse: it was used to turn +editstatus and +viewstatus into +editstatus-page and +viewstatus-page, presumably to prevent them from rendering on a NullBugTask. I've renamed the views to their exposed names, and removed the custom traversal method.
Removal of NullBugTask itself is a little more difficult, so that will be done in a later branch.
--
https://code.launchpad.net/~wgrant/launchpad/unuse-nullbugtask/+merge/53363
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/unuse-nullbugtask into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bugnomination.py'
--- lib/lp/bugs/browser/bugnomination.py 2011-02-02 15:43:31 +0000
+++ lib/lp/bugs/browser/bugnomination.py 2011-03-15 03:31:14 +0000
@@ -15,10 +15,7 @@
import pytz
from zope.component import getUtility
-from zope.publisher.interfaces import (
- implements,
- NotFound,
- )
+from zope.publisher.interfaces import implements
from canonical.launchpad import _
from canonical.launchpad.webapp import (
@@ -41,7 +38,6 @@
IBugNomination,
IBugNominationForm,
)
-from lp.bugs.interfaces.bugtask import INullBugTask
from lp.bugs.interfaces.cve import ICveSet
@@ -65,10 +61,6 @@
LaunchpadFormView.__init__(self, context, request)
def initialize(self):
- if INullBugTask.providedBy(self.current_bugtask):
- # It shouldn't be possible to nominate a bug that hasn't
- # been reported yet.
- raise NotFound(self.current_bugtask, '+nominate', self.request)
LaunchpadFormView.initialize(self)
# Update the submit label based on the user's permission.
submit_action = self.__class__.actions.byname['actions.submit']
=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py 2011-03-10 16:15:57 +0000
+++ lib/lp/bugs/browser/bugtask.py 2011-03-15 03:31:14 +0000
@@ -236,7 +236,6 @@
IDistroSeriesBugTask,
IFrontPageBugTaskSearch,
INominationsReviewTableBatchNavigator,
- INullBugTask,
IPersonBugTaskSearch,
IProductSeriesBugTask,
IRemoveQuestionFromBugTaskForm,
@@ -490,7 +489,7 @@
"""Return the IBugTask for this name in this context.
If the bug has been reported, but not in this specific context, a
- NullBugTask will be returned.
+ redirect to the default context will be returned.
Raises NotFoundError if no bug with the given name is found.
@@ -514,59 +513,15 @@
# Security proxy this object on the way out.
return getUtility(IBugTaskSet).get(bugtask.id)
- # If we've come this far, it means that no actual task exists in this
- # context, so we'll return a null bug task. This makes it possible to,
- # for example, return a bug page for a context in which the bug hasn't
- # yet been reported.
- if IProduct.providedBy(context):
- null_bugtask = bug.getNullBugTask(product=context)
- elif IProductSeries.providedBy(context):
- null_bugtask = bug.getNullBugTask(productseries=context)
- elif IDistribution.providedBy(context):
- null_bugtask = bug.getNullBugTask(distribution=context)
- elif IDistributionSourcePackage.providedBy(context):
- null_bugtask = bug.getNullBugTask(
- distribution=context.distribution,
- sourcepackagename=context.sourcepackagename)
- elif IDistroSeries.providedBy(context):
- null_bugtask = bug.getNullBugTask(distroseries=context)
- elif ISourcePackage.providedBy(context):
- null_bugtask = bug.getNullBugTask(
- distroseries=context.distroseries,
- sourcepackagename=context.sourcepackagename)
- else:
- raise TypeError(
- "Unknown context type for bug task: %s" % repr(context))
-
- return null_bugtask
+ # If we've come this far, there's no task for the requested
+ # context. Redirect to one that exists.
+ return self.redirectSubTree(canonical_url(bug.default_bugtask))
class BugTaskNavigation(Navigation):
"""Navigation for the `IBugTask`."""
usedfor = IBugTask
- def traverse(self, name):
- """Traverse the `IBugTask`."""
- # Are we traversing to the view or edit status page of the
- # bugtask? If so, and the task actually exists, return the
- # appropriate page. If the task doesn't yet exist (i.e. it's a
- # NullBugTask), then return a 404. In other words, the URL:
- #
- # /products/foo/+bug/1/+viewstatus
- #
- # will return the +viewstatus page if bug 1 has actually been
- # reported in "foo". If bug 1 has not yet been reported in "foo",
- # a 404 will be returned.
- if name not in ("+viewstatus", "+editstatus"):
- # You're going in the wrong direction.
- return None
- if INullBugTask.providedBy(self.context):
- # The bug has not been reported in this context.
- return None
- # Yes! The bug has been reported in this context.
- return getMultiAdapter((self.context, self.request),
- name=(name + "-page"))
-
@stepthrough('attachments')
def traverse_attachments(self, name):
"""traverse to an attachment by id."""
@@ -655,13 +610,8 @@
@property
def page_title(self):
- bugtask = self.context
- if INullBugTask.providedBy(bugtask):
- heading = 'Bug #%s is not in %s' % (
- bugtask.bug.id, bugtask.bugtargetdisplayname)
- else:
- heading = 'Bug #%s in %s' % (
- bugtask.bug.id, bugtask.bugtargetdisplayname)
+ heading = 'Bug #%s in %s' % (
+ self.context.bug.id, self.context.bugtargetdisplayname)
return smartquote('%s: "%s"') % (heading, self.context.bug.title)
@property
@@ -698,12 +648,6 @@
# See render() for how this flag is used.
self._redirecting_to_bug_list = False
- # If the bug is not reported in this context, redirect
- # to the default bug task.
- if not self.isReportedInContext():
- self.request.response.redirect(
- canonical_url(self.context.bug.default_bugtask))
-
self.bug_title_edit_widget = TextLineEditorWidget(
bug, IBug['title'], "Edit this summary", 'h1',
edit_url=canonical_url(self.context, view_name='+edit'))
@@ -741,69 +685,6 @@
series.bugtargetdisplayname)
self.request.response.redirect(canonical_url(self.context))
- def reportBugInContext(self):
- """Report the bug affects the current context."""
- fake_task = self.context
- if self.request.form.get("reportbug"):
- if self.isReportedInContext():
- self.notices.append(
- "The bug is already reported in this context.")
- return
- # The user has requested that the bug be reported in this
- # context.
- if IUpstreamBugTask.providedBy(fake_task):
- # Create a real upstream task in this context.
- real_task = fake_task.bug.addTask(
- getUtility(ILaunchBag).user, fake_task.product)
- elif IDistroBugTask.providedBy(fake_task):
- # Create a real distro bug task in this context.
- real_task = fake_task.bug.addTask(
- getUtility(ILaunchBag).user, fake_task.target)
- elif IDistroSeriesBugTask.providedBy(fake_task):
- self._nominateBug(fake_task.distroseries)
- return
- elif IProductSeriesBugTask.providedBy(fake_task):
- self._nominateBug(fake_task.productseries)
- return
- else:
- raise TypeError(
- "Unknown bug task type: %s" % repr(fake_task))
-
- self.context = real_task
-
- # Add an appropriate feedback message
- self.notices.append("Thank you for your bug report.")
-
- def isReportedInContext(self):
- """Is the bug reported in this context? Returns True or False.
-
- It considers a nominated bug to be reported.
-
- This is particularly useful for views that may render a
- NullBugTask.
- """
- if self.context.id is not None:
- # Fast path for real bugtasks: they have a DB id.
- return True
- params = BugTaskSearchParams(user=self.user, bug=self.context.bug)
- matching_bugtasks = self.context.target.searchTasks(params)
- if self.context.productseries is not None:
- nomination_target = self.context.productseries
- elif self.context.distroseries is not None:
- nomination_target = self.context.distroseries
- else:
- nomination_target = None
- if nomination_target is not None:
- try:
- nomination = self.context.bug.getNominationFor(
- nomination_target)
- except NotFoundError:
- nomination = None
- else:
- nomination = None
-
- return nomination is not None or matching_bugtasks.count() > 0
-
def isSeriesTargetableContext(self):
"""Is the context something that supports Series targeting?
@@ -1744,17 +1625,12 @@
The assignee is included.
"""
bugtask = self.context
-
- if INullBugTask.providedBy(bugtask):
- return u"Not reported in %s" % bugtask.bugtargetname
-
assignee = bugtask.assignee
status = bugtask.status
status_title = status.title.capitalize()
if not assignee:
return status_title + ' (unassigned)'
-
assignee_html = PersonFormatterAPI(assignee).link('+assignedbugs')
if status in (BugTaskStatus.INVALID,
=== modified file 'lib/lp/bugs/browser/configure.zcml'
--- lib/lp/bugs/browser/configure.zcml 2011-03-11 21:31:10 +0000
+++ lib/lp/bugs/browser/configure.zcml 2011-03-15 03:31:14 +0000
@@ -637,35 +637,27 @@
class="lp.bugs.browser.bugalsoaffects.BugAlsoAffectsDistroMetaView"
permission="launchpad.AnyPerson"/>
<browser:page
- name="+editstatus-page"
- for="lp.bugs.interfaces.bugtask.IUpstreamBugTask"
- class="lp.bugs.browser.bugtask.BugTaskEditView"
- permission="launchpad.Edit"
- template="../templates/bugtask-edit.pt">
- </browser:page>
- <browser:page
- name="+edit-form"
- for="lp.bugs.interfaces.bugtask.IUpstreamBugTask"
- class="lp.bugs.browser.bugtask.BugTaskEditView"
- permission="launchpad.Edit"
- template="../templates/bugtask-edit-form.pt">
- </browser:page>
- <browser:page
- name="+editstatus-page"
- for="lp.bugs.interfaces.bugtask.IProductSeriesBugTask"
- class="lp.bugs.browser.bugtask.BugTaskEditView"
- permission="launchpad.Edit"
- template="../templates/bugtask-edit.pt">
- </browser:page>
- <browser:page
- name="+edit-form"
- for="lp.bugs.interfaces.bugtask.IProductSeriesBugTask"
- class="lp.bugs.browser.bugtask.BugTaskEditView"
- permission="launchpad.Edit"
- template="../templates/bugtask-edit-form.pt">
- </browser:page>
- <browser:page
- name="+viewstatus-page"
+ name="+edit-form"
+ for="lp.bugs.interfaces.bugtask.IUpstreamBugTask"
+ class="lp.bugs.browser.bugtask.BugTaskEditView"
+ permission="launchpad.Edit"
+ template="../templates/bugtask-edit-form.pt">
+ </browser:page>
+ <browser:page
+ name="+edit-form"
+ for="lp.bugs.interfaces.bugtask.IProductSeriesBugTask"
+ class="lp.bugs.browser.bugtask.BugTaskEditView"
+ permission="launchpad.Edit"
+ template="../templates/bugtask-edit-form.pt">
+ </browser:page>
+ <browser:page
+ name="+editstatus"
+ for="lp.bugs.interfaces.bugtask.IBugTask"
+ class="lp.bugs.browser.bugtask.BugTaskEditView"
+ permission="launchpad.Edit"
+ template="../templates/bugtask-edit.pt"/>
+ <browser:page
+ name="+viewstatus"
for="lp.bugs.interfaces.bugtask.IBugTask"
class="lp.bugs.browser.bugtask.BugTaskStatusView"
permission="launchpad.View"
@@ -680,13 +672,6 @@
for="lp.bugs.interfaces.bugtask.IDistroBugTask"
name="+index"/>
<browser:page
- name="+editstatus-page"
- for="lp.bugs.interfaces.bugtask.IDistroBugTask"
- class="lp.bugs.browser.bugtask.BugTaskEditView"
- permission="launchpad.Edit"
- template="../templates/bugtask-edit.pt">
- </browser:page>
- <browser:page
name="+edit-form"
for="lp.bugs.interfaces.bugtask.IDistroBugTask"
class="lp.bugs.browser.bugtask.BugTaskEditView"
@@ -697,13 +682,6 @@
for="lp.bugs.interfaces.bugtask.IDistroSeriesBugTask"
name="+index"/>
<browser:page
- name="+editstatus-page"
- for="lp.bugs.interfaces.bugtask.IDistroSeriesBugTask"
- class="lp.bugs.browser.bugtask.BugTaskEditView"
- permission="launchpad.Edit"
- template="../templates/bugtask-edit.pt">
- </browser:page>
- <browser:page
name="+edit-form"
for="lp.bugs.interfaces.bugtask.IDistroSeriesBugTask"
class="lp.bugs.browser.bugtask.BugTaskEditView"
=== modified file 'lib/lp/bugs/browser/tests/bug-views.txt'
--- lib/lp/bugs/browser/tests/bug-views.txt 2011-02-15 09:31:20 +0000
+++ lib/lp/bugs/browser/tests/bug-views.txt 2011-03-15 03:31:14 +0000
@@ -777,7 +777,7 @@
... 'testproduct.actions.save': 'Save Changes',
... })
>>> view = getMultiAdapter(
- ... (bug.bugtasks[0], request), name="+editstatus-page")
+ ... (bug.bugtasks[0], request), name="+editstatus")
>>> view.initialize()
>>> view = getMultiAdapter(
@@ -833,7 +833,7 @@
... 'testproduct.actions.save': 'Save Changes',
... })
>>> view = getMultiAdapter(
- ... (bug.bugtasks[0], request), name="+editstatus-page")
+ ... (bug.bugtasks[0], request), name="+editstatus")
>>> view.initialize()
>>> view = getMultiAdapter(
=== modified file 'lib/lp/bugs/browser/tests/bugtask-edit-views.txt'
--- lib/lp/bugs/browser/tests/bugtask-edit-views.txt 2010-10-18 22:24:59 +0000
+++ lib/lp/bugs/browser/tests/bugtask-edit-views.txt 2011-03-15 03:31:14 +0000
@@ -29,7 +29,7 @@
... ubuntu_thunderbird_task.sourcepackagename.name}
>>> request = LaunchpadTestRequest(method='POST', form=edit_form)
>>> edit_view = getMultiAdapter(
- ... (ubuntu_thunderbird_task, request), name='+editstatus-page')
+ ... (ubuntu_thunderbird_task, request), name='+editstatus')
>>> edit_view.initialize()
>>> ubuntu_thunderbird_task.status.title
'In Progress'
@@ -50,7 +50,7 @@
>>> edit_form['ubuntu_thunderbird.sourcepackagename'] = u'linux-2.6.12'
>>> request = LaunchpadTestRequest(method='POST', form=edit_form)
>>> edit_view = getMultiAdapter(
- ... (ubuntu_thunderbird_task, request), name='+editstatus-page')
+ ... (ubuntu_thunderbird_task, request), name='+editstatus')
>>> edit_view.initialize()
>>> ubuntu_thunderbird_task.sourcepackagename.name
u'linux-source-2.6.15'
@@ -73,7 +73,7 @@
>>> edit_form['ubuntu_thunderbird.sourcepackagename'] = u'no-such-package'
>>> request = LaunchpadTestRequest(form=edit_form, method='POST')
>>> edit_view = getMultiAdapter(
- ... (ubuntu_thunderbird_task, request), name='+editstatus-page')
+ ... (ubuntu_thunderbird_task, request), name='+editstatus')
>>> edit_view.initialize()
>>> for error in edit_view.errors:
... print error
@@ -112,7 +112,7 @@
... }
>>> request = LaunchpadTestRequest(form=edit_form, method='POST')
>>> edit_view = getMultiAdapter(
- ... (ubuntu_task, request), name='+editstatus-page')
+ ... (ubuntu_task, request), name='+editstatus')
>>> edit_view.initialize()
>>> for error in edit_view.errors:
... print error
@@ -144,7 +144,7 @@
... }
>>> request = LaunchpadTestRequest(form=edit_form, method='POST')
>>> edit_view = getMultiAdapter(
- ... (ubuntu_grumpy_task, request), name='+editstatus-page')
+ ... (ubuntu_grumpy_task, request), name='+editstatus')
>>> edit_view.initialize()
>>> for error in edit_view.errors:
... print error
@@ -155,7 +155,7 @@
== Edit the Product ==
-+editstatus-page allows a bug to be retargeted to another product.
++editstatus allows a bug to be retargeted to another product.
>>> bug_seven = getUtility(IBugSet).get(7)
>>> product_task = bug_seven.bugtasks[0]
@@ -173,7 +173,7 @@
... }
>>> request = LaunchpadTestRequest(form=edit_form, method='POST')
>>> edit_view = getMultiAdapter(
- ... (product_task, request), name='+editstatus-page')
+ ... (product_task, request), name='+editstatus')
>>> edit_view.initialize()
>>> [str(error) for error in edit_view.errors]
[]
@@ -193,7 +193,7 @@
... }
>>> request = LaunchpadTestRequest(form=edit_form, method='POST')
>>> edit_view = getMultiAdapter(
- ... (product_task, request), name='+editstatus-page')
+ ... (product_task, request), name='+editstatus')
>>> edit_view.initialize()
>>> for error in edit_view.errors:
... print error
@@ -235,7 +235,7 @@
... 'thunderbird.importance': 'Critical',
... 'thunderbird.bugwatch': '6'})
>>> edit_view = getMultiAdapter(
- ... (thunderbird_task, request), name='+editstatus-page')
+ ... (thunderbird_task, request), name='+editstatus')
>>> edit_view.initialize()
>>> thunderbird_task.bugwatch == bugzilla_watch
True
@@ -251,7 +251,7 @@
... 'thunderbird.actions.save': 'Save Changes',
... 'thunderbird.bugwatch-empty-marker': '1'})
>>> edit_view = getMultiAdapter(
- ... (thunderbird_task, request), name='+editstatus-page')
+ ... (thunderbird_task, request), name='+editstatus')
>>> edit_view.initialize()
>>> thunderbird_task.bugwatch is None
True
@@ -280,7 +280,7 @@
>>> request = LaunchpadTestRequest()
>>> ubuntu_task = getUtility(IBugTaskSet).get(17)
>>> bugtask_edit_view = getMultiAdapter(
- ... (ubuntu_task, request), name="+editstatus-page")
+ ... (ubuntu_task, request), name="+editstatus")
>>> bugtask_edit_view.initialize()
>>> IInputWidget.providedBy(bugtask_edit_view.widgets['milestone'])
@@ -293,7 +293,7 @@
>>> login("no-priv@xxxxxxxxxxxxx")
>>> bugtask_edit_view = getMultiAdapter(
- ... (ubuntu_task, request), name="+editstatus-page")
+ ... (ubuntu_task, request), name="+editstatus")
>>> bugtask_edit_view.initialize()
>>> isinstance(bugtask_edit_view.widgets['milestone'], ItemDisplayWidget)
@@ -312,7 +312,7 @@
Unlike before, no-priv can now edit the milestone.
>>> bugtask_edit_view = getMultiAdapter(
- ... (ubuntu_task, request), name="+editstatus-page")
+ ... (ubuntu_task, request), name="+editstatus")
>>> bugtask_edit_view.initialize()
>>> IInputWidget.providedBy(bugtask_edit_view.widgets['milestone'])
=== modified file 'lib/lp/bugs/templates/bugtask-index.pt'
--- lib/lp/bugs/templates/bugtask-index.pt 2011-02-25 22:09:46 +0000
+++ lib/lp/bugs/templates/bugtask-index.pt 2011-03-15 03:31:14 +0000
@@ -72,7 +72,6 @@
<div id="tags-autocomplete">
<div id="tags-autocomplete-content"></div>
</div>
- <tal:block condition="view/reportBugInContext" />
<p class="informational message"
tal:condition="view/notices"
