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 support for admin inlines #315

Conversation

mathieuhentges
Copy link

The inlines couldn't be displayed in the admin form view.

I added the 'inline_admin_formsets' entry in context to make them available.

@codecov-io
Copy link

codecov-io commented Sep 28, 2017

Codecov Report

Merging #315 into master will increase coverage by 0.09%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #315      +/-   ##
==========================================
+ Coverage   97.62%   97.72%   +0.09%     
==========================================
  Files          15       15              
  Lines         590      615      +25     
  Branches       72       77       +5     
==========================================
+ Hits          576      601      +25     
  Misses          7        7              
  Partials        7        7
Impacted Files Coverage Δ
simple_history/admin.py 98.42% <100%> (+0.38%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b01b92c...0da904f. Read the comment docs.

@rossmechanic
Copy link
Collaborator

@mathieuhentges Do you mind attaching screenshots to show what this looks like after the changes? Thanks. Also please fix merge conflicts.

@rossmechanic
Copy link
Collaborator

@mathieuhentges Any updates? Or would you like me to take this over?

@rossmechanic
Copy link
Collaborator

Closing this since I haven't heard from @mathieuhentges

@merwok
Copy link

merwok commented Sep 25, 2018

@rossmechanic were you able to take this over?

I think this could be quite useful!

@rossmechanic
Copy link
Collaborator

rossmechanic commented Sep 25, 2018

@merwok Haven't been able to get around to it yet. I wanted to make it easy to run a sample app within the repo (so it would be easy to test admin features). Once I get that done, it'll be really easy to knock out a bunch of the bugs/features we want on the admin side. Feel free to work on this if you'd like though. I'd be happy to accept a PR

@merwok merwok mentioned this pull request Sep 25, 2018
@merwok
Copy link

merwok commented Sep 25, 2018

I’ll poke my co-worker Mathieu or see if I find the time myself!

I opened a ticket so that this doesn’t get forgotten.

@nickvellios
Copy link

This PR does work to allow inlines to be displayed in the history detail, however it is always the current state of the related models, not the state they were in when saving. In many cases, this is not a true representation of the state of this snapshot as you would have saved it in the admin.

Is there currently a branch working to address this?

@rossmechanic
Copy link
Collaborator

Nothing working to currently address this @nickvellios, but I'd be happy to discuss a possible solution

@nickvellios
Copy link

nickvellios commented Jan 15, 2019

This is something I quickly hacked together based off of this PR just to see how close I could get to a solution. It is far from ideal because there is no way to be sure the 5 second fuzzy history_date filter will be accurate. For example, if something hangs between saving the models, or if multiple saves take place within a 5 second period (multiple users or API). It also doesn't respect deletes and additions if the number of related objects in the inline formset has changed.

I'm struggling to find a way to accurately represent the historical state of related objects with how the historical models are currently structured.

historical_obj = get_object_or_404(model, **{
    original_opts.pk.attname: object_id,
    'history_id': version_id,
})
historical_date = historical_obj.history_date
adjusted_historical_date = historical_date + datetime.timedelta(seconds=5)
for FormSet, inline in self.get_admin_formsets_with_inline(
        *[request]):
    prefix = FormSet.get_default_prefix()
    prefixes[prefix] = prefixes.get(prefix, 0) + 1
    if prefixes[prefix] != 1 or not prefix:
        prefix = "%s-%s" % (prefix, prefixes[prefix])
    inline_qs = inline.get_queryset(request)
    inline_ids = inline_qs.values_list('id', flat=True)
    history_inline_model = inline_qs.first().history.model
    historical_ids = history_inline_model.objects\
                            .filter(id__in=inline_ids, history_date__lte=adjusted_historical_date)\
                            .order_by('-history_date')[:inline_qs.count()]\
                            .values_list('history_id', flat=True)
    historical_queryset = history_inline_model.objects.filter(history_id__in=historical_ids)
    formset_params = {
        'instance': obj,
        'prefix': prefix,
        'queryset': historical_queryset,
    }
    if request.method == 'POST':
        formset_params.update({
            'data': request.POST.copy(),
            'files': request.FILES,
            'save_as_new': '_saveasnew' in request.POST
        })
    formset.append(FormSet(**formset_params))

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.

5 participants