Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rails 8.0 #277

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Rails 8.0 #277

wants to merge 11 commits into from

Conversation

brunto
Copy link
Collaborator

@brunto brunto commented Apr 2, 2025

This pull request includes several significant changes to the codebase, focusing on upgrading dependencies, modifying configurations, and refactoring code for better performance and maintainability. The most important changes are grouped by theme below:

Dependency Upgrades:

  • Upgraded Ruby version from 3.3.7 to 3.4.2 in .ruby-version and updated Gemfile to use the new Ruby version. [1] [2]
  • Updated Rails version from 7.0.8 to 8.0.2 and removed version constraints for pg and puma gems in Gemfile.
  • Changed PostgreSQL image version from 11 to 17 in .github/workflows/ci.yml.

Configuration Changes:

  • Added DEFAULT_HOST to .env-example for better environment configuration.
  • Updated config/application.rb to use Rails 8.0 defaults and modified autoload paths.
  • Enhanced development and production environment configurations for better performance and maintainability in config/environments/development.rb and config/environments/production.rb. [1] [2]

Code Refactoring (BREAKING CHANGE):

  • Replaced CarrierWave with ActiveStorage for file uploads in app/models/attachment.rb and removed app/uploaders/file_uploader.rb. [1] [2]
  • Simplified AttachmentsController by removing unnecessary code and using ActiveStorage methods. [1] [2]
  • Updated AttachmentSerializer to use ActiveStorage for file attributes.

Authorization and Callbacks:

  • Modified ApplicationController to include a new before_action for active_storage_url_options and adjusted authorization logic. [1] [2]
  • Removed unused before_action callbacks from RevisionsController and StructuresController. [1] [2]

Miscellaneous:

  • Changed the enum declaration syntax in UserPia model for better readability.

These changes collectively enhance the application's performance, maintainability, and compatibility with newer versions of dependencies.

brunto added 5 commits March 31, 2025 22:44
Update Puma to the latest version, change Ruby version to 3.3.7, and 
upgrade the PostgreSQL image in CI configuration. These changes ensure 
compatibility with the latest features and security updates.
Add default email and password to user factory for easier test setup.  
Refactor test helper to use `find_or_create_by!` for Doorkeeper  
application and access token, ensuring idempotency. Remove unused  
development group in Gemfile and simplify pg gem versioning.
@brunto brunto requested review from Copilot and kevin-atnos April 2, 2025 19:59
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR upgrades the application to Rails 8 while also updating the Ruby version, adjusting environment configurations, and refactoring file upload handling from CarrierWave to ActiveStorage.

  • Dependency upgrades: Ruby to 3.4.2 and Rails to 8.0.2; PostgreSQL image updated in CI.
  • Configuration changes: Updated default URL options, logging, caching, and autoload settings across environments.
  • Code refactoring: Removed CarrierWave components, updated controllers and serializers to work with ActiveStorage, and refined before_action callbacks.

Reviewed Changes

Copilot reviewed 39 out of 41 changed files in this pull request and generated no comments.

Show a summary per file
File Description
config/initializers/filter_parameter_logging.rb Updated parameter filter list to include additional sensitive fields
config/initializers/default_url_options.rb Added default URL options configuration for route helpers
config/initializers/cors.rb Adjusted CORS documentation comment (minor text update)
config/initializers/carrierwave.rb Removed obsolete CarrierWave configuration
config/environments/test.rb Modified caching and error/reporting settings for the test environment
config/environments/production.rb Updated production SSL, caching, logging, and mailer default URL options
config/environments/development.rb Improved reloading behavior and logging in development
config/application.rb Updated defaults to Rails 8 and refined autoload library configuration
app/uploaders/file_uploader.rb Removed file uploader in light of migrating to ActiveStorage
app/serializers/attachment_serializer.rb Refactored file serialization to work with ActiveStorage
app/models/user_pia.rb Changed enum declaration syntax for clarity
app/models/attachment.rb Migrated attachment model from CarrierWave uploader to ActiveStorage attachment
app/controllers/structures_controller.rb Removed before_action callback for duplicate action – verify if intended
app/controllers/revisions_controller.rb Removed update callback from set_revision – confirm if update action is obsolete
app/controllers/attachments_controller.rb Simplified create and update actions in line with the ActiveStorage integration
app/controllers/application_controller.rb Added method to set ActiveStorage URL options
Gemfile Updated Ruby and Rails versions along with changes in gem declarations
.github/workflows/ci.yml Upgraded PostgreSQL image version from 11 to 17
Files not reviewed (2)
  • .env-example: Language not supported
  • .ruby-version: Language not supported
Comments suppressed due to low confidence (3)

config/environments/production.rb:25

  • Please verify that 'config.assume_ssl' is a supported configuration option in Rails 8 as it appears non-standard and may be redundant with 'config.force_ssl'.
config.assume_ssl = true

app/controllers/structures_controller.rb:2

  • If the 'duplicate' action still exists in this controller, please ensure that the necessary before_action callback is applied to properly initialize the structure.
before_action :set_structure, only: %i[show update destroy]

app/controllers/revisions_controller.rb:2

  • Confirm that the removal of the update callback is intentional and that the update action (if still present) does not depend on set_revision for correct operation.
before_action :set_revision, only: %i[show destroy]

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request upgrades dependencies to Rails 8.0 and Ruby 3.4.2, refactors file attachment handling by replacing CarrierWave with ActiveStorage, and updates various environment configurations for improved performance and maintainability.

  • Upgrades Ruby, Rails, and related Gem versions while simplifying Gemfile constraints.
  • Refactors file upload and serialization logic to use ActiveStorage instead of CarrierWave.
  • Updates environment and initializer configurations to align with the new Rails 8.0 defaults.

Reviewed Changes

Copilot reviewed 38 out of 40 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
config/initializers/filter_parameter_logging.rb Updated sensitive parameter filtering with additional keys.
config/initializers/default_url_options.rb Added a default URL options configuration with a conditional host assignment.
config/initializers/cors.rb Minor text update in the CORS configuration for clarity.
config/initializers/carrierwave.rb Removed CarrierWave initializer as CarrierWave is no longer used.
config/environments/test.rb Adjusted test environment settings including exception handling.
config/environments/production.rb Tweaked production configurations regarding SSL, logging, and caching.
config/environments/development.rb Updated reloading and caching settings; added explicit cache_store assignment.
config/application.rb Switched load defaults to Rails 8.0 and updated lib autoloading configuration.
app/uploaders/file_uploader.rb Removed deprecated CarrierWave uploader.
app/serializers/attachment_serializer.rb Refactored to utilize ActiveStorage file methods instead of file system access.
app/models/user_pia.rb Updated enum declaration syntax for clarity.
app/models/attachment.rb Replaced CarrierWave mounting with ActiveStorage's has_one_attached.
app/controllers/structures_controller.rb Removed unused before_action for a no longer used duplicate action.
app/controllers/revisions_controller.rb Removed an unnecessary before_action callback.
app/controllers/attachments_controller.rb Simplified attachment creation and updating to integrate with ActiveStorage.
app/controllers/application_controller.rb Added active_storage_url_options callback to set URL options for ActiveStorage.
Gemfile Upgraded Ruby and Rails versions while relaxing gem constraints.
.github/workflows/ci.yml Updated PostgreSQL image version for CI.
Files not reviewed (2)
  • .env-example: Language not supported
  • .ruby-version: Language not supported
Comments suppressed due to low confidence (1)

config/environments/test.rb:26

  • Using :rescuable for action_dispatch.show_exceptions in the test environment may alter the expected behavior of exception handling in tests.
config.action_dispatch.show_exceptions = :rescuable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant