Skip to content

files: added errors on draft create #296

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.

Sign up for GitHub

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

Open
carlinmack wants to merge 4 commits into
base: master
Choose a base branch
from
Open

files: added errors on draft create #296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
891a4ef
tests: add test of files on draft creation
carlinmack Mar 12, 2025
6c5b0f8
chore: extract _assign_files_enabled into common function
carlinmack Mar 12, 2025
d800127
files: added errors on draft create
carlinmack Mar 12, 2025
87949e5
chore: refactor to remove indentation
carlinmack Mar 14, 2025
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
121 changes: 59 additions & 62 deletions invenio_drafts_resources/services/records/components/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,27 +71,8 @@ def create(self, identity, data=None, record=None, errors=None):
NOTE: `record` actually refers to the draft
(this interface is used in records-resources and rdm-records)
"""
draft = record
files = self.get_record_files(draft)
enabled = data.get(self.files_data_key, {}).get(
"enabled", self.service.config.default_files_enabled
)

if files.enabled != enabled:
if not self.service.check_permission(
identity, "manage_files", record=draft
):
errors.append(
{
"field": f"{self.files_data_key}.enabled",
"messages": [
_("You don't have permissions to manage files options.")
],
}
)
return # exit early

files.enabled = enabled
self._assign_files_enabled(identity, data, record, errors)
self._check_files_exist(identity, record, errors)

def update_draft(self, identity, data=None, record=None, errors=None):
"""Assigns files.enabled and warns if files are missing.
Expand All @@ -100,49 +81,10 @@ def update_draft(self, identity, data=None, record=None, errors=None):
(this interface is used in records-resources and rdm-records)
"""
draft = record
draft_files = self.get_record_files(draft)
default_preview = data.get(self.files_data_key, {}).get("default_preview")
can_toggle_files = self.service.check_permission(
identity, "manage_files", record=draft
)

enabled = data.get(self.files_data_key, {}).get(
"enabled", self.service.config.default_files_enabled
)

if draft_files.enabled != enabled:
if not can_toggle_files:
errors.append(
{
"field": f"{self.files_data_key}.enabled",
"messages": [
_("You don't have permissions to manage files options.")
],
}
)
return # exit early

try:
self.assign_files_enabled(draft, enabled)
except ValidationError as e:
errors.append(
{"field": f"{self.files_data_key}.enabled", "messages": e.messages}
)
return # exit early

if draft_files.enabled and not draft_files.items():
if can_toggle_files:
my_message = _(
"Missing uploaded files. To disable files for this record please mark it as metadata-only."
)
else:
my_message = _("Missing uploaded files.")
errors.append(
{
"field": f"{self.files_data_key}.enabled",
"messages": [my_message],
}
)
self._assign_files_enabled(identity, data, record, errors)
self._check_files_exist(identity, record, errors)

try:
self.assign_files_default_preview(
Expand Down Expand Up @@ -187,6 +129,61 @@ def new_version(self, identity, draft=None, record=None):
record_files = self.get_record_files(record)
draft_files.enabled = record_files.enabled

def _assign_files_enabled(self, identity, data, draft, errors):
"""Assign files.enabled."""
enabled = data.get(self.files_data_key, {}).get(
"enabled", self.service.config.default_files_enabled
)
if not self._check_can_manage(identity, data, draft, errors, enabled):
return

try:
self.assign_files_enabled(draft, enabled)
except ValidationError as e:
errors.append(
{"field": f"{self.files_data_key}.enabled", "messages": e.messages}
)

def _check_can_manage(self, identity, data, draft, errors, enabled):
"""Check whether identity can manage files."""
files = self.get_record_files(draft)
can_manage_files = self.service.check_permission(
identity, "manage_files", record=draft
)
if files.enabled == enabled or can_manage_files:
return True

errors.append(
{
"field": f"{self.files_data_key}.enabled",
"messages": [_("You don't have permissions to manage files options.")],
}
)
return False

def _check_files_exist(self, identity, draft, errors):
"""If files.enabled check whether there are files."""
draft_files = self.get_record_files(draft)
if not draft_files.enabled or draft_files.items():
return

can_toggle_files = self.service.check_permission(
identity, "manage_files", record=draft
)
my_message = (
_(
"Missing uploaded files. To disable files for this record please mark it as metadata-only."
)
if can_toggle_files
else _("Missing uploaded files.")
)
errors.append(
{
"field": f"{self.files_data_key}.enabled",
"messages": [my_message],
}
)

def _purge_bucket_and_ovs(self, files):
"""Purge associated bucket and object versions."""
if files.bucket:
Expand Down
10 changes: 7 additions & 3 deletions tests/services/test_record_service_files.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# -*- coding: utf-8 -*-
#
# Copyright (C) 2020-2024 CERN.
# Copyright (C) 2020-2025 CERN.
# Copyright (C) 2020-2021 Northwestern University.
#
# Invenio-Drafts-Resources is free software; you can redistribute it and/or
Expand Down Expand Up @@ -522,7 +522,6 @@ def test_publish_with_fetch_files(
def test_missing_files(app, service, identity_simple, input_data):
# Test files.enabled = True when no files
draft = service.create(identity_simple, input_data)
draft = service.update_draft(identity_simple, draft.id, input_data)

# Files should be enabled
assert draft.data["files"]["enabled"] is True
Expand All @@ -531,6 +530,12 @@ def test_missing_files(app, service, identity_simple, input_data):
assert draft.errors[0]["field"] == "files.enabled"
assert "Missing uploaded files." in draft.errors[0]["messages"][0]

draft = service.update_draft(identity_simple, draft.id, input_data)

assert draft.data["files"]["enabled"] is True
assert draft.errors[0]["field"] == "files.enabled"
assert "Missing uploaded files." in draft.errors[0]["messages"][0]


def test_disable_files_error(app, service, identity_simple, input_data):
draft = service.create(identity_simple, input_data)
Expand Down Expand Up @@ -592,7 +597,6 @@ class ManageFilesPermissionPolicy(PermissionPolicy):

# Not modifying files options (i.e. sending `files.enabled: true`) doesn't error
draft = service.create(identity_simple, input_data)
assert not draft.errors

# Modyfing/disabling files should return an error
input_data["files"]["enabled"] = False
Expand Down
Loading