From 6b0e39da2f95d9012b8e15610d895ecae9ee7092 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Z=C3=BCbeyde=20Civelek?= Date: Mon, 18 Nov 2024 17:10:02 +0100 Subject: [PATCH 1/2] make DOI optional and add CLI command for DOI minting - Removed DOI minting code for video types - Added a CLI command to mint DOI - Updated FAQ section to request a DOI - Updated tests to reflect the changes --- cds/modules/deposit/api.py | 6 +- cds/modules/fixtures/data/pages/faq.html | 2 + cds/modules/maintenance/cli.py | 45 ++++++++++- cds/modules/records/minters.py | 13 +--- tests/unit/test_provider.py | 22 +++--- tests/unit/test_video_rest.py | 97 +++++++++++++++++++----- 6 files changed, 139 insertions(+), 46 deletions(-) diff --git a/cds/modules/deposit/api.py b/cds/modules/deposit/api.py index 1bb1dafe3..c2b5b7347 100644 --- a/cds/modules/deposit/api.py +++ b/cds/modules/deposit/api.py @@ -899,11 +899,7 @@ def _publish_edited(self): # dump again renamed subtitles self["_files"] = self.files.dumps() - from cds.modules.records.permissions import is_public - - if is_public(self, "read"): - # Mint the doi if necessary - doi_minter(record_uuid=self.id, data=self) + # Call the 'doi_minter' function if needed and if is_public return super(Video, self)._publish_edited() diff --git a/cds/modules/fixtures/data/pages/faq.html b/cds/modules/fixtures/data/pages/faq.html index 3e3e3d932..78dc6d6b4 100644 --- a/cds/modules/fixtures/data/pages/faq.html +++ b/cds/modules/fixtures/data/pages/faq.html @@ -20,6 +20,8 @@

CDS Videos

Please insert the list of e-mails and e-groups, one after the other, with "@cern.ch" at the end.

+

How to request a DOI for a video?

+

Videos are not automatically assigned a DOI. However, if you would like to obtain a DOI for your video, you can request one by creating a support ticket. Please use the following link to submit your request.

How can I share my project with some colleagues?

If you want to share the draft(s) video(s) with some colleagues, before publishing, to collaborate at the same time, edit the videos' project and use the "Project editors" tab: input the list of e-mail and e-groups (with "@cern.ch") in the "Grant access to the project" field as restricting access to a video.

How can I upload subtitles for my video?

