Skip to content

Comments

Makes container dynamically generated#1430

Merged
codycooperross merged 7 commits intomasterfrom
container
Nov 25, 2025
Merged

Makes container dynamically generated#1430
codycooperross merged 7 commits intomasterfrom
container

Conversation

@codycooperross
Copy link
Contributor

@codycooperross codycooperross commented Nov 20, 2025

Purpose

Historically, users have had to contribute a description with descriptionType SeriesInformation via XML to update the container property in the Doi model. The container property is then used to create citation and metadata transformations within bolognese. This is a cumbersome and confusing process, and updates were difficult for users of the REST API and Fabrica form, who had to submit XML metadata to see results rather than using familiar interfaces and logical metadata fields, like relatedItem.

This PR makes the accessor for container dynamic, calling a generate_container method within bolognese to build container information from relatedItem and description metadata where available. See:

If that metadata is not available, it defers to the value stored container value in the database if there is any.

Note that the factory for the Doi model has a relatedItem that maps to container in the new arrangement, so this metadata was excluded from some tests to preserve original intent.

closes: https://github.com/datacite/product-backlog/issues/513

Approach

Open Questions and Pre-Merge TODOs

Learning

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

  • New feature (non-breaking change which adds functionality)

  • Breaking change (fix or feature that would cause existing functionality to change)

Reviewer, please remember our guidelines:

  • Be humble in the language and feedback you give, ask don't tell.
  • Consider using positive language as opposed to neutral when offering feedback. This is to avoid the negative bias that can occur with neutral language appearing negative.
  • Offer suggestions on how to improve code e.g. simplification or expanding clarity.
  • Ensure you give reasons for the changes you are proposing.

@codycooperross codycooperross marked this pull request as draft November 20, 2025 15:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR makes the container property dynamically generated from relatedItem and description metadata instead of requiring users to manually set it via XML's SeriesInformation description. The change simplifies metadata management by automatically deriving container information when available, falling back to stored database values when metadata is absent.

Key changes:

  • Adds a dynamic container accessor method that calls generate_container from the bolognese gem
  • Updates Gemfile to use the bolognese "container" branch (temporary until merged)
  • Updates tests to explicitly set related_items: nil where needed to prevent factory defaults from interfering
  • Updates expected citation formats to include dynamically generated container information

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
app/models/doi.rb Adds dynamic container method that generates container from metadata or falls back to database value
Gemfile Temporarily uses bolognese from git branch "container" instead of released version
Gemfile.lock Updates dependencies to use bolognese from git branch and modifies Bundler version
spec/models/doi_spec.rb Adds comprehensive tests for dynamic container generation from various metadata sources
spec/requests/datacite_dois/post_spec.rb Adds assertions for container structure and sets related_items: nil to preserve test intent
spec/requests/datacite_dois/content_negotiation_spec.rb Expands CSL JSON expectations to include container-derived fields
spec/graphql/types/work_type_spec.rb Updates formatted citation expectations to include container information and adds related_items: nil
spec/graphql/types/software_type_spec.rb Updates formatted citation expectation to include container information

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codycooperross codycooperross requested a review from a team November 21, 2025 16:28
Copy link
Contributor

@jrhoads jrhoads left a comment

Choose a reason for hiding this comment

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

Overall the PR looks good. I would wait until the update to bolognese is released and then update this PR with the released version. That way if the bolognese branch is deleted we don't have problems installing this application

@jrhoads jrhoads dismissed their stale review November 24, 2025 14:44

Resolved

@codycooperross codycooperross merged commit 520a960 into master Nov 25, 2025
18 checks passed
@codycooperross codycooperross deleted the container branch November 25, 2025 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-all-tests Run All CI Tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants