Summary

Adding 'drop_open_issues' to StatusUpdate

Review Request #10075 — Created July 9, 2018 and submitted July 31, 2020, 1:51 p.m.

Information

Owner

jcannon

Repository

Review Board

Branch

master

Bugs

Depends On

Blocks

10061

Commit

9494233...

Reviewers

Groups

reviewboard

People

Description

Adding 'drop_open_issues' to StatusUpdate.

This feature is needed by the Review Bot feature "Adding ability to drop old issues on new review." https://reviews.reviewboard.org/r/10061/.
The method finds all open issues associated with the status update's review (if it exists) and drops them. Smart enough to update the review request's timestamp and issues counts.

Testing Done

Unit tests!

Also, called the method (via the Review Bot changes) with a review request with previous status updates with open issues. Noticed the timestamp and issue count were correct, and old issues were dropped.

Issues

Description	From	Last Updated
This should also be done on release-3.0.x instead of master. Can you rebase?	david	July 31, 2020, 1:48 p.m.
E501 line too long (81 > 79 characters)	reviewbot	July 12, 2018, 7:29 a.m.
We should import this from reviewboard.reviews.models.base_comment, to avoid potential circular import problems.	david	July 31, 2020, 1:48 p.m.
Please add this line back	david	July 31, 2020, 1:48 p.m.
There's an extra space after the """, and this should be written in the imperative rather than passive mood ("Drop …	david	July 31, 2020, 1:48 p.m.
I know a lot of old code doesn't have it, but can you add a module docstring here?	david	July 31, 2020, 1:48 p.m.
Can you specify that this is dropping open issues associated with a status update?	david	July 31, 2020, 1:48 p.m.
Same here.	david	July 31, 2020, 1:48 p.m.
Same here.	david	July 31, 2020, 1:48 p.m.
Same here.	david	July 31, 2020, 1:48 p.m.
Same here.	david	July 31, 2020, 1:48 p.m.
Same here.	david	July 31, 2020, 1:48 p.m.

flake8 passed.

JSHint passed.

I'll start on unit tests tomorrow.

jcannon July 10, 2018, 7:25 a.m.

I didn't see any test files that test the StatusUpdate model (I see other ones for the Entry). (That makes sense because there wasn't much logic in StatusUpdate before).
I plan on adding one, stop me if I'm way off track.

Change Summary:

Adding unit tests

Testing Done:

~		Call the method (via the Review Bot changes) with a review request with previous status updates with open issues. Noticed the timestamp and issue count were correct, and old issues were dropped.
	~	Unit tests!
	+
	+	Also, called the method (via the Review Bot changes) with a review request with previous status updates with open issues. Noticed the timestamp and issue count were correct, and old issues were dropped.

Commit:

f842ea8e6475d07269280a0a320f5a070692513e

b9e6631344294549686009178673b8a2c9799c5b

Diff:

Revision 2 (+168 -1)

Show changes

	reviewboard/reviews/models/status_update.py
	reviewboard/reviews/tests/test_status_update.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/reviews/tests/test_status_update.py (Diff revision 2)
The issue has been resolved. Show all issues
```
E501 line too long (81 > 79 characters)
```

Change Summary:

Fixed line length

Commit:

b9e6631344294549686009178673b8a2c9799c5b

94942339a26af4d122fb4fb9dc1b077bbd5f5759

Diff:

Revision 3 (+169 -1)

Show changes

	reviewboard/reviews/models/status_update.py
	reviewboard/reviews/tests/test_status_update.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

```
bump
```

```
bump
```

reviewboard/reviews/models/status_update.py (Diff revision 3)

The issue has been resolved. Show all issues

We should import this from reviewboard.reviews.models.base_comment, to avoid potential circular import problems.

reviewboard/reviews/models/status_update.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Please add this line back
```

reviewboard/reviews/models/status_update.py (Diff revision 3)

The issue has been resolved. Show all issues

There's an extra space after the """, and this should be written in the imperative rather than passive mood ("Drop open issues" instead of "Drops open issues")

reviewboard/reviews/tests/test_status_update.py (Diff revision 3)
The issue has been resolved. Show all issues
```
I know a lot of old code doesn't have it, but can you add a module docstring here?
```
reviewboard/reviews/tests/test_status_update.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Can you specify that this is dropping open issues associated with a status update?
```
reviewboard/reviews/tests/test_status_update.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Same here.
```
reviewboard/reviews/tests/test_status_update.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Same here.
```
reviewboard/reviews/tests/test_status_update.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Same here.
```
reviewboard/reviews/tests/test_status_update.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Same here.
```
reviewboard/reviews/tests/test_status_update.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Same here.
```

The issue has been resolved. Show all issues

This should also be done on release-3.0.x instead of master. Can you rebase?

any progress here? :-)

jcannon Jan. 29, 2020, 5:28 a.m.

My company is no longer planning on upgrading to ReviewBoard 3.0 for the forseeable future. Feel free to steal my code and continue the changes if you'd like.

Ship it!

Going to make some minor changes and get this landed. Thanks!

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