Clean up some logic around subforms in RepositoryForm.

Review Request #10802 — Created Nov. 20, 2019 and updated

chipx86
Review Board
release-3.0.x
1db79b7...
reviewboard

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
be returned.

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
cracks.

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_valid state 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
standalone repositories.

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.")

Checks run (2 succeeded)
flake8 passed.
JSHint passed.
david
  1. Ship It!
  2. 
      
Loading...