Merge lp:~submarine/ubuntu-scopes/colourlovers-set-as-wallpaper into lp:~submarine/ubuntu-scopes/colourlovers

[David Callé]

Create preview for each result. Add set as wallpaper option when previewing a pattern.

Create preview for each result. Add set as wallpaper option when previewing a pattern.

[James Henstridge (jamesh)]

A few issues:

[1]
Do you actually need the Preview.__init__() method? All it seems to be doing is chaining to the parent implementation without adding anything.

[2]
In your Preview.do_run() method, you start by assigning to a number of local variables that only get used once. It doesn't look like there is any real reason for this (e.g. code readability, or shortening long lines of code).

The preview variable is set to None, only to be assigned some other value before it is used further. Perhaps the code would be easier to read without these extra assignments?

[3]
For the path manipulation in Scope.wallpaper_action(), might it be simpler to use the Python standard library to check for the file contents and make the cache directory (i.e. os.path.isfile() and os.makedirs(..., exist_ok=True)).

[4]
In Scope.set_as_wallpaper(), you have more single-use locals. And creating a list of key strings that are then only accessed by index just makes the code more difficult to read compared to using those strings directly.

I'm also not sure what the purpose of this line is:

    gsettings.get_string(KEY[0])

At first I thought that it might be there to trigger an exception if the 'picture-uri' key didn't exist, but it seems to crash the process with unknown keys (probably a bug in the GIR data). Perhaps remove it?

[5]
Can you test this new preview code? Ideally we'd want to know that the preview is generated as expected, and that activating "wallpaper" action correctly downloads the content. You've already split the gsettings code out into a separate method, so it should be easy to stub that out for the test so you don't mess with the developer's settings.

review: Needs Fixing

[David Callé (davidc3)]

I think I've addressed all your concerns. Not sure about using /tmp for the wallpaper download test though.

[David Callé (davidc3)]

FWIW, a temp dir created with tempfile generates this error when running os.makedirs(CACHE, exist_ok=True)
"FileExistsError: [Errno 17] File exists (mode 700 != expected mode 775): '/tmp/tmpaa1fzb'"

[James Henstridge (jamesh)]

Looks better.

Two issues:

[1]
Rather than using a fixed temporary directory, create one with tempfile.mkdtemp() in the test's setUp() method, and delete it in tearDown() using shutil.rmtree(..., ignore_errors=True).

[2]
The test_preview() method looks a bit muddled. It appears to be calling unbound methods of the Preview class passing the Preview class itself (as opposed to an instance) as the first argument.

Isn't it possible to just call scope.create_previewer(result, metadata) to create the previewer instance? You could then call run() on it once, and then inspect the resulting preview.

With those two issues out of the way, I'd be happy for the branch to land.

review: Needs Fixing

[David Callé (davidc3)]

I've updated test_preview.

Regarding the temporary file: using tempfile.mkdtemp() creates a directory with the wrong permissions (unreadable by os.makedirs() in the scope). If I chmod it to the right permissions, it works fine with noestests3, but then fails in the builder. I'm not sure what is the best way to handle this.

[James Henstridge (jamesh)]

I had a look at this and pushed a fix to use mkdtemp(): it looks like the Python 3 version of nosetests is eating the exception due to some Python 2.x baggage, which made the cause a little less obvious. I also stubbed out the gsettings code so it should function in the testing environment.

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/submarine-ubuntu-scopes-colourlovers-autolanding/2/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/submarine-ubuntu-scopes-colourlovers-raring-amd64-autolanding/2/console

review: Needs Fixing (continuous-integration)

[Martin Mrazik (mrazik)]

The above is my mistake. Reapproving.

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/submarine-ubuntu-scopes-colourlovers-autolanding/3/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/submarine-ubuntu-scopes-colourlovers-raring-amd64-autolanding/3/console

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'src/unity_colourlovers_daemon.py'
--- src/unity_colourlovers_daemon.py	2013-03-14 12:27:35 +0000
+++ src/unity_colourlovers_daemon.py	2013-03-25 06:56:20 +0000
@@ -20,6 +20,7 @@
 import urllib.request
 import json
 import gettext
+import os
 
 APP_NAME = 'unity-scope-colourlovers'
 LOCAL_PATH = '/usr/share/locale/'
@@ -27,6 +28,7 @@
 gettext.textdomain(APP_NAME)
 _ = gettext.gettext
 
+CACHE = "%s/unity-scope-colourlovers/" % GLib.get_user_cache_dir()
 GROUP_NAME = 'com.canonical.Unity.Scope.Graphics.Colourlovers'
 UNIQUE_PATH = '/com/canonical/unity/scope/graphics/colourlovers'
 SEARCH_URI = "https://www.colourlovers.com/"
@@ -39,7 +41,7 @@
 DEFAULT_RESULT_MIMETYPE = 'text/html'
 DEFAULT_RESULT_TYPE = Unity.ResultType.DEFAULT
 
