Move to Spree::SoftDeletable on users model

[jtapia]

Summary

• Based on the latest gemspec of auth-devise, move to Spree::SoftDeletable

[gsmendoza]

Hello @jtapia !

• Can you reproduce the issue on the latest patch version of Solidus, v3.1.7?
• What Rails version are you using?

[jakemumu]

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'

[gsmendoza]

@jakemumu Can you tell what file you are modifying that causes hot reload to crash? How can we reproduce the error?

[jarednorman]

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.

[jtapia]

@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?

[jarednorman]

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.

[jtapia]

@jakemumu ok, here is the new PR https://github.com/solidusio/solidus_auth_devise/pull/226

[jtapia]

@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

[jtapia]

@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]

@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.

[jakemumu]

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

[jarednorman]

Yeah, it probably isn't significant what file you edit. Zeitwerk will reload everything if anything has changed since the last request.

[elia]

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?

[jarednorman]

Ah, no, that's fine then.

[elia]

@jtapia can you take care of updating the commit message and PR title?

[jtapia]

@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

[elia]

@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 🙏

[simon-isler]

@jarednorman would you mind looking into this again?