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

Upgrade globalize to version 7.0 for Rails 7.2 support #153

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

swamp09
Copy link

@swamp09 swamp09 commented Jan 20, 2025

This PR updates the globalize dependency to ~> 7.0 to support Rails 7.2.
https://github.com/globalize/globalize/releases/tag/v7.0.0

swamp09 added a commit to swamp09/solidus that referenced this pull request Jan 20, 2025
This PR modifies the Persistable module in Solidus to check the Rails version using Rails.gem_version instead of inspecting the serialize method parameters. This change is necessary only when using Solidus Globalize, ensuring compatibility with Rails 7.2.

## Background
Solidus itself does not have any issues with the current implementation:

```
if method(:serialize).parameters.include?([:key, :type]) # Rails 7.1+
  serialize :preferences, type: Hash, coder: YAML
else
  serialize :preferences, Hash, coder: YAML
end
```

However, when using Solidus Globalize, which relies on globalize, an issue arises in Rails 7.2 due to globalize overriding the serialize method:

```
ArgumentError: wrong number of arguments (given 2, expected 1)
```

This occurs because the serialize method signature is modified by globalize, causing a mismatch when Solidus inspects its parameters.
For reference, see the globalize implementation here:
https://github.com/globalize/globalize/blob/main/lib/patches/active_record/rails7_2/serialization.rb

## Solution
To avoid conflicts, this PR replaces the method introspection with a Rails version check:
```
if Rails.gem_version >= Gem::Version.new('7.1')
  serialize :preferences, type: Hash, coder: YAML
else
  serialize :preferences, Hash, coder: YAML
end
```
This approach ensures compatibility while keeping Solidus independent from dependency-specific overrides.

This issue is currently blocking the CI of the following PR in the solidus_globalize repository:
solidusio-contrib/solidus_globalize#153
Fixing this in Solidus will help unblock that PR.
github-actions bot pushed a commit to solidusio/solidus that referenced this pull request Jan 21, 2025
This PR modifies the Persistable module in Solidus to check the Rails version using Rails.gem_version instead of inspecting the serialize method parameters. This change is necessary only when using Solidus Globalize, ensuring compatibility with Rails 7.2.

## Background
Solidus itself does not have any issues with the current implementation:

```
if method(:serialize).parameters.include?([:key, :type]) # Rails 7.1+
  serialize :preferences, type: Hash, coder: YAML
else
  serialize :preferences, Hash, coder: YAML
end
```

However, when using Solidus Globalize, which relies on globalize, an issue arises in Rails 7.2 due to globalize overriding the serialize method:

```
ArgumentError: wrong number of arguments (given 2, expected 1)
```

This occurs because the serialize method signature is modified by globalize, causing a mismatch when Solidus inspects its parameters.
For reference, see the globalize implementation here:
https://github.com/globalize/globalize/blob/main/lib/patches/active_record/rails7_2/serialization.rb

## Solution
To avoid conflicts, this PR replaces the method introspection with a Rails version check:
```
if Rails.gem_version >= Gem::Version.new('7.1')
  serialize :preferences, type: Hash, coder: YAML
else
  serialize :preferences, Hash, coder: YAML
end
```
This approach ensures compatibility while keeping Solidus independent from dependency-specific overrides.

This issue is currently blocking the CI of the following PR in the solidus_globalize repository:
solidusio-contrib/solidus_globalize#153
Fixing this in Solidus will help unblock that PR.

(cherry picked from commit 2bcf076)
This PR updates the globalize dependency to ~> 7.0 to support Rails 7.2.
https://github.com/globalize/globalize/releases/tag/v7.0.0
@swamp09 swamp09 force-pushed the upgrade_globalize_7.0 branch from e3b6e89 to fb95f5f Compare February 13, 2025 06:57
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.

2 participants