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

custom JavaScript setting for metadata editing #8723

Merged
merged 11 commits into from
Sep 21, 2022

Conversation

ErykKul
Copy link
Collaborator

@ErykKul ErykKul commented May 20, 2022

What this PR does / why we need it:
It allows loading a custom JavaScript for editing metadata.

Which issue(s) this PR closes:

Closes #8722

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

Additional documentation:
For example, when you want a metadata field to be controlled by an external set of possible values that do not have translations or URIs, you can implement a simple lookup in a JavaScript. An example of a person name lookup from within an organization:

image

@coveralls
Copy link

coveralls commented May 20, 2022

Coverage Status

Coverage increased (+0.0003%) to 20.035% when pulling 39fa64d on ErykKul:8722_custom_javascript into c7b8b82 on IQSS:develop.

@qqmyers
Copy link
Member

qqmyers commented May 20, 2022

If all you want is to drop an arbitrary script on the page, perhaps adding it to a custom header or footer would work? FWIW: I did this in developing the ORCID script prior to the CVoc mechanism being in place.

@Kris-LIBIS
Copy link
Contributor

@qqmyers I tried that, but that did not work as expected. The script's document.ready function needs to run to add the search button and attributes to the metadata input fields to attach the javascript to the DOM elements. With repeating fields like author, this has to happen each time a new entry is added.

When I loaded the javascript in the footer, I noticed that the ready function was only triggered when the user opens the dataset, but no more when changing to the metadata tab, nor when the user clicks 'Edit metadata'.

By using the javascript loading from the external vocabularies, it works, but that has some disadvantages (see the related issue).

@pdurbin pdurbin self-assigned this Sep 8, 2022
Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

I'm close to approving this but I made some doc suggestions here: ErykKul#3

Also, can I have a covoc.js file to play with? Otherwise there's not much to test or review. The js file is loaded just fine.

@ErykKul
Copy link
Collaborator Author

ErykKul commented Sep 9, 2022

@pdurbin

Some extra explanation about the location of the javascript and how it is used at KU Leuven:

We have a reverse proxy that manages relative paths. /covoc/ points to ruby server that also implements the business logic calls to our internal (KU Leuven) servers. Since everything is behind a reverse proxy, you can enforce same origin policies (CORS) at the browser, etc. Reverse proxy, dataverse, ruby server and some other servers (e.g., database) are then docker images served by one docker with internal network with only the reverse proxy being exposed directly. It is a nice architecture done by @Kris-LIBIS for us.

The specific script we want to load: /covoc/js/covoc.js has relative path to the reverse proxy where dataverse is hosted. For example, in our production it becomes: https://rdr.kuleuven.be/covoc/js/covoc.js (you can just click on the link to view it in the browser). Note that it uses relative paths itself, and it won't work for you without the covoc ruby backend deployed.

@pdurbin
Copy link
Member

pdurbin commented Sep 9, 2022

@ErykKul thanks! All the extra context helps a lot. I have no plans to deploy the Ruby backend, etc. 😄 I confirmed that when I use the new setting I can insert Javascript into the dataset page from a URL (to a Javascript file, of course).

Based on this, I'm approving this PR and sending it to QA.

Something for all of us to think about... we allow a custom CSS file (:StyleCustomizationFile). Should we allow a custom Javascript file (:JavascriptCustomizationFile?). Or multiple files? In practice, people tend to add Javascript files in a custom header or footer (as @qqmyers mentions above). This PR includes the extra Javascript on just the dataset page rather than all pages. @Kris-LIBIS mentioned above that the script didn't work in the footer so it seems like the need for this PR is real.

Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

While I didn't test a Javascript file that actual does something useful like populate author names (@ErykKul explained they have a whole Ruby backend for that), I did confirm that I can add Javascript to the dataset page and at least print using console.log.

There's additional discussion in the PR about alternatives such as putting Javascript in a custom header or footer but the assertion is that this didn't work.

Approving. Off to QA.

@pdurbin pdurbin removed their assignment Sep 9, 2022
@pdurbin
Copy link
Member

pdurbin commented Sep 12, 2022

At standup I said I was happy enough with this PR to move it to QA but I'd like another set of eyes on it before we merge.

@landreev landreev self-assigned this Sep 14, 2022
@landreev
Copy link
Contributor

OK, merging.

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.

Feature Request/Idea: adding custom controlled vocabulary JavaScript script
6 participants