Skip to content
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

results: added support for drafts #1700

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions invenio_rdm_records/services/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
)
from .permissions import RDMRecordPermissionPolicy
from .result_items import GrantItem, GrantList, SecretLinkItem, SecretLinkList
from .results import RDMRecordList
from .schemas import RDMParentSchema, RDMRecordSchema
from .schemas.community_records import CommunityRecordsSchema
from .schemas.parent.access import AccessSettingsSchema
Expand Down Expand Up @@ -251,6 +252,7 @@ class RDMRecordServiceConfig(RecordServiceConfig, ConfiguratorMixin):
link_result_list_cls = SecretLinkList
grant_result_item_cls = GrantItem
grant_result_list_cls = GrantList
result_list_cls = RDMRecordList

default_files_enabled = FromConfig("RDM_DEFAULT_FILES_ENABLED", default=True)

Expand Down
35 changes: 34 additions & 1 deletion invenio_rdm_records/services/results.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@
from invenio_communities.communities.entity_resolvers import pick_fields
from invenio_communities.communities.schema import CommunityGhostSchema
from invenio_communities.proxies import current_communities
from invenio_records_resources.services.records.results import ExpandableField
from invenio_records_resources.services.records.results import (
ExpandableField,
RecordList,
)
from invenio_users_resources.proxies import current_user_resources

from .dummy import DummyExpandingService
Expand Down Expand Up @@ -69,3 +72,33 @@ def get_value_service(self, value):
def pick(self, identity, resolved_rec):
"""Pick fields defined in the entity resolver."""
return resolved_rec


class RDMRecordList(RecordList):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue was that drafts are searched through the records service, which in turn uses the default RecordList class to project the search results.

The default implementation uses self._service.record_cls unconditionally, which in turn projects the result always to a record. This is not correct if the service shares the search for records and drafts. For instance, an unpublished draft always had the attribute is_draft=False because the attribute is_draft of a Record is always set to False.

"""Record list with custom fields."""

@property
def hits(self):
"""Iterator over the hits."""
for hit in self._results:
# Load dump
record_dict = hit.to_dict()
if record_dict["is_published"]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: wouldn't it be better to rely on the is_draft property? Though I think this should be a ConstantField systemfield so that it makes it to the indexed document.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's what caused the issue in the first place. the "search" method can be used for drafts or records, however, the method lives inside the record's service. So using record.is_draft always returns False.

record = self._service.record_cls.loads(record_dict)
else:
record = self._service.draft_cls.loads(record_dict)

# Project the record
projection = self._schema.dump(
record,
context=dict(
identity=self._identity,
record=record,
),
)
if self._links_item_tpl:
projection["links"] = self._links_item_tpl.expand(
self._identity, record
)

yield projection
Loading