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.

Sign up for GitHub

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

Jump to bottom

3657 modular explore #4407

Closed
wants to merge 29 commits into from
Closed

3657 modular explore #4407

wants to merge 29 commits into from

Conversation

pdurbin
Copy link
Member

@pdurbin pdurbin commented Jan 10, 2018 •
edited
Loading

original PR for #3657 Modular Explore, superseded by pull request #4426

pdurbin and others added 14 commits January 3, 2018 16:00
@pdurbin
stub out modular explore #3657 #4249
5ae8418
Update the description for the Data Explorer
488b604
@pdurbin
Merge pull request #4397 from kevinworthington/patch-2
bd17812 
Update the description for the Data Explorer
@pdurbin
get test passing, {siteUrl} is now a valid reserved word #3657
61c4936
@pdurbin
differentiate external tools under own dropdown for now #3657
6538b93
@pdurbin
make external explore tools available on dataset page too #3657
b3be37e
@pdurbin
Data Explorer expects fileId not fileid #4249 #3657
e873c6e
@pdurbin
add type to externaltool table for "explore" vs "configure" #3657
233cdc1
@pdurbin
externaltool type enum is stored in ALL CAPS #3657
bcca40b
@pdurbin
put external tools under Explore button dropdown #3657
5d18067
@pdurbin
have external tools respect downloadPopupRequired #3657
0e1c1e9 
Note the FIXME that this is working from the file page but not the
dataset page.
@pdurbin
mention external tools in Data Exploration Guide #3657
79161d0
@pdurbin
get TwoRavens working again (FIXME for external tools) #3657
f460d5b 
Here we are differentiating between popups for TwoRavens ("explore") and
popups for external tools ("externalTool").
@pdurbin
get external tool popup working on dataset page #3657
ef87fde
@pdurbin pdurbin mentioned this pull request Jan 10, 2018
Closed
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 12.861% when pulling ef87fde on 3657-modular-explore into 0d42682 on develop.

@pdurbin
only show explore tools for tabular data #3657
4204279 
We don't want to show explore tools for images and such until there is a
use case, at which time we can add a column to the externaltool table.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 12.859% when pulling 4204279 on 3657-modular-explore into 0d42682 on develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 12.859% when pulling 0f55916 on 3657-modular-explore into 0d42682 on develop.

@sekmiller sekmiller self-requested a review January 11, 2018 19:14
sekmiller
sekmiller reviewed Jan 11, 2018
View reviewed changes
Copy link
Contributor

@sekmiller sekmiller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest that Two Ravens be treated as any other external tool. This would help to simplify the file download-button-fragment. Also, the explore on the FileDownloadServiceBean should be called from the FileDownloadHelper in order for the validator to work. This will have to be done after merging #4393.

@pdurbin
Copy link
Member Author

pdurbin commented Jan 11, 2018

the explore on the FileDownloadServiceBean should be called from the FileDownloadHelper in order for the validator to work

@sekmiller thanks. As we've been discussing, there's a good chance we'll close this pull request and put the new combo pull request at #4413 through QA instead.

@pdurbin
Merge branch 'develop' into 3657-modular-explore #3657
4a91c7d 
Conflicts (just imports):
src/main/java/edu/harvard/iq/dataverse/GuestbookResponseServiceBean.java
@pdurbin
get validation working on external tools #3657
9c1502d
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 12.83% when pulling 9c1502d on 3657-modular-explore into 9f71258 on develop.

@pdurbin pdurbin mentioned this pull request Jan 12, 2018
Closed
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 12.827% when pulling 9c1502d on 3657-modular-explore into 9f71258 on develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 12.828% when pulling 515c351 on 3657-modular-explore into 9f71258 on develop.

@pdurbin
simplify SQL update script (no UPDATE) #3657
eded8ef 
We haven't shipped a release with the externaltool table yet so people
shouldn't have any existing data.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 12.828% when pulling eded8ef on 3657-modular-explore into 9f71258 on develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 12.828% when pulling f9d11d9 on 3657-modular-explore into 9f71258 on develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 12.827% when pulling f9d11d9 on 3657-modular-explore into 9f71258 on develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 12.828% when pulling 05dd4bf on 3657-modular-explore into 9f71258 on develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 12.829% when pulling b502c21 on 3657-modular-explore into 9f71258 on develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 12.834% when pulling 6f48df7 on 3657-modular-explore into 9f71258 on develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 12.834% when pulling 332c358 on 3657-modular-explore into 9f71258 on develop.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 5826fea on 3657-modular-explore into ** on develop**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 5dee126 on 3657-modular-explore into ** on develop**.

@pdurbin
Copy link
Member Author

pdurbin commented Jan 22, 2018

Closing in favor of pull request #4426.

@pdurbin pdurbin closed this Jan 22, 2018
@pdurbin pdurbin deleted the 3657-modular-explore branch January 30, 2018 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sekmiller sekmiller sekmiller left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pdurbin @coveralls @sekmiller