Ubuntu Weather App

Merge lp:~dpm/ubuntu-weather-app/minor-weather-api-improv into lp:ubuntu-weather-app/obsolete.trunk

  1. minor-weather-api-improv
  2. Merge into trunk
Proposed by David Planella
Status: Merged
Approved by: Martin Borho
Approved revision: 193
Merged at revision: 197
Proposed branch: lp:~dpm/ubuntu-weather-app/minor-weather-api-improv
Merge into: lp:ubuntu-weather-app/obsolete.trunk
Prerequisite: lp:~martin-borho/ubuntu-weather-app/weather-channel-client
Diff against target: 103 lines (+34/-14)
1 file modified
components/WeatherApi.js (+34/-14)
To merge this branch: bzr merge lp:~dpm/ubuntu-weather-app/minor-weather-api-improv
Reviewer Review Type Date Requested Status
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Martin Borho Approve
Review via email: mp+205753@code.launchpad.net

Commit message

Minor improvements on the weather API: added parameterize() function, added locale parameters to TWC calls, used _servicename consistently

Description of the change

Minor improvements on the weather API:

- Added parameterize() function to avoid code duplication
- Added locale parameters to TWC calls
- Used _servicename consistently for openweathermap

To post a comment you must log in.
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :

FAILED: Continuous integration, rev:190
http://91.189.93.70:8080/job/ubuntu-weather-app-ci/126/
Executed test runs:
    FAILURE: http://91.189.93.70:8080/job/generic-mediumtests-trusty/1234/console
    FAILURE: http://91.189.93.70:8080/job/ubuntu-weather-app-raring-amd64-ci/124/console
    FAILURE: http://91.189.93.70:8080/job/ubuntu-weather-app-saucy-amd64-ci/124/console
    FAILURE: http://91.189.93.70:8080/job/ubuntu-weather-app-trusty-amd64-ci/38/console

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Martin Borho (martin-borho) wrote :

David,

would you mind to add "hours=48" as additional param to the TWC call? Then we would have a hourly focecast for 48 instead of 24 hours. Overlooked it and it misses in the TWC branch.

Cheers
Martin

lp:~dpm/ubuntu-weather-app/minor-weather-api-improv updated
191. By David Planella

Merged from parent branch

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :

FAILED: Continuous integration, rev:191
http://91.189.93.70:8080/job/ubuntu-weather-app-ci/129/
Executed test runs:
    UNSTABLE: http://91.189.93.70:8080/job/generic-mediumtests-trusty/1237
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-raring-amd64-ci/127
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-saucy-amd64-ci/127
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-trusty-amd64-ci/41

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

review: Needs Fixing (continuous-integration)
lp:~dpm/ubuntu-weather-app/minor-weather-api-improv updated
192. By David Planella

Made hourly forecast to be 48 hours instead of the default 24

Revision history for this message
David Planella (dpm) wrote :

Done, made hourly forecast to be 48 hours. Let me know if you need any more changes to this branch.

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :

FAILED: Continuous integration, rev:192
http://91.189.93.70:8080/job/ubuntu-weather-app-ci/130/
Executed test runs:
    UNSTABLE: http://91.189.93.70:8080/job/generic-mediumtests-trusty/1244
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-raring-amd64-ci/128
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-saucy-amd64-ci/128
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-trusty-amd64-ci/42

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Martin Borho (martin-borho) wrote :

Looks good to me, thanks!

Glad you'Ve found the way through all the callbacks! ;)

review: Approve
lp:~dpm/ubuntu-weather-app/minor-weather-api-improv updated
193. By David Planella

Merged from the latest changes in the TWC client branch

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :

PASSED: Continuous integration, rev:193
http://91.189.93.70:8080/job/ubuntu-weather-app-ci/138/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-raring-amd64-ci/136
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-saucy-amd64-ci/136
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-trusty-amd64-ci/50

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

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'components/WeatherApi.js'
2--- components/WeatherApi.js 2014-02-15 10:47:52 +0000
3+++ components/WeatherApi.js 2014-02-15 10:47:52 +0000
4@@ -83,6 +83,17 @@
5 minutes: locTime.getUTCMinutes()
6 }
7 }
8+// Serialize a JavaScript object to URL parameters
9+// E.g. {param1: value1, param2: value2} to "param1=value&param2=value"
10+// TODO: it'd be nice to make it work with either passing a single object
11+// or several at once
12+function parameterize(obj) {
13+ var str = [];
14+ for(var param in obj) {
15+ str.push(encodeURIComponent(param) + "=" + encodeURIComponent(obj[param]));
16+ }
17+ return str.join("&");
18+}
19
20 var GeoipApi = (function() {
21 var _baseUrl = "http://geoip.ubuntu.com/lookup";
22@@ -174,6 +185,8 @@
23 */
24 var _baseUrl = "http://api.openweathermap.org/data/2.5/";
25 //
26+ var _serviceName = "openweathermap";
27+ //
28 var _icon_map = {
29 "01d": "clear_day",
30 "01n": "clear_night",
31@@ -219,7 +232,7 @@
32 condition: data.weather[0]
33 };
34 if(data.id !== undefined) {
35- result["service"] = "openweathermap";
36+ result["service"] = _serviceName;
37 result["service_id"] = data.id;
38 }
39 return result;
40@@ -263,8 +276,8 @@
41 todayDate;
42 print("["+location.name+"] "+JSON.stringify(localNow))
43 // add openweathermap id for faster responses
44- if(location.services && !location.services["openweathermap"] && data["current"].id) {
45- location.services["openweathermap"] = data["current"].id
46+ if(location.services && !location.services[_serviceName] && data["current"].id) {
47+ location.services[_serviceName] = data["current"].id
48 }
49 //
50 data["daily"]["list"].forEach(function(dayData) {
51@@ -306,10 +319,10 @@
52 },
53 latLongParams = "&lat="+encodeURIComponent(params.location.coord.lat)
54 + "&lon="+encodeURIComponent(params.location.coord.lon);
55- if(params.location.services && params.location.services["openweathermap"]) {
56- urls.current = _baseUrl + "weather?units="+params.units+"&id="+params.location.services["openweathermap"];
57- urls.daily = _baseUrl + "forecast/daily?id="+params.location.services["openweathermap"]+"&cnt=10&units="+params.units
58- urls.forecast = _baseUrl + "forecast?id="+params.location.services["openweathermap"]+"&units="+params.units
59+ if(params.location.services && params.location.services[_serviceName]) {
60+ urls.current = _baseUrl + "weather?units="+params.units+"&id="+params.location.services[_serviceName];
61+ urls.daily = _baseUrl + "forecast/daily?id="+params.location.services[_serviceName]+"&cnt=10&units="+params.units
62+ urls.forecast = _baseUrl + "forecast?id="+params.location.services[_serviceName]+"&units="+params.units
63
64 } else if (params.location.coord) {
65 urls.current = _baseUrl + "weather?units="+params.units+latLongParams;
66@@ -534,15 +547,22 @@
67 //
68 function _getUrl(params) {
69 var url, serviceId,
70- units = (params.units === "metric") ? "m" : "e",
71- baseParams = "key="+params.api_key+"&units="+units;
72+ baseParams = {
73+ key: params.api_key,
74+ units: (params.units === "metric") ? "m" : "e",
75+ locale: Qt.locale().name,
76+ hours: "48",
77+ },
78+ commands = {
79+ "mobileaggregation": "mobile/mobagg/",
80+ };
81 if(params.location.services && params.location.services[_serviceName]) {
82 serviceId = encodeURIComponent(params.location.services[_serviceName]);
83- url = _baseUrl+"mobile/mobagg/"+serviceId+".js?"+baseParams;
84+ url = _baseUrl+commands["mobileaggregation"]+serviceId+".js?"+parameterize(baseParams);
85 } else if (params.location.coord) {
86- url = _baseUrl+"mobile/mobagg/get.js?"+baseParams+
87- "&lat="+encodeURIComponent(params.location.coord.lat)+
88- "&lng="+encodeURIComponent(params.location.coord.lon);
89+ var coord = {lat: params.location.coord.lat, lng: params.location.coord.lon};
90+ url = _baseUrl+commands["mobileaggregation"]+"get.js?"+parameterize(baseParams)+"&"+
91+ parameterize(coord);
92 }
93 return url;
94 }
95@@ -592,7 +612,7 @@
96 function _sendRequest(request, onSuccess, onError) {
97 var xmlHttp = new XMLHttpRequest();
98 if (xmlHttp) {
99- console.log(request.url);
100+ console.log("Sent request URL: " + request.url);
101 xmlHttp.open('GET', request.url, true);
102 xmlHttp.onreadystatechange = function () {
103 try {

Subscribers

People subscribed via source and target branches