Summary

Auto-populating branch for new post-commit review requests

Review Request #7659 — Created Sept. 25, 2015 and submitted May 2, 2016, 4:16 p.m.

Information

Owner

ahache

Repository

Review Board

Branch

release-2.5.x

Bugs

3935

Depends On

Reviewers

Groups

reviewboard

People

Description

When creating a new post-commit review request, the branch name was left blank rather than being pre-populated. Since we have the data available, this should be populated.
Added branch info to the request url when creating the new ReviewRequest, webAPI takes care of the rest.

Corrected API breaking issue. When no branch was provided in API request, None would be attempt to be saved to the database but branch had a Not Null constraint.
Added additional tests to cover this case.

Testing Done

Ran all reviewboard tests.
Added new API tests for post with and without branch info.

Issues

Description	From	Last Updated
I would prefer if we just provided the branch: this.model.collection.options.branch key-value-pair in the createFromCommit call. Its cleaner. Also, the only …	brennie	Sept. 26, 2015, 1:16 p.m.
Context is the this object that the callbacks will be bound to. You don't really want to be modifying it.	brennie	Sept. 26, 2015, 12:41 p.m.
'pdb' imported but unused	reviewbot	Sept. 26, 2015, 12:40 p.m.
'pdb' imported but unused	reviewbot	Sept. 26, 2015, 12:40 p.m.
Col: 9 E265 block comment should start with '# '	reviewbot	Sept. 26, 2015, 12:40 p.m.
redefinition of unused 'pdb' from line 4	reviewbot	Sept. 26, 2015, 12:40 p.m.
Col: 23 E231 missing whitespace after ';'	reviewbot	Sept. 26, 2015, 12:40 p.m.
Col: 23 E702 multiple statements on one line (semicolon)	reviewbot	Sept. 26, 2015, 12:40 p.m.
Col: 39 E703 statement ends with a semicolon	reviewbot	Sept. 26, 2015, 12:40 p.m.
Col: 13 E122 continuation line missing indentation or outdented	reviewbot	Sept. 26, 2015, 12:44 p.m.
Col: 13 E122 continuation line missing indentation or outdented	reviewbot	Sept. 26, 2015, 12:44 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Sept. 26, 2015, 12:47 p.m.
Col: 28 E131 continuation line unaligned for hanging indent	reviewbot	Sept. 26, 2015, 12:47 p.m.
Do you mean as a keyword parameter for the create function? Because that's already the case. Is this suggestion so …	AH ahache	Sept. 28, 2015, 8:30 a.m.
You can add branch=None as a keyword argument to create.	brennie	Sept. 28, 2015, 1 p.m.
Col: 34 E225 missing whitespace around operator	reviewbot	Oct. 9, 2015, 7:35 a.m.
Col: 41 E703 statement ends with a semicolon	reviewbot	Oct. 9, 2015, 7:35 a.m.
You should be able to just include branch=branch, in the self.model() call above.	david	Oct. 15, 2015, 6:03 a.m.
To keep this consistent with the repository, please use branch = this.model.get('branch') and then pass in branch.id below. Although, perhaps …	david	Oct. 15, 2015, 6:18 a.m.
Remove this commented out code.	brennie	Nov. 4, 2015, 11:22 a.m.
No blank line here.	brennie	Nov. 4, 2015, 11:22 a.m.
Can you format this as: review_request = ReviewRequest.objects.get( pkg=rsp['review_request']['id'])	brennie	Nov. 4, 2015, 11:22 a.m.
Can you put this on the previous line?	brennie	Nov. 4, 2015, 11:22 a.m.
Can you format this as: review_request = ReviewRequest.objects.get( pkg=rsp['review_request']['id'])	brennie	Nov. 4, 2015, 11:23 a.m.

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/webapi/resources/repository_commits.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/repositoryCommitModel.js
    reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
    reviewboard/static/rb/js/resources/models/baseResourceModel.js
    reviewboard/static/rb/js/resources/collections/repositoryCommitsCollection.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js
    reviewboard/static/rb/js/newReviewRequest/views/commitView.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/webapi/resources/repository_commits.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/repositoryCommitModel.js
    reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
    reviewboard/static/rb/js/resources/models/baseResourceModel.js
    reviewboard/static/rb/js/resources/collections/repositoryCommitsCollection.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js
    reviewboard/static/rb/js/newReviewRequest/views/commitView.js

reviewboard/webapi/resources/repository_commits.py (Diff revision 1)
The issue has been dropped. Show all issues
```
 'pdb' imported but unused
```
reviewboard/webapi/resources/review_request.py (Diff revision 1)
The issue has been resolved. Show all issues
```
 'pdb' imported but unused
```
reviewboard/webapi/resources/review_request.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 9
 E265 block comment should start with '# '
```
reviewboard/webapi/resources/review_request.py (Diff revision 1)
The issue has been resolved. Show all issues
```
 redefinition of unused 'pdb' from line 4
```
reviewboard/webapi/resources/review_request.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 23
 E231 missing whitespace after ';'
```
reviewboard/webapi/resources/review_request.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 23
 E702 multiple statements on one line (semicolon)
```
reviewboard/webapi/resources/review_request.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 39
 E703 statement ends with a semicolon
```

reviewboard/static/rb/js/newReviewRequest/views/commitView.js (Diff revision 1)

The issue has been resolved. Show all issues

I would prefer if we just provided the branch: this.model.collection.options.branch key-value-pair in the createFromCommit call. Its cleaner.

Also, the only time the branch will be set is when creating it via the post-commit UI. In that case, it makes sense to set() the data in reviewRequestModel.js:createFromCommit.

ahache Sept. 26, 2015, 12:40 p.m.

Seems that we don't have the collection anymore at that point "Uncaught TypeError: Cannot read property 'collection' of undefined".
And if I try doing it in _onCreateReviewRequest in postCommitView.js I get the error on options instead "Uncaught TypeError: Cannot read property 'options' of undefined" (even though collection itself is undefined there".

reviewboard/static/rb/js/resources/models/reviewRequestModel.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Context is the this object that the callbacks will be bound to. You don't really want to be modifying it.
```

Summary:

WIP: Trying to auto-populate branch name when creating a post-commit review request

Auto-populating branch for new post-commit review requests

Description:

~		WIP: Trying to auto-populate branch name when creating a post-commit review request
	~	Auto-populating branch for new post-commit review requests by adding branch field to the repositoryCommitsCollection. Passing the data along when creating a new ReviewRequest from an existing commit by modifiy the toJSON method in reviewRequestModel.js, and webAPI takes care of the rest.

Diff:

Revision 2 (+28 -5)

Show changes

	reviewboard/reviews/managers.py
	reviewboard/static/rb/js/newReviewRequest/views/commitView.js
	reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
	reviewboard/static/rb/js/resources/collections/repositoryCommitsCollection.js
	reviewboard/static/rb/js/resources/models/baseResourceModel.js
	reviewboard/static/rb/js/resources/models/repositoryCommitModel.js
	reviewboard/static/rb/js/resources/models/reviewRequestModel.js
	reviewboard/webapi/resources/review_request.py

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/reviews/managers.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/repositoryCommitModel.js
    reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
    reviewboard/static/rb/js/resources/models/baseResourceModel.js
    reviewboard/static/rb/js/resources/collections/repositoryCommitsCollection.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js
    reviewboard/static/rb/js/newReviewRequest/views/commitView.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/reviews/managers.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/repositoryCommitModel.js
    reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
    reviewboard/static/rb/js/resources/models/baseResourceModel.js
    reviewboard/static/rb/js/resources/collections/repositoryCommitsCollection.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js
    reviewboard/static/rb/js/newReviewRequest/views/commitView.js

reviewboard/webapi/resources/review_request.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Col: 13
 E122 continuation line missing indentation or outdented
```
reviewboard/webapi/resources/review_request.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Col: 13
 E122 continuation line missing indentation or outdented
```

Diff:

Revision 3 (+29 -5)

Show changes

	reviewboard/reviews/managers.py
	reviewboard/static/rb/js/newReviewRequest/views/commitView.js
	reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
	reviewboard/static/rb/js/resources/collections/repositoryCommitsCollection.js
	reviewboard/static/rb/js/resources/models/baseResourceModel.js
	reviewboard/static/rb/js/resources/models/repositoryCommitModel.js
	reviewboard/static/rb/js/resources/models/reviewRequestModel.js
	reviewboard/webapi/resources/review_request.py

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/reviews/managers.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/repositoryCommitModel.js
    reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
    reviewboard/static/rb/js/resources/models/baseResourceModel.js
    reviewboard/static/rb/js/resources/collections/repositoryCommitsCollection.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js
    reviewboard/static/rb/js/newReviewRequest/views/commitView.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/reviews/managers.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/repositoryCommitModel.js
    reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
    reviewboard/static/rb/js/resources/models/baseResourceModel.js
    reviewboard/static/rb/js/resources/collections/repositoryCommitsCollection.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js
    reviewboard/static/rb/js/newReviewRequest/views/commitView.js

reviewboard/webapi/resources/review_request.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
reviewboard/webapi/resources/review_request.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Col: 28
 E131 continuation line unaligned for hanging indent
```

Testing Done:: +
Testing still to come, going to dig around to find how to write appropriate tests cases for this. Any suggestions are welcome.

Diff:

Revision 4 (+29 -5)

Show changes

	reviewboard/reviews/managers.py
	reviewboard/static/rb/js/newReviewRequest/views/commitView.js
	reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
	reviewboard/static/rb/js/resources/collections/repositoryCommitsCollection.js
	reviewboard/static/rb/js/resources/models/baseResourceModel.js
	reviewboard/static/rb/js/resources/models/repositoryCommitModel.js
	reviewboard/static/rb/js/resources/models/reviewRequestModel.js
	reviewboard/webapi/resources/review_request.py

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/reviews/managers.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/repositoryCommitModel.js
    reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
    reviewboard/static/rb/js/resources/models/baseResourceModel.js
    reviewboard/static/rb/js/resources/collections/repositoryCommitsCollection.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js
    reviewboard/static/rb/js/newReviewRequest/views/commitView.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/reviews/managers.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/repositoryCommitModel.js
    reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
    reviewboard/static/rb/js/resources/models/baseResourceModel.js
    reviewboard/static/rb/js/resources/collections/repositoryCommitsCollection.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js
    reviewboard/static/rb/js/newReviewRequest/views/commitView.js

Diff:

Revision 5 (+26 -4)

Show changes

	reviewboard/reviews/managers.py
	reviewboard/static/rb/js/newReviewRequest/views/commitView.js
	reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
	reviewboard/static/rb/js/resources/models/reviewRequestModel.js
	reviewboard/webapi/resources/review_request.py

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/reviews/managers.py

Ignored Files:
    reviewboard/static/rb/js/newReviewRequest/views/commitView.js
    reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js



Tool: Pyflakes
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/reviews/managers.py

Ignored Files:
    reviewboard/static/rb/js/newReviewRequest/views/commitView.js
    reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js

Diff:

Revision 6 (+20 -4)

Show changes

	reviewboard/reviews/managers.py
	reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
	reviewboard/static/rb/js/resources/models/reviewRequestModel.js
	reviewboard/webapi/resources/review_request.py

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/reviews/managers.py

Ignored Files:
    reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/reviews/managers.py

Ignored Files:
    reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js

Can you add the bug number to the bugs field?

reviewboard/webapi/resources/review_request.py (Diff revision 6)

The issue has been resolved. Show all issues

You can add branch=None as a keyword argument to create.

ahache Sept. 28, 2015, 8:30 a.m.

Do you mean as a keyword parameter for the create function? Because that's already the case. Is this suggestion so that kwargs.get('branch', None) becomes kwargs.get('branch') ? If so, I had it that way at first but then changed it to have the default None value incase an API call was made to create the post-commit RR without passing branch information.

brennie Sept. 28, 2015, 11:22 a.m.

If you have def create(..., branch=None, ...) then you don't need to do branch = kwargs.get(branch).
It currently isnt the case beause the signaure is:


    def create(self, request, repository=None, submit_as=None, changenum=None,
               commit_id=None, local_site_name=None,
               create_from_commit_id=False, extra_fields={}, *args, **kwargs):


You need to add branch=None to that before extra_fields={}

ahache Sept. 28, 2015, 12:45 p.m.

Ohh I see, I was thinking of the create function in managers.py for some reason. Will do!

Bugs:: 3935

Description:: ~
Auto-populating branch for new post-commit review requests by adding branch field to the repositoryCommitsCollection. Passing the data along when creating a new ReviewRequest from an existing commit by modifiy the toJSON method in reviewRequestModel.js, and webAPI takes care of the rest.
~
Auto-populating branch for new post-commit review requests. Added branch info to _onCreateReviewRequest in postCommitView.js before redirecting to the new RR, modified the toJSON method in reviewRequestModel.js, and added a branch parameter to the RR create method in managers.py. webAPI takes care of the rest.

reviewboard/webapi/resources/review_request.py (Diff revision 6)

The issue has been dropped. Show all issues

Diff:

Revision 7 (+21 -5)

Show changes

	reviewboard/reviews/managers.py
	reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
	reviewboard/static/rb/js/resources/models/reviewRequestModel.js
	reviewboard/webapi/resources/review_request.py

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/reviews/managers.py

Ignored Files:
    reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/reviews/managers.py

Ignored Files:
    reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js

The description field doesn't need to describe exactly what changed in the code -- thats what the diff is for :)

Instead, it should outline the problem that this patch fixes and how it is solved (in a very high-level way).

Otherwise this patch looks great! Clean that up and we'll get it landed :)

ahache Sept. 30, 2015, 10:54 a.m.

Awesome! I was unsure if any new tests would be needed for this change. After running the entire test suite, a few tests were failing based on the fact that the branch field was defaulted to None when no branch info was found, and that RR's have a Not Null constraint for the branch. I moved a line or two around to correct this.

Description:: ~
Auto-populating branch for new post-commit review requests. Added branch info to _onCreateReviewRequest in postCommitView.js before redirecting to the new RR, modified the toJSON method in reviewRequestModel.js, and added a branch parameter to the RR create method in managers.py. webAPI takes care of the rest.
~
Auto-populating branch for new post-commit review requests. Added branch info to the request url when creating the new RR, webAPI takes care of the rest.
Testing Done:: ~
Testing still to come, going to dig around to find how to write appropriate tests cases for this. Any suggestions are welcome.
~
Ran all reviewboard tests.

Description:: ~
Auto-populating branch for new post-commit review requests. Added branch info to the request url when creating the new RR, webAPI takes care of the rest.
~
When creating a new post-commit review request, the branch name was left blank rather than being pre-populated. Since we have the data available, this should be populated.
+ Added branch info to the request url when creating the new ReviewRequest, webAPI takes care of the rest.

Diff:

Revision 8 (+23 -5)

Show changes

	reviewboard/reviews/managers.py
	reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
	reviewboard/static/rb/js/resources/models/reviewRequestModel.js
	reviewboard/webapi/resources/review_request.py

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/reviews/managers.py

Ignored Files:
    reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/reviews/managers.py

Ignored Files:
    reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js

reviewboard/reviews/managers.py (Diff revision 8)
The issue has been resolved. Show all issues
```
Col: 34
 E225 missing whitespace around operator
```
reviewboard/reviews/managers.py (Diff revision 8)
The issue has been resolved. Show all issues
```
Col: 41
 E703 statement ends with a semicolon
```

Diff:

Revision 9 (+23 -5)

Show changes

	reviewboard/reviews/managers.py
	reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
	reviewboard/static/rb/js/resources/models/reviewRequestModel.js
	reviewboard/webapi/resources/review_request.py

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/reviews/managers.py

Ignored Files:
    reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/reviews/managers.py

Ignored Files:
    reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js

reviewboard/reviews/managers.py (Diff revision 9)

The issue has been resolved. Show all issues

You should be able to just include branch=branch, in the self.model() call above.

reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js (Diff revision 9)

The issue has been resolved. Show all issues

To keep this consistent with the repository, please use branch = this.model.get('branch') and then pass in branch.id below.

Although, perhaps we should be using branch.name instead?

Diff:

Revision 10 (+21 -5)

Show changes

	reviewboard/reviews/managers.py
	reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
	reviewboard/static/rb/js/resources/models/reviewRequestModel.js
	reviewboard/webapi/resources/review_request.py

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/reviews/managers.py

Ignored Files:
    reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/reviews/managers.py

Ignored Files:
    reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js

Ship it!

This looks good now, but I'm going to hold off on pushing it because we're on the cusp of releasing 2.5. I'll get it in for 2.5.1

Status:: Completed
Change Summary:: Pushed to release-2.5.x (c6d6295)

Status:: Re-opened

Unfortunately, this ended up breaking our API, so we're having to revert it and re-release.

Can you update this to inclue a set of unit tests that test this addition?

The specific breakage is documented on https://hellosplat.com/s/beanbag/tickets/4009/

AH

ahache Nov. 4, 2015, 8:04 a.m.
```
Oh no :(

I'll get right on that!
```

ahache Nov. 4, 2015, 10:26 a.m.

Out of curiosity, is there a reason we have a Not Null constraint for branch if branch isn't a required field on a review request and we just save it as an empty string when it isn't provided?

Diff:

Revision 11 (+55 -5)

Show changes

	reviewboard/reviews/managers.py
	reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
	reviewboard/static/rb/js/resources/models/reviewRequestModel.js
	reviewboard/webapi/resources/review_request.py
	reviewboard/webapi/tests/test_review_request.py

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/tests/test_review_request.py
    reviewboard/webapi/resources/review_request.py
    reviewboard/reviews/managers.py

Ignored Files:
    reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/tests/test_review_request.py
    reviewboard/webapi/resources/review_request.py
    reviewboard/reviews/managers.py

Ignored Files:
    reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js

Description:: When creating a new post-commit review request, the branch name was left blank rather than being pre-populated. Since we have the data available, this should be populated.
Added branch info to the request url when creating the new ReviewRequest, webAPI takes care of the rest.
+
+
Corrected API breaking issue. When no branch was provided in API request, None would be attempt to be saved to the database but branch had a Not Null constraint.
+ Added additional tests to cover this case.
Testing Done:: ~
Ran all reviewboard tests.
~
Ran all reviewboard tests.
+ Added new API tests for post with and without branch info.

reviewboard/webapi/resources/review_request.py (Diff revision 11)
The issue has been resolved. Show all issues
```
Remove this commented out code.
```
reviewboard/webapi/tests/test_review_request.py (Diff revision 11)
The issue has been resolved. Show all issues
```
No blank line here.
```

reviewboard/webapi/tests/test_review_request.py (Diff revision 11)

The issue has been resolved. Show all issues

Can you format this as:

        review_request = ReviewRequest.objects.get(
            pkg=rsp['review_request']['id'])

reviewboard/webapi/tests/test_review_request.py (Diff revision 11)
The issue has been resolved. Show all issues
```
Can you put this on the previous line?
```

reviewboard/webapi/tests/test_review_request.py (Diff revision 11)

The issue has been resolved. Show all issues

Can you format this as:

        review_request = ReviewRequest.objects.get(
            pkg=rsp['review_request']['id'])

Diff:

Revision 12 (+50 -5)

Show changes

	reviewboard/reviews/managers.py
	reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
	reviewboard/static/rb/js/resources/models/reviewRequestModel.js
	reviewboard/webapi/resources/review_request.py
	reviewboard/webapi/tests/test_review_request.py

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/tests/test_review_request.py
    reviewboard/webapi/resources/review_request.py
    reviewboard/reviews/managers.py

Ignored Files:
    reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/tests/test_review_request.py
    reviewboard/webapi/resources/review_request.py
    reviewboard/reviews/managers.py

Ignored Files:
    reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js

Status:: Completed
Change Summary:: Pushed to ucosp/nicole_xin/general-comments (c6d6295)

Status:: Re-opened

Just a note, this was backed out due to bug 4009.

Status:: Completed
Change Summary:: Pushed to ucosp/dkus/dnd-inline-images (c6d6295)

~		Testing still to come, going to dig around to find how to write appropriate tests cases for this. Any suggestions are welcome.
	~	Ran all reviewboard tests.

~		Auto-populating branch for new post-commit review requests. Added branch info to the request url when creating the new RR, webAPI takes care of the rest.
	~	When creating a new post-commit review request, the branch name was left blank rather than being pre-populated. Since we have the data available, this should be populated.
	+	Added branch info to the request url when creating the new ReviewRequest, webAPI takes care of the rest.

~		Ran all reviewboard tests.
	~	Ran all reviewboard tests.
	+	Added new API tests for post with and without branch info.