[DREAM-784] Pilot LiveComponent on documents page header (experiment, do not merge) - #24442
Draft
myabc wants to merge 12 commits into
Draft
[DREAM-784] Pilot LiveComponent on documents page header (experiment, do not merge)#24442myabc wants to merge 12 commits into
myabc wants to merge 12 commits into
Conversation
Adds the live_component gem and @camertron/live-component npm package to pilot client-initiated component re-rendering on the documents page header. https://community.openproject.org/wp/DREAM-784 Registers an early action_controller load hook giving helper-less controller bases (ActionController::API) a no-op helper, because the gem's own load hook calls helper on every controller base and crashes boot (library gap in 0.4.0).
Converts the component to a kwargs-only initializer because LiveComponent round-trips only keyword parameters as client state; positional ApplicationComponent models cannot survive a re-render.
Wraps the page header in a live-component custom element and re-targets the title turbo streams at the element's DOM id, since the element renders outside the OpTurbo wrapper and nested updates would stack stale elements.
Routes client-initiated re-renders through a regular Rails controller instead of the library middleware so User.current is set and component permission checks keep working. Restricts the root component to the pilot allowlist.
Moves the show/edit flip of the documents page header into a sidecar LiveController backed by the authenticated render endpoint, and drops the edit_title and cancel_title_edit actions whose only job was re-rendering the component. Saving still posts to update_title and replaces via turbo stream.
Replaces the update_title controller action with a reflex method on the page header component so the whole edit cycle runs through LiveComponent. Validation errors render inline as a banner instead of a flash.
Rejects malformed or oversized request bodies before JSON parsing/gzip decoding runs, closing a gzip-bomb surface, and fixes the status symbol used for that rejection (:content_too_large is not a valid Rack status; :payload_too_large is). Adds a render? guard to PageHeaderComponent, plus a RenderGuard module prepended above the gem's own prepended render_in, so an unauthorized request short-circuits before LiveComponent::Base ever serializes props. A bare render? alone would not have been enough: the gem always serializes full model attributes into the data-state HTML attribute before consulting render?, so it would still leak a document's title/description (and a freshly re-signed GID) to a signed-in user who cannot view it. This matters because the render endpoint performs no authorization of its own and the gem's ModelSerializer lets a client-controlled signed flag downgrade a reload to an unsigned GID lookup.
Attaches a no-op .catch() to every render() call in the page header controller: the library's task queue logs failures but still re-rejects, so the prior fire-and-forget calls left every failed render as an unhandled promise rejection. Corrects the transport comment that described the same behavior backwards. Restores the after_update URL-param cleanup dropped from the original plan, and unifies the remaining symbol-keyed I18n.t call to match its sibling. Documents why the edit form's url: "#" is left alone.
This was referenced Jul 24, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces an experimental LiveComponent (v0.4.0) integration to pilot client-owned state + server re-render/reflexes for the Documents page header title editing flow, replacing the existing Turbo Stream/controller-action pattern with a dedicated authenticated render endpoint and a component reflex.
Changes:
- Added an authenticated
/live_components/renderendpoint (LiveComponentsController) with basic allowlisting and payload hardening for the pilot component. - Reworked
Documents::ShowEditView::PageHeaderComponentto useLiveComponent::Baseand a reflex (update_title) for server-authoritative updates, removing the prior Turbo Stream routes/actions. - Added frontend LiveComponent wiring (custom element + transport + Stimulus controller) to support edit/cancel client state flips and save via reflex.
Reviewed changes
Copilot reviewed 17 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/requests/live_components_spec.rb | Request spec coverage for the LiveComponent render endpoint (authz, reflex, allowlist). |
| modules/documents/spec/features/documents/project/show_edit_document_spec.rb | Adapts feature spec to the LiveComponent-driven edit/save/cancel flow. |
| modules/documents/lib/open_project/documents/engine.rb | Removes deleted controller actions from manage_documents permission mapping. |
| modules/documents/config/routes.rb | Removes member routes for edit_title/update_title/cancel_title_edit. |
| modules/documents/app/controllers/documents_controller.rb | Deletes the title-edit Turbo Stream actions and helper method. |
| modules/documents/app/components/documents/show_edit_view/page_header_component.rb | Converts page header to LiveComponent with render-guard + reflex update. |
| modules/documents/app/components/documents/show_edit_view/page_header_component.html.erb | Switches title editing UI from Primer editable slot to LiveComponent-driven form/actions. |
| modules/documents/app/components/documents/show_edit_view/collaboration_disabled_notice_component.html.erb | Updates PageHeaderComponent invocation (kwargs + LiveComponent attributes). |
| modules/documents/app/components/documents/show_edit_view/block_note_editor_component.html.erb | Updates PageHeaderComponent invocation (kwargs + LiveComponent attributes). |
| Gemfile | Adds live_component gem for the pilot. |
| Gemfile.lock | Locks live_component and transitive dependencies (use_context, weak_key_map). |
| frontend/src/stimulus/setup.ts | Registers LiveComponent and the Documents page-header LiveController/element. |
| frontend/src/stimulus/live-component-transport.ts | Adds a CSRF-aware transport implementation for LiveComponent render requests. |
| frontend/src/stimulus/controllers/dynamic/documents/page-header-live.controller.ts | Implements edit/cancel client state flips and save via reflex call. |
| frontend/package.json | Adds @camertron/live-component dependency. |
| frontend/package-lock.json | Locks the new npm dependency. |
| config/routes.rb | Adds the /live_components/render route. |
| config/application.rb | Adds a workaround for the LiveComponent engine on_load(:action_controller) crash on ActionController::API. |
| app/controllers/live_components_controller.rb | Implements the authenticated LiveComponent render endpoint with allowlisting/size checks. |
Files not reviewed (1)
- frontend/package-lock.json: Generated file
Comment on lines
+77
to
+82
| def initialize(document:, project:, state: :show) | ||
| super() | ||
| @document = document | ||
| @project = project | ||
| @state = state.to_sym.presence_in(STATES) || :show | ||
| end |
Comment on lines
+146
to
+157
| def title_edit_form | ||
| # url: "#" is a harmless no-JS fallback (a GET to the current page) -- | ||
| # submission is normally intercepted by the Stimulus #save action | ||
| # before it ever hits the network. There is no real route to point | ||
| # this at (saving goes through the reflex, not a controller action), | ||
| # so don't "fix" this into one. | ||
| form_with(model: document, url: "#", | ||
| class: "d-flex", | ||
| data: { action: "#{self.class.__lc_controller}#save" }) do |f| | ||
| safe_join([title_edit_input(f), title_edit_save_button, title_edit_cancel_button]) | ||
| end | ||
| end |
Comment on lines
+21
to
+25
| const json = JSON.stringify(request); | ||
| const bytes = new TextEncoder().encode(json); | ||
| let binary = ''; | ||
| bytes.forEach((b) => { binary += String.fromCharCode(b); }); | ||
| const payload = btoa(binary); |
myabc
added
DO NOT MERGE
javascript
`render?` guarded only `document` while `project` arrived as its own client-controlled prop, so a mismatched pair leaked the foreign project and evaluated `manage_documents` against a project of the caller's choosing. Reflexes run before RenderGuard, so `update_title` authorizes its own read.
The endpoint accepted any prop name, any `__lc_attributes`, any reflex and any reflex argument, and its size cap sat above an unbounded `Zlib.gunzip`. Login is now required outright, since `check_if_login_required` is a no-op on public instances.
Every `render()` swallowed its rejection, so a 500, an expired session and a permission-denied empty body all presented as the button doing nothing.
Restructures the file so contexts override `props`/`reflexes` rather than rebuilding the body, then covers each guard -- including the `_lc_symkeys` marker a real client round-trips, which no `serialize_props`-built payload exercises.
|
Warning Flaky specs
🤖 Ask Copilot to investigateCopy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer. |
camertron
reviewed
Jul 27, 2026
| render Documents::ShowEditView::PageHeaderComponent.new(document, project:, state:) | ||
| render Documents::ShowEditView::PageHeaderComponent.new( | ||
| document:, state:, | ||
| __lc_attributes: { "id" => Documents::ShowEditView::PageHeaderComponent::DOM_ID } |
Collaborator
There was a problem hiding this comment.
Hmm, perhaps LiveComponent::Base should allow providing a custom ID 🤔
Collaborator
|
Very cool stuff @myabc! I'm really interested to see where y'all go with this 😄 I did want to mention that reflexes were really meant to provide a way to keep complex state mutations in Ruby. Rather than update state in JavaScript, you write the state mutation code in Ruby and expose a public instance method on the component that can be invoked from the client-side. What you've done here is essentially turned the component into a mini controller 😅 I haven't decided yet if I like that pattern or not, but I do think it comes with some rather serious pitfalls you've already partially identified: controller performance overhead, CSRF, looking up the current user, etc etc. LiveComponent is designed to work with turbo streams for form submissions, i.e. when you need to update the database. You still need a controller, a route, etc, but LC makes it easier to respond with the updated HTML because it hangs onto the component's previous state, so the update template only includes the changes the view actually needs: <%# namespace/update.html.erb %>
<%= live.rerender do |component| %>
<% component.some_method %>
<% end %>This PR is making me realize however that, especially for small, UI-driven changes, turning the component into a miniature controller might not be a bad thing. I did some thinking yesterday on my daily walk about how we might keep the component UI-focused and put the controller-shaped logic elsewhere, but I just ended up coming back to using regular 'ol Rails controllers. Would be curious to hear your thoughts on this 😄 |
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.
Ticket
https://community.openproject.org/wp/DREAM-784
What are you trying to accomplish?
Pilot LiveComponent 0.4.0 on the documents page-header title toggle, replacing the current
state:+turbo_streamhouse pattern (three controller actions whose only job is re-rendering the component) with client-owned state and a server-authoritative reflex.The title toggle now works as:
this.render({ state })) through an authenticated render endpoint, morphed in at the component boundary.component.call("update_title", { title })), server-authoritative.Net: three controller actions + their routes + OpTurbo wiring deleted, replaced by one reflex method plus a small sidecar
LiveController. The full edit → blank-save (validation banner) → save → cancel cycle passes in real Chrome via Cuprite.Screenshots
To add before/after of the title toggle if this goes further.
What approach did you choose and why?
Eight commits, built and reviewed task-by-task:
live_component0.4.0 (gem + npm), with a boot-crash workaround inconfig/application.rb(the engine'son_load(:action_controller)hook crashes onActionController::API).include LiveComponent::Base+ re-target the title turbo streams (the custom element renders outside the OpTurbo wrapper).LiveComponentsController < ApplicationController, NOT the library's Rack middleware, soUser.current/session/CSRF apply. Root-component allowlist.update_title/edit_title/cancel_title_edit.Why a custom endpoint instead of the shipped middleware: the library's Rack middleware bypasses the controller stack, so
User.currentis never set and permission-dependent UI renders as anonymous. Routing through a real controller costs the "no Rails overhead" speed claim but is the only correct option here.Verdict: the programming model is genuinely pleasant and collapsed the boilerplate as hoped, but v0.4.0 is not adoption-ready — nine library gaps, eight filed upstream (
livecomponent/livecomponent#3–#10), one held for private security disclosure. Full write-up lives in the team KB. Headline blockers: the engine boot crash, a globalturbo:submit-endhandler that throws on every non-LiveComponent Turbo form once the library loads, and a serializer trust issue requiring a per-component read guard.Merge checklist
Not applicable — draft for visibility, not for merge.