Skip to content

Remove validity check for csv writer and make csv writing more defensive#222

Merged
codycooperross merged 2 commits intomasterfrom
csv-bug
Mar 6, 2026
Merged

Remove validity check for csv writer and make csv writing more defensive#222
codycooperross merged 2 commits intomasterfrom
csv-bug

Conversation

@codycooperross
Copy link
Contributor

@codycooperross codycooperross commented Mar 6, 2026

Purpose

When csv format is requested in lupo, the operation will silently return nil unless the ActiveRecord Doi object is valid. The Doi object may be invalid for reasons beyond metadata, including a mismatch between the Doi object's url and the allowed domains by its parent client. Consequently, lupo-generated CSV output may contain missing lines for "invalid" records even though these should be represented. This PR removes the validity check, which seems unnecessary, and adds several defensive measures to avoid errors during csv generation.

Part of: datacite/datacite#2502

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.

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Enhanced CSV export robustness with improved data handling to prevent errors when processing certain data formats.
  • Chores

    • Version bump to 2.5.2.

@coderabbitai
Copy link

coderabbitai bot commented Mar 6, 2026

📝 Walkthrough

Walkthrough

This PR increments the version number from 2.5.1 to 2.5.2 and refactors the CSV writer method to remove an early validation return, replacing it with conditional guards for accessing nested fields using .to_h conversions.

Changes

Cohort / File(s) Summary
Version Bump
lib/bolognese/version.rb
Updated VERSION constant from "2.5.1" to "2.5.2"
CSV Writer Refactoring
lib/bolognese/writers/csv_writer.rb
Removed early return validation guard and introduced defensive .to_h conversions for nested field access on resource_type_general, resource_type, and publisher fields

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: removing a validity check and adding defensive measures to CSV writing.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch csv-bug

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 RuboCop (1.85.0)
lib/bolognese/version.rb

rubocop-rails extension supports plugin, specify plugins: rubocop-rails instead of require: rubocop-rails in /.rubocop.yml.
For more information, see https://docs.rubocop.org/rubocop/plugin_migration_guide.html.
rubocop-performance extension supports plugin, specify plugins: rubocop-performance instead of require: rubocop-performance in /.rubocop.yml.
For more information, see https://docs.rubocop.org/rubocop/plugin_migration_guide.html.
Error: The Metrics/LineLength cop has been moved to Layout/LineLength.
(obsolete configuration found in .rubocop.yml, please update it)

lib/bolognese/writers/csv_writer.rb

rubocop-rails extension supports plugin, specify plugins: rubocop-rails instead of require: rubocop-rails in /.rubocop.yml.
For more information, see https://docs.rubocop.org/rubocop/plugin_migration_guide.html.
rubocop-performance extension supports plugin, specify plugins: rubocop-performance instead of require: rubocop-performance in /.rubocop.yml.
For more information, see https://docs.rubocop.org/rubocop/plugin_migration_guide.html.
Error: The Metrics/LineLength cop has been moved to Layout/LineLength.
(obsolete configuration found in .rubocop.yml, please update it)


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codycooperross codycooperross requested a review from a team March 6, 2026 17:04
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
lib/bolognese/writers/csv_writer.rb (1)

6-21: ⚠️ Potential issue | 🟡 Minor

Consider adding a validity check for consistency with other export methods.

The csv method lacks the validity check present in bibtex (which returns nil unless valid?) and codemeta (which returns nil unless valid? or show_errors). This creates an inconsistent API where csv always returns a CSV string regardless of validity, while similar export methods protect against exporting invalid metadata.

If this inconsistency is intentional, document why CSV exports should differ from other formats. Otherwise, align the implementation with other writers by adding an early validity check.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/bolognese/writers/csv_writer.rb` around lines 6 - 21, The csv method
should enforce the same validity guard as the other exporters: add an early
check in csv (the method in lib/bolognese/writers/csv_writer.rb) to return nil
when the metadata is invalid, mirroring bibtex (i.e. return nil unless valid?)
or, if you want parity with codemeta behavior, use the same condition (return
nil unless valid? || show_errors) so CSV exports are consistent with the
bibtex/codemeta writers; place this check at the top of the csv method before
building the bib array.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@lib/bolognese/writers/csv_writer.rb`:
- Around line 6-21: The csv method should enforce the same validity guard as the
other exporters: add an early check in csv (the method in
lib/bolognese/writers/csv_writer.rb) to return nil when the metadata is invalid,
mirroring bibtex (i.e. return nil unless valid?) or, if you want parity with
codemeta behavior, use the same condition (return nil unless valid? ||
show_errors) so CSV exports are consistent with the bibtex/codemeta writers;
place this check at the top of the csv method before building the bib array.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7aa40e74-ed70-48a8-af39-6efa12b49d21

📥 Commits

Reviewing files that changed from the base of the PR and between 7dc29da and 00b69d8.

⛔ Files ignored due to path filters (1)
  • Gemfile.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • lib/bolognese/version.rb
  • lib/bolognese/writers/csv_writer.rb

@codycooperross codycooperross merged commit d719faf into master Mar 6, 2026
5 checks passed
@codycooperross codycooperross deleted the csv-bug branch March 6, 2026 20:07
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.

3 participants