Issue #790: Fix entityIdPath #815
Conversation
|
MDR should be able to map an attribute from each scenario:
|
cbeach47
force-pushed
the
bugfix/issue-790/adjust-entity-id-path
branch
from
4f7c81d to
d127ca2
Compare
| id_path=id_path, | ||
| fail_if_origin_data_model_is_not_the_anchor=( | ||
| i == 0 | ||
| ), # Only fail if the first node does not originate on the anchor data model |
There was a problem hiding this comment.
I don't think this check is needed fail_if_origin_data_model_is_not_the_anchor. Is there a reason this is here that I'm forgetting about?
If the data model is BaseLIF or SourceSchema, then the entity/attribute's data_model_id should equal the anchor_data_model_id.
If the data model is OrgLIF or PartnerLIF, then even the root entity data_model_id may not equal the anchor_data_model_id.
I believe these cases are covered by your is_self_contained_anchor_model check.
There was a problem hiding this comment.
If the data model is OrgLIF or PartnerLIF, then even the root entity data_model_id may not equal the anchor_data_model_id.
ah ok, I'll relax that constraint.
| originates_in_anchor = anchor_data_model_id == node_data_model_id | ||
| if not originates_in_anchor: | ||
| signature = f"{node_type} {raw_node_id}({cleaned_node_id}) for data model {node_data_model_id} in the entityIdPath {id_path}" | ||
| if fail_if_origin_data_model_is_not_the_anchor: |
There was a problem hiding this comment.
Per my other comment, I don't think we should have this check.
| ) | ||
|
|
||
| # Will only be checked for Org LIF and Partner LIF anchor data models | ||
| await check_existing_inclusion( |
There was a problem hiding this comment.
imo, if the data model type is OrgLIF or PartnerLIF, you should check for the existing inclusion regardless of whether the entity/attribute's data_model_id == anchor_data_model_id.
There was a problem hiding this comment.
and if the data_model_id == anchor_data_model_id then the inclusion's ExtDataModelId will then be null?
There was a problem hiding this comment.
Based on looking at the frontend code, I think the Inclusion's ExtDataModelId is supposed to always equal the anchor_data_model_id.
| return result.scalar_one_or_none() is not None | ||
|
|
||
|
|
||
| async def check_entity_association_strict( |
There was a problem hiding this comment.
Why are you using this instead of check_existing_association? There are cases where ExtendedByDataModelId could be null:
- If data model type is BaseLIF or SourceSchema, then I think the ExtendedByDataModelId will always be null.
- If the data model type is OrgLIF or PartnerLIF then the ExtendedByDataModelId may or may not be null.
There was a problem hiding this comment.
I didn't like that it was an 'or' condition in check_existing_association. I felt we should be able to determine what value the ExtendedByDataModelId should be prior to invoking the method.
There was a problem hiding this comment.
I've been having a hard time trying to determine how we could reverse engineer the ExtendedByDataModelId of the association. I think what I said in my other comment is true that for the OrgLIF and PartnerLIF, these are the cases where ExtendedByDataModelId of the association will not be null:
- The entity/attribute itself is an Extension and has ExtendedByDataModelId non-null.
- Someone used "+ Existing" to create an association in their OrgLIF or PartnerLIF that does not exist in BaseLIF.
That 2nd case is difficult to discern. The only thing I can think of to prove this is:
- If the Association has Extension=true, then ExtendedByDataModelId=anchor_data_model_id.
But we're trying to fetch the Association based off of this info, so we don't know yet whether or not it's an extension. I guess if you wanted, you could fetch the Association with the 'or' condition and then check if Extension=true and if it does then check that ExtendedByDataModelId=anchor_data_model_id.
| in [DataModelType.BaseLIF, DataModelType.SourceSchema], | ||
| ) | ||
|
|
||
| extended_by_data_model_id = None if originates_in_anchor else anchor_data_model.Id |
There was a problem hiding this comment.
If the data model is BaseLIF or SourceSchema, then the association's extended_by_data_model_id should equal null always.
If the data model is OrgLIF or PartnerLIF, then the association's extended_by_data_model_id should be non-null if:
- The entity/attribute itself is an Extension and has ExtendedByDataModelId non-null.
- Someone used "+ Existing" to create an association in their OrgLIF or PartnerLIF that does not exist in BaseLIF.
I don't think we need to try to calculate which of these situations we're in though. I think it's okay to just always check for the association with extended_by_data_model_id = null or anchor_data_model_id, which is what I'm trying to suggest in my other comment when I say that I think you should use the check_entity_association method instead of the check_entity_association_strict method. Same for checking the entity-attribute association.
Description of Change
Modifies the transformation API to allow an entityIdPath that are a CSV of entity/attribute IDs.
Related Issues
Related to #790
WIP.