Fixed changes to support new solidus version #1
Conversation
lib/solidus_subscriptions.rb
Outdated
| @@ -2,7 +2,7 @@ | |||
|
|
|||
| require 'solidus_core' | |||
| require 'solidus_support' | |||
|
|
|||
| require 'solidus_legacy_promotions' | |||
There was a problem hiding this comment.
Without this, I am getting error uninitialized constant Spree::PromotionHandler hence I have added.
There was a problem hiding this comment.
But that class is never called in this extension. I suspect the problem is somewhere else. Do you have a full backtrace of the error?
There was a problem hiding this comment.
@kennyadsl I am not entirely sure about that https://github.com/search?q=repo%3Agms-electronics%2Fsolidus_subscriptions%20PromotionHandler&type=code
There was a problem hiding this comment.
You are right, I don't know what I was looking for. 😓 Also this extension adds some promotion rules for the legacy system. The quick way is to add legacy_promtionm as a dependency of this extension via its gemfile. But I guess the right way is to determine which promotion system has been used and call the right code based on that. @mamhoff if you have any guidance here, it would be appreciated.
There was a problem hiding this comment.
thanks @mamhoff!
@shahmayur001 you should be able to understand if there's the new or the legacy promotion system easily. Based on that we can avoid calling the code that only works with legacy.
There was a problem hiding this comment.
I've looked through both uses of the Promotion Handler here. You can just remove them and everything should work as it should.
what happens in that case if a promotion is applied to a product?
There was a problem hiding this comment.
@kennyadsl @fthobe, I tested the dependency integration with my demo application and found the following:
-
I was able to resolve the promotion-related issue without adding the solidus_legacy_promotions gem to the subscription extension. Instead, I included the solidus_legacy_promotions gem in my demo application, which I created using Solidus 4.4.
-
@mamhoff , when you mention removing the Promotion Handler from the subscription extension, can I assume that promotions will then be activated directly through Solidus Core?
If the first point is the expected behavior, we can simply remove the dependency from the extension.
There was a problem hiding this comment.
- The extension should work with both promo systems. You can check for the existence of the old promotion system by checking
Object.const_defined?("Spree::Promotion"). - Yes.
There was a problem hiding this comment.
We are good than. @shahmayur001 great work @mamhoff thank you for your help!
|
@mamhoff Mayur is in the process of fixing the subscription gem, could you take a look if any changes are to be done to support your new promotion system? |
I don't use this gem; if you need it to be compatible with the new promotions system, you'll have to make sure it is yourself. |
|
@shahmayur001 how is the last commit related to the breadcrumbs issue? |
@fthobe This is used by the old classic frontend, which is no longer supported. @kennyadsl Hope it's okay to remove it. |
|
It's ok to remove the locale strings, not sure how that is related to breadcrumbs (as the commit message says). |
The strings of the core are overwritten when they are bundled with the subscriptions gem creating a mess with the new starter front-end looking for strings that have changed. The front-end therfor renders as visible in #304 Subscriptions break front-end breadcrumbs (screenshot below).
|
shahmayur001
force-pushed
the
fix_for_new_solidus
branch
from
5fc85a1 to
86cf5eb
Compare
|
@shahmayur001 Squash the commits to appease @kennyadsl , I have put the fixed items in the bottom of the description, use the standard template for a PR.
|
Draft PR
Dependency Updates, Form Adjustments, and New Privileges
This pull request resolves compatibility issues with Solidus version updates by adding necessary gem dependencies, modifying forms for proper field rendering, fixing jQuery issues with subscription creation, and updating permission sets. These changes align Solidus Subscriptions with the latest Solidus architecture.
Enhancements:
Dependency Addition
solidus_legacy_promotionsgem, which is required as promotions have been separated fromsolidus_core.Form Rendering Restriction
jQuery Script Fix
subscribable_idis stored.Permission Set Updates
Fixes: solidusio#304 solidusio#303 solidusio#302