Merge lp:~vthompson/ubuntu-weather-app/reboot-allow-adding-duplicate-current-location into lp:ubuntu-weather-app

Proposed by Victor Thompson on 2015-06-28
Status: Merged
Approved by: Andrew Hayzen on 2015-07-25
Approved revision: 58
Merged at revision: 67
Proposed branch: lp:~vthompson/ubuntu-weather-app/reboot-allow-adding-duplicate-current-location
Merge into: lp:ubuntu-weather-app
Diff against target: 21 lines (+3/-2)
1 file modified
app/ubuntu-weather-app.qml (+3/-2)
To merge this branch: bzr merge lp:~vthompson/ubuntu-weather-app/reboot-allow-adding-duplicate-current-location
Reviewer Review Type Date Requested Status
Andrew Hayzen Approve on 2015-07-25
Nekhelesh Ramananthan (community) 2015-06-28 Needs Information on 2015-06-29
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve on 2015-06-29
Review via email: mp+263209@code.launchpad.net

This proposal supersedes a proposal from 2015-06-28.

Commit message

Allow adding the current location as a duplicate to the Locations List.

Description of the change

Allow adding the current location as a duplicate to the Locations List.

To post a comment you must log in.
Nekhelesh Ramananthan (nik90) wrote :

Why do we want to allow the current location as a duplicate to the locations list?

review: Needs Information
Victor Thompson (vthompson) wrote :

Let's say you travel a lot and have Chicago and NYC on your list of locations. You live in Amsterdam. Work has you traveling to Paris for an event. Upon arrival you decide to also add Paris to your list of locations--however, currently you can not do so because it matches your current location. For this reason, current location shouldn't prevent a duplicate entry, since it is simply a pointer to your current location.

Andrew Hayzen (ahayzen) wrote :

LGTM :-)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'app/ubuntu-weather-app.qml'
2--- app/ubuntu-weather-app.qml 2015-06-28 19:04:43 +0000
3+++ app/ubuntu-weather-app.qml 2015-06-28 23:49:34 +0000
4@@ -211,14 +211,15 @@
5 return !exists;
6 }
7
8- // Return true if the location given is already in the locationsList
9+ // Return true if the location given is already in the locationsList, and is not the same
10+ // as the current location.
11 function checkLocationExists(location) {
12 var exists = false;
13
14 for (var i=0; !exists && i < locationsList.length; i++) {
15 var loc = locationsList[i].location;
16
17- if (loc.services.geonames && (loc.services.geonames === location.services.geonames)) {
18+ if (loc.services.geonames && (loc.services.geonames === location.services.geonames) && !(settings.addedCurrentLocation && i === 0)) {
19 exists = true;
20 }
21 }

Subscribers

People subscribed via source and target branches

to all changes: