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.

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)
Show all issues
```
F841 local variable 'review' is assigned to but never used
```
reviewboard/reviews/tests/test_review_request.py (Diff revision 2)
Show all issues
```
F841 local variable 'review' is assigned to but never used
```

reviewboard/reviews/tests/test_review_request.py (Diff revision 2)

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)
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)
Show all issues
```
No period.
```
reviewboard/reviews/tests/test_review_request.py (Diff revision 3)
Show all issues
```
assertIsNone
```
reviewboard/reviews/tests/test_review_request.py (Diff revision 3)
Show all issues
```
No period.
```
reviewboard/reviews/tests/test_review_request.py (Diff revision 3)
Show all issues
```
No period.
```
reviewboard/reviews/tests/test_review_request.py (Diff revision 3)
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: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (4eed931)