Skip to content
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

Modular Explore #3657

Closed
scolapasta opened this issue Mar 1, 2017 · 49 comments
Closed

Modular Explore #3657

scolapasta opened this issue Mar 1, 2017 · 49 comments
Assignees
Labels
Component: Code Infrastructure formerly "Feature: Code Infrastructure" Type: Feature a feature request UX & UI: Design This issue needs input on the design of the UI and from the product owner

Comments

@scolapasta
Copy link
Contributor

scolapasta commented Mar 1, 2017

Right now we have some external tools that we allow users to "explore" with (TwoRavens, GeoConnect). We'd like to make this more modular to make it easier for installations to add new tools.

Ideally, this would involve adding a new table that could know the url of the tool and other needed info (type of file?), but it may also involve having to write a handler for each. For this, we would want to follow the SPI model, and provide a default handler for simple apps.

@pdurbin
Copy link
Member

pdurbin commented May 18, 2017

Some more on the modularity and SPI topic:

Also, https://projects.iq.harvard.edu/dcm2017/agenda shows " SPIs and Modularity: Are you interested in learning about Dataverse's emergent modular architecture?"

@pdurbin pdurbin added UX & UI: Design This issue needs input on the design of the UI and from the product owner Component: Code Infrastructure formerly "Feature: Code Infrastructure" labels Jun 8, 2017
@scolapasta scolapasta changed the title Make "Explore" calls more modular Modular Explore Oct 24, 2017
@djbrooke
Copy link
Contributor

djbrooke commented Nov 1, 2017

  • We'll limit this to the file level for now
  • Towards to goal of small chunks, the scope of this issue is that we want an installation to be able to run Data Explorer and TwoRavens in parallel.
  • @scolapasta will edit this to add additional details

@scolapasta
Copy link
Contributor Author

scolapasta commented Nov 2, 2017

Repeating part of comment from #4230:

Besides the more obvious dynamic text and links we need for external tools, a couple of aspects that we need to handle in a modular way are:

When to show the links to the external tool
How to handle the link to the external tool (i.e. wha dynamic info to send)
In both these cases, we can think of building these in incremental ways, with each step building on the previous step, building from simplest, but supporting fewer tools, to most complex, but also most flexible.

