Discard Paranoia #3488

cedum · 2020-01-24T14:50:54Z

Description
This removes paranoia and leaves discard as the only option for soft-deleting records.

TODO:

Deprecate paranoia methods and alias them to discard (e.g. with_deleted). PR Replace Paranoia methods with Discard equivalents #3554
Remove paranoia gem from dependencies once solidus_auth_devise dependency will be fixed: it's using paranoia but not declaring it explicitly as a dependency. Created PR on solidus_auth_devise Add paranoia dependency explicitly solidus_auth_devise#183
Spree::Api::ResourceController#destroy should support models with discard the same way as Spree::Admin::ResourceController#destroy PR Add soft-delete support for Api::ResourceController #3854

Checklist:

I have followed Pull Request guidelines
I have added a detailed description into each commit message
I have updated Guides and README accordingly to this change (if needed)
I have added tests to cover this change (if needed)
I have attached screenshots to this PR for visual changes (if needed)

Upgrade Notes

Paranoia has been replaced by Discard gem for models having the soft-delete feature enabled.
For more details about this move check this issue where the rationale has been discussed.

The soft-delete feature in Solidus, beyond being used as an archival tool, is mostly required to prevent hard-deletion of records other critical models depends on (i.e. Order).

NOTE: the soft-deletable models still have a default scope that filters out soft-deleted records to ease the migration.

Starting with Solidus v3 the following methods introduced by Paranoia are replaced by their Discard equivalents:

Class Methods

with_deleted becomes with_discarded
Returns both active and soft-deleted records.
only_deleted becomes discarded
Returns only soft-deleted records.
restore(id_or_ids) becomes where(id: [id(s)]).undiscard_all
Restores a list of soft-deleted records.

Instance Methods

destroy (also paranoia_destroy) becomes discard
Soft-deletes a record by firing also any callbacks defined for the model.
Before Solidus v3 when calling #destroy, hence soft-deleting a record, any [before|around|after]_destroy callbacks defined for that model were also fired.
With Solidus v3 any callbacks when soft-deleting a record must be defined using [before|around|after]_discard API.
[before|around|after]_destroy will be fired instead when hard-deleting a record.
delete (also paranoia_delete) becomes discard* (*see notes below)
Soft-deletes a record without firing the callbacks.
Discard doesn't provide a direct equivalent for soft-deleting without firing the [before|around|after]_discard callbacks.
The same behavior may be obtained by updating directly the soft-delete column:
update_attribute(self.class.discard_column, Time.current)
restore (also restore!) becomes undiscard (and undiscard!)
restores a soft deleted record. The methods with a bang ! raise an exception when failed.
really_destroy! becomes destroy
hard-deletes a record.
paranoia_destroyed? (also deleted?) becomes discarded?
checks if a record has been soft-deleted.

Model Callbacks

[before|around|after|]_destroy becomes [before|around|after|]_discard
fired when soft-deleting a record.
[before|around|after|]_real_destroy becomes [before|around|after|]_destroy
fired when hard-deleting a record.
[before|around|after|]_restore becomes [before|around|after|]_undiscard
fired when restoring a soft-deleted a record.

core/app/models/spree/product.rb

core/app/models/concerns/spree/default_price.rb

spaghetticode

@cedum thank you for working on such important and extensive change 🙏 🙇

I like the way the PR is coming 👍 , I left some discussion notes for some aspects that may be important to consider.

spaghetticode · 2020-02-21T11:26:07Z

core/app/models/spree/payment_method.rb

@@ -108,9 +103,9 @@ def active?
        where(type: to_s, active: true).count > 0
      end

-      # @deprecated Use .with_deleted.find instead
+      # @deprecated Use .with_discarded.find instead
      def find_with_destroyed(*args)


Not sure if this PR would be the best place for doing it, but I think that this deprecated method may just be removed.
Since this PR already has many breaking changes, it may make sense to take advantage of that in order to simply remove this old code as well.

spaghetticode · 2020-02-21T11:46:03Z

core/app/models/concerns/spree/soft_deletable.rb

+      include Discard::Model
+      self.discard_column = :deleted_at
+
+      default_scope { kept }


default_scope has a bit of bad rap, see for example https://rails-bestpractices.com/posts/2013/06/15/default_scope-is-evil/

I'm not implying we should not use it at all, but we should make store maintainers explicitly aware of its existence, with a message such as please verify your discard models overrides don't include a default_scope and that the new default_scope does not interact weirdly with your custom code.

Yeah, I understand the desire to use it here as it makes the migration significantly easier. I'd prefer we not use it by default for any future models we make soft-deletable though.

core/app/models/concerns/spree/soft_deletable.rb

jarednorman · 2020-02-23T21:19:17Z

core/lib/spree/core.rb

@@ -14,7 +14,6 @@
 require 'state_machines-activerecord'

 require 'spree/deprecation'
-require 'spree/paranoia_deprecations'


jarednorman

Awesome work here, thanks for doing this. All looks good to me.

ericsaupe

I like it. With such a large change we should have something in the release notes to indicate this large of a change.

jarednorman

Great work here! Excited for this.

We're replacing Paranoia with Discard as the only option for soft-deleting records.

Paranoia is being removed in favor of Discard. #destroy becomes the equivalent method of #really_destroy! for hard deleting records.

Paranoia has been completely removed. We don't need anymore to handle this case here.

Cleanup paranoia related deprecated methods and code comments.

Paranoia is replaced with Discard for soft-deleting records.

kennyadsl

The best PR's title ever! Thanks @cedum

solidusio/solidus#3488

houndci-bot reviewed Jan 24, 2020

