NCEAS / metacatui

MetacatUI: A client-side web interface for DataONE data repositories
https://nceas.github.io/metacatui
Apache License 2.0
42 stars 27 forks source link

Create anchors for entity detail sections in the Metadata View #83

Closed laurenwalker closed 4 years ago

laurenwalker commented 7 years ago

When you click on "More info" in the data package table, the URL should change to include the data object pid. e.g. #view/knb.1.1/knb.2.1

That URL, #view/knb.1.1/knb.2.1, should route to the metadata view for knb.1.1 and scroll down to the knb.2.1 section. If a user navigates to #view/knb.2.1, they should be rerouted to #view/knb.1.1/knb.2.1.

laurenwalker commented 7 years ago

The URL will need to be slightly different since identifiers can contain slashes, so it would look something more like #view/knb.1.1/data=knb.2.1 (or similar)

csjx commented 7 years ago

Heya @laurenwalker Good point about the slashes in identifiers. My preference is actually the first encoding with #view/:metadataid/:dataid. I say this because, although there's no hard and fast rules per se, the RESTful resource naming best practices I've seen are usually:

In this case, I think the metadata/data relationship is hierarchical given our packaging constructs.

To accommodate slashes in identifiers, as you of course know, the DataONE API requires that we (officially) pass only URL-encoded identifiers, and so any / would be interpreted as part of the URL, whereas any %2F would be interpreted as a / in the identifier. So I'm inclined to use that pattern so we maintain the hierarchy in the URI. Also, there's nothing that precludes identifiers from containing data=, which puts us in the same position, although yes, that's less likely than forward slashes. Anyway, that's just my $.02 -- @mbjones and @amoeba may have an opinion on this.

csjx commented 7 years ago

Also, the Github API is obviously well thought out, so perhaps it might inform us on using segments versus parameters: https://developer.github.com/v3/

laurenwalker commented 7 years ago

Revisiting this ticket - we may just want to discuss this is a bit before implementation since it will effectively break all URLs that others may have saved or bookmarked, if the URL contains a /. (Which is ALL DOI pids). We may be able to check for DOI patterns, but other pids that look like URLs (used frequently in other d1 MNs) can't be parsed confidently.

This means we likely will need to change MetacatUI routes to use encoded pids.

amoeba commented 4 years ago

Hey all, I was chatting with @mpsaloha and realized this still hasn't been done so I put together a simple change that provides support for it in https://github.com/NCEAS/metacatui/commit/23aa403399032953e54f9462d4cd2e3bf9ce63d9. Is there still interest in this?

See the commit message itself for detail on how I did it.

laurenwalker commented 4 years ago

Hi Bryce, thanks for doing this, sorry that I'm just now am taking a look at it. Is this compatible with your changes on the url-encoding branch?

amoeba commented 4 years ago

Hey @laurenwalker np. Good eye, I'm not sure if it will work off-hand. I'll take a look and report back here.

amoeba commented 4 years ago

I have a couple of tweaks to make on this and will make them later today.

amoeba commented 4 years ago

Okay, all done. I tested compatibility by merging this branch onto dev-2.11-encoding and testing them together. Ready for review and/or merge.

laurenwalker commented 4 years ago

After testing this out, I found that it only worked for datasets that had the identifier in the HTML produced by the Metacat view service, which is not always the case. If an EML doc has only basic entity metadata, such as an entity name, then it didn't work. So I restructured it a bit to leverage the work already done by MetadataView.findEntityDetailsContainer(), which goes through several checks to match EMLEntities to DataONEObjects. Now the hashes should work whenever there is a match by id, filename, online distribution URL, etc...

Merged the other encoding work in and it's on the dev-2.12 branch now, ready for release. Thanks!