-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Fix promise future dates #8367
Fix promise future dates #8367
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.
I updated the key for publish_date
, added unit tests, and tested importing with a a current or past date, and a future date, in the local dev environment, and everything seems to be working as expected in terms of future dates being removed, and it not appearing to create any downstream errors (e.g. the Work and Edition page render properly with a deleted date).
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.
Lgtm ! Patch deploying since this is causing issues in the import pipeline for archive.org
Closes #8351
The PR makes a statement that no created record from any source should have a future date.
Most records with future dates will fail to import because of the validation step. Records from sources that are exempt from validation, like bwb promise items, will have certain fields checked/pruned during the normalize stage.
Technical notes
All API imports (I'm not sure about /addbook though, which does have front-end validation) seemingly go through https://github.com/internetarchive/openlibrary/pull/8367/files#diff-2f9a6b1879442fc58af3064df91b70cd3138d6da15003d0d3127ecf05098386cL975-R997. Everything goes through
validate
unless (currently) it's a promise item, in which casevalidate
is skipped. Everything should go throughnormalize
after, including promise items.TODO