RFC: Remove Paranoia

[jarednorman]

We committed previously to fully moving from paranoia to discard in #2354. The initial work was completed and all models that "act as paranoid" include Spree::ParanoiaDeprecations which warns users that we are removing the paranoia gem from the project and that destroy is actually going to destroy things in the future. All of those models also include Discard::Model and are ready to be fully switched over.

In order to complete the switchover, I believe we need to:

1. Make sure we are using the kept scope as appropriate everywhere we need to. This isn't a small task, but is totally achievable. This work can be merged prior to actually removing the paranoia gem as it won't actually affect the behaviour of the system.
2. Remove the paranoia gem, all the references to acts_as_paranoid, and the Spree::ParanoiaDeprecations mixin. As the deprecation warns, this is a breaking change and should likely be included in next major release after it is merged.

Does anyone have any concerns about following through on this work and removing paranoia in the next major release?

[awinabi]

@jacobherrington I can take an attempt at this, if its open for work

[jarednorman]

I've cleaned out the outstanding issues/pull requests on the Discard project (I maintain it) and have started working on this, but assistance will be helpful. We're looking at using something like the functionality mentioned in this Discard issue to ensure that we don't miss any queries. I'm going to post a checklist of all the models that need to be migrated in the description of this task to make sure we don't duplicate work or miss anything.

[cedum]

I started looking into this too. While I like a lot the simplicity of discard, I think that migrating the Solidus' codebase, extensions, and client's codebases requires quite an effort: we've to manually review and test every query that touches in any way (joins for example) these tables. I'm also not sure that enforcing an explicit scope/query condition on all these queries is the right thing to do. AFAIK, there're very few cases where we need to work with soft-deleted data, most of the time we're working with non-deleted records.

I don't know if there're any valid alternatives. Maybe we should try to evaluate a completely different data structure for soft-deleted records, something similar to paper_trail (never used it in production, not sure if it's the right way to go, just an 💡).

[jarednorman]

The functionality described in the discard issue above would make it pretty easy to audit a codebase for queries that need to be addressed, so that shouldn't provide too much burden if done in a major bump. We deprecated paranoia in Solidus in anticipation of removing it in v3 years ago and I think we would need a pretty good reason to reverse our direction on that at this point.

All of the core eCommerce logic around orders needs to dal with soft deleted records that may be associated with orders; variants, products, tax rates, prices, promotion actions, shipping methods, payment methods and stock items that were deleted but are referenced by old orders still all need to work correctly.

Issue #2354 contains the original discussion around this and references the pain and issues that we've experienced using paranoia, which isn't even actively maintained currently. While I recognize there might be a not insignificant burden in migrating off, I've reviewed all the models that still use paranoia and don't think finally completing this migration is a prohibitive amount of work.

I'm pretty sure that undoing the work of this migration isn't a sensible option either.

As for paper_trail, I've used it in production on Solidus apps and I don't see a way that we could use it to solve the same problems that paranoia/discard solve for us in any reasonable way.

[kennyadsl]

Paranoia has been discarded and we'll ship Solidus without it in 3.0