fix!: partOf and sequenceOf properties are not marked as isEditable

[BalduinLandolt]

Issue Number: DEV-1439

Pull Request Checklist

Basic Requirements

Please check if your PR fulfills the following requirements:

• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• Bugfix: represents bug fixes
• Refactor: represents production code refactoring
• Feature: represents a new feature
• Documentation: documentation changes (no production code change)
• Chore: maintenance tasks (no production code change)
• Style: styles updates (no production code change)
• Test: all about tests: adding, refactoring tests (no production code change)
• Other... Please describe:

Does this PR introduce a breaking change?

• Yes
• No

The PR introduces changes to knora-base, which requires an upgrade script to run, so it needs to be marked as breaking. However, it does not change any public API or break existing code.

Does this PR change client-test-data?

• Yes (don't forget to update the JS-LIB team about the change)
• No

Other information

[swarmia]

✅  Linked to Bug DEV-1439 · DSP-API: isEditable property for isSequenceOfValue is returned as false
➡️  Part of Epic DEV-22 · isSequenceOf Support

[irinaschubert]

LGTM, I just haven't understood why isEditable now needs to be set to true. Wouldn't that mean that users can edit knora-base properties - which should not be the case? (I am sure that you did it right, I just want to understand :)

[irinaschubert]

Hm, I think I don't understand why here everything is set to :isEditable true. Shouldn't knora-base things not be editable?

[BalduinLandolt]

LGTM, I just haven't understood why isEditable now needs to be set to true. Wouldn't that mean that users can edit knora-base properties - which should not be the case? (I am sure that you did it right, I just want to understand :)

Well... I suppose isEditable is a very badly named flag. The API never lets you edit knora-base (I think/hope). There isn't really any documentation on it either, so it's hard to reconstruct the reasoning behind the flag. But in knora-base itself it's defined with the comment "Indicates whether a property's values can be updated via the Knora API."@en, so it seems to be referring to the values instantiating the property, rather than the property as defined in knora-base. And it seems that "editing a value" is to be taken as equivalent to "create a value". So it probably should be canBeInstantiated - but we already have that and it can only be used on classes, not on properties.

Does that make it clearer?

[irinaschubert]

LGTM, I just haven't understood why isEditable now needs to be set to true. Wouldn't that mean that users can edit knora-base properties - which should not be the case? (I am sure that you did it right, I just want to understand :)

Well... I suppose isEditable is a very badly named flag. The API never lets you edit knora-base (I think/hope). There isn't really any documentation on it either, so it's hard to reconstruct the reasoning behind the flag. But in knora-base itself it's defined with the comment "Indicates whether a property's values can be updated via the Knora API."@en, so it seems to be referring to the values instantiating the property, rather than the property as defined in knora-base. And it seems that "editing a value" is to be taken as equivalent to "create a value". So it probably should be canBeInstantiated - but we already have that and it can only be used on classes, not on properties.

Does that make it clearer?

Yes, thanks a lot!

[subotic]

I have a few questions, if isSequenceOf is only used for audio/video, then shouldn't :objectClassConstraint be :Representation instead of :Resource?

[BalduinLandolt]

I have a few questions, if isSequenceOf is only used for audio/video, then shouldn't :objectClassConstraint be :Representation instead of :Resource?

Yes and no: It's the same with isPartOf - we don't say it can only be on these kinds of resources, we merely say "if it's used on these resources in combination with certain other properties (i.e. sequence bounds or seqnum), the frontend may provide special behaviour"

This could of course be changed, but then we have to check the DB for other usages

[codecov]

Codecov Report

Base: 86.85% // Head: 86.91% // Increases project coverage by +0.05% 🎉

Coverage data is based on head (5af5f01) compared to base (c5c98ce).
Patch coverage: 92.30% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2268      +/-   ##
==========================================
+ Coverage   86.85%   86.91%   +0.05%     
==========================================
  Files         241      242       +1     
  Lines       27967    28062      +95     
==========================================
+ Hits        24292    24390      +98     
+ Misses       3675     3672       -3     

Impacted Files	Coverage Δ
...c/main/scala/org/knora/webapi/core/AppServer.scala	89.55% <ø> (ø)
...ages/util/search/gravsearch/GravsearchParser.scala	68.13% <0.00%> (-0.26%)	⬇️
...webapi/responders/admin/ProjectsResponderADM.scala	92.01% <ø> (ø)
...p-shared/src/main/scala/dsp/valueobjects/Iri.scala	94.18% <100.00%> (-0.14%)	⬇️
...sp-shared/src/main/scala/dsp/valueobjects/V2.scala	89.85% <100.00%> (+5.79%)	⬆️
.../org/knora/webapi/messages/OntologyConstants.scala	99.62% <100.00%> (+<0.01%)	⬆️
...la/org/knora/webapi/messages/StringFormatter.scala	90.14% <100.00%> (-0.02%)	⬇️
...min/responder/usersmessages/UsersMessagesADM.scala	79.36% <100.00%> (+0.22%)	⬆️
...bapi/src/main/scala/org/knora/webapi/package.scala	100.00% <100.00%> (ø)
...sp-shared/src/main/scala/dsp/valueobjects/Id.scala	65.38% <0.00%> (-3.85%)	⬇️
... and 13 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.