Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

20360 Update reg and change of reg outputs with alt name #2538

Merged
merged 14 commits into from
Apr 17, 2024
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<span class="bold">Name Request Number: </span>{{registration.nameRequest.nrNumber}}
</div>
<div class="pt-2">
<span class="bold">Business Name: </span>{{registration.nameRequest.businessName}}
<span class="bold">Business Name: </span>{{registration.nameRequest.legalName}}
</div>
{% if business.naicsDescription %}
<div class="pt-2">
Expand Down
29 changes: 20 additions & 9 deletions legal-api/src/legal_api/models/alternate_name.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,24 @@ def is_owned_by_colin_entity(self):
"""Return if owned by colin entity."""
return bool(self.colin_entity)

@property
def entity_delivery_address(self):
"""Return if owned by colin entity."""
return (
self.legal_entity.entity_delivery_address
if self.is_owned_by_legal_entity_person
else self.owner_delivery_address
)

@property
def entity_mailing_address(self):
"""Return if owned by colin entity."""
return (
self.legal_entity.entity_mailing_address
if self.is_owned_by_legal_entity_person
else self.owner_mailing_address
)

@property
def owner_data_json(self):
"""Return if owner data for SP only."""
Expand All @@ -226,15 +244,8 @@ def owner_data_json(self):
],
}

delivery_address = None
mailing_address = None

if self.is_owned_by_legal_entity_person:
delivery_address = self.legal_entity.entity_delivery_address
mailing_address = self.legal_entity.entity_mailing_address
else:
delivery_address = self.owner_delivery_address
mailing_address = self.owner_mailing_address
delivery_address = self.entity_delivery_address
mailing_address = self.entity_mailing_address

if delivery_address:
member_address = delivery_address.json
Expand Down
8 changes: 4 additions & 4 deletions legal-api/src/legal_api/reports/report.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ def _get_template_filename(self):

def _get_template_data(self):
if self._report_key in ["noticeOfArticles", "amendedRegistrationStatement", "correctedRegistrationStatement"]:
filing = VersionedBusinessDetailsService.get_company_details_revision(self._filing.id, self._business.id)
filing = VersionedBusinessDetailsService.get_company_details_revision(self._filing.id, self._business)
self._format_noa_data(filing)
else:
filing = copy.deepcopy(self._filing.filing_json["filing"])
Expand Down Expand Up @@ -869,9 +869,7 @@ def _format_change_of_registration_data(
if business_office := filing.get(filing_type).get("offices", {}).get("businessOffice"):
filing["offices"] = {}
filing["offices"]["businessOffice"] = business_office
offices_json = VersionedBusinessDetailsService.get_office_revision(
prev_completed_filing, self._filing.legal_entity_id
)
offices_json = VersionedBusinessDetailsService.get_office_revision(prev_completed_filing, self._business)
filing["offices"]["businessOffice"]["mailingAddress"]["changed"] = self._compare_address(
business_office.get("mailingAddress"), offices_json["businessOffice"]["mailingAddress"]
)
Expand Down Expand Up @@ -903,6 +901,8 @@ def _format_change_of_registration_data(
prev_party_json = VersionedBusinessDetailsService.party_revision_json(
prev_completed_filing, prev_party, True
)
# current_party type is person, but prev_party type is organization
# so has issue at _has_party_name_change()
if self._has_party_name_change(prev_party_json, party):
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FM1064603, filing_id: 148571:
In this case, prev_party is organization. current_party is person.
So _has_party_name_change function returns the error. We should fix this logic

Copy link
Collaborator

@argush3 argush3 Apr 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can fix this in another ticket. We need to think about this more when we get back to this work.

I'll create a new ticket.

Can you find a business to test with that doesn't have party type changing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am trying to find it

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore my previous comment. This is a code issue, not a data issue I think.

You are pulling back an AN record when the code calls get_party_revision.

image

It should be pulling back the LE person history record.

image

I'm assuming this is related to your _has_party_name_change error.

Copy link
Collaborator Author

@kzdev420 kzdev420 Apr 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, if business.legal_entity_id is null and colin_entity_id is True, then we should pull back the Colin_entity history?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but i'm not entirely sure if that will work as is. you may need to do some work to get it working. maybe put a todo and ignore that scenario for now. just say we need to still take care of that scenario in the future.

party["nameChanged"] = True
party["previousName"] = self._get_party_name(prev_party_json)
Expand Down
89 changes: 62 additions & 27 deletions legal-api/src/legal_api/services/business_details_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
)
from legal_api.utils.legislation_datetime import LegislationDatetime

from .business_service import BusinessService


class VersionedBusinessDetailsService: # pylint: disable=too-many-public-methods
"""Provides service for getting business details as of a filing."""
Expand Down Expand Up @@ -221,6 +223,9 @@ def get_business_revision_obj(filing, business) -> any:
"""Return version object associated with the given filing for a business."""
business_revision = business

if business_revision and business_revision.version == 1:
return business_revision

# The history table has the old revisions, not the current one.
if business_revision and business_revision.change_filing_id != filing.id:
business_version = (
Expand Down Expand Up @@ -276,7 +281,7 @@ def get_office_revision(filing, business) -> dict: # pylint: disable=too-many-l
office_attribute = Office.legal_entity_id if business.is_legal_entity else Office.alternate_name_id

offices_current = (
db.session.query(Office, null().label("changed"))
db.session.query(Office)
.filter(Office.change_filing_id <= filing_id)
.filter(office_attribute == business.id)
.filter(Office.deactivated_date == None) # noqa: E711,E501;
Expand All @@ -286,8 +291,9 @@ def get_office_revision(filing, business) -> dict: # pylint: disable=too-many-l
office_history_attribute = (
office_history.legal_entity_id if business.is_legal_entity else office_history.alternate_name_id
)
office_history_columns = VersionedBusinessDetailsService.select_revision_columns(office_history)
offices_historical = (
db.session.query(office_history)
db.session.query(*office_history_columns)
.filter(office_history.change_filing_id <= filing_id)
.filter(office_history_attribute == business.id)
.filter(office_history.deactivated_date == None) # noqa: E711,E501;
Expand All @@ -307,33 +313,36 @@ def get_office_revision(filing, business) -> dict: # pylint: disable=too-many-l
.filter(office_history.deactivated_date == None) # noqa: E711,E501;
) # pylint: disable=singleton-comparison

valid_office_types = current_types.union(historical_types).distinct().all()
valid_office_types = (
current_types.union(historical_types).distinct().all()
) # result is like [("businessOffice", )]

address_history = history_cls(Address)
for valid_office_type in valid_office_types:
office, _ = (
offices_current.filter(Office.office_type == valid_office_type.office_type)
.union(offices_historical.filter(office_history.office_type == valid_office_type.office_type))
add_history_columns = VersionedBusinessDetailsService.select_revision_columns(address_history)
for item in valid_office_types:
valid_office_type = item[0]
office = (
offices_current.filter(Office.office_type == valid_office_type)
.union(offices_historical.filter(office_history.office_type == valid_office_type))
.first()
)

offices_json[office.office_type] = {}

addresses_current = (
db.session.query(Address, null().label("changed"))
db.session.query(Address)
.filter(Address.change_filing_id <= filing_id)
.filter(Address.office_id == office.id)
)

addresses_historical = (
db.session.query(address_history)
db.session.query(*add_history_columns)
.filter(address_history.change_filing_id <= filing_id)
.filter(address_history.office_id == office.id)
)

addresses_result = addresses_current.union(addresses_historical).all()
for address_result in addresses_result:
address, _ = address_result
addresses = addresses_current.union(addresses_historical).all()
Copy link
Collaborator Author

@kzdev420 kzdev420 Apr 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FM1000406, filing_id: 146925
In this case, business_id is 691 ( alternate_name_id )

But, there are no records for this business in Address and Address_history table ( my local DB )
So, it returns None, and makes the error

for address in addresses:
offices_json[office.office_type][
f"{address.address_type}Address"
] = VersionedBusinessDetailsService.address_revision_json(address)
Expand Down Expand Up @@ -565,17 +574,32 @@ def party_role_revision_json(filing, party_role_revision, is_ia_or_after) -> dic
@staticmethod
def get_party_revision(filing, party_id) -> dict:
argush3 marked this conversation as resolved.
Show resolved Hide resolved
"""Consolidates all party changes up to the given transaction id."""
business = BusinessService.fetch_business_by_filing(filing)
if filing.legal_entity_id or business.legal_entity_id:
Copy link
Collaborator

@argush3 argush3 Apr 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this change and how you are grabbing party revision data in general for all the various scenarios.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FM1064603, filing_id: 148571.

In this case, the filing doesn't have the legal_entity. it has the alternate_name_entity.
And alternate_name_entity has the legal_entity.

I think you mentioned that filing pull back the legal_entity in this case

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I mentioned that. But I think the logic needs to be more precise. Anyways, we can pick this up in the new ticket.

business_attr = LegalEntity
party_version = history_cls(business_attr)
business_attr_id = business_attr.id
party_version_business_id = party_version.id
else:
business_attr = AlternateName
party_version = history_cls(business_attr)
business_attr_id = business_attr.legal_entity_id
party_version_business_id = party_version.legal_entity_id
if business.is_owned_by_colin_entity:
business_attr_id = business_attr.colin_entity_id
party_version_business_id = party_version.colin_entity_id

party_current = (
db.session.query(LegalEntity)
.filter(LegalEntity.change_filing_id == filing.id)
.filter(LegalEntity.id == party_id)
db.session.query(business_attr)
.filter(business_attr.change_filing_id == filing.id)
.filter(business_attr_id == int(party_id))
)

party_version = history_cls(LegalEntity)
columns_to_select = VersionedBusinessDetailsService.select_revision_columns(party_version)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if you are selecting the right columns for the party in all cases. For example, it looks like if it's a SP person, SP DBA LEAR or SP DBA COLIN, you are selecting the alternate name columns. I think you should be selecting the columns for the LE or AN record.

party_history = (
db.session.query(party_version)
db.session.query(*columns_to_select)
.filter(party_version.change_filing_id == filing.id)
.filter(party_version.id == party_id)
.filter(party_version_business_id == int(party_id))
)
party = party_current.union(party_history).one_or_none()
return party
Expand All @@ -602,7 +626,7 @@ def party_revision_type_json(party_revision, is_ia_or_after) -> dict:
"officer": {
"organizationName": party_revision.legal_name,
"partyType": BusinessCommon.EntityTypes.ORGANIZATION.value,
"identifier": party_revision._identifier, # pylint: disable=protected-access
"identifier": party_revision.identifier, # pylint: disable=protected-access
}
}
if party_revision.email:
Expand All @@ -613,20 +637,25 @@ def party_revision_type_json(party_revision, is_ia_or_after) -> dict:
def party_revision_json(filing, party_revision, is_ia_or_after) -> dict:
"""Return the party member as a json object."""
member = VersionedBusinessDetailsService.party_revision_type_json(party_revision, is_ia_or_after)
if party_revision.delivery_address_id:
address_revision = VersionedBusinessDetailsService.get_address_revision(
filing, party_revision.delivery_address_id
)

delivery_address_id = party_revision.delivery_address_id
mailing_address_id = party_revision.mailing_address_id
if party_revision.is_alternate_name_entity and party_revision.is_owned_by_legal_entity_person:
delivery_address_id = party_revision.legal_entity.delivery_address_id
mailing_address_id = party_revision.legal_entity.mailing_address_id

if delivery_address_id:
address_revision = VersionedBusinessDetailsService.get_address_revision(filing, delivery_address_id)
# This condition can be removed once we correct data in address and address_version table
# by removing empty address entry.
if address_revision and address_revision.postal_code:
member_address = VersionedBusinessDetailsService.address_revision_json(address_revision)
if "addressType" in member_address:
del member_address["addressType"]
member["deliveryAddress"] = member_address
if party_revision.mailing_address_id:
if mailing_address_id:
member_mailing_address = VersionedBusinessDetailsService.address_revision_json(
VersionedBusinessDetailsService.get_address_revision(filing, party_revision.mailing_address_id)
VersionedBusinessDetailsService.get_address_revision(filing, mailing_address_id)
)
if "addressType" in member_mailing_address:
del member_mailing_address["addressType"]
Expand Down Expand Up @@ -716,9 +745,10 @@ def get_address_revision(filing, address_id) -> dict:
)

address_version = history_cls(Address)
columns_to_select = VersionedBusinessDetailsService.select_revision_columns(address_version)
address_history = (
db.session.query(address_version)
.filter(address_version.change_filing_id == filing.id)
db.session.query(*columns_to_select)
.filter(address_version.change_filing_id <= filing.id)
.filter(address_version.id == address_id)
)
address = address_current.union(address_history).one_or_none()
Expand Down Expand Up @@ -840,3 +870,8 @@ def get_name_request_revision(filing):
def get_contact_point_revision(filing):
"""Return contact point from filing json."""
return filing.json["filing"]["incorporationApplication"].get("contactPoint", {})

@staticmethod
def select_revision_columns(revision_table):
"""Return contact point from filing json."""
return [col for col in revision_table.__table__.columns if col.name != "changed"]
Loading