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.

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

Jump to bottom

record files: enable file modifications depending on external DOi #2264

Closed
wants to merge 1 commit into from
Closed

record files: enable file modifications depending on external DOi #2264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions invenio_app_rdm/records_ui/templates/semantic-ui/invenio_app_rdm/records/deposit.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@
{%- if permissions %}
<input id="deposits-record-permissions" type="hidden" name="deposits-record-permissions" value='{{permissions | tojson }}'></input>
{%- endif %}
{%- if externalDOI is not none %}
<input id="deposits-externalDOI" type="hidden" name="deposits-externalDOI" value='{{ externalDOI | tojson }}'></input>
{%- endif %}
<div id="deposit-form"></div>
{%- endblock page_body %}

Expand Down
1 change: 1 addition & 0 deletions ...ecords_ui/templates/semantic-ui/invenio_app_rdm/records/details/side_bar/manage_menu.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
data-is-draft="{{ is_draft | tojson }}"
data-is-preview-submission-request="{{ is_preview_submission_request | tojson }}"
data-current-user-id="{{ current_user.id }}"
data-external-doi="{{ externalDOI | tojson }}"
>
<div class="ui placeholder">
<div class="header">
Expand Down
13 changes: 13 additions & 0 deletions invenio_app_rdm/records_ui/views/deposits.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
from invenio_rdm_records.resources.serializers import UIJSONSerializer
from invenio_rdm_records.services.schemas import RDMRecordSchema
from invenio_rdm_records.services.schemas.utils import dump_empty
from invenio_records.dictutils import dict_lookup
from invenio_search.engine import dsl
from invenio_vocabularies.proxies import current_service as vocabulary_service
from invenio_vocabularies.records.models import VocabularyScheme
Expand Down Expand Up @@ -376,6 +377,7 @@ def deposit_create(community=None):
"manage_record_access",
]
),
externalDOI=False,
)


Expand All @@ -388,6 +390,15 @@ def deposit_edit(pid_value, draft=None, draft_files=None):
ui_serializer = UIJSONSerializer()
record = ui_serializer.dump_obj(draft.to_dict())

try:
externalDOI = (
not current_app.config["RDM_LOCK_FILES_FOR_EXTERNAL_DOI"]
and dict_lookup(record, "pids.doi.provider")
== current_app.config["RDM_EXTERNAL_DOI_PROVIDER_KEY"]
)
except KeyError:
externalDOI = False

return render_template(
current_app.config["APP_RDM_DEPOSIT_FORM_TEMPLATE"],
forms_config=get_form_config(apiUrl=f"/api/records/{pid_value}/draft"),
Expand All @@ -400,6 +411,8 @@ def deposit_edit(pid_value, draft=None, draft_files=None):
"delete_draft",
"manage_files",
"manage_record_access",
# "draft_modify_files",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: commented line through the PR - the approach using permissions.
Question for the reviewers: which approach seems better?

Copy link
Contributor

@kpsherva kpsherva Jun 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

In my opinion permissions with IfConfig generator taking RDM_LOCK_FILES_FOR_EXTERNAL_DOI as parameter would be cleaner. ping @slint

Copy link
Contributor

@tmorrell tmorrell Jun 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think permissions make this cleaner, but with two separate permissions for the deposit form and landing page.

Copy link
Member

@slint slint Jun 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

On second thought, I don't think we need to rely on permissions, the record status (published or draft) or even the DOI provider (external or DataCite) for this (at least for display purposes).

The reason is that at the end of the day, the one that controls if one can modify files of a deposit is the bucket (i.e. RDMDraft.bucket), and more specifically, if it's locked/unlocked. And the one that controls the draft bucket is the DraftFilesComponent.edit component method.

As a consequence, the active/inactive/hidden state of the files upload section in the form should be controlled by passing something like filesEditable=draft.bucket.locked. This also goes a bit in the direction of making React components "dumb presentation containers" and not aware of business-logic rules (like e.g. if the record is published or has an external DOI).

Of course, we will still use config flags to control the features, and the defaults will not break the current behaviour, i.e. by default:

  • you CANNOT edit files of published external DOI records
  • you CAN create new versions of external DOI records and have mixed record versions of external/managed DOIs

]
),
externalDOI=externalDOI,
)
22 changes: 21 additions & 1 deletion invenio_app_rdm/records_ui/views/records.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from invenio_previewer.proxies import current_previewer
from invenio_rdm_records.proxies import current_rdm_records
from invenio_rdm_records.resources.serializers import UIJSONSerializer
from invenio_records.dictutils import dict_lookup
from invenio_stats.proxies import current_stats
from marshmallow import ValidationError

Expand Down Expand Up @@ -140,19 +141,38 @@ def record_detail(pid_value, record, files, is_preview=False):
emitter(current_app, record=record._record, via_api=False)

resolved_community, _ = get_record_community(record_ui)

