Closed rorads closed 2 years ago
@rory09 you’re right – it’s a bug. I raised it in #126 – the "context" (the first line) should not be //iati-activity
but should instead be e.g. //iati-activity/other-identifier/owner-org
etc. This was fixed for some rules, but not all.
Hi @rory09 and @andylolz thanks for your bug report and sorry for the lack of response, I was off last week. As far as the ruleset go, there's going to be (most likely) some changes in the repo, as I don't find our current format and chain of events read from Standard -> extract some xpath into a .json file -> reinterpret that file's content programmatically using generic strings to print the rules out
very useful, especially because it requires manual intervention in case a developer wants to re-use them in their system (which I think is the reason why all of this exists in the first place).
We're due to meet with D4D for Validator related talks over the next couple of days and one item in the agenda will be to evaluate the extraction of their ruleset and the replacement of this repository with a new, fresh and re usable set of rules that would require minimal intervention from any dev to implement and use.
We will look into this after those talks have happened so that we have a better overlook over estimated effort to fix vs. how and when the upgrade (if it happens) will proceed.
Thanks, @samuele-mattiuzzo.
One option would be to revert the production branches (version-2.03
, version-2.02
and version-2.01
) back to how they were before the 2019 work, and move the 2019 changes onto feature branches for the timebeing.
A number of bugs were introduced in this 2019 work – I’ve reviewed the rules added in this gist. Regardless of what eventually happens with this repo, it would be preferable for production branches to be in a stable state.
A bunch of these rules have been updated as a result of the gist linked above. I’ve checked some of these updates, and I’ve noticed at least one problem (https://github.com/IATI/IATI-Rulesets/pull/207#issuecomment-491551198).
Unfortunately it’s no longer feasible for me to continue monitoring and reviewing changes to this codebase. But I would echo some of @rory09’s concern. I think it’s great that the ruleset is getting some love and attention. But this should be considered production code, so care should be taken when making changes to the version-x.xx
branches.
I think these have been resolved now, as most have been split out into their own rules, e.g. https://github.com/IATI/IATI-Rulesets/blob/version-2.03/rulesets/standard.json#L938
See the last three paths in https://github.com/IATI/IATI-Rulesets/blob/b88c627bc91e4e6c70a8a0cc6f2541503ec282e4/rulesets/standard.json#L2-L11
The 'atleast_one' element in the rulesets has historically meant that of the comma separated paths included in each case, at least one must be found in any given activity.
Hence these require the existence of other-identifier elements and transaction elements (as the full paths are nested within the 'at, but neither of these are mandatory elements, so I don't see how this squares with the XSD schema?
Should these not exist in their corresponding element sections below in the same file? I.e. there would be the addition of blocks like this:
Or is it actually now an official rule that you need to have an other-identifier/owner-org[@ref or /narrative]? (If so I have some clients who will be failing ruleset validation soon!)
Thoughts appreciated :)