From 9ee572b92f3b35ef0a26c97c8b6751ca434077df Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Fri, 30 Nov 2018 04:23:14 -0500 Subject: [PATCH] Include detailed landlord information from NYCDB in HP Action packet (#401) * Factor out a fill_landlord_info() helper. * Add pad_bbl to OnboardingInfo model. * Add nycdb.models.get_management_company(). * Put NYCDB landlord info in HP action vars. Fixes #394. * Fix street address, explicitly set sue_for_harassment_tf. --- conftest.py | 3 + hpaction/build_hpactionvars.py | 103 +++++++++++++++++- hpaction/tests/test_build_hpactionvars.py | 25 ++++- loc/tests/test_landlord_lookup.py | 3 +- nycdb/models.py | 51 ++++++--- nycdb/tests/test_models.py | 27 ++++- nycdb/tests/test_nycdb_lookup_landlord.py | 2 +- .../migrations/0006_onboardinginfo_pad_bbl.py | 18 +++ onboarding/models.py | 13 ++- onboarding/tests/test_models.py | 4 +- 10 files changed, 222 insertions(+), 27 deletions(-) create mode 100644 onboarding/migrations/0006_onboardinginfo_pad_bbl.py diff --git a/conftest.py b/conftest.py index ee754d2a2..0ac3f01ef 100644 --- a/conftest.py +++ b/conftest.py @@ -194,3 +194,6 @@ def nycdb(db, settings): ''' settings.NYCDB_DATABASE = 'default' + + from nycdb.tests.test_models import fixtures + yield fixtures diff --git a/hpaction/build_hpactionvars.py b/hpaction/build_hpactionvars.py index 8dfd613bb..ee7b35fea 100644 --- a/hpaction/build_hpactionvars.py +++ b/hpaction/build_hpactionvars.py @@ -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 @@ -64,10 +66,103 @@ def create_complaint(area: str, description: str) -> hp.TenantComplaints: ) +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.first_line + 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.first_line + 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 + v.service_address_full_te = ld.address + + 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 @@ -97,6 +192,7 @@ def user_to_hpactionvars(user: JustfixUser) -> hp.HPActionVariables: ] v.sue_for_repairs_tf = True v.request_fee_waiver_tf = True + v.sue_for_harassment_tf = False # We're only serving New Yorkers at the moment... v.tenant_address_state_mc = hp.TenantAddressStateMC.NEW_YORK @@ -105,11 +201,7 @@ def user_to_hpactionvars(user: JustfixUser) -> hp.HPActionVariables: # will generate the HPD inspection forms. v.problem_is_urgent_tf = False - if hasattr(user, 'landlord_details'): - ld = user.landlord_details - v.landlord_entity_name_te = ld.name - v.served_person_te = ld.name - v.service_address_full_te = ld.address + fill_landlord_info(v, user) if hasattr(user, 'onboarding_info'): oinfo = user.onboarding_info @@ -124,6 +216,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) diff --git a/hpaction/tests/test_build_hpactionvars.py b/hpaction/tests/test_build_hpactionvars.py index dd66d8c73..7ae93c91d 100644 --- a/hpaction/tests/test_build_hpactionvars.py +++ b/hpaction/tests/test_build_hpactionvars.py @@ -49,8 +49,31 @@ 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 + assert v.landlord_address_street_te == "9 BEAN CENTER DRIVE #40" + llstate = v.landlord_address_state_mc + assert llstate and llstate.value == "NJ" + assert v.management_company_name_te == "FUNKY APARTMENT MANAGEMENT" + assert v.management_company_address_street_te == "900 EAST 25TH STREET #2" + 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() diff --git a/loc/tests/test_landlord_lookup.py b/loc/tests/test_landlord_lookup.py index 7a5359c9c..38b42b296 100644 --- a/loc/tests/test_landlord_lookup.py +++ b/loc/tests/test_landlord_lookup.py @@ -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() @@ -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" diff --git a/nycdb/models.py b/nycdb/models.py index f14e44adf..a224642a5 100644 --- a/nycdb/models.py +++ b/nycdb/models.py @@ -20,22 +20,22 @@ class Address(NamedTuple): @property def lines_for_mailing(self) -> List[str]: - first_line = f"{self.house_number} {self.street_name}" - if self.apartment: - first_line += f" #{self.apartment}" return [ - first_line, + self.first_line, f"{self.city}, {self.state} {self.zipcode}" ] - -@dataclass -class BaseContact: - address: Address + @property + def first_line(self) -> str: + first_line = f"{self.house_number} {self.street_name}" + if self.apartment: + first_line += f" #{self.apartment}" + return first_line @dataclass -class Individual(BaseContact): +class Individual: + address: Address first_name: str last_name: str @@ -45,7 +45,8 @@ def name(self) -> str: @dataclass -class Company(BaseContact): +class Company: + address: Address name: str @@ -97,13 +98,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 @@ -210,3 +212,26 @@ def get_landlord(pad_bbl: str) -> Optional[Contact]: # 'DatabaseError'. logger.exception(f'Error while retrieving data from NYCDB') return None + + +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. + """ + + # 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 diff --git a/nycdb/tests/test_models.py b/nycdb/tests/test_models.py index 2bfb6f0f9..05ec02a11 100644 --- a/nycdb/tests/test_models.py +++ b/nycdb/tests/test_models.py @@ -3,7 +3,8 @@ import pytest from nycdb.models import ( - HPDRegistration, HPDContact, Company, Individual, get_landlord) + HPDRegistration, HPDContact, Company, Individual, get_landlord, + get_management_company) from . import fixtures @@ -38,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' @@ -95,3 +96,25 @@ def test_it_returns_contact_on_success(self, nycdb): boop = get_landlord(tiny.pad_bbl) assert isinstance(boop, Individual) assert boop.name == "BOOP JONES" + + +class TestGetManagementCompany: + def test_it_returns_none_if_nycdb_is_disabled(self): + assert get_management_company('') is None + + 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) + assert isinstance(mgmtco, Company) + assert mgmtco.name == "FUNKY APARTMENT MANAGEMENT" diff --git a/nycdb/tests/test_nycdb_lookup_landlord.py b/nycdb/tests/test_nycdb_lookup_landlord.py index bc7f180c8..668e957e1 100644 --- a/nycdb/tests/test_nycdb_lookup_landlord.py +++ b/nycdb/tests/test_nycdb_lookup_landlord.py @@ -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 diff --git a/onboarding/migrations/0006_onboardinginfo_pad_bbl.py b/onboarding/migrations/0006_onboardinginfo_pad_bbl.py new file mode 100644 index 000000000..dce8b07f8 --- /dev/null +++ b/onboarding/migrations/0006_onboardinginfo_pad_bbl.py @@ -0,0 +1,18 @@ +# Generated by Django 2.1.2 on 2018-11-29 19:27 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('onboarding', '0005_onboardinginfo_signup_intent'), + ] + + operations = [ + migrations.AddField( + model_name='onboardinginfo', + name='pad_bbl', + field=models.CharField(blank=True, help_text="The user's Boro, Block, and Lot number. This field is automatically updated when you change the address or borough, so you generally shouldn't have to change it manually.", max_length=10), + ), + ] diff --git a/onboarding/models.py b/onboarding/models.py index ba65e9d16..09939744e 100644 --- a/onboarding/models.py +++ b/onboarding/models.py @@ -3,6 +3,7 @@ from project.common_data import Choices from project import geocoding +from project.util.nyc import PAD_BBL_DIGITS from users.models import JustfixUser @@ -114,7 +115,7 @@ def __init__(self, *args, **kwargs): # This keeps track of fields that comprise metadata about our address, # which can be determined from the fields comprising our address. - self.__addr_meta = InstanceChangeTracker(self, ['zipcode']) + self.__addr_meta = InstanceChangeTracker(self, ['zipcode', 'pad_bbl']) created_at = models.DateTimeField(auto_now_add=True) @@ -153,6 +154,12 @@ def __init__(self, *args, **kwargs): help_text=f"The user's ZIP code. {ADDR_META_HELP}" ) + pad_bbl: str = models.CharField( + max_length=PAD_BBL_DIGITS, + blank=True, + help_text=f"The user's Boro, Block, and Lot number. {ADDR_META_HELP}" + ) + apt_number = models.CharField(max_length=10) is_in_eviction = models.BooleanField( @@ -256,7 +263,9 @@ def __should_lookup_new_addr_metadata(self) -> bool: def lookup_addr_metadata(self): features = geocoding.search(self.full_address) if features: - self.zipcode = features[0].properties.postalcode + props = features[0].properties + self.zipcode = props.postalcode + self.pad_bbl = props.pad_bbl self.__addr.set_to_unchanged() self.__addr_meta.set_to_unchanged() diff --git a/onboarding/tests/test_models.py b/onboarding/tests/test_models.py index 0c8849b9c..5094a99e2 100644 --- a/onboarding/tests/test_models.py +++ b/onboarding/tests/test_models.py @@ -78,7 +78,7 @@ def mkinfo_without_metadata(self): return self.mkinfo() def mkinfo_with_metadata(self): - return self.mkinfo(zipcode='11231') + return self.mkinfo(zipcode='11231', pad_bbl='2002920026') def test_no_lookup_when_full_address_is_empty(self): assert OnboardingInfo().maybe_lookup_new_addr_metadata() is False @@ -87,6 +87,7 @@ def test_no_lookup_when_addr_and_metadata_have_changed(self): info = self.mkinfo_with_metadata() info.address = '120 zzz street' info.zipcode = '12345' + info.pad_bbl = '4002920026' assert info.maybe_lookup_new_addr_metadata() is False def test_no_lookup_when_metadata_exists_and_nothing_changed(self): @@ -117,3 +118,4 @@ def test_successful_lookup_is_applied_to_model(self, requests_mock, settings): info = self.mkinfo_without_metadata() assert info.maybe_lookup_new_addr_metadata() is True assert info.zipcode == '11201' + assert info.pad_bbl == '3002920026'