Merge lp:~alecu/unity-scope-click/preview-fixes into lp:unity-scope-click/devel

[Alejandro J. Cura]

Change the order of screenshot and header widgets; initial tests for previews

[lp:~alecu/unity-scope-click/preview-fixes]

353. By Alejandro J. Cura on 2014-07-22

A comment about PreviewReply being hard to use in tests

[dobey (dobey)]

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:353
http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-ci/229/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-amd64-ci/204/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-armhf-ci/203/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-i386-ci/203/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-team-unity-scope-click-devel-ci/229/rebuild

review: Needs Fixing (continuous-integration)

[Paweł Stołowski (stolowski)]

233 +class FakePreviewReply : public scopes::PreviewReply
234 +{
235 +public:
236 + // TODO: make PreviewReply more easily mockable in scopes-api

Scopes API has MockPreviewReply in the testing namespace, is there any issue with it? (if so, please open a bug against scopes api). I think it does exactly what you need and implement with with this FakePreviewReplyClass.

review: Needs Fixing

[lp:~alecu/unity-scope-click/preview-fixes]

354. By Alejandro J. Cura on 2014-07-22

Use MockPreviewReply instead

[Paweł Stołowski (stolowski)]

Looks good and works fine. +1

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:354
http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-ci/231/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-amd64-ci/206
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-armhf-ci/205
        deb: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-armhf-ci/205/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-i386-ci/205

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-team-unity-scope-click-devel-ci/231/rebuild

review: Approve (continuous-integration)

=== modified file 'libclickscope/click/preview.cpp'
--- libclickscope/click/preview.cpp	2014-07-16 21:49:11 +0000
+++ libclickscope/click/preview.cpp	2014-07-22 16:06:10 +0000
@@ -177,6 +177,16 @@
 {
 }
 
+void PreviewStrategy::pushPackagePreviewWidgets(const unity::scopes::PreviewReplyProxy &reply,
+                                                const PackageDetails &details,
+                                                const scopes::PreviewWidgetList& button_area_widgets)
+{
+    reply->push(headerWidgets(details));
+    reply->push(button_area_widgets);
+    reply->push(screenshotsWidgets(details));
+    reply->push(descriptionWidgets(details));
+}
+
 PreviewStrategy::~PreviewStrategy()
 {
 }
@@ -235,10 +245,9 @@
     }
 }
 
-scopes::PreviewWidgetList PreviewStrategy::headerWidgets(const click::PackageDetails& details)
+scopes::PreviewWidgetList PreviewStrategy::screenshotsWidgets(const click::PackageDetails& details)
 {
     scopes::PreviewWidgetList widgets;
-
     bool has_screenshots = !details.main_screenshot_url.empty() || !details.more_screenshots_urls.empty();
 
     if (has_screenshots)
@@ -259,6 +268,12 @@
         gallery.add_attribute_value("sources", scopes::Variant(arr));
         widgets.push_back(gallery);
     }
+    return widgets;
+}
+
+scopes::PreviewWidgetList PreviewStrategy::headerWidgets(const click::PackageDetails& details)
+{
+    scopes::PreviewWidgetList widgets;
 
     scopes::PreviewWidget header("hdr", "header");
     header.add_attribute_value("title", scopes::Variant(details.package.title));
@@ -423,9 +438,7 @@
                   qDebug() << "Successfully created UDM Download.";
                   populateDetails([this, reply, object_path](const PackageDetails &details) {
                           store_department(details);
-                          reply->push(headerWidgets(details));
-                          reply->push(progressBarWidget(object_path));
-                          reply->push(descriptionWidgets(details));
+                          pushPackagePreviewWidgets(reply, details, progressBarWidget(object_path));
                           startLauncherAnimation(details);
                       },
                       [this, reply](const ReviewList& reviewlist,
@@ -526,9 +539,7 @@
     getApplicationUri(manifest, [this, reply, manifest, app_name, &review](const std::string& uri) {
             populateDetails([this, reply, uri, manifest, app_name, &review](const PackageDetails &details){
                 store_department(details);
-                reply->push(headerWidgets(details));
-                reply->push(createButtons(uri, manifest));
-                reply->push(descriptionWidgets(details));
+                pushPackagePreviewWidgets(reply, details, createButtons(uri, manifest));
 
                 if (review.rating == 0 && manifest.removable) {
                     scopes::PreviewWidgetList review_input;
@@ -743,9 +754,7 @@
 
     populateDetails([this, reply](const PackageDetails &details){
             store_department(details);
-            reply->push(headerWidgets(details));
-            reply->push(uninstalledActionButtonWidgets(details));
-            reply->push(descriptionWidgets(details));
+            pushPackagePreviewWidgets(reply, details, uninstalledActionButtonWidgets(details));
         },
         [this, reply](const ReviewList& reviewlist,
                       click::Reviews::Error error) {

=== modified file 'libclickscope/click/preview.h'
--- libclickscope/click/preview.h	2014-07-14 15:35:10 +0000
+++ libclickscope/click/preview.h	2014-07-22 16:06:10 +0000
@@ -54,6 +54,7 @@
 class DepartmentUpdater
 {
 protected:
+    DepartmentUpdater() = default;
     DepartmentUpdater(const std::shared_ptr<click::DepartmentsDb>& depts);
     virtual ~DepartmentUpdater() = default;
     void store_department(const PackageDetails& pkg);
@@ -120,6 +121,7 @@
                                  std::function<void(const click::ReviewList&,
                                                     click::Reviews::Error)> reviews_callback);
     virtual scopes::PreviewWidgetList headerWidgets(const PackageDetails &details);
+    virtual scopes::PreviewWidgetList screenshotsWidgets(const PackageDetails &details);
     virtual scopes::PreviewWidgetList descriptionWidgets(const PackageDetails &details);
     virtual scopes::PreviewWidgetList reviewsWidgets(const click::ReviewList &reviewlist);
     virtual scopes::PreviewWidgetList downloadErrorWidgets();
@@ -129,6 +131,9 @@
                                                    const scopes::Variant& action_id,
                                                    const scopes::Variant& action_label,
                                                    const scopes::Variant& action_uri = scopes::Variant::null());
+    virtual void pushPackagePreviewWidgets(const unity::scopes::PreviewReplyProxy &reply,
+                                           const PackageDetails& details,
+                                           const scopes::PreviewWidgetList& button_area_widgets);
     scopes::Result result;
     QSharedPointer<click::web::Client> client;
     QSharedPointer<click::Index> index;
@@ -151,6 +156,7 @@
 class InstallingPreview : public PreviewStrategy, public DepartmentUpdater
 {
 public:
+    InstallingPreview(const unity::scopes::Result& result) : PreviewStrategy(result) {}
     InstallingPreview(std::string const& download_url,
                       const unity::scopes::Result& result,
                       const QSharedPointer<click::web::Client>& client,

=== modified file 'libclickscope/tests/CMakeLists.txt'
--- libclickscope/tests/CMakeLists.txt	2014-07-14 08:49:19 +0000
+++ libclickscope/tests/CMakeLists.txt	2014-07-22 16:06:10 +0000
@@ -30,6 +30,7 @@
   test_index.cpp
   test_interface.cpp
   test_package.cpp
+  test_preview.cpp
   test_reviews.cpp
   test_smartconnect.cpp
   test_webclient.cpp

=== added file 'libclickscope/tests/test_preview.cpp'
--- libclickscope/tests/test_preview.cpp	1970-01-01 00:00:00 +0000
+++ libclickscope/tests/test_preview.cpp	2014-07-22 16:06:10 +0000
@@ -0,0 +1,131 @@
+/*
+ * Copyright (C) 2014 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3, as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * In addition, as a special exception, the copyright holders give
+ * permission to link the code of portions of this program with the
+ * OpenSSL library under certain conditions as described in each
+ * individual source file, and distribute linked combinations
+ * including the two.
+ * You must obey the GNU General Public License in all respects
+ * for all of the code used other than OpenSSL.  If you modify
+ * file(s) with this exception, you may extend this exception to your
+ * version of the file(s), but you are not obligated to do so.  If you
+ * do not wish to do so, delete this exception statement from your
+ * version.  If you delete this exception statement from all source
+ * files in the program, then also delete it here.
+ */
+
+#include <unity/scopes/testing/MockPreviewReply.h>
+
+#include <gtest/gtest.h>
+#include <click/preview.h>
+
+using namespace ::testing;
+using namespace unity::scopes;
+
+class FakeResult : public Result
+{
+public:
+    FakeResult(VariantMap& vm) : Result(vm)
+    {
+    }
+
+};
+
+class FakePreview : public click::PreviewStrategy
+{
+public:
+    FakePreview(Result& result) : click::PreviewStrategy::PreviewStrategy(result)
+    {
+
+    }
+
+    void run(const PreviewReplyProxy &/*reply*/)
+    {
+
+    }
+    using click::PreviewStrategy::screenshotsWidgets;
+};
+
+class PreviewsBaseTest : public Test
+{
+public:
+    VariantMap vm;
+    VariantMap internal;
+    VariantMap attrs;
+
+    PreviewsBaseTest()
+    {
+        attrs["uri"] = "fake://result";
+        vm["internal"] = internal;
+        vm["attrs"] = attrs;
+    }
+};
+
+class PreviewStrategyTest : public PreviewsBaseTest
+{
+
+};
+
+TEST_F(PreviewStrategyTest, testScreenshotsWidget)
+{
+    FakeResult result{vm};
+    FakePreview preview{result};
+    click::PackageDetails details;
+    details.main_screenshot_url = "sshot1";
+    details.more_screenshots_urls = {"sshot2", "sshot3"};
+
+    auto widgets = preview.screenshotsWidgets(details);
+    ASSERT_EQ(widgets.size(), 1);
+    auto first_widget = widgets.front();
+    auto attributes = first_widget.attribute_values();
+    auto sources = attributes["sources"].get_array();
+    ASSERT_EQ(sources[0].get_string(), "sshot1");
+    ASSERT_EQ(sources[1].get_string(), "sshot2");
+    ASSERT_EQ(sources[2].get_string(), "sshot3");
+}
+
+class FakePreviewStrategy :  public click::PreviewStrategy
+{
+public:
+    FakePreviewStrategy(const unity::scopes::Result& result) : PreviewStrategy(result) {}
+    using click::PreviewStrategy::pushPackagePreviewWidgets;
+    std::vector<std::string> call_order;
+    virtual void run(unity::scopes::PreviewReplyProxy const& /*reply*/)
+    {
+    }
+
+    virtual unity::scopes::PreviewWidgetList screenshotsWidgets (const click::PackageDetails &/*details*/) {
+        call_order.push_back("screenshots");
+        return {};
+    }
+    virtual unity::scopes::PreviewWidgetList headerWidgets (const click::PackageDetails &/*details*/) {
+        call_order.push_back("header");
+        return {};
+    }
+};
+
+TEST_F(PreviewStrategyTest, testScreenshotsPushedAfterHeader)
+{
+    FakeResult result{vm};
+    unity::scopes::testing::MockPreviewReply reply;
+    std::shared_ptr<unity::scopes::testing::MockPreviewReply> replyptr{&reply, [](unity::scopes::testing::MockPreviewReply*){}};
+    click::PackageDetails details;
+
+    FakePreviewStrategy preview{result};
+    preview.pushPackagePreviewWidgets(replyptr, details, {});
+    std::vector<std::string> expected_order {"header", "screenshots"};
+    ASSERT_EQ(expected_order, preview.call_order);
+}