geovistory / design-system

MIT License
2 stars 1 forks source link

Yasgui Data Validation Component #117

Closed joschne closed 9 months ago

joschne commented 11 months ago

Problem description

We have components that are used by Yasgui plugins. These components work standalone. They are plugged in by a Yasgui Plugin class. The components have specific requirements for input data. The sparql results do not necessarily match these requirements, when the component is used as a Yasgui plugin.

Currently, the component itself is responsible for the validation of the input data and, in case of invalid data, displaying a warning that helps the user to write an adequate query.

In terms of separation of concerns, this is not clean: The standalone component should be independent from yasgui. The data validation and display of warnings should be handled by the Yasgui Plugin. The data should only be passed to the plugged-in component, if valid.

This would allow to create a generic data validation component, shared by all Yasgui Plugins. This avoids repeating the validation logic for each plugin component.

Functional requirement

The component has to validate input data and emit the validation result (valid/invalid). In case of invalid data, it has to display understandable warnings/error messages.

Technical Spec A webcomponent with two inputs:

interface ExpectedKey {
  name: string;  // name of the key
  required: boolean; // if true, the key has to be available 
  datatype: string; // expected data type of the values in the binding
}

The component then creates error objects, allowing to store keys that mismatch a validation rule.

const requiredMismatch = new Set<string>();
const datatypeMismatch = new Set<string>();

Then it iterates over the data. For each record it loops over expectedKeys and performs these validations:

The webcomponent has an even emitter "validated" that emits a boolean. True=valid, false=invalid. In case requiredMismatch and datatypeMismatch are empty, emit true;

Else emit else false and render errors:

For each key in requiredMismatch write The variable ?{key} must not return empty values. Currently it is either not bound or it returns empty records.

For each key in requiredMismatch write The variable ?{key} must be of datatype {expectedKeys.find(e=>e.name==key).datatype}. Some or all records do not match that data type.

The Yasgui Plugin first injects the data to this component and listens to the validated event. If valid, it passes the data to the plugin component and attaches the plugin component to the DOM. Else it attaches the data validation component to the DOM.

joschne commented 11 months ago

If we create such a component, it should be integrated into the Yasgui Plugins of map-with-circles, time-line, time-map-with-circles, etc.

flicksolutions commented 11 months ago

This can only provide simple testing whether data exists or not. some validations are a bit more complicated:

Those checks depend very much on the component and what it is doing with the data. I therefore think, we should keep the validation of the data within the component. Because the validations are very individual to the component.

What do you think @perrauda?

perrauda commented 11 months ago

The idea is good, but I think we need to add a third input, an optional array of one or more callback functions that check whether the data is correct ?

A webcomponent with three inputs:

data: Parser.Binding[], an array of bindings as returned by the sparql endpoint expectedKeys: ExpectedKey[] callbacks?: DataValidationCallback[]

with

type DataValidationCallback = (value: any) => boolean;

So for every callback that returns true, everything's OK, but if it's false, it can return an error const dataIsNotValid = new Set<string>();

joschne commented 11 months ago

I understand @flicksolutions – I also like the idea of @perrauda. What is still missing, is a message printed to the user in case dataIsNotValid contains data.

What if we do it so?

interface ExpectedKey {
  name: string;  // name of the key
  required: boolean; // if true, the key has to be available 
  datatype: string; // expected data type of the values in the binding
  // optional callback for custom validations. Returns validation error message in case of invalid data.
  customValidator?: (val: Parser.BindingValue)=> string | undefined
}

and this:

const dataIsNotValid: {[key:string]: new Set<string>()} = {}

If customValidator() returns a string (a validation error message), this is put to dataNotValid[key].put(message).

Let's assume this as a result of the validation:

const dataNotValid =  {
   ["lat"] : new Set(['Value must not be bigger than 90','Value must not be smaller than -90'])
}

The UI shows the messages:

The variable "lat" contains invalid values: 
- Value must not be bigger than 90. 
- Value must not be smaller than -90.

This way we could streamline the validation error UI and still keep a lot of flexibility.

Those checks depend very much on the component and what it is doing with the data. I therefore think, we should keep the validation of the data within the component. Because the validations are very individual to the component.

I agree @flicksolutions. Then the data-validation component could be directly used within the plugin-components and not in the plugin class.

flicksolutions commented 10 months ago

agreed! this seems very sensible!

flicksolutions commented 10 months ago

I'd say this is about 1.5 full day of work. (inculding integrating it in existing components)

joschne commented 10 months ago

Perfect! The issue spec is done – @perrauda it is ready for you to be worked on. You may combine it with the implementation of the entity timeline (#54) as a Yasgui Plugin :-)