Fix some bad ordering issues when setting state in RepositoryForm.

Review Request #10857 — Created Jan. 24, 2020 and submitted

Information

Review Board
release-3.0.x

Reviewers

RepositoryForm was setting some initial values for the form from an
instance a bit too late in the process, causing our hosting_type field
to be set after it was accessed. While this didn't show any regressions
on Django 1.6 (though it's possible there are some), it outright fails
on Django 1.11.

This change fixes the logic to set all the initial state earlier and in
one place, so it's more clear where similar code in the future should
go. It also adds comments to the methods responsible for setting the
state, indicating some requirements as to what state those functions are
allowed to access.

This all exposed an issue where an SCMTool form was being populated with
data when a hosting service was set. This doesn't cause any real-world
problems, but it was incorrect, and a bad unit test wasn't checking this
resulting state correctly. The above fix also fixed this, which broked
the bad unit test.

Both the unit test and another that was checking form values in a
non-future-proof way have been fixed.

Unit tests passed here and on release-4.0.x (with Django 1.11).

Summary ID
Fix some bad ordering issues when setting state in RepositoryForm.
`RepositoryForm` was setting some initial values for the form from an instance a bit too late in the process, causing our `hosting_type` field to be set after it was accessed. While this didn't show any regressions on Django 1.6 (though it's possible there are some), it outright fails on Django 1.11. This change fixes the logic to set all the initial state earlier and in one place, so it's more clear where similar code in the future should go. It also adds comments to the methods responsible for setting the state, indicating some requirements as to what state those functions are allowed to access. This all exposed an issue where an SCMTool form was being populated with data when a hosting service was set. This doesn't cause any real-world problems, but it was incorrect, and a bad unit test wasn't checking this resulting state correctly. The above fix also fixed this, which broked the bad unit test. Both the unit test and another that was checking form values in a non-future-proof way have been fixed.
13c06e35f8e1a4e97a9e129a2ab979bc7528d447
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (ffee3e6)
Loading...