-
Notifications
You must be signed in to change notification settings - Fork 3
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(apis_entities): only list possible properties in filterset form #959
Conversation
5c6039c
to
4ebe555
Compare
@b1rger In TibSchol I was asked to list the property either as forward or reverse as it applies to the entity type we are searching on. If that makes sense for APIS in general then I'd recommend that as part of #949 When the subject and object are the same type, I displayed the text to show both forward and reverse text to select the property - but in Tibschol they want to see it as two different properties to filter the search better. IMO this is probably just a Tibschol specific requirement - need not be the default behaviour of APIS core. |
544c32b
to
b356553
Compare
It makes sense to display the property with the name based on the entity we are filtering. I've tried to implement that and also added the possible property "targets". I'm not sure if it works as expected, the naming of properties in my test setups is a bit confusing |
(pls. rereview) |
@b1rger yes - now the property is diplayed as it applies to the entity type as subject. When the Subject and Object are the same type of entity but have different forward and reverse property labels, such as this Character to Character relationship mother of/daughter of, This might be because something is wrong with my setup and nothing to do with the pull request - but,
|
b356553
to
0a02022
Compare
The filter `related_properties` listed all the existing properties, but it makes more sense to only list the properties that can have the model the filter filters as a `subj` or an `obj`. Closes: #949
0a02022
to
c615670
Compare
Thats up for discussion. I've now changed that to show
I've fixed that. |
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.
Looks good to me!
The filter
related_properties
listed all the existing properties, butit makes more sense to only list the properties that can have the model
the filter filters as a
subj
or anobj
.Closes: #949