From adbc3fb74f79893bd760f502091a96d56ba5ca66 Mon Sep 17 00:00:00 2001 From: Anika Churilova Date: Thu, 18 Apr 2024 14:59:04 +0200 Subject: [PATCH] share tab: improve performance * avoid fetching data of each tab pane on opening of the modal * fetch only the data of the opened pane * add missing error handlings --- invenio_app_rdm/ext.py | 16 + .../AccessLinks/AccessDropdown.js | 9 +- .../AccessLinks/LinksSearchItem.js | 133 ++++---- .../AccessLinks/LinksSearchResultContainer.js | 34 ++- .../ShareOptions/AccessLinks/LinksTab.js | 59 ++-- .../AccessRequests/AccessRequestsTab.js | 2 +- .../AccessUsersGroups/AccessUsersGroups.js | 240 ++++++++------- .../AddUserGroupAccessModal.js | 83 ++--- .../AccessUsersGroups/GroupsTab.js | 88 ++++++ .../AccessUsersGroups/PeopleTab.js | 88 ++++++ .../UserGroupAccessSearchResult.js | 285 ------------------ .../UserGroupAccessSearchResultItem.js | 156 ++++++++++ .../landing_page/ShareOptions/ShareModal.js | 165 +++++++--- .../theme/modules/modal.overrides | 11 +- 14 files changed, 799 insertions(+), 570 deletions(-) create mode 100644 invenio_app_rdm/theme/assets/semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessUsersGroups/GroupsTab.js create mode 100644 invenio_app_rdm/theme/assets/semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessUsersGroups/PeopleTab.js delete mode 100644 invenio_app_rdm/theme/assets/semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessUsersGroups/UserGroupAccessSearchResult.js create mode 100644 invenio_app_rdm/theme/assets/semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessUsersGroups/UserGroupAccessSearchResultItem.js diff --git a/invenio_app_rdm/ext.py b/invenio_app_rdm/ext.py index 89454e3979..18c9de432d 100644 --- a/invenio_app_rdm/ext.py +++ b/invenio_app_rdm/ext.py @@ -6,6 +6,7 @@ # under the terms of the MIT License; see LICENSE file for more details. """Invenio Research Data Management.""" +import warnings from flask import request from flask_menu import current_menu @@ -23,6 +24,21 @@ def _is_branded_community(): def finalize_app(app): """Finalize app.""" init_menu(app) + init_config(app) + + +def init_config(app): + """Initialize configuration.""" + if "COMMUNITIES_GROUPS_ENABLED" in app.config: + warnings.warn( + "COMMUNITIES_GROUPS_ENABLED config variable is deprecated. Please use USERS_RESOURCES_GROUPS_ENABLED " + "instead. For now, COMMUNITIES_GROUPS_ENABLED value will be taken into account and features related to " + "groups will be disabled if this was the intention.", + DeprecationWarning, + ) + + if not app.config["COMMUNITIES_GROUPS_ENABLED"]: + app.config["USERS_RESOURCES_GROUPS_ENABLED"] = False def init_menu(app): diff --git a/invenio_app_rdm/theme/assets/semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessLinks/AccessDropdown.js b/invenio_app_rdm/theme/assets/semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessLinks/AccessDropdown.js index eb773e5186..0f209995f8 100644 --- a/invenio_app_rdm/theme/assets/semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessLinks/AccessDropdown.js +++ b/invenio_app_rdm/theme/assets/semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessLinks/AccessDropdown.js @@ -32,7 +32,7 @@ export class AccessDropdown extends Component { this.setState({ loading: false, actionSuccess: true, error: undefined }); handleUpdate = async (permission) => { - const { updateEndpoint } = this.props; + const { updateEndpoint, onPermissionChanged, result, entityType } = this.props; const data = { permission: permission }; this.setState({ loading: true, actionSuccess: false }); this.cancellableAction = withCancel(http.patch(updateEndpoint, data)); @@ -44,6 +44,11 @@ export class AccessDropdown extends Component { this.cancellableAction.promise, ]); this.onSuccess(); + onPermissionChanged( + result?.subject?.id ?? result.id, + data.permission, + entityType + ); } catch (error) { if (error === "UNMOUNTED") return; this.setState({ @@ -83,4 +88,6 @@ AccessDropdown.propTypes = { result: PropTypes.object.isRequired, dropdownOptions: PropTypes.array.isRequired, updateEndpoint: PropTypes.string.isRequired, + entityType: PropTypes.string.isRequired, + onPermissionChanged: PropTypes.func.isRequired, }; diff --git a/invenio_app_rdm/theme/assets/semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessLinks/LinksSearchItem.js b/invenio_app_rdm/theme/assets/semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessLinks/LinksSearchItem.js index c33e235f97..67819725dd 100644 --- a/invenio_app_rdm/theme/assets/semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessLinks/LinksSearchItem.js +++ b/invenio_app_rdm/theme/assets/semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessLinks/LinksSearchItem.js @@ -13,12 +13,23 @@ import { timestampToRelativeTime } from "../../../utils"; import { AccessDropdown } from "./AccessDropdown"; import _truncate from "lodash/truncate"; import { isEmpty } from "lodash"; -import { withCancel, http, dropdownOptionsGenerator } from "react-invenio-forms"; +import { + withCancel, + http, + dropdownOptionsGenerator, + ErrorMessage, +} from "react-invenio-forms"; import { dropdownOptions } from "./LinksSearchResultContainer"; -export const LinksSearchItem = ({ result, record, fetchData }) => { +export const LinksSearchItem = ({ + result, + record, + onItemAddedOrDeleted, + onPermissionChanged, +}) => { const [copied, setCopied] = useState(false); const [loading, setLoading] = useState(false); + const [error, setError] = useState(undefined); var cancellableAction = undefined; useEffect(() => { @@ -35,9 +46,10 @@ export const LinksSearchItem = ({ result, record, fetchData }) => { try { await cancellableAction.promise; setLoading(false); - fetchData(); + onItemAddedOrDeleted(record.links.access_links, "links"); } catch (error) { setLoading(false); + setError(error); console.error(error); } }; @@ -66,61 +78,75 @@ export const LinksSearchItem = ({ result, record, fetchData }) => { return ( - - {isEmpty(result.description) - ? "-" - : _truncate(result.description, { length: 60 })} - - - {timestampToRelativeTime(result.created_at)} - - - {isEmpty(result.expires_at) - ? i18next.t("Never") - : `${timestampToRelativeTime(result.expires_at)} (${result.expires_at})`} - - - - - - - + ) : ( + <> + + {isEmpty(result.description) + ? "-" + : _truncate(result.description, { length: 60 })} + + + {timestampToRelativeTime(result.created_at)} + + + {isEmpty(result.expires_at) + ? i18next.t("Never") + : `${timestampToRelativeTime(result.expires_at)} (${result.expires_at})`} + + + + + - } - /> - + copyAccessLink(result?.id)} + aria-label={i18next.t("Copy link")} + size="small" + icon + labelPosition="left" + > + + {i18next.t("Copy link")} + + } + /> + + + )} ); }; @@ -128,5 +154,6 @@ export const LinksSearchItem = ({ result, record, fetchData }) => { LinksSearchItem.propTypes = { result: PropTypes.object.isRequired, record: PropTypes.object.isRequired, - fetchData: PropTypes.func.isRequired, + onItemAddedOrDeleted: PropTypes.func.isRequired, + onPermissionChanged: PropTypes.func.isRequired, }; diff --git a/invenio_app_rdm/theme/assets/semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessLinks/LinksSearchResultContainer.js b/invenio_app_rdm/theme/assets/semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessLinks/LinksSearchResultContainer.js index 1f9bd5de5e..80d0c19eb2 100644 --- a/invenio_app_rdm/theme/assets/semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessLinks/LinksSearchResultContainer.js +++ b/invenio_app_rdm/theme/assets/semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessLinks/LinksSearchResultContainer.js @@ -19,26 +19,30 @@ export const dropdownOptions = [ { key: "view", name: "view", - text: i18next.t("Can view"), - title: i18next.t("Can view"), + text: i18next.t("Can view all versions"), + title: i18next.t("Can view all versions"), value: "view", - description: i18next.t("Can view restricted records/files."), + description: i18next.t("Can view restricted files of all versions of this record."), }, { key: "preview", name: "preview", - text: i18next.t("Can preview"), - title: i18next.t("Can preview"), + text: i18next.t("Can preview all drafts"), + title: i18next.t("Can preview all drafts"), value: "preview", - description: i18next.t("Can view drafts and restricted records/files."), + description: i18next.t( + "Can view drafts and restricted files of all versions of this record." + ), }, { key: "edit", name: "edit", - text: i18next.t("Can edit"), - title: i18next.t("Can edit"), + text: i18next.t("Can edit all versions"), + title: i18next.t("Can edit all versions"), value: "edit", - description: i18next.t("Can edit drafts and view restricted records/files."), + description: i18next.t( + "Can edit drafts and view restricted files of all versions of this record." + ), }, ]; @@ -92,7 +96,7 @@ export class LinksSearchResultContainer extends Component { }; handleCreation = async (permission, expiresAt, description) => { - const { fetchData, record } = this.props; + const { onItemAddedOrDeleted, record } = this.props; this.setState({ loading: true }); try { const data = { @@ -102,7 +106,7 @@ export class LinksSearchResultContainer extends Component { }; this.cancellableAction = withCancel(http.post(record.links.access_links, data)); await this.cancellableAction.promise; - fetchData(); + onItemAddedOrDeleted(record.links.access_links, "links"); this.setState({ loading: false, error: undefined }); } catch (error) { if (error === "UNMOUNTED") return; @@ -115,7 +119,7 @@ export class LinksSearchResultContainer extends Component { }; render() { - const { results, record, fetchData } = this.props; + const { results, record, onItemAddedOrDeleted, onPermissionChanged } = this.props; const { loading, error } = this.state; return ( <> @@ -148,7 +152,8 @@ export class LinksSearchResultContainer extends Component { key={result.id} result={result} record={record} - fetchData={fetchData} + onItemAddedOrDeleted={onItemAddedOrDeleted} + onPermissionChanged={onPermissionChanged} /> )) ) : ( @@ -174,5 +179,6 @@ export class LinksSearchResultContainer extends Component { LinksSearchResultContainer.propTypes = { results: PropTypes.array.isRequired, record: PropTypes.object.isRequired, - fetchData: PropTypes.func.isRequired, + onItemAddedOrDeleted: PropTypes.func.isRequired, + onPermissionChanged: PropTypes.func.isRequired, }; diff --git a/invenio_app_rdm/theme/assets/semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessLinks/LinksTab.js b/invenio_app_rdm/theme/assets/semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessLinks/LinksTab.js index 6a60d966f7..4ffb9321cf 100644 --- a/invenio_app_rdm/theme/assets/semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessLinks/LinksTab.js +++ b/invenio_app_rdm/theme/assets/semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessLinks/LinksTab.js @@ -5,10 +5,10 @@ // under the terms of the MIT License; see LICENSE file for more details. import React, { Component } from "react"; -import { Modal, Loader, Button } from "semantic-ui-react"; +import { Modal, Loader } from "semantic-ui-react"; import PropTypes from "prop-types"; import { i18next } from "@translations/invenio_app_rdm/i18next"; - +import { ErrorMessage } from "react-invenio-forms"; import { LinksSearchResultContainer } from "./LinksSearchResultContainer"; import { withCancel } from "react-invenio-forms"; import { http } from "react-invenio-forms"; @@ -17,31 +17,46 @@ export class LinksTab extends Component { constructor(props) { super(props); this.state = { - results: undefined, loading: false, error: undefined, }; } componentDidMount() { - this.fetchData(); + this.fetchLinks(); } componentWillUnmount() { this.cancellableAction && this.cancellableAction.cancel(); } - fetchData = async () => { - const { record } = this.props; + onLinksAddedOrDeleted = async () => { + await this.fetchLinks(true); + }; + + onPermissionChanged = (id, permission) => { + const { results } = this.props; + results.forEach((result) => { + if (result.id === id) { + result.permission = permission; + } + }); + }; + + fetchLinks = async (isDataChanged) => { + const { record, results, updateLinksState } = this.props; + if (results && !isDataChanged) return; + this.setState({ loading: true }); try { this.cancellableAction = withCancel(http.get(record.links.access_links)); const response = await this.cancellableAction.promise; + this.setState({ loading: false, error: undefined, - results: response.data.hits.hits, }); + updateLinksState(response.data.hits.hits, isDataChanged); } catch (error) { if (error === "UNMOUNTED") return; this.setState({ @@ -53,30 +68,31 @@ export class LinksTab extends Component { }; render() { - const { results, loading, error } = this.state; - const { record, handleClose } = this.props; + const { record, results } = this.props; + const { loading, error } = this.state; return ( <> - {error && error} - + {error && ( + + )} + {loading ? ( ) : ( )} - -