Confirmation screen not shown after validation error

[harris-irfan]

Steps to reproduce:

• Go to the admin change form
• Enter an invalid input and submit the form
• After being re-routed to the same form with the validation error, correct the validation error and submit the valid form (e.g. press the "Save" button).
• Confirmation screen will not be shown, rather default behaviour of changing a record is seen.

[TrangPham]

Hello! Thanks for reporting this issue and writing down the steps to recreate the problem.

May I ask where the validation is placed? Is it on the model field, on a model method override or on the AdminModel or on the ModelForm?

Thanks

On Fri, Apr 16, 2021 at 13:21 Harris Irfan @.***> wrote:

Steps to reproduce:

• Go to the admin change form of any model
• Enter an invalid input and submit the form
• After being re-routed to the same form with the validation error, correct the validation error and submit the valid form (e.g. press the "Save" button).
• Confirmation screen will not be shown, rather default behaviour of changing a record is seen.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/TrangPham/django-admin-confirm/issues/27, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQE5QIO3T36YDWTOPDZ34DTJCL3LANCNFSM43CFSAGQ .

[harris-irfan]

Hi @TrangPham, thanks for responding!

The validation is placed in model field. For example, a simple model field, which is by default a required field, if we do not provide its value, it will result in a validation error. If we resubmit the form after the validation error, we get the default behaviour and the confirm screen is not shown. I was able to debug the problem and find out the issue, and its solution. The issue lies in the changeform_view of the AdminConfirmMixin:

    @cache_control(private=True)
    def changeform_view(self, request, object_id=None, form_url="", extra_context=None):
        if request.method == "POST":
            if (not object_id and CONFIRM_ADD in request.POST) or (
                object_id and CONFIRM_CHANGE in request.POST
            ):
                cache.delete_many(CACHE_KEYS.values())
                return self._change_confirmation_view(
                    request, object_id, form_url, extra_context
                )
            elif CONFIRMATION_RECEIVED in request.POST:
                return self._confirmation_received_view(
                    request, object_id, form_url, extra_context
                )
            else:
                cache.delete_many(CACHE_KEYS.values())

        extra_context = {
            **(extra_context or {}),
            "confirm_add": self.confirm_add,
            "confirm_change": self.confirm_change,
        }
        return super().changeform_view(request, object_id, form_url, extra_context)

What happens is that when we submit an invalid form, it is a POST request, and in response a validation error occurs. Because of it being a POST request, extra context containing confirm_add and confirm_change is not populated (which is only populated in case of a GET request). The fix is to populate the extra context in the first conditional under the POST request as well, as shown below:

    @cache_control(private=True)
    def changeform_view(self, request, object_id=None, form_url="", extra_context=None):
        if request.method == "POST":
            if (not object_id and CONFIRM_ADD in request.POST) or (
                object_id and CONFIRM_CHANGE in request.POST
            ):
                extra_context = {
                    **(extra_context or {}),
                    "confirm_add": self.confirm_add,
                    "confirm_change": self.confirm_change,
                }
                cache.delete_many(CACHE_KEYS.values())
                return self._change_confirmation_view(
                    request, object_id, form_url, extra_context
                )
            elif CONFIRMATION_RECEIVED in request.POST:
                return self._confirmation_received_view(
                    request, object_id, form_url, extra_context
                )
            else:
                cache.delete_many(CACHE_KEYS.values())

        extra_context = {
            **(extra_context or {}),
            "confirm_add": self.confirm_add,
            "confirm_change": self.confirm_change,
        }
        return super().changeform_view(request, object_id, form_url, extra_context)

Obviously a cleaner approach would be to write a separate function to populate the context and use that to minimize code duplication.

[TrangPham]

Thanks Harris! I am in the process of adding integration tests for the validations. I also came to the same conclusion about the extra_context. Though debugging/fixing it is feeling like I'm playing whack a mole.

[TrangPham]

Affected Release: v0.2.3a0 and prior Fixed in: v0.2.4