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

[v4.4] Fix preferences serialization compatibility with Rails version check #6086

Merged
merged 1 commit into from
Jan 22, 2025

Conversation

github-actions[bot]
Copy link

Backport

This will backport the following commits from main to v4.4:

Questions ?

Please refer to the Backport tool documentation

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)
@github-actions github-actions bot requested a review from a team as a code owner January 21, 2025 08:29
@github-actions github-actions bot added the changelog:solidus_core Changes to the solidus_core gem label Jan 21, 2025
@tvdeyen tvdeyen merged commit 329d598 into v4.4 Jan 22, 2025
10 of 11 checks passed
@tvdeyen tvdeyen deleted the backport/v4.4/pr-6083 branch January 22, 2025 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:solidus_core Changes to the solidus_core gem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants