Skip to content

[868gfp7y1] RoR upgrade (v5.0.0)#71

Closed
ale1HQagui wants to merge 29 commits intomasterfrom
ror_upgrade
Closed

[868gfp7y1] RoR upgrade (v5.0.0)#71
ale1HQagui wants to merge 29 commits intomasterfrom
ror_upgrade

Conversation

@ale1HQagui
Copy link
Copy Markdown

Description

Please describe your pull request and any relevant information.

Risk Level

  • Low
  • Med
  • High

Rollback Plan

Please describe and add steps for rollback if needed.

Screenshots

Screenshots or videos of the feature/fix if needed.

Checklist

  • I linked the change to an approved ticket
  • I have added/updated appropriate tests and evidence
  • I considered security/privacy impact
  • I updated docs/runbook if needed
  • I added a changelog/version entry for this PR

@grvp88
Copy link
Copy Markdown

grvp88 commented Apr 9, 2026

Code Review — Rails 7.2 / Ruby 3.4 Compatibility Analysis

🔴 Blocker: build-ci-image failing

Misma causa raíz que todos los repos del upgrade. Resolver antes de mergear.


✅ Compatible con Rails 7.2 y Ruby 3.4 — con una observación importante

Breaking change de semántica: authorized_action default cambió a :view

# Antes
@authorized_action ||= :read
def initialize(*args, authorize_action: :read, ...)

# Después
@authorized_action ||= :view
def initialize(*args, authorize_action: :view, ...)

Este es un breaking change real. Cualquier app que pase authorize_action: :read explícitamente o que tenga lógica que dependa del valor :read en los fields va a tener comportamiento distinto. Necesita una entrada en el CHANGELOG indicando qué debe cambiar en el consumidor. No es solo un refactor de estilo.

concurrent-ruby bumpeado a 1.3.5 (anterior PR tenía 1.3.4) — verificar que la remoción del logger en 1.3.5 no rompe nada al cargar este gem.

require "set" removido — correcto para Ruby 3.4 donde Set es core.

Time.now -> Time.current en relative_date_expression.rb

# Antes (puede ignorar zona horaria)
Time.zone ? Time.zone.now : Time.now

# Después (Rails-aware, correcto)
Time.zone ? Time.zone.now : Time.current

Fix correcto — Time.current respeta Time.zone si está configurado.

Manager model en specs — inverse_of + foreign_key explícitos — Rails 7.2 es más estricto con la detección automática de inverse associations. El fix es correcto.

@grvp88
Copy link
Copy Markdown

grvp88 commented Apr 9, 2026

TODOs antes de mergear

  • Resolver build-ci-image — blocker.
  • Documentar el breaking change de authorized_action: :view en CHANGELOG — cualquier consumidor del gem que haga authorize_action: :read o que dependa del valor :read en la autorización va a tener un comportamiento distinto. Sin documentación, este cambio va a romper apps silenciosamente.
  • Verificar concurrent-ruby 1.3.5 — el bump de 1.3.4 a 1.3.5 remueve logger del bundle de concurrent-ruby. Asegurarse de que el gem o sus consumidores declaran logger explícitamente (ya está en Gemfile via ostruct y compañía, pero verificar).

@ale1HQagui
Copy link
Copy Markdown
Author

This PR i'll cose it because it was pointing to the wrong branch (master), should be main

@ale1HQagui ale1HQagui closed this Apr 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants