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

Rules with attribute as context (UBL) #50

Closed
klakegg opened this issue Feb 6, 2018 · 20 comments · Fixed by #51
Closed

Rules with attribute as context (UBL) #50

klakegg opened this issue Feb 6, 2018 · 20 comments · Fixed by #51

Comments

@klakegg
Copy link
Contributor

klakegg commented Feb 6, 2018

The following rules has a context targeting an attribute:

  • BR-CL-03
  • BR-CL-23
  • BR-CL-24

Those rules will not trigger when implementers use the official ISO Schematron stylesheets to generate the XSLT version.

@oriol
Copy link
Collaborator

oriol commented Feb 8, 2018

The problem affects all contexts targeting an attribute.

It seems it is a bug in Schematron's official stylesheets.

https://stackoverflow.com/questions/40045725/when-using-xslt-2-schematron-ignores-rules-that-have-attributes-in-their-contex

The second solution described in the link above seems the best solution.

@phax
Copy link
Collaborator

phax commented Feb 8, 2018

It is an official Schematron bug acknowledged in Schematron/schematron#29

@SiwMeckelborg
Copy link
Contributor

SiwMeckelborg commented Feb 8, 2018

I think these rules rather should be rewritten, to not have the attribute as the context. Either have the cbc:*[@Attribute], or possibly even better for performance issues, add the effected elements as context, like this:

<rule context="cbc:Amount | cbc:BaseAmount | cbc:PriceAmount >
All currencyID attributes must have the same value as the invoice currency code (BT-5), except for the invoice total VAT amount in accounting currency (BT-111)

@oriol
Copy link
Collaborator

oriol commented Feb 8, 2018 via email

@SiwMeckelborg
Copy link
Contributor

Changed my comment, as the first variant was wrong, any /@Attribute will have the same problem. But as stated, I think the best solution is to name all elements, to prevent it from looping through all elements looking for the attribute.

@oriol
Copy link
Collaborator

oriol commented Feb 8, 2018

The issue is on the stylesheets generating the XSLTs from the Schematron files, so I would not recommend changing the context but asking official Schematron to fix their bug.

@phax
Copy link
Collaborator

phax commented Feb 9, 2018

Which will not happen quickly, because they don't have a test suite and afraid of side effects ;-) Maybe XSpec will be used to build up a test-suite in the future.... I would add this as a "known issue" and link to the fix. The official Schematron stylesheets are also not compatible with Saxon 9.8 because they are XSLT v1 and Saxon 9.8 only supports XSLT v2...

@SiwMeckelborg
Copy link
Contributor

SiwMeckelborg commented Feb 9, 2018

I think it is important to change our validation artefacts to avoid using a special schematron xslt. If implementers use the standard ISO artefacts, several rules will not fire, and invalid instance documents will be sent. My suggestion is to fix this in the TC434 artefacts, to ensure users can use the standard ISO schematron files.

@SiwMeckelborg
Copy link
Contributor

SiwMeckelborg commented Feb 9, 2018 via email

@phax
Copy link
Collaborator

phax commented Feb 9, 2018

There are too many types that can contain an Amount currency (this is A to M):
grafik

@SiwMeckelborg
Copy link
Contributor

SiwMeckelborg commented Feb 9, 2018

We do not use all these in the spec, so the check can be written as follows:

     <rule context="cbc:Amount | cbc:BaseAmount | cbc:PriceAmount | 
    cac:TaxTotal[cac:TaxSubtotal]/cbc:TaxAmount | cbc:TaxableAmount | 
    cbc:LineExtensionAmount | cbc:TaxExclusiveAmount | cbc:TaxInclusiveAmount | 
    cbc:AllowanceTotalAmount | cbc:ChargeTotalAmount | cbc:PrepaidAmount | 
    cbc:PayableRoundingAmount | cbc:PayableAmount">

@oriol
Copy link
Collaborator

oriol commented Feb 9, 2018

There are other rules that contain an attribute in the context. Not only currencyID...

@SiwMeckelborg
Copy link
Contributor

There are 3 rules for EN16931, thats all

@oriol
Copy link
Collaborator

oriol commented Feb 9, 2018

  <rule context="@currencyID" flag="fatal">
  <rule context="cac:AdditionalDocumentReference[cbc:DocumentTypeCode = '130']/cbc:ID/@schemeID | cac:DocumentReference[cbc:DocumentTypeCode = '130']/cbc:ID/@schemeID" flag="fatal">
  <rule context="cac:PartyIdentification/cbc:ID/@schemeID" flag="fatal">
  <rule context="cbc:CompanyID/@schemeID[not(ancestor::cac:PartyTaxScheme)]" flag="fatal">
  <rule context="cac:CommodityClassification/cbc:ItemClassificationCode/@listID" flag="fatal">
  <rule context="cac:StandardItemIdentification/cbc:ID/@schemeID" flag="fatal">
  <rule context="@unitCode" flag="fatal">
  <rule context="@mimeCode" flag="fatal">

@SiwMeckelborg
Copy link
Contributor

SiwMeckelborg commented Feb 9, 2018

My mistake.

I think the rules where it is not only the attribute, can easily be rewritten to use the predicate [@Attribute] , or move the attribute from the context to the test.

@klakegg
Copy link
Contributor Author

klakegg commented Feb 9, 2018

I don't see a change to Schematron being provided anytime soon, and after that you have a lot of tools that need to update something they've never updated before. Also I think the lack of support for addressing attributes in contexts is by design, because fixing the visiting part results in another problem related to the reported path of the assert.

I think it is more important to make sure the rules provided by TC434 works with what we have in Schematron today and not something we potentially may have in the future, and potentially even further into the future have updated tools.

@phax
Copy link
Collaborator

phax commented Feb 9, 2018

This is a bug in the Schematron release and not in the Validation artefacts. So please force them to solve the issue - I don't see a problem at our side.

@SiwMeckelborg
Copy link
Contributor

In my opinion, I don't think the discussion should be if this is a bug in Schematron or not. TC434 publishes validation artefacts that will be used by a large number of communities, and I find it very unfortunate to publish artefacts that need a special version of the Schematron stylesheets. The workaround for TC434 is quite simple, it is a small change in 8 of our rules, and then the ISO version can be used.

@phax
Copy link
Collaborator

phax commented Feb 9, 2018

Please go ahead and make a PR - we will than cross-check it. Does that sound reasonable?

@klakegg
Copy link
Contributor Author

klakegg commented Feb 10, 2018

PR provided - it's time to accept the update.

@oriol oriol closed this as completed in #51 Feb 10, 2018
@phax phax added this to the v1.1 milestone Feb 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants