Merge lp:~deryck/launchpad/not-implemented-errors into lp:launchpad

[Deryck Hodge]

Fix NotImplementedError OPPS when filing a bug by ensuring errors are displayed on filebug page.

[Deryck Hodge (deryck)]

This is a fix for bug 508302, which describes the problems we've been
having with NotImplementedError OPPS on filing a bug.

The error was introduced when we changed the filebug page to use an
inline JavaScript dupe search. The inline submit bug form used a
different view to process the form submission, which didn't implement
the showFileBugForm method.

To fix this, I simply changed the action on the form to match +filebug
pages, so that any validation errors would be displayed back to the user.

== Tests ==

I updated a Windmill test to test that a single error displays on the
web page. The story test didn't fail because the non-JavaScript version
of the pages used the +filebug view, rather than the inline form. (This
also seemed to make the case for having the inline form use the same
action url.)

To test, run:

./bin/test --layer=BugsWindmillLayer -cvv -t test_filebug_dupe_finder

== Demo and Q/A ==

To demo in a browser, go to any +filebug page and submit the form
without filling in the description input. You should see the page
reload and display the form validation error.

Finally...

I must admit, I have no idea how to fix these lint errors. Nothing I
tried would work.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/bugs/browser/bugtarget.py
  lib/lp/bugs/windmill/tests/test_filebug_dupe_finder.py

== Pylint notices ==

lib/lp/bugs/browser/bugtarget.py
    495: [C0322, FileBugViewBase.submit_bug_action] Operator not
preceded by a space
    failure=handleSubmitBugFailure)
    ^
    def submit_bug_action(self, action, data):
    673: [C0322, FileBugViewBase.this_is_my_bug_action] Operator not
preceded by a space
    name="this_is_my_bug", failure=handleSubmitBugFailure)
    ^
    def this_is_my_bug_action(self, action, data):
    1045: [C0322, FileBugGuidedView.continue_action] Operator not
preceded by a space
    validator="validate_no_dupe_found")
    ^
    def continue_action(self, action, data):

[Paul Hummer (rockstar)]

review: Approve

[Francis J. Lacoste (flacoste)]

review: Approve (release-critical)

=== modified file 'lib/lp/bugs/browser/bugtarget.py'
--- lib/lp/bugs/browser/bugtarget.py	2010-01-26 14:22:41 +0000
+++ lib/lp/bugs/browser/bugtarget.py	2010-01-29 19:40:32 +0000
@@ -896,6 +896,18 @@
     show_summary_in_results = False
 
     @property
+    def action_url(self):
+        """Return the +filebug page as the action URL.
+
+        This enables better validation error handling,
+        since the form is always used inline on the +filebug page.
+        """
+        url = '%s/+filebug' % canonical_url(self.context)
+        if self.extra_data_token is not None:
+            url = urlappend(url, self.extra_data_token)
+        return url
+
+    @property
     def search_context(self):
         """Return the context used to search for similar bugs."""
         return self.context

=== modified file 'lib/lp/bugs/windmill/tests/test_filebug_dupe_finder.py'
--- lib/lp/bugs/windmill/tests/test_filebug_dupe_finder.py	2009-12-14 13:24:07 +0000
+++ lib/lp/bugs/windmill/tests/test_filebug_dupe_finder.py	2010-01-29 19:40:32 +0000
@@ -105,5 +105,16 @@
         client.waits.sleep(milliseconds=constants.SLEEP)
         client.asserts.assertElemJS(xpath=FORM_OVERLAY, js=FORM_NOT_VISIBLE)
 
+        # Validation errors are displayed on the current page.
+        client.click(id="bug-not-already-reported")
+        client.asserts.assertProperty(
+            id="filebug-form-container", validator="style.display|block",
+            timeout=constants.FOR_ELEMENT)
+        client.click(id="field.actions.submit_bug")
+        client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
+        client.asserts.assertText(
+            xpath=u'//div[@class="message"]',
+            validator="Required input is missing.")
+
 def test_suite():
     return unittest.TestLoader().loadTestsFromName(__name__)