Merge lp:~adeuring/launchpad/bug-528788-no-searchbox-without-hot-bugs into lp:launchpad

[Abel Deuring]

This branch fixes bug 528788: "bug search field disappears when all bugs marked as Fixed/Invalid/Wontfix".

The fix is straightforward: The search box is only shown when at least one "hot" bug exists, but it should be shown if at least one bug exists, be it hot or not hot.

This requires a separate DB query: "Does at least one bug exist"? While IHasBugs objects have a property all_bugtasks, I did not want to use it: The SQL queries for similar properties like open_bugtasks, new_bugtasks etc need quite long to execute -- long enough that the content of the portlet showing the number of open bugs, new bugs etc is retrieved in a separate HTTP request.

Instead I defined a new property IHbasBugs.has_bugtasksthat return True if at least one bugtask for the target exists that does not have the status BugTaskStatus.UNKNOWN.

mthaddon kindly ran an EXPLAIN ANALZYE of this query for Ubuntu: I t needs less than 1 millisecond. So, fast enough to not cause any timeout trouble, I think.

tests:
./bin/test -t hasbugs.txt
./bin/test -t xx-front-page-info.txt

= 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/doc/hasbugs.txt
  lib/lp/bugs/interfaces/bugtarget.py
  lib/lp/bugs/model/bugtarget.py
  lib/lp/bugs/stories/bugs/xx-front-page-info.txt
  lib/lp/bugs/templates/bugtarget-bugs.pt
  lib/lp/registry/configure.zcml

== Pylint notices ==

