Skip to content

Add fast-fails for new mandatory OAPEN fields to ONIX output generation#752

Open
rhigman wants to merge 2 commits into
developfrom
feature/oapen_onix_restrictions
Open

Add fast-fails for new mandatory OAPEN fields to ONIX output generation#752
rhigman wants to merge 2 commits into
developfrom
feature/oapen_onix_restrictions

Conversation

@rhigman
Copy link
Copy Markdown
Member

@rhigman rhigman commented May 19, 2026

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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()) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@rhigman rhigman requested a review from ja573 May 20, 2026 08:51
@rhigman rhigman linked an issue May 20, 2026 that may be closed by this pull request
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

Successfully merging this pull request may close these issues.

Update OAPEN ONIX 3.0 to match new requirements

1 participant