Hand-check unique_together constraints with null values.

Review Request #5274 — Created Jan. 17, 2014 and submitted

Information

Review Board
release-1.7.x

Reviewers

Hand-check unique_together constraints with null values.

Django has a bug where unique_together constraints for foreign keys aren't
enforced properly when one of the foreign keys is null (well, technically it's
a database bug). This meant that users who weren't using local sites could
create Groups or Repositories that had conflicting names and paths.

This change implements the clean() method for the Group and Repository
models. This method is automatically called from ModelForms, which means the
admin site is fixed automatically.

The web API has a similar issue, but only for repositories. The Group
create/update methods had an implementation which already checked for these
sorts of problems. I haven't implemented a unit test for the repository API
changes because this change is currently on 1.7.x, but I'll add it to the to-do
list for master.

  • Tried to create a Group with a conflicting name from the admin UI.
  • Tried to create a Repository with a conflicting path from the admin UI.
  • Tried to create a Repository with a conflicting name from the admin UI.
  • Tried to create a Repository with a conflicting path from the web API.
  • Tried to create a Repository with a conflicting name from the web API.
  • Ran unit tests.
Description From Last Updated

"foreign"

chipx86chipx86

Can you put a blank line between these?

chipx86chipx86

"foreign"

chipx86chipx86

This should probably go inside the conditional.

chipx86chipx86

Blank line between these? Helps with readability, as they're both independent checks.

chipx86chipx86
chipx86
  1. 
      
  2. reviewboard/reviews/models.py (Diff revision 1)
     
     
    Show all issues

    "foreign"

  3. reviewboard/reviews/models.py (Diff revision 1)
     
     
     
    Show all issues

    Can you put a blank line between these?

  4. reviewboard/scmtools/models.py (Diff revision 1)
     
     
    Show all issues

    "foreign"

  5. reviewboard/scmtools/models.py (Diff revision 1)
     
     
    Show all issues

    This should probably go inside the conditional.

  6. reviewboard/scmtools/models.py (Diff revision 1)
     
     
     
    Show all issues

    Blank line between these? Helps with readability, as they're both independent checks.

  7. 
      
david
chipx86
  1. Ship It!
  2. 
      
david
Review request changed
Status:
Completed
Change Summary:
Pushed to release-1.7.x (3edb765).