Proposal to solve #118 #119
Conversation
Undeletes only the related models that were deleted in the same time. This assumes that the model and all related were deleted in a cascade operation that lasted less than 60 seconds. (Maybe this should be in a configuration in settings.py?) Signed-off-by: Vinicius Cubas Brand <[email protected]>
There was a problem hiding this comment.
Code looks fine, though some more thought needs to be given whether this breaking change should be made in this way. There are reasons only undeleting all of them and only those who were also deleted with it. I would add a parameter to the undelete method that indicates you want this behaviour, instead of making it the default and making it a breaking change.
| ) | ||
|
|
||
| with patch.object(timezone, 'now', return_value=datetime.datetime(2012, 12, 21)) as mock_now: |
There was a problem hiding this comment.
Remove the mock_now as it's unused. See the travis build log, which tests for pyflakes.
There was a problem hiding this comment.
I'd also separate this patch into a test to test for the behaviour you changed. Reason being is that you'd have to keep this in mind for all of the tests. Best to test for this separately each time imo.
|
I don't like adding this as it is, there will be race conditons with this. Imho we should: add a setting/parameter to block this behaviour (undeleting related objects) and document this issue. |
|
@Gagaro could you give some more insight in what race conditions might occur? What would you propose as a parameter/setting then? I'm not sure what your plan is. |
|
Let's say we have an object User A delete Then, User B delete |
|
You're right, I didn't think about multiple users. Perhaps we could provide some kind of functionality or leave it for others to propose on how to make this library more extendible in different ways? So for now perhaps out of scope. |
Gagaro
force-pushed
the
master
branch
2 times, most recently
from
89d601d
to
734b1e9
Compare
Gagaro
force-pushed
the
master
branch
2 times, most recently
from
7a52a2a
to
0a0daba
Compare
Undeletes only the related models that were deleted in the same time.
This assumes that the model and all related were deleted in a cascade
operation that lasted less than 60 seconds. (Maybe this should be in a
configuration in settings.py?)
Signed-off-by: Vinicius Cubas Brand [email protected]