launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/chromium-filebug-1008543 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1008543 in Launchpad itself: "ChoiceSources on +filebug are empty in Chromium"
  https://bugs.launchpad.net/launchpad/+bug/1008543

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/chromium-filebug-1008543/+merge/108871

== Implementation ==

The filebug forms are rendered using a number of views and macros. It turns out some key json request cache data was being inserted into the cache in a subclass and it should have been done in a class higher up so that a macro (+filebug-reporting-guidelines) had access to it.

Also did a drive by fix the the filebug setup javascript to only run the code to wire up the file bug form f the file bug form is rendered.

== QA ==

Check the usual filebug scenarios. If you are quick at typing, open a filebug page and hit the submt button before the javascript loads - see linked bug for more info.

== Tests ==

Add a new test to test_bugtarget_filebug to ensure the +filebug-reporting-guidelines macro has all the data it needs.

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/browser/bugtarget.py
  lib/lp/bugs/browser/tests/test_bugtarget_filebug.py
  lib/lp/bugs/javascript/filebug.js
  lib/lp/bugs/javascript/tests/test_filebug.html
-- 
https://code.launchpad.net/~wallyworld/launchpad/chromium-filebug-1008543/+merge/108871
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/chromium-filebug-1008543 into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugtarget.py'
--- lib/lp/bugs/browser/bugtarget.py	2012-06-01 05:11:24 +0000
+++ lib/lp/bugs/browser/bugtarget.py	2012-06-06 04:49:19 +0000
@@ -280,9 +280,26 @@
             self.show_information_type_in_ui)
         cache.objects['bug_private_by_default'] = (
             IProduct.providedBy(self.context) and self.context.private_bugs)
-        cache.objects['enable_bugfiling_duplicate_search'] = (
-            IProjectGroup.providedBy(self.context)
-            or self.context.enable_bugfiling_duplicate_search)
+        cache.objects['information_type_data'] = [
+            {'value': term.name, 'description': term.description,
+            'name': term.title,
+            'description_css_class': 'choice-description'}
+            for term in InformationTypeVocabulary(self.context)]
+        bugtask_status_data = vocabulary_to_choice_edit_items(
+            BugTaskStatus, include_description=True, css_class_prefix='status',
+            excluded_items=[
+                BugTaskStatus.UNKNOWN,
+                BugTaskStatus.EXPIRED,
+                BugTaskStatus.INVALID,
+                BugTaskStatus.OPINION,
+                BugTaskStatus.WONTFIX,
+                BugTaskStatus.INCOMPLETE])
+        cache.objects['bugtask_status_data'] = bugtask_status_data
+        bugtask_importance_data = vocabulary_to_choice_edit_items(
+            BugTaskImportance, include_description=True,
+            css_class_prefix='importance',
+            excluded_items=[BugTaskImportance.UNKNOWN])
+        cache.objects['bugtask_importance_data'] = bugtask_importance_data
 
     def setUpFields(self):
         """Set up the form fields. See `LaunchpadFormView`."""
@@ -387,31 +404,11 @@
         # either. It makes for better diagnosis of failing tests.
         if self.redirect_ubuntu_filebug:
             pass
-        LaunchpadFormView.initialize(self)
+        super(FileBugReportingGuidelines, self).initialize()
         cache = IJSONRequestCache(self.request)
         cache.objects['enable_bugfiling_duplicate_search'] = (
             IProjectGroup.providedBy(self.context)
             or self.context.enable_bugfiling_duplicate_search)
