From 9b376e45881c6bd0a899f931547957d3a7c2bd5b Mon Sep 17 00:00:00 2001
From: Karl Hobley <karlhobley10@gmail.com>
Date: Fri, 5 Jun 2015 12:27:42 +0100
Subject: [PATCH 1/7] Tidied up embed filter tests

Less complicated and a bit more isolated now
---
 .../templatetags/wagtailembeds_tags.py        |  4 +-
 wagtail/wagtailembeds/tests.py                | 40 +++++--------------
 2 files changed, 12 insertions(+), 32 deletions(-)

diff --git a/wagtail/wagtailembeds/templatetags/wagtailembeds_tags.py b/wagtail/wagtailembeds/templatetags/wagtailembeds_tags.py
index 40f42aeb0..8c6963960 100644
--- a/wagtail/wagtailembeds/templatetags/wagtailembeds_tags.py
+++ b/wagtail/wagtailembeds/templatetags/wagtailembeds_tags.py
@@ -1,7 +1,7 @@
 from django import template
 from django.utils.safestring import mark_safe
 
-from wagtail.wagtailembeds.embeds import get_embed
+from wagtail.wagtailembeds import embeds
 
 
 register = template.Library()
@@ -9,7 +9,7 @@ register = template.Library()
 
 @register.filter
 def embed(url, max_width=None):
-    embed = get_embed(url, max_width=max_width)
+    embed = embeds.get_embed(url, max_width=max_width)
     try:
         if embed is not None:
             return mark_safe(embed.html)
diff --git a/wagtail/wagtailembeds/tests.py b/wagtail/wagtailembeds/tests.py
index ab28dec41..e02e8ce25 100644
--- a/wagtail/wagtailembeds/tests.py
+++ b/wagtail/wagtailembeds/tests.py
@@ -273,42 +273,22 @@ class TestOembed(TestCase):
 
 
 class TestEmbedFilter(TestCase):
-    def setUp(self):
-        class DummyResponse(object):
-            def read(self):
-                return b"foo"
-        self.dummy_response = DummyResponse()
+    @patch('wagtail.wagtailembeds.embeds.get_embed')
+    def test_direct_call(self, get_embed):
+        get_embed.return_value = Embed(html='<img src="http://www.example.com" />')
 
-    @patch('six.moves.urllib.request.urlopen')
-    @patch('json.loads')
-    def test_valid_embed(self, loads, urlopen):
-        urlopen.return_value = self.dummy_response
-        loads.return_value = {'type': 'photo',
-                              'url': 'http://www.example.com'}
         result = embed_filter('http://www.youtube.com/watch/')
+
         self.assertEqual(result, '<img src="http://www.example.com" />')
 
-    @patch('six.moves.urllib.request.urlopen')
-    @patch('json.loads')
-    def test_render_filter(self, loads, urlopen):
-        urlopen.return_value = self.dummy_response
-        loads.return_value = {'type': 'photo',
-                              'url': 'http://www.example.com'}
-        temp = template.Template('{% load wagtailembeds_tags %}{{ "http://www.youtube.com/watch/"|embed }}')
-        context = template.Context()
-        result = temp.render(context)
-        self.assertEqual(result, '<img src="http://www.example.com" />')
+    @patch('wagtail.wagtailembeds.embeds.get_embed')
+    def test_call_from_template(self, get_embed):
+        get_embed.return_value = Embed(html='<img src="http://www.example.com" />')
 
-    @patch('six.moves.urllib.request.urlopen')
-    @patch('json.loads')
-    def test_render_filter_nonexistent_type(self, loads, urlopen):
-        urlopen.return_value = self.dummy_response
-        loads.return_value = {'type': 'foo',
-                              'url': 'http://www.example.com'}
         temp = template.Template('{% load wagtailembeds_tags %}{{ "http://www.youtube.com/watch/"|embed }}')
-        context = template.Context()
-        result = temp.render(context)
-        self.assertEqual(result, '')
+        result = temp.render(template.Context())
+
+        self.assertEqual(result, '<img src="http://www.example.com" />')
 
 
 class TestEmbedBlock(TestCase):

From 127dd6cc8eecec7bf6aab73f229ac58d1a0d5058 Mon Sep 17 00:00:00 2001
From: Karl Hobley <karlhobley10@gmail.com>
Date: Fri, 5 Jun 2015 12:43:34 +0100
Subject: [PATCH 2/7] Tidied embed rich text handler tests

---
 wagtail/wagtailembeds/format.py |  6 ++--
 wagtail/wagtailembeds/tests.py  | 60 ++++++++++++++++++---------------
 2 files changed, 36 insertions(+), 30 deletions(-)

diff --git a/wagtail/wagtailembeds/format.py b/wagtail/wagtailembeds/format.py
index abbb6b54b..cba95eb55 100644
--- a/wagtail/wagtailembeds/format.py
+++ b/wagtail/wagtailembeds/format.py
@@ -2,12 +2,12 @@ from __future__ import division  # Use true division
 
 from django.template.loader import render_to_string
 
-from wagtail.wagtailembeds.embeds import get_embed
+from wagtail.wagtailembeds import embeds
 
 
 def embed_to_frontend_html(url):
     try:
-        embed = get_embed(url)
+        embed = embeds.get_embed(url)
         if embed is not None:
             # Work out ratio
             if embed.width and embed.height:
@@ -27,7 +27,7 @@ def embed_to_frontend_html(url):
 
 
 def embed_to_editor_html(url):
-    embed = get_embed(url)
+    embed = embeds.get_embed(url)
     if embed is None:
         return
 
diff --git a/wagtail/wagtailembeds/tests.py b/wagtail/wagtailembeds/tests.py
index e02e8ce25..b6eb2a91b 100644
--- a/wagtail/wagtailembeds/tests.py
+++ b/wagtail/wagtailembeds/tests.py
@@ -317,7 +317,7 @@ class TestEmbedBlock(TestCase):
         serialized_empty_val = block.get_prep_value(None)
         self.assertEqual(serialized_empty_val, '')
 
-    @patch('wagtail.wagtailembeds.format.get_embed')
+    @patch('wagtail.wagtailembeds.embeds.get_embed')
     def test_render(self, get_embed):
         get_embed.return_value = Embed(html='<h1>Hello world!</h1>')
 
@@ -334,7 +334,7 @@ class TestEmbedBlock(TestCase):
         # Check that get_embed was called correctly
         get_embed.assert_any_call('http://www.example.com/foo')
 
-    @patch('wagtail.wagtailembeds.format.get_embed')
+    @patch('wagtail.wagtailembeds.embeds.get_embed')
     def test_render_within_structblock(self, get_embed):
         """
         When rendering the value of an EmbedBlock directly in a template
@@ -434,18 +434,21 @@ class TestMediaEmbedHandler(TestCase):
         self.assertEqual(result,
                          {'url': 'test-url'})
 
-    @patch('wagtail.wagtailembeds.embeds.oembed')
-    def test_expand_db_attributes_for_editor(self, oembed):
-        oembed.return_value = {
-            'title': 'test title',
-            'author_name': 'test author name',
-            'provider_name': 'test provider name',
-            'type': 'test type',
-            'thumbnail_url': 'test thumbnail url',
-            'width': 'test width',
-            'height': 'test height',
-            'html': 'test html'
-        }
+    @patch('wagtail.wagtailembeds.embeds.get_embed')
+    def test_expand_db_attributes_for_editor(self, get_embed):
+        get_embed.return_value = Embed(
+            url='http://www.youtube.com/watch/',
+            max_width=None,
+            type='video',
+            html='test html',
+            title='test title',
+            author_name='test author name',
+            provider_name='test provider name',
+            thumbnail_url='http://test/thumbnail.url',
+            width=1000,
+            height=1000,
+        )
+
         result = MediaEmbedHandler.expand_db_attributes(
             {'url': 'http://www.youtube.com/watch/'},
             True
@@ -455,20 +458,23 @@ class TestMediaEmbedHandler(TestCase):
         self.assertIn('<p>URL: http://www.youtube.com/watch/</p>', result)
         self.assertIn('<p>Provider: test provider name</p>', result)
         self.assertIn('<p>Author: test author name</p>', result)
-        self.assertIn('<img src="test thumbnail url" alt="test title">', result)
+        self.assertIn('<img src="http://test/thumbnail.url" alt="test title">', result)
+
+    @patch('wagtail.wagtailembeds.embeds.get_embed')
+    def test_expand_db_attributes(self, get_embed):
+        get_embed.return_value = Embed(
+            url='http://www.youtube.com/watch/',
+            max_width=None,
+            type='video',
+            html='test html',
+            title='test title',
+            author_name='test author name',
+            provider_name='test provider name',
+            thumbnail_url='htto://test/thumbnail.url',
+            width=1000,
+            height=1000,
+        )
 
-    @patch('wagtail.wagtailembeds.embeds.oembed')
-    def test_expand_db_attributes_not_for_editor(self, oembed):
-        oembed.return_value = {
-            'title': 'test title',
-            'author_name': 'test author name',
-            'provider_name': 'test provider name',
-            'type': 'test type',
-            'thumbnail_url': 'test thumbnail url',
-            'width': 'test width',
-            'height': 'test height',
-            'html': 'test html'
-        }
         result = MediaEmbedHandler.expand_db_attributes(
             {'url': 'http://www.youtube.com/watch/'},
             False

From 3737e026bcc5379dad9696f513ec649dba4044c9 Mon Sep 17 00:00:00 2001
From: Karl Hobley <karlhobley10@gmail.com>
Date: Fri, 5 Jun 2015 12:46:18 +0100
Subject: [PATCH 3/7] Failing tests for #1324

---
 wagtail/wagtailembeds/tests.py | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/wagtail/wagtailembeds/tests.py b/wagtail/wagtailembeds/tests.py
index b6eb2a91b..d49c52420 100644
--- a/wagtail/wagtailembeds/tests.py
+++ b/wagtail/wagtailembeds/tests.py
@@ -290,6 +290,16 @@ class TestEmbedFilter(TestCase):
 
         self.assertEqual(result, '<img src="http://www.example.com" />')
 
+    @unittest.expectedFailure
+    @patch('wagtail.wagtailembeds.embeds.get_embed')
+    def test_catches_embed_not_found(self, get_embed):
+        get_embed.side_effect = EmbedNotFoundException
+
+        temp = template.Template('{% load wagtailembeds_tags %}{{ "http://www.youtube.com/watch/"|embed }}')
+        result = temp.render(template.Context())
+
+        self.assertEqual(result, '')
+
 
 class TestEmbedBlock(TestCase):
     def test_deserialize(self):
@@ -460,6 +470,18 @@ class TestMediaEmbedHandler(TestCase):
         self.assertIn('<p>Author: test author name</p>', result)
         self.assertIn('<img src="http://test/thumbnail.url" alt="test title">', result)
 
+    @unittest.expectedFailure
+    @patch('wagtail.wagtailembeds.embeds.get_embed')
+    def test_test_expand_db_attributes_for_editor_catches_embed_not_found(self, get_embed):
+        get_embed.side_effect = EmbedNotFoundException
+
+        result = MediaEmbedHandler.expand_db_attributes(
+            {'url': 'http://www.youtube.com/watch/'},
+            True
+        )
+
+        self.assertEqual(result, '')
+
     @patch('wagtail.wagtailembeds.embeds.get_embed')
     def test_expand_db_attributes(self, get_embed):
         get_embed.return_value = Embed(
@@ -480,3 +502,14 @@ class TestMediaEmbedHandler(TestCase):
             False
         )
         self.assertIn('test html', result)
+
+    @patch('wagtail.wagtailembeds.embeds.get_embed')
+    def test_expand_db_attributes_catches_embed_not_found(self, get_embed):
+        get_embed.side_effect = EmbedNotFoundException
+
+        result = MediaEmbedHandler.expand_db_attributes(
+            {'url': 'http://www.youtube.com/watch/'},
+            False
+        )
+
+        self.assertEqual(result, '')

From 225f54ad6c275d49fba93f01300ee8fe095467f4 Mon Sep 17 00:00:00 2001
From: Karl Hobley <karlhobley10@gmail.com>
Date: Fri, 5 Jun 2015 12:47:44 +0100
Subject: [PATCH 4/7] Tidy up

 - Removed catch all blocks
 - get_embed never returns None so removed check for that
---
 wagtail/wagtailembeds/format.py                          | 2 +-
 wagtail/wagtailembeds/templatetags/wagtailembeds_tags.py | 8 +-------
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/wagtail/wagtailembeds/format.py b/wagtail/wagtailembeds/format.py
index cba95eb55..177331957 100644
--- a/wagtail/wagtailembeds/format.py
+++ b/wagtail/wagtailembeds/format.py
@@ -22,7 +22,7 @@ def embed_to_frontend_html(url):
             })
         else:
             return ''
-    except:
+    except embeds.EmbedNotFoundException:
         return ''
 
 
diff --git a/wagtail/wagtailembeds/templatetags/wagtailembeds_tags.py b/wagtail/wagtailembeds/templatetags/wagtailembeds_tags.py
index 8c6963960..7cb376201 100644
--- a/wagtail/wagtailembeds/templatetags/wagtailembeds_tags.py
+++ b/wagtail/wagtailembeds/templatetags/wagtailembeds_tags.py
@@ -10,10 +10,4 @@ register = template.Library()
 @register.filter
 def embed(url, max_width=None):
     embed = embeds.get_embed(url, max_width=max_width)
-    try:
-        if embed is not None:
-            return mark_safe(embed.html)
-        else:
-            return ''
-    except:
-        return ''
+    return mark_safe(embed.html)

From 00d558c9bebd4c2aaad44961b068d84d2b9ab001 Mon Sep 17 00:00:00 2001
From: Karl Hobley <karlhobley10@gmail.com>
Date: Fri, 5 Jun 2015 12:50:13 +0100
Subject: [PATCH 5/7] Catch EmbedNotFoundExeception fixes #1324

---
 wagtail/wagtailembeds/format.py                | 18 +++++++++++-------
 .../templatetags/wagtailembeds_tags.py         |  7 +++++--
 wagtail/wagtailembeds/tests.py                 |  2 --
 3 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/wagtail/wagtailembeds/format.py b/wagtail/wagtailembeds/format.py
index 177331957..cedb78e1c 100644
--- a/wagtail/wagtailembeds/format.py
+++ b/wagtail/wagtailembeds/format.py
@@ -27,11 +27,15 @@ def embed_to_frontend_html(url):
 
 
 def embed_to_editor_html(url):
-    embed = embeds.get_embed(url)
-    if embed is None:
-        return
+    try:
+        embed = embeds.get_embed(url)
+        if embed is None:
+            return
 
-    # Render template
-    return render_to_string('wagtailembeds/embed_editor.html', {
-        'embed': embed,
-    })
+        # Render template
+        return render_to_string('wagtailembeds/embed_editor.html', {
+            'embed': embed,
+        })
+    except embeds.EmbedNotFoundException:
+        # Could be replaced with a nice error message
+        return ''
diff --git a/wagtail/wagtailembeds/templatetags/wagtailembeds_tags.py b/wagtail/wagtailembeds/templatetags/wagtailembeds_tags.py
index 7cb376201..2c10d21b7 100644
--- a/wagtail/wagtailembeds/templatetags/wagtailembeds_tags.py
+++ b/wagtail/wagtailembeds/templatetags/wagtailembeds_tags.py
@@ -9,5 +9,8 @@ register = template.Library()
 
 @register.filter
 def embed(url, max_width=None):
-    embed = embeds.get_embed(url, max_width=max_width)
-    return mark_safe(embed.html)
+    try:
+        embed = embeds.get_embed(url, max_width=max_width)
+        return mark_safe(embed.html)
+    except embeds.EmbedNotFoundException:
+        return ''
diff --git a/wagtail/wagtailembeds/tests.py b/wagtail/wagtailembeds/tests.py
index d49c52420..1ba97f86d 100644
--- a/wagtail/wagtailembeds/tests.py
+++ b/wagtail/wagtailembeds/tests.py
@@ -290,7 +290,6 @@ class TestEmbedFilter(TestCase):
 
         self.assertEqual(result, '<img src="http://www.example.com" />')
 
-    @unittest.expectedFailure
     @patch('wagtail.wagtailembeds.embeds.get_embed')
     def test_catches_embed_not_found(self, get_embed):
         get_embed.side_effect = EmbedNotFoundException
@@ -470,7 +469,6 @@ class TestMediaEmbedHandler(TestCase):
         self.assertIn('<p>Author: test author name</p>', result)
         self.assertIn('<img src="http://test/thumbnail.url" alt="test title">', result)
 
-    @unittest.expectedFailure
     @patch('wagtail.wagtailembeds.embeds.get_embed')
     def test_test_expand_db_attributes_for_editor_catches_embed_not_found(self, get_embed):
         get_embed.side_effect = EmbedNotFoundException

From 0dc4cbcf218339fa668b7d50452b797fc7c58e31 Mon Sep 17 00:00:00 2001
From: Karl Hobley <karlhobley10@gmail.com>
Date: Fri, 5 Jun 2015 14:21:02 +0100
Subject: [PATCH 6/7] get_embed never returns None

---
 wagtail/wagtailembeds/format.py | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/wagtail/wagtailembeds/format.py b/wagtail/wagtailembeds/format.py
index cedb78e1c..96108f94e 100644
--- a/wagtail/wagtailembeds/format.py
+++ b/wagtail/wagtailembeds/format.py
@@ -8,20 +8,18 @@ from wagtail.wagtailembeds import embeds
 def embed_to_frontend_html(url):
     try:
         embed = embeds.get_embed(url)
-        if embed is not None:
-            # Work out ratio
-            if embed.width and embed.height:
-                ratio = str(embed.height / embed.width * 100) + "%"
-            else:
-                ratio = "0"
 
-            # Render template
-            return render_to_string('wagtailembeds/embed_frontend.html', {
-                'embed': embed,
-                'ratio': ratio,
-            })
+        # Work out ratio
+        if embed.width and embed.height:
+            ratio = str(embed.height / embed.width * 100) + "%"
         else:
-            return ''
+            ratio = "0"
+
+        # Render template
+        return render_to_string('wagtailembeds/embed_frontend.html', {
+            'embed': embed,
+            'ratio': ratio,
+        })
     except embeds.EmbedNotFoundException:
         return ''
 
@@ -29,8 +27,6 @@ def embed_to_frontend_html(url):
 def embed_to_editor_html(url):
     try:
         embed = embeds.get_embed(url)
-        if embed is None:
-            return
 
         # Render template
         return render_to_string('wagtailembeds/embed_editor.html', {

From 4c51d97ae5f91b4d77e1ba0ad7a6980a87cb059a Mon Sep 17 00:00:00 2001
From: Karl Hobley <karlhobley10@gmail.com>
Date: Fri, 5 Jun 2015 14:23:15 +0100
Subject: [PATCH 7/7] Catch all embed exceptions in frontend code

---
 wagtail/wagtailembeds/embeds.py                          | 7 +++++--
 wagtail/wagtailembeds/format.py                          | 4 ++--
 wagtail/wagtailembeds/templatetags/wagtailembeds_tags.py | 2 +-
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/wagtail/wagtailembeds/embeds.py b/wagtail/wagtailembeds/embeds.py
index 9d2f6fcdc..e4690b825 100644
--- a/wagtail/wagtailembeds/embeds.py
+++ b/wagtail/wagtailembeds/embeds.py
@@ -15,10 +15,13 @@ from wagtail.wagtailembeds.oembed_providers import get_oembed_provider
 from wagtail.wagtailembeds.models import Embed
 
 
-class EmbedNotFoundException(Exception):
+class EmbedException(Exception):
     pass
 
-class EmbedlyException(Exception):
+class EmbedNotFoundException(EmbedException):
+    pass
+
+class EmbedlyException(EmbedException):
     pass
 
 class AccessDeniedEmbedlyException(EmbedlyException):
diff --git a/wagtail/wagtailembeds/format.py b/wagtail/wagtailembeds/format.py
index 96108f94e..a2cf72c9d 100644
--- a/wagtail/wagtailembeds/format.py
+++ b/wagtail/wagtailembeds/format.py
@@ -20,7 +20,7 @@ def embed_to_frontend_html(url):
             'embed': embed,
             'ratio': ratio,
         })
-    except embeds.EmbedNotFoundException:
+    except embeds.EmbedException:
         return ''
 
 
@@ -32,6 +32,6 @@ def embed_to_editor_html(url):
         return render_to_string('wagtailembeds/embed_editor.html', {
             'embed': embed,
         })
-    except embeds.EmbedNotFoundException:
+    except embeds.EmbedException:
         # Could be replaced with a nice error message
         return ''
diff --git a/wagtail/wagtailembeds/templatetags/wagtailembeds_tags.py b/wagtail/wagtailembeds/templatetags/wagtailembeds_tags.py
index 2c10d21b7..e527c5f33 100644
--- a/wagtail/wagtailembeds/templatetags/wagtailembeds_tags.py
+++ b/wagtail/wagtailembeds/templatetags/wagtailembeds_tags.py
@@ -12,5 +12,5 @@ def embed(url, max_width=None):
     try:
         embed = embeds.get_embed(url, max_width=max_width)
         return mark_safe(embed.html)
-    except embeds.EmbedNotFoundException:
+    except embeds.EmbedException:
         return ''