-
Notifications
You must be signed in to change notification settings - Fork 7
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
Remove sources
from schema and consolidate its URL field into caseReference
#372
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicely done!
|
||
Returns: | ||
None: When the input is empty. | ||
Dict[str, str]: When the input is nonempty. The dictionary is in the | ||
format: | ||
{ | ||
'url': str, | ||
'other': str | ||
'sourceUrl': str, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Granted there's no real sourceId
for this data, but it's a little odd that we're inserting data that disagrees with our Mongoose validation rules. Is it worth filling the ID with a hash of the URL (or perhaps some portion of it)?
Mostly related: should we add a required
stanza(s) in our json schema?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Granted there's no real
sourceId
for this data, but it's a little odd that we're inserting data that disagrees with our Mongoose validation rules. Is it worth filling the ID with a hash of the URL (or perhaps some portion of it)?
I guess I was hoping we'd be able to backfill some of these ids, after we've added some data to the sources collection -- does that sound reasonable, and if reasonable, worth it?
Mostly related: should we add a
required
stanza(s) in our json schema?
unfortunately, there are a few existing cases that do not have a source URL, so they would fail to validate & insert into the db :( I can look into getting those fixed upstream though and then we can make it required
Sorry it's so huge :\ I tried to break commits up somewhat logically
Note: I made
caseReference
required, as well ascaseReference.sourceUrl