AttributeError: 'HttpResponseRedirect' object has no attribute 'render'

[PavelPancocha]

Hi! First thank you for all the work on this awesome tool.

We encountered one problem, that we think should be addressed directly in django-reversion:

It is all about the "recover" view for a deleted object, let's take a look at this part:
https://github.com/etianen/django-reversion/blob/master/reversion/admin.py#L169-L184

        try:
            with transaction.atomic(using=version.db):
                # Revert the revision.
                version.revision.revert(delete=True)
                # Run the normal changeform view.
                with self.create_revision(request):
                    response = self.changeform_view(request, quote(version.object_id), request.path, extra_context)
                    # Decide on whether the keep the changes.
                    if request.method == "POST" and response.status_code == 302:
                        set_comment(_("Reverted to previous version, saved on %(datetime)s") % {
                            "datetime": localize(template_localtime(version.revision.date_created)),
                        })
                    else:
                        response.template_name = template_name  # Set the template name to the correct template.
                        response.render()  # Eagerly render the response, so it's using the latest version.
                        raise _RollBackRevisionView(response)  # Raise exception to undo the transaction and revision.

We do revert and get the response.

But which response? If you use the custom "get_queryset" it could happen, that you will not be able to get the given "restored" object. Why? There can be some different constraints that fire up when the transaction is committed, which doesn't happen here. But some other queries then can return "unexpected" results. Or expected, because in fact when the object would not be able to be committed, the query is also not able to return it. Let's show an example:

Eg. you can use select_related, which will try to do the select on the user:

class Admission(Model):
    user = models.ForeignKey(User, on_delete=models.PROTECT)
    location = models.ForeignKey(Location, on_delete=models.PROTECT)
    valid_from = models.DateTimeField()
    valid_to = models.DateTimeField()
    ...

    class Meta:
        unique_together = ("user", "location")


class AdmissionAdmin(ModelAdmin):
    ...

    def get_queryset(self, request: HttpRequest) -> QuerySet:
        qs = super().get_queryset(request)
        return qs.select_related("user")

If it is reversible (no other admission for the given user and location exists):

>>> with transaction.atomic():
...     ver.revision.revert(delete=True)
...     admission_qs = Admission.objects.filter(pk=16389)
...     print(admission_qs)
...     admission_qs_sel = admission_qs.select_related("user")
...     print(admission_qs_sel)
...     admission_qs_ann = admission_qs_sel.annotate_user_chipcard_status()
...     print(admission_qs_ann)
...     raise Exception("revert!")
...
<AdmissionSet [<Admission: Admission: 16389>]>
<AdmissionSet [<Admission: Admission: 16389>]>
<AdmissionSet [<Admission: Admission: 16389>]>

When you commit the ver.revision.revert(delete=True) transaction, it will get processed.

If it is not reversible (another admission for a given user and location exists):

>>> with transaction.atomic():                             
...     ver.revision.revert(delete=True)                   
...     admission_qs = Admission.objects.filter(pk=16198)    
...     print(admission_qs)
...     admission_qs_sel = admission_qs.select_related("user")
...     print(admission_qs_sel)
...     admission_qs_ann = admission_qs_sel.annotate_user_chipcard_status()
...     print(admission_qs_ann)
...     raise Exception("revert!")
... 
<AdmissionSet [<Admission: Admission: 16198>]>
<AdmissionSet []>
<AdmissionSet []>

When you commit the ver.revision.revert(delete=True) transaction, it will fail with IntegrityError

Back to our problem: If the object cannot be found (see above), Django will redirect you to /admin/ with a user message that the given object does not exist. So the HttpResponse is not a response but HttpResponseRedirect, which cannot be rendered.

I would propose to check the status code for the response and if it is not HTTP 200 (OK), raise RevertError("Could not load %(object_repr)s version") or similar.

If this proposal is accepted, I can create a PR for this.

Thank you for your time and consideration.

[etianen]

Sorry it took me so long to get back to you. I've had a baby and it's taken up a lot of my time! 👶

Please take a look at #946 and let me know if it fixes your problem! ❤️

[PavelPancocha]

@etianen that should, I will test it when I will get to it. Thank you! Btw: Congratulations 🎉 👶🏻

[etianen]

Released as v5.0.5

[etianen]

What status code is given on error? Is it 404?

See discussion in #948