try:
externalDOI = (
not current_app.config["RDM_LOCK_FILES_FOR_EXTERNAL_DOI"]
and dict_lookup(record, "pids.doi.provider")
== current_app.config["RDM_EXTERNAL_DOI_PROVIDER_KEY"]
)
except KeyError:
externalDOI = False

return render_template(
current_app.config.get("APP_RDM_RECORD_LANDING_PAGE_TEMPLATE"),
record=record_ui,
files=files_dict,
user_communities_memberships=get_user_communities_memberships(),
permissions=record.has_permissions_to(
["edit", "new_version", "manage", "update_draft", "read_files", "review"]
[
"edit",
"new_version",
"manage",
"update_draft",
"read_files",
"review",
# "draft_modify_files",
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd call this "show_version_button". Though I'd check what the "new_version" permission does to see if that might be duplicative.

]
),
custom_fields_ui=custom_fields["ui"],
is_preview=is_preview,
is_draft=is_draft,
community=resolved_community,
external_resources=get_external_resources(record_ui),
externalDOI=externalDOI,
)


Expand Down
6 changes: 5 additions & 1 deletion invenio_app_rdm/theme/assets/semantic-ui/js/invenio_app_rdm/deposit/RDMDepositForm.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ export class RDMDepositForm extends Component {
sidebarRef = createRef();

render() {
const { record, files, permissions, preselectedCommunity } = this.props;
const { record, files, permissions, preselectedCommunity, externalDOI } =
this.props;
const customFieldsUI = this.config.custom_fields.ui;
return (
<DepositFormApp
Expand Down Expand Up @@ -147,6 +148,8 @@ export class RDMDepositForm extends Component {
quota={this.config.quota}
decimalSizeDisplay={this.config.decimal_size_display}
showMetadataOnlyToggle={permissions?.can_manage_files}
// externalDOI={permissions?.can_draft_modify_files}
externalDOI={externalDOI}
/>
</Overridable>
</AccordionField>
Expand Down Expand Up @@ -659,6 +662,7 @@ RDMDepositForm.propTypes = {
preselectedCommunity: PropTypes.object,
files: PropTypes.object,
permissions: PropTypes.object,
externalDOI: PropTypes.bool, // required?
};

RDMDepositForm.defaultProps = {
Expand Down
1 change: 1 addition & 0 deletions invenio_app_rdm/theme/assets/semantic-ui/js/invenio_app_rdm/deposit/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ ReactDOM.render(
files={getInputFromDOM("deposits-record-files")}
config={getInputFromDOM("deposits-config")}
permissions={getInputFromDOM("deposits-record-permissions")}
externalDOI={getInputFromDOM("deposits-externalDOI")}
/>
</OverridableContext.Provider>,
document.getElementById("deposit-form")
Expand Down
33 changes: 21 additions & 12 deletions invenio_app_rdm/theme/assets/semantic-ui/js/invenio_app_rdm/landing_page/RecordManagement.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,14 @@ export class RecordManagement extends Component {
}

render() {
const { record, permissions, isDraft, isPreviewSubmissionRequest, currentUserId } =
this.props;
const {
record,
permissions,
isDraft,
isPreviewSubmissionRequest,
currentUserId,
externalDOI,
} = this.props;
const { error } = this.state;
const { id: recid } = record;

Expand Down Expand Up @@ -59,16 +65,18 @@ export class RecordManagement extends Component {
)}
{!isPreviewSubmissionRequest && (
<>
<Grid.Column className="pt-5 pb-5">
<NewVersionButton
fluid
size="medium"
record={record}
onError={handleError}
disabled={!permissions.can_new_version}
/>
</Grid.Column>

{/*{!permissions?.can_draft_modify_files && (*/}
{!externalDOI && (
<Grid.Column className="pt-5 pb-5">
<NewVersionButton
fluid
size="medium"
record={record}
onError={handleError}
disabled={!permissions.can_new_version}
/>
</Grid.Column>
)}
<Grid.Column className="pt-5">
{permissions.can_manage && (
<ShareButton disabled={!permissions.can_update_draft} recid={recid} />
Expand Down Expand Up @@ -100,4 +108,5 @@ RecordManagement.propTypes = {
isDraft: PropTypes.bool.isRequired,
isPreviewSubmissionRequest: PropTypes.bool.isRequired,
currentUserId: PropTypes.string.isRequired,
externalDOI: PropTypes.bool, // required?
};
1 change: 1 addition & 0 deletions invenio_app_rdm/theme/assets/semantic-ui/js/invenio_app_rdm/landing_page/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ function renderRecordManagement(element) {
recordManagementAppDiv.dataset.isPreviewSubmissionRequest
)}
currentUserId={recordManagementAppDiv.dataset.currentUserId}
externalDOI={JSON.parse(recordManagementAppDiv.dataset.externalDoi)}
/>
</OverridableContext.Provider>,
element
Expand Down