launchpad-reviewers team mailing list archive

["j.c.sackett" <jcsackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/fix-404-on-bug-comment into lp:launchpad.

Commit message:
Fixes obscure 404 error based on the canonical_url of bug comments.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/fix-404-on-bug-comment/+merge/126567

Summary
=======
Under certain conditions a bug comment can be generated with a canonical_url
that references an inactive project. Because most users can't see this
project, they get a 404 error js popup when using the js add comment form.

This is a result of the UrlData for bug messages assuming bugtask[0] is safe
to use for canonical_urls.

Preimp
======
Spoke with Curtis Hovey

Implementation
==============
The UrlData generator for bug messages is changed from using bugtasks[0] to
default_bugtask. Tests were added.

Tests
=====
bin/test -vvct test_bugcomment

QA
==
Find a bug comment; ensure its url uses the target for the default_bugtask.

LoC
===
I have more than 400 LoC credit.

Lint
====

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/browser/tests/test_bugcomment.py
  lib/lp/services/messages/browser/message.py
-- 
https://code.launchpad.net/~jcsackett/launchpad/fix-404-on-bug-comment/+merge/126567
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/fix-404-on-bug-comment into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/tests/test_bugcomment.py'
--- lib/lp/bugs/browser/tests/test_bugcomment.py	2012-09-17 16:13:40 +0000
+++ lib/lp/bugs/browser/tests/test_bugcomment.py	2012-09-26 23:00:29 +0000
@@ -308,6 +308,15 @@
         url = canonical_url(bug_comment, view_name='+download')
         self.assertEqual(url, bug_comment.download_url)
 
+    def test_bug_comment_canonical_url(self):
+        """The bug comment url should use the default bugtastk."""
+        bug_message = self.factory.makeBugComment()
+        bugtask = bug_message.bugs[0].default_bugtask
+        product = removeSecurityProxy(bugtask).target
+        url = 'http://bugs.launchpad.dev/%s/+bug/%s/comments/%s' % (
+           product.name, bugtask.bug.id, 1)
+        self.assertEqual(url, canonical_url(bug_message))
+
 
 def make_bug_comment(factory, *args, **kwargs):
     bug_message = factory.makeBugComment(*args, **kwargs)

=== modified file 'lib/lp/services/messages/browser/message.py'
--- lib/lp/services/messages/browser/message.py	2012-04-16 23:02:44 +0000
+++ lib/lp/services/messages/browser/message.py	2012-09-26 23:00:29 +0000
@@ -27,7 +27,7 @@
     rootsite = 'bugs'
 
     def __init__(self, bug, message):
-        self.inside = bug.bugtasks[0]
+        self.inside = bug.default_bugtask
         self.path = "comments/%d" % list(bug.messages).index(message)