• 
      

    Actually delete repositories through the webapi when they're empty.

    Review Request #3880 — Created Feb. 17, 2013 and submitted

    Information

    Review Board
    release-1.6.x

    Reviewers

    Actually delete repositories through the webapi when they're empty.
    
    If you delete a repository through the API, it doesn't actually delete it, it
    just "archives" it (sets it to be invisible and changes the name). The reasoning
    behind this is that we don't want to break any existing review requests.
    Unfortunately, if you then try to create a new repository with the same path, it
    will fail due to the unique_together constraints. This keeps hitting users of
    RBCommons, where the API is the primary way that repositories are
    managed--what's happening is that users create a repository and as they're
    trying to get things working, they delete it. They then recreate it and hit a
    constraint error.
    
    Instead of blindly archiving, we now only archive if we actually have review
    requests to preserve. This doesn't fix every possible case (maybe someone
    created a test review request and then deleted the repository) but it should
    handle most of the failures I've seen so far.
    
    A better fix would be to do constraint checking by hand and only test for
    uniqueness among visible repositories, but that involves schema changes.
    Ran unit tests. Verified that a DELETE call did what I expected.
    Description From Last Updated

    Alternatively, if the same path is used again, un-archive the review request.

    chipx86chipx86

    Should this be 'if not repository.review_requests.exists():' If not I'm seriously confusing the logic here, what am I not seeing?

    SM smacleod

    What Steven said. Also, can we get unit tests for the two cases?

    chipx86chipx86

    Col: 80 E501 line too long (96 > 79 characters)

    reviewbotreviewbot
    reviewbot
    1. This is a review from Review Bot.
        Tool: PEP8 Style Checker
        Processed Files:
          reviewboard/webapi/resources.py
          reviewboard/scmtools/models.py
        Ignored Files:
      
      
    2. 
        
    SM
    1. 
        
    2. reviewboard/webapi/resources.py (Diff revision 1)
       
       
      Show all issues
      Should this be 'if not repository.review_requests.exists():'
      
      If not I'm seriously confusing the logic here, what am I not seeing?
    3. 
        
    chipx86
    1. 
        
    2. reviewboard/scmtools/models.py (Diff revision 1)
       
       
       
       
      Show all issues
      Alternatively, if the same path is used again, un-archive the review request.
      1. That might be a better solution, but I struggled for a couple hours with ModelForm before I gave up.
    3. reviewboard/webapi/resources.py (Diff revision 1)
       
       
       
      Show all issues
      What Steven said. Also, can we get unit tests for the two cases?
    4. 
        
    david
    reviewbot
    1. This is a review from Review Bot.
        Tool: PEP8 Style Checker
        Processed Files:
          reviewboard/webapi/tests.py
          reviewboard/webapi/resources.py
          reviewboard/scmtools/models.py
        Ignored Files:
      
      
    2. reviewboard/webapi/tests.py (Diff revision 2)
       
       
      Show all issues
      Col: 80
       E501 line too long (96 > 79 characters)
      
    3. 
        
    chipx86
    1. Ship It!
    2. 
        
    david
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-1.6.x (5eec703).