Clean up some logic around subforms in RepositoryForm.
Review Request #10802 — Created Nov. 20, 2019 and updated
When validating repositories, we build a list of subforms that we're
operating on and check them for data or errors. This is done in two
locations, depending on whether the form has posted data, and some
upcoming work for SCMTool-provided forms would have doubled this logic.
This is ultimately pretty error-prone, and hid a bug where, if you
choose a bug tracker hosting service and then enable the "Use hosting
service's bug tracker" checkbox, the bug tracker form was still being
populated with data.
We now loop through all the bound subforms and use those, instead of
manually building a list of subforms. This is done through an
iter_subforms()method, which takes criteria on which subforms should
Unit tests take advantage of this method to check that we always work
with exactly the subforms we expect, so others don't slip through the
It also fixes up the
is_valid()logic, which was duplicating a check
unnecessarily for a valid subform. That now relies entirely on the
subforms_validstate we already calculate.
This will help keep things maintainable as we add SCMTool-specific
subforms. Note that we also aren't including authentication subforms by
default at this time, since they're treated pretty differently, but
we're now in a better position to change that going forward.
Unit tests pass.
Tested saving repositories for different hosting services and for
Tested saving bug tracker information in all bug tracker modes
(no bug tracker, custom bug tracker URL, hosting service bug tracker,
and with the "Use hosting service's bug tracker.")