Skip to content

Commit

Permalink
Add support for Puerto Rico (#1362)
Browse files Browse the repository at this point in the history
This adds NoRent support for cities in Puerto Rico (which weren't working because Mapbox doesn't return PR results in the same way that it does for other U.S. states).

I verified that PR addresses work by going through the NoRent letter builder using the address of San Juan's city hall at 153 Calle San Francisco, San Juan Puerto  Rico 00901, and everything worked without errors or warnings.
  • Loading branch information
toolness authored May 3, 2020
1 parent dc2d8be commit e740871
Show file tree
Hide file tree
Showing 6 changed files with 89 additions and 8 deletions.
24 changes: 20 additions & 4 deletions frontend/lib/forms/mapbox/common.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
import {
USStateChoice,
isUSStateChoice,
} from "../../../../common-data/us-state-choices";

/**
* These are forward geocoding search results as documented in:
*
Expand Down Expand Up @@ -43,6 +48,8 @@ export type MapboxStateInfo = {

const MAPBOX_PLACES_URL = "https://api.mapbox.com/geocoding/v5/mapbox.places";

const MAPBOX_STATE_SHORT_CODE_RE = /^US-([A-Z][A-Z])$/;

type MapboxSearchOptions = {
access_token: string;
country: "US";
Expand All @@ -68,14 +75,23 @@ function mapboxSearchOptionsToURLSearchParams(
});
}

function stateCodeFromShortCode(shortCode?: string): USStateChoice | null {
if (shortCode === "pr") return "PR";
const match = (shortCode || "").match(MAPBOX_STATE_SHORT_CODE_RE);
const state = match ? match[1] : "";
if (isUSStateChoice(state)) {
return state;
}
return null;
}

export function getMapboxStateInfo(
feature: MapboxFeature
): MapboxStateInfo | null {
const SHORT_CODE_RE = /^US-([A-Z][A-Z])$/;
for (let context of feature.context) {
const match = (context.short_code || "").match(SHORT_CODE_RE);
if (match && context.text) {
return { stateCode: match[1], stateName: context.text };
const stateCode = stateCodeFromShortCode(context.short_code);
if (stateCode && context.text) {
return { stateCode, stateName: context.text };
}
}
return null;
Expand Down
9 changes: 8 additions & 1 deletion frontend/lib/forms/mapbox/tests/common.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { getMapboxStateInfo, createMapboxPlacesURL } from "../common";
import { BROOKLYN_MAPBOX_FEATURE } from "./data";
import { BROOKLYN_MAPBOX_FEATURE, SAN_JUAN_MAPBOX_FEATURE } from "./data";

describe("getMapboxStateInfo", () => {
it("returns state info when state is found", () => {
Expand All @@ -9,6 +9,13 @@ describe("getMapboxStateInfo", () => {
});
});

it("works with puerto rico", () => {
expect(getMapboxStateInfo(SAN_JUAN_MAPBOX_FEATURE)).toEqual({
stateCode: "PR",
stateName: "Puerto Rico",
});
});

it("returns null when no state info was found", () => {
expect(getMapboxStateInfo({ context: [] } as any)).toBe(null);
});
Expand Down
3 changes: 3 additions & 0 deletions frontend/lib/forms/mapbox/tests/data.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import _BROOKLYN from "./brooklyn.json";
import _SAN_JUAN from "./san-juan.json";
import { MapboxFeature, MapboxResults } from "../common.js";

export const BROOKLYN_MAPBOX_FEATURE = _BROOKLYN as MapboxFeature;

export const SAN_JUAN_MAPBOX_FEATURE = _SAN_JUAN as MapboxFeature;

export const BROOKLYN_MAPBOX_RESULTS: MapboxResults = {
type: "FeatureCollection",
query: ["brooklyn"],
Expand Down
37 changes: 37 additions & 0 deletions frontend/lib/forms/mapbox/tests/san-juan.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
{
"id": "place.11752167710103880",
"type": "Feature",
"place_type": ["place"],
"relevance": 1,
"properties": {
"wikidata": "Q41211"
},
"text_en": "San Juan",
"language_en": "en",
"place_name_en": "San Juan, Puerto Rico",
"text": "San Juan",
"language": "en",
"place_name": "San Juan, Puerto Rico",
"bbox": [
-66.1250540194812,
18.3019210164431,
-65.9914915322709,
18.4718989803279
],
"center": [-66.0571, 18.3744],
"geometry": {
"type": "Point",
"coordinates": [-66.0571, 18.3744]
},
"context": [
{
"id": "country.10177928452498950",
"short_code": "pr",
"wikidata": "Q1183",
"text_en": "Puerto Rico",
"language_en": "en",
"text": "Puerto Rico",
"language": "en"
}
]
}
19 changes: 16 additions & 3 deletions project/mapbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,16 +136,29 @@ def get_mapbox_street_addr(feature: MapboxFeature) -> str:
return feature.text


def get_state_from_short_code(short_code: Optional[str]) -> Optional[str]:
'''
Given a Mapbox short code, returns the state it corresponds to.
'''

if short_code == "pr":
return "PR"
match = re.match(MAPBOX_STATE_SHORT_CODE_RE, short_code or '')
if match:
return match[1]
return None


def get_mapbox_state(feature: MapboxFeature) -> Optional[str]:
'''
Returns the two-letter state code for the given Mapbox Feature, if
one exists.
'''

for context in feature.context:
match = re.match(MAPBOX_STATE_SHORT_CODE_RE, context.short_code or '')
if match:
return match[1]
state = get_state_from_short_code(context.short_code)
if state:
return state
return None


Expand Down
5 changes: 5 additions & 0 deletions project/tests/test_mapbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@

BROOKLYN_FEATURE_JSON = json.loads((JSON_DIR / 'brooklyn.json').read_text())
BROOKLYN_FEATURE = MapboxFeature(**BROOKLYN_FEATURE_JSON)
SAN_JUAN_FEATURE_JSON = json.loads((JSON_DIR / 'san-juan.json').read_text())
SAN_JUAN_FEATURE = MapboxFeature(**SAN_JUAN_FEATURE_JSON)
BRL_FEATURE_JSON = json.loads((JSON_DIR / 'brl.json').read_text())
BRL_FEATURE = MapboxFeature(**BRL_FEATURE_JSON)
BRL_RESULTS_JSON = {
Expand Down Expand Up @@ -68,6 +70,9 @@ def test_it_returns_none_on_no_match(self):
def test_it_returns_state_on_match(self):
assert get_mapbox_state(BROOKLYN_FEATURE) == "NY"

def test_it_works_with_puerto_rico(self):
assert get_mapbox_state(SAN_JUAN_FEATURE) == "PR"


class TestGetMapboxZipCode:
def test_it_returns_none_on_no_match(self):
Expand Down

0 comments on commit e740871

Please sign in to comment.