Merge lp:~stolowski/unity-scope-click/fix-1343242 into lp:unity-scope-click

[Paweł Stołowski]

Report all departments in the Store scope so that Shell receives also the siblings of current department and can properly render departments menu when going to a specific department via 'Ubuntu store' button of Apps scope.

Report all departments in the Store scope so that Shell receives also the siblings of current department and can properly render departments menu when going to a specific department via 'Ubuntu store' button of Apps scope.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:300
http://jenkins.qa.ubuntu.com/job/unity-scope-click-ci/553/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-vivid/286/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-click-vivid-amd64-ci/9
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-click-vivid-armhf-ci/9
        deb: http://jenkins.qa.ubuntu.com/job/unity-scope-click-vivid-armhf-ci/9/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-vivid/247/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/292
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/292/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[dobey (dobey)]

Looks ok to me.

review: Approve

=== modified file 'scope/clickstore/store-query.cpp'
--- scope/clickstore/store-query.cpp	2014-10-22 19:32:37 +0000
+++ scope/clickstore/store-query.cpp	2014-12-04 15:01:55 +0000
@@ -224,62 +224,40 @@
     });
 }
 
-//
-// creates department menu with narrowed-down list of subdepartments of current department, as
-// returned by server call
-void click::Query::populate_departments(const click::DepartmentList& subdepts, const std::string& current_dep_id, unity::scopes::Department::SPtr &root)
+unity::scopes::Department::SPtr click::Query::fromClickDepartment(const click::Department::SCPtr click_dept, const std::string& current_dept_id, const click::DepartmentList& subdepts)
 {
+    const std::locale loc("");
+    unity::scopes::Department::SPtr dep = unity::scopes::Department::create(click_dept->id(), query(), click_dept->name());
+    if (click_dept->has_children_flag())
+    {
+        dep->set_has_subdepartments();
+    }
     unity::scopes::DepartmentList departments;
 
-    // create a list of subdepartments of current department
-    foreach (auto d, subdepts)
+    // subdepts is a list of subdepartments of current department fetched from the Store for current search; if we encountered current department in the tree,
+    // insert the list from the server rather than the one from internal cache.
+    for (auto click_subdep: (click_dept->id() == current_dept_id ? subdepts : click_dept->sub_departments()))
     {
-        unity::scopes::Department::SPtr department = unity::scopes::Department::create(d->id(), query(), d->name());
-        if (d->has_children_flag())
-        {
-            department->set_has_subdepartments();
-        }
-        departments.push_back(department);
+        departments.push_back(fromClickDepartment(click_subdep, current_dept_id, subdepts));
     }
-
-    const std::locale loc("");
     departments.sort([&loc](const unity::scopes::Department::SCPtr &d1, const unity::scopes::Department::SCPtr &d2) -> bool {
             return loc(d1->label(), d2->label()) > 0;
-            });
-
-    if (current_dep_id != "")
-    {
-        auto curr_dpt = impl->department_lookup.get_department_info(current_dep_id);
-        if (curr_dpt != nullptr)
-        {
-            unity::scopes::Department::SPtr current = unity::scopes::Department::create(current_dep_id, query(), curr_dpt->name());
-            if (departments.size() > 0) // this may be a leaf department
-            {
-                current->set_subdepartments(departments);
-            }
-
-            auto parent_info = impl->department_lookup.get_parent(current_dep_id);
-            if (parent_info != nullptr)
-            {
-                root = unity::scopes::Department::create(parent_info->id(), query(), parent_info->name());
-                root->set_subdepartments({current});
-                return;
-            }
-            else
-            {
-                root = unity::scopes::Department::create("", query(), _("All"));
-                root->set_subdepartments({current});
-                return;
-            }
-        }
-        else
-        {
-            qWarning() << "Unknown department:" << QString::fromStdString(current_dep_id);
-        }
-    }
-
-    root = unity::scopes::Department::create("", query(), _("All"));
-    root->set_subdepartments(departments);
+    });
+    dep->set_subdepartments(departments);
+
+    return dep;
+}
+
+//
+// creates department menu with narrowed-down list of subdepartments of current department, as
+// returned by server call
+unity::scopes::Department::SPtr click::Query::populate_departments(const click::DepartmentList& subdepts, const std::string& current_dep_id)
+{
+    // Return complete departments tree (starting from 'All') every time, rather than only parent - current - children; this is the only
+    // way we can display siblings corrctly when navigating from Apps scope straight into a subdepartment of Store - see LP #1343242.
+    // For currently visible department include its subdepartments as returned for current search by the server (subdepts) -
+    // all others are constructed from the department lookup.
+    return fromClickDepartment(impl->department_lookup.get_department_info(""), current_dep_id, subdepts);
 }
 
 // recursively store all departments in the departments database
@@ -446,9 +424,8 @@
 
     if (query().department_id() == "") // top-level departments
     {
-        unity::scopes::Department::SPtr root;
         auto subdepts = curdep->sub_departments();
-        populate_departments(subdepts, query().department_id(), root);
+        auto root = populate_departments(subdepts, query().department_id());
         push_departments(searchReply, root);
 
         qDebug() << "pushing cached highlights";
@@ -464,8 +441,7 @@
                     if (error == click::Index::Error::NoError)
                     {
                         qDebug() << "departments call completed";
-                        unity::scopes::Department::SPtr root;
-                        populate_departments(depts, query().department_id(), root);
+                        auto root = populate_departments(depts, query().department_id());
                         push_departments(searchReply, root);
                         push_highlights(searchReply, highlights, locallyInstalledApps);
                     }

=== modified file 'scope/clickstore/store-query.h'
--- scope/clickstore/store-query.h	2014-10-17 19:36:02 +0000
+++ scope/clickstore/store-query.h	2014-12-04 15:01:55 +0000
@@ -84,7 +84,8 @@
     pay::PurchasedList purchased_apps;
 
 protected:
-    virtual void populate_departments(const click::DepartmentList& depts, const std::string& current_department_id, unity::scopes::Department::SPtr &root);
+    virtual unity::scopes::Department::SPtr fromClickDepartment(const click::Department::SCPtr click_dept, const std::string& current_dept_id, const click::DepartmentList& subdepts);
+    virtual unity::scopes::Department::SPtr populate_departments(const click::DepartmentList& depts, const std::string& current_department_id);
     virtual void store_departments(const click::DepartmentList& depts);
     virtual void push_departments(const scopes::SearchReplyProxy& searchReply, const scopes::Department::SCPtr& root);
     virtual void add_highlights(scopes::SearchReplyProxy const& searchReply, const PackageSet& installedPackages);