Skip to content

Rails8 phase2 step3#2582

Merged
jacopen merged 5 commits intomainfrom
rails8-phase2-step3
Aug 1, 2025
Merged

Rails8 phase2 step3#2582
jacopen merged 5 commits intomainfrom
rails8-phase2-step3

Conversation

@takaishi
Copy link
Contributor

No description provided.

takaishi and others added 2 commits July 26, 2025 11:14
- Update Gemfile: Rails 7.2 → 8.0
- Replace redis-rails with redis + redis-session-store for Rails 8 compatibility
- Update session store configuration
- Enable Rails 8.0 framework defaults
- Configure Japanese locale and Tokyo timezone
- Fix deprecation warnings (to_time_preserves_timezone, Regexp.timeout)
- Update configuration files via rails app:update
- Fix code style with rubocop

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
…upgrade

Development environment:
- Restore SQS queue adapter configuration
- Restore AWS SES email delivery settings
- Restore Bullet gem configuration for development
- Restore web console and Action Cable settings
- Restore Docker host configurations
- Restore assets and cache settings

Production environment:
- Restore SQS queue adapter configuration
- Restore AWS SES email delivery settings
- Restore review app and environment-specific URL configurations
- Restore OmniAuth failure handling
- Restore logging configuration
- Restore SSL and assets configurations

Application:
- Fix timezone setting to use 'Asia/Tokyo' instead of 'Tokyo'

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@gitops-for-cloudnativedays gitops-for-cloudnativedays bot added the reviewapps Build ReviewApp environment automatically if this label is granted label Jul 26, 2025
gitops-for-cloudnativedays bot added a commit to cloudnativedaysjp/dreamkast-infra that referenced this pull request Jul 26, 2025
commit: cloudnativedaysjp/dreamkast@4fd71d4
action URL: https://github.com/cloudnativedaysjp/dreamkast/actions/runs/16535508326

Co-authored-by: gitops-for-cloudnativedays[bot] <113280573+gitops-for-cloudnativedays[bot]@users.noreply.github.com>
gitops-for-cloudnativedays bot added a commit to cloudnativedaysjp/dreamkast-infra that referenced this pull request Jul 26, 2025
commit: cloudnativedaysjp/dreamkast@a20e118
action URL: https://github.com/cloudnativedaysjp/dreamkast/actions/runs/16535861042

Co-authored-by: gitops-for-cloudnativedays[bot] <113280573+gitops-for-cloudnativedays[bot]@users.noreply.github.com>
@claude
Copy link

claude bot commented Jul 26, 2025

Claude finished @takaishi's task —— View job


Rails 8アップグレード Pull Request レビュー

レビュータスク完了状況

  • 変更ファイルの分析
  • Rails 8へのアップグレード内容の確認
  • ID型の変更(bigint → integer)の調査
  • セキュリティとパフォーマンスの観点からのレビュー
  • テストカバレッジへの影響確認
  • annotate gem削除の影響確認

📋 概要

このPRはRails 7.2から8.0.2へのアップグレードを実施しています。22ファイルが変更され、主な変更点は:

  • Rails 8.0.2へのアップグレード
  • redis-railsからredis + redis-session-storeへの移行
  • 設定ファイルの更新
  • annotate gemの削除(Rails 8非対応のため)

⚠️ 重要な懸念事項

1. ID型の変更問題について

@jacopenさんが指摘されている通り、スキーマでID型がbigintからintegerに変更されているように見える問題について調査しました:

# schema.rbを確認すると、以下のような混在が見られます:
# - 一部のテーブルではinteger型のID/外部キー
# - 一部のテーブルではbigint型の外部キー

分析結果

  • 元のマイグレーションファイルでは、ID型を明示的に指定していません
  • config.load_defaults(8.0)に変更されたことで、Rails 8のデフォルトが適用されています
  • ただし、Rails 8でもデフォルトのプライマリキータイプはbigintのはずです

推奨事項
このスキーマの不整合は意図的なものではないと思われます。以下の対応を推奨します:

# config/application.rbに追加
config.generators do |g|
  g.orm :active_record, primary_key_type: :bigint
end

または、既存のデータベースとの整合性を保つため、明示的に設定することを検討してください。

2. セキュリティの改善

良い変更点:

  • config.filter_parametersに追加のセンシティブフィールドが含まれています(cvv, cvc等)
  • config.assume_ssl = trueが本番環境で設定されています
  • CORS設定はデフォルトで無効化されており、必要に応じて有効化できます

3. パフォーマンスへの影響

  • Solid Queueのサポートが追加されていますが、まだ有効化されていません
  • config.cache_storeの設定は開発環境で適切に設定されています
  • Puma設定にSolid Queueプラグインが追加されています

