Conversation
no message
…ust environment configurations
- Upgraded esbuild and its architecture-specific packages from version 0.24.2 to 0.27.3. - Updated Font Awesome from version 5.15.4 to 7.2.0. - Changed jQuery version constraints to allow for version 4.0.0 and above. - Updated marked from version 3.0.8 to 17.0.3. - Upgraded mdb-ui-kit from version 3.11.0 to 9.3.0. - Updated spark-md5 from version 3.0.0 to 3.0.1. - Adjusted dependencies for @rails/actioncable and @rails/activestorage to version 8.1.200.
…t-create.shにclaude.aiのインストールを追加。schema.rbのActiveRecordバージョンを8.1に更新。
There was a problem hiding this comment.
Pull request overview
This PR implements a major Rails framework upgrade from Rails 7.0.2 to Rails 8.1.2 for the Radvent (Qiita-style advent calendar) application. The upgrade includes a complete frontend tooling migration from Webpacker 5 to esbuild, replacement of Turbolinks with Hotwire Turbo, and updates to Ruby (3.2+) and all major dependencies.
Changes:
- Rails framework upgraded from 7.0 to 8.1 with all configuration updates
- Frontend build system migrated from Webpacker to custom esbuild configuration
- Turbolinks replaced with Turbo (Hotwire) including event listener updates
- All models updated from
ActiveRecord::BasetoApplicationRecord - Test framework updated for Rails 8 patterns (
xhr: true→as: :turbo_stream) - AJAX forms converted to Turbo Streams with corresponding view templates
- Major dependency updates: jquery 3→4, marked 3→17, mdb-ui-kit 3→9
- Docker, CI/CD, and development environment configurations updated for Rails 8
- Version bumped to 3.0.0beta reflecting major upgrade
Reviewed changes
Copilot reviewed 97 out of 142 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| specs/rails_upgrade_specification.md | Comprehensive upgrade plan documenting the 5-phase migration strategy |
| Gemfile / Gemfile.lock | Rails 8.1, Ruby 3.2+, jsbundling-rails, turbo-rails, updated dependencies |
| package.json | Removed Webpacker, added esbuild, major version jumps for marked/jquery/mdb |
| esbuild.config.mjs | Custom esbuild configuration replacing Webpacker |
| config/application.rb | load_defaults 8.0, factory_bot reference fix, autoload_lib configuration |
| config/environments/* | Rails 8 environment configs (enable_reloading, report_deprecations) |
| app/models/* | All models migrated to ApplicationRecord inheritance |
| app/javascript/* | Turbolinks→Turbo event migration, reorganized from packs/ structure |
| app/controllers/items_controller.rb | Preview refactored to dedicated action, Turbo Stream responses |
| app/controllers/likes_controller.rb | Added explicit Turbo Stream response handlers |
| app/views/items/_form.html.haml | Updated preview button with turbo:false and dynamic formaction |
| app/views/items/_like.html.haml | Removed remote: true, button_to for Turbo compatibility |
| app/views/likes/*.turbo_stream.haml | New Turbo Stream templates replacing .js.erb |
| spec/controllers/* | Test updates for as: :turbo_stream pattern |
| spec/rails_helper.rb | fixture_path→fixture_paths, SimpleCov formatter syntax |
| db/schema.rb | ActiveRecord::Schema[8.1], alphabetized column ordering |
| config/credentials.yml.enc | Regenerated encrypted credentials |
| lib/radvent/version.rb | Module renamed VERSION→Version, bumped to 3.0.0beta |
| .github/workflows/* | CI updated for Ruby 3.2/3.3, MySQL 8, new PR policies |
| Dockerfile | Updated to Ruby 3.2, Node LTS installation method |
| .gitignore | Updated for asset builds location |
Comments suppressed due to low confidence (2)
app/javascript/items.js:3
- The mdb-ui-kit import path has changed from
mdb-ui-kit/js/mdb.min.jstomdb-ui-kit/js/mdb.es.min.js. Verify that version 9.3.0 of mdb-ui-kit actually exports from this path, as this is a significant version jump from 3.10.2 and the package structure may have changed.
app/javascript/items.js:2 - The import statement uses the old marked default export pattern
import { marked } from 'marked', but marked v17.x may have changed its export structure. Verify this import works with the new version - you may need to useimport {marked} from 'marked'or check the v17 migration guide for the correct import syntax.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ### 6.1 Webpacker → jsbundling-rails への移行(最大の作業) | ||
|
|
||
| **現状:** Webpacker 5.4.3 を使用。Rails 8 では完全に非対応。 | ||
|
|
||
| **移行方針:** `jsbundling-rails` + `esbuild` に移行する。 |
There was a problem hiding this comment.
The specification document states the migration path is Webpacker → jsbundling-rails with esbuild, but the actual implementation uses a custom esbuild.config.mjs without jsbundling-rails gem integration. The Gemfile includes jsbundling-rails but it's not configured via the standard Rails installers. Verify that the manual esbuild configuration is intentional and properly integrated, or consider using the standard jsbundling-rails approach with rails javascript:install:esbuild.
| SimpleCov.formatter = SimpleCov::Formatter::MultiFormatter.new([ | ||
| SimpleCov::Formatter::LcovFormatter, | ||
| SimpleCov::Formatter::HTMLFormatter | ||
| # Coveralls::SimpleCov::Formatter | ||
| ] | ||
| ]) |
There was a problem hiding this comment.
The SimpleCov formatter initialization syntax has been updated correctly from array bracket notation to .new() method. However, ensure that this syntax is compatible with the version of SimpleCov being used (0.22.0 in Gemfile.lock).
|
|
||
| | 項目 | 内容 | | ||
| |------|------| | ||
| | 対象アプリケーション | Radvent(Qiita風アドベントカレンダーWebアプリ) | |
There was a problem hiding this comment.
The specification indicates the target is Rails 8.0.x but the actual Gemfile shows Rails 8.1.0 is being used. This inconsistency should be resolved - either update the specification to reflect Rails 8.1.x or adjust the Gemfile to use Rails 8.0.x as originally planned.
| describe 'POST #preview' do | ||
| it 'renders the :preview for a new item' do | ||
| post :preview, params: { item: attributes_for(:item) } | ||
| expect(response).to render_template :preview | ||
| end | ||
|
|
||
| it 'renders the :preview for an existing item' do | ||
| item = @advent_calendar_item.item | ||
| post :preview, params: { id: item, item: attributes_for(:item) } | ||
| expect(response).to render_template :preview | ||
| end | ||
| end |
There was a problem hiding this comment.
The preview tests have been refactored from testing the preview_button parameter within create/update actions to a dedicated preview action. This is a cleaner separation of concerns. However, verify that the routes are properly configured to support both collection and member preview routes as mentioned in the routes.rb changes.
| ActiveRecord::Schema[8.1].define(version: 2017_12_11_141402) do | ||
| create_table "advent_calendar_items", force: :cascade do |t| | ||
| t.string "user_name" | ||
| t.string "comment" | ||
| t.integer "date" | ||
| t.datetime "created_at", precision: nil | ||
| t.integer "date" | ||
| t.integer "event_id" | ||
| t.datetime "updated_at", precision: nil | ||
| t.integer "user_id" | ||
| t.integer "event_id" | ||
| t.string "user_name" | ||
| end | ||
|
|
||
| create_table "attachments", force: :cascade do |t| | ||
| t.integer "advent_calendar_item_id" | ||
| t.string "image" | ||
| t.datetime "created_at", precision: nil | ||
| t.string "image" | ||
| t.datetime "updated_at", precision: nil | ||
| t.index ["advent_calendar_item_id"], name: "index_attachments_on_advent_calendar_item_id" | ||
| end | ||
|
|
||
| create_table "comments", force: :cascade do |t| | ||
| t.integer "item_id" | ||
| t.string "user_name" | ||
| t.text "body" | ||
| t.datetime "created_at", precision: nil | ||
| t.integer "item_id" | ||
| t.datetime "updated_at", precision: nil | ||
| t.string "user_name" | ||
| t.index ["item_id"], name: "index_comments_on_item_id" | ||
| end | ||
|
|
||
| create_table "events", force: :cascade do |t| | ||
| t.string "title" | ||
| t.datetime "created_at", precision: nil, null: false | ||
| t.integer "created_by_id" | ||
| t.text "description" | ||
| t.date "end_date" | ||
| t.string "name" | ||
| t.integer "version" | ||
| t.date "start_date" | ||
| t.date "end_date" | ||
| t.integer "created_by_id" | ||
| t.integer "updated_by_id" | ||
| t.datetime "created_at", precision: nil, null: false | ||
| t.string "title" | ||
| t.datetime "updated_at", precision: nil, null: false | ||
| t.text "description" | ||
| t.integer "updated_by_id" | ||
| t.integer "version" | ||
| end | ||
|
|
||
| create_table "items", force: :cascade do |t| | ||
| t.string "title" | ||
| t.integer "advent_calendar_item_id" | ||
| t.text "body" | ||
| t.integer "comments_count", default: 0, null: false | ||
| t.datetime "created_at", precision: nil | ||
| t.string "title" | ||
| t.datetime "updated_at", precision: nil | ||
| t.integer "comments_count", default: 0, null: false | ||
| t.text "body" | ||
| t.index ["advent_calendar_item_id"], name: "index_items_on_advent_calendar_item_id", unique: true | ||
| end | ||
|
|
||
| create_table "likes", force: :cascade do |t| | ||
| t.integer "item_id" | ||
| t.integer "user_id" | ||
| t.datetime "created_at", precision: nil | ||
| t.integer "item_id" | ||
| t.datetime "updated_at", precision: nil | ||
| t.integer "user_id" | ||
| end | ||
|
|
||
| create_table "users", force: :cascade do |t| | ||
| t.boolean "admin" | ||
| t.datetime "confirmation_sent_at", precision: nil | ||
| t.string "confirmation_token" | ||
| t.datetime "confirmed_at", precision: nil | ||
| t.datetime "created_at", precision: nil, null: false | ||
| t.datetime "current_sign_in_at", precision: nil | ||
| t.string "current_sign_in_ip" | ||
| t.string "email", default: "", null: false | ||
| t.string "encrypted_password", default: "", null: false | ||
| t.string "reset_password_token" | ||
| t.datetime "reset_password_sent_at", precision: nil | ||
| t.datetime "remember_created_at", precision: nil | ||
| t.integer "sign_in_count", default: 0, null: false | ||
| t.datetime "current_sign_in_at", precision: nil | ||
| t.integer "failed_attempts", default: 0, null: false | ||
| t.datetime "last_sign_in_at", precision: nil | ||
| t.string "current_sign_in_ip" | ||
| t.string "last_sign_in_ip" | ||
| t.string "confirmation_token" | ||
| t.datetime "confirmed_at", precision: nil | ||
| t.datetime "confirmation_sent_at", precision: nil | ||
| t.datetime "locked_at", precision: nil | ||
| t.string "name" | ||
| t.datetime "remember_created_at", precision: nil | ||
| t.datetime "reset_password_sent_at", precision: nil | ||
| t.string "reset_password_token" | ||
| t.integer "sign_in_count", default: 0, null: false | ||
| t.string "unconfirmed_email" | ||
| t.integer "failed_attempts", default: 0, null: false | ||
| t.string "unlock_token" | ||
| t.datetime "locked_at", precision: nil | ||
| t.datetime "created_at", precision: nil, null: false | ||
| t.datetime "updated_at", precision: nil, null: false | ||
| t.string "name" | ||
| t.boolean "admin" | ||
| t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true | ||
| t.index ["email"], name: "index_users_on_email", unique: true | ||
| t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true | ||
| t.index ["unlock_token"], name: "index_users_on_unlock_token", unique: true | ||
| end | ||
|
|
||
| end |
There was a problem hiding this comment.
The schema version has been updated from ActiveRecord::Schema[7.0] to ActiveRecord::Schema[8.1], but the column ordering within tables has changed (appears to be alphabetical now). This is a cosmetic change caused by Rails 8's schema dumper, but verify that no actual column definitions or constraints have been unintentionally modified beyond the reordering.
| module Version | ||
| VERSION = '3.0.0beta'.freeze | ||
| def self.version | ||
| VERSION | ||
| end |
There was a problem hiding this comment.
The module name has been changed from Radvent::VERSION to Radvent::Version. Ensure all references throughout the codebase have been updated. A quick search shows app/views/layouts/application.html.haml has been updated correctly to use Radvent::Version.version, but verify there are no other references to the old constant name.
| %i.fa.fa-user | ||
| = "#{current_user.name}" | ||
| = link_to destroy_user_session_path, method: :delete, class: "nav-item nav-link" do | ||
| = link_to destroy_user_session_path, class: "nav-item nav-link", data: { "turbo-method": :delete } do |
There was a problem hiding this comment.
The data: { "turbo-method": :delete } attribute is correct for Turbo, but you should also update the confirmation to use data: { turbo_confirm: ... } instead of the old data: { confirm: ... } pattern for consistency. Check if other delete links in the codebase are using the same pattern.
| { | ||
| "application.css": "/packs/css/application.css", | ||
| "application.css.map": "/packs/css/application.css.map", | ||
| "application.eot": "/packs/static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-brands-400-23f19bb08961f37aaf69.eot", | ||
| "application.js": "/packs/js/application.js", | ||
| "application.js.map": "/packs/js/application.js.map", | ||
| "application.svg": "/packs/static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-regular-400-4689f52cc96215721344.svg", | ||
| "application.ttf": "/packs/static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-brands-400-527940b104eb2ea366c8.ttf", | ||
| "application.woff": "/packs/static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-brands-400-2285773e6b4b172f07d9.woff", | ||
| "application.woff2": "/packs/static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-regular-400-7a3337626410ca2f4071.woff2", | ||
| "common.js": "/packs/js/common.js", | ||
| "common.js.map": "/packs/js/common.js.map", | ||
| "entrypoints": { | ||
| "application": { | ||
| "assets": { | ||
| "js": [ | ||
| "/packs/js/runtime.js", | ||
| "/packs/js/vendors-node_modules_jquery_src_jquery_js.js", | ||
| "/packs/js/vendors-node_modules_datatables_net-bs5_js_dataTables_bootstrap5_mjs.js", | ||
| "/packs/js/vendors-node_modules_mdb-ui-kit_js_mdb_min_js.js", | ||
| "/packs/js/vendors-node_modules_marked_lib_marked_js.js", | ||
| "/packs/js/vendors-node_modules_fortawesome_fontawesome-free_js_all_js-node_modules_rails_activestorage_-728e17.js", | ||
| "/packs/js/application.js" | ||
| ], | ||
| "css": [ | ||
| "/packs/css/application.css" | ||
| ] | ||
| } | ||
| }, | ||
| "common": { | ||
| "assets": { | ||
| "js": [ | ||
| "/packs/js/runtime.js", | ||
| "/packs/js/vendors-node_modules_jquery_src_jquery_js.js", | ||
| "/packs/js/common.js" | ||
| ] | ||
| } | ||
| }, | ||
| "events": { | ||
| "assets": { | ||
| "js": [ | ||
| "/packs/js/runtime.js", | ||
| "/packs/js/vendors-node_modules_jquery_src_jquery_js.js", | ||
| "/packs/js/vendors-node_modules_datatables_net-bs5_js_dataTables_bootstrap5_mjs.js", | ||
| "/packs/js/events.js" | ||
| ] | ||
| } | ||
| }, | ||
| "items": { | ||
| "assets": { | ||
| "js": [ | ||
| "/packs/js/runtime.js", | ||
| "/packs/js/vendors-node_modules_jquery_src_jquery_js.js", | ||
| "/packs/js/vendors-node_modules_mdb-ui-kit_js_mdb_min_js.js", | ||
| "/packs/js/vendors-node_modules_marked_lib_marked_js.js", | ||
| "/packs/js/items.js" | ||
| ] | ||
| } | ||
| }, | ||
| "likes": { | ||
| "assets": { | ||
| "js": [ | ||
| "/packs/js/runtime.js", | ||
| "/packs/js/likes.js" | ||
| ] | ||
| } | ||
| }, | ||
| "mdb": { | ||
| "assets": { | ||
| "js": [ | ||
| "/packs/js/runtime.js", | ||
| "/packs/js/vendors-node_modules_mdb-ui-kit_js_mdb_min_js.js", | ||
| "/packs/js/mdb.js" | ||
| ] | ||
| } | ||
| }, | ||
| "users": { | ||
| "assets": { | ||
| "js": [ | ||
| "/packs/js/runtime.js", | ||
| "/packs/js/vendors-node_modules_jquery_src_jquery_js.js", | ||
| "/packs/js/vendors-node_modules_datatables_net-bs5_js_dataTables_bootstrap5_mjs.js", | ||
| "/packs/js/users.js" | ||
| ] | ||
| } | ||
| } | ||
| }, | ||
| "events.js": "/packs/js/events.js", | ||
| "events.js.map": "/packs/js/events.js.map", | ||
| "items.js": "/packs/js/items.js", | ||
| "items.js.map": "/packs/js/items.js.map", | ||
| "js/vendors-node_modules_datatables_net-bs5_js_dataTables_bootstrap5_mjs.js": "/packs/js/vendors-node_modules_datatables_net-bs5_js_dataTables_bootstrap5_mjs.js", | ||
| "js/vendors-node_modules_datatables_net-bs5_js_dataTables_bootstrap5_mjs.js.map": "/packs/js/vendors-node_modules_datatables_net-bs5_js_dataTables_bootstrap5_mjs.js.map", | ||
| "js/vendors-node_modules_fortawesome_fontawesome-free_js_all_js-node_modules_rails_activestorage_-728e17.js": "/packs/js/vendors-node_modules_fortawesome_fontawesome-free_js_all_js-node_modules_rails_activestorage_-728e17.js", | ||
| "js/vendors-node_modules_fortawesome_fontawesome-free_js_all_js-node_modules_rails_activestorage_-728e17.js.map": "/packs/js/vendors-node_modules_fortawesome_fontawesome-free_js_all_js-node_modules_rails_activestorage_-728e17.js.map", | ||
| "js/vendors-node_modules_jquery_src_jquery_js.js": "/packs/js/vendors-node_modules_jquery_src_jquery_js.js", | ||
| "js/vendors-node_modules_jquery_src_jquery_js.js.map": "/packs/js/vendors-node_modules_jquery_src_jquery_js.js.map", | ||
| "js/vendors-node_modules_marked_lib_marked_js.js": "/packs/js/vendors-node_modules_marked_lib_marked_js.js", | ||
| "js/vendors-node_modules_marked_lib_marked_js.js.map": "/packs/js/vendors-node_modules_marked_lib_marked_js.js.map", | ||
| "js/vendors-node_modules_mdb-ui-kit_js_mdb_min_js.js": "/packs/js/vendors-node_modules_mdb-ui-kit_js_mdb_min_js.js", | ||
| "js/vendors-node_modules_mdb-ui-kit_js_mdb_min_js.js.map": "/packs/js/vendors-node_modules_mdb-ui-kit_js_mdb_min_js.js.map", | ||
| "likes.js": "/packs/js/likes.js", | ||
| "likes.js.map": "/packs/js/likes.js.map", | ||
| "mdb.js": "/packs/js/mdb.js", | ||
| "mdb.js.map": "/packs/js/mdb.js.map", | ||
| "runtime.js": "/packs/js/runtime.js", | ||
| "runtime.js.map": "/packs/js/runtime.js.map", | ||
| "static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-brands-400.eot": "/packs/static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-brands-400-23f19bb08961f37aaf69.eot", | ||
| "static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-brands-400.svg": "/packs/static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-brands-400-2f517e09eb2ca6650ff5.svg", | ||
| "static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-brands-400.ttf": "/packs/static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-brands-400-527940b104eb2ea366c8.ttf", | ||
| "static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-brands-400.woff": "/packs/static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-brands-400-2285773e6b4b172f07d9.woff", | ||
| "static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-brands-400.woff2": "/packs/static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-brands-400-d878b0a6a1144760244f.woff2", | ||
| "static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-regular-400.eot": "/packs/static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-regular-400-77206a6bb316fa0aded5.eot", | ||
| "static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-regular-400.svg": "/packs/static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-regular-400-4689f52cc96215721344.svg", | ||
| "static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-regular-400.ttf": "/packs/static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-regular-400-491974d108fe4002b2aa.ttf", | ||
| "static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-regular-400.woff": "/packs/static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-regular-400-bb58e57c48a3e911f15f.woff", | ||
| "static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-regular-400.woff2": "/packs/static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-regular-400-7a3337626410ca2f4071.woff2", | ||
| "static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-solid-900.eot": "/packs/static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-solid-900-9bbb245e67a133f6e486.eot", | ||
| "static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-solid-900.svg": "/packs/static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-solid-900-7a8b4f130182d19a2d7c.svg", | ||
| "static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-solid-900.ttf": "/packs/static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-solid-900-be9ee23c0c6390141475.ttf", | ||
| "static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-solid-900.woff": "/packs/static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-solid-900-eeccf4f66002c6f2ba24.woff", | ||
| "static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-solid-900.woff2": "/packs/static/node_modules/@fortawesome/fontawesome-free/webfonts/fa-solid-900-1551f4f60c37af51121f.woff2", | ||
| "users.js": "/packs/js/users.js", | ||
| "users.js.map": "/packs/js/users.js.map" | ||
| } No newline at end of file |
There was a problem hiding this comment.
Compiled webpacker output files (public/packs/) should not be committed to version control. These files are generated artifacts that should be built during deployment. Add public/packs/ to .gitignore or ensure these are only being included for demonstration purposes. The .gitignore already excludes /node_modules, but webpack build outputs should also be excluded.
| "@fortawesome/fontawesome-free": "^7.2.0", | ||
| "@hotwired/turbo-rails": "^8.0.0", | ||
| "@rails/actioncable": "^8.1.0", | ||
| "@rails/activestorage": "^8.1.0", | ||
| "datatables.net-bs5": "^2.3.7", | ||
| "esbuild": "^0.27.3", | ||
| "jquery": "^4.0.0", | ||
| "marked": "^17.0.3", | ||
| "mdb-ui-kit": "^9.3.0", | ||
| "sass": "^1.97.3" |
There was a problem hiding this comment.
The package.json shows extremely aggressive version updates that may introduce breaking changes:
- jquery: ^3.6.0 → ^4.0.0 (major version bump)
- marked: ^3.0.8 → ^17.0.3 (14 major versions jump!)
- @fortawesome/fontawesome-free: ^5.15.4 → ^7.2.0
- mdb-ui-kit: ^3.10.2 → ^9.3.0
These massive version jumps are very risky. Each should be tested individually for breaking changes. The marked library especially has likely introduced significant API changes across 14 major versions. Consider upgrading dependencies more incrementally and testing thoroughly.
…ntroller and update form for preview submission
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 97 out of 142 changed files in this pull request and generated 12 comments.
Comments suppressed due to low confidence (2)
.devcontainer/post-create.sh:10
- devcontainer の初期化が
workspace/config/secrets.ymlの存在チェックに依存していますが、今回secrets.yml廃止(credentials化)を進めているので条件が合わなくなります。/workspace/config/credentials*.yml.encなどに合わせてチェックを更新し、パスも/workspace/...に揃えてください。
bundle install
if [ ! -f workspace/config/secrets.yml ]
then
bundle exec rake radvent:generate_default_settings
fi
config/shakapacker.yml:66
- dev_server 設定を Shakapacker 用に更新していますが、Gemfile/Gemfile.lock に
shakapackerが無く、このPRではjsbundling-rails+app/assets/buildsを採用しています。Shakapacker を使うなら gem/ビルド/ヘルパー参照を揃える、使わないならconfig/shakapacker.ymlとbin/shakapacker*を削除して構成を一本化してください。
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Radvent is a Qiita-style Advent Calendar web application (Japanese-focused). Users can create events, publish markdown articles on specific calendar dates, and interact via likes and comments. Built with Ruby on Rails and Webpacker. | ||
|
|
||
| ## Common Commands | ||
|
|
||
| ### Setup | ||
| ```bash | ||
| bundle install | ||
| yarn install | ||
| bundle exec rake radvent:generate_default_settings # generates config/database.yml, config/secrets.yml, config/initializers/devise.yml | ||
| bundle exec rake db:create db:migrate |
There was a problem hiding this comment.
このドキュメントが「Webpacker 5」や config/secrets.yml 生成、CI が Ruby 3.0/3.1 など、今回の Rails 8.1 / jsbundling / credentials 方針と食い違っています。現状の構成に合わせて内容を更新しないとオンボーディング資料として誤解を招きます。
| | 現行バージョン | Rails 7.0.2.2 / Ruby 3.1.4 | | ||
| | 目標バージョン | Rails 8.0.x(最新安定版) | | ||
| | Ruby要件 | Ruby 3.2以上(Rails 8の必須要件) | |
There was a problem hiding this comment.
仕様書では目標バージョンが Rails 8.0.x になっていますが、このPRでは Gemfile/Gemfile.lock と db/schema.rb が Rails 8.1 系になっています。仕様書側も 8.1 に更新するか、コード側を 8.0 に留めるかを揃えないと作業計画が誤解されます。
| def create | ||
| @like = Like.create(user_id: current_user.id, item_id: params[:item_id]) | ||
| @likes = Like.where(item_id: params[:item_id]) | ||
| @item = Item.find_by(id: params[:item_id]) | ||
|
|
||
| respond_to do |format| | ||
| format.turbo_stream | ||
| format.html { redirect_to @item } | ||
| end |
There was a problem hiding this comment.
LikesController#create が current_user.id を前提にしていますが、このコントローラに before_action :authenticate_user! が無いので未ログインアクセスで例外になります。create/destroy に認証を必須化し、あわせて @item が nil の場合は 404 などで早期リターンする処理を追加してください。
| routing_specs: false, | ||
| controller_specs: true, | ||
| request_specs: false | ||
| g.fixture_replacement :factory_girl, dir: 'spec/factories' | ||
| g.fixture_replacement :factory_bot, dir: 'spec/factories' | ||
| end |
There was a problem hiding this comment.
config.generators ブロック内の g.test_framework 設定がRuby構文として壊れています(request_specs: false の後にカンマがなく、さらに g.fixture_replacement がハッシュ引数の途中に入ってしまっています)。RSpec設定と g.fixture_replacement を正しいインデント/位置に直し、オプション行末のカンマも追加してください。
| def preview | ||
| @item = if params[:id] | ||
| item = Item.find_by(id: params[:id]) | ||
| render_404 and return unless item | ||
| item.tap { |i| i.assign_attributes(item_params) } | ||
| else |
There was a problem hiding this comment.
preview は params[:id] がある場合に既存 Item を取得していますが、edit_permission? が適用されないため、ログインさえしていれば他人の item を指定してプレビューできてしまいます。preview でも edit_permission? を通す(例: before_action :find_item, :edit_permission?, only: :preview か、idありの場合だけチェック)ようにしてください。
| yarn install | ||
|
|
||
| # 2. デフォルト設定ファイルを生成 | ||
| # config/database.yml / config/secrets.yml / config/initializers/devise.rb が作成される |
There was a problem hiding this comment.
radvent:generate_default_settings の説明で config/secrets.yml が生成されると書かれていますが、今回のタスク定義では secrets.yml を対象から外しています。README の記述を実装(database.yml と devise initializer など)に合わせて修正してください。
| # config/database.yml / config/secrets.yml / config/initializers/devise.rb が作成される | |
| # config/database.yml / config/initializers/devise.rb が作成される |
| def destroy | ||
| like = Like.find_by(user_id: current_user.id, item_id: params[:item_id]) | ||
| like.destroy | ||
| @likes = Like.where(item_id: params[:item_id]) | ||
| @item = Item.find_by(id: params[:item_id]) |
There was a problem hiding this comment.
destroy で like = Like.find_by(...) が nil の場合に like.destroy で例外になります(item未いいね/不正パラメータ等)。nil の場合は 404/204 を返す、もしくはエラーハンドリングして Turbo Stream/HTML の両方で整合するレスポンスにしてください。
| ### 前提条件 | ||
|
|
||
| が作られるので必要に応じて編集します。 | ||
| - Ruby >= 3.0 |
There was a problem hiding this comment.
README の前提条件が Ruby >= 3.0 になっていますが、Gemfile は ruby '>= 3.2.0' です。実際の要件に合わせて更新してください。
| - Ruby >= 3.0 | |
| - Ruby >= 3.2.0 |
| # Initialize configuration defaults for originally generated Rails version. | ||
| config.load_defaults 6.1 | ||
| config.load_defaults 8.0 | ||
|
|
There was a problem hiding this comment.
Rails が 8.1 系(Gemfile/Gemfile.lock, schema.rb)になっているのに config.load_defaults 8.0 のままだと新しいデフォルト設定が適用されません。config.load_defaults 8.1 に揃えるか、8.0 に固定する理由(互換目的など)を明記してください。
| @@ -1,21 +1,19 @@ | |||
| FROM ruby:3.1.1 | |||
| FROM ruby:3.2 | |||
There was a problem hiding this comment.
Gemfile.lock の RUBY VERSION が 3.4.8 なので、FROM ruby:3.2 だと bundle install 時に Ruby バージョン不一致で失敗します。Dockerfile の Ruby を lock に合わせるか、lock を Ruby 3.2 系で作り直して整合させてください。
| FROM ruby:3.2 | |
| FROM ruby:3.4.8 |
No description provided.