solidus_auth_devise
solidus_auth_devise copied to clipboard
solidusio
Move to Spree::SoftDeletable on users model
Summary
- Based on the latest gemspec of
auth-devise, move toSpree::SoftDeletable
Hello @jtapia !
- Can you reproduce the issue on the latest patch version of Solidus, v3.1.7?
- What Rails version are you using?
I'm seeing this autoloader issue on 3.1.7 -- App boots well but after modifying hot reload crashes:
undefined local variable or method acts_as_paranoid for Spree::User:Class
gem 'rails', '6.1.4.1'
@jakemumu Can you tell what file you are modifying that causes hot reload to crash? How can we reproduce the error?
We probably don't want https://github.com/solidusio/solidus/blob/master/core/app/models/concerns/spree/soft_deletable.rb to be code-reloaded. It sounds like there's an issue with code reload order with it.
@jarednorman ok, I moved it into 2 commits and created this issue https://github.com/solidusio/solidus_auth_devise/issues/225 is anything else that do you need?
If you put the keyword commit in a new PR we can merge it right away. I don't think the other commit will be the solution we want for the autoload issue though.
@jakemumu ok, here is the new PR https://github.com/solidusio/solidus_auth_devise/pull/226
@jarednorman if you removed all the references for acts_as_paranoid here https://github.com/solidusio/solidus_auth_devise/blob/master/app/models/spree/user.rb#L11=, why we continue using here https://github.com/solidusio/solidus_auth_devise/blob/master/app/models/spree/user.rb#L14=?
for older Solidus versions is ok, but since 3.0 removed Paranoia, I think we don't need that condition anymore or if we want to continue supporting older versions, we gonna need to bring back the paranoia reference
@gsmendoza I'm using rails', '6.1.4.1' and didn't test it yet with Solidus v3.1.7, on the issue I added more information
@jarednorman if you removed all the references for acts_as_paranoid here https://github.com/solidusio/solidus_auth_devise/blob/master/app/models/spree/user.rb#L11=, why we continue using here https://github.com/solidusio/solidus_auth_devise/blob/master/app/models/spree/user.rb#L14=? for older Solidus versions is ok, but since 3.0 removed Paranoia, I think we don't need that condition anymore or if we want to continue supporting older versions, we gonna need to bring back the paranoia reference
I'm not sure I understand what you're asking. The conditional above allows stores that are significantly behind (pre-v2.10.4 I think) on their Solidus upgrades to still use the latest version of solidus_auth_devise.
I get what you're saying @jarednorman -- I think that sounds correct. During hot reloading that check is failing & it's trying to load paranoid -- on initial app launch it's all good. It must be the user loading first for some reason.
@gsmendoza -- It's not any single file but we use view_component gem in our app and I've noticed it occur whenever I edit a x_component.rb
Yeah, it probably isn't significant what file you edit. Zeitwerk will reload everything if anything has changed since the last request.
I'm not sure I understand what you're asking. The conditional above allows stores that are significantly behind (pre-v2.10.4 I think) on their Solidus upgrades to still use the latest version of solidus_auth_devise.
@jarednorman the latest gemspec of auth-devise requires solidus 3.x, so I think it's ok to rely on Spree::SoftDeletable now. Any objections to merging other than updating the commit message?
@elia I updated it, but not sure if that is the title and description you want to, please let me know if you want to change it and I can do it
@jtapia I mentioned the PR title, but the most important thing is the commit message, for future lookups, as the current one could be confusing 🙏
