External tools for other mimetypes

[qqmyers]

The existing tools mechanism is limited to working on tabular files. QDR is interested in being able to 'explore' other types of files, in general, but also specifically to view annotations created using hypothesis that link a paper and the dataset in Dataverse. I looked into the external tools mechanism and have been able to extend it to allow tools to be registered per mimetype, which should be a fairly generic way to support additional tools.

In doing this, I also noticed that the files table only displays the generic 'Explore' string when there is one tool for a file (versus showing individual tool titles in a pull-down menu when there are multiple tools). It seems like using the name of the tool itself when there is only one would be more useful to users.

I plan to submit a PR that makes these updates ...

[pdurbin]

In terms of whether there is a drop down and the name of the tool when there is only one tool, please note that when the external tool framework shipped in Dataverse 4.8.6 (#3657) you already had the behavior you want. Then #4572 came along (and pull request #4574) and the behavior changed in 4.9 to the behavior above that you don't like. 😄

So maybe you can make it configurable, since there are differing opinions on how the Explore button should look at behave when there is only one external tool? Please discuss. 😄

[qqmyers]

@pdurbin - I think its slightly different - I'm not suggesting to go back to a menu with one dropdown entry, but to keep the button with a tool specific label when there is only one. Currently, if there's one tool, there is no way to see what it is since the button just says "Explore" (Nominally that string is configurable in Bundle so if there's only one file type with tools and only one tool, you could change the string in Bundle, but, with tools for different mimetypes, this won't work.) If that doesn't work for Dataverse as a whole, I can pull it out of the PR / potentially look at further work to make it configurable, etc.

[pdurbin]

@qqmyers ok, Kevin W isn't working at Scholars Portal anymore so maybe you can ask @amberleahey if she has strong opinions about button labels. On a related note, the design team is hard at work on a dataset page redesign and I've seen some mockups but I'm not sure which are public. /cc @TaniaSchlatter @mheppler @dlmurphy @jggautier . I'd suggest making a different issue and pull request for this. Small chunks.

Finally, I'm not sure if you've seen #5028 but it's related.

[pdurbin]

@qqmyers I spoke with @kcondon about pull request #5464 this morning and I have some feedback for you:

• A doc update for a future version of http://guides.dataverse.org/en/4.10.1/installation/external-tools.html#inventory-of-external-tools seems necessary. It looks like you're adding a newly required field so it should be flagged as required (like "fileId").
• The newly required field ("contentType") does not have a database constraint (NOT NULL) in the SQL update script, making it out of sync with @Column(nullable = false) on the Entity. Also, what's the plan for sites like TDL that have TwoRavens as an external tool? Should the SQL update script include an update to existing rows to add "text/tab-separated-values" as the content type? Because that's what we call tabular data internally? I'm not sure. This might need a little more thought.
• @kcondon noticed that the SQL update script is for 4.10.2. @djbrooke is the one who decides if the next release will be 4.10.2 or 4.11.

[djbrooke]

4.11 will be the next release

[pdurbin]

Oh, and there are merge conflicts, that's enough for me to send this back to "community dev" at https://waffle.io/IQSS/dataverse . Sorry! But I think these maybe be self-inflicted from your previous pull request in this area, @qqmyers . 😄

Oh, and I mentioned that previous pull request (#5457) to @craig-willis from @whole-tale and he's looking forward to trying it out. And the new pull request, once it gets merged. 😄

[qqmyers]

@pdurbin - looking at the changes above and there's a discussion point to raise: As you note things are inconsistent now but I could make it so that contenttype can't be null (and create an update script to map current tools to the DataFileServiceBean.MIME_TYPE_TSV_ALT type, and request updates to the tool json scripts from everyone...), or I could allow null and assume tools with null are legacy ones for tabular data. Any preference? The latter looks less disruptive but it keeps the legacy approach alive...

[pdurbin]

@qqmyers thanks for chatting about this at http://irclog.iq.harvard.edu/dataverse/2019-01-29#i_85834 and I'm sorry I didn't have time to dig in deeper. The bottom line seemed to be that you plan to change the new "contentType" keyword from optional to required.

@scolapasta @matthew-a-dunlap any strong opinions on any of this? Also, please note that @manikon has worked on adding a "localeCode" to over at scholarsportal@ef68eee and it seems to be used in production at https://dataverse.scholarsportal.info . We should coordinate with @manikon and @JayanthyChengan and @amberleahey about this. Kudos to @qqmyers for spotting this and bringing it out our attention. Seems like a great feature! When you click "Explore" the external tool gets the language in use in Dataverse.

[qqmyers]

@pdurbin - just the opposite :-) - If contentType has to be non-null, the database needs to be updated as do the json tool manifests for 2Ravens and the data explorer. Instead, I went ahead to add code to allow null and to assume that null means working on tabular data, so not adding a contentType is equivalent to specifying the "text/tab-separated-values" mimetype. The updated code for this is checked in to #5464. I'm OK with doing the opposite as well - cleaner code but more work to coordinate updates.

[pdurbin]

@qqmyers gah! I was in a hurry (standup in one minute). Yes! You've moved it from required to optional. I assume you're read for code review. Thanks!

[scolapasta]

@qqmyers I think I'd prefer it the other way, make it not null and update existing entries. In order to not break older manifests, I'd be ok with the parsing code to populate text/tab if it encounters a manifest that doesn't define content type. (maybe the api response would suggest that the manifest should get updated, so that we can someday deprecate the idea, if we desire). @qqmyers @pdurbin thoughts?

[qqmyers]

@scolapasta - sure - how's the PR look now with the latest commits? content type is an optional parameter and if it is null in a tool manifest, the table gets contentype = text/tab-separated-values ... db script also set to require not null and to default to this type for any existing entries.

I didn't change the api response - it will show the updated json in the response, with an added default contenttype if that was not sent, but tracking whether the underlying service bean had to add a contenttype or one was in the manifest to start would require something like an added flag in ExternalTool to be able to pass that fact back to the API.

[qqmyers]

@djbrooke - the requested updates are already committed...

[djbrooke]

@qqmyers sorry!

[kcondon]

@qqmyers @scolapasta Sorry to be slow but how would I test this? I am not certain what it does at this point.

[qqmyers]

@kcondon - probably the easiest test would be to create a tool manifest that specifies a non-tabular mimetype (e.g. image/tiff) with some URL and verify a) that you can install that via the api, b) the button shows up for a file of that type, c) that clicking it causes the URL to be requested, d) that the query parameters show up on the URL. If you just pick a non-existent URL, I think you will see a new window with that URL and a 404.

[kcondon]

@qqmyers Thanks, will do.

[manikon]

Hello Philip, Sorry I didn't reply sooner. Below is the commit that adds an additional external tool parameter for passing the locale from Dataverse to external applications. scholarsportal@ef68eee We currently use it on production to pass the locale from Dataverse to the Dataverse Data Explorer tool. Regards, Sunil On 2019-01-30 11:04 a.m., Philip Durbin wrote: @qqmyers<https://github.com/qqmyers> thanks for chatting about this at http://irclog.iq.harvard.edu/dataverse/2019-01-29#i_85834 and I'm sorry I didn't have time to dig in deeper. The bottom line seemed to be that you plan to change the new "contentType" keyword from optional to required. @scolapasta<https://github.com/scolapasta> @matthew-a-dunlap<https://github.com/matthew-a-dunlap> any strong opinions on any of this? Also, please note that @manikon<https://github.com/manikon> has worked on adding a "localeCode" to over at scholarsportal@ef68eee<scholarsportal@ef68eee> and it seems to be used in production at https://dataverse.scholarsportal.info . We should coordinate with @manikon<https://github.com/manikon> and @JayanthyChengan<https://github.com/JayanthyChengan> and @amberleahey<https://github.com/amberleahey> about this. Kudos to @qqmyers<https://github.com/qqmyers> for spotting this and bringing it out our attention. Seems like a great feature! When you click "Explore" the external tool gets the language in use in Dataverse. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#5456 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ApPgt8QNj8Fq8l-mZMWRm3Jv4iJfLVj1ks5vIcKYgaJpZM4Z8CEm>.

[pdurbin]

@manikon hi! Yes, that looks like the same commit I linked to above. This seems like a great addition to the external tool framework. Are you interested in contributing this code? The process is to create an issue and then a pull request. I don't know how closely you work with @JayanthyChengan but she does this all the time. She's been a great contributor. 😄

[pdurbin]

Below is the commit that adds an additional external tool parameter for passing the locale from Dataverse to external applications

See also: Internationalization of external tools #6215