Merge lp:~bac/launchpad/lep-projconfig into lp:launchpad

[Brad Crittenden]

Show project maintainers application configuration links and a progress bar showing completion status.

= Summary =

The summary of this work is in a LEP at
https://dev.launchpad.net/LEP/ProjectConfiguration
and bug 70613.

In short, show project owners how to finish setting up their project and
allow all users the ability to see the things they could be doing, if
only they were configured.

== Proposed fix ==

Add a progress bar and icons showing the owner remaining configuration
items.

If some one chooses not to use a LP facility there is no way to make
that known and silence the warning.

== Pre-implementation notes ==

Talks with Jono and Curtis.

== Implementation details ==

As above.

== Tests ==

Since changes were made to the menu links, really all tests need to be run.

== Demo and Q/A ==

http://launchpad.dev/firefox

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/templates/product-index.pt
  lib/canonical/launchpad/webapp/interfaces.py
  lib/lp/registry/browser/product.py
  lib/lp/registry/browser/pillar.py
  lib/canonical/launchpad/icing/style-3-0.css.in
  lib/lp/registry/stories/product/xx-product-files.txt
  lib/lp/registry/templates/pillar-involvement-portlet.pt
  lib/lp/registry/browser/tests/pillar-views.txt
  lib/canonical/launchpad/webapp/menu.py
  lib/canonical/launchpad/templates/launchpad-inline-link.pt
  lib/lp/registry/tests/test_product.py

./lib/canonical/launchpad/webapp/interfaces.py
     905: redefinition of unused 'StartRequestEvent' from line 897
     300: E301 expected 1 blank line, found 0
     302: E202 whitespace before ')'
     310: E302 expected 2 blank lines, found 1
     331: E202 whitespace before ')'
     336: E301 expected 1 blank line, found 0
     339: E301 expected 1 blank line, found 0
     341: E301 expected 1 blank line, found 0
     343: E301 expected 1 blank line, found 0
     446: E301 expected 1 blank line, found 0
     453: E301 expected 1 blank line, found 0
     461: E301 expected 1 blank line, found 0
     476: E301 expected 1 blank line, found 0
     520: E302 expected 2 blank lines, found 1
     626: E202 whitespace before ')'
     659: E202 whitespace before ')'
     774: E301 expected 1 blank line, found 0
     844: E301 expected 1 blank line, found 0
     899: E301 expected 1 blank line, found 0
     905: E301 expected 1 blank line, found 2
     217: Line exceeds 78 characters.
./lib/lp/registry/browser/product.py
    2026: E222 multiple spaces after operator
./lib/canonical/launchpad/icing/style-3-0.css.in
    1290: Line exceeds 78 characters.
    1887: Line exceeds 78 characters.
    1903: Line exceeds 78 characters.
    1907: Line exceeds 78 characters.
    1915: Line exceeds 78 characters.
    1919: Line exceeds 78 characters.
    1947: Line exceeds 78 characters.
    1963: Line exceeds 78 characters.
    1971: Line exceeds 78 characters.
    1975: Line exceeds 78 characters.
    1979: Line exceeds 78 characters.
    1983: Line exceeds 78 characters.
    1987: Line exceeds 78 characters.
    1991: Line exceeds 78 characters.
    2004: Line exceeds 78 characters.
    2008: Line exceeds 78 characters.
    2040: Line exceeds 78 characters.
    2052: Line exceeds 78 characters.
    2056: Line exceeds 78 characters.
    2060: Line exceeds 78 characters.
    2068: Line exceeds 78 characters.
    2072: Line exceeds 78 characters.
    2080: Line exceeds 78 characters.
    2108: Line exceeds 78 characters.
    2112: Line exceeds 78 characters.
    2140: Line exceeds 78 characters.
    2148: Line exceeds 78 characters.
    2153: Line exceeds 78 characters.
    2157: Line exceeds 78 characters.
    2162: Line exceeds 78 characters.
    2166: Line exceeds 78 characters.
    2170: Line exceeds 78 characters.
    2174: Line exceeds 78 characters.
    2178: Line exceeds 78 characters.
    2230: Line exceeds 78 characters.
    2238: Line exceeds 78 characters.
    2242: Line exceeds 78 characters.
    2250: Line exceeds 78 characters.
    2262: Line exceeds 78 characters.
    2270: Line exceeds 78 characters.
    2274: Line exceeds 78 characters.
    2278: Line exceeds 78 characters.
    2286: Line exceeds 78 characters.
    2290: Line exceeds 78 characters.
    2294: Line exceeds 78 characters.
    2298: Line exceeds 78 characters.
    2302: Line exceeds 78 characters.
    2311: Line exceeds 78 characters.
    2319: Line exceeds 78 characters.
    2451: Line exceeds 78 characters.
    2452: Line exceeds 78 characters.
    2521: Line exceeds 78 characters.
    2522: Line exceeds 78 characters.
