refactor(architecture): unify rendering, token security, and frontend service modules - #1053
Merged
Merged
Conversation
This unifies XML and JSON Feed formatting, error representation, and empty feed warning outputs under a single unified Feeds::Renderer module, removing redundant RssRenderer, JsonRenderer, XmlBuilder, and JsonFeedBuilder helpers.
Consolidates path extensions and HTTP Accept header parsing and negotiation rules into a single FeedResponseFormat module, removing the separate FeedAcceptHeader module.
Consolidates the FeedToken value object definition alongside its nested Codec and Signer namespaces inside feed_token.rb, removing the subfolder structure and keeping token verification, compression, and signature validation cohesive.
Merges feed creation api, error normalization, utility functions, and preview loading/retries from four separate files under frontend/src/feeds/ into a unified feedsService.ts file.
Moves state management, route coordination, and submit orchestration out of App.tsx into the useAppPresenter custom hook, leaving App.tsx as a pure stateless view layout.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR performs a broad backend/frontend refactor to consolidate feed rendering, Accept-header negotiation, and token encoding/signing on the Ruby side, while merging multiple frontend feed modules into a single feedsService.ts and extracting App.tsx coordination into useAppPresenter.
Changes:
- Backend: consolidate RSS + JSON Feed rendering and error/empty-feed output into
Html2rss::Web::Feeds::Renderer, and fold Accept-header parsing intoFeedResponseFormat. - Backend: move feed-token Codec/Signer under
Html2rss::Web::FeedToken(single file). - Frontend: merge feed creation + preview hydration + shared helpers into
feedsService.ts, and extractuseAppPresenterfromApp.tsx.
Commands:
- Not run (review only).
Results:
- One test-coverage regression noted (RSS stylesheet PI expectation dropped during spec consolidation).
Next steps:
- Add/restore a regression assertion for the RSS stylesheet processing-instruction in the new
Feeds::Rendererspec.
One-line Summary:
Large consolidation refactor across backend + frontend modules with one identified testing gap to address.
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| spec/html2rss/web/xml_builder_spec.rb | Removed in favor of consolidated renderer specs. |
| spec/html2rss/web/json_feed_builder_spec.rb | Removed in favor of consolidated renderer specs. |
| spec/html2rss/web/feeds/rss_renderer_spec.rb | Removed after renderer consolidation. |
| spec/html2rss/web/feeds/json_renderer_spec.rb | Removed after renderer consolidation. |
| spec/html2rss/web/feeds/responder_spec.rb | Updated to stub Feeds::Renderer with explicit format:. |
| spec/html2rss/web/feeds/renderer_spec.rb | New spec covering RSS/JSON Feed rendering paths for ok/empty/error. |
| spec/html2rss/web/feed_response_format_spec.rb | New spec for path/Accept negotiation helpers. |
| spec/html2rss/web/feed_notice_text_spec.rb | Removed after notice copy moved into renderer. |
| spec/html2rss/web/feed_accept_header_spec.rb | Removed after Accept parsing merged into FeedResponseFormat. |
| spec/html2rss/web/error_responder_spec.rb | Updated to stub Feeds::Renderer.call_error. |
| spec/html2rss/web/app_spec.rb | Updated render stubs to Feeds::Renderer APIs. |
| spec/html2rss/web/app_integration_spec.rb | Updated render stubs to Feeds::Renderer APIs. |
| spec/html2rss/web/api/v1_spec.rb | Updated render stubs to Feeds::Renderer APIs. |
| frontend/src/hooks/useFeedConversion.ts | Updated imports to use consolidated feedsService.ts. |
| frontend/src/hooks/useAppPresenter.ts | New hook extracting route/state/effects from App.tsx. |
| frontend/src/components/App.tsx | Simplified by delegating coordination to useAppPresenter. |
| frontend/src/feeds/shared.ts | Deleted; utilities moved into feedsService.ts. |
| frontend/src/feeds/previewHydration.ts | Deleted; preview hydration moved into feedsService.ts. |
| frontend/src/feeds/feedsApi.ts | Deleted; request helpers moved into feedsService.ts. |
| frontend/src/feeds/feedCreationError.ts | Deleted; error normalization moved into feedsService.ts. |
| frontend/src/feeds/feedsService.ts | New consolidated module for feed API + errors + preview hydration + helpers. |
| frontend/src/tests/previewHydration.test.ts | Updated imports to feedsService.ts. |
| frontend/src/tests/feedCreationError.test.ts | Updated imports to feedsService.ts. |
| app/web/security/feed_token/signer.rb | Deleted; moved into app/web/security/feed_token.rb. |
| app/web/security/feed_token/codec.rb | Deleted; moved into app/web/security/feed_token.rb. |
| app/web/security/feed_token.rb | Now contains FeedToken, FeedToken::Codec, and FeedToken::Signer. |
| app/web/rendering/xml_builder.rb | Deleted; RSS building moved into Feeds::Renderer. |
| app/web/rendering/json_feed_builder.rb | Deleted; JSON Feed building moved into Feeds::Renderer. |
| app/web/rendering/feed_notice_text.rb | Deleted; notice copy moved into Feeds::Renderer. |
| app/web/rendering/feed_accept_header.rb | Deleted; parsing moved into FeedResponseFormat. |
| app/web/rendering/feed_response_format.rb | Now includes Accept-header parsing logic (MediaRange parsing/scoring). |
| app/web/feeds/rss_renderer.rb | Deleted; behavior merged into Feeds::Renderer. |
| app/web/feeds/json_renderer.rb | Deleted; behavior merged into Feeds::Renderer. |
| app/web/feeds/responder.rb | Now delegates rendering to Feeds::Renderer.call(result, format:). |
| app/web/feeds/renderer.rb | New unified renderer handling ok/empty/error for RSS + JSON Feed. |
| app/web/errors/error_responder.rb | Uses Feeds::Renderer.call_error for feed-format errors. |
Adds an explicit regression test to renderer_spec.rb to ensure configured stylesheets are correctly embedded as processing-instructions in the generated RSS xml.
gildesmarais
marked this pull request as ready for review
August 2, 2026 10:16
gildesmarais
enabled auto-merge (squash)
August 2, 2026 10:17
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR unifies various modules across the backend and frontend to reduce cognitive load, simplify dependencies, and net-delete 16 obsolete files (over 1,300 lines of code).
Changes
Feeds::Renderer.FeedAcceptHeaderintoFeedResponseFormat.CodecandSignernamespaces intoFeedToken.feedsService.ts.App.tsxinto a custom hookuseAppPresenter.ts.