[pull] main from solidusio:main #411
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![pull[bot]](https://avatars.githubusercontent.com/in/12910?s=60&v=4){kind=small}
Otherwise we don't load solidus_legacy_promotions, and its migrations don't run.
This spec was failing before because the sync option was not set.
This mirrors the setup in Solidus core, and in Solidus Legacy Promotions.
Solidus core does not depend on solidus_dev_support, so we should not either.
As preparation for the solidus integration, we don't need to create tables and then rename them. Let's rather have them the right name from the get-go.
This has the same effect and removes a rubocop issue.
In the new solidus_promotions gem, we need some way of distinguishing factories and associations from the legacy_promotion ones. In `solidus_friendly_promotions`, that prefix was `friendly_`, in `solidus_promotions`, it's `solidus_`.
It's not needed, we have SolidusPromotions::TestingSupport::FactoryBot.
This will always be available, and users should not configure it.
If we have this gem loaded, we definitely want it to be visible. We also want the menu items to be tested.
The else case here cannot be reached (I tried), and the case statement is repeated in the method below.
This does not make any difference but gives better stats.
All of these have no preferences, but associations, and therefore need to be updateable.
Also fix the spec name so it actually runs.
This lambda has to be set on the instantiated class, requiring the controller it's set on to be autoloaded on boot. Loading a controller also loads all helpers, and that takes quite a bit of time. So this comes with a performance benefit, because neither the controller nor the handler need to be loaded at configuration time.
The previous `unauthorized_redirect` can really only be set globally, but it's a common use case to handle unauthorized access differently for the admin. This adds a configuration option for the admin base controller, allowing customizers to set that behavior without having to patch any controller.
Customizers can now set their own unauthorized_redirect class with any behavior from the controller they might need. There's no need for this suggestion (and the comment is not entirely true either, as the handler provided does not re-raise the exception, but raises another one).
This make sure the admin also uses the configured redirect handler class.
…config Unauthorized redirect handling config
Rubocop was understandably not happy about comparing very obviously identical things, so I've made it less obvious by creating a new, identical object.
Lint: Fix Money spec
Linting runs parallel to testing and circle ci is slow and our free plan has limited resources. Also GH actions are better integrated and has better dev feedback.
CI: Lint code on GH actions
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.
…version_check Fix preferences serialization compatibility with Rails version check
If a table has no rowUrl defined we currently redirect to root path. Which is not preferable. We should do nothing instead.
We do not want to render the whole admin layout if the request is a turbo frame request. We just want to render the component's html in the minimal rails turbo frame layout, which still gives us the option to yield something into the head for ie. cache control.
We don't want to deal with Turbo caching issues in the new admin. The content a customer agent or product administrator should always be the latest and most accurate version. Prefetching is a performance nightmare for an admin interface like ours. Co-authored-by: Eugene Chaikin <[email protected]>
This controller and components should be used for the Solidus Admin RESTful resource views. It handles Turbo Frame requests from the forms inside the modal dialog and redirects or rerenders via turbo stream accordingly. That way we can use normal links and form submits for our resource views without the need for custom JS.
This tells turbo to reload the page if we redirect from a turbo frame request. No need to respond with a turbo refresh stream.
[Admin] Open edit and new forms in dialog with turbo frame
There's no real good reason to keep these files in lib and require them on app startup. Let's put them in app/ and let Zeitwerk handle their loading.
These are easy enough.
Make Controller Helpers autoloadable
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot]
Can you help keep this open source service alive? 💖 Please sponsor : )