lib/lp/bugs/interfaces/bugtarget.py
    60: [C0301] Line too long (79/78)
    29: [F0401] Unable to import 'lazr.enum' (No module named enum)
    30: [F0401] Unable to import 'lazr.restful.fields' (No module named restful)
    31: [F0401] Unable to import 'lazr.restful.interface' (No module named restful)
    32: [F0401] Unable to import 'lazr.restful.declarations' (No module named restful)
    66: [C0322, IHasBugs.searchTasks] Operator not preceded by a space
    value_type=Text(),
    ^
    required=False),
    search_text=copy_field(IBugTaskSearch['searchtext']),
    status=copy_field(IBugTaskSearch['status']),
    importance=copy_field(IBugTaskSearch['importance']),
    assignee=Reference(schema=Interface),
    bug_reporter=Reference(schema=Interface),
    bug_supervisor=Reference(schema=Interface),
    bug_commenter=Reference(schema=Interface),
    bug_subscriber=Reference(schema=Interface),
    owner=Reference(schema=Interface),
    affected_user=Reference(schema=Interface),
    has_patch=copy_field(IBugTaskSearch['has_patch']),
    has_cve=copy_field(IBugTaskSearch['has_cve']),
    tags=copy_field(IBugTaskSearch['tag']),
    tags_combinator=copy_field(IBugTaskSearch['tags_combinator']),
    omit_duplicates=copy_field(IBugTaskSearch['omit_dupes']),
    omit_targeted=copy_field(IBugTaskSearch['omit_targeted']),
    status_upstream=copy_field(IBugTaskSearch['status_upstream']),
    milestone_assignment=copy_field(
    IBugTaskSearch['milestone_assignment']),
    milestone=copy_field(IBugTaskSearch['milestone']),
    component=copy_field(IBugTaskSearch['component']),
    nominated_for=Reference(schema=Interface),
    has_no_package=copy_field(IBugTaskSearch['has_no_package']),
    hardware_bus=Choice(
    title=u'The bus of a hardware device related to a bug',

    vocabulary=DBEnumeratedType, required=False),
    hardware_vendor_id=TextLine(
    title=(
    u"The vendor ID of a hardware device related to a bug."),
    description=(
    u"Allowed values of the vendor ID depend on the bus of the "
    "device.nn"
    "Vendor IDs of PCI, PCCard and USB devices are hexadecimal "
    "string representations of 16 bit integers in the format "
    "'0x01ab': The prefix '0x', followed by exactly 4 digits; "
    "where a digit is one of the characters 0..9, a..f. The "
    "characters A..F are not allowed.nn"
    "SCSI vendor IDs are strings with exactly 8 characters. "
    "Shorter names are right-padded with space (0x20) characters."
    "nn"
    "IDs for other buses may be arbitrary strings."),
    required=False),
    hardware_product_id=TextLine(
    title=(
    u"The product ID of a hardware device related to a bug."),
    description=(
    u"Allowed values of the product ID depend on the bus of the "
    "device.nn"
    "Product IDs of PCI, PCCard and USB devices are hexadecimal "
    "string representations of 16 bit integers in the format "
    "'0x01ab': The prefix '0x', followed by exactly 4 digits; "
    "where a digit is one of the characters 0..9, a..f. The "
    "characters A..F are not allowed.nn"
    "SCSI product IDs are strings with exactly 16 characters. "
    "Shorter names are right-padded with space (0x20) characters."
    "nn"
    "IDs for other buses may be arbitrary strings."),
    required=False),
    hardware_driver_name=TextLine(
    title=(
    u"The driver controlling a hardware device related to a "
    "bug."),
    required=False),
    hardware_driver_package_name=TextLine(
    title=(
    u"The package of the driver which controls a hardware "
    "device related to a bug."),
    required=False),
    hardware_owner_is_bug_reporter=Bool(
    title=(
    u"Search for bugs reported by people who own the given "
    "device or who use the given hardware driver."),
    required=False),
    hardware_owner_is_affected_by_bug=Bool(
    title=(
    u"Search for bugs where people affected by a bug own the "
    "given device or use the given hardware driver."),
    required=False),
    hardware_owner_is_subscribed_to_bug=Bool(
    title=(
    u"Search for bugs where a bug subscriber owns the "
    "given device or uses the given hardware driver."),
    required=False),
    hardware_is_linked_to_bug=Bool(
    title=(
    u"Search for bugs which are linked to hardware reports "
    "wich contain the given device or whcih contain a device"
    "contolled by the given driver."),
    required=False))
    @operation_returns_collection_of(IBugTask)
    @export_read_operation()
    def searchTasks(search_params, user=None,
    order_by=None, search_text=None,
    status=None, importance=None,
    assignee=None, bug_reporter=None, bug_supervisor=None,
    bug_commenter=None, bug_subscriber=None, owner=None,
    affected_user=None, has_patch=None, has_cve=None,
    distribution=None, tags=None,
    tags_combinator=BugTagsSearchCombinator.ALL,
    omit_duplicates=True, omit_targeted=None,
    status_upstream=None, milestone_assignment=None,
    milestone=None, component=None, nominated_for=None,
    sourcepackagename=None, has_no_package=None,
    hardware_bus=None, hardware_vendor_id=None,
    hardware_product_id=None, hardware_driver_name=None,
    hardware_driver_package_name=None,
    hardware_owner_is_bug_reporter=None,
    hardware_owner_is_affected_by_bug=False,
    hardware_owner_is_subscribed_to_bug=False,
    hardware_is_linked_to_bug=False, linked_branches=None):

[Eleanor Berger (intellectronica)]

review: Approve (code)

[Abel Deuring (adeuring)]

Accessing has_bugtasks failed for IPerson objects in self.all_bugtasks.limit(1) because limit() is not defined in ISQLObjectResultSet. Salgado suggested to cast instead all_bugtasks into a boolean object.

tests:

./bin/test -t hasbugs.txt (the "real" test of HasBugsBase.all_bugtasks)
./bin/test -t doc/person.txt (the previously failing test)

no lint

Incremental diff:

=== modified file 'lib/lp/bugs/model/bugtarget.py'
--- lib/lp/bugs/model/bugtarget.py 2010-03-08 09:38:21 +0000
+++ lib/lp/bugs/model/bugtarget.py 2010-03-09 14:29:31 +0000
@@ -242,17 +242,13 @@
     @property
     def has_bugtasks(self):
         """See `IHasBugs`."""
