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

[DAR-3920][External] Bug fix for importing property values to identically named properties during property creation #930

Merged
merged 1 commit into from
Sep 25, 2024

Conversation

JBWilkie
Copy link
Collaborator

Problem

This PR address an issue that occurs when creating two identically named properties on separate classes and then importing values of those properties in the same import action. In this scenario, we get this type of error:

Errors importing 000000580635.jpg
        {'errors': {'annotations': [{}, {}, {}, {}, {'annotation_properties': ["index '0', property 'bc5b7760-7786-4a0c-af89-8fdace15a0bf': property is not linked to the annotation's class"]}, {'annotation_properties': ["index '0', 
property 'bc5b7760-7786-4a0c-af89-8fdace15a0bf': property is not linked to the annotation's class"]}]}}

However: The import does successfully create the properties & their values. Additionally, a 2nd import will import the annotations without error

The problem is that the annotation_property_map returned from _import_properties is constructed incorrectly in this scenario. Specifically, we only used to check that the name of a property matched the name of the property to be imported. If importing annotations containing multiple identically named properties, this logic allows for mis-matching

Solution

In addition to checking that the property names match, check that the annotation classes match too

Changelog

Fixed an edge case when importing property values during property creation

Copy link

linear bot commented Sep 24, 2024

Copy link
Contributor

@dorfmanrobert dorfmanrobert left a comment

Choose a reason for hiding this comment

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

Nice find! Non-blocking, but seems an alternative approach could be to construct created_properties and updated_properties as maps with keys as annotation_class_id rather than lists of all created/updated properties, so that only need to loop over the relevant properties for each ann when updating the annotation_property_map

@JBWilkie
Copy link
Collaborator Author

Nice find! Non-blocking, but seems an alternative approach could be to construct created_properties and updated_properties as maps with keys as annotation_class_id rather than lists of all created/updated properties, so that only need to loop over the relevant properties for each ann when updating the annotation_property_map

I think this logic will change in the item-level property import that I'm about to continue working on. Given this, I'm going to merge this but have included this as a point in the relevant ticket

@JBWilkie JBWilkie merged commit 970c065 into master Sep 25, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants