Merge lp:~larryprice/libertine-scope/filter-state into lp:libertine-scope

Proposed by Larry Price
Status: Merged
Approved by: Christopher Townsend
Approved revision: 55
Merged at revision: 61
Proposed branch: lp:~larryprice/libertine-scope/filter-state
Merge into: lp:libertine-scope
Diff against target: 350 lines (+179/-43)
8 files modified
scope/apps/action.cpp (+26/-15)
scope/apps/action.h (+21/-15)
scope/apps/scope.cpp (+14/-1)
scope/apps/scope.h (+1/-0)
tests/CMakeLists.txt (+2/-0)
tests/mock_hidden_apps.h (+32/-0)
tests/test_action.cpp (+82/-0)
tests/test_query.cpp (+1/-12)
To merge this branch: bzr merge lp:~larryprice/libertine-scope/filter-state
Reviewer Review Type Date Requested Status
Christopher Townsend Approve
Libertine CI Bot continuous-integration Approve
Review via email: mp+301426@code.launchpad.net

Commit message

Showing/hiding applications should retain filter state.

Description of the change

Showing/hiding applications should retain filter state.

I added two items to the ctor of Action: filter_state gives the last known state of the scope filters and allows us to set the filters after a show/hide, and open_action is a lambda which tells Action what to do when "open" is called. Since we use url-dispatcher, this was the quickest way I came up with to mock this function call so I could set up some proper unit tests.

To post a comment you must log in.
Revision history for this message
Libertine CI Bot (libertine-ci-bot) wrote :

PASSED: Continuous integration, rev:54
https://jenkins.canonical.com/libertine/job/lp-libertine-scope-ci/56/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/libertine/job/build/214
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=default/174
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=default/174
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=default/174
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=i386,release=vivid+overlay,testname=default/174
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=i386,release=xenial+overlay,testname=default/174
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=i386,release=yakkety,testname=default/174
    None: https://jenkins.canonical.com/libertine/job/lp-generic-update-mp/163/console
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-0-fetch/217
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-1-sourcepkg/release=vivid+overlay/202
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-1-sourcepkg/release=xenial+overlay/202
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-1-sourcepkg/release=yakkety/202
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=vivid+overlay/195
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=vivid+overlay/195/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=xenial+overlay/195
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=xenial+overlay/195/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=yakkety/195
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=yakkety/195/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=vivid+overlay/195
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=vivid+overlay/195/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=xenial+overlay/195
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=xenial+overlay/195/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=yakkety/195
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=yakkety/195/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/libertine/job/lp-libertine-scope-ci/56/rebuild

review: Approve (continuous-integration)
55. By Larry Price

Merge

Revision history for this message
Libertine CI Bot (libertine-ci-bot) wrote :

PASSED: Continuous integration, rev:55
https://jenkins.canonical.com/libertine/job/lp-libertine-scope-ci/57/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/libertine/job/build/217
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=default/177
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=default/177
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=default/177
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=i386,release=vivid+overlay,testname=default/177
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=i386,release=xenial+overlay,testname=default/177
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=i386,release=yakkety,testname=default/177
    None: https://jenkins.canonical.com/libertine/job/lp-generic-update-mp/164/console
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-0-fetch/220
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-1-sourcepkg/release=vivid+overlay/205
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-1-sourcepkg/release=xenial+overlay/205
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-1-sourcepkg/release=yakkety/205
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=vivid+overlay/198
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=vivid+overlay/198/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=xenial+overlay/198
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=xenial+overlay/198/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=yakkety/198
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=yakkety/198/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=vivid+overlay/198
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=vivid+overlay/198/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=xenial+overlay/198
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=xenial+overlay/198/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=yakkety/198
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=yakkety/198/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/libertine/job/lp-libertine-scope-ci/57/rebuild

review: Approve (continuous-integration)
Revision history for this message
Christopher Townsend (townsend) wrote :

Good, thanks

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'scope/apps/action.cpp'
2--- scope/apps/action.cpp 2016-07-28 15:46:36 +0000
3+++ scope/apps/action.cpp 2016-07-29 13:21:50 +0000
4@@ -29,14 +29,30 @@
5 namespace usc = unity::scopes;
6
7
8+namespace
9+{
10+static usc::ActivationResponse
11+sendToResults(usc::FilterState const& filter_state)
12+{
13+ usc::CannedQuery query(FULLY_QUALIFIED_APPS_SCOPE);
14+ query.set_filter_state(filter_state);
15+ return usc::ActivationResponse(query);
16+}
17+}
18+
19+
20 Action::
21-Action(usc::Result const& result,
22- usc::ActionMetadata const& metadata,
23- std::string const& action_id,
24- std::shared_ptr<HiddenApps> hidden)
25- : usc::ActivationQueryBase(result, metadata),
26- action_id_(action_id),
27- hidden_(hidden)
28+Action(unity::scopes::Result const& result,
29+ unity::scopes::ActionMetadata const& metadata,
30+ std::string const& action_id,
31+ Action::OpenUriAction open_action,
32+ std::shared_ptr<HiddenApps> hidden,
33+ unity::scopes::FilterState const& filter_state)
34+ : usc::ActivationQueryBase(result, metadata)
35+ , action_id_(action_id)
36+ , open_action_(open_action)
37+ , hidden_(hidden)
38+ , filter_state_(filter_state)
39 {
40 }
41
42@@ -46,22 +62,17 @@
43 {
44 if (action_id_ == "open")
45 {
46- url_dispatch_send(result().uri().c_str() , NULL, NULL);
47- return usc::ActivationResponse(usc::ActivationResponse::Status::NotHandled);
48+ open_action_(result().uri());
49 }
50 else if (action_id_ == "hide")
51 {
52 hidden_->add(QString::fromStdString(result()["app_id"].get_string()));
53-
54- usc::CannedQuery cq(FULLY_QUALIFIED_APPS_SCOPE);
55- return usc::ActivationResponse(cq);
56+ return sendToResults(filter_state_);
57 }
58 else if (action_id_ == "show")
59 {
60 hidden_->remove(QString::fromStdString(result()["app_id"].get_string()));
61-
62- usc::CannedQuery cq(FULLY_QUALIFIED_APPS_SCOPE);
63- return usc::ActivationResponse(cq);
64+ return sendToResults(filter_state_);
65 }
66 return usc::ActivationResponse(usc::ActivationResponse::Status::NotHandled);
67 }
68
69=== modified file 'scope/apps/action.h'
70--- scope/apps/action.h 2016-07-12 18:26:07 +0000
71+++ scope/apps/action.h 2016-07-29 13:21:50 +0000
72@@ -15,31 +15,37 @@
73 * * Authored by:
74 * Kyle Nitzsche <kyle.nitzsche@canonical.com>
75 */
76-
77 #ifndef SCOPE_ACTION_H_
78 #define SCOPE_ACTION_H_
79
80-#include "scope/apps/scope.h"
81+
82 #include <unity/scopes/ActionMetadata.h>
83 #include <unity/scopes/ActivationQueryBase.h>
84 #include <unity/scopes/ActivationResponse.h>
85 #include <unity/scopes/Result.h>
86
87+
88 class HiddenApps;
89
90 class Action : public unity::scopes::ActivationQueryBase {
91- public:
92- Action(unity::scopes::Result const& result,
93- unity::scopes::ActionMetadata const& metadata,
94- std::string const& action_id,
95- std::shared_ptr<HiddenApps> hidden);
96-
97- virtual ~Action() = default;
98- virtual unity::scopes::ActivationResponse activate() override;
99-
100- private:
101- std::string action_id_;
102- std::string cache_dir_;
103- std::shared_ptr<HiddenApps> hidden_;
104+public:
105+ typedef std::function<void(std::string const&)> OpenUriAction;
106+
107+ explicit Action(unity::scopes::Result const& result,
108+ unity::scopes::ActionMetadata const& metadata,
109+ std::string const& action_id,
110+ OpenUriAction open_action,
111+ std::shared_ptr<HiddenApps> hidden,
112+ unity::scopes::FilterState const& filterState);
113+
114+ virtual ~Action() = default;
115+ virtual unity::scopes::ActivationResponse activate() override;
116+
117+private:
118+ std::string action_id_;
119+ std::string cache_dir_;
120+ OpenUriAction open_action_;
121+ std::shared_ptr<HiddenApps> hidden_;
122+ unity::scopes::FilterState filter_state_;
123 };
124 #endif
125
126=== modified file 'scope/apps/scope.cpp'
127--- scope/apps/scope.cpp 2016-07-28 15:47:35 +0000
128+++ scope/apps/scope.cpp 2016-07-29 13:21:50 +0000
129@@ -29,6 +29,16 @@
130 namespace usc = unity::scopes;
131
132
133+namespace
134+{
135+static void
136+open_application(std::string const& app_uri)
137+{
138+ url_dispatch_send(app_uri.c_str(), NULL, NULL);
139+}
140+}
141+
142+
143 Scope::
144 Scope(Libertine::Factory const& libertine_factory)
145 : libertine_factory_(libertine_factory)
146@@ -56,6 +66,7 @@
147 search(usc::CannedQuery const& query,
148 usc::SearchMetadata const& metadata)
149 {
150+ filter_state_ = query.filter_state();
151 return usc::SearchQueryBase::UPtr(new Query(query,
152 metadata,
153 libertine_factory_,
154@@ -81,7 +92,9 @@
155 return usc::ActivationQueryBase::UPtr(new Action(result,
156 metadata,
157 action_id,
158- std::make_shared<HiddenApps>(cache_directory())));
159+ open_application,
160+ std::make_shared<HiddenApps>(cache_directory()),
161+ filter_state_));
162 }
163
164
165
166=== modified file 'scope/apps/scope.h'
167--- scope/apps/scope.h 2016-07-12 18:26:07 +0000
168+++ scope/apps/scope.h 2016-07-29 13:21:50 +0000
169@@ -78,6 +78,7 @@
170
171 private:
172 Libertine::Factory libertine_factory_;
173+ unity::scopes::FilterState filter_state_;
174 };
175
176 #endif // LIBERTINE_SCOPE_SCOPE_H_
177
178=== modified file 'tests/CMakeLists.txt'
179--- tests/CMakeLists.txt 2016-07-12 18:26:07 +0000
180+++ tests/CMakeLists.txt 2016-07-29 13:21:50 +0000
181@@ -7,6 +7,7 @@
182 add_executable(${test_name}_exe
183 fake_container.cpp
184 fake_libertine.cpp
185+ mock_hidden_apps.h
186 ${test_name}.cpp
187 )
188
189@@ -25,5 +26,6 @@
190 create_test(test_query)
191 create_test(test_hidden_apps)
192 create_test(test_blacklist)
193+create_test(test_action)
194
195 file(COPY data DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
196
197=== added file 'tests/mock_hidden_apps.h'
198--- tests/mock_hidden_apps.h 1970-01-01 00:00:00 +0000
199+++ tests/mock_hidden_apps.h 2016-07-29 13:21:50 +0000
200@@ -0,0 +1,32 @@
201+/*
202+ * Copyright 2016 Canonical Ltd.
203+ *
204+ * This program is free software: you can redistribute it and/or modify it under
205+ * the terms of the GNU General Public License, version 3, as published by the
206+ * Free Software Foundation.
207+ *
208+ * This program is distributed in the hope that it will be useful,
209+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
210+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
211+ * GNU General Public License for more details.
212+ *
213+ * You should have received a copy of the GNU General Public License
214+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
215+ */
216+#include "scope/apps/hidden_apps.h"
217+#include <gmock/gmock.h>
218+
219+
220+class MockHiddenApps : public HiddenApps
221+{
222+public:
223+ MockHiddenApps()
224+ : HiddenApps("")
225+ {
226+ }
227+
228+ MOCK_CONST_METHOD1(app_is_hidden, bool(QString const&));
229+ MOCK_CONST_METHOD0(empty, bool());
230+ MOCK_METHOD1(remove, void(QString const&));
231+ MOCK_METHOD1(add, void(QString const&));
232+};
233
234=== added file 'tests/test_action.cpp'
235--- tests/test_action.cpp 1970-01-01 00:00:00 +0000
236+++ tests/test_action.cpp 2016-07-29 13:21:50 +0000
237@@ -0,0 +1,82 @@
238+/*
239+ * Copyright 2016 Canonical Ltd.
240+ *
241+ * This program is free software: you can redistribute it and/or modify it under
242+ * the terms of the GNU General Public License, version 3, as published by the
243+ * Free Software Foundation.
244+ *
245+ * This program is distributed in the hope that it will be useful,
246+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
247+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
248+ * GNU General Public License for more details.
249+ *
250+ * You should have received a copy of the GNU General Public License
251+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
252+ */
253+#include "scope/apps/action.h"
254+#include "tests/mock_hidden_apps.h"
255+
256+#include <unity/scopes/ActionMetadata.h>
257+#include <unity/scopes/testing/Result.h>
258+#include <gmock/gmock.h>
259+
260+
261+namespace
262+{
263+TEST(ActionTest, OpenSendsDispatchURI)
264+{
265+ unity::scopes::testing::Result result;
266+ result.set_uri("app://this/is/my/app");
267+ unity::scopes::ActionMetadata metadata("en_US", "phone");
268+ auto hidden = std::make_shared<testing::NiceMock<MockHiddenApps> >();
269+ unity::scopes::FilterState filter_state;
270+
271+ std::string uri;
272+
273+ Action action(result, metadata, "open", [&](std::string const& app_uri) {
274+ uri = app_uri;
275+ }, hidden, filter_state);
276+
277+ auto response = action.activate();
278+ EXPECT_EQ(unity::scopes::ActivationResponse::Status::NotHandled, response.status());
279+ EXPECT_EQ(result.uri(), uri);
280+}
281+
282+
283+TEST(ActionTest, ShowRemovesAppFromHidden)
284+{
285+ unity::scopes::testing::Result result;
286+ unity::scopes::ActionMetadata metadata("en_US", "phone");
287+ unity::scopes::FilterState filter_state;
288+ auto hidden = std::make_shared<testing::NiceMock<MockHiddenApps> >();
289+
290+ QString app_id = "something.desktop";
291+ result["app_id"] = app_id.toStdString();
292+ EXPECT_CALL(*hidden, remove(app_id));
293+
294+ Action action(result, metadata, "show", Action::OpenUriAction{}, hidden, filter_state);
295+
296+ auto response = action.activate();
297+ EXPECT_EQ(filter_state.serialize(), response.query().filter_state().serialize());
298+ EXPECT_EQ(unity::scopes::ActivationResponse::Status::PerformQuery, response.status());
299+}
300+
301+
302+TEST(ActionTest, HideAddsAppToHidden)
303+{
304+ unity::scopes::testing::Result result;
305+ unity::scopes::ActionMetadata metadata("en_US", "phone");
306+ unity::scopes::FilterState filter_state;
307+ auto hidden = std::make_shared<testing::NiceMock<MockHiddenApps> >();
308+
309+ QString app_id = "something.desktop";
310+ result["app_id"] = app_id.toStdString();
311+ EXPECT_CALL(*hidden, add(app_id));
312+
313+ Action action(result, metadata, "hide", Action::OpenUriAction{}, hidden, filter_state);
314+
315+ auto response = action.activate();
316+ EXPECT_EQ(filter_state.serialize(), response.query().filter_state().serialize());
317+ EXPECT_EQ(unity::scopes::ActivationResponse::Status::PerformQuery, response.status());
318+}
319+}
320
321=== modified file 'tests/test_query.cpp'
322--- tests/test_query.cpp 2016-07-12 18:26:07 +0000
323+++ tests/test_query.cpp 2016-07-29 13:21:50 +0000
324@@ -17,6 +17,7 @@
325 #include "scope/apps/query.h"
326 #include "scope/apps/config.h"
327 #include "tests/fake_libertine.h"
328+#include "tests/mock_hidden_apps.h"
329 #include <unity/scopes/SearchMetadata.h>
330 #include <unity/scopes/CannedQuery.h>
331 #include <unity/scopes/SearchReplyProxyFwd.h>
332@@ -67,18 +68,6 @@
333 };
334
335
336-class MockHiddenApps : public HiddenApps
337-{
338-public:
339- MockHiddenApps()
340- : HiddenApps("")
341- {
342- }
343-
344- MOCK_CONST_METHOD1(app_is_hidden, bool(QString const&));
345- MOCK_CONST_METHOD0(empty, bool());
346-};
347-
348 class MockBlacklist : public Blacklist
349 {
350 public:

Subscribers

People subscribed via source and target branches