Merge lp:~henninge/launchpad/bug-467079 into lp:launchpad

= Bug 467079 =

The list of featured projects on the home page had a fixed length, an error which I introduced last cylcle although the old solution was bad too. It capped the first column at a fixed number and let the second grow to infinivity.

== Proposed fix ==

Dynamically calculate the length for each column according to the number of featured projects minus the top featured project. If the number is odd, make the first column one longer.

== Text ==

bin/test -vvct featuredprojects

== Demo/QA ==

Look at the homepage and see if Ubuntu and Zope are visible again ...

= 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/registry/browser/root.py
  lib/lp/registry/stories/launchpad-root/xx-featuredprojects.txt

== Pylint notices ==

lib/lp/registry/browser/root.py
    45: [F0401] Unable to import 'lazr.batchnavigator.z3batching' (No module named batchnavigator)
    509: [E1002, WindowedListBatch.__iter__] Use super on an old style class

Hi Henning,

Thanks for fixing this! I'd prefer to see this land on edge next week - I can't classify it as a blocker, but I won't be offended if you get a second opinion :)

Hi Henning.

  review needs-fixing

Your view and CSS changes are good. The narrative of the story needs
revising. It is not a story. The narrative must explain what is seen and
happens from the users perspective. *We* care about contracts, and if
this test ware really about what *we* want to create, we could
instantiate the view, call render() and verify the markup much easier in
a view test.

I do not see how this test can pass. It checks for ids that do not exist
since the columns table-column properties were replace with simple CSS.

On Tue, 2009-11-17 at 17:23 +0000, Henning Eggers wrote:
> === modified file 'lib/lp/registry/stories/launchpad-root/xx-featuredprojects.txt'
> --- lib/lp/registry/stories/launchpad-root/xx-featuredprojects.txt 2009-09-21 14:48:17 +0000
> +++ lib/lp/registry/stories/launchpad-root/xx-featuredprojects.txt 2009-11-17 17:21:11 +0000
> @@ -24,21 +24,32 @@
> >>> print extract_text(featured.h2)
> Featured projects
>
> -We show the featured projects in our sample data, the "project of the day" is
> -"GNOME" and it is listed first:
> +We show the featured projects in our sample data. The "project of the day"
> +is "GNOME" and it is listed separately:

Any user can see the Launchpad featured projects. The project of the day
is listed separately.

> - >>> for link in featured.findAll('a'):
> - ... print extract_text(link)
> + >>> top_project = find_tag_by_id(featured, 'homepage-featured-top-title')
> + >>> print extract_text(top_project)
> GNOME
> +
> +These are the projects in the left column.

He see's content in a left column.

--but I cannot see how this will work because the template does not have
these ids, we let the stylesheet do the layout. There is a single list
that we need to test. If the layout is changed in the CSS, we do not
need to worry about the test breaking. This was one of the 3.0 UI
goals--no brittle tests that verifies markup. While this does not verify
the markup, it still feels like it is trying to force a two column
table.

> + >>> def print_column(column_id):
> + ... column = find_tag_by_id(featured, column_id)
> + ... for link in column.findAll('a'):
> + ... print extract_text(link)
> + >>> print_column('homepage-featured-left')
> Gnome Applets
> Bazaar
> Mozilla Firefox
> Gentoo
> +
> +These are the projects in the right column.

And content in a right column.

-- but I know that this test will fail

> + >>> print_column('homepage-featured-right')
> GNOME Terminal
> the Mozilla Project
> Mozilla Thunderbird
> Ubuntu
> - Browse all ... projects
>
> == Adding a featured project ==
>
> @@ -88,19 +99,27 @@
>
> >>> anon_browser.open('http://launchpad.dev/')
> >>> featured = find_tag_by_id(anon_browser.contents, 'homepage-featured')
> - >>> for link in featured.findAll('a'):
> - ... print extract_text(link)
> + >>> top_project = find_tag_by_id(featured, 'homepage-featured-top-title')
> + >>> print extract_text(top_project)
> Gentoo

I am not sure 'odd' is important here if we change the css, to be 3
column or 1 column it will not be odd. As...

