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

Fixes/variants form submit #475

Merged
merged 6 commits into from
Jan 7, 2025
Merged

Fixes/variants form submit #475

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
55adf6f
fix: refactor search form verification
gsfk Dec 20, 2024
edda7e3
update antd form when editing form tabs
gsfk Jan 6, 2025
b2040fb
use util fn objectWithoutProp instead
gsfk Jan 6, 2025
619ff50
better comment
gsfk Jan 7, 2025
72eab4e
move conditionValidator to search utils + import fixes
gsfk Jan 7, 2025
07cd5e4
move constant out of component
gsfk Jan 7, 2025
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
37 changes: 37 additions & 0 deletions src/components/discovery/DiscoveryQueryBuilder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,35 @@ import {
import { useDataTypes, useServices } from "@/modules/services/hooks";
import { useAppDispatch, useAppSelector } from "@/store";
import { nop } from "@/utils/misc";
import { VARIANT_OPTIONAL_FIELDS } from "@/utils/search";
import { objectWithoutProp } from "@/utils/misc";
davidlougheed marked this conversation as resolved.
Show resolved Hide resolved

import DataTypeExplorationModal from "./DataTypeExplorationModal";
import DiscoverySearchForm from "./DiscoverySearchForm";
import { getSchemaTypeTransformer } from "./DiscoverySearchCondition";

const conditionValidator = (rule, { field, fieldSchema, searchValue }) => {
if (field === undefined) {
return Promise.reject("A field must be specified for this search condition.");
}

const transformedSearchValue = getSchemaTypeTransformer(fieldSchema.type)[1](searchValue);
const isEnum = fieldSchema.hasOwnProperty("enum");
const isString = fieldSchema.type === "string";

// noinspection JSCheckFunctionSignatures
if (
!VARIANT_OPTIONAL_FIELDS.includes(field) &&
(transformedSearchValue === null ||
(!isEnum && !transformedSearchValue) ||
(!isEnum && isString && !transformedSearchValue.trim()) || // Forbid whitespace-only free-text searches
(isEnum && !fieldSchema.enum.includes(transformedSearchValue)))
) {
return Promise.reject(`This field must have a value: ${field}`);
}

return Promise.resolve();
};

const DiscoveryQueryBuilder = ({ activeDataset, dataTypeForms, requiredDataTypes, onSubmit, searchLoading }) => {
const dispatch = useAppDispatch();
Expand Down Expand Up @@ -44,6 +70,9 @@ const DiscoveryQueryBuilder = ({ activeDataset, dataTypeForms, requiredDataTypes
(key, action) => {
if (action !== "remove") return;
dispatch(removeDataTypeQueryForm(activeDataset, dataTypesByID[key]));

// remove this field from submitted form
setForms((fs) => objectWithoutProp(fs, key));
},
[dispatch, activeDataset, dataTypesByID],
);
Expand All @@ -57,6 +86,13 @@ const DiscoveryQueryBuilder = ({ activeDataset, dataTypeForms, requiredDataTypes

try {
await Promise.all(Object.values(forms).map((f) => f.validateFields()));

// force validation of variant fields, validateFields() has no effect
if (forms["variant"]) {
const variantFields = forms["variant"].getFieldValue("conditions");
await Promise.all(variantFields.map((field) => conditionValidator(null, field)));
}

// TODO: If error, switch to errored tab
(onSubmit ?? nop)();
} catch (err) {
Expand Down Expand Up @@ -133,6 +169,7 @@ const DiscoveryQueryBuilder = ({ activeDataset, dataTypeForms, requiredDataTypes
setFormRef={setFormRef}
onChange={handleChange}
handleVariantHiddenFieldChange={handleVariantHiddenFieldChange}
conditionValidator={conditionValidator}
davidlougheed marked this conversation as resolved.
Show resolved Hide resolved
/>
),
};
Expand Down
54 changes: 13 additions & 41 deletions src/components/discovery/DiscoverySearchForm.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,55 +13,18 @@ import {
OP_GREATER_THAN_OR_EQUAL,
OP_LESS_THAN_OR_EQUAL,
searchUiMappings,
VARIANT_REQUIRED_FIELDS,
VARIANT_OPTIONAL_FIELDS,
} from "@/utils/search";

import DiscoverySearchCondition, { getSchemaTypeTransformer } from "./DiscoverySearchCondition";
import DiscoverySearchCondition from "./DiscoverySearchCondition";
import VariantSearchHeader from "./VariantSearchHeader";

const TOOLTIP_DELAY_SECONDS = 0.8;

// required by ui not precisely the same as required in spec, possible TODO
const VARIANT_REQUIRED_FIELDS = [
"[dataset item].assembly_id",
"[dataset item].chromosome",
"[dataset item].start",
"[dataset item].end",
];

const VARIANT_OPTIONAL_FIELDS = [
"[dataset item].calls.[item].genotype_type",
"[dataset item].alternative",
"[dataset item].reference",
];

const updateVariantConditions = (conditions, fieldName, searchValue) =>
conditions.map((c) => (c.field === fieldName ? { ...c, searchValue } : c));

const conditionValidator = (rule, { field, fieldSchema, searchValue }) => {
if (field === undefined) {
return Promise.reject("A field must be specified for this search condition.");
}

const transformedSearchValue = getSchemaTypeTransformer(fieldSchema.type)[1](searchValue);
const isEnum = fieldSchema.hasOwnProperty("enum");
const isString = fieldSchema.type === "string";

// noinspection JSCheckFunctionSignatures
if (
!VARIANT_OPTIONAL_FIELDS.includes(field) &&
(transformedSearchValue === null ||
(!isEnum && !transformedSearchValue) ||
(!isEnum && isString && !transformedSearchValue.trim()) || // Forbid whitespace-only free-text searches
(isEnum && !fieldSchema.enum.includes(transformedSearchValue)))
) {
return Promise.reject(`This field must have a value: ${field}`);
}

return Promise.resolve();
};

const CONDITION_RULES = [{ validator: conditionValidator }];

const CONDITION_LABEL_COL = {
lg: { span: 24 },
xl: { span: 4 },
Expand Down Expand Up @@ -92,9 +55,17 @@ PhenopacketDropdownOption.propTypes = {
getDataTypeFieldSchema: PropTypes.func,
};

const DiscoverySearchForm = ({ onChange, dataType, setFormRef, handleVariantHiddenFieldChange }) => {
const DiscoverySearchForm = ({
onChange,
dataType,
setFormRef,
handleVariantHiddenFieldChange,
conditionValidator,
}) => {
const [form] = Form.useForm();

const CONDITION_RULES = [{ validator: conditionValidator }];
davidlougheed marked this conversation as resolved.
Show resolved Hide resolved

useEffect(() => {
if (setFormRef) setFormRef(form);
}, [form, setFormRef]);
Expand Down Expand Up @@ -394,6 +365,7 @@ DiscoverySearchForm.propTypes = {
dataType: PropTypes.object, // TODO: Shape?
setFormRef: PropTypes.func,
handleVariantHiddenFieldChange: PropTypes.func.isRequired,
conditionValidator: PropTypes.func,
};

export default DiscoverySearchForm;
9 changes: 8 additions & 1 deletion src/utils/search.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,14 @@ export const DEFAULT_SEARCH_PARAMETERS = {
queryable: "all",
};

const VARIANT_OPTIONAL_FIELDS = [
export const VARIANT_REQUIRED_FIELDS = [
"[dataset item].assembly_id",
"[dataset item].chromosome",
"[dataset item].start",
"[dataset item].end", //always filled in UI but not required in spec
];

export const VARIANT_OPTIONAL_FIELDS = [
"[dataset item].calls.[item].genotype_type",
"[dataset item].alternative",
"[dataset item].reference",
Expand Down
Loading