-        cache.objects['information_type_data'] = [
-            {'value': term.name, 'description': term.description,
-            'name': term.title,
-            'description_css_class': 'choice-description'}
-            for term in InformationTypeVocabulary(self.context)]
-        bugtask_status_data = vocabulary_to_choice_edit_items(
-            BugTaskStatus, include_description=True, css_class_prefix='status',
-            excluded_items=[
-                BugTaskStatus.UNKNOWN,
-                BugTaskStatus.EXPIRED,
-                BugTaskStatus.INVALID,
-                BugTaskStatus.OPINION,
-                BugTaskStatus.WONTFIX,
-                BugTaskStatus.INCOMPLETE])
-        cache.objects['bugtask_status_data'] = bugtask_status_data
-        bugtask_importance_data = vocabulary_to_choice_edit_items(
-            BugTaskImportance, include_description=True,
-            css_class_prefix='importance',
-            excluded_items=[BugTaskImportance.UNKNOWN])
-        cache.objects['bugtask_importance_data'] = bugtask_importance_data
         if (self.extra_data_token is not None and
             not self.extra_data_to_process):
             # self.extra_data has been initialized in publishTraverse().

=== modified file 'lib/lp/bugs/browser/tests/test_bugtarget_filebug.py'
--- lib/lp/bugs/browser/tests/test_bugtarget_filebug.py	2012-06-01 05:11:24 +0000
+++ lib/lp/bugs/browser/tests/test_bugtarget_filebug.py	2012-06-06 04:49:19 +0000
@@ -628,3 +628,11 @@
         login_person(user)
         view = create_initialized_view(project, '+filebug', principal=user)
         self._assert_cache_values(view, True)
+
+    def test_filebug_reportinguidelines(self):
+        project = self.factory.makeProduct(official_malone=True)
+        user = self.factory.makePerson()
+        login_person(user)
+        view = create_initialized_view(
+            project, '+filebug-reporting-guidelines', principal=user)
+        self._assert_cache_values(view, True)

=== modified file 'lib/lp/bugs/javascript/filebug.js'
--- lib/lp/bugs/javascript/filebug.js	2012-06-01 05:11:24 +0000
+++ lib/lp/bugs/javascript/filebug.js	2012-06-06 04:49:19 +0000
@@ -19,16 +19,22 @@
         Y.lp.bugs.filebug_dupefinder.setup_dupe_finder();
         Y.lp.bugs.filebug_dupefinder.setup_dupes();
     }
-    var search_button = Y.one(Y.DOM.byId('field.actions.projectgroupsearch'));
-    if (Y.Lang.isValue(search_button )) {
-        search_button.set('value', 'Check again');
-    }
-    if (LP.cache.show_information_type_in_ui) {
-        setup_information_type();
-    } else {
-        setup_security_related();
-    }
-    setupChoiceWidgets();
+    // Only attempt to wire up the file bug form if the form is rendered.
+    var filebugform = Y.one('#filebug-form');
+    var bugreportingform = Y.one('#bug-reporting-form');
+    if (Y.Lang.isValue(filebugform) || Y.Lang.isValue(bugreportingform)) {
+        var search_button =
+            Y.one(Y.DOM.byId('field.actions.projectgroupsearch'));
+        if (Y.Lang.isValue(search_button )) {
+            search_button.set('value', 'Check again');
+        }
+        if (LP.cache.show_information_type_in_ui) {
+            setup_information_type();
+        } else {
+            setup_security_related();
+        }
+        setupChoiceWidgets();
+    }
     var filebug_privacy_text = "This report will be private. " +
         "You can disclose it later.";
     update_privacy_banner(

=== modified file 'lib/lp/bugs/javascript/tests/test_filebug.html'
--- lib/lp/bugs/javascript/tests/test_filebug.html	2012-05-28 12:34:29 +0000
+++ lib/lp/bugs/javascript/tests/test_filebug.html	2012-06-06 04:49:19 +0000
@@ -68,6 +68,7 @@
         <div class='login-logout'></div>
         <div id="fixture"></div>
         <script type="text/x-template" id="privacy-banner-template">
+        <div id="filebug-form">
         <table class="radio-button-widget">
             <tbody>
             <tr>
@@ -114,6 +115,7 @@
             <option value="High">High</option>
             </select>
         </div>
+        </div>
         </script>
     </body>
 </html>