launchpad-reviewers team mailing list archive

[Thomi Richards <thomi.richards@xxxxxxxxxxxxx>]

Fixes made. Let me know what you think. Thanks.

Diff comments:

> === modified file 'lib/lp/bugs/browser/buglisting.py'
> --- lib/lp/bugs/browser/buglisting.py	2014-11-29 02:16:28 +0000
> +++ lib/lp/bugs/browser/buglisting.py	2015-01-08 03:46:20 +0000
> @@ -1229,13 +1229,21 @@
>  
>          if data:
>              searchtext = data.get("searchtext")
> -            if searchtext and searchtext.isdigit():
> -                try:
> -                    bug = getUtility(IBugSet).get(searchtext)
> -                except NotFoundError:
> -                    pass
> -                else:
> -                    self.request.response.redirect(canonical_url(bug))
> +            if searchtext:
> +                bug_number = ""
> +                if searchtext.isdigit():
> +                    bug_number = searchtext
> +                if len(searchtext.split(' ')) == 1 and searchtext[0] == '#' and \
> +                searchtext[1:].isdigit():
> +                    bug_number = searchtext[1:]

Done, although both 'if' statements need to check '.isdigit()', because we don't want to redirect on "#123 foobar" (as per bug comments). 

Realised that since 'searchtext' isn't used anywhere else in this method, we don't need a second copy of that string.

> +
> +                if bug_number:
> +                    try:
> +                        bug = getUtility(IBugSet).get(bug_number)
> +                    except NotFoundError:
> +                        pass
> +                    else:
> +                        self.request.response.redirect(canonical_url(bug))
>  
>              assignee_option = self.request.form.get("assignee_option")
>              if assignee_option == "none":
> 
> === modified file 'lib/lp/bugs/browser/tests/test_buglisting.py'
> --- lib/lp/bugs/browser/tests/test_buglisting.py	2015-01-07 03:07:22 +0000
> +++ lib/lp/bugs/browser/tests/test_buglisting.py	2015-01-08 03:46:20 +0000
> @@ -249,6 +249,30 @@
>  
>          self.assertEqual(default_bugtask_url, browser.url)
>  
> +    def test_redirects_to_bug_from_search_form_with_hash(self):
> +        bug = self.factory.makeBug()
> +        login_person(bug.owner)
> +        default_bugtask_url = canonical_url(bug.default_bugtask, rootsite='bugs')
> +
> +        browser = self.getUserBrowser("http://bugs.launchpad.dev/";)
> +        input_field = browser.getControl(name='field.searchtext')
> +        input_field.value = "#%d" % bug.id
> +        browser.getControl(name='search').click()
> +
> +        self.assertEqual(default_bugtask_url, browser.url)
> +
> +    def test_doesnt_redirect_to_bug_from_search_form_with_multiple_terms(self):
> +        bug = self.factory.makeBug()
> +        login_person(bug.owner)
> +        default_bugtask_url = canonical_url(bug.default_bugtask, rootsite='bugs')
> +
> +        browser = self.getUserBrowser("http://bugs.launchpad.dev/";)
> +        input_field = browser.getControl(name='field.searchtext')
> +        input_field.value = "#%d otherterm" % bug.id
> +        browser.getControl(name='search').click()
> +
> +        self.assertNotEqual(default_bugtask_url, browser.url)

Done.

> +
>  
>  class BugTargetTestCase(TestCaseWithFactory):
>      """Test helpers for setting up `IBugTarget` tests."""
> 


-- 
https://code.launchpad.net/~thomir/launchpad/devel-fix-bug-redirect/+merge/245816
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.