Use PostGIS/GeoDjango to see if lnglat is in NYC. (#2166)

Fixes #2152.  Our test suite should actually take a tiny bit shorter to run now, too.

It should be noted that in order for this to work on local dev setups (outside of just running the test suite), you'll need to run `manage.py loadfindhelpdata`.  This is mentioned in the README but I wanted to call it out here.  Also, it's automatically run every time we push a deployment (the command is idempotent and will update any data that needs updating).

findhelp/models.py

@@ -1,6 +1,6 @@
 import itertools
 from project.util.mailing_address import STATE_KWARGS
-from typing import Union, Iterator, Optional
+from typing import Tuple, Union, Iterator, Optional
 from django.contrib.gis.db import models
 from django.contrib.gis.geos import GEOSGeometry, Point, Polygon, MultiPolygon
 from django.contrib.gis.db.models.functions import Distance
@@ -66,6 +66,10 @@ def __str__(self):
         return self.name
 
 
+def is_lnglat_in_nyc(lnglat: Tuple[float, float]) -> bool:
+    return Borough.objects.filter(geom__contains=Point(*lnglat)).exists()
+
+
 class Neighborhood(models.Model):
     class Meta:
         ordering = ["name", "county"]

findhelp/tests/factories.py

@@ -16,6 +16,12 @@
 POLY_2 = Polygon.from_bbox((1, 1, 2, 2))
 
 
+class LngLats:
+    BROOKLYN_HEIGHTS = (-73.9943, 40.6977)
+    ALBANY = (-73.755, 42.6512)
+    YONKERS = (-73.8987, 40.9312)
+
+
 class CountyFactory(factory.django.DjangoModelFactory):
     class Meta:
         model = County

findhelp/tests/test_loadfindhelpdata.py

@@ -1,6 +1,7 @@
 from django.core.management import call_command
 
-from findhelp.models import Zipcode
+from findhelp.models import Zipcode, is_lnglat_in_nyc
+from .factories import LngLats
 
 
 def test_it_works(db):
@@ -10,3 +11,13 @@ def test_it_works(db):
     # Because our temporary 'loadfindhelpcbos' command depends
     # on 'loadfindhelpdata', we'll just smoke test it now too.
     call_command("loadfindhelpcbos")
+
+    # This is a bit annoying; is_lnglat_in_nyc() depends on
+    # the borough data being loaded, which is what has just
+    # been done.  We don't want to have to load it in a
+    # separate test because that would make our test suite
+    # take seconds longer to run, which we'd rather not have,
+    # so we're just going to test the function here.
+    assert is_lnglat_in_nyc(LngLats.BROOKLYN_HEIGHTS) is True
+    assert is_lnglat_in_nyc(LngLats.ALBANY) is False
+    assert is_lnglat_in_nyc(LngLats.YONKERS) is False

norent/schema.py

@@ -24,9 +24,9 @@
     get_scaffolding,
     update_scaffolding,
     purge_scaffolding,
-    is_lnglat_in_nyc,
     GraphQlOnboardingScaffolding,
 )
+from findhelp.models import is_lnglat_in_nyc
 from loc.models import LandlordDetails
 from . import forms, models, letter_sending
 

norent/tests/test_schema.py

@@ -230,7 +230,10 @@ def test_it_validates_addresses(self, settings, requests_mock):
             "aptNumber": "2",
         }
 
-    def test_it_errors_on_nyc_addresses(self, settings, requests_mock):
+    def test_it_errors_on_nyc_addresses(self, settings, requests_mock, monkeypatch):
+        from norent import schema
+
+        monkeypatch.setattr(schema, "is_lnglat_in_nyc", lambda point: True)
         settings.MAPBOX_ACCESS_TOKEN = "blah"
         self.set_prior_info()
         mock_brl_results("150 court st, Brooklyn, NY 12345", requests_mock)

onboarding/scaffolding.py

@@ -1,14 +1,11 @@
-import json
-from pathlib import Path
 from project.util.rename_dict_keys import with_keys_renamed
 from project.util.session_mutation import SessionFormMutation
 from typing import Any, Dict, Optional, Tuple
-from django.contrib.gis.geos import GEOSGeometry, Point
 import graphene
 from graphql import ResolveInfo
 import pydantic
 
-from findhelp.models import union_geometries
+from findhelp.models import is_lnglat_in_nyc
 
 
 # This should change whenever our scaffolding model's fields change in a
@@ -34,10 +31,6 @@
     "the bronx",
 ]
 
-BOROUGH_BOUNDS_PATH = Path("findhelp") / "data" / "Borough-Boundaries.geojson"
-
-_nyc_bounds: Optional[GEOSGeometry] = None
-
 
 def is_city_name_in_nyc(city: str) -> bool:
     parts = city.split("/")
@@ -117,21 +110,6 @@ def is_zip_code_in_la(self) -> Optional[bool]:
         return is_zip_code_in_la(self.zip_code)
 
 
-def is_lnglat_in_nyc(lnglat: Tuple[float, float]) -> bool:
-    global _nyc_bounds
-
-    if _nyc_bounds is None:
-        # TODO: Now that findhelp is always enabled and we have access to PostGIS in
-        # production, we should just delegate this to the database to figure out. It
-        # will also save memory in our server process.
-        bbounds = json.loads(BOROUGH_BOUNDS_PATH.read_text())
-        _nyc_bounds = union_geometries(
-            GEOSGeometry(json.dumps(feature["geometry"])) for feature in bbounds["features"]
-        )
-        assert _nyc_bounds is not None
-    return _nyc_bounds.contains(Point(*lnglat))
-
-
 class GraphQlOnboardingScaffolding(graphene.ObjectType):
     """
     Represents the public fields of our Onboarding scaffolding, as a GraphQL type.

onboarding/tests/test_scaffolding.py

@@ -2,6 +2,21 @@
 
 from onboarding.scaffolding import OnboardingScaffolding, get_scaffolding, update_scaffolding
 from onboarding.schema import session_key_for_step
+from findhelp.tests.factories import LngLats
+
+
+@pytest.fixture
+def fake_is_lnglat_in_nyc(monkeypatch):
+    from onboarding import scaffolding
+
+    def is_lnglat_in_nyc(point):
+        return {
+            LngLats.BROOKLYN_HEIGHTS: True,
+            LngLats.ALBANY: False,
+            LngLats.YONKERS: False,
+        }[point]
+
+    monkeypatch.setattr(scaffolding, "is_lnglat_in_nyc", is_lnglat_in_nyc)
 
 
 @pytest.mark.parametrize(
@@ -19,14 +34,16 @@
         (OnboardingScaffolding(city="South ozone park./Queens", state="NY"), True),
         (OnboardingScaffolding(city="blarg / flarg", state="NY"), False),
         (
-            OnboardingScaffolding(city="brooklyn heights", state="NY", lnglat=(-73.9943, 40.6977)),
+            OnboardingScaffolding(
+                city="brooklyn heights", state="NY", lnglat=LngLats.BROOKLYN_HEIGHTS
+            ),
             True,
         ),
-        (OnboardingScaffolding(city="Albany", state="NY", lnglat=(-73.755, 42.6512)), False),
-        (OnboardingScaffolding(city="Yonkers", state="NY", lnglat=(-73.8987, 40.9312)), False),
+        (OnboardingScaffolding(city="Albany", state="NY", lnglat=LngLats.ALBANY), False),
+        (OnboardingScaffolding(city="Yonkers", state="NY", lnglat=LngLats.YONKERS), False),
     ],
 )
-def test_is_city_in_nyc_works(scaffolding, expected):
+def test_is_city_in_nyc_works(scaffolding, fake_is_lnglat_in_nyc, expected):
     assert scaffolding.is_city_in_nyc() is expected