Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SafeDeleteMixin is not a mixin #55

Closed
AndreasBackx opened this issue Dec 21, 2016 · 6 comments
Closed

SafeDeleteMixin is not a mixin #55

AndreasBackx opened this issue Dec 21, 2016 · 6 comments

Comments

@AndreasBackx
Copy link
Contributor

The name SafeDeleteMixin seems to say that the class is a Mixin when it's not because it inherits from django.db.models. Wouldn't SafeDeleteModelbe a better name in this case? It clears up some confusion regarding inheritance.

@Gagaro
Copy link
Member

Gagaro commented Dec 22, 2016

I agree. SafeDeleteMixin should be deprecated in the 0.4.x (it's not released yet but I'm not sure it is not used yet).

@thenewguy
Copy link

I believe the previous way to use the mixin was like this:

from safedelete.models import safedelete_mixin_factory, SOFT_DELETE
SoftDeleteMixin = safedelete_mixin_factory(SOFT_DELETE)

I just got updated by unpinned dependencies and hit an issue where safedelete_mixin_factory is no longer included at safedelete.models.safedelete_mixin_factory but there are no docs that indicate the correct way to migrate from this pattern

@Iftahh
Copy link

Iftahh commented Jan 19, 2017

@thenewguy did you find a solution? I hit the same issue.

I got the Mixin replacement:

  class SafeDeleteMixin(safedelete.models.SafeDeleteMixin):
    _safedelete_policy = SOFT_DELETE   # replace by whatever you want
    _safedelete_visibility = DELETED_VISIBLE_BY_FIELD  # replace by whatever you want
    
    class Meta:
        abstract = True

but the next problem I faced is making the database migration - the deleted field changed from BooleanField to DateTimeField which cannot be automatically populated.

@AndreasBackx
Copy link
Contributor Author

@thenewguy the example is pretty clear on how to use the new "Mixin". Inherit from SafeDeleteMixin.

@Iftahh It should be easy to populate from BooleanField to DateTimeField. If true, set the DateTimeField to the current time, else set it to None. There should've been something in the docs about moving from 0.3.x to 0.4.x in my opinion. Could we perhaps add this in the README @Gagaro?

@Gagaro
Copy link
Member

Gagaro commented Jan 19, 2017

Yes indeed I completely forgot about documenting the migration. Sorry about that.

I'm really busy these days, I'll do that ASAP but I don't know when it will be. You can pin on 3.5 in the meantime.

@Iftahh
Copy link

Iftahh commented Jan 23, 2017

@AndreasBackx yes, the logic is simple but still its not as easy as "normal" Django migrations - you have to add a column of type datetime (eg. deleted_temp) populate it, remove the old deleted and rename the deleted_temp to deleted.
Still after that was done I had different unit tests failures that I decided to just revert the database and freeze version to 0.3.2.

wli added a commit to wli/django-safedelete that referenced this issue Feb 17, 2017
wli added a commit to wli/django-safedelete that referenced this issue Feb 17, 2017
wli added a commit to wli/django-safedelete that referenced this issue Feb 20, 2017
wli added a commit to wli/django-safedelete that referenced this issue Feb 21, 2017
Gagaro pushed a commit that referenced this issue Feb 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants