Merge lp:~unity-api-team/unity-scopes-shell/department-doesnt-go-away into lp:unity-scopes-shell

[Pete Woods]

Remove missing departments

Remove missing departments

[lp:~unity-api-team/unity-scopes-shell/department-doesnt-go-away]

150. By Paweł Stołowski on 2014-09-30

Merged trunk

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:150
http://jenkins.qa.ubuntu.com/job/unity-scopes-shell-ci/238/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scopes-shell-utopic-amd64-ci/135/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-shell-utopic-armhf-ci/135
        deb: http://jenkins.qa.ubuntu.com/job/unity-scopes-shell-utopic-armhf-ci/135/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-shell-utopic-i386-ci/138

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-shell-ci/238/rebuild

review: Needs Fixing (continuous-integration)

[Marcus Tomlinson (marcustomlinson)]

Looks sane & works for me.

review: Approve

=== modified file 'src/Unity/scope.cpp'
--- src/Unity/scope.cpp	2014-09-25 05:16:02 +0000
+++ src/Unity/scope.cpp	2014-09-30 10:00:42 +0000
@@ -442,6 +442,7 @@
     Q_FOREACH(DepartmentNode* child, node->childNodes()) {
         cachedChildrenIds << child->id();
     }
+
     auto subdeps = scopeNode->subdepartments();
     QMap<QString, scopes::Department::SCPtr> childIdMap;
     for (auto it = subdeps.begin(); it != subdeps.end(); ++it) {
@@ -471,6 +472,13 @@
         }
     }
 
+    // department has been removed (not reported by scope); make sure it's only treated as such when
+    // we're examining children of *current* department, othwerwise it would break on partial trees when visiting a leaf.
+    if (firstMismatchingChild == nullptr && scopeNode->subdepartments().size() < cachedChildrenIds.size() &&
+            m_currentNavigationId.toStdString() == scopeNode->id()) {
+        return scopeNode;
+    }
+
     return firstMismatchingChild; // will be nullptr if everything matches
 }
 
 
=== modified file 'src/Unity/scope.h'
--- src/Unity/scope.h	2014-09-15 12:03:13 +0000
+++ src/Unity/scope.h	2014-09-30 10:00:42 +0000
@@ -203,7 +203,7 @@
     void processResultSet(QList<std::shared_ptr<unity::scopes::CategorisedResult>>& result_set);
 
     static unity::scopes::Department::SCPtr findDepartmentById(unity::scopes::Department::SCPtr const& root, std::string const& id);
-    static unity::scopes::Department::SCPtr findUpdateNode(DepartmentNode* node, unity::scopes::Department::SCPtr const& scopeNode);
+    unity::scopes::Department::SCPtr findUpdateNode(DepartmentNode* node, unity::scopes::Department::SCPtr const& scopeNode);
 
     QUuid m_session_id;
     int m_query_id;
 
=== modified file 'tests/data/CMakeLists.txt'
--- tests/data/CMakeLists.txt	2014-08-14 10:14:39 +0000
+++ tests/data/CMakeLists.txt	2014-09-30 10:00:42 +0000
@@ -1,5 +1,6 @@
 add_subdirectory(mock-scope)
 add_subdirectory(mock-scope-departments)
+add_subdirectory(mock-scope-departments-flipflop)
 add_subdirectory(mock-scope-double-nav)
 add_subdirectory(mock-scope-info)
 add_subdirectory(mock-scope-ttl)
 
=== added directory 'tests/data/mock-scope-departments-flipflop'
=== added file 'tests/data/mock-scope-departments-flipflop/CMakeLists.txt'
--- tests/data/mock-scope-departments-flipflop/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ tests/data/mock-scope-departments-flipflop/CMakeLists.txt	2014-09-30 10:00:42 +0000
@@ -0,0 +1,16 @@
+include(FindPkgConfig)
+pkg_check_modules(SCOPESLIB REQUIRED libunity-scopes>=0.6.0)
+
+set(SCOPES_BIN_DIR ${SCOPESLIB_LIBDIR})
+
+include_directories(${SCOPESLIB_INCLUDE_DIRS})
+include_directories(${CMAKE_CURRENT_BINARY_DIR})
+
+set(SCOPE_SOURCES
+    mock-scope-departments-flipflop.cpp
+    )
+
+add_library(mock-scope-departments-flipflop MODULE ${SCOPE_SOURCES})
+target_link_libraries(mock-scope-departments-flipflop ${SCOPESLIB_LDFLAGS})
+
+configure_file(mock-scope-departments-flipflop.ini.in mock-scope-departments-flipflop.ini)
 
=== added file 'tests/data/mock-scope-departments-flipflop/mock-scope-departments-flipflop.cpp'
--- tests/data/mock-scope-departments-flipflop/mock-scope-departments-flipflop.cpp	1970-01-01 00:00:00 +0000
+++ tests/data/mock-scope-departments-flipflop/mock-scope-departments-flipflop.cpp	2014-09-30 10:00:42 +0000
@@ -0,0 +1,144 @@
+/*
+ * Copyright (C) 2014 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser 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 warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Pete Woods <[e-mail]>
+ */
+
+#include <unity/scopes/CategorisedResult.h>
+#include <unity/scopes/ScopeBase.h>
+#include <unity/scopes/SearchReply.h>
+
+#include <iostream>
+#include <thread>
+#include <atomic>
+#include <sstream>
+
+#define EXPORT __attribute__ ((visibility ("default")))
+
+using namespace std;
+using namespace unity::scopes;
+
+class MyQuery : public SearchQueryBase
+{
+public:
+    MyQuery(CannedQuery const& query, SearchMetadata const& metadata, bool flipflop) :
+        SearchQueryBase(query, metadata),
+        flipflop_(flipflop)
+    {
+    }
+
+    ~MyQuery()
+    {
+    }
+
+    virtual void cancelled() override
+    {
+    }
+
+    Department::SPtr create_root_dep(CannedQuery const& query)
+    {
+        Department::SPtr child_dep;
+        Department::SPtr root_dep;
+        root_dep = Department::create("", query, "All departments");
+
+        child_dep = Department::create("books", query, "Books");
+        child_dep->set_has_subdepartments();
+        root_dep->add_subdepartment(child_dep);
+
+        child_dep = Department::create("movies", query, "Movies, TV, Music");
+        child_dep->set_has_subdepartments();
+        root_dep->add_subdepartment(child_dep);
+
+        if (flipflop_)
+        {
+            child_dep = Department::create("electronics", query, "Electronics");
+            child_dep->set_has_subdepartments();
+            root_dep->add_subdepartment(child_dep);
+        }
+
+        child_dep = Department::create("home", query, "Home, Garden & DIY");
+        child_dep->set_has_subdepartments();
+        root_dep->add_subdepartment(child_dep);
+
+        child_dep = Department::create("toys", query, "Toys, Children & Baby");
+        child_dep->set_has_subdepartments();
+        root_dep->add_subdepartment(child_dep);
+
+        return root_dep;
+    }
+
+    virtual void run(SearchReplyProxy const& reply) override
+    {
+        Department::SPtr child_dep;
+        Department::SPtr root_dep;
+        Department::SPtr active_dep;
+
+        root_dep = create_root_dep(query());
+
+        reply->register_departments(root_dep);
+
+        auto cat1 = reply->register_category("cat1", "Category 1", "");
+        CategorisedResult res1(cat1);
+        res1.set_uri("test:uri");
+        res1.set_title("result for: \"" + query().query_string() + "\"");
+        reply->push(res1);
+    }
+
+protected:
+    bool flipflop_;
+};
+
+class MyScope : public ScopeBase
+{
+public:
+    MyScope()
+    {
+        flipflop_ = false;
+    }
+
+    virtual SearchQueryBase::UPtr search(CannedQuery const& q, SearchMetadata const& metadata) override
+    {
+        flipflop_ = !flipflop_;
+        return SearchQueryBase::UPtr(new MyQuery(q, metadata, flipflop_));
+    }
+
+    virtual PreviewQueryBase::UPtr preview(Result const&, ActionMetadata const&) override
+    {
+        return nullptr;
+    }
+
+    std::atomic<bool> flipflop_;
+};
+
+extern "C"
+{
+
+    EXPORT
+    unity::scopes::ScopeBase*
+    // cppcheck-suppress unusedFunction
+    UNITY_SCOPE_CREATE_FUNCTION()
+    {
+        return new MyScope;
+    }
+
+    EXPORT
+    void
+    // cppcheck-suppress unusedFunction
+    UNITY_SCOPE_DESTROY_FUNCTION(unity::scopes::ScopeBase* scope_base)
+    {
+        delete scope_base;
+    }
+
+}
 
=== added file 'tests/data/mock-scope-departments-flipflop/mock-scope-departments-flipflop.ini.in'
--- tests/data/mock-scope-departments-flipflop/mock-scope-departments-flipflop.ini.in	1970-01-01 00:00:00 +0000
+++ tests/data/mock-scope-departments-flipflop/mock-scope-departments-flipflop.ini.in	2014-09-30 10:00:42 +0000
@@ -0,0 +1,8 @@
+[ScopeConfig]
+DisplayName = mock-departments-flipflop.DisplayName
+Description = mock-departments-flipflop.Description
+Art = /mock-departments-flipflop.Art
+Icon = /mock-departments-flipflop.Icon
+SearchHint = mock-departments-flipflop.SearchHint
+HotKey = mock-departments-flipflop.HotKey
+Author = mock-departments-flipflop.Author
 
=== modified file 'tests/departmentstest.cpp'
--- tests/departmentstest.cpp	2014-09-02 09:39:41 +0000
+++ tests/departmentstest.cpp	2014-09-30 10:00:42 +0000
@@ -49,6 +49,7 @@
     QScopedPointer<Scopes> m_scopes;
     Scope* m_scope;
     Scope* m_scope_navs;
+    Scope* m_scope_flipflop;
     QScopedPointer<RegistrySpawner> m_registry;
 
 private Q_SLOTS:
@@ -65,7 +66,7 @@
     void init()
     {
         QStringList favs;
-        favs << "scope://mock-scope-departments" << "scope://mock-scope-double-nav";
+        favs << "scope://mock-scope-departments" << "scope://mock-scope-double-nav" << "scope://mock-scope-departments-flipflop";
         setFavouriteScopes(favs);
 
         m_scopes.reset(new Scopes(nullptr));
@@ -88,8 +89,13 @@
         QVERIFY(m_scope_navs != nullptr);
         m_scope_navs->setActive(true);
 
+        m_scope_flipflop = qobject_cast<scopes_ng::Scope*>(m_scopes->getScope(QString("mock-scope-departments-flipflop")));
+        QVERIFY(m_scope_flipflop != nullptr);
+        m_scope_flipflop->setActive(true);
+
         QTRY_COMPARE(m_scope->searchInProgress(), false);
         QTRY_COMPARE(m_scope_navs->searchInProgress(), false);
+        QTRY_COMPARE(m_scope_flipflop->searchInProgress(), false);
     }
 
     void cleanup()
@@ -304,6 +310,47 @@
         QCOMPARE(sortOrderModel->data(idx, Department::Roles::RoleNavigationId), QVariant(QString("top")));
         QCOMPARE(sortOrderModel->data(idx, Department::Roles::RoleIsActive), QVariant(true));
     }
+
+    void testDepartmentDissapear()
+    {
+        QCOMPARE(m_scope_flipflop->hasNavigation(), true);
+        QCOMPARE(m_scope_flipflop->hasAltNavigation(), false);
+        QCOMPARE(m_scope_flipflop->currentNavigationId(), QString(""));
+
+        QScopedPointer<NavigationInterface> departmentModel(m_scope_flipflop->getNavigation(m_scope_flipflop->currentNavigationId()));
+        QVERIFY(departmentModel != nullptr);
+
+        QVERIFY(departmentModel->navigationId().isEmpty());
+        QCOMPARE(departmentModel->label(), QString("All departments"));
+        QCOMPARE(departmentModel->allLabel(), QString(""));
+        QCOMPARE(departmentModel->parentNavigationId(), QString());
+        QCOMPARE(departmentModel->parentLabel(), QString());
+        QCOMPARE(departmentModel->loaded(), true);
+        QCOMPARE(departmentModel->isRoot(), true);
+        QCOMPARE(departmentModel->hidden(), false);
+
+        QCOMPARE(departmentModel->rowCount(), 5);
+
+        refreshSearch(m_scope_flipflop);
+
+        // one department removed
+        QCOMPARE(departmentModel->rowCount(), 4);
+
+        QModelIndex idx;
+
+        idx = departmentModel->index(0);
+        QCOMPARE(departmentModel->data(idx, Department::Roles::RoleNavigationId), QVariant(QString("books")));
+        QCOMPARE(departmentModel->data(idx, Department::Roles::RoleLabel), QVariant(QString("Books")));
+        QCOMPARE(departmentModel->data(idx, Department::Roles::RoleHasChildren), QVariant(true));
+        QCOMPARE(departmentModel->data(idx, Department::Roles::RoleIsActive), QVariant(false));
+
+        idx = departmentModel->index(3);
+        QCOMPARE(departmentModel->data(idx, Department::Roles::RoleNavigationId), QVariant(QString("toys")));
+        QCOMPARE(departmentModel->data(idx, Department::Roles::RoleLabel), QVariant(QString("Toys, Children & Baby")));
+        QCOMPARE(departmentModel->data(idx, Department::Roles::RoleHasChildren), QVariant(true));
+        QCOMPARE(departmentModel->data(idx, Department::Roles::RoleIsActive), QVariant(false));
+    }
+
 };
 
 QTEST_GUILESS_MAIN(DepartmentsTest)
 
=== modified file 'tests/test-utils.cpp'
--- tests/test-utils.cpp	2014-09-15 12:03:13 +0000
+++ tests/test-utils.cpp	2014-09-30 10:00:42 +0000
@@ -61,6 +61,20 @@
     return success;
 }
 
+void scopes_ng::refreshSearch(Scope* scope)
+{
+    QCOMPARE(scope->searchInProgress(), false);
+    QSignalSpy spy(scope, SIGNAL(searchInProgressChanged()));
+    // refresh the search
+    scope->refresh();
+    QVERIFY(scope->searchInProgress() || spy.count() > 1);
+    if (scope->searchInProgress()) {
+        // wait for the search to finish
+        QVERIFY(spy.wait());
+    }
+    QCOMPARE(scope->searchInProgress(), false);
+}
+
 void scopes_ng::performSearch(Scope* scope, QString const& searchString)
 {
     QCOMPARE(scope->searchInProgress(), false);
 
=== modified file 'tests/test-utils.h'
--- tests/test-utils.h	2014-09-15 12:03:13 +0000
+++ tests/test-utils.h	2014-09-30 10:00:42 +0000
@@ -37,6 +37,9 @@
 bool getFirstResult(Scope* scope, unity::scopes::Result::SPtr& result);
 
 Q_DECL_EXPORT
+void refreshSearch(scopes_ng::Scope*);
+
+Q_DECL_EXPORT
 void performSearch(Scope* scope, QString const& searchString);
 
 Q_DECL_EXPORT