inveniosoftware / invenio-app-rdm

Turn-key research data management platform.
https://inveniordm.docs.cern.ch
MIT License
110 stars 150 forks source link

edit: if record already has DOI, UI should reflect that correctly #1006

Open tmorrell opened 3 years ago

tmorrell commented 3 years ago

Describe the bug

If you edit the metadata for a record where InvenioRDM has minted a DOI, the "Do you already have a DOI for this upload?" field says No. This doesn't make sense, since the record has a DOI. The deposit form correctly doesn't allow you to change the DOI or answer, so this is purely a UI issue.

Steps to Reproduce

  1. Create a record with a DOI
  2. Click on Edit
  3. Look at the DOI section

Expected behavior

There should not be confusing DOI status information when editing record metadata. The help text is also confusing. It probably makes sense to remove the DOI question entirely and change the help text to something like "This record has been assigned a DOI, which cannot be changed."

Screenshots (if applicable)

Screen Shot 2021-07-21 at 12 42 23 PM
github-actions[bot] commented 3 years ago

This issue was automatically marked as stale.

github-actions[bot] commented 2 years ago

This issue was automatically marked as stale.

github-actions[bot] commented 2 years ago

This issue was automatically marked as stale.

fenekku commented 1 year ago

:wave: @tmorrell @chriz-uniba You both seemed to be interested in improvements to the DOI component, so I wanted to share this with you.

I've created a modified version of the current DOI component for us at NU and thought here would be a good place to get your feedback on it. (I didn't really know of a better place to gather feedback from a small group so I am doing it here. If you know a better place let me know - the rapids of Discord would probably drown it).

Our criteria at Northwestern for the component were:

The resulting screenshots look like:

image

TODO in the future:

To achieve this I overrode the PIDField (and I mean completely). I didn't use the CustomPIDField because it's not exported and didn't try to copy-and-tweak it because its abstraction made it harder for me to change (and I suspect was why it had a couple of glitches :smile: ).

Anyway I wanted to gauge interest and see how we could proceed. Because it's a larger change it may be more challenging to get in the mainline.

tmorrell commented 1 year ago

@fenekku This looks really clean and like a great improvement. However, I am missing the "Get DOI now" button for a couple of reasons

So my thought would be to have the "get DOI" button as an option to the right of No. Otherwise the functionality works much better than the current confusing version.

fenekku commented 1 year ago

Ah yes. I think that focuses the discussion on how to configure an InvenioRDM component from an instance and how to share custom components in the InvenioRDM ecosystem.

Overriding via react-overridable is essentially the way right now. In mapping.js, the original component is imported and wrapped in another. The original components' props are set to what is needed and the parent component is set to override. It's a little intrusive, but it does allow an instance to really configure.

I guess the alternative could be to have InvenioRDM components take a config props derived from the backend (from some settings really). Then an instance would only have to change some settings to populate that config props appropriately and have it in turn configure pre-existing components... I tend to be less attracted by that solution, because ultimately it's already possible by override and the configuration approach tends to make the resulting component very abstract to the point it becomes hard to understand what is going on. However, the configuration approach does cut down on some boilerplate that each instance would have to write in some cases...

To use a component shared by a third-party, there'd have to be better support for that. Right now an installer would have to manually add the {{ webpack['component-key'] }} to their deposit page html I think... Something to bring up to people more familiar with webpack/assets building.

Thanks!

chriz-uniba commented 1 year ago

hi,

thanks a lot for tackling this!

I like your approach.

fenekku commented 1 year ago

that No was the last answer, even though you have somewhere laying around the "filled text input" - please do not use it ;)

Yes, I remember seeing this issue in the past somewhere. Here the form is explicitly altered to remove the Yes value, so we should be good. :+1:

"I arelady have a DOI" [...] wording issue

We haven't encountered the issue directly yet, so we don't know what kind of wording would work. We'll keep our eye on it though.

"Get DOI now"

So far this is my understanding of this whole thing:

What we are toying with is to provide a text next to the "No" option when the record is saved that says something like: "Once your upload is published, its DOI will be 10.XYZ/1234-abcd". This can be done for DOI confidently without additional API calls.

Thanks!