launchpad-reviewers team mailing list archive

["j.c.sackett" <jonathan.sackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/nofollow-noindex-652315 into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #652315 unknown code page is missing meta robots noindex,nofollow
  https://bugs.launchpad.net/bugs/652315


Summary
=======

Adds robots=nofollow,noindex to the product code page if codehosting_usage is
UNKNOWN.

Proposed fix
============

Add robots=nofollow,noindex to the product code in a condtional in the head_epilogue.

Implementation details
======================

As above.

Tests
=====

bin/test -m lp.code.browser.tests.test_product

Demo and Q/A
============

If you view source for code.launchpad.dev/mega-money-maker you will see a meta tag setting robots to nofollow,nodinex.

If you view source for code.launchpad.dev/evolution, you should not see the tag.

Lint
====

make lint output:

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/code/browser/tests/test_product.py
  lib/lp/code/templates/product-branches.pt

-- 
https://code.launchpad.net/~jcsackett/launchpad/nofollow-noindex-652315/+merge/37986
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/nofollow-noindex-652315 into lp:launchpad/devel.

=== modified file 'lib/lp/code/browser/tests/test_product.py'
--- lib/lp/code/browser/tests/test_product.py	2010-10-04 19:50:45 +0000
+++ lib/lp/code/browser/tests/test_product.py	2010-10-08 16:06:49 +0000
@@ -158,7 +158,8 @@
     """Tests for the product code page, especially the usage messasges."""
 
     def test_external_mirrored(self):
-        # Test that the correct URL is displayed for a mirrored branch.
+        # A mirrored branch says code is hosted externally, and displays
+        # upstream data.
         product, branch = self.makeProductAndDevelopmentFocusBranch(
             branch_type=BranchType.MIRRORED,
             url="http://example.com/mybranch";)
@@ -174,8 +175,13 @@
                         branch_url=branch.url))
         self.assertTextMatchesExpressionIgnoreWhitespace(expected, text)
 
+        # The code page should set robots to noindex, nofollow.
+        meta_string = '<meta name="robots" content="noindex,nofollow" />'
+        self.assertNotIn(meta_string, browser.contents)
+
     def test_external_remote(self):
-        # Test that a remote branch is shown properly.
+        # A remote branch says code is hosted externally, and displays
+        # upstream data.
         product, branch = self.makeProductAndDevelopmentFocusBranch(
             branch_type=BranchType.REMOTE,
             url="http://example.com/mybranch";)
@@ -192,7 +198,13 @@
                         branch_url=branch.url))
         self.assertTextMatchesExpressionIgnoreWhitespace(expected, text)
 
+        # The code page should set robots to noindex, nofollow.
+        meta_string = '<meta name="robots" content="noindex,nofollow" />'
+        self.assertNotIn(meta_string, browser.contents)
+
     def test_unknown(self):
+        # A product with no branches should tell the user that Launchpad
+        # doesn't know where the code is hosted.
         product = self.factory.makeProduct()
         self.assertEqual(ServiceUsage.UNKNOWN, product.codehosting_usage)
         browser = self.getUserBrowser(canonical_url(product, rootsite='code'))
@@ -205,7 +217,12 @@
             dict(product_title=product.title))
         self.assertTextMatchesExpressionIgnoreWhitespace(expected, text)
 
+        # The code page should set robots to noindex, nofollow.
+        meta_string = '<meta name="robots" content="noindex,nofollow" />'
+        self.assertIn(meta_string, browser.contents)
+
     def test_on_launchpad(self):
+        # A product that hosts its code on Launchpad just shows the branches.
         product, branch = self.makeProductAndDevelopmentFocusBranch()
         self.assertEqual(ServiceUsage.LAUNCHPAD, product.codehosting_usage)
         browser = self.getUserBrowser(canonical_url(product, rootsite='code'))
@@ -215,7 +232,12 @@
         expected = "1 Active  branch owned by 1 person.*"
         self.assertTextMatchesExpressionIgnoreWhitespace(expected, text)
 
+        # The code page should not set robots to noindex, nofollow.
+        meta_string = '<meta name="robots" content="noindex,nofollow" />'
+        self.assertNotIn(meta_string, browser.contents)
+
     def test_view_mirror_location(self):
+        # Mirror's show the correct upstream mirror url.
         url = "http://example.com/mybranch";
         product, branch = self.makeProductAndDevelopmentFocusBranch(
             branch_type=BranchType.MIRRORED,

=== modified file 'lib/lp/code/templates/product-branches.pt'
--- lib/lp/code/templates/product-branches.pt	2010-09-28 19:25:54 +0000
+++ lib/lp/code/templates/product-branches.pt	2010-10-08 16:06:49 +0000
@@ -6,6 +6,12 @@
   metal:use-macro="view/macro:page/main_side"
   i18n:domain="launchpad"
 >
+<head>
+  <tal:head-epilogue metal:fill-slot="head_epilogue">
+    <meta tal:condition="context/codehosting_usage/enumvalue:UNKNOWN"
+      name="robots" content="noindex,nofollow" />
+  </tal:head-epilogue>
+</head>
 
 <body>