Merge lp:~jonas-drange/ubuntu-system-settings/no-empty-contexts-1433278 into lp:ubuntu-system-settings

[Jonas G. Drange]

[cellular/apn] no creation of custom contexts on editor exit

What changed:
I moved creation of a new, custom internet context to when the user presses "Activate".

Why did I add apn.js:
I want to move the tons of JavaScript code out of the QML and into a nice, documented collection of helper code.

How to test this:
1. $ /usr/share/ofono/scripts/remove-contexts # removes your contexts. Backup your gprs* files if you want to keep them.
2. Open up the APN editor and press "Custom Internet APN"
3. Immediately press Cancel or even "<"
4. Confirm no new contexts were created.

* /var/lib/ofono/<IMSI>/gprs

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1359
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/2033/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/1998
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-vivid-i386-ci/303
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/1759
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/1996
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/1996/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19209

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/2033/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1359
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/2034/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2003
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-vivid-i386-ci/304
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/1764
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2001
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2001/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19221

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/2034/rebuild

review: Approve (continuous-integration)

[Ken VanDine (ken-vandine)]

A couple inline comments

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1360
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/2036/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2016
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-vivid-i386-ci/306
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/1779
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2014
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2014/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19246

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/2036/rebuild

review: Approve (continuous-integration)

[Ken VanDine (ken-vandine)]

Looks good and works as described

review: Approve

=== modified file 'plugins/cellular/CMakeLists.txt'
--- plugins/cellular/CMakeLists.txt	2015-01-14 15:57:10 +0000
+++ plugins/cellular/CMakeLists.txt	2015-03-27 15:01:16 +0000
@@ -4,6 +4,7 @@
 install(FILES settings-cellular.svg DESTINATION ${PLUGIN_MANIFEST_DIR}/icons)
 
 set(QML_SOURCES
+    apn.js
     carriers.js
     CustomApnEditor.qml
     PageChooseApn.qml

=== modified file 'plugins/cellular/CustomApnEditor.qml'
--- plugins/cellular/CustomApnEditor.qml	2015-02-16 14:02:22 +0000
+++ plugins/cellular/CustomApnEditor.qml	2015-03-27 15:01:16 +0000
@@ -23,6 +23,7 @@
 import SystemSettings 1.0
 import Ubuntu.Components 1.1
 import Ubuntu.Components.ListItems 1.0 as ListItem
+import "apn.js" as APN
 
 ItemPage {
     objectName: "customapnPage"
@@ -37,6 +38,9 @@
     /// work around LP(#1361919)
     property var activateCb;
 
+    // Sim qml object
+    property var sim
+
     QtObject {
         id: d
         property var typeText : type === "internet" ? i18n.tr("Internet") : i18n.tr("MMS")
@@ -83,6 +87,12 @@
             ctx = contexts["internet"]
         }
 
+        // We do not have any context yet, we will create one, but not now.
+        // We create it once the user presses 'Activate'.
+        if (!ctx) {
+            return;
+        }
+
         apnName.text = ctx.accessPointName;
         userName.text = ctx.username;
         pword.text = ctx.password;
@@ -277,6 +287,8 @@
 
                     onClicked: {
                         var ctx;
+                        var values;
+
                         if (d.isMms)
                             ctx = contexts["mms"];
                         else
@@ -300,23 +312,71 @@
                             pageStack.pop();
                             return;
                         }
-                        ctx.accessPointName = apnName.text;
-                        ctx.username = userName.text;
-                        ctx.password = pword.text;
+
+                        values = {
+                            'accessPointName': apnName.text,
+                            'username': userName.text,
+                            'password': pword.text
+                        };
+
                         if (d.isMms) {
-                            ctx.messageCenter = mmsc.text;
+                            values['messageCenter'] = mmsc.text;
                             var proxyValue = "";
                             if (proxy.text !== "") {
                                 proxyValue = proxy.text;
                                 if (port.text !== "")
                                     proxyValue = proxyValue + ":" + port.text;
                             }
-                            ctx.messageProxy = proxyValue
-                        }
-                        /// @todo map protocol values
-
-                        activateCb(ctx.type, ctx.contextPath);
-                        pageStack.pop();
+                            values['messageProxy'] = proxyValue;
+                        }
+
+                        // If we are editing an existing context, update
+                        // its values, activate and exit.
+                        if (ctx) {
+                            APN.updateContext(ctx, values);
+                            activateCb(ctx.type, ctx.contextPath);
+                            pageStack.pop();
+                        } else {
+                            // If we do not have a context, create one and defer
+                            // editing it to when it has been created—and we
+                            // will also wait until Ofono has given it a name.
+                            // This is all very async, so we will use connect()
+                            // and dynamic QML.
+
+                            function addedCustomContext (path) {
+                                // This is a handler for when we add a context
+                                // in the custom apn editor.
+                                // We create a temporary QML object for the
+                                // context like this, until we get the APN
+                                // editor refactored.
+                                // TODO(jgdx): create some kind of libqofono
+                                // objects framework in apn.js
+                                var newCtx = Qt.createQmlObject(
+                                    'import MeeGo.QOfono 0.2;'+
+                                    'OfonoContextConnection {'+
+                                    'contextPath: "'+path+'" }',
+                                    root, "apn.js");
+
+                                // Ofono sets a default name (Internet or MMS).
+                                // We are going to change it to our default name.
+                                newCtx.nameChanged.connect(function (name) {
+                                    if (name === "Internet") {
+                                        newCtx.name = APN.CUSTOM_INTERNET_CONTEXT_NAME();
+                                        APN.updateContext(newCtx, values);
+                                    } else if (name === "MMS") {
+                                        newCtx.name = APN.CUSTOM_MMS_CONTEXT_NAME();
+                                        APN.updateContext(newCtx, values);
+                                    } else {
+                                        newCtx.destroy(100);
+                                        activateCb(newCtx.type, newCtx.contextPath);
+                                        pageStack.pop();
+                                    }
+                                });
+                            }
+
+                            sim.connMan.addContext(root.type);
+                            sim.connMan.contextAdded.connect(addedCustomContext);
+                        }
                     }
                 }
             } // item for buttons

=== modified file 'plugins/cellular/PageChooseApn.qml'
--- plugins/cellular/PageChooseApn.qml	2015-03-27 15:01:16 +0000
+++ plugins/cellular/PageChooseApn.qml	2015-03-27 15:01:16 +0000
@@ -150,10 +150,7 @@
                 });
             }
 
-            if (customInternet.length === 0) {
-                sim.connMan.addContext("internet");
-                sim.connMan.contextAdded.connect(addedCustomContext);
-            } else {
+            if (customInternet.length !== 0) {
                 d.__haveCustomContexts = true;
             }
 
@@ -239,7 +236,8 @@
                                type: type,
                                contexts: {"internet": mCustomContextInternet,
                                           "mms":      mCustomContextMms},
-                               activateCb: activateHelper
+                               activateCb: activateHelper,
+                               sim: sim
                            });
         }
 
@@ -424,29 +422,7 @@
                 objectName: "customApnEdit"
                 text: i18n.tr("Custom Internet APN…")
                 width: parent.width - units.gu(4)
-                onClicked: {
-
-                    function contextAdded (path) {
-                        d.updateContexts();
-
-                        if (d.mContexts[path]) {
-                            d.mCustomContextInternet = d.mContexts[path];
-                        }
-                        d.openApnEditor("internet")
-                        d.contextsChanged.disconnect(contextAdded);
-                    }
-
-                    if (d.__haveCustomContexts) {
-                        d.openApnEditor("internet")
-                    } else {
-                        d.checkAndCreateCustomContexts();
-                        d.__haveCustomContexts = true;
-
-                        // We defer opening the editor until we have
-                        // added the custom context to mContexts
-                        sim.connMan.contextAdded.connect(contextAdded);
-                    }
-                }
+                onClicked: d.openApnEditor("internet")
             }
 
             ListItem.ItemSelector {

=== added file 'plugins/cellular/apn.js'
--- plugins/cellular/apn.js	1970-01-01 00:00:00 +0000
+++ plugins/cellular/apn.js	2015-03-27 15:01:16 +0000
@@ -0,0 +1,61 @@
+var _CUSTOM_INTERNET_CONTEXT_NAME = '___ubuntu_custom_apn_internet';
+var _CUSTOM_MMS_CONTEXT_NAME = '___ubuntu_custom_apn_mms';
+
+/*
+Updates a context with new values.
+
+@param {QOfonoConnectionContext} context
+@param {Object} values dict with new values
+*/
+function updateContext (context, values) {
+    var messageProxy;
+    console.warn('updateContext', values.accessPointName, values.messageCenter,
+        values.messageProxy, values.port, values.useraccessPointName,
+        values.password, values.type);
+    if (typeof values.accessPointName !== 'undefined') {
+        context.accessPointName = values.accessPointName;
+    }
+    if (typeof values.messageCenter !== 'undefined') {
+        context.messageCenter = values.messageCenter;
+    }
+    if (typeof values.messageProxy !== 'undefined') {
+        messageProxy = values.messageProxy;
+        if (messageProxy !== '') {
+             messageProxy = messageProxy + ':' + values.port;
+        }
+        context.messageProxy = messageProxy;
+    }
+    if (typeof values.username !== 'undefined') {
+        context.username = values.username;
+    }
+    if (typeof values.password !== 'undefined') {
+        context.password = values.password;
+    }
+    if (typeof values.type !== 'undefined') {
+        context.type = values.type;
+    }
+
+    if (context.type === 'internet') {
+        context.name = CUSTOM_INTERNET_CONTEXT_NAME();
+    } else if (context.type === 'mms') {
+        context.name = CUSTOM_MMS_CONTEXT_NAME();
+    }
+}
+
+/*
+Exposes the custom internet context name to the world.
+
+@return {String} custom internet context name
+*/
+function CUSTOM_INTERNET_CONTEXT_NAME () {
+    return _CUSTOM_INTERNET_CONTEXT_NAME;
+}
+
+/*
+Exposes the custom mms context name to the world.
+
+@return {String} custom mms context name
+*/
+function CUSTOM_MMS_CONTEXT_NAME () {
+    return _CUSTOM_MMS_CONTEXT_NAME;
+}