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

Add history support for proxy models #154

Closed
wants to merge 3 commits into from
Closed

Add history support for proxy models #154

wants to merge 3 commits into from

Conversation

macro1
Copy link
Collaborator

@macro1 macro1 commented Feb 9, 2015

Somewhat based on arrai-innovations@9bb5400.

  • Test different proxy configurations (historical concrete with proxy, concrete with historical proxy, etc)

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.56% when pulling 48412ea on proxy-models into 38792a7 on master.

1 similar comment
@landscape-bot
Copy link

Code Health
Repository health decreased by 0.56% when pulling 48412ea on proxy-models into 38792a7 on master.

@u1735067
Copy link

u1735067 commented Feb 1, 2016

Is this still on the "roadmap" ?

@macro1
Copy link
Collaborator Author

macro1 commented Feb 3, 2016

Hi @Alex131089. I pulled in #112 first, as there was quite a bit more interest in allowing tracking to be inherited. From a data perspective, this support is a little odd though. Proxy models reuse the table the parent model used. If I find some time to think back through what should be happening here you'll see the progress on this branch.

Feel free to take a look at it as well of course. Pull requests are very welcome.

@u1735067
Copy link

u1735067 commented Feb 3, 2016

I don't know if it can help, but for my usage, I'm using this very ugly "hack"/workaround which seems to be working :
admin.py:

class HistoTrackedModel(TrackedModel):
    # /!\ Very ugly hack
    try:
        import sys
        if sys.argv[1] not in ('makemigrations',):
            raise Exception()
    except:
        history = HistoricalRecords(table_name='project_historicalTrackedModel')

    class Meta:
        proxy = True
        default_permissions = ('change',) # So the object appear

class HistoTrackedModelAdmin(ModelAdmin):
    readonly_fields = tuple((f.name for f in TrackedModel._meta.fields))

The idea here is to set an history object pointing to the historical table of the proxyfied model, except when running makemigrations because it would try to create another table for this proxy (which will not succed). No creation/modification/deletion are allowed for this proxy model, It's a readonly view of history (revert is possible & working though).
Sorry that I cannot help more, your plugin is giving a very easy way to offer history to a model :)

@luzfcb
Copy link
Contributor

luzfcb commented Mar 21, 2016

hello @macro1, I need this feature.

I tried to rebase to 1.8.1, but without success. it seems that the code is no longer compatible or maybe I did something wrong.

https://travis-ci.org/luzfcb/django-simple-history/builds/117542656

@kissgyorgy
Copy link

Guys, no offense, but do you even know what a proxy model is for? I suggest reading the official Django documentation: https://docs.djangoproject.com/en/1.10/topics/db/models/#proxy-models

Also if you want this "feature" because you want to track built-in models or third party models, you can already do that: https://django-simple-history.readthedocs.io/en/latest/advanced.html#history-for-a-third-party-model

This ticket should be closed IMO.

@macro1
Copy link
Collaborator Author

macro1 commented Jan 23, 2017

I've been somewhat on the fence myself about adding support for proxy models. Closing, both for simplicity of the existing registry logic, and because I can't imagine a good use for it.

@macro1 macro1 closed this Jan 23, 2017
@macro1 macro1 deleted the proxy-models branch January 23, 2017 22:32
@ThunderTrent
Copy link

I wanted to bring up a potential use case for this. I currently have an Account Data model that has several proxy models drawn from it in which all have various querysets/permissions restricting and changing various views for the django-admin. After a while of playing around with django-simple-history, everything seemed great -- but I realized later that on all of the proxy models, the saves aren't being tracked -- and the history gives an "'Options' object has no attribute 'simple_history_manager_attribute'" error.

Perhaps I'm using Proxy models incorrectly, but it sure would of been nice for it all to work out of the box.

@ThunderTrent
Copy link

Actually, I used:
register(workAccounts,table_name='web_historicalaccount')
for all the proxy models and referenced the original table and it seems to work.

@macro1
Copy link
Collaborator Author

macro1 commented Aug 1, 2017

@ThunderTrent: Would something like this solve it?
https://github.com/treyhunner/django-simple-history/compare/fix-proxy-signals

I think the original ask was the ability to register proxy models... But what you're looking for is to preserve tracking when the proxy interface is used.

@silviogutierrez
Copy link

@macro1 : that's exactly what we need for our use case. Basically, make sure history is saved even when using proxy models. If there's any way I can help to get this merged, please let me know.

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

Successfully merging this pull request may close these issues.

None yet

7 participants