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

Updating FormFeedback component #1958

Open
wants to merge 3 commits 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
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@

import { i18next } from "@translations/invenio_rdm_records/i18next";
import _get from "lodash/get";
import _isObject from "lodash/isObject";

Check warning on line 11 in invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/errors/FormFeedback.js

View workflow job for this annotation

GitHub Actions / JS / Tests (18.x)

'_isObject' is defined but never used

Check warning on line 11 in invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/errors/FormFeedback.js

View workflow job for this annotation

GitHub Actions / JS / Tests (20.x)

'_isObject' is defined but never used
import React, { Component } from "react";
import { connect } from "react-redux";
import { Grid, Message } from "semantic-ui-react";
import { Grid, Message, Label, Icon, List } from "semantic-ui-react";

Check warning on line 14 in invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/errors/FormFeedback.js

View workflow job for this annotation

GitHub Actions / JS / Tests (18.x)

'List' is defined but never used

Check warning on line 14 in invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/errors/FormFeedback.js

View workflow job for this annotation

GitHub Actions / JS / Tests (20.x)

'List' is defined but never used
import {
DISCARD_PID_FAILED,
DRAFT_DELETE_FAILED,
Expand All @@ -27,40 +27,16 @@
FILE_UPLOAD_SAVE_DRAFT_FAILED,
RESERVE_PID_FAILED,
} from "../state/types";
import { leafTraverse } from "../utils";
import PropTypes from "prop-types";

const defaultLabels = {
"files.enabled": i18next.t("Files"),
"metadata.resource_type": i18next.t("Resource type"),
"metadata.title": i18next.t("Title"),
"metadata.additional_titles": i18next.t("Additional titles"),
"metadata.publication_date": i18next.t("Publication date"),
"metadata.creators": i18next.t("Creators"),
"metadata.contributors": i18next.t("Contributors"),
"metadata.description": i18next.t("Description"),
"metadata.additional_descriptions": i18next.t("Additional descriptions"),
"metadata.rights": i18next.t("Licenses"),
"metadata.languages": i18next.t("Languages"),
"metadata.dates": i18next.t("Dates"),
"metadata.version": i18next.t("Version"),
"metadata.publisher": i18next.t("Publisher"),
"metadata.related_identifiers": i18next.t("Related works"),
"metadata.references": i18next.t("References"),
"metadata.identifiers": i18next.t("Alternate identifiers"),
"metadata.subjects": i18next.t("Keywords and subjects"),
"access.embargo.until": i18next.t("Embargo until"),
"pids.doi": i18next.t("DOI"),
};

const ACTIONS = {
[DRAFT_SAVE_SUCCEEDED]: {
feedback: "positive",
message: i18next.t("Record successfully saved."),
},
[DRAFT_HAS_VALIDATION_ERRORS]: {
feedback: "warning",
message: i18next.t("Record saved with validation errors:"),
feedback: "negative",
message: i18next.t("Record saved with validation errors in"),
},
[DRAFT_SAVE_FAILED]: {
feedback: "negative",
Expand All @@ -77,7 +53,7 @@
[DRAFT_PUBLISH_FAILED_WITH_VALIDATION_ERRORS]: {
feedback: "negative",
message: i18next.t(
"The draft was not published. Record saved with validation errors:"
"The draft was not published. Record saved with validation errors in"
),
},
[DRAFT_SUBMIT_REVIEW_FAILED]: {
Expand All @@ -89,7 +65,7 @@
[DRAFT_SUBMIT_REVIEW_FAILED_WITH_VALIDATION_ERRORS]: {
feedback: "negative",
message: i18next.t(
"The draft was not submitted for review. Record saved with validation errors:"
"The draft was not submitted for review. Record saved with validation errors in"
),
},
[DRAFT_DELETE_FAILED]: {
Expand Down Expand Up @@ -134,114 +110,88 @@
constructor(props) {
super(props);
this.labels = {
...defaultLabels,
...props.labels,
};
this.sections = {
...props.sectionsConfig,
};
}

/**
* Render error messages inline (if 1) or as list (if multiple).
*
* @param {Array<String>} messages
* @returns String or React node
*/
renderErrorMessages(messages) {
const uniqueMessages = [...new Set(messages)];
if (uniqueMessages.length === 1) {
return messages[0];
} else {
return (
<ul>
{uniqueMessages.map((m) => (
<li key={m}>{m}</li>
))}
</ul>
);
}
}
getErrorPaths(obj, prefix = "") {
const paths = new Set();

/**
* Return array of error messages from errorValue object.
*
* The error message(s) might be deeply nested in the errorValue e.g.
*
* errorValue = [
* {
* title: "Missing value"
* }
* ];
*
* @param {object} errorValue
* @returns array of Strings (error messages)
*/
toErrorMessages(errorValue) {
let messages = [];
let store = (l) => {
messages.push(l);
const recurse = (currentObj, currentPath) => {
if (Array.isArray(currentObj)) {
currentObj.forEach((item, index) => recurse(item, `${currentPath}[${index}]`));
} else if (currentObj && typeof currentObj === "object") {
Object.keys(currentObj).forEach((key) =>
recurse(currentObj[key], currentPath ? `${currentPath}.${key}` : key)
);
} else {
paths.add(currentPath);
}
};
leafTraverse(errorValue, store);
return messages;

recurse(obj, prefix);
return [...paths];
}

/**
* Return object with human readbable labels as keys and error messages as
* values given an errors object.
*
* @param {object} errors
* @returns object
*/
toLabelledErrorMessages(errors) {
// Step 0 - Create object with collapsed 1st and 2nd level keys
// e.g., {metadata: {creators: ,,,}} => {"metadata.creators": ...}
// For now, only for metadata, files and access.embargo
const metadata = errors.metadata || {};
const step0Metadata = Object.entries(metadata).map(([key, value]) => {
return ["metadata." + key, value];
});
const files = errors.files || {};
const step0Files = Object.entries(files).map(([key, value]) => {
return ["files." + key, value];
});
const access = errors.access?.embargo || {};
const step0Access = Object.entries(access).map(([key, value]) => {
return ["access.embargo." + key, value];
});
const pids = errors.pids || {};
const step0Pids = _isObject(pids)
? Object.entries(pids).map(([key, value]) => {
return ["pids." + key, value];
})
: [["pids", pids]];
const customFields = errors.custom_fields || {};
const step0CustomFields = Object.entries(customFields).map(([key, value]) => {
return ["custom_fields." + key, value];
});
const step0 = Object.fromEntries(
step0Metadata
.concat(step0Files)
.concat(step0Access)
.concat(step0Pids)
.concat(step0CustomFields)
);
getErrorSections(errorPaths) {
const errorSections = new Map();

// Step 1 - Transform each error value into array of error messages
const step1 = Object.fromEntries(
Object.entries(step0).map(([key, value]) => {
return [key, this.toErrorMessages(value)];
})
);
errorPaths.forEach((path) => {
let errorCount = 1;

// Step 2 - Group error messages by label
// (different error keys can map to same label e.g. title and
// additional_titles)
const labelledErrorMessages = {};
for (const key in step1) {
const label = this.labels[key] || "Unknown field";
let messages = labelledErrorMessages[label] || [];
labelledErrorMessages[label] = messages.concat(step1[key]);
}
for (const [section, fields] of Object.entries(this.sections)) {
if (fields.some((field) => path.startsWith(field))) {
const sectionElement = document.getElementById(section);
if (sectionElement) {
const label = sectionElement.getAttribute("label") || "Unknown section";
const errorField = _get(this.props.errors, path, null);

Check warning on line 150 in invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/errors/FormFeedback.js

View workflow job for this annotation

GitHub Actions / JS / Tests (18.x)

Must use destructuring props assignment

Check warning on line 150 in invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/errors/FormFeedback.js

View workflow job for this annotation

GitHub Actions / JS / Tests (20.x)

Must use destructuring props assignment
errorCount = Array.isArray(errorField) ? errorField.length : 1;

errorSections.set(section, {
label,
count: (errorSections.get(section)?.count || 0) + errorCount,
});
}
return;
}
}

const labelElement = document.querySelector(
`label[for^="${path.replace(/^(.*?)(\[\d+\].*)?$/, "$1")}"]`
);
const sectionElement = labelElement?.closest(".accordion");

if (sectionElement) {
const sectionId = sectionElement.id;
const label = sectionElement.getAttribute("label") || "Unknown section";
const errorField = _get(this.props.errors, path, null);

Check warning on line 170 in invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/errors/FormFeedback.js

View workflow job for this annotation

GitHub Actions / JS / Tests (18.x)

Must use destructuring props assignment

Check warning on line 170 in invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/errors/FormFeedback.js

View workflow job for this annotation

GitHub Actions / JS / Tests (20.x)

Must use destructuring props assignment
errorCount = Array.isArray(errorField) ? errorField.length : 1;

return labelledErrorMessages;
errorSections.set(sectionId, {
label,
count: (errorSections.get(sectionId)?.count || 0) + errorCount,
});
}
});

const orderedSections = [
...(errorSections.has("files-section")
? [["files-section", errorSections.get("files-section")]]
: []),
...[...errorSections].filter(([key]) => key !== "files-section"),
];

return orderedSections.map(([sectionId, { label, count }], i) => (
<a className="pl-5" key={i} href={`#${sectionId}`}>

Check warning on line 188 in invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/errors/FormFeedback.js

View workflow job for this annotation

GitHub Actions / JS / Tests (18.x)

Do not use Array index in keys

Check warning on line 188 in invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/errors/FormFeedback.js

View workflow job for this annotation

GitHub Actions / JS / Tests (20.x)

Do not use Array index in keys
{label}{" "}
<Label circular size="tiny">
{count}
</Label>
</a>
));
}

render() {
Expand All @@ -259,13 +209,10 @@
return null;
}

const labelledMessages = this.toLabelledErrorMessages(errors);
const listErrors = Object.entries(labelledMessages).map(([label, messages]) => (
<Message.Item key={label}>
<b>{label}</b>: {this.renderErrorMessages(messages)}
</Message.Item>
));

const errorPaths = this.getErrorPaths(errors);
const errorSections = this.getErrorSections(errorPaths);
//eslint-disable-next-line
debugger;
// errors not related to validation, following a different format {status:.., message:..}
const backendErrorMessage = errors.message;

Expand All @@ -279,8 +226,13 @@
>
<Grid container>
<Grid.Column width={15} textAlign="left">
<strong>{backendErrorMessage || message}</strong>
{listErrors.length > 0 && <Message.List>{listErrors}</Message.List>}
<strong>
<Icon name={feedback === "positive" ? "check" : "exclamation triangle"} />{" "}
{backendErrorMessage || message}
{errorSections.length > 0 && (
<>{errorSections.reduce((prev, curr) => [prev, ", ", curr])}</>
)}
</strong>
</Grid.Column>
</Grid>
</Message>
Expand All @@ -292,12 +244,14 @@
errors: PropTypes.object,
actionState: PropTypes.string,
labels: PropTypes.object,
sectionsConfig: PropTypes.object,
};

DisconnectedFormFeedback.defaultProps = {
errors: undefined,
actionState: undefined,
labels: undefined,
sectionsConfig: undefined,
};

const mapStateToProps = (state) => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

render() {
const {
id,
fieldPath,
formik, // this is our access to the shared current draft
label,
Expand All @@ -42,7 +43,7 @@
const isMetadataOnly = !formik.form.values.files.enabled;

return (
<Card className="access-right">
<Card label={label} id={id} className="access-right">
<Form.Field required>
<Card.Content>
<Card.Header>
Expand Down Expand Up @@ -97,6 +98,7 @@
}

AccessRightFieldCmp.propTypes = {
id: PropTypes.string,

Check warning on line 101 in invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/fields/AccessField/AccessRightField.js

View workflow job for this annotation

GitHub Actions / JS / Tests (18.x)

propType "id" is not required, but has no corresponding defaultProps declaration

Check warning on line 101 in invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/fields/AccessField/AccessRightField.js

View workflow job for this annotation

GitHub Actions / JS / Tests (20.x)

propType "id" is not required, but has no corresponding defaultProps declaration
fieldPath: PropTypes.string.isRequired,
formik: PropTypes.object.isRequired,
label: PropTypes.string.isRequired,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import { getIn, FieldArray } from "formik";
import { Button, Form, Label, List, Icon } from "semantic-ui-react";
import _get from "lodash/get";
import _map from "lodash/map";

Check warning on line 14 in invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/fields/CreatibutorsField/CreatibutorsField.js

View workflow job for this annotation

GitHub Actions / JS / Tests (18.x)

'_map' is defined but never used

Check warning on line 14 in invenio_rdm_records/assets/semantic-ui/js/invenio_rdm_records/src/deposit/fields/CreatibutorsField/CreatibutorsField.js

View workflow job for this annotation

GitHub Actions / JS / Tests (20.x)

'_map' is defined but never used
import { FieldLabel } from "react-invenio-forms";
import { HTML5Backend } from "react-dnd-html5-backend";
import { DndProvider } from "react-dnd";
Expand Down Expand Up @@ -119,7 +119,12 @@
schema={schema}
autocompleteNames={autocompleteNames}
trigger={
<Button type="button" icon labelPosition="left">
<Button
type="button"
icon
labelPosition="left"
className={creatibutorsError ? "error" : ""}
>
<Icon name="add" />
{addButtonLabel}
</Button>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ export const FileUploaderComponent = ({
...uiProps
}) => {
// We extract the working copy of the draft stored as `values` in formik
const { values: formikDraft } = useFormikContext();
const { values: formikDraft, errors, initialErrors } = useFormikContext();
const hasError = (errors.files || initialErrors?.files) && files;

const filesEnabled = _get(formikDraft, "files.enabled", false);
const [warningMsg, setWarningMsg] = useState();
const lockFileUploader = !isDraftRecord && filesLocked;
Expand Down Expand Up @@ -216,6 +218,7 @@ export const FileUploaderComponent = ({
warningMsg={warningMsg}
setWarningMsg={setWarningMsg}
filesLocked={lockFileUploader}
hasError={hasError}
{...uiProps}
>
<>
Expand Down Expand Up @@ -273,6 +276,7 @@ export const FileUploaderComponent = ({
filesList={filesList}
dropzoneParams={dropzoneParams}
filesLocked={lockFileUploader}
hasError={hasError}
filesEnabled={filesEnabled}
deleteFile={deleteFile}
decimalSizeDisplay={decimalSizeDisplay}
Expand All @@ -285,6 +289,7 @@ export const FileUploaderComponent = ({
filesList={filesList}
dropzoneParams={dropzoneParams}
filesLocked={lockFileUploader}
hasError={hasError}
filesEnabled={filesEnabled}
deleteFile={deleteFile}
decimalSizeDisplay={decimalSizeDisplay}
Expand Down
Loading
Loading