Break PlanItLocation out from org model

- Add helper on PlanItUser model to get location
- Fix tests with appropriate mock
- Only update location info when created
- Implement OrganizationSerializer create/update methods
- Remove uniqueness constraint on location.api_city_id

Removing the constraint was the lesser of two evils while
we're interating quickly on the definition of this object.
See:
- encode/django-rest-framework#2996
- https://medium.com/django-rest-framework/dealing-with-unique-constraints-in-nested-serializers-dade33b831d9
Handling nested serializers with uniqueness constraints
requires overriding the default validators generated for the
entire serializer which is super inelegant. No harm to us for
now if what are essentially duplicate planitlocation objects
get created somehow.

src/django/planit/settings.py

@@ -84,6 +84,7 @@
     'watchman',
     'rest_framework',
     'rest_framework.authtoken',
+    'rest_framework_gis',
     'bootstrap3',
 
     # local apps

src/django/planit_data/tests/test_views.py

@@ -6,6 +6,7 @@
 from rest_framework import status
 
 from planit_data.models import Indicator, Concern
+from users.models import PlanItLocation, PlanItOrganization
 
 
 class PlanitApiTestCase(APITestCase):
@@ -48,7 +49,9 @@ def test_concern_list_nonauth(self):
     def test_concern_detail(self, calculate_mock):
         calculate_mock.return_value = 5.3
 
-        self.user.api_city_id = 14
+        location = PlanItLocation.objects.create(api_city_id=14)
+        org = PlanItOrganization.objects.create(name='Test', location=location)
+        self.user.organizations.add(org)
 
         indicator = Indicator.objects.create(name='Foobar')
         concern = Concern.objects.create(indicator=indicator,
@@ -62,7 +65,7 @@ def test_concern_detail(self, calculate_mock):
         self.assertDictEqual(response.data,
                              {'id': concern.id, 'indicator': 'Foobar', 'tagline': 'test',
                               'is_relative': True, 'value': 5.3})
-        calculate_mock.assert_called_with(14)
+        calculate_mock.assert_called_with(self.user.get_current_location().api_city_id)
 
     def test_concern_detail_invalid(self):
         url = reverse('concern-detail', kwargs={'pk': 999})

src/django/planit_data/views.py

@@ -17,5 +17,6 @@ def retrieve(self, request, pk=None):
         concern = get_object_or_404(Concern, id=pk)
         payload = ConcernSerializer(concern).data
 
-        payload['value'] = concern.calculate(request.user.api_city_id)
+        location = request.user.get_current_location()
+        payload['value'] = concern.calculate(location.api_city_id)
         return Response(payload)

src/django/requirements.txt

@@ -1,6 +1,7 @@
 django-bootstrap3==9.0.0
 django-cors-headers==2.1.0
 djangorestframework==3.7.0
+djangorestframework-gis==0.12.0
 django-amazon-ses==0.3.0
 django-debug-toolbar==1.8
 django-extensions==1.9.1

src/django/users/migrations/0010_planitlocation.py

@@ -0,0 +1,25 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.7 on 2017-11-14 14:32
+from __future__ import unicode_literals
+
+import django.contrib.gis.db.models.fields
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('users', '0009_auto_20171108_2156'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='PlanItLocation',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('name', models.CharField(blank=True, max_length=256, null=True)),
+                ('api_city_id', models.IntegerField(null=True, unique=True)),
+                ('point', django.contrib.gis.db.models.fields.PointField(blank=True, null=True, srid=4326)),
+            ],
+        ),
+    ]

src/django/users/migrations/0011_auto_20171114_1438.py

@@ -0,0 +1,25 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.7 on 2017-11-14 14:38
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('users', '0010_planitlocation'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='planitorganization',
+            name='api_city_id',
+        ),
+        migrations.AddField(
+            model_name='planitorganization',
+            name='location',
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='users.PlanItLocation'),
+        ),
+    ]

src/django/users/migrations/0012_auto_20171114_1917.py

@@ -0,0 +1,20 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.7 on 2017-11-14 19:17
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('users', '0011_auto_20171114_1438'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='planitlocation',
+            name='api_city_id',
+            field=models.IntegerField(blank=True, null=True),
+        ),
+    ]

src/django/users/models.py

@@ -1,6 +1,7 @@
 from django.contrib.auth.base_user import AbstractBaseUser, BaseUserManager
 from django.contrib.auth.models import PermissionsMixin
 from django.contrib.gis.db import models
+from django.contrib.gis.geos import GEOSGeometry
 from django.core.mail import send_mail
 from django.utils import timezone
 
@@ -9,6 +10,28 @@
 from django.dispatch import receiver
 from rest_framework.authtoken.models import Token
 