-c1 = {'id'      :'colours',
+c1 = {'id'      :'top',
       'name'    :_('Colours'),
       'icon'    :SVG_DIR+'group-graphics.svg',
       'renderer':Unity.CategoryRenderer.VERTICAL_TILE}
@@ -135,10 +137,26 @@
         except Exception as error:
             print (error)
 
+class Preview (Unity.ResultPreviewer):
+
+    def do_run(self):
+        preview = Unity.GenericPreview.new(self.result.title, '', None)
+        preview.props.subtitle = _("By ") + self.result.comment
+        preview.props.image_source_uri = self.result.icon_hint
+        icon = Gio.FileIcon.new (Gio.file_new_for_path(PROVIDER_ICON))
+        view_action = Unity.PreviewAction.new("view", _("View"), icon)
+        preview.add_action(view_action)
+        if "/patterns/" in self.result.icon_hint:
+            wallpaper_action = Unity.PreviewAction.new("wallpaper", _("Set as wallpaper"), None)
+            preview.add_action(wallpaper_action)
+        return preview
+
+
 class Scope (Unity.AbstractScope):
     def __init__(self):
         Unity.AbstractScope.__init__(self)
 
+
     def do_get_search_hint (self):
         return SEARCH_HINT
 
@@ -186,5 +204,33 @@
         se = MySearch (search_context)
         return se
 
+    def do_create_previewer(self, result, metadata):
+        rp = Preview()
+        rp.set_scope_result(result)
+        rp.set_search_metadata(metadata)
+        return rp
+
+    def do_activate(self, result, metadata, id):
+        if id == 'wallpaper':
+            self.wallpaper_action(result.icon_hint)
+            return Unity.ActivationResponse(handled=Unity.HandledType.SHOW_DASH, goto_uri=None)
+        return Unity.ActivationResponse()
+
+    def wallpaper_action(self, image):
+        pattern_name = image.split('/')[-1]
+        local_path = os.path.join(CACHE, pattern_name)
+        if not os.path.isfile(local_path):
+            if not os.path.isdir(CACHE):
+                os.makedirs(CACHE)
+            urllib.request.urlretrieve(image, local_path)
+        self.set_as_wallpaper(local_path)
+        return
+
+    def set_as_wallpaper(self, pattern):
+        gsettings = Gio.Settings.new('org.gnome.desktop.background')
+        gsettings.set_string('picture-uri', "file://" + pattern)
+        gsettings.set_string('picture-options', "wallpaper")
+        gsettings.apply()
+
 def load_scope():
     return Scope()

=== added file 'tests/data/ubuntu-raptor.png'
Binary files tests/data/ubuntu-raptor.png	1970-01-01 00:00:00 +0000 and tests/data/ubuntu-raptor.png	2013-03-25 06:56:20 +0000 differ
=== modified file 'tests/test_colourlovers.py'
--- tests/test_colourlovers.py	2013-02-18 23:17:07 +0000
+++ tests/test_colourlovers.py	2013-03-25 06:56:20 +0000
@@ -1,8 +1,13 @@
 #! /usr/bin/python3
 # -*- coding: utf-8 -*-
+import imp
+import os
+import shutil
+import tempfile
+from unittest import TestCase
+
 from gi.repository import Unity
-from unittest import TestCase
-import imp
+
 
 class ResultSet(Unity.ResultSet):
     def __init__(self):
@@ -29,14 +34,19 @@
         s.run()
         return result_set
 
+class WallpaperTestScope(Unity.AbstractScope):
+    def set_as_wallpaper(pattern):
+        return
 
 class TestColourlovers(ScopeTestCase):
     def setUp(self):
         self.init_scope('src/unity_colourlovers_daemon.py')
+        self.cache_dir = tempfile.mkdtemp()
 
     def tearDown(self):
         self.scope = None
         self.scope_module = None
+        shutil.rmtree(self.cache_dir, ignore_errors=True)
 
     def test_valid_searches(self):
         self.scope_module.SEARCH_URI = 'file:tests/data/mock_colourlovers_pass#'
@@ -53,6 +63,28 @@
             results.append(result_set.results[0]['icon'])
         self.assertEqual(results, expected_results)
 
+    def test_preview(self):
+        result = Unity.ScopeResult()
+        result.title = 'Fancy Title'
+        result.comment = 'author'
+        result.icon_hint = 'http://www.colourlovers.com/img/fancy_pattern.png'
+        scope = self.scope_module.Scope()
+        previewer = scope.create_previewer(result, Unity.SearchMetadata())
+        self.assertEqual(previewer.run().props.title, result.title)
+        self.assertEqual(previewer.run().props.subtitle, 'By '+result.comment)
+        self.assertEqual(previewer.run().props.image_source_uri, result.icon_hint)
+
+    def test_wallpaper_caching(self):
+        self.scope_module.CACHE = self.cache_dir
+        patterns = []
+        def set_as_wallpaper(pattern):
+            patterns.append(pattern)
+        self.scope.set_as_wallpaper = set_as_wallpaper
+        self.scope.wallpaper_action('file:tests/data/ubuntu-raptor.png')
+        local_path = os.path.join(self.cache_dir, 'ubuntu-raptor.png')
+        self.assertEqual(os.path.isfile(local_path), True)
+        self.assertEqual(patterns, [local_path])
+
     def test_failing_search(self):
         self.scope_module.SEARCH_URI = 'file:tests/data/mock_colourlovers_fail#'
         for s in ['query']: