Merge lp:~knitzsche/ubuntu-rest-scopes/fix-10-day-forecast-url into lp:ubuntu-rest-scopes

[Kyle Nitzsche]

Ensure that the url used to open the 10-day forecast on weather.com uses lat and long of sufficient precision to avoid 404 style error when user taps Open on Preview from Today scope.

[Facundo Batista (facundo)]

Ah, it's a good catch that the URL needs a 5 digit precision (even if the numbers are completed with zeros).

A detail in the formatting: the 2 as width doesn't make sense, as if you request 5 digits for precision, it always will be greater than 2, so please remove it.

Also, please include this formatting change in the ENGLISH_URL_TEMPLATE itself, no need to prebuild other string.

Thanks!!

review: Needs Fixing

[Facundo Batista (facundo)]

It will also be great if you include a test case where it shows that this change is needed, thanks!

[lp:~knitzsche/ubuntu-rest-scopes/fix-10-day-forecast-url]

535. By Kyle Nitzsche on 2016-10-04

weatherchannel.py:
* don't prebuild string to set lat and lng for url
* improve the format specifier
* also apply format to URL_TEMPLATE so lat an lng have 5 digit precisions

[Kyle Nitzsche (knitzsche)]

Hey Facundo:

Do want a unit test that *fails* when a URL is sent to weather.com with insufficient lat/lng precision? (who knows - they might fix their issue and someday such URLs could succeed.)

Or is this url (pulled from bug reporters log) enough:

https://weather.com/weather/10day/l/47,8?par=ubuntu_bq

See also: https://bugs.launchpad.net/ubuntu-rest-scopes/+bug/1588997/comments/8

[Facundo Batista (facundo)]

That URL is enough... I mean, make a test that would end up producing a URL with latitude=47 and longitude=8, and assert that correct URL is returned in the response.

Thanks!

[lp:~knitzsche/ubuntu-rest-scopes/fix-10-day-forecast-url]

536. By Kyle Nitzsche on 2016-10-07

modify weatherchannel tests to ensure that the actual url produced has
five digit percision.

[Facundo Batista (facundo)]

Please include this formatting change in the ENGLISH_URL_TEMPLATE itself, no need to prebuild other string.

review: Needs Fixing

[Kyle Nitzsche (knitzsche)]

Hi Facundo,

I don't understand :) I do not prebuild any other string, and the format is applied directly to the ENGLISH_URL_TEMPLATE itself lines 64-65, and to URL_TEMPLATE lines 72-73.

(the only other changes were to for tests and in these cases I only modified the hard coded url strings.)

[Facundo Batista (facundo)]

You're doing

     latitude="{:.5f}".format(latitude),

to put a pre-formatted latitude in URL_TEMPLATE or ENGLISH_URL_TEMPLATE.

Instead of that, just add the ":.5f" format modifier to the URL_TEMPLATE or ENGLISH_URL_TEMPLATE themselves.

[lp:~knitzsche/ubuntu-rest-scopes/fix-10-day-forecast-url]

537. By Kyle Nitzsche on 2016-11-10

add the ":.5f" format modifier to the URL_TEMPLATE or ENGLISH_URL_TEMPLATE
themselves, per review request

[Kyle Nitzsche (knitzsche)]

Facundo, how's this? :) (I forgot about this for a while ...)

review: Needs Resubmitting

[Kyle Nitzsche (knitzsche)]

I set this to "Resubmit" thinking that meant "I've Resubmitted", which is probably incorrect.

Anyway, it is ready for review.

[Kyle Nitzsche (knitzsche)]

review: Abstain

Unmerged revisions

537. By Kyle Nitzsche on 2016-11-10

add the ":.5f" format modifier to the URL_TEMPLATE or ENGLISH_URL_TEMPLATE
themselves, per review request

536. By Kyle Nitzsche on 2016-10-07

modify weatherchannel tests to ensure that the actual url produced has
five digit percision.

535. By Kyle Nitzsche on 2016-10-04

weatherchannel.py:
* don't prebuild string to set lat and lng for url
* improve the format specifier
* also apply format to URL_TEMPLATE so lat an lng have 5 digit precisions

534. By Kyle Nitzsche on 2016-09-30

fixes lp:1629243

533. By Kyle Nitzsche on 2016-09-30

create url used from preview to open weatherchannel.com web page
with 10-day forecast with latitude and longitude that has 5 digit
precisions, as seems to be required.

