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

Allow digits in communication class type definition #8307

Merged
merged 1 commit into from
Feb 3, 2017
Merged

Allow digits in communication class type definition #8307

merged 1 commit into from
Feb 3, 2017

Conversation

cmuench
Copy link
Contributor

@cmuench cmuench commented Jan 27, 2017

Our company vendor prefix "N98" is not allowed. The XSD prohibits the usage of digits.

Our company vendor prefix "N98" is not allowed. The XSD prohibits the usage of digits.
@vrann vrann self-assigned this Jan 30, 2017
@vrann
Copy link
Contributor

vrann commented Jan 31, 2017

@cmuench can you please create integration test to expose the issue? You can update existing fixture: dev/tests/integration/testsuite/Magento/Framework/Communication/_files/valid_communication.xml

@vrann vrann self-requested a review January 31, 2017 20:42
@vrann
Copy link
Contributor

vrann commented Jan 31, 2017

@cmuench also I noticed, that the schema attribute of the topic node has the similar issue. Do you use any custom schema? Probably should be fixed as well

@mmansoor-magento mmansoor-magento merged commit ced639a into magento:develop Feb 3, 2017
mmansoor-magento pushed a commit that referenced this pull request Feb 3, 2017
- covered additional numeric cases with the communication.xsd schema changes
- added integration tests
mmansoor-magento pushed a commit that referenced this pull request Feb 3, 2017
- add unit test for AbstractTemplate Exception
mmansoor-magento pushed a commit that referenced this pull request Feb 3, 2017
- disable static test check for long line in unit tests
mmansoor-magento pushed a commit that referenced this pull request Feb 3, 2017
- fix static test check for long line in unit tests
@okorshenko
Copy link
Contributor

Thank you for contribution to Magento 2 project!

@tkn98
Copy link
Contributor

tkn98 commented Mar 28, 2017

Thanks for the fix, albeit for the flaw, this has more incarnations and like this issue it looks like that these are reported each on it's own (e.g. #4470 / #5420). The more fixes will be done peu-a-peu, the harder it will become to fix the flaw in it's whole. What about creating one issue we can review the XML type and check the diverse formats for same patterns?

tkn98 added a commit to tkn98/magento2 that referenced this pull request Mar 29, 2017
The solidusbackslash character ("\")  was put more than once in the same
regex character class across some XSD files. Per each character class
there is no need to specify it twice.

I ran over this while looking into an issue with class-name validation
which do not allow digits within class names.

Refs:

- https://www.w3.org/TR/xmlschema11-2/#cces

- magento#4470

- magento#5420

- magento#8307
@vrann
Copy link
Contributor

vrann commented Mar 30, 2017

@tkn98 Thank you, that's a great suggestion. It seem that you did something similar as a part of this PR #9044, is that right?

@tkn98
Copy link
Contributor

tkn98 commented Mar 31, 2017

@vrann: I'll update then, read this comment here later than the other issue #9044.

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.

5 participants