./lib/lp/registry/stories/product/xx-product-files.txt
       0: narrative uses a moin header.
      36: want exceeds 78 characters.
      44: narrative uses a moin header.
     109: narrative uses a moin header.
     158: narrative uses a moin header.
     386: narrative uses a moin header.
     436: narrative uses a moin header.
     475: narrative uses a moin header.
./lib/lp/registry/browser/tests/pillar-views.txt
     133: narrative exceeds 78 characters.
./lib/canonical/launchpad/webapp/menu.py
      85: E222 multiple spaces after operator
     133: W602 deprecated form of raising exception
     180: E301 expected 1 blank line, found 0
     482: E301 expected 1 blank line, found 0
     500: E302 expected 2 blank lines, found 3
./lib/lp/registry/tests/test_product.py
      30: E302 expected 2 blank lines, found 1

I'll wade through these and correct as necessary.

[Curtis Hovey (sinzui)]

Hi Brad.

This change looks great. I see I see extra white space between the download
portlet and announcements.

I think we need a model change to support this :( I an configure Answers to
not be official and my application may not be translatable. So the progress
bar will never be complete. Consider Launchpad. It does not have a po file,
so it cannot be translated, but the UI requires me to set translations to be
official to make the bar be complete. I think we need to None, True, or False
to make this work. I am seeing this same issue my the feature I am working
on. I propose we switch from bool to a vocab or UNKNOWN, LAUNCHPAD, EXTERNAL.
We may only need this some of the apps.

I have a lot of concern about the implementation. I do not think ILink, its
model, and template needed to change. I certainly do not think we should
be making invalid HTML on all Launchpad pages for a single portlet that
appears on one page. I think the fix is easy, but I have not spent a lot
of time looking at the issue.

> === modified file 'lib/canonical/launchpad/icing/style-3-0.css.in'
> --- lib/canonical/launchpad/icing/style-3-0.css.in 2010-07-26 14:48:43 +0000
> +++ lib/canonical/launchpad/icing/style-3-0.css.in 2010-07-26 22:01:07 +0000
> @@ -686,6 +686,13 @@
> margin: 0;
> padding: 0;
> }
> +div.centered {
> + text-align: center;
> + }
> +div.centered table {
> + margin: 0 auto;
> + text-align: left;
> + }

We used to have rules like this for the 1.0 layouts. I guess It was naive
of me to remove them.

> === modified file 'lib/canonical/launchpad/templates/launchpad-inline-link.pt'
> --- lib/canonical/launchpad/templates/launchpad-inline-link.pt 2009-08-27 02:22:07 +0000
> +++ lib/canonical/launchpad/templates/launchpad-inline-link.pt 2010-07-26 22:01:07 +0000
> @@ -2,8 +2,10 @@
> xmlns:tal="http://xml.zope.org/namespaces/tal"
> condition="context/enabled">
> <tal:link-linked
> - condition="context/linked"><a
> - href="" class="" title=""
> + condition="context/linked">
> + <table width="100%"><tr>
> + <td width="90%">
> + <a href="" class="" title=""

This looks scary. I think it is wrong. This file is used to adapt every link,
and most links are inline text.

/me runs lp

This cannot land. It is create tables in paragraphs and inline markup that
cannot contain tables.

Since we are designing a presentation that will appear on exactly one page
and we have not discussed reusing it. I expected to see either a subclass
or some form of delegates so that the portlet's needs did not step on any
other part of launchpad. The Involvement portlet is already a bundle of
exceptions for menus and does its own layout. I think the view and existing
template could be doing all the work.

Maybe we should have looked closer at why the Involvement used CSS to place
a right aligned background image.

...

> === modified file 'lib/canonical/launchpad/webapp/interfaces.py'
> --- lib/canonical/launchpad/webapp/interfaces.py 2010-07-21 08:15:57 +0000
> +++ lib/canonical/launchpad/webapp/interfaces.py 2010-07-26 22:01:07 +0000
> @@ -238,6 +238,10 @@
> icon_url = Attribute(
> "The full URL for this link'...

review: Needs Fixing (code and ui)

[Brad Crittenden (bac)]

Hi Curtis,
Thanks for the review and the excellent suggestions to save me from myself.

Diff at http://pastebin.ubuntu.com/472353/
New screenshot at http://people.canonical.com/~bac/config-progress-bar.png

[Curtis Hovey (sinzui)]

The inline diff is really messed up so I am just looking at the paste.

in lib/lp/registry/templates/pillar-involvement-portlet.pt, I see invalid markup. A list must contain an li, and it can never contain a table. The ul is not needed since the table is a block element.

+ <ul tal:condition="view/configuration_links" style="padding-top: 1em">
+ <table style="width: 100%">

maybe this works
     <table style="padding-top: 1em; width: 100%"
       tal:condition="view/configuration_links">

review: Needs Fixing (code)

[Brad Crittenden (bac)]

Sorry I failed to see and strip out that <ul>.

Diff: http://pastebin.ubuntu.com/472388/

[Curtis Hovey (sinzui)]

Looks good to land.

review: Approve (code)

=== modified file 'lib/canonical/launchpad/icing/style-3-0.css.in'
--- lib/canonical/launchpad/icing/style-3-0.css.in	2010-07-26 14:48:43 +0000
+++ lib/canonical/launchpad/icing/style-3-0.css.in	2010-08-03 12:43:45 +0000
@@ -686,6 +686,13 @@
     margin: 0;
     padding: 0;
     }
+div.centered {
+    text-align: center;
+    }
+div.centered table {
+    margin: 0 auto;
+    text-align: left;
+    }
 .batch-navigation-links .next {
     /* Next links have icons: */
     background: center right no-repeat;

=== modified file 'lib/canonical/launchpad/webapp/menu.py'
--- lib/canonical/launchpad/webapp/menu.py	2010-06-15 19:35:41 +0000
+++ lib/canonical/launchpad/webapp/menu.py	2010-08-03 12:43:45 +0000
@@ -122,12 +122,13 @@
 
         :param menu: The sub menu used by the page that the link represents.
         """
+
         self.target = target
         self.text = text
         self.summary = summary
         self.icon = icon
         if not isinstance(enabled, bool):
-            raise AssertionError, "enabled must be boolean, got %r" % enabled
+            raise AssertionError("enabled must be boolean, got %r" % enabled)
         self.enabled = enabled
         self.site = site
         self.menu = menu

=== modified file 'lib/lp/registry/browser/pillar.py'
--- lib/lp/registry/browser/pillar.py	2010-06-17 15:26:05 +0000
+++ lib/lp/registry/browser/pillar.py	2010-08-03 12:43:45 +0000
@@ -154,6 +154,14 @@
             link for link in important_links if not link.enabled],
             key=attrgetter('sort_key'))
 
+    @property
+    def registration_completeness(self):
+        """The percent complete for registration.
+
+        Not used by all pillars.
+        """
+        return None
+
 
 class PillarBugsMenu(ApplicationMenu, StructuralSubscriptionMenuMixin):
     """Base class for pillar bugs menus."""

=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py	2010-08-02 02:13:52 +0000
+++ lib/lp/registry/browser/product.py	2010-08-03 12:43:45 +0000
@@ -350,26 +350,77 @@
     """Encourage configuration of involvement links for projects."""
 
     has_involvement = True
-    visible_disabled_link_names = ['submit_code']
+
+    @property
+    def visible_disabled_link_names(self):
+        """Show all disabled links...except blueprints"""
+        involved_menu = MenuAPI(self).navigation
+        all_links = involved_menu.keys()
+        # The register blueprints link should not be shown since its use is
+        # not encouraged.
+        all_links.remove('register_blueprint')
+        return all_links
+
+    @cachedproperty
+    def configuration_states(self):
+        """Create a dictionary indicating the configuration statuses.
+
+        Each app area will be represented in the return dictionary, except
+        blueprints which we are not currently promoting.
+        """
+        states = {}
+        states['configure_bugtracker'] = (
+            self.official_malone or self.context.bugtracker is not None)
+        states['configure_answers'] = self.official_answers
+        states['configure_translations'] = self.official_rosetta
+        states['configure_codehosting'] = (
+            self.context.development_focus.branch is not None)
+        return states
 
     @property
     def configuration_links(self):
-        """The enabled involvement links."""
+        """The enabled involvement links.
+
+        Returns a list of dicts keyed by:
+        'link' -- the menu link, and
+        'configured' -- a boolean representing the configuration status.
+        """
         overview_menu = MenuAPI(self.context).overview
         series_menu = MenuAPI(self.context.development_focus).overview
         configuration_names = [
+            'configure_bugtracker',
             'configure_answers',
-            'configure_bugtracker',
             'configure_translations',
+            #'configure_blueprints',
             ]
-        configuration_links = [
-            overview_menu[name] for name in configuration_names]
+        config_list = []
+        config_statuses = self.configuration_states
+        for key in configuration_names:
+            config_list.append(dict(link=overview_menu[key],
+                                    configured=config_statuses[key]))
+
+        # Add the branch configuration in separately.
         set_branch = series_menu['set_branch']
         set_branch.text = 'Configure project branch'
-        configuration_links.append(set_branch)
-        return sorted([
-            link for link in configuration_links if link.enabled],
-            key=attrgetter('sort_key'))
+        set_branch.configured = (
+            )
+        config_list.append(
+            dict(link=set_branch,
+                 configured=config_statuses['configure_codehosting']))
+        return config_list
+
+    @property
+    def registration_completeness(self):
+        """The percent complete for registration."""
+        configured = 0
+        config_statuses = self.configuration_states
+        for key, value in config_statuses.items():
+            if value:
+                configured += 1
+        scale = 100
+        done = int(float(configured) / len(config_statuses) * scale)
+        undone = scale - done
+        return dict(done=done, undone=undone)
 
 
 class ProductNavigationMenu(NavigationMenu):

=== modified file 'lib/lp/registry/browser/productseries.py'
--- lib/lp/registry/browser/productseries.py	2010-08-02 02:51:42 +0000
+++ lib/lp/registry/browser/productseries.py	2010-08-03 12:43:45 +0000
@@ -215,7 +215,8 @@
         series_menu = MenuAPI(self.context).overview
         set_branch = series_menu['set_branch']
         set_branch.text = 'Configure series branch'
-        return [set_branch]
+        return [dict(link=set_branch,
+                     configured=self.official_codehosting)]
 
 
 class ProductSeriesOverviewMenu(

=== modified file 'lib/lp/registry/browser/tests/pillar-views.txt'
--- lib/lp/registry/browser/tests/pillar-views.txt	2010-05-03 18:44:42 +0000
+++ lib/lp/registry/browser/tests/pillar-views.txt	2010-08-03 12:43:45 +0000
@@ -53,10 +53,10 @@
       <h2>Get Involved</h2>
       <ul class="involvement">
         <li>
-          <a href=... class="...bugs">Report a bug</a>
+          <a href=... class="...bugs">Report a bug</a>...
         </li>
         <li>
-          <a href=... class="...answers">Ask a question</a>
+          <a href=... class="...answers">Ask a question</a>...
         </li>
       </ul>
     ...
@@ -80,14 +80,54 @@
 
     >>> for link in view.visible_disabled_links:
     ...     print link.name
+    report_bug
+    ask_question
+    help_translate
     submit_code
+
     >>> for link in view.configuration_links:
-    ...     print link.name
+    ...     print link['link'].name
+    configure_bugtracker
     configure_answers
-    configure_bugtracker
     configure_translations
     set_branch
 
+The registration status is determined with the 'configuration_states'
+property.  Notice that blueprints are not included in the
+configuration links nor the completeness computation as the use of
+blueprints is not promoted.
+
+    >>> for key in sorted(view.configuration_states.keys()):
+    ...     print key, view.configuration_states[key]
+    configure_answers False
+    configure_bugtracker False
+    configure_codehosting False
+    configure_translations False
+
+The percentage of the registration completed can be determined by
+using the 'registration_completeness' property, which returns a
+dictionary, which makes it easy for use in the page template.
+
+    >>> print pretty(view.registration_completeness)
+    {'done': 0,
+     'undone': 100}
+
+Changing the product's official usage is reflected in the view properties.
+
+    >>> product.official_malone = True
+    >>> view = create_view(product, '+get-involved')
+    >>> for key in sorted(view.configuration_states.keys()):
+    ...     print key, view.configuration_states[key]
+    configure_answers False
+    configure_bugtracker True
+    configure_codehosting False
+    configure_translations False
+
+    >>> print pretty(view.registration_completeness)
+    {'done': 25,
+     'undone': 75}
+
+
 Project groups are supported too, but they only display the applications used by
 their products.
 

=== modified file 'lib/lp/registry/stories/product/xx-product-development-focus.txt'
--- lib/lp/registry/stories/product/xx-product-development-focus.txt	2010-04-09 16:19:25 +0000
+++ lib/lp/registry/stories/product/xx-product-development-focus.txt	2010-08-03 12:43:45 +0000
@@ -73,10 +73,10 @@
     <span class="invisible-link">Change details</span>
      (http://launchpad.dev/fooix/+edit)
     >>> print_involvement_portlet(owner_browser)
+    Configure bug tracker
+    http://launchpad.dev/fooix/+configure-bugtracker
     Configure support tracker
     http://launchpad.dev/fooix/+configure-answers
-    Configure bug tracker
-    http://launchpad.dev/fooix/+configure-bugtracker
     Configure translations
     http://launchpad.dev/fooix/+configure-translations
     Configure project branch

=== modified file 'lib/lp/registry/templates/pillar-involvement-portlet.pt'
--- lib/lp/registry/templates/pillar-involvement-portlet.pt	2010-06-15 19:37:37 +0000
+++ lib/lp/registry/templates/pillar-involvement-portlet.pt	2010-08-03 12:43:45 +0000
@@ -31,9 +31,40 @@
     </tal:disabled>
   </ul>
 
-  <ul tal:condition="view/configuration_links" style="padding-top: 1em">
-    <li tal:repeat="link view/configuration_links">
-      <a tal:replace="structure link/fmt:link" />
-    </li>
-  </ul>
+  <tal:editor condition="context/required:launchpad.Edit"
+              define="registration view/registration_completeness">
+    <tal:show_configuration condition="registration">
+      <h2>Configuration Progress</h2>
+      <div class="centered">
+          <table width="80%" border="1">
+            <tr>
+              <td>
+              <img
+                 tal:attributes="alt string:${registration/done}% registration complete;
+                                 title string:${registration/done}% registration complete;
+                                 width registration/done;
+                                 style string:width: ${registration/done}%;"
+                 src="/@@/green-bar"
+                 height="10"/>
+              </td>
+            </tr>
+          </table>
+      </div>
+    </tal:show_configuration>
+
+    <table style="width: 100%; padding-top: 1em"
+           tal:condition="view/configuration_links">
+      <tal:item repeat="link_status view/configuration_links">
+        <tr>
+          <td>
+            <a tal:replace="structure link_status/link/fmt:link" />
+          </td>
+          <td style="text-align: right;" tal:condition="registration">
+            <tal:yes-no replace="structure link_status/configured/image:boolean" />
+          </td>
+        </tr>
+      </tal:item>
+    </table>
+
+  </tal:editor>
 </div>

=== modified file 'lib/lp/registry/templates/product-index.pt'
--- lib/lp/registry/templates/product-index.pt	2010-06-24 13:13:01 +0000
+++ lib/lp/registry/templates/product-index.pt	2010-08-03 12:43:45 +0000
@@ -241,6 +241,8 @@
       define="overview_menu context/menu:overview">
       <tal:menu replace="structure view/@@+global-actions" />
 
+      <div tal:replace="structure context/@@+get-involved" />
+
       <div id="downloads" class="top-portlet downloads"
         tal:define="release view/latest_release_with_download_files">
         <h2>Downloads</h2>
@@ -262,8 +264,6 @@
         </p>
       </div>
 
-      <div tal:replace="structure context/@@+get-involved" />
-
       <div tal:replace="structure context/@@+portlet-latestannouncements" />
     </tal:side>
 

=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py	2010-04-21 16:15:36 +0000
+++ lib/lp/registry/tests/test_product.py	2010-08-03 12:43:45 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -84,7 +84,7 @@
         firefox_owner.open('http://launchpad.dev/firefox/+download')
         content = find_main_content(firefox_owner.contents)
         rows = content.findAll('tr')
-        print
+
         a_list = rows[-1].findAll('a')
         # 1st row
         a_element = a_list[0]