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

Ensure unique volume layer names when changing names in frontend #6289

Merged
merged 11 commits into from
Jul 4, 2022
Merged
6 changes: 4 additions & 2 deletions frontend/javascripts/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,10 @@ instead. Only enable this option if you understand its effect. All layers will n
"tracing.volume_missing_segmentation": "Volume is allowed, but segmentation does not exist.",
"tracing.volume_layer_name_duplication":
"This layer name already exists! Please change it to resolve duplicates.",
"tracing.volume_layer_name_includes_slash":
'This layer name includes the disallowed character "/". Please delete it.',
"tracing.volume_layer_name_includes_invalid_characters": (disallowedCharacters: string) =>
`This layer name includes the disallowed character${
disallowedCharacters.length > 1 ? "s" : ""
} '${disallowedCharacters}'. Please delete it.`,
"tracing.delete_initial_node": "Do you really want to delete the initial node?",
"tracing.delete_tree": "Do you really want to delete the whole tree?",
"tracing.delete_tree_with_initial_node":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ import {
getVisibleSegmentationLayer,
getDataLayers,
} from "oxalis/model/accessors/dataset_accessor";
import _ from "lodash";
import messages from "messages";
import { getMaxZoomStepDiff } from "oxalis/model/bucket_data_handling/loading_strategy_logic";
import { getFlooredPosition, getRequestLogZoomStep } from "oxalis/model/accessors/flycam_accessor";
import { reuseInstanceOnEquality } from "oxalis/model/accessors/accessor_helpers";
Expand Down Expand Up @@ -109,44 +107,6 @@ export function getAllReadableLayerNames(dataset: APIDataset, tracing: Tracing)
return allReadableLayerNames;
}

export function checkForLayerNameDuplication(
readableLayerName: string,
dataset: APIDataset,
tracing: Tracing,
currentName?: string | null,
): string | true {
let countToFindDuplication = 1;
if (readableLayerName === currentName) {
countToFindDuplication = 2;
}
const allReadableLayerNames = getAllReadableLayerNames(dataset, tracing);
const doesNewNameAlreadyExist =
_.countBy(allReadableLayerNames)[readableLayerName] >= countToFindDuplication;
return doesNewNameAlreadyExist ? messages["tracing.volume_layer_name_duplication"] : true;
}

export function checkForInvalidCharacters(readableLayerName: string): string | true {
return readableLayerName.includes("/")
? messages["tracing.volume_layer_name_includes_slash"]
: true;
}

export function validateReadableLayerName(
readableLayerName: string,
dataset: APIDataset,
tracing: Tracing,
dontCountGivenName?: boolean,
): string | true {
const hasDuplicatedName = checkForLayerNameDuplication(
readableLayerName,
dataset,
tracing,
dontCountGivenName ? readableLayerName : null,
);
const nameContainsSlash = checkForInvalidCharacters(readableLayerName);
return hasDuplicatedName === true ? nameContainsSlash : hasDuplicatedName;
}

function getSegmentationLayerForTracing(
state: OxalisState,
volumeTracing: VolumeTracing,
Expand Down
23 changes: 13 additions & 10 deletions frontend/javascripts/oxalis/view/components/editable_text_label.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ import Markdown from "react-remarkable";
import * as React from "react";
import { MarkdownModal } from "oxalis/view/components/markdown_modal";
import Toast from "libs/toast";
import { ValidationResult } from "../left-border-tabs/modals/add_volume_layer_modal";
type Rule = {
message?: string;
type?: string;
validator?: (arg0: number | string) => boolean;
validator?: (arg0: string) => ValidationResult;
};
export type EditableTextLabelProp = {
value: string;
Expand All @@ -22,7 +23,6 @@ export type EditableTextLabelProp = {
disableEditing?: boolean;
onContextMenu?: () => void;
width?: string | number;
updateOnAllChanges?: boolean;
isInvalid?: boolean | null | undefined;
trimValue?: boolean | null | undefined;
};
Expand All @@ -34,7 +34,6 @@ type State = {
class EditableTextLabel extends React.PureComponent<EditableTextLabelProp, State> {
static defaultProps = {
rows: 1,
updateOnAllChanges: false,
isInvalid: false,
trimValue: false,
rules: [],
Expand Down Expand Up @@ -91,18 +90,22 @@ class EditableTextLabel extends React.PureComponent<EditableTextLabelProp, State
return true;
}
const allRulesValid = this.props.rules.every((rule) => {
let isValid = true;
if (rule.type === "email") {
const re =
/^(([^<>()[\]\\.,;:\s@"]+(\.[^<>()[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/;
isValid = re.test(this.state.value);
const isValid = re.test(this.state.value);
if (!isValid) {
Toast.error(rule.message);
return false;
}
} else if (rule.validator != null) {
isValid = rule.validator(this.state.value);
const validationResult = rule.validator(this.state.value);
if (!validationResult.isValid) {
Toast.error(rule.message);
MichaelBuessemeyer marked this conversation as resolved.
Show resolved Hide resolved
return false;
}
}
if (!isValid && rule.message) {
Toast.error(rule.message);
}
return isValid;
return true;
});
return allRulesValid;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,10 @@ import {
} from "oxalis/model/accessors/dataset_accessor";
import { getMaxZoomValueForResolution } from "oxalis/model/accessors/flycam_accessor";
import {
checkForInvalidCharacters,
checkForLayerNameDuplication,
getAllReadableLayerNames,
getReadableNameByVolumeTracingId,
getVolumeDescriptorById,
getVolumeTracingById,
validateReadableLayerName,
} from "oxalis/model/accessors/volumetracing_accessor";
import {
setNodeRadiusAction,
Expand Down Expand Up @@ -83,9 +81,9 @@ import Store from "oxalis/store";
import Toast from "libs/toast";
import * as Utils from "libs/utils";
import api from "oxalis/api/internal_api";
import messages, { settings } from "messages";
import { settings } from "messages";
import { MaterializeVolumeAnnotationModal } from "oxalis/view/right-border-tabs/starting_job_modals";
import AddVolumeLayerModal from "./modals/add_volume_layer_modal";
import AddVolumeLayerModal, { validateReadableLayerName } from "./modals/add_volume_layer_modal";
import DownsampleVolumeModal from "./modals/downsample_volume_modal";
import Histogram, { isHistogramSupported } from "./histogram_view";
import MappingSettingsView from "./mapping_settings_view";
Expand Down Expand Up @@ -348,11 +346,11 @@ class DatasetSettings extends React.PureComponent<DatasetSettingsProps, State> {
"tracingId" in layer && layer.tracingId != null
? getReadableNameByVolumeTracingId(tracing, layer.tracingId)
: layerName;
const isReadableNameValidOrWarning = validateReadableLayerName(
const allReadableLayerNames = getAllReadableLayerNames(dataset, tracing);
const readableLayerNameValidationResult = validateReadableLayerName(
readableName,
allReadableLayerNames,
readableName,
dataset,
tracing,
true,
);
return (
<div className="flex-container">
Expand All @@ -366,14 +364,18 @@ class DatasetSettings extends React.PureComponent<DatasetSettingsProps, State> {
>
{volumeDescriptor != null ? (
<Tooltip
title={isReadableNameValidOrWarning === true ? null : isReadableNameValidOrWarning}
title={
readableLayerNameValidationResult.isValid
? null
: readableLayerNameValidationResult.message
}
>
<span style={{ display: "inline-block" }}>
<EditableTextLabel
margin="0 10px 0 0"
width={150}
value={readableName}
isInvalid={isReadableNameValidOrWarning !== true}
isInvalid={!readableLayerNameValidationResult.isValid}
trimValue
onChange={(newName) => {
this.props.onEditAnnotationLayer(volumeDescriptor.tracingId, {
Expand All @@ -382,23 +384,12 @@ class DatasetSettings extends React.PureComponent<DatasetSettingsProps, State> {
}}
rules={[
{
message: messages["tracing.volume_layer_name_duplication"],
validator: (newLayerName) =>
typeof newLayerName === "string"
? checkForLayerNameDuplication(
newLayerName,
dataset,
tracing,
readableName,
) === true
: false,
},
{
message: messages["tracing.volume_layer_name_includes_slash"],
validator: (newLayerName) =>
typeof newLayerName === "string"
? checkForInvalidCharacters(newLayerName) === true
: false,
validator: (newReadableLayerName) =>
validateReadableLayerName(
newReadableLayerName,
allReadableLayerNames,
readableName,
),
},
]}
label="Volume Layer Name"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,67 @@ import {
import {
getAllReadableLayerNames,
getVolumeTracingLayers,
validateReadableLayerName,
} from "oxalis/model/accessors/volumetracing_accessor";
import messages from "messages";
import InputComponent from "oxalis/view/components/input_component";
import api from "oxalis/api/internal_api";
import Toast from "libs/toast";

export type ValidationResult = { isValid: boolean; message: string };
export function checkForLayerNameDuplication(
readableLayerName: string,
allReadableLayerNames: string[],
): ValidationResult {
const layerNameDoesNotExist = _.countBy(allReadableLayerNames)[readableLayerName] <= 0;
daniel-wer marked this conversation as resolved.
Show resolved Hide resolved
return {
isValid: layerNameDoesNotExist,
message: layerNameDoesNotExist ? "" : messages["tracing.volume_layer_name_duplication"],
};
}

export function checkLayerNameForInvalidCharacters(readableLayerName: string): ValidationResult {
const uriSaveCharactersRegex = /[0-9a-zA-Z-._~]+/g;
MichaelBuessemeyer marked this conversation as resolved.
Show resolved Hide resolved
// Removing all URISaveCharacters from readableLayerName. The left over chars are all invalid.
const allInvalidChars = readableLayerName.replace(uriSaveCharactersRegex, "");
const allUniqueInvalidCharsAsSet = new Set(...allInvalidChars);
daniel-wer marked this conversation as resolved.
Show resolved Hide resolved
const allUniqueInvalidCharsAsString = "".concat(...allUniqueInvalidCharsAsSet.values());
const isValid = allUniqueInvalidCharsAsString.length === 0;
return {
isValid,
message: isValid
? ""
: messages["tracing.volume_layer_name_includes_invalid_characters"](
allUniqueInvalidCharsAsString,
),
};
}

export function validateReadableLayerName(
readableLayerName: string,
allReadableLayerNames: string[],
nameNotToCount?: string,
): ValidationResult {
if (nameNotToCount) {
// If the given nameNotToCount is already once within the allReadableLayerNames. But as this name is now being renamed and having
// the same name as the previous name given by nameNotToCount should be still allowed, this name is removed from the array once.
// Nevertheless, additional duplicated of nameNotToCount are counted as a duplication.
MichaelBuessemeyer marked this conversation as resolved.
Show resolved Hide resolved
const index = allReadableLayerNames.indexOf(nameNotToCount);
if (index > -1) {
allReadableLayerNames = _.clone(allReadableLayerNames); // Avoiding modifying passed parameters.
allReadableLayerNames.splice(index, 1);
}
}
const duplicatedNameResult = checkForLayerNameDuplication(
readableLayerName,
allReadableLayerNames,
);
if (!duplicatedNameResult.isValid) {
return duplicatedNameResult;
}
const invalidCharactersResult = checkLayerNameForInvalidCharacters(readableLayerName);
return invalidCharactersResult;
}

export default function AddVolumeLayerModal({
dataset,
onCancel,
Expand All @@ -34,8 +90,11 @@ export default function AddVolumeLayerModal({
const [selectedSegmentationLayerIndex, setSelectedSegmentationLayerIndex] = useState<
number | null | undefined
>(null);
const allReadableLayerNames = useMemo(
() => getAllReadableLayerNames(dataset, tracing),
[dataset, tracing],
);
const initialNewLayerName = useMemo(() => {
const allReadableLayerNames = getAllReadableLayerNames(dataset, tracing);
if (allReadableLayerNames.indexOf("Volume") === -1) {
return "Volume";
} else {
Expand Down Expand Up @@ -64,9 +123,9 @@ export default function AddVolumeLayerModal({
let selectedSegmentationLayer = null;
const handleAddVolumeLayer = async () => {
await api.tracing.save();
const isNewLayerNameValidOrWarning = validateReadableLayerName(newLayerName, dataset, tracing);
if (isNewLayerNameValidOrWarning !== true) {
Toast.error(isNewLayerNameValidOrWarning);
const validationResult = validateReadableLayerName(newLayerName, allReadableLayerNames);
if (!validationResult.isValid) {
Toast.error(validationResult.message);
return;
}
const minResolutionAllowed = Math.max(
Expand Down