- # Basically, the return value is self.all_bugtasks.count() > 0.
- # But finding, sorting and counting all bug tasks can take
- # a longer time (long enough that the HTML data for "number of
- # open, critical etc bugs" on a bug page is retrieved in a
- # separate request). all_bugtasks returns a
- # storm.SQLObjectResultSet instance, and this class does not
- # provide methods like is_empty(), so limit the query
- # manually and simply check if at least one bug task exists.
- all_tasks = self.all_bugtasks
- all_tasks = all_tasks.limit(1)
- return all_tasks.count() > 0
+ # Check efficiently if any bugtasks exist. We should avoid
+ # expensive calls like all_bugtasks.count(). all_bugtasks
+ # returns a storm.SQLObjectResultSet instance, and this
+ # class does not provide methods like is_empty(). But we can
+ # indirectly call SQLObjectResultSet._result_set.is_empty()
+ # by converting all_bugtasks into a boolean object.
+ return bool(self.all_bugtasks)

     def getBugCounts(self, user, statuses=None):
         """See `IHasBugs`."""

review: Needs Resubmitting

[Gavin Panella (allenap)]

Hi Abel,

One tiny point in the doctest where it matches against an
auto-generated name. Other than that, great :)

Gavin.

> === modified file 'lib/lp/bugs/doc/hasbugs.txt'
> --- lib/lp/bugs/doc/hasbugs.txt 2009-06-12 16:36:02 +0000
> +++ lib/lp/bugs/doc/hasbugs.txt 2010-03-09 15:35:44 +0000
> @@ -63,3 +63,29 @@
>
> >>> [bugtask.bug.id for bugtask in debian_firefox.all_bugtasks]
> [1, 2, 3, 8]
> +
> +IHasBugs.has_bugtasks is True if at least on bugtask exists.
> +
> + >>> debian_firefox.has_bugtasks
> + True
> +
> +For a new product, has_bugtasks is False.
> +
> + >>> product = factory.makeProduct()
> + >>> product.has_bugtasks
> + False
> +
> +When we add a bug, the property becomes True.
> +
> + >>> bug = factory.makeBug(product=product)
> + >>> product.has_bugtasks
> + True
> +
> +Note that bugtasks having the status BugTaskStatus.UNKNOWN are not
> +considered for has_bugtasks.
> +
> + >>> from lp.bugs.interfaces.bugtask import BugTaskStatus
> + >>> bug.default_bugtask.transitionToStatus(
> + ... BugTaskStatus.UNKNOWN, user=bug.owner)
> + >>> product.has_bugtasks
> + False
>
> === modified file 'lib/lp/bugs/interfaces/bugtarget.py'
> --- lib/lp/bugs/interfaces/bugtarget.py 2010-03-01 11:28:05 +0000
> +++ lib/lp/bugs/interfaces/bugtarget.py 2010-03-09 15:35:44 +0000
> @@ -58,6 +58,8 @@
> "A list of all BugTasks ever reported for this target.")
> max_bug_heat = Attribute(
> "The current highest bug heat value for this target.")
> + has_bugtasks = Attribute(
> + "True if at least one BugTask has ever been reported for this target.")
>
> @call_with(search_params=None, user=REQUEST_USER)
> @operation_parameters(
>
> === modified file 'lib/lp/bugs/model/bugtarget.py'
> --- lib/lp/bugs/model/bugtarget.py 2010-03-03 16:30:00 +0000
> +++ lib/lp/bugs/model/bugtarget.py 2010-03-09 15:35:44 +0000
> @@ -239,6 +239,16 @@
> if IProduct.providedBy(self) and self.project is not None:
> self.project.recalculateMaxBugHeat()
>
> + @property
> + def has_bugtasks(self):
> + """See `IHasBugs`."""
> + # Check efficiently if any bugtasks exist. We should avoid
> + # expensive calls like all_bugtasks.count(). all_bugtasks
> + # returns a storm.SQLObjectResultSet instance, and this
> + # class does not provide methods like is_empty(). But we can
> + # indirectly call SQLObjectResultSet._result_set.is_empty()
> + # by converting all_bugtasks into a boolean object.
> + return bool(self.all_bugtasks)
>
> def getBugCounts(self, user, statuses=None):
> """See `IHasBugs`."""
>
> === modified file 'lib/lp/bugs/stories/bugs/xx-front-page-info.txt'
> --- lib/lp/bugs/stories/bugs/xx-front-page-info.txt 2010-02-18 13:32:47 +0000
> +++ lib/lp/bugs/stories/bugs/xx-front-page-info.txt 2010-03-09 15:35:44 +0000
> @@ -54,3 +54,59 @@
> ... anon_browser.contents, 'no-bugs-filed')
> >>> print extract_text(bug_list)
> There are currently no bugs filed against Project Uses Malone.
> +
> +Since there are no bugs at all filed for the project, no search box is
> +shown.
> +
> + ...

review: Approve (code)

=== modified file 'lib/lp/bugs/doc/hasbugs.txt'
--- lib/lp/bugs/doc/hasbugs.txt	2009-06-12 16:36:02 +0000
+++ lib/lp/bugs/doc/hasbugs.txt	2010-03-09 17:24:33 +0000
@@ -63,3 +63,29 @@
 
     >>> [bugtask.bug.id for bugtask in debian_firefox.all_bugtasks]
     [1, 2, 3, 8]
+
+IHasBugs.has_bugtasks is True if at least on bugtask exists.
+
+    >>> debian_firefox.has_bugtasks
+    True
+
+For a new product, has_bugtasks is False.
+
+    >>> product = factory.makeProduct()
+    >>> product.has_bugtasks
+    False
+
+When we add a bug, the property becomes True.
+
+    >>> bug = factory.makeBug(product=product)
+    >>> product.has_bugtasks
+    True
+
+Note that bugtasks having the status BugTaskStatus.UNKNOWN are not
+considered for has_bugtasks.
+
+    >>> from lp.bugs.interfaces.bugtask import BugTaskStatus
+    >>> bug.default_bugtask.transitionToStatus(
+    ...     BugTaskStatus.UNKNOWN, user=bug.owner)
+    >>> product.has_bugtasks
+    False

=== modified file 'lib/lp/bugs/interfaces/bugtarget.py'
--- lib/lp/bugs/interfaces/bugtarget.py	2010-03-01 11:28:05 +0000
+++ lib/lp/bugs/interfaces/bugtarget.py	2010-03-09 17:24:33 +0000
@@ -58,6 +58,8 @@
         "A list of all BugTasks ever reported for this target.")
     max_bug_heat = Attribute(
         "The current highest bug heat value for this target.")
+    has_bugtasks = Attribute(
+        "True if at least one BugTask has ever been reported for this target.")
 
     @call_with(search_params=None, user=REQUEST_USER)
     @operation_parameters(

=== modified file 'lib/lp/bugs/model/bugtarget.py'
--- lib/lp/bugs/model/bugtarget.py	2010-03-03 16:30:00 +0000
+++ lib/lp/bugs/model/bugtarget.py	2010-03-09 17:24:33 +0000
@@ -239,6 +239,16 @@
         if IProduct.providedBy(self) and self.project is not None:
             self.project.recalculateMaxBugHeat()
 
+    @property
+    def has_bugtasks(self):
+        """See `IHasBugs`."""
+        # Check efficiently if any bugtasks exist. We should avoid
+        # expensive calls like all_bugtasks.count(). all_bugtasks
+        # returns a storm.SQLObjectResultSet instance, and this
+        # class does not provide methods like is_empty(). But we can
+        # indirectly call SQLObjectResultSet._result_set.is_empty()
+        # by converting all_bugtasks into a boolean object.
+        return bool(self.all_bugtasks)
 
     def getBugCounts(self, user, statuses=None):
         """See `IHasBugs`."""

=== modified file 'lib/lp/bugs/stories/bugs/xx-front-page-info.txt'
--- lib/lp/bugs/stories/bugs/xx-front-page-info.txt	2010-02-18 13:32:47 +0000
+++ lib/lp/bugs/stories/bugs/xx-front-page-info.txt	2010-03-09 17:24:33 +0000
@@ -54,3 +54,59 @@
     ...     anon_browser.contents, 'no-bugs-filed')
     >>> print extract_text(bug_list)
     There are currently no bugs filed against Project Uses Malone.
+
+Since there are no bugs at all filed for the project, no search box is
+shown.
+
+    >>> print len(find_tags_by_class(anon_browser.contents, 'search-box'))
+    0
+
+If we add a bug, we see the search box and the list of hot bugs instead of
+the message "There are currently no bugs filed..."
+
+    >>> login('[e-mail]')
+    >>> bug = factory.makeBug(product=uses_malone)
+    >>> logout()
+    >>> anon_browser.open('http://bugs.launchpad.dev/uses-malone')
+    >>> print extract_text(
+    ...     find_tags_by_class(anon_browser.contents, 'search-box')[0])
+    by importance
+    by status
+    ...
+    Advanced search
+
+
+    >>> print find_tag_by_id(anon_browser.contents, 'no-bugs-filed')
+    None
+
+    >>> hot_bugs = find_tag_by_id(anon_browser.contents, 'hot-bugs')
+    >>> print extract_text(hot_bugs)
+    Summary   Status  Importance   Last changed
+    #16  ...  New     Undecided    0 seconds ago
+
+When we mark our bug as "fix released", it is no longer a hot bug,
+and we see a message "no hot bugs" instead of the bug listing.
+
+    >>> from lp.bugs.interfaces.bugtask import BugTaskStatus
+    >>> login('[e-mail]')
+    >>> bug.default_bugtask.transitionToStatus(
+    ...     BugTaskStatus.FIXRELEASED, user=bug.owner)
+    >>> logout()
+
+    >>> anon_browser.open('http://bugs.launchpad.dev/uses-malone')
+    >>> print find_tag_by_id(anon_browser.contents, 'hot-bugs')
+    None
+
+    >>> bug_list = find_tag_by_id(
+    ...     anon_browser.contents, 'no-bugs-filed')
+    >>> print extract_text(bug_list)
+    There are currently no hot bugs filed against Project Uses Malone.
+
+But since the project has a bug, the search box is still visible.
+
+    >>> print extract_text(
+    ...     find_tags_by_class(anon_browser.contents, 'search-box')[0])
+    by importance
+    by status
+    ...
+    Advanced search

=== modified file 'lib/lp/bugs/templates/bugtarget-bugs.pt'
--- lib/lp/bugs/templates/bugtarget-bugs.pt	2010-02-18 14:22:16 +0000
+++ lib/lp/bugs/templates/bugtarget-bugs.pt	2010-03-09 17:24:33 +0000
@@ -68,7 +68,7 @@
                      tal:condition="link/enabled"
                      tal:attributes="href link/url; title link/text">
                     <img tal:attributes="alt link/text" src="/@@/edit" />
-                  </a> 
+                  </a>
                 </tal:edit-bug-supervisor>
               </span>
             </li>
@@ -96,7 +96,7 @@
           </ul>
         </div>
 
-      <tal:has_hot_bugs condition="view/hot_bugs_info/bugtasks">
+      <tal:has_bugtasks condition="context/has_bugtasks">
         <div class="search-box">
           <metal:search
             use-macro="context/@@+bugtarget-macros-search/simple-search-form"
@@ -105,7 +105,9 @@
         <script type="text/javascript"><!--
              $('field.searchtext').focus();
         --></script>
+      </tal:has_bugtasks>
 
+      <tal:has_hot_bugs condition="view/hot_bugs_info/bugtasks">
         <h2 style="margin-top: 1em">Hot bugs</h2>
 
         <table class="listing" id="hot-bugs"
@@ -154,8 +156,9 @@
       </tal:has_hot_bugs>
 
       <tal:no_hot_bugs condition="not: view/hot_bugs_info/bugtasks">
-        <p id="no-bugs-filed"><strong>There are currently no bugs filed against
-          <tal:project_title replace="context/title" />.</strong></p>
+        <p id="no-bugs-filed"><strong>There are currently no
+          <span tal:condition="context/has_bugtasks">hot</span> bugs filed
+          against <tal:project_title replace="context/title" />.</strong></p>
 
         <p id="no-bugs-report"><a href="+filebug">Report a bug.</a></p>
       </tal:no_hot_bugs>

=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml	2010-03-01 11:28:05 +0000
+++ lib/lp/registry/configure.zcml	2010-03-09 17:24:33 +0000
@@ -390,6 +390,7 @@
                 unassigned_bugtasks
                 new_bugtasks
                 all_bugtasks
+                has_bugtasks
                 getUsedBugTags
                 getUsedBugTagsWithOpenCounts
                 bugtargetdisplayname