For when to show the link, we have 3 steps:
A. simple list of tools, will only support one operation (i.e. Configure)
B. Add "Operation" to list, in order to support multiple operations (i.e. Configure vs. Explore)
C. Add "Subject of Operation" in order to support operations on different kind of objects (i.e. tools that work on tabular files, like DataExplorer, vs tools that work on geospatial, like World Map.

For how to handle link we similarly have 3 steps:

  1. Dictate the exact parameter names and values that an external tool will need.
  2. Define parameters dynamically from a list of "reserved" terms that we define.
  3. Define individual "Handler" Java classes and SPI interface, in order to handle any tool.

In order to support Modular Explore for Two Ravens and DataExplore, the minimum we technically need is A1. However, since we are working on Modular Configure first, that bumps us to B1. In addition, in this scenario we would need to contact the external tools and have them change their parameters and we would rather be more flexible. So the decision is that the definition for done for this issue is B2.

Note, that we would still not be able to support geoConnect / WorldMap in a modular way yet, until we support at least C2, but per the previous comment, that is out of scope for this issue.

@djbrooke
Copy link
Contributor

djbrooke commented Nov 8, 2017

Thanks @scolapasta for taking the lead on breaking this up before next week's backlog grooming session.

@pdurbin
Copy link
Member

pdurbin commented Nov 22, 2017

@michbarsinai I pinged you in Slack because I said we'll need to write a more intelligent parser. Please see 272b4c2 for a terrible kludge and tests with examples of what we need. Thanks.

@pdurbin
Copy link
Member

pdurbin commented Jan 23, 2018

@kcondon kcondon self-assigned this Jan 23, 2018
mheppler added a commit that referenced this issue Jan 23, 2018
@kcondon
Copy link
Contributor

kcondon commented Jan 23, 2018

Notes to self:

  1. db update script is required to run this.
  2. Significant doc changes with instructions on how to add external tools, including new api endpoints?

@mheppler
Copy link
Contributor

  • Revised popup title to "Dataset Terms" for download/explore workflows on dataset/file pgs.

screen shot 2018-01-23 at 4 15 36 pm

@scolapasta
Copy link
Contributor Author

@pdurbin to respond to your App store comparison, yes the name is important, but behind the scenes what is used (at least in Android) is the package name. Beyond multiple tools with the same name, another concern is the same tool with a changing name, i.e having two rows that should map to the same thing, but don't (or when we internationalize and remove the name from the db).

I generally prefer to tackle issues which would require future database updates that modify data (as opposed to structure) preemptively. I don't see this as an expansion of scope (rather as a good coding practice); the expansion of scope already occurred when we decided to track the specific tool.

@pdurbin
Copy link
Member

pdurbin commented Jan 24, 2018

@scolapasta as I mentioned, I'm interested in controlling the scope of this issue. I'm happy to revert c236079 to make it so we go back to persisting "Explore" rather than "Two Ravens" or "Data Explorer" or whatever. That commit was so recent I expect it will revert cleanly and easily.

@djbrooke in the demo you seemed to like that individual tools are being tracked. What would you like?

@djbrooke
Copy link
Contributor

Good discussion, thanks. I understand the reasoning for IDs instead of names, but let's keep this moving for now. On to QA!

pdurbin added a commit that referenced this issue Jan 24, 2018
pdurbin added a commit that referenced this issue Jan 24, 2018
Other tweaks. Link to current roadmap, waffle.
@kcondon
Copy link
Contributor

kcondon commented Jan 24, 2018

Issues found so far:
-last line in db update script is missing a ;

pdurbin added a commit that referenced this issue Jan 25, 2018
@pdurbin
Copy link
Member

pdurbin commented Jan 25, 2018

@kcondon whoops, good catch. I added the missing semicolon in 92e0a43

As we discussed after standup yesterday there are technically no new API endpoints but the "manifest" format (JSON) has changed in that external tool authors must specify if their tool should appear under the "Explore" button or the "Configure" button.

@kcondon
Copy link
Contributor

kcondon commented Jan 25, 2018

Thanks. So this is not new?
curl -X POST -H 'Content-type: application/json' --upload-file twoRavens.json http://localhost:8080/api/admin/externalTools

@pdurbin
Copy link
Member

pdurbin commented Jan 25, 2018

Well, twoRavens.json is new (in the future, we'd like the TwoRavens team to host this file and #4429 is related) but the API endpoint is not new. It shipped with 4.8.5 and is documented at http://guides.dataverse.org/en/4.8.5/installation/external-tools.html#making-an-external-tool-available-in-dataverse

pdurbin added a commit that referenced this issue Jan 26, 2018
We were seeing "WorldMap" under the dropdown "Explore" button (good!)
and a non-dropdown "Explore" button was appearing next to it (bad!).

We are now making all these explore tools consistent. They are always
available via the dropdown "Explore" button. No more clicking an
"Explore" button and wondering which tool will launch.
@pdurbin
Copy link
Member

pdurbin commented Jan 26, 2018

@kcondon just found a bug (thanks!) where two "Explore" buttons were being displayed side by side (one a dropdown and one a non-drop down). I fixed this in c6c0c87 and left a detailed comment about the fix in the commit.

@kcondon
Copy link
Contributor

kcondon commented Jan 26, 2018

x-Seeing two explore buttons on a shape file that has already been mapped.

@pdurbin
Copy link
Member

pdurbin commented Jun 6, 2018

Known issues:

  • When you launch an external tool from a popup the tool will take over the browser window rather than opening in a new tab. When there is no popup, the tool launches in a new tab, which is the desired behavior. This bug exists in production for TwoRavens and has been discussed with Gustavo, Steve, and Mike but we don't have a fix. The inconsistency is unfortunate.

This is now being tracked at #4742.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Code Infrastructure formerly "Feature: Code Infrastructure" Type: Feature a feature request UX & UI: Design This issue needs input on the design of the UI and from the product owner
Projects
None yet
Development

No branches or pull requests