-
Notifications
You must be signed in to change notification settings - Fork 89
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
doi: handle UI for optional DOI feature #1896
doi: handle UI for optional DOI feature #1896
Conversation
33af9cd
to
404339b
Compare
e77fb48
to
b6287de
Compare
...ssets/semantic-ui/js/invenio_rdm_records/src/deposit/controls/PublishButton/PublishButton.js
Outdated
Show resolved
Hide resolved
0d0b336
to
57f5113
Compare
Current implementation First version - no or external DOIIn any subsequent version, we check on the backend that you cannot ask for a locally managed DOI ScreenshotFirst version - locally managed DOIIn any subsequent version, we check on the backend that you cannot add an external or omit DOI Screenshot |
57f5113
to
fdea96d
Compare
const errorData = error.response.data; | ||
let errorData = error.response.data; | ||
const errors = this.recordSerializer.deserializeErrors( | ||
error.response.data.errors || [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need the empty array?
isManagedSelected: | ||
isDraft === true && value?.identifier && value?.provider !== PROVIDER_EXTERNAL | ||
? true | ||
: undefined, | ||
isNoNeedSelected: undefined, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isManagedSelected: | |
isDraft === true && value?.identifier && value?.provider !== PROVIDER_EXTERNAL | |
? true | |
: undefined, | |
isNoNeedSelected: undefined, | |
const isInternalProvider = value?.provider !== PROVIDER_EXTERNAL; | |
const isDraft = record?.is_draft == true; | |
const hasIdentifier = value?.identifier; | |
const isManagedSelected = isDraft && hasIdentifier && isInternalProvider ? true : undefined; | |
this.state = { | |
isManagedSelected: isManagedSelected; | |
isNoNeedSelected: undefined, |
also, why do we set isManagedSelected to undefined
not false
?
@@ -407,40 +442,72 @@ class CustomPIDField extends Component { | |||
} | |||
|
|||
const hasManagedIdentifier = managedIdentifier !== ""; | |||
const hasUnmanagedIdentifier = unmanagedIdentifier !== ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we set empty string ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The initialization happens above and here we check if the value is not empty string. I am confused about the question :)
fdea96d
to
47712af
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (reviewed IRL as well)
47712af
to
08f080b
Compare
* closes CERNDocumentServer/cds-rdm#163 Co-authored-by: Zacharias Zacharodimos <[email protected]>
08f080b
to
1c177e1
Compare
If DOI is required:
If DOI is not required:
Different options overview:
Errors displaying that DOI was needed, but was not reserved (on publish of the draft):