=== modified file 'src/scopes/tests/test_weatherchannel.py'
--- src/scopes/tests/test_weatherchannel.py	2016-06-07 14:01:21 +0000
+++ src/scopes/tests/test_weatherchannel.py	2016-11-10 20:52:05 +0000
@@ -37,7 +37,7 @@
         self.assertEqual(response[0]['category']['title'], u'SE1, England, GB')
 
         # the two big items, the first very detailed for sanity check
-        url = 'https://weather.com/weather/today/l/123,456?par=ptest'
+        url = 'https://weather.com/weather/today/l/123.00000,456.00000?par=ptest'
         comment = u'Light rain. Low 4\xbaC.'
         self.assertDictEqual(response[1], dict(result={
             'art': 'foo/weatherchannel/suru/weather-showers.svg',
@@ -207,7 +207,7 @@
         self.assertEqual(response[0]['category']['title'], u'SE1, 英格兰, GB')
 
         # the result
-        url = 'https://weather.com/weather/today/l/123,456?par=ptest'
+        url = 'https://weather.com/weather/today/l/123.00000,456.00000?par=ptest'
         comment = u'\u5c11\u4e91\u3002 \u6700\u4f4e 4\xbaC\u3002'
         self.assertDictEqual(response[1], dict(result={
             'art': 'foo/weatherchannel/suru/weather-clouds-night.svg',
@@ -399,7 +399,7 @@
                 'icon': None,
                 'id': 'action1',
                 'label': '10 day forecast',
-                'uri': 'https://weather.com/weather/10day/l/123,456?par=ptest'
+                'uri': 'https://weather.com/weather/10day/l/123.00000,456.00000?par=ptest'
             }],
         }))
 
@@ -604,7 +604,7 @@
         self.assertEqual(len(resp), 11)
 
         self.assertDictEqual(resp[0], {
-            'url': 'https://weather.com/weather/today/l/123,456?par=ptest',
+            'url': 'https://weather.com/weather/today/l/123.00000,456.00000?par=ptest',
             'art': 'foo/weatherchannel/suru/weather-showers.svg',
             'art_wlogo': 'foo/weatherchannel/suru/weather-showers-wlogo.svg',
             'day': u'Tonight',
@@ -627,7 +627,7 @@
         })
 
         self.assertDictEqual(resp[1], {
-            'url': 'https://weather.com/weather/10day/l/123,456?par=ptest',
+            'url': 'https://weather.com/weather/10day/l/123.00000,456.00000?par=ptest',
             'art': 'foo/weatherchannel/suru/weather-clouds.svg',
             'art_wlogo': 'foo/weatherchannel/suru/weather-clouds-wlogo.svg',
             'day': u'Tomorrow',

=== modified file 'src/scopes/weatherchannel.py'
--- src/scopes/weatherchannel.py	2016-06-07 14:01:21 +0000
+++ src/scopes/weatherchannel.py	2016-11-10 20:52:05 +0000
@@ -81,8 +81,8 @@
 for icon, wids in _icons_map:
     WEATHER_ICONS_MAP.update(dict.fromkeys(wids, icon))
 
-URL_TEMPLATE = 'https://weather.com/{language}/weather/{hint}/l/{latitude},{longitude}?par={partner}'  # NOQA
-ENGLISH_URL_TEMPLATE = 'https://weather.com/weather/{hint}/l/{latitude},{longitude}?par={partner}'
+URL_TEMPLATE = 'https://weather.com/{language}/weather/{hint}/l/{latitude:.5f},{longitude:.5f}?par={partner}'  # NOQA
+ENGLISH_URL_TEMPLATE = 'https://weather.com/weather/{hint}/l/{latitude:.5f},{longitude:.5f}?par={partner}'
 
 # the categories to use
 CATEGORY_GRID_MAIN = {
@@ -259,6 +259,7 @@
 
     def get_localized_url(self, hint, latitude, longitude):
         """Build a localized URL with several parameters."""
+        #ensure url has lat & long of sufficient length
         if self.language.startswith('en-') or self.language == 'en':
             # TWC URLs english are special
             return ENGLISH_URL_TEMPLATE.format(
@@ -269,8 +270,8 @@
         else:
             return URL_TEMPLATE.format(
                 hint=hint,
-                latitude=latitude,
-                longitude=longitude,
+                latitude="{:.5f}".format(latitude),
+                longitude="{:.5f}".format(longitude),
                 language=self.language,
                 partner=self.auth['partner'])
 
@@ -395,7 +396,6 @@
                                                    location_info['ccode'])))
 
                 url = self.get_localized_url(uri_date, latitude, longitude)
-
                 # improve conditions, in case got nothing from TWC
                 conditions = daily_info['conditions']
                 if not conditions: