Skip to content

Conversation

@aaxelb
Copy link
Contributor

@aaxelb aaxelb commented Oct 9, 2025

(note: includes #885 because conflicts)

  • add feed renderers:
    • trove.render.cardsearch_rss
    • trove.render.cardsearch_atom
  • add feed paths/views:
    • /trove/index-card-search/rss.xml
    • /trove/index-card-search/atom.xml
  • add links from cardsearch responses to feeds (with appropriate params)
  • miscellaneous improvements:
    • better renderer test coverage
    • consolidate more shared logic into trove.util
    • more accurate type annotations
    • link to login admin with osf

- replace `trove.render._rendering` module with `trove.render.rendering`
  package (separate file for each rendering class)
- make `ProtoRendering` an actual `typing.Protocol` and narrow types to
  only unicode `str`
- add `trove.render.rendering.html_wrapped.HtmlWrappedRendering` that
  puts minimal `<!DOCTYPE html><pre>...</pre>` around an inner rendering
- use `HtmlWrappedRendering` to wrap responses that aren't html or json,
  when `Accept` header allows html and query params omit `withFileName`
- use mediatype constants more consistently (leaving off charset except
  for content-type header)
- stream pages as lists, not dicts with colliding keys
- tidy types
- move `iter_unique` to `trove.util.iter` and add tests
- add feed renderers:
    - trove.render.cardsearch_rss
    - trove.render.cardsearch_atom
- add feed paths:
    - /trove/index-card-search/rss.xml
    - /trove/index-card-search/atom.xml
- add links from cardsearch responses to feeds (with appropriate params)
- miscellaneous improvements:
    - better renderer test coverage
    - consolidate more shared logic into trove.util
    - more accurate type annotations
@coveralls
Copy link

coveralls commented Oct 9, 2025

Coverage Status

coverage: 84.29% (+2.6%) from 81.693%
when pulling 6a3306f on aaxelb:feat/9046-shtrove-rss
into 8f08e7f on CenterForOpenScience:develop.

@aaxelb aaxelb changed the title Feat/9046 shtrove rss [ENG-9046] shtrove cardsearch feeds Oct 9, 2025
@aaxelb aaxelb marked this pull request as ready for review October 9, 2025 14:03
@aaxelb aaxelb requested a review from felliott October 9, 2025 14:35
Copy link
Member

@felliott felliott left a comment

Choose a reason for hiding this comment

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

Bellissima! Two comments, neither are blockers. Merge as thou wilt.

Cheers,
@felliott

and its field rendered_content to entire_content
- SimpleTrovesearchRenderer => TrovesearchCardOnlyRenderer
- remove "simple" renderer convenience methods; it's not that hard to
  build an EntireRendering (previously named "SimpleRendering")
@aaxelb aaxelb merged commit 71ebed7 into CenterForOpenScience:develop Oct 17, 2025
3 checks passed
@aaxelb aaxelb deleted the feat/9046-shtrove-rss branch October 17, 2025 20:35
@aaxelb aaxelb mentioned this pull request Oct 30, 2025
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