+from climate_api.wrapper import make_token_api_request
+
+
+class PlanItLocationManager(models.Manager):
+
+    def from_api_city(self, api_city_id):
+        location, created = PlanItLocation.objects.get_or_create(api_city_id=api_city_id)
+        if created:
+            city = make_token_api_request('/api/city/{}/'.format(api_city_id))
+            location.name = city['properties']['name']
+            location.point = GEOSGeometry(str(city['geometry']))
+            location.save()
+        return location
+
+
+class PlanItLocation(models.Model):
+    name = models.CharField(max_length=256, null=True, blank=True)
+    api_city_id = models.IntegerField(null=True, blank=True)
+    point = models.PointField(srid=4326, null=True, blank=True)
+
+    objects = PlanItLocationManager()
+
 
 class PlanItOrganization(models.Model):
     """Users belong to one or more organizations."""
@@ -22,7 +45,7 @@ class PlanItOrganization(models.Model):
 
     name = models.CharField(max_length=256, blank=False, null=False, unique=True)
     units = models.CharField(max_length=16, choices=UNITS_CHOICES, default=IMPERIAL)
-    api_city_id = models.IntegerField(null=True)
+    location = models.ForeignKey(PlanItLocation, on_delete=models.SET_NULL, null=True, blank=True)
 
     def __str__(self):
         return self.name
@@ -105,6 +128,10 @@ def clean(self):
         super().clean()
         self.email = self.__class__.objects.normalize_email(self.email)
 
+    def get_current_location(self):
+        """ Return the appropriate PlanItLocation for this user."""
+        return self.organizations.select_related('location').first().location
+
     def get_full_name(self):
         """
         Return the first_name plus the last_name, with a space in between.

src/django/users/serializers.py

@@ -1,10 +1,10 @@
 from django.contrib.auth import authenticate
 from django.contrib.auth.password_validation import validate_password
 
-from rest_framework.authtoken.models import Token
 from rest_framework import serializers
+from rest_framework_gis.serializers import GeoFeatureModelSerializer
 
-from users.models import PlanItOrganization, PlanItUser
+from users.models import PlanItLocation, PlanItOrganization, PlanItUser
 
 
 class AuthTokenSerializer(serializers.Serializer):
@@ -35,14 +35,42 @@ def validate(self, attrs):
         return attrs
 
 
+class LocationSerializer(GeoFeatureModelSerializer):
+    """Serializer for organization locations."""
+
+    class Meta:
+        model = PlanItLocation
+        geo_field = 'point'
+        fields = ('name', 'api_city_id',)
+        read_only_fields = ('name', 'point',)
+
+
 class OrganizationSerializer(serializers.ModelSerializer):
     """Serializer for user organizations"""
 
-    city = serializers.IntegerField(source='api_city_id', required=False, allow_null=True)
+    location = LocationSerializer()
+
+    def create(self, validated_data):
+        location_data = validated_data.pop('location')
+        instance = PlanItOrganization.objects.create(**validated_data)
+        if location_data['api_city_id'] is not None:
+            instance.location = PlanItLocation.objects.from_api_city(location_data['api_city_id'])
+            instance.save()
+        return instance
+
+    def update(self, instance, validated_data):
+        validated_data.pop('name')
+        location_data = validated_data.pop('location')
+        for k, v in validated_data.items():
+            setattr(instance, k, v)
+        if location_data['api_city_id'] is not None:
+            instance.location = PlanItLocation.objects.from_api_city(location_data['api_city_id'])
+        instance.save()
+        return instance
 
     class Meta:
         model = PlanItOrganization
-        fields = ('id', 'name', 'city', 'units')
+        fields = ('id', 'name', 'location', 'units')
 
 
 class UserSerializer(serializers.ModelSerializer):

src/django/users/tests.py

@@ -1,6 +1,6 @@
 # User management tests
-
 import json
+from unittest import mock
 
 from django.test import TestCase
 
@@ -57,10 +57,34 @@ def test_user_created(self):
         self.assertTrue(self.client.login(username=user_data['email'],
                                           password=user_data['password1']))
 
-    def test_org_created(self):
+    @mock.patch('users.models.make_token_api_request')
+    def test_org_created(self, api_wrapper_mock):
+        api_wrapper_mock.return_value = {
+            "id": 7,
+            "type": "Feature",
+            "geometry": {
+                "type": "Point",
+                "coordinates": [
+                    -75.16379,
+                    39.95233
+                ]
+            },
+            "properties": {
+                "datasets": [
+                    "NEX-GDDP",
+                    "LOCA"
+                ],
+                "name": "Philadelphia",
+                "admin": "PA",
+                "population": 1526006,
+                "region": 11
+            }
+        }
         org_data = {
             'name': 'Test Organization',
-            'city': 1,
+            'location': {
+                'api_city_id': 7,
+            },
             'units': 'METRIC'
         }
 
@@ -71,7 +95,7 @@ def test_org_created(self):
 
         # check organization exists
         org = PlanItOrganization.objects.get(name='Test Organization')
-        self.assertEqual(org.api_city_id, org_data['city'])
+        self.assertEqual(org.location.api_city_id, org_data['location']['api_city_id'])
         self.assertEqual(org.units, org_data['units'])
 
     def test_user_passwords_must_match(self):