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

Add logic for creating new annotations #406

Open
wants to merge 13 commits into
base: feature/metadata-editing/develop
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { createSandbox } from "sinon";

import AnnotationFilterForm from "..";
import Annotation from "../../../entity/Annotation";
import { AnnotationType } from "../../../entity/AnnotationFormatter";
import FileFilter from "../../../entity/FileFilter";
import { initialState, reducer, reduxLogics, interaction, selection } from "../../../state";
import HttpAnnotationService from "../../../services/AnnotationService/HttpAnnotationService";
Expand All @@ -20,7 +21,7 @@ describe("<AnnotationFilterForm />", () => {
annotationDisplayName: "Foo",
annotationName: "foo",
description: "",
type: "Text",
type: AnnotationType.STRING,
});

const sandbox = createSandbox();
Expand Down Expand Up @@ -169,7 +170,7 @@ describe("<AnnotationFilterForm />", () => {
annotationDisplayName: "Foo",
annotationName: "foo",
description: "",
type: "YesNo",
type: AnnotationType.BOOLEAN,
});

const responseStub = {
Expand Down Expand Up @@ -268,7 +269,7 @@ describe("<AnnotationFilterForm />", () => {
annotationDisplayName: "Foo",
annotationName: "foo",
description: "",
type: "Number",
type: AnnotationType.NUMBER,
});

const sandbox = createSandbox();
Expand Down Expand Up @@ -323,7 +324,7 @@ describe("<AnnotationFilterForm />", () => {
annotationDisplayName: "Foo",
annotationName: "foo",
description: "",
type: "Duration",
type: AnnotationType.DURATION,
});

const sandbox = createSandbox();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { createSandbox } from "sinon";
import { FESBaseUrl, TOP_LEVEL_FILE_ANNOTATIONS } from "../../../constants";
import Annotation from "../../../entity/Annotation";
import AnnotationName from "../../../entity/Annotation/AnnotationName";
import { AnnotationType } from "../../../entity/AnnotationFormatter";
import { FmsFileAnnotation } from "../../../services/FileService";
import FileFilter from "../../../entity/FileFilter";
import FileDownloadServiceNoop from "../../../services/FileDownloadService/FileDownloadServiceNoop";
Expand All @@ -44,13 +45,13 @@ describe("<DirectoryTree />", () => {
annotationDisplayName: "Foo",
annotationName: "foo",
description: "",
type: "Text",
type: AnnotationType.STRING,
});
const barAnnotation = new Annotation({
annotationDisplayName: "Bar",
annotationName: "bar",
description: "",
type: "Text",
type: AnnotationType.STRING,
});

const baseDisplayAnnotations = TOP_LEVEL_FILE_ANNOTATIONS.filter(
Expand Down
20 changes: 19 additions & 1 deletion packages/core/components/EditMetadata/EditMetadata.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,24 @@
padding-left: 5px;
}

.error-message {
color: var(--error-status-text-color);
line-height: 1.15;
}

.status-message {
color: var(--info-status-text-color);
display: flex;
}

.spinner {
margin: 0 0.5em;
}

.spinner > div {
border-color: var(--info-status-background-color) var(--info-status-text-color) var(--info-status-text-color);
}

.footer {
margin-top: 20px;
width: 100%;
Expand Down Expand Up @@ -89,7 +107,7 @@
font-size: var(--l-paragraph-size);
}

.text-field :is(input) {
.text-field :is(input), .text-field :is(textarea) {
background-color: var(--secondary-background-color) !important;
color: var(--secondary-text-color) !important;
border-radius: 4px;
Expand Down
185 changes: 161 additions & 24 deletions packages/core/components/EditMetadata/NewAnnotationPathway.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,25 @@
import { IComboBoxOption, IconButton, Stack, StackItem, TextField } from "@fluentui/react";
import {
IComboBoxOption,
IconButton,
Spinner,
SpinnerSize,
Stack,
StackItem,
TextField,
} from "@fluentui/react";
import classNames from "classnames";
import * as React from "react";
import { useDispatch, useSelector } from "react-redux";

import MetadataDetails, { ValueCountItem } from "./MetadataDetails";
import { PrimaryButton, SecondaryButton } from "../Buttons";
import ComboBox from "../ComboBox";
import Tooltip from "../Tooltip";
import Annotation from "../../entity/Annotation";
import { AnnotationType } from "../../entity/AnnotationFormatter";
import { AnnotationValue } from "../../services/AnnotationService";
import { interaction, metadata } from "../../state";
import { ProcessStatus } from "../../state/interaction/actions";

import styles from "./EditMetadata.module.css";

Expand All @@ -18,21 +31,96 @@ enum EditStep {

interface NewAnnotationProps {
onDismiss: () => void;
onUnsavedChanges: () => void;
setHasUnsavedChanges: (arg: boolean) => void;
selectedFileCount: number;
}

// Simplified version of status message
interface AnnotationStatus {
status: ProcessStatus;
message: string | undefined;
}

/**
* Component for submitting a new annotation
* and then entering values for the selected files
*/
export default function NewAnnotationPathway(props: NewAnnotationProps) {
const dispatch = useDispatch();
// Destructure to prevent unnecessary useEffect triggers
const { onDismiss, setHasUnsavedChanges } = props;

const [step, setStep] = React.useState<EditStep>(EditStep.CREATE_FIELD);
const [newValues, setNewValues] = React.useState<string | undefined>();
const [newValues, setNewValues] = React.useState<AnnotationValue | undefined>();
const [newFieldName, setNewFieldName] = React.useState<string>("");
const [newFieldDataType, setNewFieldDataType] = React.useState<AnnotationType | undefined>();
const [newFieldDescription, setNewFieldDescription] = React.useState<string>("");
const [newFieldDataType, setNewFieldDataType] = React.useState<AnnotationType>(
AnnotationType.STRING
);
const [newDropdownOption, setNewDropdownOption] = React.useState<string>("");
const [dropdownOptions, setDropdownOptions] = React.useState<IComboBoxOption[]>([]);
const [submissionStatus, setSubmissionStatus] = React.useState<AnnotationStatus | undefined>();

const statuses = useSelector(interaction.selectors.getProcessStatuses);
const annotationCreationStatus = React.useMemo(
() => statuses.find((status) => status.processId === newFieldName),
[newFieldName, statuses]
);
// Check for updates to the annotation submission status
React.useEffect(() => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We aren't able to directly chain off of the createAnnotation dispatch (we don't use thunk in this codebase) so instead we need to wait for the status to update in state and then respond accordingly when we see SUCCEEDED

const checkForStatusUpdates = async () => {
const currentStatus = annotationCreationStatus?.data?.status;
switch (currentStatus) {
case ProcessStatus.ERROR:
case ProcessStatus.STARTED:
case ProcessStatus.PROGRESS:
setSubmissionStatus({
status: currentStatus,
message: annotationCreationStatus?.data?.msg,
});
return;
case ProcessStatus.SUCCEEDED:
if (newFieldName && newValues) {
try {
dispatch(
interaction.actions.editFiles({ [newFieldName]: [newValues] })
);
} catch (e) {
setSubmissionStatus({
status: ProcessStatus.ERROR,
message: `Failed to create annotation: ${e}`,
});
} finally {
setHasUnsavedChanges(false);
onDismiss();
}
}
default:
return;
}
};
checkForStatusUpdates();
}, [
annotationCreationStatus,
dispatch,
setHasUnsavedChanges,
newFieldName,
newValues,
onDismiss,
]);

const onChangeAlphanumericField = (
e: React.FormEvent<HTMLInputElement | HTMLTextAreaElement>,
newValue: string | undefined
) => {
const regex = /^[\w\-\s]+$/g;
// Restricts character entry to alphanumeric
if (newValue && !regex.test(newValue)) {
Copy link

Choose a reason for hiding this comment

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

aww, no injection attacks? But what if scientists want to mark an image ";Drop tables;":True?

e.preventDefault();
} else {
setNewFieldName(newValue || "");
}
};

const addDropdownChip = (evt: React.FormEvent) => {
evt.preventDefault();
Expand All @@ -53,30 +141,56 @@ export default function NewAnnotationPathway(props: NewAnnotationProps) {
setDropdownOptions(dropdownOptions.filter((opt) => opt !== optionToRemove));
};

function onSubmit() {
// TO DO: endpoint logic is in progress on a different branch
props.onDismiss();
}

// TO DO: determine when to submit the new annotation post req
function onCreateNewAnnotation() {
props?.onUnsavedChanges();
setHasUnsavedChanges(true);
setStep(EditStep.EDIT_FILES);
}

function onSubmit() {
if (!newFieldName || !newValues) {
setSubmissionStatus({
status: ProcessStatus.ERROR,
message: `Missing ${!newFieldName ? "field name" : "values for file"}`,
});
return;
}
const annotation = new Annotation({
annotationDisplayName: newFieldName,
annotationName: newFieldName,
description: newFieldDescription,
type: newFieldDataType,
});
// File editing step occurs after dispatch is processed and status is updated
dispatch(
metadata.actions.createAnnotation(
annotation,
dropdownOptions.map((opt) => opt.text)
)
);
}

return (
<>
{/* TO DO: Prevent user from entering a name that collides with existing annotation */}
<TextField
required
label="New metadata field name"
className={styles.textField}
onChange={(_, newValue) => setNewFieldName(newValue || "")}
onChange={(ev, newValue) => onChangeAlphanumericField(ev, newValue)}
placeholder="Add a new field name..."
value={newFieldName}
/>
{step === EditStep.CREATE_FIELD && (
<>
<TextField
multiline
rows={2}
label="Description"
className={styles.textField}
onChange={(_, newValue) => setNewFieldDescription(newValue || "")}
placeholder="Add a short description of the new field..."
value={newFieldDescription}
/>
<ComboBox
className={styles.comboBox}
selectedKey={newFieldDataType || undefined}
Expand All @@ -91,7 +205,9 @@ export default function NewAnnotationPathway(props: NewAnnotationProps) {
};
})}
onChange={(option) =>
setNewFieldDataType((option?.key as AnnotationType) || "")
setNewFieldDataType(
(option?.key as AnnotationType) || AnnotationType.STRING
)
}
/>
{newFieldDataType === AnnotationType.DROPDOWN && (
Expand Down Expand Up @@ -132,17 +248,38 @@ export default function NewAnnotationPathway(props: NewAnnotationProps) {
</>
)}
{step === EditStep.EDIT_FILES && (
<MetadataDetails
fieldType={newFieldDataType}
items={[
{
value: undefined,
fileCount: props.selectedFileCount,
} as ValueCountItem,
]}
dropdownOptions={dropdownOptions}
onChange={(value) => setNewValues(value)}
/>
<>
{newFieldDescription.trim() && (
<>
<b>Description: </b> {newFieldDescription}
</>
)}
<MetadataDetails
fieldType={newFieldDataType}
items={[
{
value: undefined,
fileCount: props.selectedFileCount,
} as ValueCountItem,
]}
dropdownOptions={dropdownOptions}
onChange={(value) => setNewValues(value)}
/>
{submissionStatus && (
<div
Copy link
Contributor

Choose a reason for hiding this comment

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

very nice feedback here

className={classNames(
submissionStatus.status === ProcessStatus.ERROR
? styles.errorMessage
: styles.statusMessage
)}
>
{submissionStatus.status === ProcessStatus.STARTED && (
<Spinner className={styles.spinner} size={SpinnerSize.small} />
)}
{submissionStatus?.message}
</div>
)}
</>
)}
<div className={styles.footer}>
<Stack horizontal>
Expand Down
4 changes: 2 additions & 2 deletions packages/core/components/EditMetadata/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ enum EditMetadataPathway {
interface EditMetadataProps {
className?: string;
onDismiss: () => void;
onUnsavedChanges: (hasUnsavedChanges: boolean) => void;
setHasUnsavedChanges: (hasUnsavedChanges: boolean) => void;
}

/**
Expand Down Expand Up @@ -101,7 +101,7 @@ export default function EditMetadataForm(props: EditMetadataProps) {
) : (
<NewAnnotationPathway
onDismiss={props.onDismiss}
onUnsavedChanges={() => props.onUnsavedChanges(true)}
setHasUnsavedChanges={(arg) => props.setHasUnsavedChanges(arg)}
selectedFileCount={fileCount}
/>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { Provider } from "react-redux";
import * as sinon from "sinon";

import Annotation from "../../../entity/Annotation";
import { AnnotationType } from "../../../entity/AnnotationFormatter";
import FileDetail from "../../../entity/FileDetail";
import FileSet from "../../../entity/FileSet";
import { initialState } from "../../../state";
Expand All @@ -18,7 +19,7 @@ describe("<LazilyRenderedRow />", () => {
annotationDisplayName: "Name",
annotationName: "file_name",
description: "name of file",
type: "Text",
type: AnnotationType.STRING,
});

function makeItemData() {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/components/Modal/EditMetadata/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export default function EditMetadata({ onDismiss }: ModalProps) {
<EditMetadataForm
className={classNames({ [styles.hidden]: showWarning })}
onDismiss={onDismissWithWarning}
onUnsavedChanges={setHasUnsavedChanges}
setHasUnsavedChanges={setHasUnsavedChanges}
/>
<div className={classNames({ [styles.hidden]: !showWarning })}>
<p className={styles.warning}>
Expand Down
Loading