-
Notifications
You must be signed in to change notification settings - Fork 31
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
Replace field_geo_broader with taxonomy-provided parent field #21
Conversation
@Islandora-CLAW/committers anybody want to review this? |
I'm testing... I pulled the code and did Side conversation: I'm not sure what was "incorrectly configured" (according to the ticket) but I have 2 concerns.
|
@rosiel, doh! Yes, the Controlled Access Terms Feature needs to import the changes to the rdf mapping. |
@rosiel, as for the second issue. You are right, we probably shouldn't box ourselves in using the Taxonomy hierarchy and we should simply hide the taxonomy-provided relations widget... So, if we close this PR without merging we would preserve the multi-parent geo_broader (schema:containedInPlace). We could still navigate the hierarchy by clicking through each page, but there wouldn't be a pretty tree-view (yet, at least). If we merge the PR we are stuck with a single parent (schema:containedInPlace) but we potentially get the benefits of tree display and re-ordering. (Or not, if we have too many of them, as @rosiel notes.) Sounds like we should ditch the PR (although open a new one to hide the taxonomy hierarchy in the edit form). Perhaps we should ask the MIG group for their thoughts first? Any other thoughts, @Islandora-CLAW/committers ? |
@seth-shaw-unlv In my experience a hierarchy with multiple parents is a better approach for spatial data. e.g. a feature can exist in a territorial authority region or state and in a national park. This was supported by the D7 Node Hierarchy module but is not supported AFAICT by the D8 entity_hierarchy module (https://www.drupal.org/project/entity_hierarchy). entity_hierarchy uses nested sets for very fast lookup of descendants; we are using it for Islandora 8, but I'd prefer a solution that supported multiple parents. |
@seth-shaw-unlv But I didn't see any Controlled Access Terms Feature!!! I looked! So yeah the code works as indicated. But as for whether we should... I haven't done a "full survey" of geo data, but Geonames allows "multiple parents" though encodes them as [edited cause I can't read] "Country", "Admin 1" (province/state), "Admin 2" (whatever's below province/state) etc. I think the existing "geo broader" field could fulfill that and might do a "good enough" job. |
Closing this ticket as current behavior, supporting multiple geo_broaders, is the preferred behavior as per @rosiel's comments. |
GitHub Issue: Islandora/documentation#1059
What does this Pull Request do?
Replace field_geo_broader with taxonomy-provided parent field
What's new?
How should this be tested?
Interested parties
@Islandora-CLAW/committers