Fix the repository configuration form on Django 1.11.

Review Request #10874 — Created Feb. 2, 2020 and submitted

chipx86
Review Board
release-4.0.x
reviewboard

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 off required attributes for fields by setting
use_required_attribute on RepositoryForm 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 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
for QueryDict, 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
Fix the repository configuration form on Django 1.11.
Description From Last Updated

I think this would be more appropriate as $('#repository_form').prop('novalidate', true). Can we add a comment explaining this?

daviddavid
david
  1. 
      
  2. I think this would be more appropriate as $('#repository_form').prop('novalidate', true).

    Can we add a comment explaining this?

    1. Going a different route on the fix.

  3. 
      
chipx86
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-4.0.x (8e6db54)
Loading...