View reviewed changes

core/app/models/spree/product.rb Outdated Show resolved Hide resolved

cedum force-pushed the remove-paranoia branch 2 times, most recently from a4774da to d2573bb Compare January 24, 2020 17:11

houndci-bot reviewed Jan 24, 2020

View reviewed changes

core/app/models/concerns/spree/default_price.rb Outdated Show resolved Hide resolved

cedum force-pushed the remove-paranoia branch 7 times, most recently from 625c12d to 99e93e9 Compare January 31, 2020 16:39

cedum force-pushed the remove-paranoia branch 4 times, most recently from e612a9f to 0780b35 Compare February 7, 2020 16:10

cedum marked this pull request as ready for review February 7, 2020 16:11

cedum force-pushed the remove-paranoia branch from 0780b35 to 921bf8b Compare February 14, 2020 15:24

cedum mentioned this pull request Feb 14, 2020

Add paranoia dependency explicitly solidusio/solidus_auth_devise#183

Merged

spaghetticode reviewed Feb 21, 2020

View reviewed changes

jarednorman reviewed Feb 23, 2020

View reviewed changes

core/app/models/concerns/spree/soft_deletable.rb Outdated Show resolved Hide resolved

jarednorman reviewed Feb 23, 2020

View reviewed changes

jarednorman approved these changes Feb 23, 2020

View reviewed changes

ericsaupe reviewed Feb 25, 2020

View reviewed changes

This was referenced Mar 13, 2020

Replace Paranoia methods with Discard equivalents #3554

Merged

DRY paranoia and discard inclusion into models #3555

Merged

kennyadsl added this to the 3.0.0 milestone Mar 24, 2020

cedum force-pushed the remove-paranoia branch 2 times, most recently from 7310037 to 2a1c21d Compare March 27, 2020 13:31

cedum changed the title ~~Remove paranoia~~ Discard Paranoia Mar 27, 2020

jarednorman approved these changes Mar 27, 2020

View reviewed changes

kennyadsl added the release:major Breaking change on hold until next major release label Apr 20, 2020

cedum force-pushed the remove-paranoia branch from 589249b to f73f89a Compare October 30, 2020 12:42

cedum mentioned this pull request Dec 4, 2020

Add soft-delete support for Api::ResourceController #3854

Merged

5 tasks

kennyadsl force-pushed the remove-paranoia branch from f73f89a to 949082b Compare January 28, 2021 14:52

cedum added 5 commits January 29, 2021 11:52

Don't include Paranoia for handling soft-deletions

466251e

We're replacing Paranoia with Discard as the only option for soft-deleting records.

Replace #really_destroy! with #destroy

4b58c26

Paranoia is being removed in favor of Discard. #destroy becomes the equivalent method of #really_destroy! for hard deleting records.

Remove paranoia_destroy from Admin resource controller

b881778

Paranoia has been completely removed. We don't need anymore to handle this case here.

Remove paranoia deprecations

a698f7e

Cleanup paranoia related deprecated methods and code comments.

Remove Paranoia gem from core dependencies

df54786

Paranoia is replaced with Discard for soft-deleting records.

kennyadsl force-pushed the remove-paranoia branch from 949082b to df54786 Compare January 29, 2021 10:54

kennyadsl approved these changes Jan 29, 2021

View reviewed changes

kennyadsl merged commit b0092d0 into solidusio:master Jan 29, 2021

kennyadsl deleted the remove-paranoia branch January 29, 2021 11:43

SamuelMartini added a commit to solidusio-contrib/solidus_sale_prices that referenced this pull request Feb 26, 2021

Use discard gem api

0aab6a1

solidusio/solidus#3488

SamuelMartini added a commit to solidusio-contrib/solidus_sale_prices that referenced this pull request Feb 26, 2021

Use discard gem api

2628619

solidusio/solidus#3488

kennyadsl removed the release:major Breaking change on hold until next major release label Apr 30, 2021

cpfergus1 pushed a commit to cpfergus1/solidus_sale_prices that referenced this pull request Jul 1, 2021

Use discard gem api

a97adc3

solidusio/solidus#3488

cpfergus1 pushed a commit to cpfergus1/solidus_sale_prices that referenced this pull request Jul 21, 2021

Use discard gem api

9f9e9cc

solidusio/solidus#3488

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Discard Paranoia #3488

Discard Paranoia #3488

cedum commented Jan 24, 2020 •

edited by kennyadsl

Loading

spaghetticode left a comment

spaghetticode Feb 21, 2020

spaghetticode Feb 21, 2020

jarednorman Feb 23, 2020

jarednorman Feb 23, 2020

jarednorman left a comment

ericsaupe left a comment

jarednorman left a comment

kennyadsl left a comment

Discard Paranoia #3488

Discard Paranoia #3488

Conversation

cedum commented Jan 24, 2020 • edited by kennyadsl Loading

Upgrade Notes

Class Methods

Instance Methods

Model Callbacks

spaghetticode left a comment

Choose a reason for hiding this comment

spaghetticode Feb 21, 2020

Choose a reason for hiding this comment

spaghetticode Feb 21, 2020

Choose a reason for hiding this comment

jarednorman Feb 23, 2020

Choose a reason for hiding this comment

jarednorman Feb 23, 2020

Choose a reason for hiding this comment

jarednorman left a comment

Choose a reason for hiding this comment

ericsaupe left a comment

Choose a reason for hiding this comment

jarednorman left a comment

Choose a reason for hiding this comment

kennyadsl left a comment

Choose a reason for hiding this comment

cedum commented Jan 24, 2020 •

edited by kennyadsl

Loading