[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}
Without this before the DummyApp call, we don't get the migrations and models from `solidus_legacy_promotions` that we need.
This loads the SolidusAdmin gem if available, and adds the routes to the Dummy App if the example in question is marked `solidus_admin`. In addition, this loads the accessibility matchers many of the admin specs need.
This allows the new Solidus Admin to display Adjustments with a source of type `SolidusFriendlyPromotions::Benefit`.
This was referencing a factory from solidus_legacy_promotions
This is all that's implemented in core currently.
With 10b555b merged, we don't need this override anymore, and our build should go green again.
This was an oversight.
This is still mentioning actions when we mean to say benefits.
If this is a vanilla install, we cannot make a foreign key constraint to the `spree_promotions` table as it is not present.
In case we don't have the legacy promotion system present, we can't sync things by default.
This way we keep our specs clear of the legacy_promotions gem, but we provide a way of syncing that is visible.
Solidus still supports Rails 7.0, and we should, too.
Adjustment#eligible defaults to true, and we never set it to false. It's a remnant of the legacy promotion system. I'm also changing the spec setup of those specs that indicate ineligible promotion adjustments from setting the eligible flag to false to setting the promotion up with an impossible condition instead.
This association is not used anywhere in the codebase, and it's probably a remnant of adjustments being directly associated with promotion codes. SolidusLegacyPromotions does this, but we have no need for this, because we can just as well join via the `friendly_order_promotions` table. An adjustment from a promotion with codes will have one of the promotion benefits as a source, and an order_promotion with a reference to the promotion code used. I could have also changed the association to one that follows this path, but I'm really not sure what we would use this association for.
In friendlycart/solidus_friendly_promotions#130, we changed the default of SolidusFriendlyPromotions.config.sync_order_promotions, but failed to adjust this spec accordingly.
In friendlycart/solidus_friendly_promotions#128 we renamed the config option `actions` to `benefits`, but failed to include that renaming in the benefits controller.
In our test suite, we need to have solidus_legacy_promotions loaded. This gem sets the `Spree::Config.order_contents_class` to the legacy `Spree::OrderContents`, but we need `SimpleOrderContents` so we don't run into the legacy promotion system.
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.
3.21.0 introduced a bug with testing view components.
Pin view_component to < 3.21.0
Rather than implementing routing proxies ourselves, we can use what Rails offers. Fixes #6038
Rails' `mounted_helpers` do not work when a component is initialized. They need to be called when the component is rendered. This change moves the rendering of tabs from initialization time to render time.
The mock component in the ComponentHelpers module was not a real constant, which messes with ViewComponent's expectations about what render_inline is given. Using `stub_const` in the helper allows us to give it an actual name, and view_component > 3.21.0 will work for us. The helper is only used in the base component spec. I could have spent more time giving it an optional block, but this solution is the most straightforward.
Fix unsafe html view component, allow ViewComponent 3.21+
[Admin] Use Rails.application.mounted_helpers in base component
This moves the default state machine modules into `app`, making them autoloadable. In order to accomplish that, I had to change the superclass of `Spree::Core::StateMachines` to a `Spree::Preferences::Configuration`, so that the `class_name_attribute` method is available. Technically, we're using it for modules here, but that does not actually matter in this context. We just need it to run `constantize`. This also removes some custom "autoloading" code that is not actually all that necessary. The configuration interface stays the same.
Make state machine modules auto-loadable
Spree::Money is a model that should be able to be autoloaded.
Make Spree::Money autoloadable
This makes Variant#tax_category_id work consistently with Variant#tax_category so they both reference the same tax_category Co-authored-by: Adam Mueller <[email protected]>
This is similar to how variant itself delegates to product in accessing tax_categories. line_item sets it’s tax_category in a before_validation, but this will be updated in a future commit. Co-authored-by: Adam Mueller <[email protected]>
A future commit will update tax calculations to ensure the variant’s tax_category is considered if it is updated compared with the line_item’s
This test is demonstrating a false passing because the OrderInventory instance actually receives the `verify` message on being created, and previous to this change, that would happen after the `expect_` line, causing this test to pass despite `verify` not being received during the `destory`. This change adds a `target_shipment` to the `line_item`, which is needed to trigger the `verify` call.
In the default tax_calculator, this change ensures that the variant’s current tax_category is considered when calculating taxes. In the case of the item_rates method, only line_item items will have this method, so it is accessed with `try` first, with a fallback to without `variant_`. `try` is necessary to use instead of `.&` or `try!` because `Spree::Shipment`s and `Spree::ShippingRate`s do not implement `#variant_tax_category_id`. Co-authored-by: Adam Mueller <[email protected]>
The tax rates used for updating adjustments respect the current tax_category of the variant. This update to the line_item’s tax_category_id represents the tax_category that was used to calculate the taxes. Co-authored-by: Adam Mueller <[email protected]>
…ems-respect-updates Tax Categories on Line Items respect updates to Variant and Product Tax Categories
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
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 : )