ubuntu-touch-coreapps-reviewers team mailing list archive

[Andrew Hayzen <ahayzen@xxxxxxxxx>]

OK, this is the first iteration of a 'working' solution, still needs lots of testing :-) Basically as TWC now, from what I understand, requires us to do multiple url calls to get the current, daily and hourly data. I've copied the OWM code and modified it to match the TWC api (as OWM is similar in the fact that it requires multiple url calls).

Main inline comment so far is:
Not sure what todo here? This is for "weather codes" such as, UKXX0085, which we were using, but I cannot see that in the new API? Currently the 1==2 is forcing it to use coordinate based lookups.

Also was the search using TWC? I need to check if that needs to be updated?

Diff comments:

> === modified file 'app/data/WeatherApi.js'
> --- app/data/WeatherApi.js	2015-10-21 02:16:50 +0000
> +++ app/data/WeatherApi.js	2016-05-28 00:41:30 +0000
> @@ -591,48 +610,101 @@
>          return result;
>      }
>      //
> -    function _getUrl(params) {
> -        var url, serviceId,
> -            baseParams = {
> -                key: params.twc_api_key,
> -                units: (params.units === "metric") ? "m" : "e",
> -                locale: Qt.locale().name,
> -                hours: "48",
> -            },
> -            commands = {
> -                "mobileaggregation": "mobile/mobagg/",
> +    function _getUrls(params) {
> +        var baseParams = {
> +            units: (params.units === "metric") ? "m" : "e",
> +            language: Qt.locale().name.replace("_","-"),
> +            apiKey: params.twc_api_key,
> +        };
> +        var commands = {
> +            "geocode": "v1/geocode/",
> +        };
> +        var urls = {
> +            current: "",
> +            daily: "",
> +            hourly: "",
> +        };
> +
> +        if(1==2 && params.location.services && params.location.services[_serviceName]) {  // FIXME: disable for now (UKXX0085)

Not sure what todo here? This is for "weather codes" such as, UKXX0085, which we were using, but I cannot see that in the new API? Currently the 1==2 is forcing it to use coordinate based lookups.

> +            var serviceId = encodeURIComponent(params.location.services[_serviceName]);
> +
> +            urls.current = _baseUrl + commands["geocode"] +
> +                    serviceId + ".js?" + parameterize(baseParams);
> +            urls.daily = _baseUrl + commands["geocode"] +
> +                    serviceId + ".js?" + parameterize(baseParams);
> +            urls.hourly = _baseUrl + commands["geocode"] +
> +                    serviceId + ".js?" + parameterize(baseParams);
> +        } else if (params.location.coord) {
> +            var coord = {
> +                lat: params.location.coord.lat,
> +                lng: params.location.coord.lon
>              };
> -        if(params.location.services && params.location.services[_serviceName]) {
> -            serviceId = encodeURIComponent(params.location.services[_serviceName]);
> -            url = _baseUrl+commands["mobileaggregation"]+serviceId+".js?"+parameterize(baseParams);
> -        } else if (params.location.coord) {
> -            var coord = {lat: params.location.coord.lat, lng: params.location.coord.lon};
> -            url = _baseUrl+commands["mobileaggregation"]+"get.js?"+parameterize(baseParams)+"&"+
> -                  parameterize(coord);
> +
> +            urls.current = _baseUrl + commands["geocode"] +
> +                    coord.lat + "/" + coord.lng +
> +                    "/observations/current.json?" +
> +                    parameterize(baseParams);
> +            urls.daily = _baseUrl + commands["geocode"] +
> +                    coord.lat + "/" + coord.lng +
> +                    "/forecast/daily/5day.json?" +
> +                    parameterize(baseParams);
> +            urls.hourly = _baseUrl + commands["geocode"] +
> +                    coord.lat + "/" + coord.lng +
> +                    "/forecast/hourly/48hour.json?" +
> +                    parameterize(baseParams);
>          }
> -        return url;
> +
> +        return urls;
>      }
>      //
>      return {
>          getData: function(params, apiCaller, onSuccess, onError) {
> -            var url = _getUrl(params),
> +            var urls = _getUrls(params),
>                  handlerMap = {
> -                    all: { type: "all", url: url}
> -                },
> -                response = {
> -                    location: params.location,
> -                    db: (params.db) ? params.db : null,
> -                    format: RESPONSE_DATA_VERSION
> -                },
> -                addDataToResponse = (function(request, data) {
> -                    var formattedResult;
> -                    response["data"] = formatResult(data, params.location);
> +                current: {
> +                    type: "current",
> +                    url: urls.current
> +                },
> +                daily: {
> +                    type: "daily",
> +                    url: urls.daily
> +                },
> +                forecast: {
> +                    type: "forecast",
> +                    url: urls.hourly
> +                }
> +            },
> +            response = {
> +                location: params.location,
> +                db: (params.db) ? params.db : null,
> +                format: RESPONSE_DATA_VERSION
> +            },
> +            respData = {},
> +            addDataToResponse = (function(request, data) {
> +                var formattedResult;
> +                respData[request.type] = data;
> +
> +                if (respData["current"] !== undefined &&
> +                        respData["forecast"] !== undefined &&
> +                            respData["daily"] !== undefined) {
> +
> +                    response["data"] = formatResult(respData, params.location);
>                      onSuccess(response);
> -                }),
> -                onErrorHandler = (function(err) {
> -                    onError(err);
> -                });
> -            apiCaller(handlerMap.all, addDataToResponse, onErrorHandler);
> +                }
> +            }),
> +            onErrorHandler = (function(err) {
> +                onError(err);
> +            }),
> +            retryHandler = (function(err) {
> +                console.log("retry of " + trimAPIKey(err.request.url));
> +                var retryFunc = handlerMap[err.request.type];
> +
> +                apiCaller(retryFunc, addDataToResponse, onErrorHandler);
> +            });
> +
> +            apiCaller(handlerMap.current, addDataToResponse, retryHandler);
> +            apiCaller(handlerMap.forecast, addDataToResponse, retryHandler);
> +            apiCaller(handlerMap.daily, addDataToResponse, retryHandler);
>          }
>      }
>  })();


-- 
https://code.launchpad.net/~ubuntu-weather-dev/ubuntu-weather-app/new-weather-api/+merge/289584
Your team Ubuntu Weather Developers is requested to review the proposed merge of lp:~ubuntu-weather-dev/ubuntu-weather-app/new-weather-api into lp:ubuntu-weather-app.