Add fast-fails for new mandatory OAPEN fields to ONIX output generation#752
Add fast-fails for new mandatory OAPEN fields to ONIX output generation#752rhigman wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ce28fbcc2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "Missing Landing Page".to_string(), | ||
| ) | ||
| })?; | ||
| if !self.publications.iter().any(|p| p.isbn.is_some()) { |
There was a problem hiding this comment.
Require an ISBN usable as the main product identifier
When a work has a PDF URL but its only ISBN belongs to a non-PDF/non-paperback publication, this new fast-fail passes because it accepts any publication ISBN. get_publications_data only assigns main_isbn from PDF or, as a fallback, paperback publications, so the generated PDF product still gets an empty ISBN-13 <IDValue> instead of failing the mandatory ISBN check. Please restrict this check to the publication types that can populate main_isbn (or check main_isbn after computing it).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This wasn't specified, but the guidelines we're following (currently still draft, and shared directly from OAPEN) do indicate that "primary ISBN" (ProductIdentifier in ONIX) is mandatory and other ISBNs (under RelatedProduct in ONIX) are optional. The comparable SWORD route doesn't make a distinction between types of ISBN when submitting.
Fixes #750. Parallels thoth-pub/thoth-dissemination@3a5e37e.