Summary

Include unverified issues in the review request approval state.

Review Request #9170 — Created Sept. 4, 2017 and submitted Sept. 29, 2017, 10:31 p.m.

Information

Owner

david

Repository

Review Board

Branch

release-3.0.x

Bugs

Depends On

Commit

1c9e418...

Reviewers

Groups

reviewboard

People

Description

If issues are waiting for verification, don't show the review request as
approved.

Testing Done

Ran unit tests.

Issues

Description	From	Last Updated
I know we don't have unit tests for this yet. Would you mind writing some, since you're modifying the logic?	chipx86	Sept. 10, 2017, 2:15 p.m.
F841 local variable 'review' is assigned to but never used	reviewbot	Sept. 10, 2017, 2:27 p.m.
F841 local variable 'review' is assigned to but never used	reviewbot	Sept. 10, 2017, 2:27 p.m.
F811 redefinition of unused 'test_approval_states_open_issues' from line 630	reviewbot	Sept. 10, 2017, 2:27 p.m.
F841 local variable 'comment' is assigned to but never used	reviewbot	Sept. 10, 2017, 2:27 p.m.
No period.	chipx86	Sept. 26, 2017, 11:41 a.m.
assertIsNone	chipx86	Sept. 26, 2017, 11:41 a.m.
No period.	chipx86	Sept. 26, 2017, 11:41 a.m.
No period.	chipx86	Sept. 26, 2017, 11:41 a.m.
""" on the next line, and no period.	chipx86	Sept. 26, 2017, 11:41 a.m.

flake8 passed.

JSHint passed.

The issue has been resolved. Show all issues

I know we don't have unit tests for this yet. Would you mind writing some, since you're modifying the logic?

david Sept. 8, 2017, 4:32 p.m.

What kind of test are you thinking? I don't see any way of testing this method that isn't just "duplicate the logic".

chipx86 Sept. 8, 2017, 7:12 p.m.

We should have unit tests that test the default approved and unapproved states along with the descriptions, so any future logic changes don't regress, particularly if we ever end up weaving this in deeper with any other services down the road.

Commit:

228ab6e36a0475cc31e1537e93820e588a628a25

12e1cedd5155d612fdc84971ae45481672e9a200

Diff:

Revision 2 (+47)

Show changes

	reviewboard/reviews/models/review_request.py
	reviewboard/reviews/tests/test_review_request.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/reviews/tests/test_review_request.py (Diff revision 2)
The issue has been resolved. Show all issues
```
F841 local variable 'review' is assigned to but never used
```
reviewboard/reviews/tests/test_review_request.py (Diff revision 2)
The issue has been resolved. Show all issues
```
F841 local variable 'review' is assigned to but never used
```
reviewboard/reviews/tests/test_review_request.py (Diff revision 2)
The issue has been resolved. Show all issues
```
F811 redefinition of unused 'test_approval_states_open_issues' from line 630
```
reviewboard/reviews/tests/test_review_request.py (Diff revision 2)
The issue has been resolved. Show all issues
```
F841 local variable 'comment' is assigned to but never used
```

Commit:

12e1cedd5155d612fdc84971ae45481672e9a200

ad52fc1964e732c7cf2574ede2b310d051769197

Diff:

Revision 3 (+45)

Show changes

	reviewboard/reviews/models/review_request.py
	reviewboard/reviews/tests/test_review_request.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/reviews/tests/test_review_request.py (Diff revision 3)
The issue has been resolved. Show all issues
```
No period.
```
reviewboard/reviews/tests/test_review_request.py (Diff revision 3)
The issue has been resolved. Show all issues
```
assertIsNone
```
reviewboard/reviews/tests/test_review_request.py (Diff revision 3)
The issue has been resolved. Show all issues
```
No period.
```
reviewboard/reviews/tests/test_review_request.py (Diff revision 3)
The issue has been resolved. Show all issues
```
No period.
```
reviewboard/reviews/tests/test_review_request.py (Diff revision 3)
The issue has been resolved. Show all issues
```
""" on the next line, and no period.
```

Commit:

ad52fc1964e732c7cf2574ede2b310d051769197

1c9e4188869f7e0d0b208107b27b2f790d439790

Diff:

Revision 4 (+46)

Show changes

	reviewboard/reviews/models/review_request.py
	reviewboard/reviews/tests/test_review_request.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-3.0.x (4eed931)