-
Notifications
You must be signed in to change notification settings - Fork 12
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
Get rid of all of the arrays with requiredFields
#771
Comments
This also applies to "changedFields" in the mutations btw |
AFAICS we need to modify how we integrate types of application inputs, as we can't import the application.js config file directly as it is right now because it imports frontend components and the node import resolver doesn't like importing svgs as modules. This could be fixed, but that would be creating tech debt to sustain our tech debt. I think a discriminated union type enumerating the different input types and all of their possible options would be really good. I imagine it to look like this: type Textbox = {
input: "textbox"; // Field to be switched on
placeholder?: string;
validation?: RegExp | (value: string) => boolean;
typeaheadOptions: string[] | () => Promise<string[]>; // Fn allows options to be lazy-loaded
}
type SelectableValue<T extends string | number> = {
displayText: string;
value: T;
// Ideally we would get away from the current way we store the displayText in the DB as the value.
};
// T could be enum or string or something
type Checkboxes<T extends string|number> = {
input: "checkboxes";
options: SelectableValue<T>[];
// allows multiple to be required? not sure I love this API compared to ApplicationQuestion['required']
requiredValues: T[];
defaultValues: T[];
// Could exclusively use JSON.stringify/JSON.parse as deserializer/serializer to store values in the DB
// as strings in `ApplicationField` collection or could provide optional custom serializer/deserializer?
serializer?: (values: T[]) => string;
deserializer?: (serialized: string) => T[];
};
type Select<T extends string|number> = {
input: "select";
options: SelectableValue<T>[];
default: T;
addOther?: boolean;
};
type Boolean = {
input: "boolean";
default: 'yes' | 'no';
};
type Calendar = {
input: "calendar";
validRange: [Date, Date];
// Will use Date.toIsoString by default
serializer?: (value: Date) => string;
deserializer?: (value: string) => Date;
};
type ApplicationQuestion = {
/** Displayed only */
displayText: string;
/** Used to transmit/store this question in the DB. */
fieldName: string;
/** Field is required for successful application submission. */
required: boolean;
/** Discriminated union type which can be used to specify other properties in a type-safe fashion */
type: Text | Checkboxes | Slider | Boolean | File | ...;
}; We could then make Thoughts, @cktang88 @bencooper222 @aadibajpai @SamLee514 @gfting? |
There are several arrays throughout the codebase that are some form of a list of fields that are required or something. They're also out of sync with each other because... well that's how codebases work. Especially now that questions are declared in application.js (which should be a ts file I think?), each question should just be marked as "required" in there and enforced throughout the codebase.
The text was updated successfully, but these errors were encountered: