launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Thiago F. Pappacena has proposed merging ~pappacena/launchpad:hide-opengraph-meta-on-404 into launchpad:master.

Commit message:
Hiding OpenGraph metadata on 404 pages.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1883935 in Launchpad itself: "Friendlier URL preview metadata for private links"
  https://bugs.launchpad.net/launchpad/+bug/1883935

For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/386210
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:hide-opengraph-meta-on-404 into launchpad:master.

diff --git a/lib/lp/app/browser/tests/test_base_layout.py b/lib/lp/app/browser/tests/test_base_layout.py
index db768e2..e0fefde 100644
--- a/lib/lp/app/browser/tests/test_base_layout.py
+++ b/lib/lp/app/browser/tests/test_base_layout.py
@@ -1,4 +1,4 @@
-# Copyright 2010-2019 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for base-layout.pt and its macros.
@@ -40,8 +40,12 @@ class TestBaseLayout(TestCaseWithFactory):
         self.user = self.factory.makePerson(name='waffles')
         self.context = None
 
-    def makeTemplateView(self, layout, context=None):
-        """Return a view that uses the specified layout."""
+    def makeTemplateView(self, layout, context=None, view_attributes=None):
+        """Return a view that uses the specified layout.
+
+        :params view_attributes: A dict containing extra attributes for the
+                                 view object.
+        """
 
         class TemplateView(LaunchpadView):
             """A simple view to test base-layout."""
@@ -61,6 +65,9 @@ class TestBaseLayout(TestCaseWithFactory):
         request.setPrincipal(self.user)
         request.traversed_objects.append(self.context)
         view = TemplateView(self.context, request)
+        if view_attributes:
+            for k, v in view_attributes.items():
+                setattr(view, k, v)
         request.traversed_objects.append(view)
         return view
 
@@ -270,3 +277,17 @@ class TestBaseLayout(TestCaseWithFactory):
         self.assertIn('png', og_image.get('content'))
         self.assertIn('Test', og_title.get('content'))
         self.assertIn('http', og_url.get('content'))
+
+    def test_opengraph_metadata_missing_on_404_page(self):
+        view = self.makeTemplateView(
+            'main_side', view_attributes={'show_opengraph_meta': False})
+        content = BeautifulSoup(view())
+
+        og_title = content.find('meta', {'property': 'og:title'})
+        self.assertIsNone(og_title)
+        og_type = content.find('meta', {'property': 'og:type'})
+        self.assertIsNone(og_type)
+        og_image = content.find('meta', {'property': 'og:image'})
+        self.assertIsNone(og_image)
+        og_url = content.find('meta', {'property': 'og:url'})
+        self.assertIsNone(og_url)
diff --git a/lib/lp/app/templates/base-layout.pt b/lib/lp/app/templates/base-layout.pt
index b54d36d..08df3dc 100644
--- a/lib/lp/app/templates/base-layout.pt
+++ b/lib/lp/app/templates/base-layout.pt
@@ -54,11 +54,18 @@
         tal:attributes="content view/page_description/fmt:strip-email/fmt:shorten/500" />
     </tal:view>
 
-    <meta property="og:title" tal:attributes="content view/fmt:pagetitle" />
-    <meta property="og:type" content="website" />
-    <meta property="og:image" content="/@@/launchpad.png" />
-    <meta property="og:url" tal:attributes="content python: view.recommended_canonical_url or request.URL" />
-    <meta property="og:site_name" content="Launchpad" />
+    <tal:comment condition="nothing">
+    By default, we include OpenGraph metadata on every page, unless the view
+    explicitly sets its `show_opengraph_meta` attribute to False.
+    </tal:comment>
+    <tal:opengraph
+        condition="python: getattr(view, 'show_opengraph_meta', True)">
+      <meta property="og:title" tal:attributes="content view/fmt:pagetitle" />
+      <meta property="og:type" content="website" />
+      <meta property="og:image" content="/@@/launchpad.png" />
+      <meta property="og:url" tal:attributes="content python: view.recommended_canonical_url or request.URL" />
+      <meta property="og:site_name" content="Launchpad" />
+    </tal:opengraph>
 
     <tal:view condition="view/private">
       <meta name="referrer" content="origin-when-cross-origin"/>
diff --git a/lib/lp/services/webapp/error.py b/lib/lp/services/webapp/error.py
index 1fbc2b2..e8ebc6f 100644
--- a/lib/lp/services/webapp/error.py
+++ b/lib/lp/services/webapp/error.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -187,6 +187,8 @@ class NotFoundView(SystemErrorView):
 
     response_code = http_client.NOT_FOUND
 
+    show_opengraph_meta = False
+
     def __call__(self):
         return self.index()