Skip to content

Commit

Permalink
Put NYCDB landlord info in HP action vars. Fixes #394.
Browse files Browse the repository at this point in the history
  • Loading branch information
toolness committed Nov 29, 2018
1 parent 5426f11 commit 05451a3
Show file tree
Hide file tree
Showing 7 changed files with 160 additions and 48 deletions.
3 changes: 3 additions & 0 deletions conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,3 +194,6 @@ def nycdb(db, settings):
'''

settings.NYCDB_DATABASE = 'default'

from nycdb.tests.test_models import fixtures
yield fixtures
92 changes: 90 additions & 2 deletions hpaction/build_hpactionvars.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
from typing import Dict
from enum import Enum

from users.models import JustfixUser
from onboarding.models import BOROUGH_CHOICES
from issues.models import ISSUE_AREA_CHOICES, ISSUE_CHOICES
import nycdb.models
from . import hpactionvars as hp


Expand Down Expand Up @@ -64,8 +66,87 @@ def create_complaint(area: str, description: str) -> hp.TenantComplaints:
)


def fill_landlord_info(v: hp.HPActionVariables, user: JustfixUser):
if hasattr(user, 'landlord_details'):
def nycdb_addr_to_hp_state(address: nycdb.models.Address) -> hp.LandlordAddressStateMC:
# This is kind of yucky, because NYCDB/HPD provides state information as
# all-caps two-letter strings (e.g. "NY") while the HotDocs endpoint wants
# them fully spelled out (e.g. "New York"). However, in practice it doesn't seem
# to mind if we give it the two-letter strings, so we'll just do that.
class OneOffEnum(Enum):
VALUE = address.state

return OneOffEnum.VALUE # type: ignore


def fill_landlord_info_from_contact(
v: hp.HPActionVariables,
contact: nycdb.models.Contact
) -> None:
v.landlord_address_city_te = contact.address.city
v.landlord_address_street_te = contact.address.street_name
v.landlord_address_zip_te = contact.address.zipcode
v.landlord_address_state_mc = nycdb_addr_to_hp_state(contact.address)
v.service_address_full_te = ", ".join(contact.address.lines_for_mailing)
if isinstance(contact, nycdb.models.Company):
v.landlord_entity_or_individual_mc = hp.LandlordEntityOrIndividualMC.COMPANY
else:
v.landlord_entity_or_individual_mc = hp.LandlordEntityOrIndividualMC.INDIVIDUAL

# I don't think these are actually used in rendering the form, but just in case...
v.landlord_name_first_te = contact.first_name
v.landlord_name_last_te = contact.last_name
v.landlord_entity_name_te = contact.name
v.served_person_te = contact.name


def fill_landlord_management_info_from_company(
v: hp.HPActionVariables,
mgmtco: nycdb.models.Company
) -> None:
v.management_company_address_city_te = mgmtco.address.city
v.management_company_address_street_te = mgmtco.address.street_name
v.management_company_address_zip_te = mgmtco.address.zipcode
v.management_company_address_state_mc = nycdb_addr_to_hp_state(mgmtco.address)
v.service_address_full_management_company_te = ", ".join(
mgmtco.address.lines_for_mailing)
v.served_person_management_company_te = mgmtco.name
v.management_company_name_te = mgmtco.name

# TODO: We might actually want to fill this out even if we don't find
# a management company, as this could at least generate the required
# forms. Need to find this out.
v.management_company_to_be_sued_tf = True
v.mgmt_co_is_party_tf = True
v.service_already_completed_mgmt_co_tf = False
v.hpd_service_mgmt_co_mc = hp.HPDServiceMgmtCoMC.MAIL
v.service_method_mgmt_co_mc = hp.ServiceMethodMgmtCoMC.MAIL


def fill_landlord_info_from_bbl(v: hp.HPActionVariables, pad_bbl: str) -> bool:
landlord_found = False
contact = nycdb.models.get_landlord(pad_bbl)
if contact:
landlord_found = True
fill_landlord_info_from_contact(v, contact)
mgmtco = nycdb.models.get_management_company(pad_bbl)
if mgmtco:
fill_landlord_management_info_from_company(v, mgmtco)
return landlord_found


def fill_landlord_info(v: hp.HPActionVariables, user: JustfixUser) -> None:
landlord_found = False

v.service_already_completed_landlord_tf = False
v.hpd_service_landlord_mc = hp.HPDServiceLandlordMC.MAIL
v.service_method_mc = hp.ServiceMethodMC.MAIL
v.landlord_is_party_tf = True

if hasattr(user, 'onboarding_info'):
pad_bbl: str = user.onboarding_info.pad_bbl
if pad_bbl:
landlord_found = fill_landlord_info_from_bbl(v, pad_bbl)

if not landlord_found and hasattr(user, 'landlord_details'):
ld = user.landlord_details
v.landlord_entity_name_te = ld.name
v.served_person_te = ld.name
Expand All @@ -75,7 +156,13 @@ def fill_landlord_info(v: hp.HPActionVariables, user: JustfixUser):
def user_to_hpactionvars(user: JustfixUser) -> hp.HPActionVariables:
v = hp.HPActionVariables()

# TODO: The HP Action form actually has a field for home phone
# and a separate one for work; it's unclear which one the
# user may have provided, but we'll assume it's home for now.
v.tenant_phone_home_te = user.formatted_phone_number()

v.server_name_full_te = user.full_name
v.server_name_full_management_company_te = user.full_name
v.server_name_full_hpd_te = user.full_name
v.tenant_name_first_te = user.first_name
v.tenant_name_last_te = user.last_name
Expand Down Expand Up @@ -128,6 +215,7 @@ def user_to_hpactionvars(user: JustfixUser) -> hp.HPActionVariables:
full_addr = ', '.join(oinfo.address_lines_for_mailing)
v.server_address_full_hpd_te = full_addr
v.server_address_full_te = full_addr
v.server_address_full_management_company_te = full_addr

for issue in user.issues.all():
desc = ISSUE_CHOICES.get_label(issue.value)
Expand Down
23 changes: 22 additions & 1 deletion hpaction/tests/test_build_hpactionvars.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,29 @@ def test_user_to_hpactionvars_populates_issues(db):
v.to_answer_set()


def test_user_to_hpactionvars_populates_landlord_info(db):
def test_user_to_hpactionvars_populates_basic_landlord_info(db):
ld = LandlordDetailsFactory(name="Landlordo Calrissian")
v = user_to_hpactionvars(ld.user)
assert v.landlord_entity_name_te == "Landlordo Calrissian"
v.to_answer_set()


def test_user_to_hpactionvars_populates_med_ll_info_from_nycdb(db, nycdb):
med = nycdb.load_hpd_registration('medium-landlord.json')
oinfo = OnboardingInfoFactory(pad_bbl=med.pad_bbl)
v = user_to_hpactionvars(oinfo.user)
assert v.landlord_entity_name_te == "LANDLORDO CALRISSIAN"
assert v.landlord_entity_or_individual_mc == hp.LandlordEntityOrIndividualMC.COMPANY
llstate = v.landlord_address_state_mc
assert llstate and llstate.value == "NJ"
assert v.management_company_name_te == "FUNKY APARTMENT MANAGEMENT"
v.to_answer_set()


def test_user_to_hpactionvars_populates_tiny_ll_info_from_nycdb(db, nycdb):
med = nycdb.load_hpd_registration('tiny-landlord.json')
oinfo = OnboardingInfoFactory(pad_bbl=med.pad_bbl)
v = user_to_hpactionvars(oinfo.user)
assert v.landlord_entity_name_te == "BOOP JONES"
assert v.landlord_entity_or_individual_mc == hp.LandlordEntityOrIndividualMC.INDIVIDUAL
v.to_answer_set()
3 changes: 1 addition & 2 deletions loc/tests/test_landlord_lookup.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
from project.tests.util import simplepatch
from loc.landlord_lookup import (
lookup_landlord, _extract_landlord_info, LandlordInfo, _lookup_landlord_via_nycdb)
from nycdb.tests.test_models import fixtures as nycdb_fixtures


MY_DIR = Path(__file__).parent.resolve()
Expand Down Expand Up @@ -68,7 +67,7 @@ def test_lookup_landlord_works(requests_mock):


def test_lookup_landlord_via_nycdb_works(nycdb):
reg = nycdb_fixtures.load_hpd_registration('tiny-landlord.json')
reg = nycdb.load_hpd_registration('tiny-landlord.json')
ll = _lookup_landlord_via_nycdb(reg.pad_bbl)
assert isinstance(ll, LandlordInfo)
assert ll.name == "BOOP JONES"
Expand Down
75 changes: 34 additions & 41 deletions nycdb/models.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import logging
from typing import Optional, NamedTuple, List, Union, Callable, TypeVar
from typing import Optional, NamedTuple, List, Union
from django.db.utils import DatabaseError
from dataclasses import dataclass
from django.conf import settings
Expand Down Expand Up @@ -30,12 +30,8 @@ def lines_for_mailing(self) -> List[str]:


@dataclass
class BaseContact:
class Individual:
address: Address


@dataclass
class Individual(BaseContact):
first_name: str
last_name: str

Expand All @@ -45,7 +41,8 @@ def name(self) -> str:


@dataclass
class Company(BaseContact):
class Company:
address: Address
name: str


Expand Down Expand Up @@ -97,13 +94,14 @@ def _get_company_landlord(self) -> Optional[Company]:
]
if owners:
head_officer_addresses = [
c.address for c in self.contact_list
(c.firstname, c.lastname, c.address) for c in self.contact_list
if c.type == HPDContact.HEAD_OFFICER and c.address
]
if head_officer_addresses:
first_name, last_name, address = head_officer_addresses[0]
return Company(
name=owners[0],
address=head_officer_addresses[0]
name=f"{first_name} {last_name}",
address=address
)
return None

Expand Down Expand Up @@ -193,48 +191,43 @@ def address(self) -> Optional[Address]:
)


T = TypeVar('T')


def fault_tolerant(f: Callable[[str], Optional[T]]) -> Callable[[str], Optional[T]]:
"""
Decorator that facilitates the creation of functions that are fault-tolerant
with respect to NYCDB, returning None if the NYCDB connection fails or
is disabled entirely.
"""

def wrapped(pad_bbl: str) -> Optional[T]:
if not settings.NYCDB_DATABASE:
return None
try:
return f(pad_bbl)
except (DatabaseError, Exception):
# TODO: Once we have more confidence in the underlying code,
# we should remove the above 'Exception' and only catch
# 'DatabaseError'.
logger.exception(f'Error while retrieving data from NYCDB')
return None
wrapped.__name__ = f.__name__
return wrapped


@fault_tolerant
def get_landlord(pad_bbl: str) -> Optional[Contact]:
"""
Fault-tolerant retriever of landlord information that assumes
the NYCDB connection is unreliable, or disabled entirely.
"""

reg = HPDRegistration.objects.from_pad_bbl(pad_bbl).first()
return reg.get_landlord() if reg else None
if not settings.NYCDB_DATABASE:
return None
try:
reg = HPDRegistration.objects.from_pad_bbl(pad_bbl).first()
return reg.get_landlord() if reg else None
except (DatabaseError, Exception):
# TODO: Once we have more confidence in the underlying code,
# we should remove the above 'Exception' and only catch
# 'DatabaseError'.
logger.exception(f'Error while retrieving data from NYCDB')
return None


@fault_tolerant
def get_management_company(pad_bbl: str) -> Optional[Company]:
"""
Fault-tolerant retriever of management company information that assumes
the NYCDB connection is unreliable, or disabled entirely.
"""

reg = HPDRegistration.objects.from_pad_bbl(pad_bbl).first()
return reg.get_management_company() if reg else None
# Yes, this contains a ton of duplicate logic from get_landlord().
# Ideally we'd use a decorator but apparently mypy has major
# problems with it.

if not settings.NYCDB_DATABASE:
return None
try:
reg = HPDRegistration.objects.from_pad_bbl(pad_bbl).first()
return reg.get_management_company() if reg else None
except (DatabaseError, Exception):
# TODO: Once we have more confidence in the underlying code,
# we should remove the above 'Exception' and only catch
# 'DatabaseError'.
logger.exception(f'Error while retrieving data from NYCDB')
return None
10 changes: 9 additions & 1 deletion nycdb/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def test_medium_landlord_works(nycdb):

ll = reg.get_landlord()
assert isinstance(ll, Company)
assert ll.name == "ULTRA DEVELOPERS, LLC"
assert ll.name == "LANDLORDO CALRISSIAN"
assert ll.address.lines_for_mailing == [
'9 BEAN CENTER DRIVE #40',
'FUNKYPLACE, NJ 07099'
Expand Down Expand Up @@ -105,6 +105,14 @@ def test_it_returns_none_if_nycdb_is_disabled(self):
def test_it_returns_none_if_pad_bbl_does_not_exist(self, nycdb):
assert get_management_company('1234567890') is None

def test_it_returns_none_on_db_error(self, nycdb):
with patch.object(HPDRegistration.objects, 'from_pad_bbl') as fpbblmock:
fpbblmock.side_effect = DatabaseError()
with patch('nycdb.models.logger.exception') as loggermock:
assert get_management_company('1234567890') is None
loggermock.assert_called_once_with(
f'Error while retrieving data from NYCDB')

def test_it_returns_company_on_success(self, nycdb):
medium = fixtures.load_hpd_registration("medium-landlord.json")
mgmtco = get_management_company(medium.pad_bbl)
Expand Down
2 changes: 1 addition & 1 deletion nycdb/tests/test_nycdb_lookup_landlord.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def test_it_works_with_tiny_landlord(nycdb):

def test_it_works_with_medium_landlord(nycdb):
output = get_output("medium-landlord.json")
assert ' ULTRA DEVELOPERS, LLC' in output
assert ' LANDLORDO CALRISSIAN' in output
assert ' FUNKY APARTMENT MANAGEMENT' in output


Expand Down

0 comments on commit 05451a3

Please sign in to comment.