Merge lp:~knitzsche/scope-aggregator/bug-1602141-only-show-help-for-installed into lp:scope-aggregator

Proposed by Kyle Nitzsche
Status: Merged
Approved by: Kyle Nitzsche
Approved revision: 174
Merged at revision: 169
Proposed branch: lp:~knitzsche/scope-aggregator/bug-1602141-only-show-help-for-installed
Merge into: lp:scope-aggregator
Diff against target: 86 lines (+36/-4)
3 files modified
CMakeLists.txt (+1/-1)
data/hints.json (+1/-0)
src/utils.cpp (+34/-3)
To merge this branch: bzr merge lp:~knitzsche/scope-aggregator/bug-1602141-only-show-help-for-installed
Reviewer Review Type Date Requested Status
Gary.Wang Needs Fixing
Zhang Enwei (community) Approve
Penk Chen Pending
Review via email: mp+300480@code.launchpad.net

Description of the change

do not display quick start help result for any click child scope that is not installed.

To post a comment you must log in.
170. By Kyle Nitzsche

The previous commit handled half of the problem: suppressing irrelevant
quick start resuls by not displaying it when it relates to a click packaged
scope that is not installed.

This commit handles the another case, currently invoked for google.

That is, the Today scope quick start help has a result that
simply allows the user to open Settings > Accounts for Google provider.
This result is not connected to any specific child scope.
With this commit. you can declare in the hints.json file that a result
is not to be displayed in the supplied comma separated list of locales
in the form:
ll_CC (where ll is the lang code and CC is the country code).

For example, this hides the only item in the google-account category
when in two locales: es_ES and fr_FR:

(A more real-world example would be China: zh_CN)

"local":
{
    "content":
    {
        "categories":
        [
            {
                "id": "google-account",
                "_title": "See your events and more from Google.",
                "layout":
                {
                    [...]
                },
                "items": [
                    {
                        "hide_in_locales":"es_ES,fr_FR",
                        "_title": "<b> Review your Google sync settings</b>",
                        "_description": "Adding your Google ...",
                        "art":"google.png",
                        "action": {
                            "_name": "Add account",
                            "uri": "settings:///system/online-accounts"
                        }
                    }
                ]
            },

171. By Kyle Nitzsche

remove debug from log and set ver to 4.12

172. By Kyle Nitzsche

remove a couple qDebug() statemetns.

Revision history for this message
Zhang Enwei (zhangew401) wrote :

Hi Kyle,
For scopes that are using new online account settings, the service name is not the scope id. It is scopeid_providername and it is generated by online-account-hooks2.
Please refer to
http://bazaar.launchpad.net/~facebook-scope-team/facebook-scope/trunk/revision/144#src/query.cpp
https://bazaar.launchpad.net/~hanloon-team/hanloon/unity-scope-fitbit/revision/48#src/fitbit.go
https://developer.ubuntu.com/en/phone/platform/guides/online-accounts-developer-guide/(there are errors in this page)

173. By Kyle Nitzsche

This commit also supports making an online accounts client for child scopes
that have been been migrated to the new online accounts starting with
15.04.4. Such child scopes use a different id when creating the client for them.

before 15.04.4: id is pkg_app
starting with 15.04.4: id is pkg_ap_provider

This commit splits the ServiceName value (from hints.json)
on "_". The scope ID (needed to check if the scope is installed/registered
is the first two tokens (rejoined by "_"). If there is no third token, that
is used to create the online accounts client, thus working for child scopes
not yet migrated to 15.04.4 online accounts.

If there is a third token, it is the provider and it is appended to the
scope id and is used to create the online accounts client, thus supporting
updated child scopes.

Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

Note that I tested rev 170 change with:
* fbphotos scope 1.32 (which uses the new post 15.04.4 online accounts)
* photos-scope from here: https://code.launchpad.net/~knitzsche/photos-scope/mr-300480-verification

Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

Oops. previous comment should have said I tested rev *173* (not rev 170)

Revision history for this message
Zhang Enwei (zhangew401) wrote :

Look good to me!

review: Approve
Revision history for this message
Gary.Wang (gary-wzl77) wrote :

Hi Kyle
   Thanks for this MP.
   Please see my inline comment.

...
(A more real-world example would be China: zh_CN)
...
This made me LOL.:P

review: Needs Fixing
174. By Kyle Nitzsche

use QStringLiteral
version to 4.11
supress QT debug

Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

thanks Gary (for the QStringLiteral reminder) and Enwei (for your reviews). I'll merge this. Cheers

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2016-07-19 01:59:33 +0000
3+++ CMakeLists.txt 2016-07-26 15:15:42 +0000
4@@ -1,4 +1,4 @@
5-set(VERSION "4.10")
6+set(VERSION "4.11")
7
8 # Supress qDebug() output
9 ADD_DEFINITIONS( -DQT_NO_DEBUG_OUTPUT )
10
11=== modified file 'data/hints.json'
12--- data/hints.json 2016-06-23 19:37:49 +0000
13+++ data/hints.json 2016-07-26 15:15:42 +0000
14@@ -68,6 +68,7 @@
15 },
16 "items": [
17 {
18+ "hide_in_locales": "zh_CN",
19 "_title": "<b> Review your Google sync settings</b>",
20 "_description": "Adding your Google account will allow Today to display your events and contacts.<br><br>Press the button below to add your account (or go to accounts in the Settings app).<br><br>Remember to enable the contact and calendar sync once you have successfully added it.",
21 "art":"icons/today/google.png",
22
23=== modified file 'src/utils.cpp'
24--- src/utils.cpp 2016-07-19 01:59:33 +0000
25+++ src/utils.cpp 2016-07-26 15:15:42 +0000
26@@ -1231,6 +1231,18 @@
27 res["subtitle"] = _(sstr(res_[QStringLiteral("_subtitle")].toString()).c_str());
28 res["description"] = _(sstr(res_[QStringLiteral("_description")].toString()).c_str());
29 std::string uri = "http://www.ubuntu.com";
30+ if (res_.contains(QStringLiteral("hide_in_locales")))
31+ {
32+ QStringList hide_in_locales = res_[QStringLiteral("hide_in_locales")].toString().split(",");
33+ std::string lcRaw = setlocale(LC_ALL, "");
34+ std::string lc_lang = lcRaw.size() > 5 ? lcRaw.substr(0, 2) : "";
35+ std::string lc_country = lcRaw.size() > 5 ? lcRaw.substr(3, 2) : "";
36+ std::string lc = lc_lang + "_" + lc_country;
37+ if (hide_in_locales.contains(qstr(lc)))
38+ {
39+ continue;
40+ }
41+ }
42 if (res_.contains("uri"))
43 {
44 uri = sstr(res_["uri"].toString());
45@@ -1255,12 +1267,23 @@
46 auto oaccount = res_[QStringLiteral("oaccount")].toObject();
47 auto queryScope = sstr(oaccount[QStringLiteral("QueryScope")].toString());
48
49- us::OnlineAccountClient oa_client(sstr(oaccount[QStringLiteral("ServiceName")].toString()),
50+ // before framework 15.0.4.4 the service name is the FQ scope id and contains one "_"
51+ // with 15.04.4, it is FQ scope id += _PROVIDER
52+ // we also want the fully qualified scope id to check with registry if it is installed.
53+ // to support both cases:
54+ QStringList parts = oaccount[QStringLiteral("ServiceName")].toString().split("_");
55+ QString provider = oaccount[QStringLiteral("ProviderName")].toString();
56+ QString sc_id = parts[0] + "_" + parts[1];
57+ QString id = sc_id;
58+ if (parts.size() > 2) // framework 15.04.4 or higher
59+ {
60+ id += "_" + provider;
61+ }
62+ us::OnlineAccountClient oa_client(sstr(id),
63 sstr(oaccount[QStringLiteral("ServiceType")].toString()),
64- sstr(oaccount[QStringLiteral("ProviderName")].toString()));
65+ sstr(provider));
66 bool alreadyLoggedIn = false;
67 oa_client.refresh_service_statuses();
68- auto vectoraccounts = oa_client.get_service_statuses();
69 for (auto const& status : oa_client.get_service_statuses())
70 {
71 if (status.service_enabled)
72@@ -1269,6 +1292,14 @@
73 break;
74 }
75 }
76+ // do not display quick start help for scopes that are not registered in the system
77+ auto reg_scopes = registry_->list();
78+ us::MetadataMap::const_iterator scope_it;
79+ scope_it = reg_scopes.find(sstr(sc_id));
80+ if (scope_it == reg_scopes.end()) {
81+ qWarning () << QString("%1: OA scope is NOT REGISTERED, skipping for HINTS: %2").arg(oaccount[QStringLiteral("ServiceName")].toString());
82+ continue;
83+ }
84 if (alreadyLoggedIn) {
85 //the hints code does this when the person is already logged in to the service.
86 //auto tmp = oaccount["_loggedin"].toString().toStdString();

Subscribers

People subscribed via source and target branches