diff --git a/cds/modules/maintenance/cli.py b/cds/modules/maintenance/cli.py index 441e575c5..68f33c811 100644 --- a/cds/modules/maintenance/cli.py +++ b/cds/modules/maintenance/cli.py @@ -28,7 +28,6 @@ from invenio_files_rest.models import ObjectVersion, ObjectVersionTag from invenio_records_files.models import RecordsBuckets -from cds.modules.deposit.api import Video from cds.modules.flows.deposit import index_deposit_project from cds.modules.flows.models import FlowMetadata from cds.modules.flows.tasks import ExtractFramesTask @@ -38,13 +37,57 @@ create_subformat, ) from cds.modules.records.api import CDSVideosFilesIterator +from cds.modules.deposit.api import Video from cds.modules.records.resolver import record_resolver +from cds.modules.records.minters import doi_minter +from cds.modules.deposit.tasks import datacite_register +from invenio_db import db +from cds.modules.records.permissions import is_public +from invenio_indexer.api import RecordIndexer + def abort_if_false(ctx, param, value): if not value: ctx.abort() +@click.command() +@click.option("--recid", "recid", help="ID of the video record", default=None) +@with_appcontext +def create_doi(recid): + if not recid: + raise ClickException('Missing option "--recid') + + try: + # Get the video object with recid + _, record = record_resolver.resolve(recid) + depid = record.depid + video_deposit = Video.get_record(depid.object_uuid) + except Exception as exc: + raise ClickException("Failed to fetch the video record") + + if is_public(video_deposit, "read") and video_deposit.is_published(): + try: + # sync deposit files <--> record files + edit_record = video_deposit.edit() + doi_minter(record_uuid=edit_record.id, data=edit_record) + edit_record.publish().commit() + + # save changes + db.session.commit() + + # Register the doi to datacite + datacite_register(recid, str(record.id)) + except Exception as exc: + db.session.rollback() + # index the record again + _, record_video = edit_record.fetch_published() + RecordIndexer().index(record_video) + click.echo(f"DOI created, registered, and indexed successfully for record '{recid}'") + else: + click.echo(f"Record '{recid}' is either not public or not published. Skipping DOI creation.") + + @click.group() def subformats(): diff --git a/cds/modules/records/minters.py b/cds/modules/records/minters.py index a6d6bdcfc..fdb500e4f 100644 --- a/cds/modules/records/minters.py +++ b/cds/modules/records/minters.py @@ -37,15 +37,10 @@ def cds_record_minter(record_uuid, data): """Mint record identifiers.""" provider = _rec_minter(record_uuid, data) - from cds.modules.deposit.api import Project - - from .permissions import is_public - - project_schema = current_jsonschemas.path_to_url(Project._schema) - - # We shouldn't mint the DOI for the project (CDS#996) - if data.get("$schema") != project_schema and is_public(data, "read"): - doi_minter(record_uuid, data) + # from cds.modules.deposit.api import Project + # from .permissions import is_public + # project_schema = current_jsonschemas.path_to_url(Project._schema) + # Call the 'doi_minter' function if needed (not project and published) return provider.pid diff --git a/tests/unit/test_provider.py b/tests/unit/test_provider.py index cb9673765..18d04bc8d 100644 --- a/tests/unit/test_provider.py +++ b/tests/unit/test_provider.py @@ -31,7 +31,7 @@ import pytest from invenio_pidstore.models import PersistentIdentifier, PIDStatus -from cds.modules.records.minters import cds_record_minter, is_local_doi +from cds.modules.records.minters import cds_record_minter, doi_minter, is_local_doi def test_recid_provider(db): @@ -57,14 +57,9 @@ def test_recid_provider(db): object_uuid=uuid, status=PIDStatus.REGISTERED, ) - pid_create.assert_any_call( - "doi", - "10.0000/videos.1", - object_type="rec", - object_uuid=uuid, - pid_provider="datacite", - status=PIDStatus.RESERVED, - ) + # Verify the video has no DOI after publishing + assert "doi" not in data + @pytest.mark.parametrize( @@ -80,6 +75,7 @@ def test_doi_minting(db, doi_in, doi_out): rec_uuid = uuid4() data = dict(doi=doi_in) cds_record_minter(rec_uuid, data) + doi_minter(rec_uuid, data) db.session.commit() pid = PersistentIdentifier.get("doi", doi_out) @@ -99,7 +95,7 @@ def test_invalid_doi(db, doi): uuid = uuid4() data = dict(doi=doi) with pytest.raises(AssertionError): - cds_record_minter(uuid, data) + doi_minter(uuid, data) def test_no_doi_minted_for_projects(db, api_project): @@ -111,6 +107,9 @@ def test_no_doi_minted_for_projects(db, api_project): # Project shouldn't have a DOI assert project.get("doi") is None cds_record_minter(uuid2, video_1) + # Video shouldn't have a DOI + assert "doi" not in video_1 + doi_minter(uuid2, video_1) # Video should have a DOI assert video_1.get("doi") @@ -122,7 +121,7 @@ def test_recid_provider_exception(db): def test_minting_recid(db): - """Test reminting doi for published record.""" + """Test reminting recid for published record.""" data = dict() # Assert registration of recid. rec_uuid = uuid4() @@ -131,7 +130,6 @@ def test_minting_recid(db): assert pid.pid_value == "1" assert pid.status == PIDStatus.REGISTERED assert pid.object_uuid == rec_uuid - assert data["doi"] == "10.0000/videos.1" with pytest.raises(AssertionError): cds_record_minter(rec_uuid, data) diff --git a/tests/unit/test_video_rest.py b/tests/unit/test_video_rest.py index b10c1c444..d28f979a3 100644 --- a/tests/unit/test_video_rest.py +++ b/tests/unit/test_video_rest.py @@ -30,6 +30,10 @@ from time import sleep import mock +from cds.modules.maintenance.cli import create_doi +from cds.modules.records.minters import doi_minter +from click.testing import CliRunner + import pytest from celery.exceptions import Retry from flask import url_for @@ -90,22 +94,18 @@ def test_video_publish_registering_the_datacite( sender=api_app, action="publish", deposit=video_1 ) - assert datacite_mock.called is True - assert datacite_mock().metadata_post.call_count == 1 - datacite_mock().doi_post.assert_called_once_with( - "10.0000/videos.1", "https://videos.cern.ch/record/1" - ) + # [[ CONFIRM THERE'S NO DOI ]] + assert datacite_mock.called is False + assert datacite_mock().metadata_post.call_count == 0 + datacite_mock().doi_post.assert_not_called() # [[ UPDATE DATACITE ]] datacite_register_after_publish( sender=api_app, action="publish", deposit=video_1 ) - assert datacite_mock.called is True - assert datacite_mock().metadata_post.call_count == 2 - datacite_mock().doi_post.assert_called_with( - "10.0000/videos.1", "https://videos.cern.ch/record/1" - ) + assert datacite_mock().metadata_post.call_count == 0 + datacite_mock().doi_post.assert_not_called() @mock.patch("invenio_pidstore.providers.datacite.DataCiteMDSClient") @@ -151,12 +151,10 @@ def test_video_publish_registering_the_datacite_if_fail( ): datacite_register.s(pid_value="1", record_uuid=str(video_1.id)).apply() - assert datacite_mock.called is True - assert datacite_mock().metadata_post.call_count == 1 - datacite_mock().doi_post.assert_called_once_with( - "10.0000/videos.1", "https://videos.cern.ch/record/1" - ) - assert datacite_mock.call_count == 3 + # [[ CONFIRM THERE'S NO DOI ]] + assert datacite_mock.called is False + assert datacite_mock().metadata_post.call_count == 0 + datacite_mock().doi_post.assert_not_called() @mock.patch("invenio_pidstore.providers.datacite.DataCiteMDSClient") @@ -403,8 +401,8 @@ def test_video_publish_edit_publish_again( # [[ MODIFY DOI -> SAVE ]] video_1 = deposit_video_resolver(video_1_depid) video_1_dict = copy.deepcopy(video_1) - old_doi = video_1_dict["doi"] - video_1_dict["doi"] = "10.1123/doi" + old_doi = video_1_dict.get("doi") + # video_1_dict["doi"] = "10.1123/doi" del video_1_dict["_files"] res = client.put( url_for( @@ -418,7 +416,7 @@ def test_video_publish_edit_publish_again( assert res.status_code == 200 data = json.loads(res.data.decode("utf-8")) # Ensure that doi once minted cannot be changed to another value - assert data["metadata"]["doi"] == old_doi + assert data["metadata"].get("doi") == old_doi video_1 = deposit_video_resolver(video_1_depid) # video_1['doi'] = old_doi @@ -528,6 +526,67 @@ def test_record_video_links( } +@mock.patch("invenio_pidstore.providers.datacite.DataCiteMDSClient") +def test_mint_doi_with_cli( + datacite_mock, + api_app, + users, + location, + json_headers, + json_partial_project_headers, + json_partial_video_headers, + deposit_metadata, + video_deposit_metadata, + project_deposit_metadata, +): + """Test video publish without DOI, then mint DOI using CLI.""" + api_app.config["DEPOSIT_DATACITE_MINTING_ENABLED"] = True + + with api_app.test_client() as client: + # Log in as the first user + login_user(User.query.get(users[0])) + + # Create a new project + project_dict = _create_new_project( + client, json_partial_project_headers, project_deposit_metadata + ) + + # Add a new empty video + video_1_dict = _add_video_info_to_project( + client, json_partial_video_headers, project_dict, video_deposit_metadata + ) + + video_1_depid = video_1_dict["metadata"]["_deposit"]["id"] + video_1 = deposit_video_resolver(video_1_depid) + prepare_videos_for_publish([video_1]) + + # Publish the video + video_1.publish() + + # Verify the video has no DOI after publishing + assert "doi" not in video_1 + + # Use the CLI command to mint the DOI + recid = video_1['_deposit']['pid']['value'] + runner = CliRunner() + result = runner.invoke(create_doi, ["--recid", recid]) + + assert result.exit_code == 0, f"CLI command failed: {result.output}" + + # Fetch the updated record + _, updated_video = video_1.fetch_published() + + # Verify that the DOI was minted successfully + doi = updated_video.get("doi") + assert doi is not None, "DOI was not minted" + + # Check that the DOI was registered with DataCite + assert datacite_mock.called is True + datacite_mock().doi_post.assert_called_once_with( + doi, f"https://videos.cern.ch/record/{recid}" + ) + + def _deposit_edit(client, json_headers, id): """Post action to edit deposit.""" res = client.post( From 0700b953752028c70d6c428cd7a31b8949b924c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Z=C3=BCbeyde=20Civelek?= Date: Tue, 19 Nov 2024 13:34:25 +0100 Subject: [PATCH 2/2] move mint_doi method to Video and update the tests --- cds/modules/deposit/api.py | 30 ++++++++++++++++--- cds/modules/maintenance/cli.py | 55 +++++++++++++++------------------- cds/modules/records/minters.py | 5 ---- tests/unit/test_video_rest.py | 12 ++++---- 4 files changed, 57 insertions(+), 45 deletions(-) diff --git a/cds/modules/deposit/api.py b/cds/modules/deposit/api.py index c2b5b7347..ea7168ce9 100644 --- a/cds/modules/deposit/api.py +++ b/cds/modules/deposit/api.py @@ -25,6 +25,7 @@ """Deposit API.""" +import idutils import datetime import os import re @@ -52,7 +53,7 @@ PIDInvalidAction, ResolverError, ) -from invenio_pidstore.models import PersistentIdentifier +from invenio_pidstore.models import PersistentIdentifier, PIDStatus from invenio_pidstore.resolver import Resolver from invenio_records_files.models import RecordsBuckets from invenio_records_files.utils import sorted_files_from_bucket @@ -73,10 +74,11 @@ CDSRecord, CDSVideosFilesIterator, ) -from ..records.minters import doi_minter, is_local_doi, report_number_minter +from ..records.minters import cds_doi_generator, is_local_doi, report_number_minter from ..records.resolver import record_resolver from ..records.utils import is_record, lowercase_value from ..records.validators import PartialDraft4Validator +from ..records.permissions import is_public from .errors import DiscardConflict from .resolver import get_video_pid @@ -899,8 +901,6 @@ def _publish_edited(self): # dump again renamed subtitles self["_files"] = self.files.dumps() - # Call the 'doi_minter' function if needed and if is_public - return super(Video, self)._publish_edited() @mark_as_action @@ -1087,6 +1087,28 @@ def _create_tags(self): return + def mint_doi(self): + """Mint DOI.""" + assert self.has_record() + assert not self.has_minted_doi(), "DOI already exists for this video." + assert is_public(self, "read"), "Record is not public and cannot mint a DOI." + + doi = cds_doi_generator(self["recid"]) + # Make sure it's a proper DOI + assert idutils.is_doi(doi) + + self["doi"] = doi + PersistentIdentifier.create( + "doi", + doi, + pid_provider="datacite", + object_type="rec", + object_uuid=self.id, + status=PIDStatus.RESERVED, + ) + return self + + project_resolver = Resolver( pid_type="depid", object_type="rec", diff --git a/cds/modules/maintenance/cli.py b/cds/modules/maintenance/cli.py index 68f33c811..440857d18 100644 --- a/cds/modules/maintenance/cli.py +++ b/cds/modules/maintenance/cli.py @@ -39,11 +39,10 @@ from cds.modules.records.api import CDSVideosFilesIterator from cds.modules.deposit.api import Video from cds.modules.records.resolver import record_resolver -from cds.modules.records.minters import doi_minter from cds.modules.deposit.tasks import datacite_register from invenio_db import db -from cds.modules.records.permissions import is_public from invenio_indexer.api import RecordIndexer +from datacite.errors import DataCiteError @@ -52,41 +51,35 @@ def abort_if_false(ctx, param, value): ctx.abort() @click.command() -@click.option("--recid", "recid", help="ID of the video record", default=None) +@click.option("--recid", "recid", help="ID of the video record", default=None, required=True) @with_appcontext def create_doi(recid): - if not recid: - raise ClickException('Missing option "--recid') - + """Mints the DOI for a video record.""" + # Get the video object with recid + _, record = record_resolver.resolve(recid) + depid = record.depid + video_deposit = Video.get_record(depid.object_uuid) + try: - # Get the video object with recid - _, record = record_resolver.resolve(recid) - depid = record.depid - video_deposit = Video.get_record(depid.object_uuid) - except Exception as exc: - raise ClickException("Failed to fetch the video record") - - if is_public(video_deposit, "read") and video_deposit.is_published(): - try: - # sync deposit files <--> record files - edit_record = video_deposit.edit() - doi_minter(record_uuid=edit_record.id, data=edit_record) - edit_record.publish().commit() - - # save changes - db.session.commit() - - # Register the doi to datacite - datacite_register(recid, str(record.id)) - except Exception as exc: - db.session.rollback() - # index the record again + # Mint the doi and publish + edit_record = video_deposit.edit().mint_doi().publish().commit() + # Save changes + db.session.commit() + + # Index the record again _, record_video = edit_record.fetch_published() RecordIndexer().index(record_video) - click.echo(f"DOI created, registered, and indexed successfully for record '{recid}'") - else: - click.echo(f"Record '{recid}' is either not public or not published. Skipping DOI creation.") + click.echo(f"DOI minted, and indexed successfully for record '{recid}'") + # Register the doi to datacite + datacite_register(recid, str(record_video.id)) + + except DataCiteError as dexc: + raise ClickException(f"Failed to register DOI on datacite for the video record '{recid}': {str(dexc)}") + except Exception as exc: + db.session.rollback() + raise ClickException(f"Failed to mint DOI for the video record '{recid}': {str(exc)}") + @click.group() diff --git a/cds/modules/records/minters.py b/cds/modules/records/minters.py index fdb500e4f..9425b7cc7 100644 --- a/cds/modules/records/minters.py +++ b/cds/modules/records/minters.py @@ -37,11 +37,6 @@ def cds_record_minter(record_uuid, data): """Mint record identifiers.""" provider = _rec_minter(record_uuid, data) - # from cds.modules.deposit.api import Project - # from .permissions import is_public - # project_schema = current_jsonschemas.path_to_url(Project._schema) - # Call the 'doi_minter' function if needed (not project and published) - return provider.pid diff --git a/tests/unit/test_video_rest.py b/tests/unit/test_video_rest.py index d28f979a3..0fa87973b 100644 --- a/tests/unit/test_video_rest.py +++ b/tests/unit/test_video_rest.py @@ -31,7 +31,6 @@ import mock from cds.modules.maintenance.cli import create_doi -from cds.modules.records.minters import doi_minter from click.testing import CliRunner import pytest @@ -99,7 +98,7 @@ def test_video_publish_registering_the_datacite( assert datacite_mock().metadata_post.call_count == 0 datacite_mock().doi_post.assert_not_called() - # [[ UPDATE DATACITE ]] + # [[ ENSURE NO UPDATE IN DATACITE ]] datacite_register_after_publish( sender=api_app, action="publish", deposit=video_1 ) @@ -393,6 +392,9 @@ def test_video_publish_edit_publish_again( # [[ PUBLISH VIDEO ]] _deposit_publish(client, json_headers, video_1["_deposit"]["id"]) + # [[ MINT DOI TO VIDEO ]] + video_1 = deposit_video_resolver(video_1_depid) + video_1.edit().mint_doi().publish().commit() datacite_register.s(pid_value="123", record_uuid=str(video_1.id)).apply() # [[ EDIT VIDEO ]] @@ -401,8 +403,8 @@ def test_video_publish_edit_publish_again( # [[ MODIFY DOI -> SAVE ]] video_1 = deposit_video_resolver(video_1_depid) video_1_dict = copy.deepcopy(video_1) - old_doi = video_1_dict.get("doi") - # video_1_dict["doi"] = "10.1123/doi" + old_doi = video_1_dict["doi"] + video_1_dict["doi"] = "10.1123/doi" del video_1_dict["_files"] res = client.put( url_for( @@ -416,7 +418,7 @@ def test_video_publish_edit_publish_again( assert res.status_code == 200 data = json.loads(res.data.decode("utf-8")) # Ensure that doi once minted cannot be changed to another value - assert data["metadata"].get("doi") == old_doi + assert data["metadata"]["doi"] == old_doi video_1 = deposit_video_resolver(video_1_depid) # video_1['doi'] = old_doi