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

fix: removing cardinality of a link property (DEV-90) #1919

Merged
merged 7 commits into from Oct 13, 2021

Conversation

@subotic
Copy link
Collaborator

@subotic subotic commented Oct 8, 2021

resolves DEV-90

@subotic subotic self-assigned this Oct 8, 2021
@subotic subotic requested review from mpro7 and irinaschubert Oct 8, 2021
mpro7
mpro7 approved these changes Oct 11, 2021
internalClassDef = internalClassDef,
allBaseClassIris = allBaseClassIris.toSet,
cacheData = cacheData
Copy link
Contributor

@mpro7 mpro7 Oct 11, 2021

Choose a reason for hiding this comment

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

I think this could be simplified:

.checkCardinalitiesBeforeAddingAndIfNecessaryAddLinkValueProperties(
             internalClassDef,
             allBaseClassIris.toSet,
             cacheData,
...

Loading

Copy link
Contributor

@irinaschubert irinaschubert left a comment

LGTM

Loading

cacheData.ontologies(propertyIri.getOntologyFromEntity).properties(propertyIri)
) // turn the propertyIri into a ReadPropertyInfoV2
.filter(_.isLinkProp) // we are only interested in link properties
.map(_.entityInfoContent.propertyIri) // turn whatever is left back to a propertyIri
)
Copy link
Contributor

@irinaschubert irinaschubert Oct 11, 2021

Choose a reason for hiding this comment

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

I don't understand why this has to be done

Loading

Copy link
Collaborator Author

@subotic subotic Oct 12, 2021

Choose a reason for hiding this comment

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

the data model for links consist of the linked property and a corresponding link property value. This checkCardinalitiesBeforeAddingAndIfNecessaryAddLinkValueProperties method will complain if there are already link property values defined, because this method should add them. This is why this method has the existingLinkPropsToKeep parameter. Since this parameter only takes IRIs we first need to convert all property IRIs to ReadPropertyInfoV2 then filter out anything that is not a link property and then turn it back to a property IRI.

Loading

Loading
Loading
Loading
subotic and others added 4 commits Oct 12, 2021
…c.scala

Co-authored-by: irinaschubert <irina.schubert@dasch.swiss>
…c.scala

Co-authored-by: irinaschubert <irina.schubert@dasch.swiss>
…c.scala

Co-authored-by: irinaschubert <irina.schubert@dasch.swiss>
@subotic subotic merged commit c79c194 into main Oct 13, 2021
11 checks passed
Loading
@subotic subotic deleted the wip/DEV-90-canDeleteCardinalities branch Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants