-
Notifications
You must be signed in to change notification settings - Fork 75
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
Changes from all commits
a5a069a
ee05ba6
9941abd
2f260eb
fdd39e0
91f2567
1b40e6d
d84d221
f71cacc
8765941
aba05d3
fbfb6c9
2ae2d8c
3428dae
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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.""" | ||
|
@@ -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 = ( | ||
|
@@ -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; | ||
|
@@ -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; | ||
|
@@ -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() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
But, there are no records for this business in Address and Address_history table ( my local DB ) |
||
for address in addresses: | ||
offices_json[office.office_type][ | ||
f"{address.address_type}Address" | ||
] = VersionedBusinessDetailsService.address_revision_json(address) | ||
|
@@ -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: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. I think you mentioned that filing pull back the legal_entity in this case There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
@@ -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: | ||
|
@@ -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"] | ||
|
@@ -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() | ||
|
@@ -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"] |
There was a problem hiding this comment.
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 logicThere was a problem hiding this comment.
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?There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
.It should be pulling back the LE person history record.
I'm assuming this is related to your
_has_party_name_change
error.There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.