• 
      

    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:
    Completed
    Change Summary:
    Pushed to release-3.0.x (ffee3e6)