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)
     
     
    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)
     
     
     
     
    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)
     
     
     
    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)
     
     
    Col: 80
     E501 line too long (96 > 79 characters)
    
  3. 
      
chipx86
  1. Ship It!
  2. 
      
david
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-1.6.x (5eec703).
Loading...