Merge lp:~martin-borho/ubuntu-weather-app/fix-1366343-deleting into lp:ubuntu-weather-app/obsolete.trunk

[Martin Borho]

Deleting and adding locations in database immediately after removing or adding in location manager, fixes Bug #1366343

Deleting and adding locations in database immediately after removing or adding in location manager, not on back action than. Fixes Bug #1366343

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:370
http://91.189.93.70:8080/job/ubuntu-weather-app-ci/270/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic-weather-app-python3/8
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic-weather-app-python3/8/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-utopic-amd64-ci/76

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-weather-app-ci/270/rebuild

review: Approve (continuous-integration)

[Alan Pope 🍺🐧🐱 🦄 (popey)]

Looks good to me! Thanks!

review: Approve

=== modified file 'components/LocationManagerPage.qml'
--- components/LocationManagerPage.qml	2014-09-05 09:57:29 +0000
+++ components/LocationManagerPage.qml	2014-09-20 12:10:04 +0000
@@ -36,53 +36,27 @@
 
     property int initial_sum: 0
     property bool limited: (locationModel.count > 7)
+    property bool locationsChanged: false
 
     head.backAction: Action {
         iconName: "back"
         onTriggered: {
-            var locationsChanged = false,
-                refresh = false;
-            // remove
-            for(var x=0;x<locationsRemoved.count;x++) {
-                var delLoc = locationsRemoved.get(x);
-                storage.clearLocation(delLoc.dbId);
-                locationsChanged = true;
-            }
-            // add
-            for(var y=0;y<locationModel.count;y++) {
-                var loc = locationModel.get(y);
-                if(loc.dbId === undefined || loc.dbId=== 0) {
-                    storage.insertLocation({location:loc});
-                    locationsChanged = true;
-                }
-            }
-            // refresh tabs if neccessary
-            if(locationsChanged || locationModel.count === 0) {
-                refresh = true;
-            }
-            // handle dummy "current location" item
-            currentLocationItem.visible = false
-            locationLookupItem.visible = true
-            lookupItemAddButton.visible = true
-            mainView.shortCutsLimited = false;
             if(locationModel.count > 0) {
-                popFromStack(refresh)
+                popFromStack()
             }
         }
     }
 
-    Component.onCompleted: {
-        mainView.shortCutsLimited = true;
-        locationsRemoved.clear();
+    Component.onCompleted: {        
         loadData();
     }
 
     Connections { target: mainView; onEscapeKey: popFromStack()}
 
-    function popFromStack(refresh) {
+    function popFromStack() {
         pageStack.pop()
         // refresh at least from storage, to prevent header content from disapearing
-        refreshData(!refresh, refresh);
+        refreshData(!locationsChanged, locationsChanged);
     }
 
     function lookupResponseHandler(messageObject) {
@@ -131,10 +105,22 @@
 
     function addLocation(location) {
         if(!checkLocationExists(location)) {
-            locationModel.append(location)
+            if(location.dbId === undefined || location.dbId=== 0) {
+                locationModel.append(location)
+                storage.insertLocation({location:location});
+                locationsChanged = true;
+            }
         }
     }
 
+    function deleteLocation(index) {
+        var location = locationModel.get(index),
+            locDbId = location.dbId;
+        locationModel.remove(index)
+        storage.clearLocation(locDbId);
+        locationsChanged = true;
+    }
+
     Dialog {
         id:existsNotification
         title: i18n.tr("Location already added.")
@@ -148,10 +134,6 @@
         id: locationModel
     }
 
-    ListModel {
-        id: locationsRemoved
-    }
-
     Rectangle {
         anchors.fill:parent
         color: "lightgrey"
@@ -347,11 +329,7 @@
                         }
                         removable: true
                         onItemRemoved: {
-                            var location = locationModel.get(index)
-                            // clone location obj
-                            location = JSON.parse(JSON.stringify(location));
-                            locationsRemoved.append(location)
-                            locationModel.remove(index)
+                            deleteLocation(index);
                         }
                     }
                     footer: ListItem.Standard {

=== modified file 'tests/autopilot/ubuntu_weather_app/tests/test_locationmanager.py'
--- tests/autopilot/ubuntu_weather_app/tests/test_locationmanager.py	2014-09-04 16:21:49 +0000
+++ tests/autopilot/ubuntu_weather_app/tests/test_locationmanager.py	2014-09-20 12:10:04 +0000
@@ -307,25 +307,6 @@
         tabObjects = lambda: len(self.main_view.select_many('LocationTab'))
         self.assertThat(tabObjects, Eventually(Equals(tabsSumStart)))
 
-    def test_del_add_bug(self):
-        """Tests if removing and add a location in one action works
-           https://bugs.launchpad.net/ubuntu-weather-app/+bug/1230297"""
-        # remove a location
-        self._remove_location(stay=True)
-
-        # add a location
-        self._add_location("Cairo")
-
-        # back to locations, wait till data is loaded
-        self.main_view.wait_for_activity_to_finish()
-
-        # count the tabs
-        tabObjects = self.main_view.select_many('LocationTab')
-        self.assertThat(lambda: len(tabObjects), Eventually(Equals(2)))
-        self.assertThat(
-            self.main_view.select_single('LocationTab', index=1).title,
-            Eventually(Equals("Cairo")))
-
     @skip("skipped because of Bug #1279073")
     def test_open_location_manager_keyboard(self):
         if self.platform_model != "Desktop":