Merge lp:~jonas-drange/ubuntu-system-settings/apn-no-overwrite-1415495 into lp:ubuntu-system-settings

[Jonas G. Drange]

[cellular] apn page does not overwrite empty, provisioned contexts

What changed:
The current APN page made the assumption that an empty internet context was really a recently added custom context. I have made this logic more precise by using a temporary event handler on ConnectionManager's contextAdded.

How to test:

If you have special a special APN configuration, please back up the gprs file.

1. Stop ofono (sudo service ofono stop)
2. Edit /var/lib/ofono/<IMSI>/gprs, let it look like this [1].
3. Start ofono (sudo service ofono start)
4. Wait 5 secs. Confirm that the output of /usr/share/ofono/scripts/list-contexts is what you'd expect.
5. Enter the APN page
6. Add a custom context and confirm it is not overwriting anything.

[1] https://launchpadlibrarian.net/196032938/gprs

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1358
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/2030/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/1967
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-vivid-i386-ci/300
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/1734
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/1965
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/1965/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19150

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

review: Approve (continuous-integration)

[Ken VanDine (ken-vandine)]

Looks good and works as described.

review: Approve

=== modified file 'plugins/cellular/PageChooseApn.qml'
--- plugins/cellular/PageChooseApn.qml	2015-02-16 14:02:22 +0000
+++ plugins/cellular/PageChooseApn.qml	2015-03-25 15:10:25 +0000
@@ -52,31 +52,6 @@
         property bool __suppressActivation : true;
         property bool __haveCustomContexts : false;
 
-        function isEmptyCustom (type, ctx)
-        {
-            /* OK, this sucks _hard_,
-             * QOfono does not return the added context, so instead we have to "figure it out"
-             * by looking for "contextAdded" for totally empty context with default Name values.
-             * LP(#1361864)
-             */
-
-            var targetName = "";
-            var targetAccessPointName = "";
-            if (type === "internet") {
-                targetName = "Internet";
-                targetAccessPointName = "";
-            } else if (type == "mms") {
-                targetName = "MMS";
-                targetAccessPointName = "";
-            }
-
-            if (ctx.type === type &&
-                ctx.name === targetName &&
-                ctx.accessPointName === targetAccessPointName)
-                return true;
-            return false;
-        }
-
         function updateContexts(contexts)
         {
             var tmp = contexts || sim.connMan.contexts.slice(0);
@@ -113,7 +88,6 @@
                 if (mContexts[currentValue] !== undefined) {
                     throw "updateContexts: added is broken";
                 }
-
                 var ctx = connCtx.createObject(parent,
                                                {
                                                    "contextPath": currentValue
@@ -138,15 +112,26 @@
 
         // expects updateContexts() to have ran before executing.
         function checkAndCreateCustomContexts() {
+
+            // When a context is added, we assume it is a custom one.
+            function addedCustomContext (path) {
+
+                // We do not have a QML object representing this context,
+                // so we ask updateContexts to create one.
+                if (!d.mContexts[path]) {
+                    d.updateContexts();
+                }
+                d.mContexts[path].name = mCustomContextNameInternet;
+                sim.connMan.contextAdded.disconnect(addedCustomContext);
+            }
+
             var customInternet = Object.keys(mContexts).filter(function (i) {
                 var ctx = mContexts[i];
-                return ctx.name === mCustomContextNameInternet ||
-                       isEmptyCustom("internet", ctx);
+                return ctx.name === mCustomContextNameInternet;
             });
             var customMms = Object.keys(mContexts).filter(function (i) {
                 var ctx = mContexts[i];
-                return ctx.name === mCustomContextNameMms ||
-                       isEmptyCustom("mms", ctx);
+                return ctx.name === mCustomContextNameMms;
             });
 
             // make sure there is only one context per type
@@ -167,6 +152,9 @@
 
             if (customInternet.length === 0) {
                 sim.connMan.addContext("internet");
+                sim.connMan.contextAdded.connect(addedCustomContext);
+            } else {
+                d.__haveCustomContexts = true;
             }
 
             // @bug don't create the custom MMS context
@@ -304,9 +292,12 @@
 
             onActiveChanged: if (type === "internet") internetApnSelector.updateSelectedIndex()
             onNameChanged: {
-                if (name === "Internet") {
-                    this.name = d.mCustomContextNameInternet;
-                } else if (name === "MMS") {
+
+                if (name === d.mCustomContextNameInternet) {
+                    d.__haveCustomContexts = true;
+                }
+
+                if (name === "MMS") {
                     this.name = d.mCustomContextNameMms;
                     this.accessPointName = d.pendingCustomMmsData["accessPointName"];
                     this.username = d.pendingCustomMmsData["username"];
@@ -434,12 +425,27 @@
                 text: i18n.tr("Custom Internet APN…")
                 width: parent.width - units.gu(4)
                 onClicked: {
-                    if (!d.__haveCustomContexts) {
+
+                    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);
                     }
-
-                    d.openApnEditor("internet")
                 }
             }