Skip to content

Display 'Year Registered' facet on all doi list pages#950

Open
svogt0511 wants to merge 1 commit intomainfrom
pb635-year-registered
Open

Display 'Year Registered' facet on all doi list pages#950
svogt0511 wants to merge 1 commit intomainfrom
pb635-year-registered

Conversation

@svogt0511
Copy link
Contributor

@svogt0511 svogt0511 commented Mar 12, 2026

Purpose

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

preview: https://bracco-6k5h0fyjm-datacite.vercel.app/

This might be on hold due to the accuracy of the facet data as returned by lupo. I created an issue in lupo for that: datacite/lupo#1497.

Approach

Remove the access check on displaying that facet.

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

  • Bug Fixes
    • Updated visibility conditions for the Year registered panel to display whenever registration data is available.

@svogt0511 svogt0511 self-assigned this Mar 12, 2026
@coderabbitai
Copy link

coderabbitai bot commented Mar 12, 2026

📝 Walkthrough

Walkthrough

The permission guard can 'source doi' was removed from the conditional that renders the Year registered panel in a template. The panel now displays whenever registration data exists, independent of user permissions.

Changes

Cohort / File(s) Summary
Template permission logic
app/components/doi-list.hbs
Removed source-DOI permission check from conditional rendering. Year registered panel now displays based solely on presence of registration data, not user permissions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: removing a permission guard to display the 'Year Registered' facet on all DOI list pages, which aligns with the changeset's objective.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch pb635-year-registered
📝 Coding Plan
  • Generate coding plan for human review comments

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.

@svogt0511
Copy link
Contributor Author

svogt0511 commented Mar 17, 2026

This is working as expected. It is already in production at https://doi.test.datacite.org/providers/dc/dois. All I did was make it available on other pages.

@codycooperross had questions about the data accuracy. I looked at that and determined that it is a lupo problem. I filed a lupo issue at: datacite/lupo#1497. This should be considered work separate from this PR.

There was another problem noted by @KellyStathis at https://bracco-6k5h0fyjm-datacite.vercel.app/repositories/datacite.kelly/dois. There is no 'year registered' facet on that page even though there should be. I looked into that and found that it is because there is no data returned from lupo for that facet. It is working as expected on other pages. It could either be the problem mentioned above, or perhaps a data integrity problem in staging. Again, this should be considered work separate from this PR.

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.

1 participant