Am 18.11.2009 01:18, Curtis Hovey schrieb:
> Review: Needs Fixing
> Hi Henning.
>
> review needs-fixing
>
> Your view and CSS changes are good.

Thanks, great to hear.

> The narrative of the story needs
> revising. It is not a story. The narrative must explain what is seen and
> happens from the users perspective. *We* care about contracts, and if
> this test ware really about what *we* want to create, we could
> instantiate the view, call render() and verify the markup much easier in
> a view test.

Ok, I reworded the test in that manner.

>
> I do not see how this test can pass. It checks for ids that do not exist
> since the columns table-column properties were replace with simple CSS.

I am very sorry but I knew that and I thought I had told you about it on
IRC yesterday. I just didn't have the time to update the test. This test
was still referring to the changes I had originally introduced. Again,
sorry for getting you confused...

I fixed the the test and also changed an id in the template to a class
because the id was used twice. I'll paste the incremental diff.

Cheers,
Henning

=== modified file 'lib/canonical/launchpad/templates/root-index.pt'
--- lib/canonical/launchpad/templates/root-index.pt 2009-11-13 18:03:35 +0000
+++ lib/canonical/launchpad/templates/root-index.pt 2009-11-18 11:03:37 +0000
@@ -51,14 +51,14 @@
           margin-top: 1em;
           font-size: 161%;
           }
- .featured-project h3 {
+ .featured-project-top h3 {
           font-size: 131%;
           font-weight: bold;
           }
- .featured-project h3 img {
+ .featured-project-top h3 img {
           vertical-align: middle;
           }
- .featured-project p {
+ .featured-project-top p {
           margin-top: 0.5em;
           margin-bottom: 1em;
           border-bottom: 1px solid #888888;
@@ -226,7 +226,7 @@
             <div id="homepage-featured" class="homepage-portlet">
               <h2>Featured projects</h2>

- <div class="featured-project"
+ <div class="featured-project-top"
                    tal:define="topproject view/featured_projects_top"
                    tal:condition="topproject">
                 <h3>
@@ -241,7 +241,7 @@
               </div>

               <div class="two-column-list">
- <ul id="homepage-featured">
+ <ul class="featured-projects-list">
                   <li tal:repeat="project view/featured_projects">
                     <a tal:replace="structure project/fmt:link" />
                   </li>

=== modified file 'lib/lp/registry/stories/launchpad-root/xx-featuredprojects.txt'
--- lib/lp/registry/stories/launchpad-root/xx-featuredprojects.txt 2009-10-31 18:32:05 +0000
+++ lib/lp/registry/stories/launchpad-root/xx-featuredprojects.txt 2009-11-18 11:22:40 +0000
@@ -19,33 +19,26 @@
     >>> from lp.registry.browser.root import LaunchpadRootIndexView
     >>> LaunchpadRootIndexView._get_day_of_year = staticmethod(
     ... fake_get_day_of_year)
+
+Anonymous users will see the list of featured projects with links to the
+projects' pages in Launchpad. The "project of the day" is listed separately.
+
     >>> anon_browser.open('http://launchpad.dev/')
     >>> featured = find_tag_by_id(anon_browser.contents, 'homepage-featured')
     >>> print extract_text(featured.h2)
     Featured projects

-We show the featured projects in our sample data. The "project of the day"
-is "GNOME" and it is listed separately:
-
- >>> top_project = find_tag_by_id(featured, 'homepage-featured-top-title')
- >>> print extract_text(top_project)
+ >>> top_project = featured.find('', 'featured-project-top')
+ >>> print extract_text(top_project.h3)
     GNOME

-These are the projects in the left column.
-
- >>> def print_column(column_id):
- ... column = find_tag_by_id(featured, column_id)
- ... for link in column.findAll('a'):
- ... print extract_text(link)
- >>> print_column('homepage-featured-left')
+ >>> featured_list = featured.find('', 'featured-projects-list')
+ >>> for link in featured_list.findAll('a'):
+ ... print extract_text(link)
     Gnome Applets
     Bazaar
     Mozilla Firefox
     Gentoo
-
-These are the projects in the ri...

Thanks for removing the limitiation