✅ 良い点

  1. 段階的なアップグレード: Rails 7.0 → 7.1 → 7.2 → 8.0と段階的にアップグレードを実施
  2. 互換性対応: redis-railsのRails 8非対応問題をredis-session-storeで解決
  3. 設定の更新: Rails 8のデフォルトを適切に適用(config.load_defaults(8.0)
  4. エラーページの更新: 新しいRails 8スタイルのエラーページ(400.html等)

🔧 改善提案

  1. ID型の統一

    # 新しいマイグレーションで統一することを検討
    class UnifyPrimaryKeyTypes < ActiveRecord::Migration[8.0]
      def change
        # 必要に応じてinteger型のIDをbigintに変換
      end
    end
  2. annotate gemの代替
    @TatsukiMinamiさんが提案されているannotaterbへの移行、または完全に削除(現在のPRの方針)のどちらでも問題ありません

  3. テストカバレッジ
    現在64.47%(閾値60%以上)なので問題ありませんが、Rails 8の新機能を使用する際は追加のテストを検討してください

📝 その他の確認事項

  • 本番環境でのRuby 3.3.6の動作確認
  • ステージング環境での十分な検証
  • ロールバック手順の準備
  • ID型の問題への対応方針の決定

🎯 結論

Rails 8へのアップグレードは概ね適切に実施されていますが、ID型の不整合問題は本番環境でのデータ増加時に問題となる可能性があるため、対応が必要です。その他の変更は適切で、セキュリティとパフォーマンスの観点からも改善が見られます。

レビューを完了しました。ご質問があればお知らせください。

gitops-for-cloudnativedays bot added a commit to cloudnativedaysjp/dreamkast-infra that referenced this pull request Jul 26, 2025
commit: cloudnativedaysjp/dreamkast@7bebc1f
action URL: https://github.com/cloudnativedaysjp/dreamkast/actions/runs/16538096439

Co-authored-by: gitops-for-cloudnativedays[bot] <113280573+gitops-for-cloudnativedays[bot]@users.noreply.github.com>
@github-actions
Copy link

Simplecov Report

Covered Threshold
64.47% 60%

@takaishi takaishi marked this pull request as ready for review July 26, 2025 08:55
@takaishi takaishi requested a review from a team July 26, 2025 09:03
Copy link
Collaborator

@jacopen jacopen left a comment

Choose a reason for hiding this comment

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

Claudeが指摘している

ID型の変更 - 多くのモデルで bigint から integer へ変更されています

admin_profiles、speakers、sponsors など
これは意図的な変更でしょうか?将来的にIDが枯渇する可能性があります

これは確かにそうだったので気になった。これは自動的になった?

@jacopen
Copy link
Collaborator

jacopen commented Jul 29, 2025

reviewappが起動してないかも

@TatsukiMinami
Copy link
Contributor

TatsukiMinami commented Jul 29, 2025

Claudeが指摘している差分がannotateのもので
gemのannotate がRailsの8.xで動かないので
ctran/annotate_models#1032
以下のgemに移行するのがいいと思います
https://github.com/drwl/annotaterb

@takaishi
Copy link
Contributor Author

annotate、なくても困らないし消しちゃおうかな

gitops-for-cloudnativedays bot added a commit to cloudnativedaysjp/dreamkast-infra that referenced this pull request Jul 31, 2025
commit: cloudnativedaysjp/dreamkast@8a5bf8f
action URL: https://github.com/cloudnativedaysjp/dreamkast/actions/runs/16646630052

Co-authored-by: gitops-for-cloudnativedays[bot] <113280573+gitops-for-cloudnativedays[bot]@users.noreply.github.com>
@takaishi takaishi force-pushed the rails8-phase2-step3 branch from 8a5bf8f to 7bebc1f Compare July 31, 2025 10:35
gitops-for-cloudnativedays bot added a commit to cloudnativedaysjp/dreamkast-infra that referenced this pull request Jul 31, 2025
commit: cloudnativedaysjp/dreamkast@7bebc1f
action URL: https://github.com/cloudnativedaysjp/dreamkast/actions/runs/16646668370

Co-authored-by: gitops-for-cloudnativedays[bot] <113280573+gitops-for-cloudnativedays[bot]@users.noreply.github.com>
gitops-for-cloudnativedays bot added a commit to cloudnativedaysjp/dreamkast-infra that referenced this pull request Jul 31, 2025
commit: cloudnativedaysjp/dreamkast@d42422b
action URL: https://github.com/cloudnativedaysjp/dreamkast/actions/runs/16646712827

Co-authored-by: gitops-for-cloudnativedays[bot] <113280573+gitops-for-cloudnativedays[bot]@users.noreply.github.com>
@github-actions
Copy link

@jacopen jacopen merged commit 4534c71 into main Aug 1, 2025
9 checks passed
@jacopen jacopen deleted the rails8-phase2-step3 branch August 1, 2025 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reviewapps Build ReviewApp environment automatically if this label is granted

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants