Update gem to support the latest Solidus version(4.4) #306
Conversation
|
@jarednorman we still struggle with SQLite testing, everything else looks ok. I think the test routine for SQLite is a little flawed. I have the impression it's more SQLite than us. |
|
Hey @jarednorman I believe the sqlite tests are outdated. Is it a you task to fix them or our job? I would be in favor to just remove them as they seem malconfigured. |
There was a problem hiding this comment.
We can merge without those passing until someone can fix them, but for future contributions, please split up the individual changes and explain them in the commit messages. The PR description is for additional information that's relevant to reviewers. The commits are the authoritative record of information about changes to the project.
|
@jarednorman you are getting soft |
|
#305 cross reference |
| insert_after: "[data-hook='admin_product_form_promotionable']", | ||
| partial: "spree/admin/products/subscribable_checkbox" | ||
| ) | ||
| module Views |
There was a problem hiding this comment.
Can we move these files into app/overrides/solidus_subscriptions/* and update the module? I don't want the extension defining random namespaces like Views that it doesn't "own".
There was a problem hiding this comment.
Seems doable, mayur is implementing that today.
There was a problem hiding this comment.
Resolved, Moved files to proper location
shahmayur001
force-pushed
the
fix-solidus4-compatibility
branch
from
38a4189 to
64700df
Compare
| # Rerun the promotion handler to pickup subscription promotions | ||
| ::Spree::PromotionHandler::Cart.new(line_item.order).activate |
There was a problem hiding this comment.
I guess my only concern is this. Is this safe? Do we need to do something different to handle legacy versus new promotions engines?
There was a problem hiding this comment.
@jarednorman Man, like really, from the bottom of my heart, could you be more specific, I am sure when you write this kind of comments you already have a very specific way in mind to do it.
There was a problem hiding this comment.
@mamhoff can you see anything that you dislike?
There was a problem hiding this comment.
And I wish that the PR explained why this seemingly random line was removed, but it doesn't even mention it. 🤷🏻♂️
There was a problem hiding this comment.
@jarednorman that is actually a tangible point and helps much more than the previous comment!!!
There was a problem hiding this comment.
@jarednorman hey, sorry for replying so late. The issue of the removal of ::Spree::PromotionHandler::Cart.new(line_item.order).activate was discussed here: gms-electronics#1 If you have something else to consider please tell us.
Summary
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:
Form Rendering Restriction
jQuery Script Fix
subscribable_idis stored.Permission Set Updates
Fixed breadcrumbs issue
Wrap deface overrides in a module
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: