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.

Sign up for GitHub

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

Jump to bottom

Lock solidus_core.gemspec to ransack '< 4.2' #5812

Merged
merged 2 commits into from
Aug 8, 2024
Merged

Lock solidus_core.gemspec to ransack '< 4.2' #5812

merged 2 commits into from
Aug 8, 2024

Commits on Aug 8, 2024

  1. Fix failing solidus_admin thumbnail spec 

    Failure: /spec/components/solidus_admin/ui/thumbnail/component_spec.rb:6

Error:

/home/circleci/solidus/admin/spec/components/previews/solidus_admin/ui/thumbnail/component_preview/overview.html.erb:53:
numbered parameter is already used in
/home/circleci/solidus/admin/spec/components/previews/solidus_admin/ui/thumbnail/component_preview/overview.html.erb:52:
outer block here Failure/Error: <% image = Spree::Image.new.tap {
_1.define_singleton_method(:attachment) { attachment } } %>

By replaing the second instance of numbered params, we can avoid this
error with no behavioural or functional changes to the code.

Interestingly though, it was only failing on 2 out of our 8 spec runs,
and it did so consistently.
    @MadelineCollier
    MadelineCollier committed Aug 8, 2024
    Configuration menu
    Copy the full SHA
    7176948 View commit details
    Browse the repository at this point in the history

  2. Lock solidus_core.gemspec to ransack '< 4.2' 

    Solidus core's gemspec already required that ransack be '~> 4.0', but
the latest version of ransack, v4.2.0, released July 10 2024, introduces
a bug. The previous implementation was taking for granted that every
predicate would respond to #value, which doesn't seem to be the case
when the predicate is an instance of a Arel::SelectManager.

This has already been flagged by @spaghetticode in his PR against
ransack: activerecord-hackery/ransack#1468

Since there has been little movement on this PR since January, we should
lock to a version that works for us since currently many of our product
specs are failing. (eg. spec/models/spree/product_spec.rb:659)

We can remove this lock once the PR is merged and once the above test
(and the others that are failing) are able to pass in ransack v4.2.0 or
subsequent versions.
    @MadelineCollier
    MadelineCollier committed Aug 8, 2024
    Configuration menu
    Copy the full SHA
    1664d10 View commit details
    Browse the repository at this point in the history