Fix the repository configuration form on Django 1.11.
Review Request #10874 — Created Feb. 2, 2020 and submitted
The repository configuration form had a couple of breakages due to
changes made in Django since 1.6. There were two main issues:
Client-side validation of hidden subforms (for hosting services and
SCMTools that aren't selected) and server-side attempts at setting an
immutable dictionary.The client-side form validation issue is going to be common amongst some
other forms in our codebase (such as the authentication settings form),
and a proper fix is a larger project, but to get things unstuck we're
now turning offrequired
attributes for fields by setting
use_required_attribute
onRepositoryForm
and the subforms. A better
solution going forward would involve swapping out any fields that are
hidden.The server-side issues stem from our need to set certain values in the
POSTed form data while cleaning the form. Typically, there's no need to
ever set this data, and on modern versions of Django the form's
dictionary is generally immutable (as it comes fromrequest.POST
,
which is immutable on 1.11).
RepositoryForm
is more complex, since it can fail to save a repository
but still successfully create intermediary objects. For instance, you
can successfully link an account but not validate a repository. In these
cases, we have certain fields that we want set back in the form data so
that they'll appear as selected values when next rendering the form.We now check to see if we're using a
MultiValueDict
(the base class
forQueryDict
, which holds POST data), and if so we convert it to a
standard dictionary so we can write to it.This should fix the form's behavior when running on Django 1.11.
Unit tests pass.
Successfully created repositories through the admin UI.
Summary | ID |
---|---|
5ae20f28e5a1551df9f810153853f7383fbbe646 |
Description | From | Last Updated |
---|---|---|
I think this would be more appropriate as $('#repository_form').prop('novalidate', true). Can we add a comment explaining this? |
david |
- Change Summary:
-
- Removed the
novalidate
attribute on the<form>
and instead setuse_required_attribute = False
on the form subclasses. - Added a missing change to the test hosting service needed for triggering a
RepositoryError
during validation.
- Removed the
- Description:
-
The repository configuration form had a couple of breakages due to
changes made in Django since 1.6. There were two main issues: Client-side validation of hidden subforms (for hosting services and SCMTools that aren't selected) and server-side attempts at setting an immutable dictionary. The client-side form validation issue is going to be common amongst some
other forms in our codebase (such as the authentication settings form), and a proper fix is a larger project, but to get things unstuck we're ~ now setting novalidate
on the<form>
. This is currently done~ dynamically, and is a temporary fix until a more general solution is ~ implemented. ~ now turning off required
attributes for fields by setting~ use_required_attribute
onRepositoryForm
and the subforms. A better~ solution going forward would involve swapping out any fields that are + hidden. The server-side issues stem from our need to set certain values in the
POSTed form data while cleaning the form. Typically, there's no need to ever set this data, and on modern versions of Django the form's dictionary is generally immutable (as it comes from request.POST
,which is immutable on 1.11). RepositoryForm
is more complex, since it can fail to save a repositorybut still successfully create intermediary objects. For instance, you can successfully link an account but not validate a repository. In these cases, we have certain fields that we want set back in the form data so that they'll appear as selected values when next rendering the form. We now check to see if we're using a
MultiValueDict
(the base classfor QueryDict
, which holds POST data), and if so we convert it to astandard dictionary so we can write to it. This should fix the form's behavior when running on Django 1.11.
- Commits:
-
Summary ID f560e551ad163c04a5e6aad3db421f6aead503ec 5ae20f28e5a1551df9f810153853f7383fbbe646