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

fix: LEAP-897: Fix incorrect usage of name attr in visual tags #5745

Merged
merged 12 commits into from
Apr 24, 2024
8 changes: 4 additions & 4 deletions web/dist/apps/labelstudio/version.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"message": "Merge branch 'develop' into 'fb-LEAP-1004'",
"commit": "2174366308f877c5eed33bb1ba03346e6eb8d76e",
"date": "2024-04-16T12:46:28.000Z",
"branch": "fb-LEAP-1004"
"message": "fix unit tests for Config Validator",
"commit": "26ea83343a36548f7325f504f581ba48b2400513",
"date": "2024-04-17T19:19:29.000Z",
"branch": "fb-leap-897"
}
8 changes: 4 additions & 4 deletions web/dist/libs/datamanager/version.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"message": "Merge branch 'develop' into 'fb-LEAP-1004'",
"commit": "2174366308f877c5eed33bb1ba03346e6eb8d76e",
"date": "2024-04-16T12:46:28.000Z",
"branch": "fb-LEAP-1004"
"message": "fix unit tests for Config Validator",
"commit": "26ea83343a36548f7325f504f581ba48b2400513",
"date": "2024-04-17T19:19:29.000Z",
"branch": "fb-leap-897"
}
2 changes: 1 addition & 1 deletion web/dist/libs/editor/main.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion web/dist/libs/editor/main.js.map

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions web/dist/libs/editor/version.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"message": "Merge branch 'develop' into 'fb-LEAP-1004'",
"commit": "2174366308f877c5eed33bb1ba03346e6eb8d76e",
"date": "2024-04-16T12:46:28.000Z",
"branch": "fb-LEAP-1004"
"message": "fix unit tests for Config Validator",
"commit": "26ea83343a36548f7325f504f581ba48b2400513",
"date": "2024-04-17T19:19:29.000Z",
"branch": "fb-leap-897"
}
20 changes: 20 additions & 0 deletions web/libs/editor/src/core/DataValidator/ConfigValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,21 @@ const validateParentTag = (element, model) => {
return errorBuilder.parentTagUnexpected(model.name, 'parent', element.tagName, model.properties.parentTypes);
};

/**
* Validates if visual tags have name attribute
* @param {Object} element
*/
const validateVisualTags = (element) => {
const visualTags = ['Collapse', 'Filter', 'Header', 'Style', 'View'];
Gondragos marked this conversation as resolved.
Show resolved Hide resolved
const { tagName } = element;

if (visualTags.includes(tagName) && element.name) {
return errorBuilder.generalError('Attribute <b>name</b> is not allowed for tag <b>' + tagName + '</b>.');
juliosgarbi marked this conversation as resolved.
Show resolved Hide resolved
}

return null;
}

/**
* Validate other tag attributes other than name and toName
* @param {Object} child
Expand Down Expand Up @@ -301,6 +316,11 @@ export class ConfigValidator {

if (parentValidation !== null) validationResult.push(parentValidation);

//validate visualTags
juliosgarbi marked this conversation as resolved.
Show resolved Hide resolved
const visualTagsValidation = validateVisualTags(child);

if (visualTagsValidation !== null) validationResult.push(visualTagsValidation);

validationResult.push(...validatePerRegion(child));

validationResult.push(...validateAttributes(child, model, propertiesToSkip));
Expand Down
73 changes: 46 additions & 27 deletions web/libs/editor/src/core/__tests__/ConfigValidator.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,42 +4,41 @@ import '../../tags/object/Image';
import '../../tags/object/RichText';
import '../../tags/control/RectangleLabels';
import '../../tags/control/Label';
import '../../tags/control/Choices';
import '../../tags/control/Choice';
import '../../tags/visual/Header';
import { ConfigValidator } from "../DataValidator/ConfigValidator";

// IMPORTANT NOTE
// all tests are disabled because they wait `validation` in the root component
// and this doesn't happen, may be that was some old behavior
// @todo fix this
function createStore(utterances) {
return { task: { dataObj: { utterances } } };
juliosgarbi marked this conversation as resolved.
Show resolved Hide resolved
}

it.skip('Should fail if a tag referenced by toName doesn\'t exist', () => {
const result = Tree.treeToModel(`
it('Should fail if a tag referenced by toName doesn\'t exist', () => {
const result = ConfigValidator.validate(Tree.treeToModel(`
<View>
<Image name="img1" value="$image"></Image>
<RectangleLabels name="tag" toName="img" fillOpacity="0.5" strokeWidth="5">
<Label value="Planet"></Label>
<Label value="Moonwalker" background="blue"></Label>
</RectangleLabels>
</View>
`);
`, createStore(['hello', 'world'])));

const errorItem = result.validation[0];

expect(errorItem.error).toBe('ERR_TAG_NOT_FOUND');
expect(result[0].error).toBe('ERR_TAG_NOT_FOUND');
});

it.skip('Should fail if a tag referenced by toName is not image', () => {
const result = Tree.treeToModel(`
it('Should fail if a tag referenced by toName is not image', () => {
const result = ConfigValidator.validate(Tree.treeToModel(`
<View>
<HyperText name="img" value="$text"></HyperText>
<RectangleLabels name="tag" toName="img" fillOpacity="0.5" strokeWidth="5">
<Label value="Planet"></Label>
<Label value="Moonwalker" background="blue"></Label>
</RectangleLabels>
</View>
`);

const errorItem = result.validation[0];
`, createStore(['hello', 'world'])));

expect(errorItem.error).toBe('ERR_TAG_UNSUPPORTED');
expect(result[0].error).toBe('ERR_TAG_UNSUPPORTED');
});

it.skip('Should fail if tag lacks mandatory attribute toName', () => {
Expand All @@ -58,34 +57,54 @@ it.skip('Should fail if tag lacks mandatory attribute toName', () => {
expect(errorItem.error).toBe('ERR_REQUIRED');
});

it.skip('Should fail if opacity attribute is out of range', () => {
const result = Tree.treeToModel(`
it('Should fail if opacity attribute is out of range', () => {
const result = ConfigValidator.validate(Tree.treeToModel(`
<View>
<Image name="img" value="$image"></Image>
<RectangleLabels name="tag" toName="img" fillOpacity="-1" strokeWidth="5">
<Label value="Planet"></Label>
<Label value="Moonwalker" background="blue"></Label>
</RectangleLabels>
</View>
`);

const errorItem = result.validation[0];
`, createStore(['hello', 'world'])));

expect(errorItem.error).toBe('ERR_BAD_TYPE');
expect(result[0].error).toBe('ERR_BAD_TYPE');
});

it.skip('Should fail if color is not a proper CSS color', () => {
const result = Tree.treeToModel(`
it('Should fail if color is not a proper CSS color', () => {
const result = ConfigValidator.validate(Tree.treeToModel(`
<View>
<Image name="img" value="$image"></Image>
<RectangleLabels name="tag" toName="img" fillOpacity="0.6" strokeWidth="5">
<Label value="Planet"></Label>
<Label value="Moonwalker" background="verywrongcolor"></Label>
</RectangleLabels>
</View>
`);
`, createStore(['hello', 'world'])));

expect(result[0].error).toBe('ERR_BAD_TYPE');
});

const errorItem = result.validation[0];

expect(errorItem.error).toBe('ERR_BAD_TYPE');
it('Should fail if visual tags have name attribute', () => {
const result = ConfigValidator.validate(Tree.treeToModel(`
<View>
<Header name="w3"/>
<Text name="text1" value="Did the agent follow up to ensure that both parties were satisfied with the outcome and understood the resolution"/>
<Image name="image" value="$image" zoom="true"/>
<Choices name="choice" toName="label" choice="single">
<Choice value="Yes"/>
<Choice value="No"/>
</Choices>
<PolygonLabels name="label" toName="image"
strokeWidth="3" pointSize="small"
opacity="0.9">
<Label value="Airplane" background="red"/>
<Label value="Car" background="blue"/>
</PolygonLabels>
</View>
`, createStore(['hello', 'world'])));

expect(result[0].error).toBe('ERR_GENERAL');
expect(result[0].value).toBe('Attribute <b>name</b> is not allowed for tag <b>Header</b>.');
});
Loading