Repository navigation
Update Rails to main - #314
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Framework security and upload defaults need deployment-specific validation, and redundant attachment analysis remains unresolved.
Review effort: Balanced
Findings: None
What changed in this PR
Updates Campfire to current Rails main and adapts application code, templates, and tests to Rails 8.2 behavior.
Changes:
- Updates Rails, Propshaft, and related dependencies.
- Fixes compatibility with framework APIs and Herb template compilation.
- Adds rendering coverage and a Herb CI check.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
| File | Description |
|---|---|
| test/test_helper.rb | Removes obsolete Minitest require. |
| test/controllers/rooms_controller_test.rb | Tests platform-specific notification help. |
| test/controllers/accounts_controller_test.rb | Tests restriction checkbox rendering. |
| test/channels/unread_rooms_channel_test.rb | Uses public stream inspection API. |
| Gemfile.lock | Updates framework and dependencies. |
| config/initializers/time_formats.rb | Migrates time-format registration. |
| config/initializers/sentry.rb | Adjusts Action Cable wrapper visibility. |
| config/environments/development.rb | Updates debug middleware placement. |
| app/views/pwa/_system_settings.html.erb | Separates Herb-incompatible ERB tags. |
| app/views/pwa/_install_instructions.html.erb | Separates Herb-incompatible ERB tags. |
| app/views/pwa/_browser_settings.html.erb | Separates Herb-incompatible ERB tags. |
| app/views/layouts/application.html.erb | Removes unmatched closing tags. |
| app/views/accounts/edit.html.erb | Generates checkbox through tag helper. |
| app/models/user/mentionable.rb | Defines mention editor partial. |
| .github/workflows/ci.yml | Adds Herb compilation check. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
thomasklemm
approved these changes
Oct 5, 2026
rubys
force-pushed
the
update-rails-main
branch
from
October 5, 2026 11:45
e4f0b42 to
fcea10f
Compare
rubys
added a commit
to roundhouse-rb/once-campfire
that referenced
this pull request
Oct 5, 2026
Rails main's Redis cache store, which production uses, requires redis-client 0.28.0 or later, so a production boot, and the image build's assets:precompile, aborted on 0.25.2. Found when the same bump ran upstream as basecamp#314. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rails main compiles HTML templates through Herb under the 8.2 framework defaults, which Campfire loads. Herb rejects `case` and its first `when` in a single ERB tag, so the three pwa/ partials failed to compile, and `herb:check` rejects ERB output in attribute names, which the account settings' switch used for `checked`. Give `case` its own tag and build the switch with tag.input; both render the same under Erubi. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 244e772)
Rails main (e3d5c569) moves Campfire's pin forward ten months. Because Campfire loads the 8.2 framework defaults, the ones added since then take effect too, among them header-only forgery protection, Herb as the HTML template engine, strict Accept headers and immediate blob analysis. Changes it needed: - Minitest 6 has no minitest/unit; the test helper no longer requires it. - Rack::Sendfile is no longer in the middleware stack; DebugLocks goes before ActionDispatch::Executor, as the Rails guide now says. - Time::DATE_FORMATS is deprecated; :epoch is registered through ActiveSupport::TimeFormats. - Channel test subscriptions expose stream_names; streams is private. - sentry-rails declares its Action Cable handle_open and handle_close wrappers private, which Rails 8.2 calls from outside the connection, so no /cable connection succeeded. An initializer makes them public until getsentry/sentry-ruby#2972 ships (issue #2975). - Lexxy renders editor content through Rails' editor adapter when Rails has one, which asks a mention for its editor partial. It is the same users/mention partial the mention prompt already inserts. - redis-client moves to 0.30.1: Rails main's Redis cache store, which production uses, requires 0.28.0 or later, and assets:precompile (the Docker build) aborted on 0.25.2. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit b4d3880)
Rails 8.2 compiles HTML templates through Herb in strict mode, so a template that doesn't compile fails to render. herb:check boots the app, which loads ruby-vips, hence libvips. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
rubys
force-pushed
the
update-rails-main
branch
from
October 5, 2026 15:50
fcea10f to
e0c0a1d
Compare
rosa
approved these changes
Oct 6, 2026
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.
Moves Campfire's Rails pin from main as of 2025-12-01 (
1a02651) to current main (e3d5c569). Campfire loads the 8.2 framework defaults, so the defaults added since then take effect too.Changes the bump needed
minitest/unit, sotest/test_helper.rbno longer requires it.Rack::Sendfileis no longer in the middleware stack.ActionDispatch::DebugLocksnow goes beforeActionDispatch::Executor, as the threading guide now says.Time::DATE_FORMATSis deprecated.:epochis registered throughActiveSupport::TimeFormats.register.subscription.streamsis now private, so the unread rooms test usesstream_names.handle_open/handle_closewrappers private, and Rails 8.2 calls them from outside the connection, so no/cableconnection succeeded. That's ActionCable instrumentation breaks all/cableconnections on Rails 8.2 / main (private handle_open called publicly by Server::Socket) getsentry/sentry-ruby#2975; the fix, [on-hold] fix(rails): make Action Cable handle_open/handle_close overrides public for Rails 8.2 getsentry/sentry-ruby#2972, is on hold until an 8.2 beta.config/initializers/sentry.rbmakes the two wrappers public until it ships.users/_user, so editing a message with a mention raised.User::Mentionable#to_editor_content_attachment_partial_pathreturnsusers/mention, the partial the mention prompt already inserts.Mime::EXTENSION_LOOKUP.assets:precompilein the Docker build aborted on 0.25.2. The test environment doesn't use that store, so only the image build showed it.8.2 defaults that switch on
Sec-Fetch-Site, thenOrigin), with no tokens.strict_accept_header, the new default security headers,active_storage.analyze = :immediatelyandenqueue_after_transaction_commit.The suites pass with all of them. The two I'd watch on a real deploy are forgery protection, for plain-HTTP installs and older browsers, and immediate blob analysis.
Herb
Herb's strict mode rejects four of Campfire's templates. This branch includes #302 (two unmatched
</span>in the layout, which would take every page down) and #303 (the PWA partials' one-tagcase/when, and the account settings checkbox). Merging those first shrinks this to two commits. The last commit adds abin/rails herb:checkjob to CI; drop it if you'd rather not.Verification
bin/rails test: 413 runs, 0 failures, 1 skip, no deprecation warningsbin/rails test:system: 22 runs, 0 failuresbin/rails herb:check: all templates compilebin/rubocop: no offensesbin/brakeman: no warningsThe same bump has been running CI on roundhouse-rb/once-campfire's
previewbranch, which also carries the other open PRs.🤖 Generated with Claude Code