diff --git a/invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/api/DepositApiClient.js b/invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/api/DepositApiClient.js index 0d35c2c8d..7a443698a 100644 --- a/invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/api/DepositApiClient.js +++ b/invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/api/DepositApiClient.js @@ -103,7 +103,12 @@ export class RDMDepositApiClient extends DepositApiClient { ); return new DepositApiClientResponse(data, errors); } catch (error) { - const errorData = error.response.data; + let errorData = error.response.data; + const errors = this.recordSerializer.deserializeErrors( + error.response.data.errors || [] + ); + // this is to serialize raised error from the backend on publish + if (errors) errorData = errors; throw new DepositApiClientResponse({}, errorData); } } diff --git a/invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/controls/PublishButton/PublishButton.js b/invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/controls/PublishButton/PublishButton.js index 71dafb704..34518b157 100644 --- a/invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/controls/PublishButton/PublishButton.js +++ b/invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/controls/PublishButton/PublishButton.js @@ -18,6 +18,8 @@ import { DepositFormSubmitContext, } from "../../api/DepositFormSubmitContext"; import { DRAFT_PUBLISH_STARTED } from "../../state/types"; +import { scrollTop } from "../../utils"; +import { DRAFT_PUBLISH_FAILED_WITH_VALIDATION_ERRORS } from "../../state/types"; class PublishButtonComponent extends Component { state = { isConfirmModalOpen: false }; @@ -30,14 +32,36 @@ class PublishButtonComponent extends Component { handlePublish = (event, handleSubmit, publishWithoutCommunity) => { const { setSubmitContext } = this.context; - - setSubmitContext( - publishWithoutCommunity - ? DepositFormSubmitActions.PUBLISH_WITHOUT_COMMUNITY - : DepositFormSubmitActions.PUBLISH - ); - handleSubmit(event); - this.closeConfirmModal(); + const { formik, raiseDOINeededButNotReserved, isDOIRequired } = this.props; + const noINeedOne = formik?.values?.noINeedOne; + // Check for explicit DOI reservation via the "GET DOI button" only when DOI is + // optional in the instance's settings. If it is required, backend will automatically + // mint one even if it was not explicitly reserved + const shouldCheckForExplicitDOIReservation = + isDOIRequired !== undefined && // isDOIRequired is undefined when no value was provided from Invenio-app-rdm + !isDOIRequired && + noINeedOne && + Object.keys(formik?.values?.pids).length === 0; + if (shouldCheckForExplicitDOIReservation) { + const errors = { + pids: { + doi: i18next.t("DOI is needed. Please click on the button to reserve it."), + }, + }; + formik.setErrors(errors); + raiseDOINeededButNotReserved(formik?.values, errors); + this.closeConfirmModal(); + } else { + setSubmitContext( + publishWithoutCommunity + ? DepositFormSubmitActions.PUBLISH_WITHOUT_COMMUNITY + : DepositFormSubmitActions.PUBLISH + ); + handleSubmit(event); + this.closeConfirmModal(); + } + // scroll top to show the global error + scrollTop(); }; isDisabled = (values, isSubmitting, filesState) => { @@ -67,6 +91,7 @@ class PublishButtonComponent extends Component { publishWithoutCommunity, formik, publishModalExtraContent, + raiseDOINeededButNotReserved, ...ui } = this.props; const { isConfirmModalOpen } = this.state; @@ -139,6 +164,8 @@ PublishButtonComponent.propTypes = { formik: PropTypes.object.isRequired, publishModalExtraContent: PropTypes.string, filesState: PropTypes.object, + raiseDOINeededButNotReserved: PropTypes.func.isRequired, + isDOIRequired: PropTypes.bool, }; PublishButtonComponent.defaultProps = { @@ -147,15 +174,22 @@ PublishButtonComponent.defaultProps = { actionState: undefined, publishModalExtraContent: undefined, filesState: undefined, + isDOIRequired: undefined, }; const mapStateToProps = (state) => ({ actionState: state.deposit.actionState, publishModalExtraContent: state.deposit.config.publish_modal_extra, filesState: state.files, + isDOIRequired: state.deposit.config.is_doi_required, }); -export const PublishButton = connect( - mapStateToProps, - null -)(connectFormik(PublishButtonComponent)); +export const PublishButton = connect(mapStateToProps, (dispatch) => { + return { + raiseDOINeededButNotReserved: (data, errors) => + dispatch({ + type: DRAFT_PUBLISH_FAILED_WITH_VALIDATION_ERRORS, + payload: { data: data, errors: errors }, + }), + }; +})(connectFormik(PublishButtonComponent)); diff --git a/invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/fields/Identifiers/PIDField.js b/invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/fields/Identifiers/PIDField.js index bd5b00cae..e49437ef1 100644 --- a/invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/fields/Identifiers/PIDField.js +++ b/invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/fields/Identifiers/PIDField.js @@ -33,7 +33,7 @@ const getFieldErrors = (form, fieldPath) => { */ class ReservePIDBtn extends Component { render() { - const { disabled, handleReservePID, label, loading } = this.props; + const { disabled, handleReservePID, label, loading, fieldError } = this.props; return ( {({ form: formik }) => ( @@ -44,6 +44,7 @@ class ReservePIDBtn extends Component { disabled={disabled || loading} onClick={(e) => handleReservePID(e, formik)} content={label} + error={fieldError} /> )} @@ -54,6 +55,7 @@ class ReservePIDBtn extends Component { ReservePIDBtn.propTypes = { disabled: PropTypes.bool, handleReservePID: PropTypes.func.isRequired, + fieldError: PropTypes.object, label: PropTypes.string.isRequired, loading: PropTypes.bool, }; @@ -61,6 +63,7 @@ ReservePIDBtn.propTypes = { ReservePIDBtn.defaultProps = { disabled: false, loading: false, + fieldError: null, }; /** @@ -110,11 +113,13 @@ class ManagedUnmanagedSwitch extends Component { handleChange = (e, { value }) => { const { onManagedUnmanagedChange } = this.props; const isManagedSelected = value === "managed"; - onManagedUnmanagedChange(isManagedSelected); + const isNoNeedSelected = value === "notneeded"; + onManagedUnmanagedChange(isManagedSelected, isNoNeedSelected); }; render() { - const { disabled, isManagedSelected, pidLabel } = this.props; + const { disabled, isManagedSelected, isNoNeedSelected, pidLabel, required } = + this.props; return ( @@ -123,28 +128,41 @@ class ManagedUnmanagedSwitch extends Component { pidLabel: pidLabel, })} - + - + + {!required && ( + + + + )} ); } @@ -153,8 +171,10 @@ class ManagedUnmanagedSwitch extends Component { ManagedUnmanagedSwitch.propTypes = { disabled: PropTypes.bool, isManagedSelected: PropTypes.bool.isRequired, + isNoNeedSelected: PropTypes.bool.isRequired, onManagedUnmanagedChange: PropTypes.func.isRequired, pidLabel: PropTypes.string, + required: PropTypes.bool.isRequired, }; ManagedUnmanagedSwitch.defaultProps = { @@ -198,6 +218,8 @@ class ManagedIdentifierComponent extends Component { identifier, pidPlaceholder, pidType, + form, + fieldPath, } = this.props; const hasIdentifier = identifier !== ""; @@ -209,6 +231,7 @@ class ManagedIdentifierComponent extends Component { actionState === RESERVE_PID_STARTED && actionStateExtra.pidType === pidType } handleReservePID={this.handleReservePID} + fieldError={getFieldErrors(form, fieldPath)} /> ); @@ -253,6 +276,8 @@ ManagedIdentifierComponent.propTypes = { btnLabelDiscardPID: PropTypes.string.isRequired, pidPlaceholder: PropTypes.string.isRequired, pidType: PropTypes.string.isRequired, + form: PropTypes.object.isRequired, + fieldPath: PropTypes.string.isRequired, /* from Redux */ actionState: PropTypes.string, actionStateExtra: PropTypes.object, @@ -307,7 +332,7 @@ class UnmanagedIdentifierCmp extends Component { render() { const { localIdentifier } = this.state; - const { form, fieldPath, helpText, pidPlaceholder } = this.props; + const { form, fieldPath, helpText, pidPlaceholder, disabled } = this.props; const fieldError = getFieldErrors(form, fieldPath); return ( <> @@ -318,6 +343,7 @@ class UnmanagedIdentifierCmp extends Component { placeholder={pidPlaceholder} width={16} error={fieldError} + disabled={disabled} /> {helpText && } @@ -333,10 +359,12 @@ UnmanagedIdentifierCmp.propTypes = { identifier: PropTypes.string.isRequired, onIdentifierChanged: PropTypes.func.isRequired, pidPlaceholder: PropTypes.string.isRequired, + disabled: PropTypes.bool, }; UnmanagedIdentifierCmp.defaultProps = { helpText: null, + disabled: false, }; /** @@ -349,11 +377,18 @@ class CustomPIDField extends Component { constructor(props) { super(props); - const { canBeManaged, canBeUnmanaged } = this.props; + const { canBeManaged, canBeUnmanaged, record, field } = this.props; this.canBeManagedAndUnmanaged = canBeManaged && canBeUnmanaged; + const value = field?.value; + 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: undefined, + isManagedSelected: isManagedSelected, + isNoNeedSelected: undefined, }; } @@ -373,7 +408,7 @@ class CustomPIDField extends Component { }; render() { - const { isManagedSelected } = this.state; + const { isManagedSelected, isNoNeedSelected } = this.state; const { btnLabelDiscardPID, btnLabelGetPID, @@ -394,6 +429,8 @@ class CustomPIDField extends Component { record, } = this.props; + let { doiDefaultSelection } = this.props; + const value = field.value || {}; const currentIdentifier = value.identifier || ""; const currentProvider = value.provider || ""; @@ -407,19 +444,43 @@ class CustomPIDField extends Component { } const hasManagedIdentifier = managedIdentifier !== ""; + const hasUnmanagedIdentifier = unmanagedIdentifier !== ""; + const doi = record?.pids?.doi?.identifier || ""; + const parentDoi = record.parent?.pids?.doi?.identifier || ""; + + const hasDoi = doi !== ""; + const hasParentDoi = parentDoi !== ""; + const isDoiCreated = currentIdentifier !== ""; + const isDraft = record.is_draft; + + const _isUnmanagedSelected = + isManagedSelected === undefined + ? hasUnmanagedIdentifier || + (currentIdentifier === "" && doiDefaultSelection === "yes") + : !isManagedSelected; const _isManagedSelected = isManagedSelected === undefined - ? hasManagedIdentifier || currentProvider === "" // i.e pids: {} + ? hasManagedIdentifier || + (currentIdentifier === "" && doiDefaultSelection === "no") // i.e pids: {} : isManagedSelected; - const doi = record?.pids?.doi?.identifier || ""; - const hasDoi = doi !== ""; - const isDoiCreated = currentIdentifier !== ""; + const _isNoNeedSelected = + isNoNeedSelected === undefined + ? (!_isManagedSelected && !_isUnmanagedSelected) || + (isDraft !== true && + currentIdentifier === "" && + doiDefaultSelection === "not_needed") + : isNoNeedSelected; + const fieldError = getFieldErrors(form, fieldPath); + return ( <> - + @@ -427,20 +488,32 @@ class CustomPIDField extends Component { { + isNoNeedSelected={_isNoNeedSelected} + onManagedUnmanagedChange={(userSelectedManaged, userSelectedNoNeed) => { if (userSelectedManaged) { form.setFieldValue("pids", {}); + if (!required) { + // We set the + form.setFieldValue("noINeedOne", true); + } + } else if (userSelectedNoNeed) { + form.setFieldValue("pids", {}); + form.setFieldValue("noINeedOne", false); } else { this.onExternalIdentifierChanged(""); + form.setFieldValue("noINeedOne", false); } + form.setFieldError(fieldPath, false); this.setState({ isManagedSelected: userSelectedManaged, + isNoNeedSelected: userSelectedNoNeed, }); }} pidLabel={pidLabel} + required={required} /> )} @@ -450,6 +523,7 @@ class CustomPIDField extends Component { btnLabelDiscardPID={btnLabelDiscardPID} btnLabelGetPID={btnLabelGetPID} form={form} + fieldPath={fieldPath} identifier={managedIdentifier} helpText={managedHelpText} pidPlaceholder={pidPlaceholder} @@ -458,7 +532,7 @@ class CustomPIDField extends Component { /> )} - {canBeUnmanaged && !_isManagedSelected && ( + {canBeUnmanaged && (!_isManagedSelected || _isNoNeedSelected) && ( { @@ -468,6 +542,7 @@ class CustomPIDField extends Component { fieldPath={fieldPath} pidPlaceholder={pidPlaceholder} helpText={unmanagedHelpText} + disabled={_isNoNeedSelected || isEditingPublishedRecord} /> )} @@ -493,6 +568,7 @@ CustomPIDField.propTypes = { required: PropTypes.bool.isRequired, unmanagedHelpText: PropTypes.string, record: PropTypes.object.isRequired, + doiDefaultSelection: PropTypes.object.isRequired, }; CustomPIDField.defaultProps = { @@ -542,6 +618,7 @@ PIDField.propTypes = { required: PropTypes.bool, unmanagedHelpText: PropTypes.string, record: PropTypes.object.isRequired, + doiDefaultSelection: PropTypes.object.isRequired, }; PIDField.defaultProps = { diff --git a/invenio_rdm_records/config.py b/invenio_rdm_records/config.py index 9ea88dddd..fdb96df32 100644 --- a/invenio_rdm_records/config.py +++ b/invenio_rdm_records/config.py @@ -391,6 +391,7 @@ def always_valid(identifier): "validator": idutils.is_doi, "normalizer": idutils.normalize_doi, "is_enabled": providers.DataCitePIDProvider.is_enabled, + "ui": {"default_selected": "yes"}, # "yes", "no" or "not_needed" }, "oai": { "providers": ["oai"], diff --git a/invenio_rdm_records/records/api.py b/invenio_rdm_records/records/api.py index 377ef6c82..0c017db3b 100644 --- a/invenio_rdm_records/records/api.py +++ b/invenio_rdm_records/records/api.py @@ -536,10 +536,40 @@ def get_latest_published_by_parent(cls, parent): published yet or all versions are deleted. """ latest_record = cls.get_latest_by_parent(parent) - if latest_record.deletion_status != RecordDeletionStatusEnum.PUBLISHED.value: + if ( + latest_record + and latest_record.deletion_status + != RecordDeletionStatusEnum.PUBLISHED.value + ): return None return latest_record + @classmethod + def get_previous_published_by_parent(cls, parent): + """Get the previous of latest published record for the specified parent record. + + It might return None if there is no latest published version i.e not + published yet or all versions are deleted or there is only one published record. + + This method is needed instead of `get_latest_published_by_parent` because during + publish the version state is updated before the record is actually published. + That means, that `get_latest_published_by_parent` returns always the record that + is about to be published and thus, we cannot use it in the `component.publish()` + method to retrieve the actual last published record. + + Check `services.components.pids.PIDsComponent.publish()` for how it is used. + """ + # We need no_autoflush because the record.versions access triggers automatically + # one + with db.session.no_autoflush: + records = cls.get_records_by_parent(parent) + for record in records: + latest_version_index = record.versions.latest_index + if latest_version_index > 1: + if record.versions.index == latest_version_index - 1: + return record + return None + RDMFileRecord.record_cls = RDMRecord diff --git a/invenio_rdm_records/resources/serializers/datacite/schema.py b/invenio_rdm_records/resources/serializers/datacite/schema.py index 82d685e41..6e8c00544 100644 --- a/invenio_rdm_records/resources/serializers/datacite/schema.py +++ b/invenio_rdm_records/resources/serializers/datacite/schema.py @@ -418,7 +418,7 @@ def get_related_identifiers(self, obj): params={"_source_includes": "pids.doi"}, ) for version in record_versions: - version_doi = version["pids"]["doi"] + version_doi = version.get("pids", {}).get("doi") id_scheme = get_scheme_datacite( "doi", "RDM_RECORDS_IDENTIFIERS_SCHEMES", diff --git a/invenio_rdm_records/services/components/pids.py b/invenio_rdm_records/services/components/pids.py index 8a08a67b2..07d14d4f0 100644 --- a/invenio_rdm_records/services/components/pids.py +++ b/invenio_rdm_records/services/components/pids.py @@ -15,14 +15,76 @@ from flask import current_app from invenio_drafts_resources.services.records.components import ServiceComponent from invenio_drafts_resources.services.records.uow import ParentRecordCommitOp +from invenio_i18n import lazy_gettext as _ from invenio_records_resources.services.uow import TaskOp +from ..errors import ValidationErrorWithMessageAsList from ..pids.tasks import register_or_update_pid class PIDsComponent(ServiceComponent): """Service component for PIDs.""" + ALLOWED_DOI_PROVIDERS_TRANSITIONS = { + "datacite": { + "allowed_providers": ["datacite"], + "message": _( + "A previous version used a DOI registered from {sitename}. This version must also use a DOI from {sitename}." + ), + }, + "external": { + "allowed_providers": ["external", "not_needed"], + "message": _( + "A previous version was published with a DOI from an external provider or without one. You cannot use a DOI registered from {sitename} for this version." + ), + }, + "not_needed": { + "allowed_providers": ["external", "not_needed"], + "message": _( + "A previous version was published with a DOI from an external provider or without one. You cannot use a DOI registered from {sitename} for this version." + ), + }, + } + + def _validate_doi_transition( + self, new_provider, previous_published_provider, errors=None + ): + """If DOI is not required then we validate allowed DOI providers. + + Each new version that is published must follow the ALLOWED_DOI_PROVIDERS_TRANSITIONS. + """ + sitename = current_app.config.get("THEME_SITENAME", "this repository") + sitename = current_app.config.get("THEME_SITENAME", "this repository") + + valid_transitions = self.ALLOWED_DOI_PROVIDERS_TRANSITIONS.get( + previous_published_provider, {} + ) + if new_provider not in valid_transitions.get("allowed_providers", []): + error_message = { + "field": "pids.doi", + "messages": [ + valid_transitions.get("message").format(sitename=sitename) + ], + } + + if errors is not None: + errors.append(error_message) + else: + raise ValidationErrorWithMessageAsList(message=[error_message]) + + def _validate_optional_doi(self, record, previous_published, errors=None): + """Reusable method to validate optional DOI.""" + if previous_published: + previous_published_pids = previous_published.get("pids", {}) + doi_pid = [pid for pid in record.pids.values() if "doi" in record.pids] + previous_published_provider = previous_published_pids.get("doi", {}).get( + "provider", "not_needed" + ) + new_provider = "not_needed" if not doi_pid else doi_pid[0]["provider"] + self._validate_doi_transition( + new_provider, previous_published_provider, errors + ) + def create(self, identity, data=None, record=None, errors=None): """This method is called on draft creation. @@ -41,6 +103,16 @@ def update_draft(self, identity, data=None, record=None, errors=None): if "pids" in data: # there is new input data for PIDs pids_data = data["pids"] + required_schemes = set(self.service.config.pids_required) + + # if DOI is not required in an instance check validate allowed providers + # for each record version + if "doi" not in required_schemes: + previous_published = self.service.record_cls.get_latest_published_by_parent( + record.parent + ) + self._validate_optional_doi(record, previous_published, errors) + self.service.pids.pid_manager.validate(pids_data, record, errors) record.pids = pids_data @@ -63,6 +135,7 @@ def delete_draft(self, identity, draft=None, record=None, force=False): def publish(self, identity, draft=None, record=None): """Publish handler.""" + # ATTENTION: A draft can be for both an unpublished or published # record. For an unpublished record, we usually simply need to create # and reserve all PIDs. For a published record, some PIDs may allow @@ -75,7 +148,19 @@ def publish(self, identity, draft=None, record=None): record_schemes = set(record_pids.keys()) required_schemes = set(self.service.config.pids_required) - # Validate the draft PIDs + # if DOI is not required in an instance check validate allowed providers + # for each record version + if "doi" not in required_schemes: + # if a doi was ever minted for the parent record then we always require one + # for any version of the record that will be published + if draft.parent.get("pids", {}).get("doi"): + required_schemes.add("doi") + + previous_published = ( + self.service.record_cls.get_previous_published_by_parent(record.parent) + ) + self._validate_optional_doi(draft, previous_published) + self.service.pids.pid_manager.validate(draft_pids, draft, raise_errors=True) # Detect which PIDs on a published record that has been changed. @@ -129,12 +214,7 @@ def publish(self, identity, draft=None, record=None): def new_version(self, identity, draft=None, record=None): """A new draft should not have any pids from the previous record.""" - # This makes the draft use the same identifier as the previous - # version - if record.pids.get("doi", {}).get("provider") == "external": - draft.pids = {"doi": {"provider": "external", "identifier": ""}} - else: - draft.pids = {} + draft.pids = {} def edit(self, identity, draft=None, record=None): """Add current pids from the record to the draft. @@ -172,6 +252,14 @@ def publish(self, identity, draft=None, record=None): current_schemes = set(current_pids.keys()) required_schemes = set(self.service.config.parent_pids_required) + # Check if a doi was added in the draft and create a parent DOI independently if + # doi is required. + # Note: we don't have to check explicitely to the parent DOI creation only for + # datacite provider because we pass a `condition_func` below that it omits the + # minting if the pid selected is external + if draft.get("pids", {}).get("doi"): + required_schemes.add("doi") + conditional_schemes = self.service.config.parent_pids_conditional for scheme in set(required_schemes): condition_func = conditional_schemes.get(scheme) diff --git a/tests/services/components/test_pids_component.py b/tests/services/components/test_pids_component.py index cbd3dc841..dc102a6f0 100644 --- a/tests/services/components/test_pids_component.py +++ b/tests/services/components/test_pids_component.py @@ -123,12 +123,11 @@ class TestServiceConfigRequiredExternalPID(RDMRecordServiceConfig): @pytest.fixture(scope="module") -def no_pids_cmp(): +def no_pids_cmp(app): + service_config = TestServiceConfigNoPIDs.build(app) service = RDMRecordService( - config=TestServiceConfigNoPIDs, - pids_service=PIDsService( - config=TestServiceConfigNoPIDs, manager_cls=PIDManager - ), + config=service_config, + pids_service=PIDsService(config=service_config, manager_cls=PIDManager), ) c = PIDsComponent(service=service) c.uow = UnitOfWork() @@ -136,12 +135,11 @@ def no_pids_cmp(): @pytest.fixture(scope="module") -def no_required_pids_service(): +def no_required_pids_service(app): + service_config = TestServiceConfigNoRequiredPIDs.build(app) return RDMRecordService( - config=TestServiceConfigNoRequiredPIDs, - pids_service=PIDsService( - config=TestServiceConfigNoRequiredPIDs, manager_cls=PIDManager - ), + config=service_config, + pids_service=PIDsService(config=service_config, manager_cls=PIDManager), ) @@ -153,12 +151,11 @@ def no_required_pids_cmp(no_required_pids_service): @pytest.fixture(scope="module") -def required_managed_pids_cmp(): +def required_managed_pids_cmp(app): + service_config = TestServiceConfigRequiredManagedPID.build(app) service = RDMRecordService( - config=TestServiceConfigRequiredManagedPID, - pids_service=PIDsService( - config=TestServiceConfigRequiredManagedPID, manager_cls=PIDManager - ), + config=service_config, + pids_service=PIDsService(config=service_config, manager_cls=PIDManager), ) c = PIDsComponent(service=service) c.uow = UnitOfWork() @@ -166,12 +163,11 @@ def required_managed_pids_cmp(): @pytest.fixture(scope="module") -def required_external_pids_cmp(): +def required_external_pids_cmp(app): + service_config = TestServiceConfigRequiredExternalPID.build(app) service = RDMRecordService( - config=TestServiceConfigRequiredExternalPID, - pids_service=PIDsService( - config=TestServiceConfigRequiredExternalPID, manager_cls=PIDManager - ), + config=service_config, + pids_service=PIDsService(config=service_config, manager_cls=PIDManager), ) c = PIDsComponent(service=service) c.uow = UnitOfWork() @@ -318,6 +314,7 @@ def test_publish_no_pids(no_pids_cmp, minimal_record, identity_simple, location) ], ) def test_publish_no_required_pids( + app, pids, no_required_pids_service, no_required_pids_cmp, diff --git a/tests/services/test_rdm_service.py b/tests/services/test_rdm_service.py index ae1158d99..daf7aedb2 100644 --- a/tests/services/test_rdm_service.py +++ b/tests/services/test_rdm_service.py @@ -10,10 +10,15 @@ """Service level tests for Invenio RDM Records.""" +from copy import deepcopy + import pytest from invenio_rdm_records.proxies import current_rdm_records -from invenio_rdm_records.services.errors import EmbargoNotLiftedError +from invenio_rdm_records.services.errors import ( + EmbargoNotLiftedError, + ValidationErrorWithMessageAsList, +) def test_minimal_draft_creation(running_app, search_clear, minimal_record): @@ -50,7 +55,7 @@ def test_draft_w_languages_creation(running_app, search_clear, minimal_record): def test_publish_public_record_with_default_doi( - running_app, search_clear, minimal_record + running_app, search_clear, minimal_record, uploader ): superuser_identity = running_app.superuser_identity service = current_rdm_records.records_service @@ -68,10 +73,115 @@ def test_publish_public_record_with_optional_doi( draft = service.create(superuser_identity, minimal_record) record = service.publish(id_=draft.id, identity=superuser_identity) assert "doi" not in record._record.pids + assert "doi" not in record._record.parent.pids + # Reset the running_app config for next tests + running_app.app.config["RDM_PERSISTENT_IDENTIFIERS"]["doi"]["required"] = True + + +def test_publish_public_record_versions_no_or_external_doi_managed_doi( + running_app, search_clear, minimal_record, verified_user +): + running_app.app.config["RDM_PERSISTENT_IDENTIFIERS"]["doi"]["required"] = False + verified_user_identity = verified_user.identity + service = current_rdm_records.records_service + # Publish without DOI + draft = service.create(verified_user_identity, minimal_record) + record = service.publish(id_=draft.id, identity=verified_user_identity) + assert "doi" not in record._record.pids + assert "doi" not in record._record.parent.pids + + # create a new version with an external DOI + draft = service.new_version(verified_user_identity, record.id) + draft_data = deepcopy(draft.data) + draft_data["metadata"]["publication_date"] = "2023-01-01" + draft_data["pids"]["doi"] = { + "identifier": "10.4321/test.1234", + "provider": "external", + } + draft = service.update_draft(verified_user_identity, draft.id, data=draft_data) + record = service.publish(id_=draft.id, identity=verified_user_identity) + assert "doi" in record._record.pids + assert "doi" not in record._record.parent.pids + + # create a new version and and try to mint a managed DOI now when you publish + draft = service.new_version(verified_user_identity, record.id) + draft = service.pids.create(verified_user_identity, draft.id, "doi") + draft_data = deepcopy(draft.data) + draft_data["metadata"]["publication_date"] = "2023-01-01" + draft = service.update_draft(verified_user_identity, draft.id, data=draft_data) + + with pytest.raises(ValidationErrorWithMessageAsList): + record = service.publish(id_=draft.id, identity=verified_user_identity) + + # Reset the running_app config for next tests + running_app.app.config["RDM_PERSISTENT_IDENTIFIERS"]["doi"]["required"] = True + + +def test_publish_public_record_versions_managed_doi_external_doi( + running_app, search_clear, minimal_record, verified_user +): + running_app.app.config["RDM_PERSISTENT_IDENTIFIERS"]["doi"]["required"] = False + verified_user_identity = verified_user.identity + service = current_rdm_records.records_service + # Publish with locally managed DOI + draft = service.create(verified_user_identity, minimal_record) + draft = service.pids.create(verified_user_identity, draft.id, "doi") + record = service.publish(id_=draft.id, identity=verified_user_identity) + assert "doi" in record._record.pids + assert "doi" in record._record.parent.pids + + # create a new version with an external DOI + draft = service.new_version(verified_user_identity, record.id) + draft_data = deepcopy(draft.data) + draft_data["metadata"]["publication_date"] = "2023-01-01" + draft_data["pids"]["doi"] = { + "identifier": "10.4321/test.1234", + "provider": "external", + } + draft = service.update_draft(verified_user_identity, draft.id, data=draft_data) + with pytest.raises(ValidationErrorWithMessageAsList): + record = service.publish(id_=draft.id, identity=verified_user_identity) + # Reset the running_app config for next tests running_app.app.config["RDM_PERSISTENT_IDENTIFIERS"]["doi"]["required"] = True +def test_publish_public_record_versions_managed_doi_no_doi( + running_app, search_clear, minimal_record, verified_user +): + running_app.app.config["RDM_PERSISTENT_IDENTIFIERS"]["doi"]["required"] = False + verified_user_identity = verified_user.identity + service = current_rdm_records.records_service + # Publish with locally managed DOI + draft = service.create(verified_user_identity, minimal_record) + draft = service.pids.create(verified_user_identity, draft.id, "doi") + record = service.publish(id_=draft.id, identity=verified_user_identity) + assert "doi" in record._record.pids + assert "doi" in record._record.parent.pids + + # create a new version with no DOI + draft = service.new_version(verified_user_identity, record.id) + draft_data = deepcopy(draft.data) + draft_data["metadata"]["publication_date"] = "2023-01-01" + draft_data["pids"] = {} + draft = service.update_draft(verified_user_identity, draft.id, data=draft_data) + with pytest.raises(ValidationErrorWithMessageAsList): + record = service.publish(id_=draft.id, identity=verified_user_identity) + + # Reset the running_app config for next tests + running_app.app.config["RDM_PERSISTENT_IDENTIFIERS"]["doi"]["required"] = True + + +def test_publish_public_record_with_default_doi( + running_app, search_clear, minimal_record, uploader +): + superuser_identity = running_app.superuser_identity + service = current_rdm_records.records_service + draft = service.create(superuser_identity, minimal_record) + record = service.publish(id_=draft.id, identity=superuser_identity) + assert "doi" in record._record.pids + + def test_publish_restricted_record_without_default_doi( running_app, search_clear, minimal_restricted_record ):