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

Clarify meaning of 'required' #26

Open
murphyke opened this issue Jan 19, 2016 · 3 comments
Open

Clarify meaning of 'required' #26

murphyke opened this issue Jan 19, 2016 · 3 comments

Comments

@murphyke
Copy link
Member

@aaron0browne I notice that the validator treats 'required' fields as NOT NULL fields. Originally I thought that a 'required' column should be transmitted and populated as possible but not necessarily be NOT NULL - hence the separate existence of NOT NULL constraints in the data model. It doesn't look like the validator pays attention to the NOT NULL constraints. Is this the wave of the future, and if so, should we get rid of the separate NOT NULL constraints in the data-models repo?

@gracebrownecodes
Copy link
Contributor

I think the difference between NOT NULL and "required" is (and always has been) unclear.

I argued from the beginning that all fields should be "required" (in that they must be sent in a data submission, even if they are null), and I believe we are moving in that direction.

However, the validator should definitely be using the NOT NULL constraints instead of the required attribute for this check. Unfortunately, this means passing an entirely different piece of the data models client information into the TableValidator and the FieldValidators below it. I believe the relevant sections of code (working up from the actual validation code) are here, here, and here.

@bruth
Copy link
Contributor

bruth commented Apr 5, 2016

Is this issue still relevant? If so, can it be clarified?

@murphyke
Copy link
Member Author

murphyke commented Apr 5, 2016

The requirement is that the validator needs to check the NOT NULL constraints instead of the murky "required" attribute, and AFAIK the issue is still relevant.

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

No branches or pull requests

3 participants