Merge lp:~stolowski/unity-scope-home/canned-queries-session-id into lp:unity-scope-home

Proposed by Paweł Stołowski on 2013-09-19
Status: Merged
Approved by: Michal Hruby on 2013-09-19
Approved revision: 167
Merged at revision: 165
Proposed branch: lp:~stolowski/unity-scope-home/canned-queries-session-id
Merge into: lp:unity-scope-home
Diff against target: 129 lines (+50/-19)
3 files modified
src/scope.vala (+4/-1)
src/search-query-state.vala (+34/-18)
tests/unit/test-home-scope.vala (+12/-0)
To merge this branch: bzr merge lp:~stolowski/unity-scope-home/canned-queries-session-id
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve on 2013-09-19
Michal Hruby (community) 2013-09-19 Approve on 2013-09-19
Review via email: mp+186506@code.launchpad.net

Commit message

Reuse existing session_id for canned queries.

Description of the change

Added new CANNED_QUERY state to SearchQueryChange / SearchQueryState, so that we're able to reuse existing session_id for canned queries.
When a scopes-query:// item is activated, the activation handler sets a flag indicating that upcoming search query for current channel will be a canned query. Search request handler will then receive state == CANNED_QUERY when it calls query_state.search_query_changed.

To post a comment you must log in.
166. By Paweł Stołowski on 2013-09-19

Remove canned_query flag on remove().

Michal Hruby (mhr3) wrote :

What if the scope gets a query "foo", then a canned query is activated, but meanwhile the user changes something and instead of "keyword:foo" the scope gets "bar" as next query?

review: Needs Information
167. By Paweł Stołowski on 2013-09-19

Keep canned query string in the query state object rather than just bool, and only consider a query to be canned query if the search string matches current search request.

Paweł Stołowski (stolowski) wrote :

> What if the scope gets a query "foo", then a canned query is activated, but
> meanwhile the user changes something and instead of "keyword:foo" the scope
> gets "bar" as next query?

Good point, though rather unlikely. Anyway, update the code to deal with this.

Michal Hruby (mhr3) wrote :

Looks good now :)

review: Approve
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/scope.vala'
2--- src/scope.vala 2013-09-04 10:28:06 +0000
3+++ src/scope.vala 2013-09-19 13:59:12 +0000
4@@ -539,7 +539,10 @@
5 warning ("Invalid UTF8 received from the server");
6 }
7
8- return new ActivationResponse.with_search (escaped_query != null ? escaped_query : query, null, null);
9+ if (escaped_query != null)
10+ query = escaped_query;
11+ query_state.set_canned_query (activation.channel_id, query);
12+ return new ActivationResponse.with_search (query, null, null);
13 }
14
15 if (!scope_mgr.remote_content_search || sss_client == null ||
16
17=== modified file 'src/search-query-state.vala'
18--- src/search-query-state.vala 2013-03-21 21:40:00 +0000
19+++ src/search-query-state.vala 2013-09-19 13:59:12 +0000
20@@ -23,6 +23,7 @@
21 {
22 NOT_CHANGED,
23 NEW_QUERY,
24+ CANNED_QUERY,
25 APPENDS_TO_PREVIOUS_QUERY,
26 REMOVES_FROM_PREVIOUS_QUERY
27 }
28@@ -30,10 +31,12 @@
29 public class SearchQueryState
30 {
31 private HashTable<string, string> previous_query = new HashTable<string, string> (str_hash, str_equal);
32+ private HashTable<string, string> canned_query = new HashTable<string, string> (str_hash, str_equal);
33
34 public void remove (string home_channel_id)
35 {
36 previous_query.remove (home_channel_id);
37+ canned_query.remove (home_channel_id);
38 }
39
40 public bool has_channel (string home_channel_id)
41@@ -41,34 +44,47 @@
42 return previous_query.contains (home_channel_id);
43 }
44
45+ public void set_canned_query (string channel_id, string search_string)
46+ {
47+ canned_query[channel_id] = search_string;
48+ }
49+
50 /**
51 * Compare search string with previous string for given home channel. Store new search string.
52 */
53 public SearchQueryChange search_query_changed (string channel_id, string search_string)
54 {
55 var query = search_string.strip ();
56-
57 var changed = SearchQueryChange.NEW_QUERY;
58- if (previous_query.contains (channel_id))
59- {
60- var prev = previous_query[channel_id];
61- if (query == "" && prev != "") // there was a query previously, but user removed all characters in new one
62- {
63- changed = SearchQueryChange.NEW_QUERY;
64- }
65- else
66- {
67- if (prev == query)
68- changed = SearchQueryChange.NOT_CHANGED;
69- else if (prev == "")
70+
71+ if (canned_query.contains(channel_id) && canned_query[channel_id] == query)
72+ {
73+ changed = SearchQueryChange.CANNED_QUERY;
74+ }
75+ else
76+ {
77+ if (previous_query.contains (channel_id))
78+ {
79+ var prev = previous_query[channel_id];
80+ if (query == "" && prev != "") // there was a query previously, but user removed all characters in new one
81+ {
82 changed = SearchQueryChange.NEW_QUERY;
83- else if (query.has_prefix (prev))
84- changed = SearchQueryChange.APPENDS_TO_PREVIOUS_QUERY;
85- else if (prev.has_prefix (query))
86- changed = SearchQueryChange.REMOVES_FROM_PREVIOUS_QUERY;
87+ }
88+ else
89+ {
90+ if (prev == query)
91+ changed = SearchQueryChange.NOT_CHANGED;
92+ else if (prev == "")
93+ changed = SearchQueryChange.NEW_QUERY;
94+ else if (query.has_prefix (prev))
95+ changed = SearchQueryChange.APPENDS_TO_PREVIOUS_QUERY;
96+ else if (prev.has_prefix (query))
97+ changed = SearchQueryChange.REMOVES_FROM_PREVIOUS_QUERY;
98+ }
99 }
100 }
101-
102+
103+ canned_query.remove (channel_id);
104 previous_query[channel_id] = query;
105
106 debug ("search_query_changed for channel %s: %s", channel_id, changed.to_string ());
107
108=== modified file 'tests/unit/test-home-scope.vala'
109--- tests/unit/test-home-scope.vala 2013-09-04 10:28:06 +0000
110+++ tests/unit/test-home-scope.vala 2013-09-19 13:59:12 +0000
111@@ -230,6 +230,18 @@
112 assert (search_state.search_query_changed ("channel1", "te") == SearchQueryChange.REMOVES_FROM_PREVIOUS_QUERY);
113 assert (search_state.search_query_changed ("channel1", "") == SearchQueryChange.NEW_QUERY);
114 assert (search_state.has_channel ("channel1") == true);
115+
116+ // simulate canned queries
117+ assert (search_state.search_query_changed ("channel1", "ab") == SearchQueryChange.NEW_QUERY);
118+ search_state.set_canned_query("channel1", "foo:lo");
119+ assert (search_state.search_query_changed ("channel1", "foo:lo") == SearchQueryChange.CANNED_QUERY);
120+ assert (search_state.search_query_changed ("channel1", "foo:lo") == SearchQueryChange.NOT_CHANGED);
121+
122+ // simulate canned queries, user types a query immediately after activation
123+ assert (search_state.search_query_changed ("channel1", "me") == SearchQueryChange.NEW_QUERY);
124+ search_state.set_canned_query("channel1", "foo:metallica");
125+ assert (search_state.search_query_changed ("channel1", "iron maiden") == SearchQueryChange.NEW_QUERY);
126+ assert (search_state.search_query_changed ("channel1", "foo:metallica") == SearchQueryChange.NEW_QUERY); //it's no longer considered canned query
127 }
128
129 internal void test_category_manager_binary_search ()

Subscribers

People subscribed via source and target branches

to all changes: