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

Additional fields missing from OCDS Validator results #679

Closed
duncandewhurst opened this issue May 17, 2017 · 14 comments
Closed

Additional fields missing from OCDS Validator results #679

duncandewhurst opened this issue May 17, 2017 · 14 comments
Labels

Comments

@duncandewhurst
Copy link
Contributor

When loading this data (results here) into the OCDS Validator there two additional fields in the tender section which are not reported:

  • procuringEntity_id
  • procuring_entity

Since the data at the above link is likely to change I have copied an extract from the tender section below:

"procuringEntity_id": 11,
"documents": [],
"milestones": [],
"amendment": {},
"system": "egpII",
"procuring_entity": {
	"id": 704,
	"procuringEntity_id": 11,
	"identifier": {
		"id": "11",
		"legalName": "DepartmentofWaterSupply&Sewerage"
@timgdavies
Copy link
Contributor

Is this because of our language pattern validation approach?

I.e. _id could be procuringEntity in Indonesian...

We might need to do some rethinking around this, possibly taking @kindly's approach of having auto-generated language-specific extensions

@edugomez edugomez added the OCDS label May 19, 2017
@edugomez
Copy link
Contributor

@duncandewhurst , @timgdavies is right, this is due to pattern properties in the schema. This does need some rethinking, as e.g. procuringEntity_hello won't be caught as an additional field either.

@robredpath
Copy link
Member

I've added a non-sprint work issue so that this doesn't get lost

@duncandewhurst
Copy link
Contributor Author

Flagging that some of the implementers we met with in Georgia were looking at publishing multi-lingual OCDS data using this approach.

@jpmckinney
Copy link

Submitting the following to http://standard.open-contracting.org/validator/ warns about whatevername but not whatever_name:

{
    "version": "1.1",
    "uri": "http://www.example.com",
    "publishedDate": "2017-09-19T00:00:00Z",
    "publisher": {
        "name": "Open Data Services Co-operative Limited"
    },
    "releases": [
        {
            "ocid": "ocds-213czf-000-00001",
            "id": "ocds-213czf-000-00001-01-planning",
            "date": "2017-09-19T00:00:00Z",
            "initiationType": "tender",
            "tag": [
                "planning"
            ],
            "tender": {
                "id": "ocds-213czf-000-00001-01-planning",
                "whatever_name": "",
                "whatevername": ""
            }
        }
    ]
}

This is because of the test on this line being insufficient:

if LANGUAGE_RE.search(field.split('/')[-1]):

That test checks that the field name is suffixed with what could potentially be a BCP 47 language tag. In this case, _name qualifies. However, the test doesn't check that the root field name (whatever) exists in the schema, but it should.

@jpmckinney
Copy link

Also, procuringEntity_id is not covered by patternProperties. Perhaps a more robust implementation would add "additionalProperties": false to all objects in the schema, collect the errors, and then report the additional fields.

@robredpath
Copy link
Member

@jpmckinney I've added this to the bottom of the Priority board so that we can attend to it in due course. Does that work for you?

@jpmckinney
Copy link

That works. In terms of priority, it can move up as it seems simpler/faster to fix that some others, but I'll leave that to ODS to decide.

@jpmckinney
Copy link

I think this duplicates / is related to #402.

@robredpath
Copy link
Member

Definitely related, but I don't think it's a duplicate. The link is helpful though, thanks!

@timgdavies
Copy link
Contributor

The handling of patternProperties, this is also important for the new ocds_budgets_and_spend_extension which allows arbitrary key-value pairs within two objects (classifications and measures).

At present, validating the examples reports all the fields in these objects as additional fields, whereas the pattern properties regEx in the schema means they should not be reported as a problem.

I think with this, plus #402, this should push this up the priority list - ideally with us finding a good way to respect use of patternProperties in the schema.

@jpmckinney
Copy link

This could be a candidate for the Apr-May 2019 sprint.

@robredpath
Copy link
Member

@robredpath
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants