From 208898a202a6aac6727511af50673284c1893193 Mon Sep 17 00:00:00 2001 From: Zacharias Zacharodimos Date: Mon, 16 Dec 2024 15:11:43 +0100 Subject: [PATCH 1/3] pids: add manage permission to be able to skip doi transitions checks --- .../services/components/pids.py | 8 +++- invenio_rdm_records/services/permissions.py | 1 + tests/services/test_rdm_service.py | 40 +++++++++++++++++++ 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/invenio_rdm_records/services/components/pids.py b/invenio_rdm_records/services/components/pids.py index 20d50fed7..cb0401d05 100644 --- a/invenio_rdm_records/services/components/pids.py +++ b/invenio_rdm_records/services/components/pids.py @@ -107,7 +107,9 @@ def update_draft(self, identity, data=None, record=None, errors=None): # if DOI is not required in an instance check validate allowed providers # for each record version - if "doi" not in required_schemes: + if "doi" not in required_schemes and not self.service.check_permission( + identity, "pid_manage" + ): previous_published = self.service.record_cls.get_latest_published_by_parent( record.parent ) @@ -149,7 +151,9 @@ def publish(self, identity, draft=None, record=None): # if DOI is not required in an instance check validate allowed providers # for each record version - if "doi" not in required_schemes: + if "doi" not in required_schemes and not self.service.check_permission( + identity, "pid_manage" + ): # 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"): diff --git a/invenio_rdm_records/services/permissions.py b/invenio_rdm_records/services/permissions.py index 8d6f06477..6c89de5e2 100644 --- a/invenio_rdm_records/services/permissions.py +++ b/invenio_rdm_records/services/permissions.py @@ -188,6 +188,7 @@ class RDMRecordPermissionPolicy(RecordPermissionPolicy): can_pid_update = can_review can_pid_discard = can_review can_pid_delete = can_review + can_pid_manage = [SystemProcess()] # # Actions diff --git a/tests/services/test_rdm_service.py b/tests/services/test_rdm_service.py index daf7aedb2..48351bc0d 100644 --- a/tests/services/test_rdm_service.py +++ b/tests/services/test_rdm_service.py @@ -14,6 +14,8 @@ import pytest +from invenio_access.permissions import system_identity + from invenio_rdm_records.proxies import current_rdm_records from invenio_rdm_records.services.errors import ( EmbargoNotLiftedError, @@ -172,6 +174,44 @@ def test_publish_public_record_versions_managed_doi_no_doi( running_app.app.config["RDM_PERSISTENT_IDENTIFIERS"]["doi"]["required"] = True +def test_edit_published_record_change_doi_when_optional( + 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 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 = service.update_draft(verified_user_identity, draft.id, data=draft_data) + 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 + + # edit the new published version and change the DOI to locally managed with + # system user + draft = service.edit(system_identity, record.id) + draft = service.pids.create(system_identity, draft.id, "doi") + draft_data = deepcopy(draft.data) + draft_data["metadata"]["publication_date"] = "2023-01-01" + draft = service.update_draft(system_identity, draft.id, data=draft_data) + + record = service.publish(id_=draft.id, identity=system_identity) + assert "doi" in record._record.pids + assert "doi" 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_with_default_doi( running_app, search_clear, minimal_record, uploader ): From 1dee16ca6cb790a4eba2d72e9943af1f40b4b693 Mon Sep 17 00:00:00 2001 From: Zacharias Zacharodimos Date: Mon, 16 Dec 2024 15:12:29 +0100 Subject: [PATCH 2/3] optional-doi: fix validation check when user needs a DOI --- .../controls/PublishButton/PublishButton.js | 6 +++--- .../deposit/fields/Identifiers/PIDField.js | 6 +++--- .../src/deposit/state/actions/deposit.js | 20 +++++++++++++++++++ .../src/deposit/state/reducers/deposit.js | 11 +++++++++- .../src/deposit/state/types/index.js | 2 ++ .../services/components/pids.py | 12 +++++------ tests/services/test_rdm_service.py | 1 - 7 files changed, 44 insertions(+), 14 deletions(-) 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 34518b157..2e9082af2 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 @@ -33,19 +33,19 @@ class PublishButtonComponent extends Component { handlePublish = (event, handleSubmit, publishWithoutCommunity) => { const { setSubmitContext } = this.context; const { formik, raiseDOINeededButNotReserved, isDOIRequired } = this.props; - const noINeedOne = formik?.values?.noINeedOne; + const noINeedDOI = formik?.values?.noINeedDOI; // 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 && + noINeedDOI && 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."), + doi: i18next.t("DOI is needed. You need to reserve a DOI before publishing."), }, }; formik.setErrors(errors); 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 e49437ef1..74e63df37 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 @@ -497,14 +497,14 @@ class CustomPIDField extends Component { form.setFieldValue("pids", {}); if (!required) { // We set the - form.setFieldValue("noINeedOne", true); + form.setFieldValue("noINeedDOI", true); } } else if (userSelectedNoNeed) { form.setFieldValue("pids", {}); - form.setFieldValue("noINeedOne", false); + form.setFieldValue("noINeedDOI", false); } else { this.onExternalIdentifierChanged(""); - form.setFieldValue("noINeedOne", false); + form.setFieldValue("noINeedDOI", false); } form.setFieldError(fieldPath, false); this.setState({ diff --git a/invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/state/actions/deposit.js b/invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/state/actions/deposit.js index f26659f68..393898bbd 100644 --- a/invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/state/actions/deposit.js +++ b/invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/state/actions/deposit.js @@ -30,6 +30,7 @@ import { RESERVE_PID_STARTED, RESERVE_PID_SUCCEEDED, SET_COMMUNITY, + SET_DOI_NEEDED, } from "../types"; async function changeURLAfterCreation(draftURL) { @@ -152,6 +153,16 @@ export const save = (draft) => { type: DRAFT_SAVE_SUCCEEDED, payload: { data: response.data }, }); + + if (draft.noINeedDOI) { + // Save the choice that user selected that DOI is needed. This is used to validate + // if user has reserved a DOI before clicking publish. This check is valid when + // DOI is optional + dispatch({ + type: SET_DOI_NEEDED, + payload: { noINeedDOI: draft.noINeedDOI }, + }); + } }; }; @@ -344,3 +355,12 @@ export const changeSelectedCommunity = (community) => { }); }; }; + +export const setDOINeeded = (value) => { + return async (dispatch) => { + dispatch({ + type: SET_DOI_NEEDED, + payload: { noINeedDOI: value }, + }); + }; +}; diff --git a/invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/state/reducers/deposit.js b/invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/state/reducers/deposit.js index d910ece74..60e6ac2fb 100644 --- a/invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/state/reducers/deposit.js +++ b/invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/state/reducers/deposit.js @@ -29,6 +29,7 @@ import { RESERVE_PID_STARTED, RESERVE_PID_SUCCEEDED, SET_COMMUNITY, + SET_DOI_NEEDED, } from "../types"; export class DepositStatus { @@ -332,13 +333,21 @@ const depositReducer = (state = {}, action) => { }, }; } - return { ...state, record: recordCopy, editorState: computeDepositState(recordCopy, action.payload.community), }; } + case SET_DOI_NEEDED: { + const recordCopy = { + ...state.record, + }; + return { + ...state, + record: { ...recordCopy, ...action.payload }, + }; + } default: return state; } diff --git a/invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/state/types/index.js b/invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/state/types/index.js index 81263b65f..bfb472ae3 100644 --- a/invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/state/types/index.js +++ b/invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/state/types/index.js @@ -43,6 +43,8 @@ export const DISCARD_PID_STARTED = "DISCARD_PID_STARTED"; export const DISCARD_PID_SUCCEEDED = "DISCARD_PID_SUCCEEDED"; export const DISCARD_PID_FAILED = "DISCARD_PID_FAILED"; +export const SET_DOI_NEEDED = "SET_DOI_NEEDED"; + // Files export const FILE_UPLOAD_ADDED = "FILE_UPLOAD_ADDED"; export const FILE_UPLOAD_IN_PROGRESS = "FILE_UPLOAD_IN_PROGRESS"; diff --git a/invenio_rdm_records/services/components/pids.py b/invenio_rdm_records/services/components/pids.py index cb0401d05..2a35312ed 100644 --- a/invenio_rdm_records/services/components/pids.py +++ b/invenio_rdm_records/services/components/pids.py @@ -107,9 +107,9 @@ def update_draft(self, identity, data=None, record=None, errors=None): # if DOI is not required in an instance check validate allowed providers # for each record version - if "doi" not in required_schemes and not self.service.check_permission( - identity, "pid_manage" - ): + doi_required = "doi" in required_schemes + can_manage_dois = self.service.check_permission(identity, "pid_manage") + if not doi_required and not can_manage_dois: previous_published = self.service.record_cls.get_latest_published_by_parent( record.parent ) @@ -151,9 +151,9 @@ def publish(self, identity, draft=None, record=None): # if DOI is not required in an instance check validate allowed providers # for each record version - if "doi" not in required_schemes and not self.service.check_permission( - identity, "pid_manage" - ): + doi_required = "doi" in required_schemes + can_manage_dois = self.service.check_permission(identity, "pid_manage") + if not doi_required and not can_manage_dois: # 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"): diff --git a/tests/services/test_rdm_service.py b/tests/services/test_rdm_service.py index 48351bc0d..76c2b7d05 100644 --- a/tests/services/test_rdm_service.py +++ b/tests/services/test_rdm_service.py @@ -13,7 +13,6 @@ from copy import deepcopy import pytest - from invenio_access.permissions import system_identity from invenio_rdm_records.proxies import current_rdm_records From 4b4650ff74e2a3d77814711b3d93f82f572996af Mon Sep 17 00:00:00 2001 From: Zacharias Zacharodimos Date: Mon, 16 Dec 2024 16:32:26 +0100 Subject: [PATCH 3/3] release: v16.5.1 --- CHANGES.rst | 5 +++++ invenio_rdm_records/__init__.py | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 8355e27d6..3ef075f98 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -12,6 +12,11 @@ Changes ======= +Version v16.5.1 (released 2024-12-16) + +- pids: add manage permission to be able to manage DOIs +- deposit: fix validation check when user needs a DOI and DOI is optional + Version v16.5.0 (released 2024-12-16) - pids: add support for optional DOI diff --git a/invenio_rdm_records/__init__.py b/invenio_rdm_records/__init__.py index 0531060a2..093d404c3 100644 --- a/invenio_rdm_records/__init__.py +++ b/invenio_rdm_records/__init__.py @@ -12,6 +12,6 @@ from .ext import InvenioRDMRecords -__version__ = "16.5.0" +__version__ = "16.5.1" __all__ = ("__version__", "InvenioRDMRecords")