Skip to content

Conversation

@neillrobson
Copy link
Contributor

Description

In an attempt to resolve #14140, new complex types have been defined for every UI component specified in ui_settings.xsd line 49 (valid attribute codes for "formElement"). These complex types only include the <settings> child node, along with any grandchildren that pertain specifically to that form element (no abstractSettings). Finally, ui_configuration.xsd has been updated with a new group holding these complex types, and the corresponding validation for the <formElements> tag references this new group rather than the existing, more general group.

Fixed Issues (if relevant)

  1. UI component definition.map.xml overlooks XSD-valid nodes #14140: UI component definition.map.xml overlooks XSD-valid nodes

Manual testing scenarios

I have not tested these changes due to lack of time, nor am I sure that I have applied the appropriate fix to the task at hand. Please review both the functionality of the changes and the issue that they are intended to fix before merging, and feel free to make changes/a new PR if necessary.

In an attempt to resolve #14140, new complex types have been defined for
every UI component specified in `ui_settings.xsd` line 49 (valid
attribute codes for "formElement"). These complex types only include the
`<settings>` child node, along with any grandchildren that pertain
specifically to that form element (no `abstractSettings`). Finally,
`ui_configuration.xsd` has been updated with a new group holding these
complex types, and the corresponding validation for the `<formElements>`
tag references this new group rather than the existing, more general
group.
Copy link
Contributor

@VladimirZaets VladimirZaets left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RNanoware Please look at builds result on the travis-ci. You have set "formElementPrice" type to "price" element (formElementsConfig), but this type isn't declared.

I did a bit of code digging but couldn't figure out why the price
component definition did not have any valid/configured child nodes for
<settings> whatsoever. I decided that the best route would be to add
blank groups with the proper labels, so that the actual validation
results remain unchanged, but the skeleton now exists for future
developers to add valid configuration.
@neillrobson
Copy link
Contributor Author

@VladimirZaets I completely missed that--thank you! I couldn't figure out why the price component never had any valid <settings> configuration specified, but I made the assumption that adding empty groups/types with the proper labels would be the best course of action. The behavior of the validation itself remains unchanged, but the skeleton exists now for future developers to add configuration as needed.

I'm not sure if the solution proposed in this commit is correct, but I
couldn't figure out any more logical way to deal with the validation
errors on the Travis CI build. The error follows:

1) Magento\Test\Integrity\Xml\SchemaTest::testXmlFiles Passed: 1881,
Failed: 6, Incomplete: 0, Skipped: 0.  Data set:
/app/code/Magento/Catalog/view/adminhtml/ui_component/category_form.xml
Error validating
/home/travis/build/magento/magento2/app/code/Magento/Catalog/view/adminhtml/ui_component/category_form.xml
against urn:magento:module:Magento_Ui:etc/ui_configuration.xsd
Array (
[0] => Element 'fileUploader': This element is not expected. Expected is
one of ( hidden, file, input, date, boolean, checkbox, checkboxset,
email, select, multiselect ).  Line: 168
[1] => Element 'wysiwyg',
attribute 'class': The attribute 'class' is not allowed.  Line: 206
)
Failed asserting that an array is empty.

Element zero in that array is what I am attempting to fix here.
Without this part of the configuration, children of <formElements> nodes
can not have their attributes (such as "class") configured.
@neillrobson
Copy link
Contributor Author

Is there any way to run the Travis CI tests locally, or is pushing up code to my Github branch the only way to check for errors? Alas, the CI build takes a few hours, and I often don't have time during the week to come back and check on my mistakes.

Again, this error was discovered during the Travis CI build,
specifically during validation of
/app/code/Magento/Theme/view/adminhtml/ui_component/design_config_form.xml,
line 275.
@neillrobson neillrobson dismissed VladimirZaets’s stale review March 30, 2018 11:31

I believe that the issue mentioned about the undeclared type has been resolved with the recent commits. All Travis checks are passing as of the time this comment is being written.

@VladimirZaets VladimirZaets self-assigned this Apr 2, 2018
@magento-engcom-team magento-engcom-team added this to the April 2018 milestone Apr 2, 2018
@VladimirZaets VladimirZaets changed the base branch from 2.2-develop to 2.3-develop May 7, 2018 14:30
@VladimirZaets VladimirZaets changed the base branch from 2.3-develop to 2.2-develop May 7, 2018 14:30
@VladimirZaets
Copy link
Contributor

Hi @RNanoware , we had the discussion your PR, because it has backward incompatible changes.
Unfortunately, we can't merge it to Magento 2.2, but we are ok to merge it to Magento 2.3.
Can you please move your commits to the branch that will be based on 2.3-develop and change target pull request branch to 2.3-develop.

@neillrobson
Copy link
Contributor Author

I just made a new PR (which can be found here). I don't trust myself trying to manually edit an existing pull request to target a different branch without breaking something. Hope this helps; let me know if there is anything else I can do!

@VladimirZaets
Copy link
Contributor

@RNanoware thanks. I close this PR and will start work with PR to the 2.3 branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants