diff --git a/CHANGES.rst b/CHANGES.rst index 2fac8dd..0bdca2c 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -6,6 +6,9 @@ There's a frood who really knows where his towel is. 1.0a4 (unreleased) ^^^^^^^^^^^^^^^^^^ +- Refactor weather_utility to use location_id as key. (closes `#36`_). + [marcosfromero] + - Reduce complexity in utilities. (closes `#8`_). [marcosfromero] - Remove dependency on pywapi. @@ -84,4 +87,5 @@ There's a frood who really knows where his towel is. .. _`#14`: https://github.com/collective/collective.weather/issues/14 .. _`#18`: https://github.com/collective/collective.weather/issues/18 .. _`#19`: https://github.com/collective/collective.weather/issues/19 +.. _`#36`: https://github.com/collective/collective.weather/issues/36 .. _`How to make your Plone add-on products uninstall cleanly`: http://blog.keul.it/2013/05/how-to-make-your-plone-add-on-products.html diff --git a/src/collective/weather/browser/templates/current_weather.pt b/src/collective/weather/browser/templates/current_weather.pt index a421ff0..66ac579 100644 --- a/src/collective/weather/browser/templates/current_weather.pt +++ b/src/collective/weather/browser/templates/current_weather.pt @@ -5,7 +5,7 @@ Córdoba, Argentina diff --git a/src/collective/weather/config.py b/src/collective/weather/config.py index d6c4d51..5c28d81 100644 --- a/src/collective/weather/config.py +++ b/src/collective/weather/config.py @@ -11,10 +11,6 @@ # Time that should pass before fetching new weather information. TIME_THRESHOLD = timedelta(minutes=30) -WEATHER_APIS = SimpleVocabulary([ - SimpleTerm(value=u'yahoo', title=_(u'Yahoo! Weather')), -]) - UNIT_SYSTEMS = SimpleVocabulary([ SimpleTerm(value=u'metric', title=_(u'Metric')), SimpleTerm(value=u'imperial', title=_(u'Imperial')), diff --git a/src/collective/weather/interfaces.py b/src/collective/weather/interfaces.py index 722aaeb..a17eeda 100644 --- a/src/collective/weather/interfaces.py +++ b/src/collective/weather/interfaces.py @@ -70,7 +70,7 @@ class IWeatherSettings(form.Schema): description=_( u'help_location_ids', default=u'Enter here all available locations that will be ' - u'available. Format: id|name|location_id. Check the ' + u'available. Format: location_id|name. Check the ' u'package documentation ' u'for further information on how to find ids.'), value_type=schema.TextLine(), diff --git a/src/collective/weather/portlets/weather.py b/src/collective/weather/portlets/weather.py index cd2b7f2..f7597f7 100644 --- a/src/collective/weather/portlets/weather.py +++ b/src/collective/weather/portlets/weather.py @@ -65,11 +65,11 @@ def update(self): self.current_city = None if self.data.location in vocab: location = vocab.by_value[self.data.location] - self.current_city = {'id': location.value, 'name': location.title} + self.current_city = {'location_id': location.value, 'name': location.title} self.weather_info = None if not self.current_city is None: - weather_utility.update_weather_info(self.current_city['id']) + weather_utility.update_weather_info(self.current_city['location_id']) self.weather_info = weather_utility.get_weather_info(self.current_city) diff --git a/src/collective/weather/profiles/test_fixture/registry.xml b/src/collective/weather/profiles/test_fixture/registry.xml index 26830cb..2b5be93 100644 --- a/src/collective/weather/profiles/test_fixture/registry.xml +++ b/src/collective/weather/profiles/test_fixture/registry.xml @@ -8,8 +8,8 @@ - Cordoba|Cordoba, Argentina|ARCA0023 - Los Angeles|Los Angeles, California|USCA0638 + ARCA0023|Cordoba, Argentina + USCA0638|Los Angeles, California diff --git a/src/collective/weather/tests/functional.txt b/src/collective/weather/tests/functional.txt index 41cd0de..edb863c 100644 --- a/src/collective/weather/tests/functional.txt +++ b/src/collective/weather/tests/functional.txt @@ -48,8 +48,8 @@ And we check again Now, change the city - >>> browser.open(portalURL+'/@@update-weather?city=Los Angeles') - >>> browser.open(portalURL+'?city=Los Angeles') + >>> browser.open(portalURL+'/@@update-weather?city=USCA0638') + >>> browser.open(portalURL+'?city=USCA0638') >>> "No weather information" in browser.contents False >>> "-8°C" in browser.contents diff --git a/src/collective/weather/tests/test_controlpanel.py b/src/collective/weather/tests/test_controlpanel.py index d0f2f1e..9e36b75 100644 --- a/src/collective/weather/tests/test_controlpanel.py +++ b/src/collective/weather/tests/test_controlpanel.py @@ -68,8 +68,8 @@ def test_location_ids_record_in_registry(self): self.assertTrue(hasattr(self.settings, 'location_ids')) # as defined in the test fixture expected = [ - u'Cordoba|Cordoba, Argentina|ARCA0023', - u'Los Angeles|Los Angeles, California|USCA0638' + u'ARCA0023|Cordoba, Argentina', + u'USCA0638|Los Angeles, California' ] self.assertEqual(self.settings.location_ids, expected) diff --git a/src/collective/weather/tests/test_vocabularies.py b/src/collective/weather/tests/test_vocabularies.py index 50dd89d..b455003 100644 --- a/src/collective/weather/tests/test_vocabularies.py +++ b/src/collective/weather/tests/test_vocabularies.py @@ -21,8 +21,8 @@ def test_locations_vocabulary(self): locations = util(self.portal) # as defined in the test fixture self.assertEqual(len(locations), 2) - self.assertIn('Cordoba', locations) - self.assertIn(u'Los Angeles', locations) + self.assertIn('ARCA0023', locations) + self.assertIn('USCA0638', locations) def test_providers_vocabulary(self): name = 'collective.weather.Providers' diff --git a/src/collective/weather/tests/test_weather_portlet.py b/src/collective/weather/tests/test_weather_portlet.py index 55436bb..d61569e 100644 --- a/src/collective/weather/tests/test_weather_portlet.py +++ b/src/collective/weather/tests/test_weather_portlet.py @@ -54,7 +54,7 @@ def test_invoke_add_view(self): del mapping[m] addview = mapping.restrictedTraverse('+/' + portlet.addview) - addview.createAndAdd(data={'header': u'Weather', 'location': u'Cordoba'}) + addview.createAndAdd(data={'header': u'Weather', 'location': u'ARCA0023'}) self.assertEqual(len(mapping), 1) self.assertTrue(isinstance(mapping.values()[0], weather.Assignment)) @@ -99,7 +99,7 @@ def renderer(self, context=None, request=None, view=None, manager=None, assignme (context, request, view, manager, assignment), IPortletRenderer) def test_render(self): - assignment = weather.Assignment(header=u'Weather', location=u'Cordoba') + assignment = weather.Assignment(header=u'Weather', location=u'ARCA0023') r = self.renderer(assignment=assignment) r = r.__of__(self.portal) diff --git a/src/collective/weather/tests/test_weather_utility.py b/src/collective/weather/tests/test_weather_utility.py index 46812c8..e5cfb1a 100644 --- a/src/collective/weather/tests/test_weather_utility.py +++ b/src/collective/weather/tests/test_weather_utility.py @@ -30,12 +30,10 @@ def test_update_locations(self): settings = registry.forInterface(IWeatherSettings) old_ids = deepcopy(settings.location_ids) - expected_values = [{'id': u'Cordoba', - 'location_id': u'ARCA0023', + expected_values = [{'location_id': u'ARCA0023', 'name': u'Cordoba, Argentina', 'type': 'testprovider'}, - {'id': u'Los Angeles', - 'location_id': u'USCA0638', + {'location_id': u'USCA0638', 'name': u'Los Angeles, California', 'type': 'testprovider'}] @@ -44,18 +42,15 @@ def test_update_locations(self): self.assertEqual(actual_values, expected_values) # We add a new city to the registry - settings.location_ids.append(u'New city|New city, Test|LALA1212') + settings.location_ids.append(u'LALA1212|New city, Test') - expected_values = [{'id': u'Cordoba', - 'location_id': u'ARCA0023', + expected_values = [{'location_id': u'ARCA0023', 'name': u'Cordoba, Argentina', 'type': 'testprovider'}, - {'id': u'Los Angeles', - 'location_id': u'USCA0638', + {'location_id': u'USCA0638', 'name': u'Los Angeles, California', 'type': 'testprovider'}, - {'id': u'New city', - 'location_id': u'LALA1212', + {'location_id': u'LALA1212', 'name': u'New city, Test', 'type': 'testprovider'}] @@ -66,16 +61,13 @@ def test_update_locations(self): # Finally, we add a malformed value to the list settings.location_ids.append('My city') - expected_values = [{'id': u'Cordoba', - 'location_id': u'ARCA0023', + expected_values = [{'location_id': u'ARCA0023', 'name': u'Cordoba, Argentina', 'type': 'testprovider'}, - {'id': u'Los Angeles', - 'location_id': u'USCA0638', + {'location_id': u'USCA0638', 'name': u'Los Angeles, California', 'type': 'testprovider'}, - {'id': u'New city', - 'location_id': u'LALA1212', + {'location_id': u'LALA1212', 'name': u'New city, Test', 'type': 'testprovider'}] @@ -93,12 +85,12 @@ def test_update_weather_info(self): # Update weather info self.weather_utility.update_weather_info() - expected_values = {u'Los Angeles': {'conditions': u'Snowing', - 'temp': u'-8\xb0C', - 'icon': u'icon.png'}, - u'Cordoba': {'conditions': u'Windy', - 'temp': u'20\xb0C', - 'icon': u'icon.png'}} + expected_values = {u'USCA0638': {'conditions': u'Snowing', + 'temp': u'-8\xb0C', + 'icon': u'icon.png'}, + u'ARCA0023': {'conditions': u'Windy', + 'temp': u'20\xb0C', + 'icon': u'icon.png'}} actual_values = dict([(i, self.weather_utility.get_weather_info()[i]['weather']) for i in self.weather_utility.get_weather_info()]) @@ -106,19 +98,19 @@ def test_update_weather_info(self): # Now, we add a new city old_locations = deepcopy(settings.location_ids) - settings.location_ids.append(u'New weather|New weather|NEW123') + settings.location_ids.append(u'NEW123|New weather') self.weather_utility.update_weather_info() - expected_values = {u'Los Angeles': {'conditions': u'Snowing', - 'temp': u'-8\xb0C', - 'icon': u'icon.png'}, - u'Cordoba': {'conditions': u'Windy', - 'temp': u'20\xb0C', - 'icon': u'icon.png'}, - u'New weather': {'conditions': u'Snowing', - 'icon': u'icon.png', - 'temp': u'-8\xb0C'}} + expected_values = {u'USCA0638': {'conditions': u'Snowing', + 'temp': u'-8\xb0C', + 'icon': u'icon.png'}, + u'ARCA0023': {'conditions': u'Windy', + 'temp': u'20\xb0C', + 'icon': u'icon.png'}, + u'NEW123': {'conditions': u'Snowing', + 'icon': u'icon.png', + 'temp': u'-8\xb0C'}} actual_values = dict([(i, self.weather_utility.get_weather_info()[i]['weather']) for i in self.weather_utility.get_weather_info()]) @@ -129,83 +121,12 @@ def test_update_weather_info(self): self.weather_utility.update_weather_info() - expected_values = {u'Los Angeles': {'conditions': u'Snowing', - 'temp': u'-8\xb0C', - 'icon': u'icon.png'}, - u'Cordoba': {'conditions': u'Windy', - 'temp': u'20\xb0C', - 'icon': u'icon.png'}} - - actual_values = dict([(i, self.weather_utility.get_weather_info()[i]['weather']) for i in self.weather_utility.get_weather_info()]) - - self.assertEqual(actual_values, expected_values) - - # We are going to forge weather info into the utility so we simulate invalid returned values and errors - self.weather_utility.weather_info['New weather'] = {'weather': {'conditions': u'Snowing', - 'temp': u'-8\xb0C', - 'icon': u'icon.png'}} - - # We create a city with the same name but different location_id, so we can simulate - # invalid results for an existing city. In this case, we should still see old data - settings.location_ids.append(u'New weather|New weather|NEW123-invalid') - self.weather_utility.update_weather_info() - - expected_values = {u'Los Angeles': {'conditions': u'Snowing', - 'temp': u'-8\xb0C', - 'icon': u'icon.png'}, - u'Cordoba': {'conditions': u'Windy', - 'temp': u'20\xb0C', - 'icon': u'icon.png'}, - u'New weather': {'conditions': u'Snowing', - 'temp': u'-8\xb0C', - 'icon': u'icon.png'}} - - actual_values = dict([(i, self.weather_utility.get_weather_info()[i]['weather']) for i in self.weather_utility.get_weather_info()]) - - self.assertEqual(actual_values, expected_values) - - self.weather_utility.weather_info['Buenos Aires'] = {'weather': {'conditions': u'Snowing', - 'temp': u'-8\xb0C', - 'icon': u'icon.png'}} - - # If we get a urllib exception, then also keep the existing value - settings.location_ids.append(u'Buenos Aires|Buenos Aires, Argentina|ARBA0023-urllib-exception') - - self.weather_utility.update_weather_info() - - expected_values = {u'Los Angeles': {'conditions': u'Snowing', - 'temp': u'-8\xb0C', - 'icon': u'icon.png'}, - u'Cordoba': {'conditions': u'Windy', - 'temp': u'20\xb0C', - 'icon': u'icon.png'}, - u'New weather': {'conditions': u'Snowing', - 'temp': u'-8\xb0C', - 'icon': u'icon.png'}, - u'Buenos Aires': {'conditions': u'Snowing', - 'temp': u'-8\xb0C', - 'icon': u'icon.png'}} - - actual_values = dict([(i, self.weather_utility.get_weather_info()[i]['weather']) for i in self.weather_utility.get_weather_info()]) - self.assertEqual(actual_values, expected_values) - - # If we get a any exception, then also keep the existing value - settings.location_ids.append(u'Buenos Aires|Buenos Aires, Argentina|ARBA0023-exception') - - self.weather_utility.update_weather_info() - - expected_values = {u'Los Angeles': {'conditions': u'Snowing', - 'temp': u'-8\xb0C', - 'icon': u'icon.png'}, - u'Cordoba': {'conditions': u'Windy', - 'temp': u'20\xb0C', - 'icon': u'icon.png'}, - u'New weather': {'conditions': u'Snowing', - 'temp': u'-8\xb0C', - 'icon': u'icon.png'}, - u'Buenos Aires': {'conditions': u'Snowing', - 'temp': u'-8\xb0C', - 'icon': u'icon.png'}} + expected_values = {u'USCA0638': {'conditions': u'Snowing', + 'temp': u'-8\xb0C', + 'icon': u'icon.png'}, + u'ARCA0023': {'conditions': u'Windy', + 'temp': u'20\xb0C', + 'icon': u'icon.png'}} actual_values = dict([(i, self.weather_utility.get_weather_info()[i]['weather']) for i in self.weather_utility.get_weather_info()]) @@ -216,27 +137,26 @@ def test_update_weather_info(self): self.weather_utility.weather_info = {} self.weather_utility.update_weather_info() - expected_values = {u'Los Angeles': {'conditions': u'Snowing', - 'temp': u'-8\xb0F', - 'icon': u'icon.png'}, - u'Cordoba': {'conditions': u'Windy', - 'temp': u'20\xb0F', - 'icon': u'icon.png'}} + expected_values = {u'USCA0638': {'conditions': u'Snowing', + 'temp': u'-8\xb0F', + 'icon': u'icon.png'}, + u'ARCA0023': {'conditions': u'Windy', + 'temp': u'20\xb0F', + 'icon': u'icon.png'}} actual_values = dict([(i, self.weather_utility.get_weather_info()[i]['weather']) for i in self.weather_utility.get_weather_info()]) self.assertEqual(actual_values, expected_values) def test_get_city(self): - city = self.weather_utility.get_city('Cordoba') - expected_city = {'id': u'Cordoba', - 'location_id': u'ARCA0023', + city = self.weather_utility.get_city('ARCA0023') + expected_city = {'location_id': u'ARCA0023', 'name': u'Cordoba, Argentina', 'type': 'testprovider'} self.assertEqual(city, expected_city) - city = self.weather_utility.get_city('Cordoba2') + city = self.weather_utility.get_city('ARCA00232') self.assertEqual(city, expected_city) @@ -245,28 +165,25 @@ def test_get_current_city(self): COOKIE_KEY = 'collective.weather.current_city' city = self.weather_utility.get_current_city() - expected_city = {'id': u'Cordoba', - 'location_id': u'ARCA0023', + expected_city = {'location_id': u'ARCA0023', 'name': u'Cordoba, Argentina', 'type': 'testprovider'} self.assertEqual(city, expected_city) - self.request.cookies[COOKIE_KEY] = 'Los Angeles' + self.request.cookies[COOKIE_KEY] = 'USCA0638' city = self.weather_utility.get_current_city() - expected_city = {'id': u'Los Angeles', - 'location_id': u'USCA0638', + expected_city = {'location_id': u'USCA0638', 'name': u'Los Angeles, California', 'type': 'testprovider'} self.assertEqual(city, expected_city) - self.request.cookies[COOKIE_KEY] = 'Los Angeles5' + self.request.cookies[COOKIE_KEY] = 'USCA06385' city = self.weather_utility.get_current_city() - expected_city = {'id': u'Cordoba', - 'location_id': u'ARCA0023', + expected_city = {'location_id': u'ARCA0023', 'name': u'Cordoba, Argentina', 'type': 'testprovider'} @@ -280,20 +197,20 @@ def test_update_specific_weather_info(self): # What we first do is replace all locations created from the registry, with new values old_locations = deepcopy(settings.location_ids) settings.location_ids = [ - u'Cordoba|Cordoba, Argentina|ARCA0024', - u'Los Angeles|Los Angeles, California|USCA0639', - u'New weather|New weather|NEW124', - u'New weather2|New weather|NEW125', + u'ARCA0024|Cordoba, Argentina', + u'USCA0639|Los Angeles, California', + u'NEW124|New weather', + u'NEW125|New weather2', ] # And reset any existing weather info self.weather_utility.weather_info = {} # Update weather info for Los Angeles - self.weather_utility.update_weather_info('Los Angeles') + self.weather_utility.update_weather_info('USCA0639') - expected_values = {u'Los Angeles': {'conditions': u'Snowing', - 'temp': u'-10\xb0C', - 'icon': u'icon.png'}} + expected_values = {u'USCA0639': {'conditions': u'Snowing', + 'temp': u'-10\xb0C', + 'icon': u'icon.png'}} wi = self.weather_utility.get_weather_info() @@ -302,14 +219,14 @@ def test_update_specific_weather_info(self): self.assertEqual(actual_values, expected_values) # Now, for New weather2 - self.weather_utility.update_weather_info('New weather2') + self.weather_utility.update_weather_info('NEW125') - expected_values = {u'Los Angeles': {'conditions': u'Snowing', - 'temp': u'-10\xb0C', - 'icon': u'icon.png'}, - u'New weather2': {'conditions': u'Snowing', - 'icon': u'icon.png', - 'temp': u'-20\xb0C'}} + expected_values = {u'USCA0639': {'conditions': u'Snowing', + 'temp': u'-10\xb0C', + 'icon': u'icon.png'}, + u'NEW125': {'conditions': u'Snowing', + 'icon': u'icon.png', + 'temp': u'-20\xb0C'}} wi = self.weather_utility.get_weather_info() @@ -320,15 +237,15 @@ def test_update_specific_weather_info(self): # Now, if i ask for an invalid city, i should get for the first one self.weather_utility.update_weather_info('New weather3') - expected_values = {u'Cordoba': {'conditions': u'Windy', - 'temp': u'10\xb0C', - 'icon': u'icon.png'}, - u'Los Angeles': {'conditions': u'Snowing', - 'temp': u'-10\xb0C', - 'icon': u'icon.png'}, - u'New weather2': {'conditions': u'Snowing', - 'icon': u'icon.png', - 'temp': u'-20\xb0C'}} + expected_values = {u'ARCA0024': {'conditions': u'Windy', + 'temp': u'10\xb0C', + 'icon': u'icon.png'}, + u'USCA0639': {'conditions': u'Snowing', + 'temp': u'-10\xb0C', + 'icon': u'icon.png'}, + u'NEW125': {'conditions': u'Snowing', + 'icon': u'icon.png', + 'temp': u'-20\xb0C'}} wi = self.weather_utility.get_weather_info() @@ -337,20 +254,20 @@ def test_update_specific_weather_info(self): self.assertEqual(actual_values, expected_values) # Finally, ask for New weather - self.weather_utility.update_weather_info('New weather') - - expected_values = {u'Cordoba': {'conditions': u'Windy', - 'temp': u'10\xb0C', - 'icon': u'icon.png'}, - u'Los Angeles': {'conditions': u'Snowing', - 'temp': u'-10\xb0C', - 'icon': u'icon.png'}, - u'New weather': {'conditions': u'Snowing', - 'icon': u'icon.png', - 'temp': u'-20\xb0C'}, - u'New weather2': {'conditions': u'Snowing', - 'icon': u'icon.png', - 'temp': u'-20\xb0C'}} + self.weather_utility.update_weather_info('NEW124') + + expected_values = {u'ARCA0024': {'conditions': u'Windy', + 'temp': u'10\xb0C', + 'icon': u'icon.png'}, + u'USCA0639': {'conditions': u'Snowing', + 'temp': u'-10\xb0C', + 'icon': u'icon.png'}, + u'NEW124': {'conditions': u'Snowing', + 'icon': u'icon.png', + 'temp': u'-20\xb0C'}, + u'NEW125': {'conditions': u'Snowing', + 'icon': u'icon.png', + 'temp': u'-20\xb0C'}} wi = self.weather_utility.get_weather_info() diff --git a/src/collective/weather/top_bar_weather.pt b/src/collective/weather/top_bar_weather.pt index fa29841..c6e2651 100644 --- a/src/collective/weather/top_bar_weather.pt +++ b/src/collective/weather/top_bar_weather.pt @@ -13,8 +13,8 @@