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

Validation does not occur in Step 1 for newly added traits which do not specify unit #37

Open
carolyncaron opened this issue Mar 15, 2018 · 1 comment
Assignees

Comments

@carolyncaron
Copy link
Member

When reviewing Pull request #32, I realized that allowing column headers to omit the unit in the format (unit) makes it difficult to validate that the unit actually makes sense. For example, cm = integer, date actually reflects a date, and so on. Sometimes this has resulted in an error during step 3 as in #32, but this is not always the case. Regardless, we have discussed the issue of whether or not we want to allow this kind of flexibility in the first place. Concerns that arose were:

  • Implementing validation at the second stage, or redesigning the entire flow of the module will take up too much time
  • "Yelling" at users who are uploading bonus traits does not feel right, and may discourage further use of the module or encourage abandonment at the second stage
  • What happens if the same "bonus" trait is uploaded twice? It will then be picked up by validation in step 1. Now what happens if it was fine the first time but not the second time? Not only can this discourage the user, but we shudder at the thought of the data being fixed only after a first attempt was successfully made, resulting in heterogenous values for this trait.

We propose the following solution to address all 3 concerns. This will occur during step 3:

Check if the trait is a newly-defined trait. If yes:
   Validate the unit. If validation passes:
      Save values
   Else if validation fails:
      Ignore values, but send an email to the administrator detailing the problem trait
Else if not a new trait:
   Save values

Thus, this issue can be solved by confirming the unit manually by the admin (or asking a local expert) or even contacting the original phenotyper for clarification (as the notification will be immediate), but the remainder of the data still gets saved.

@reynoldtan
Copy link
Member

Code ready for review.
Pull Req: #39

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