Merge lp:~mterry/unity8/set-real-name into lp:unity8

[Michael Terry]

Fix OOBE wizard not setting your name.

Fix how we set some standard properties of AccountsService (like Email and RealName).

AS does not let you set those properties directly. You must call SetEmail or SetRealName.

I've modified our DBus wrapper to notice such sets and use the right call. I've also updated our test suite to match and added a new test for SetRealName.

= To reproduce =

- Use tablet mode
- Go through the wizard, setting name

When done with wizard, your name should be above the greeter password field, instead of "Ubuntu"

= Checklist =

 * Are there any related MPs required for this MP to build/function as expected? Please list.
 No (well, except silo 41)

 * Did you perform an exploratory manual test run of your code change and any related functionality?
 Yes

 * Did you make sure that your branch does not contain spurious tags?
 Yes

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
 NA

 * If you changed the UI, has there been a design review?
 NA

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:2297
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/759/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/998
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1014
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1014
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1012/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/1012/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1012/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/1012/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1012/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/1012/console

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/759/rebuild

review: Needs Fixing (continuous-integration)

[lp:~mterry/unity8/set-real-name]

2279. By Launchpad Translations on behalf of unity-team on 2016-03-18

Launchpad automatic translations update.

2280. By Michael Zanetti on 2016-03-18

more fixes for the new theme palette Fixes: #1554616, #1557534
Approved by: Michael Zanetti

2281. By Lukáš Tinkl on 2016-03-18

Fix notifications with the new UITK color schemes Fixes: #1554616, #1556523
Approved by: Michael Terry

2282. By CI Train Bot Account on 2016-03-18

Releasing 8.11+16.04.20160318.1-0ubuntu1

2283. By CI Train Bot Account on 2016-03-18

Update translation template

[Albert Astals Cid (aacid)]

The diff looks weird, i guess launchpad got confused by the prereq? Maybe put this as Work in Progress until the prereq silo lands?

[Michael Terry (mterry)]

Or just look at commit 2297:
http://bazaar.launchpad.net/~mterry/unity8/set-real-name/revision/2297

[Albert Astals Cid (aacid)]

The code from commit 2297 looks good but i'm afraid you will need to rework this branch since even if i download the current lp:~ci-train-bot/unity8/unity8-ubuntu-xenial-landing-041 and then merge this branch in, i get conflicts, i guess because lp:~ci-train-bot/unity8/unity8-ubuntu-xenial-landing-041 has been changing with time.

So half-approving

review: Approve (code looks good, branch may need rework)

[Michael Terry (mterry)]

OK good point, I'll mark this WIP until we land one of the silos we're waiting on.

[lp:~mterry/unity8/set-real-name]

2284. By CI Train Bot Account on 2016-03-21

Resync trunk.

2285. By Launchpad Translations on behalf of unity-team on 2016-03-22

Launchpad automatic translations update.

2286. By Michael Terry on 2016-03-23

New first-run wizard for the phone (part of the OOBE effort) Fixes: #1536617, #1536688, #1543790, #1555222
Approved by: Michael Zanetti

2287. By Michael Zanetti on 2016-03-23

Sidestage load/unload redesign Fixes: #1544479
Approved by: Michael Terry

2288. By Michael Zanetti on 2016-03-23

Add support for low shell chrome. Fixes: #1535397
Approved by: Gerry Boland

2289. By Michael Zanetti on 2016-03-23

Redesign the first-boot edge tutorial
Approved by: Michael Zanetti

2290. By Michael Terry on 2016-03-23

Stop the left edge tutorial from being shown when the bottom edge tutorial wants to be shown after receiving a call.
Approved by: Albert Astals Cid

2291. By Lukáš Tinkl on 2016-03-23

Keymap switching support Fixes: #1412492, #1491340, #1524400
Approved by: Unity8 CI Bot, Michael Terry

2292. By Michael Zanetti on 2016-03-23

disabling some tests that are flaky in adt but can't be made fail on our machines
Approved by: Michael Terry

2293. By Michael Terry on 2016-03-23

Fix setting OSK layout in OOBE wizard
Approved by: Lukáš Tinkl

2294. By Albert Astals Cid on 2016-03-23

Implement the API changes due to filters
Approved by: Andrea Cimitan, Michael Terry

2295. By Albert Astals Cid on 2016-03-23

New Dash Navigation UX
Approved by: Andrea Cimitan

2296. By Albert Astals Cid on 2016-03-23

Filter implementation with Option Selector filter as the only filter
Approved by: Andrea Cimitan

2297. By Albert Astals Cid on 2016-03-23

Match design visuals for the new dash navigation/page header popup
Approved by: Andrea Cimitan

2298. By Albert Astals Cid on 2016-03-23

Range Input filter
Approved by: Andrea Cimitan

2299. By Albert Astals Cid on 2016-03-23

Value slider filter
Approved by: Andrea Cimitan

2300. By Albert Astals Cid on 2016-03-23

Workaround SDK ListItem bug

Approved by: Michael Terry

2301. By Albert Astals Cid on 2016-03-23

Fix various tests

test_navigationShowFilterButton is gone since it was testing a feature that no longer exists
Approved by: Michael Zanetti

2302. By Albert Astals Cid on 2016-03-23

Get the engine from the parent context

Approved by: Michael Zanetti

2303. By Michael Zanetti on 2016-03-23

Change the behavior alond with the switch in indicator-display Fixes: #1538591
Approved by: Michael Terry

2304. By Michael Terry on 2016-03-23

Session indicator convergence Fixes: #1538591
Approved by: Michael Terry

2305. By CI Train Bot Account on 2016-03-23

Releasing 8.12+16.04.20160323.2-0ubuntu1

2306. By CI Train Bot Account on 2016-03-23

Update translation template

2307. By Michael Terry on 2016-03-23

Fix setting some AS properties

2308. By Michael Terry on 2016-03-23

Add missing file

2309. By Michael Terry on 2016-03-24

Fix integrated test

Unmerged revisions

=== modified file 'plugins/AccountsService/AccountsServiceDBusAdaptor.cpp'
--- plugins/AccountsService/AccountsServiceDBusAdaptor.cpp	2016-02-16 14:12:58 +0000
+++ plugins/AccountsService/AccountsServiceDBusAdaptor.cpp	2016-03-23 20:48:16 +0000
@@ -57,8 +57,18 @@
 {
     QDBusInterface *iface = getUserInterface(user);
     if (iface != nullptr && iface->isValid()) {
-        // The value needs to be carefully wrapped
-        return iface->asyncCall(QStringLiteral("Set"), interface, property, QVariant::fromValue(QDBusVariant(value)));
+        if (interface == QStringLiteral("org.freedesktop.Accounts.User")) {
+            // Standard AccountsService properties use special set methods.
+            // It will not let you use the usual DBus property setters.
+            QDBusInterface accountsIface(iface->service(),
+                                         iface->path(),
+                                         interface,
+                                         iface->connection());
+            return accountsIface.asyncCall(QStringLiteral("Set") + property, value);
+        } else {
+            // The value needs to be carefully wrapped
+            return iface->asyncCall(QStringLiteral("Set"), interface, property, QVariant::fromValue(QDBusVariant(value)));
+        }
     }
     return QDBusPendingCall::fromCompletedCall(QDBusMessage::createError(QDBusError::Other, QStringLiteral("Invalid Interface")));
 }

=== modified file 'tests/plugins/AccountsService/CMakeLists.txt'
--- tests/plugins/AccountsService/CMakeLists.txt	2016-02-25 10:57:17 +0000
+++ tests/plugins/AccountsService/CMakeLists.txt	2016-03-23 20:48:16 +0000
@@ -5,7 +5,7 @@
 endif()
 macro(make_dbus_class NAME INTERFACE)
     if(${CMAKE_CURRENT_SOURCE_DIR}/interfaces.xml IS_NEWER_THAN ${CMAKE_CURRENT_BINARY_DIR}/${NAME}Adaptor.h)
-        execute_process(COMMAND ${QDBUSXML2CPP_EXECUTABLE} -c ${NAME}Adaptor -a ${CMAKE_CURRENT_BINARY_DIR}/${NAME}Adaptor ${CMAKE_CURRENT_SOURCE_DIR}/interfaces.xml ${INTERFACE})
+        execute_process(COMMAND ${QDBUSXML2CPP_EXECUTABLE} -c ${NAME}Adaptor -i types.h -a ${CMAKE_CURRENT_BINARY_DIR}/${NAME}Adaptor ${CMAKE_CURRENT_SOURCE_DIR}/interfaces.xml ${INTERFACE})
     endif()
 endmacro(make_dbus_class)
 
@@ -20,6 +20,7 @@
 
 include_directories(
     ${CMAKE_CURRENT_BINARY_DIR}
+    ${CMAKE_CURRENT_SOURCE_DIR}
     ${CMAKE_SOURCE_DIR}/plugins/AccountsService
     )
 

=== modified file 'tests/plugins/AccountsService/PropertiesServer.cpp'
--- tests/plugins/AccountsService/PropertiesServer.cpp	2016-03-23 09:50:10 +0000
+++ tests/plugins/AccountsService/PropertiesServer.cpp	2016-03-23 20:48:16 +0000
@@ -25,9 +25,7 @@
 #include <QDBusMessage>
 #include <QDBusMetaType>
 
-using StringMap = QMap<QString,QString>;
-using StringMapList = QList<StringMap>;
-Q_DECLARE_METATYPE(StringMapList)
+#define IFACE_ACCOUNTS_USER QStringLiteral("org.freedesktop.Accounts.User")
 
 PropertiesServer::PropertiesServer(QObject *parent)
     : QObject(parent)
@@ -62,6 +60,38 @@
 {
     QVariant newValue = variant.variant();
 
+    if (interface == IFACE_ACCOUNTS_USER) {
+        sendErrorReply(QDBusError::InvalidArgs, QStringLiteral("Property is not writable"));
+        return;
+    }
+
+    internalSet(interface, property, newValue);
+}
+
+void PropertiesServer::SetBackgroundFile(const QString &backgroundFile)
+{
+    internalSet(IFACE_ACCOUNTS_USER, QStringLiteral("BackgroundFile"), backgroundFile);
+}
+
+void PropertiesServer::SetEmail(const QString &email)
+{
+    internalSet(IFACE_ACCOUNTS_USER, QStringLiteral("Email"), email);
+}
+
+void PropertiesServer::SetInputSources(const StringMapList &inputSources)
+{
+    internalSet(IFACE_ACCOUNTS_USER, QStringLiteral("InputSources"), QVariant::fromValue(inputSources));
+}
+
+void PropertiesServer::SetRealName(const QString &realName)
+{
+    internalSet(IFACE_ACCOUNTS_USER, QStringLiteral("RealName"), realName);
+}
+
+void PropertiesServer::internalSet(const QString &interface, const QString &property, const QVariant &variant)
+{
+    QVariant newValue = variant;
+
     if (m_properties[interface].contains(property)) {
         QVariant& oldValue = m_properties[interface][property];
         if (oldValue != newValue) {
@@ -69,14 +99,12 @@
             if (interface == QStringLiteral("com.canonical.unity.AccountsService") &&
                     property == QStringLiteral("LauncherItems")) {
                 newValue = QVariant::fromValue(qdbus_cast<QList<QVariantMap>>(newValue.value<QDBusArgument>()));
-            } else if (interface == "org.freedesktop.Accounts.User" && property == "InputSources") {
-                newValue = QVariant::fromValue(qdbus_cast<StringMapList>(newValue.value<QDBusArgument>()));
             }
 
             oldValue = newValue;
 
             // Special case for user properties.
-            if (interface == "org.freedesktop.Accounts.User") {
+            if (interface == IFACE_ACCOUNTS_USER) {
                 Q_EMIT Changed();
             } else {
                 QVariantMap propertyChanges;
@@ -108,6 +136,7 @@
     m_properties["com.ubuntu.location.providers.here.AccountsService"]["LicenseAccepted"] = false;
     m_properties["com.ubuntu.location.providers.here.AccountsService"]["LicenseBasePath"] = "";
     m_properties["org.freedesktop.Accounts.User"]["BackgroundFile"] = "";
+    m_properties["org.freedesktop.Accounts.User"]["Email"] = "";
+    m_properties["org.freedesktop.Accounts.User"]["InputSources"] = QVariant::fromValue(StringMapList());
     m_properties["org.freedesktop.Accounts.User"]["RealName"] = "";
-    m_properties["org.freedesktop.Accounts.User"]["InputSources"] = QVariant::fromValue(StringMapList());
 }

=== modified file 'tests/plugins/AccountsService/PropertiesServer.h'
--- tests/plugins/AccountsService/PropertiesServer.h	2016-03-11 12:54:58 +0000
+++ tests/plugins/AccountsService/PropertiesServer.h	2016-03-23 20:48:16 +0000
@@ -24,6 +24,7 @@
 #include <QDBusVariant>
 #include <QObject>
 #include <QString>
+#include "types.h"
 
 class PropertiesServer: public QObject, protected QDBusContext
 {
@@ -36,6 +37,10 @@
     QDBusVariant Get(const QString &interface, const QString &property) const;
     QVariantMap GetAll(const QString &interface) const;
     void Set(const QString &interface, const QString &property, const QDBusVariant &variant);
+    void SetBackgroundFile(const QString &backgroundFile);
+    void SetEmail(const QString &email);
+    void SetInputSources(const StringMapList &inputSources);
+    void SetRealName(const QString &realName);
 
     // mock only.
     void Reset();
@@ -45,6 +50,8 @@
     void Changed();
 
 private:
+    void internalSet(const QString &interface, const QString &property, const QVariant &variant);
+
     QHash<QString, QVariantMap> m_properties;
 };
 

=== modified file 'tests/plugins/AccountsService/client.cpp'
--- tests/plugins/AccountsService/client.cpp	2016-03-23 09:50:10 +0000
+++ tests/plugins/AccountsService/client.cpp	2016-03-23 20:48:16 +0000
@@ -19,16 +19,13 @@
 
 #include "AccountsService.h"
 #include "AccountsServiceDBusAdaptor.h"
+#include "types.h"
 #include <QSignalSpy>
 #include <QTest>
 #include <QDebug>
 #include <QDBusReply>
 #include <QDBusMetaType>
 
-using StringMap = QMap<QString,QString>;
-using StringMapList = QList<StringMap>;
-Q_DECLARE_METATYPE(StringMapList)
-
 template <class T>
 QVariant dbusVariant(const T& value) { return QVariant::fromValue(QDBusVariant(value)); }
 
@@ -263,10 +260,13 @@
         AccountsService session(this, QTest::currentTestFunction());
 
         QCOMPARE(session.backgroundFile(), QString());
-        ASSERT_DBUS_CALL(m_userInterface->asyncCall("Set",
-                                                   "org.freedesktop.Accounts.User",
-                                                    "BackgroundFile",
-                                                    dbusVariant("/test/BackgroundFile")));
+
+        QDBusInterface accountsIface(m_userInterface->service(),
+                                     m_userInterface->path(),
+                                     "org.freedesktop.Accounts.User");
+        ASSERT_DBUS_CALL(accountsIface.asyncCall("SetBackgroundFile",
+                                                 "/test/BackgroundFile"));
+
         QTRY_COMPARE(session.backgroundFile(), QString("/test/BackgroundFile"));
     }
 
@@ -311,6 +311,39 @@
         QCOMPARE(arguments.at(0).toInt(), 0);
     }
 
+    void testSetRealName()
+    {
+        AccountsService session(this, QTest::currentTestFunction());
+
+        // Just confirm that we can't set normal way
+        auto message = m_userInterface->call("Set",
+                                             "org.freedesktop.Accounts.User",
+                                             "RealName",
+                                             dbusVariant("Stallman"));
+        QCOMPARE(message.type(), QDBusMessage::ErrorMessage);
+        QCOMPARE(message.errorName(), QStringLiteral("org.freedesktop.DBus.Error.InvalidArgs"));
+
+        message = m_userInterface->call("Get",
+                                        "org.freedesktop.Accounts.User",
+                                        "RealName");
+        QCOMPARE(message.type(), QDBusMessage::ReplyMessage);
+        auto response = message.arguments().first().value<QDBusVariant>().variant().toString();
+        QCOMPARE(response, QStringLiteral(""));
+
+        // Now try it via our AS wrapper and confirm it did get through, unlike above
+        session.setRealName("Stallman");
+
+        QCOMPARE(session.realName(), QStringLiteral("Stallman"));
+
+        message = m_userInterface->call("Get",
+                                        "org.freedesktop.Accounts.User",
+                                        "RealName");
+        QCOMPARE(message.type(), QDBusMessage::ReplyMessage);
+        response = message.arguments().first().value<QDBusVariant>().variant().toString();
+        QCOMPARE(response, QStringLiteral("Stallman"));
+
+    }
+
     void testAsynchronousChangeForKeymaps()
     {
         AccountsService session(this, QTest::currentTestFunction());
@@ -325,10 +358,11 @@
         map2.insert("xkb", "fr");
         inputSources.append(map2);
 
-        ASSERT_DBUS_CALL(m_userInterface->asyncCall("Set",
-                                                    "org.freedesktop.Accounts.User",
-                                                    "InputSources",
-                                                    QVariant::fromValue(QDBusVariant(QVariant::fromValue(inputSources)))));
+        QDBusInterface accountsIface(m_userInterface->service(),
+                                     m_userInterface->path(),
+                                     "org.freedesktop.Accounts.User");
+        ASSERT_DBUS_CALL(accountsIface.asyncCall("SetInputSources",
+                                                 QVariant::fromValue(inputSources)));
         QStringList result = {"cz+qwerty", "fr"};
         QTRY_COMPARE(session.keymaps(), result);
     }

=== modified file 'tests/plugins/AccountsService/interfaces.xml'
--- tests/plugins/AccountsService/interfaces.xml	2016-02-25 10:57:17 +0000
+++ tests/plugins/AccountsService/interfaces.xml	2016-03-23 20:48:16 +0000
@@ -42,6 +42,19 @@
   <interface name="com.ubuntu.location.providers.here.AccountsService" />
   <interface name="org.freedesktop.Accounts.User">
     <signal name="Changed" />
+    <method name="SetBackgroundFile">
+      <arg name="backgroundFile" type="s" direction="in" />
+    </method>
+    <method name="SetEmail">
+      <arg name="email" type="s" direction="in" />
+    </method>
+    <method name="SetInputSources">
+      <annotation name="org.qtproject.QtDBus.QtTypeName.In0" value="StringMapList"/>
+      <arg name="inputSources" type="aa{ss}" direction="in" />
+    </method>
+    <method name="SetRealName">
+      <arg name="realName" type="s" direction="in" />
+    </method>
   </interface>
   <interface name="com.canonical.unity.AccountsService.Private" />