Skip to content

Comments

keep integration (4 widgets) working with Drupal 8 version (under development) of OpenScholar #6381#6486

Merged
kcondon merged 2 commits intodevelopfrom
6381-decode
Jan 14, 2020
Merged

keep integration (4 widgets) working with Drupal 8 version (under development) of OpenScholar #6381#6486
kcondon merged 2 commits intodevelopfrom
6381-decode

Conversation

@pdurbin
Copy link
Member

@pdurbin pdurbin commented Jan 6, 2020

What this PR does / why we need it:

OpenScholar is upgrading to Drupal 8 and some of the Dataverse widgets aren't working. Maybe all.

Which issue(s) this PR closes:

Closes #6381

Special notes for your reviewer:

This is the tiniest fix I could make to fix a simple non-OpenScholar test I tried with these two variations:

original

<script src="http://localhost:8080/resources/js/widgets.js?persistentId=doi:10.5072/FK2/6YRLFR&amp;dvUrl=http://localhost:8080&amp;widget=iframe&amp;heightPx=500"</script>

encoded colons

<script src="http://localhost:8080/resources/js/widgets.js?persistentId=doi%3A10.5072/FK2/6YRLFR&amp;dvUrl=http%3A//localhost:8080&amp;widget=iframe&amp;heightPx=500"</script>

Suggestions on how to test this:

  • Deploy this branch to a server
  • Contact OpenScholar and ask them to point a test server at your Dataverse deployment

Does this PR introduce a user interface change?:

No. In fact, I didn't touch the Dataverse UI where people can copy with widgets from. The old URL with a colon (:) still seemed to work from my simply test.

Is there a release notes update needed for this change?:

Maybe something like "If you are running OpenScholar future version whatever, Dataverse widgets do not work. You must be running this version of Dataverse or newer."

Additional documentation:

With no changes to Dataverse, I was getting this error with the "encoded colons" version above:

Screen Shot 2020-01-06 at 3 59 45 PM

With this pull request, it seems to work, at least for this widget:

Screen Shot 2020-01-06 at 4 00 00 PM

@coveralls
Copy link

coveralls commented Jan 6, 2020

Coverage Status

Coverage decreased (-0.009%) to 19.504% when pulling 4abf83c on 6381-decode into b70cd18 on develop.

@stamina84
Copy link

@pdurbin I see you replace only colon char, what if you use this function to decode the whole string? https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/decodeURIComponent

@pdurbin
Copy link
Member Author

pdurbin commented Jan 7, 2020

@stamina84 thanks, I did think about that and could try it. At the moment the problem is that I don't have a way to test any changes I'm making with OpenScholar. This is because OpenScholar is hard coded to https://dataverse.harvard.edu as we've been discussing at #6381 (comment)

@stamina84
Copy link

Ok, thanks! Then lets Bilsi discuss it with Richard and see whats the next step.

@djbrooke djbrooke assigned djbrooke and unassigned mheppler Jan 7, 2020
@rbran100
Copy link

rbran100 commented Jan 7, 2020

@pdurbin we are going to add a UI component to allow for this to be changed in a vsite, didn't realize there were so many DV installs live since Harvard mostly uses just the one.

@rbran100
Copy link

rbran100 commented Jan 7, 2020

@pdurbin any way to validate the URL is actually a dataverse install?

@pdurbin
Copy link
Member Author

pdurbin commented Jan 7, 2020

any way to validate the URL is actually a dataverse install?

@rbran100 I usually tell people to got to the "version" API endpoint like https://demo.dataverse.org/api/info/version to check for what I guess I'd call a Dataverse "fingerprint". The output should look something like this (but not pretty printed):

{
  "status": "OK",
  "data": {
    "version": "4.18.1",
    "build": "267-a91d370"
  }
}

@djbrooke
Copy link
Contributor

djbrooke commented Jan 7, 2020

Thanks @rbran100 @stamina84, we'll remove this PR from our project board for now and will revisit once the new UI is in place in OS.

@pdurbin
Copy link
Member Author

pdurbin commented Jan 10, 2020

I deployed this pull request my "dev2" server and tried all four widgets. I'm numbering them like this:

  1. Dataset Citation
  2. Dataset
  3. Dataverse Search Box
  4. Dataverse Listing

To make this a little more real, here's a screenshot of all four widgets on a vanilla HTML page:

chat dataverse org_error-documents_widgets html_-_2020-01-10_15 36 45

And here's the vanilla HTML I used:

<h2>1. Dataset Citation</h2>

<script src="https://dev2.dataverse.org/resources/js/widgets.js?persistentId=doi:10.5072/FK2/ASZCUR&amp;dvUrl=https://dev2.dataverse.org&amp;widget=citation&amp;heightPx=350"></script>

<hr>

<h2>2. Dataset</h2> 

<script src="https://dev2.dataverse.org/resources/js/widgets.js?persistentId=doi:10.5072/FK2/ASZCUR&amp;dvUrl=https://dev2.dataverse.org&amp;widget=iframe&amp;heightPx=800"></script>

<hr>

<h2>3. Dataverse Search Box</h2> 

<script src="https://dev2.dataverse.org/resources/js/widgets.js?alias=dataverse-project&amp;dvUrl=https://dev2.dataverse.org&amp;widget=search&amp;text=Search+my+dataverse"></script>

<hr>

<h2>4. Dataverse Listing</h2> 

<script src="https://dev2.dataverse.org/resources/js/widgets.js?alias=dataverse-project&amp;dvUrl=https://dev2.dataverse.org&amp;widgetScope=dataverse-project&amp;widget=iframe&amp;heightPx=800"></script>

Now, on to OpenScholar.

My observation is that the first two widgets don't work in OpenScholar without the fix in this pull request:

  1. Dataset Citation
  2. Dataset

That said, I just got off a Zoom with @Bilsi where we talked about some strange behavior within OpenScholar. Namely:

  • While I can remove "Custom Text/HTML" block by dragging it, it is not possible to remove any of the Dataverse widgets by dragging or any other means. I demoed this to @Bilsi and she will open a bug on the OpenScholar side for this.
  • In one case I created a page with the same title as a page we had just deleted and instead of the page being blank, it has a widget in it that I had been testing with the page I had deleted.

In short, we are very aware of bugs in layout and such in the Drupal 8 version of OpenScholar that's still under development. These bugs should be expected when testing this pull request.

@pdurbin pdurbin changed the title decode %3A as a colon (:) #6381 keep integration (4 widgets) working with Drupal 8 version (under development) of OpenScholar #6381 Jan 10, 2020
Copy link
Contributor

@djbrooke djbrooke left a comment

Choose a reason for hiding this comment

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

Docs look good but someone else should look at the code!

@stamina84
Copy link

@pdurbin Thanks for the testing, I did a tests too, and seems good for me. The query url is "decoded" and embed properly. I think it can be merge if you agree

@sekmiller sekmiller removed their assignment Jan 14, 2020
@kcondon kcondon self-assigned this Jan 14, 2020
@kcondon kcondon merged commit 098611a into develop Jan 14, 2020
@kcondon kcondon deleted the 6381-decode branch January 14, 2020 22:51
@djbrooke djbrooke added this to the 4.19 milestone Jan 15, 2020
@stamina84
Copy link

stamina84 commented Jan 16, 2020

@pdurbin could you please inform me, if its available at https://dataverse.harvard.edu/dataverse/ install? This is what we are using primary, thanks!

@pdurbin
Copy link
Member Author

pdurbin commented Jan 16, 2020

@stamina84 no, sorry, if you look at the bottom right corner of https://dataverse.harvard.edu (screenshot below), you'll see that it's still running 4.18.1.

Screen Shot 2020-01-16 at 7 20 42 AM

In #6506 we just started writing release notes for 4.19 and you are welcome to take a look and give feedback. This pull request will be part of 4.19.

Our release and deploy process is something like this:

I hope this helps! Also, thanks for working on the following issue that I opened!

I know @jggautier @TaniaSchlatter and I have talked a lot about a related issue within Dataverse itself (users not knowing if they should enter "doi:" or not) but I can't find it right now.

@stamina84
Copy link

Ok, thanks for you information, its clear now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

keep integration (4 widgets) working with Drupal 8 version (under development) of OpenScholar

8 participants