Summary

Add a signal for when the issue status of a comment gets updated.

Review Request #14297 — Created Jan. 17, 2025 and submitted Feb. 10, 2026, 8:35 p.m.

Information

Owner

maubin

Repository

Review Board

Branch

release-7.1.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

While here, I also fixed up two bugs that I noticed with our comments:

We override the save method for the BaseComment class, but failed to pass
any **kwargs to django.db.models.Model.save(), so full model updates
were happening every time, even if update_fields was passed. I added a
unit test that had I used to check this.

We use an attribute called _loaded_issue_status to track the previous
issue status on a comment, and compare it to the new issue status on updates.
Previously, we only set this attribute on comment initialization. So if one
instance of a comment went from open, to dropped, to re-opened, the issue
state wouldn't actually update from dropped to re-opened. Our issues status
updating logic would compare the re-opened state to the initial open state
that was set during initialization, even though the real previous issue
state at the time was dropped. This was never an issue in production
because when toggling issue statuses from the UI, a new comment object
instance would be created upon each toggle. This is now fixed by
setting the _loaded_issue_status whenever the issue status changes.

I also fixed some typos in some signal doc comments.

Testing Done


Ran unit tests.
Tested creating comments with and without issues, then toggling between open/fixed/dropped/re-opened states.

Commits

Summary	ID
Add a signal for when the issue status of a comment gets updated. This change adds a signal for when the issues status of a comment gets updated. This signal is needed for the WIP Bitbucket Server Pull Request integration (and future pull request integrations), which will listen to the signal so that it can update the review request approval status displayed on the pull request whenever an issue is dropped/fixed/re-opened. It's also just generally useful to have. While here, I also fixed up two bugs that I noticed with our comments: We override the save method for the `BaseComment` class, but failed to pass any `**kwargs` to `django.db.models.Model.save()`, so full model updates were happening every time, even if `update_fields` was passed. I added a unit test that had I used to check this. We use an attribute called `_loaded_issue_status` to track the previous issue status on a comment, and compare it to the new issue status on updates. Previously, we only set this attribute on comment initialization. So if one instance of a comment went from open, to dropped, to re-opened, the issue state wouldn't actually update from dropped to re-opened. Our issues status updating logic would compare the re-opened state to the initial open state that was set during initialization, even though the real previous issue state at the time was dropped. This was never an issue in production because when toggling issue statuses from the UI, a new comment object instance would be created upon each toggle. This is now fixed by setting the `_loaded_issue_status` whenever the issue status changes. I also fixed some typos in some signal doc comments.	162023fd448e5bc49f08f4211a439cf6d02237c4

Summary

Add a signal for when the issue status of a comment gets updated.

This change adds a signal for when the issues status of a comment gets updated. This signal is needed for the WIP Bitbucket Server Pull Request integration (and future pull request integrations), which will listen to the signal so that it can update the review request approval status displayed on the pull request whenever an issue is dropped/fixed/re-opened. It's also just generally useful to have. While here, I also fixed up two bugs that I noticed with our comments: We override the save method for the `BaseComment` class, but failed to pass any `**kwargs` to `django.db.models.Model.save()`, so full model updates were happening every time, even if `update_fields` was passed. I added a unit test that had I used to check this. We use an attribute called `_loaded_issue_status` to track the previous issue status on a comment, and compare it to the new issue status on updates. Previously, we only set this attribute on comment initialization. So if one instance of a comment went from open, to dropped, to re-opened, the issue state wouldn't actually update from dropped to re-opened. Our issues status updating logic would compare the re-opened state to the initial open state that was set during initialization, even though the real previous issue state at the time was dropped. This was never an issue in production because when toggling issue statuses from the UI, a new comment object instance would be created upon each toggle. This is now fixed by setting the `_loaded_issue_status` whenever the issue status changes. I also fixed some typos in some signal doc comments.

162023fd448e5bc49f08f4211a439cf6d02237c4

Issues

Description	From	Last Updated
'django.contrib.auth.models.User' imported but unused Column: 5 Error code: F401	reviewbot	Jan. 17, 2025, 4:32 p.m.
We can simplify this with: kwargs['update_fields'] = \ set(kwargs['update_fields'} \| {'timestamp'}	chipx86	June 3, 2025, 12:48 a.m.
Even better syntax for this can be: if update_fields := kwargs.get('update_fields'): kwargs['update_fields'] = { *update_fields, 'timestamp' }	david	Feb. 2, 2026, 8:48 p.m.
I think we can remove the BaseComment part in the description, since it's in the type.	chipx86	June 3, 2025, 12:49 a.m.
For this, we need to pass owner=models.Model to ensure the type can always be found. There should be a kgb …	chipx86	June 3, 2025, 12:49 a.m.
Would it be worth including the old and new values of the status?	david	Feb. 8, 2026, 7:52 p.m.
Can we add a blank line between these two?	david	Feb. 8, 2026, 7:40 p.m.
Can you add -> None on here? Same below.	david	Feb. 8, 2026, 7:40 p.m.
If we have an assertion failure, this will never run, leaving the signal connected. We could either put everything into …	david	Feb. 8, 2026, 7:42 p.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/reviews/models/base_comment.py (Diff revision 1)
The issue has been resolved. Show all issues
```
'django.contrib.auth.models.User' imported but unused

Column: 5
Error code: F401
```

Change Summary:

Removed an unused import.

Commits:

	Summary	ID
	Add a signal for when the issue status of a comment gets updated. This change adds a signal for when the issues status of a comment gets updated. This signal is needed for the WIP Bitbucket Server Pull Request integration (and future pull request integrations), which will listen to the signal so that it can update the review request approval status displayed on the pull request whenever an issue is dropped/fixed/re-opened. It's also just generally useful to have. While here, I also fixed up two bugs that I noticed with our comments: We override the save method for the `BaseComment` class, but failed to pass any `**kwargs` to `django.db.models.Model.save()`, so full model updates were happening every time, even if `update_fields` was passed. I added a unit test that had I used to check this. We use an attribute called `_loaded_issue_status` to track the previous issue status on a comment, and compare it to the new issue status on updates. Previously, we only set this attribute on comment initialization. So if one instance of a comment went from open, to dropped, to re-opened, the issue state wouldn't actually update from dropped to re-opened. Our issues status updating logic would compare the re-opened state to the initial open state that was set during initialization, even though the real previous issue state at the time was dropped. This was never an issue in production because when toggling issue statuses from the UI, a new comment object instance would be created upon each toggle. This is now fixed by setting the `_loaded_issue_status` whenever the issue status changes. I also fixed some typos in some signal doc comments.	39f2cbb169d3377b46179ef40be5c94d4d365801
	Add a signal for when the issue status of a comment gets updated. This change adds a signal for when the issues status of a comment gets updated. This signal is needed for the WIP Bitbucket Server Pull Request integration (and future pull request integrations), which will listen to the signal so that it can update the review request approval status displayed on the pull request whenever an issue is dropped/fixed/re-opened. It's also just generally useful to have. While here, I also fixed up two bugs that I noticed with our comments: We override the save method for the `BaseComment` class, but failed to pass any `**kwargs` to `django.db.models.Model.save()`, so full model updates were happening every time, even if `update_fields` was passed. I added a unit test that had I used to check this. We use an attribute called `_loaded_issue_status` to track the previous issue status on a comment, and compare it to the new issue status on updates. Previously, we only set this attribute on comment initialization. So if one instance of a comment went from open, to dropped, to re-opened, the issue state wouldn't actually update from dropped to re-opened. Our issues status updating logic would compare the re-opened state to the initial open state that was set during initialization, even though the real previous issue state at the time was dropped. This was never an issue in production because when toggling issue statuses from the UI, a new comment object instance would be created upon each toggle. This is now fixed by setting the `_loaded_issue_status` whenever the issue status changes. I also fixed some typos in some signal doc comments.	47fe51e933b5287531dadad5cd429823fdbee04e

Diff:

Revision 2 (+416 -14)

Show changes

	reviewboard/reviews/signals.py
	reviewboard/reviews/models/base_comment.py
	reviewboard/reviews/tests/test_base_comment.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

reviewboard/reviews/models/base_comment.py (Diff revision 2)

The issue has been resolved. Show all issues

We can simplify this with:

kwargs['update_fields'] = \
    set(kwargs['update_fields'} | {'timestamp'}

reviewboard/reviews/signals.py (Diff revision 2)

The issue has been resolved. Show all issues

I think we can remove the BaseComment part in the description, since it's in the type.

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

The issue has been resolved. Show all issues

For this, we need to pass owner=models.Model to ensure the type can always be found. There should be a kgb warning if not passed and it's an unbound method.

maubin June 3, 2025, 1:27 a.m.

Oh, I missed the warning because it only gets displayed in the "Captured log calls" section when the unit test fails. If the unit test passes, you don't see any captured logs. Maybe we wanna update the kgb code to use warnings.warn instead of logger.warning so that the warning will always appear in the "Warnings summary" section?

Commits:

	Summary	ID
	Add a signal for when the issue status of a comment gets updated. This change adds a signal for when the issues status of a comment gets updated. This signal is needed for the WIP Bitbucket Server Pull Request integration (and future pull request integrations), which will listen to the signal so that it can update the review request approval status displayed on the pull request whenever an issue is dropped/fixed/re-opened. It's also just generally useful to have. While here, I also fixed up two bugs that I noticed with our comments: We override the save method for the `BaseComment` class, but failed to pass any `**kwargs` to `django.db.models.Model.save()`, so full model updates were happening every time, even if `update_fields` was passed. I added a unit test that had I used to check this. We use an attribute called `_loaded_issue_status` to track the previous issue status on a comment, and compare it to the new issue status on updates. Previously, we only set this attribute on comment initialization. So if one instance of a comment went from open, to dropped, to re-opened, the issue state wouldn't actually update from dropped to re-opened. Our issues status updating logic would compare the re-opened state to the initial open state that was set during initialization, even though the real previous issue state at the time was dropped. This was never an issue in production because when toggling issue statuses from the UI, a new comment object instance would be created upon each toggle. This is now fixed by setting the `_loaded_issue_status` whenever the issue status changes. I also fixed some typos in some signal doc comments.	47fe51e933b5287531dadad5cd429823fdbee04e
	Add a signal for when the issue status of a comment gets updated. This change adds a signal for when the issues status of a comment gets updated. This signal is needed for the WIP Bitbucket Server Pull Request integration (and future pull request integrations), which will listen to the signal so that it can update the review request approval status displayed on the pull request whenever an issue is dropped/fixed/re-opened. It's also just generally useful to have. While here, I also fixed up two bugs that I noticed with our comments: We override the save method for the `BaseComment` class, but failed to pass any `**kwargs` to `django.db.models.Model.save()`, so full model updates were happening every time, even if `update_fields` was passed. I added a unit test that had I used to check this. We use an attribute called `_loaded_issue_status` to track the previous issue status on a comment, and compare it to the new issue status on updates. Previously, we only set this attribute on comment initialization. So if one instance of a comment went from open, to dropped, to re-opened, the issue state wouldn't actually update from dropped to re-opened. Our issues status updating logic would compare the re-opened state to the initial open state that was set during initialization, even though the real previous issue state at the time was dropped. This was never an issue in production because when toggling issue statuses from the UI, a new comment object instance would be created upon each toggle. This is now fixed by setting the `_loaded_issue_status` whenever the issue status changes. I also fixed some typos in some signal doc comments.	2582feb8cad6ef8f041f64f9e87501c4a25aa144

Diff:

Revision 3 (+412 -14)

Show changes

	reviewboard/reviews/signals.py
	reviewboard/reviews/models/base_comment.py
	reviewboard/reviews/tests/test_base_comment.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/reviews/models/base_comment.py (Diff revisions 2 - 3)

The issue has been resolved. Show all issues

Even better syntax for this can be:

if update_fields := kwargs.get('update_fields'):
    kwargs['update_fields'] = { *update_fields, 'timestamp' }

Change Summary:


Moved to branch 7.1.x
Fixed compatibility with the supertype method signature
Rewrote a dict union in a better style.

Commits:

	Summary	ID
	Add a signal for when the issue status of a comment gets updated. This change adds a signal for when the issues status of a comment gets updated. This signal is needed for the WIP Bitbucket Server Pull Request integration (and future pull request integrations), which will listen to the signal so that it can update the review request approval status displayed on the pull request whenever an issue is dropped/fixed/re-opened. It's also just generally useful to have. While here, I also fixed up two bugs that I noticed with our comments: We override the save method for the `BaseComment` class, but failed to pass any `**kwargs` to `django.db.models.Model.save()`, so full model updates were happening every time, even if `update_fields` was passed. I added a unit test that had I used to check this. We use an attribute called `_loaded_issue_status` to track the previous issue status on a comment, and compare it to the new issue status on updates. Previously, we only set this attribute on comment initialization. So if one instance of a comment went from open, to dropped, to re-opened, the issue state wouldn't actually update from dropped to re-opened. Our issues status updating logic would compare the re-opened state to the initial open state that was set during initialization, even though the real previous issue state at the time was dropped. This was never an issue in production because when toggling issue statuses from the UI, a new comment object instance would be created upon each toggle. This is now fixed by setting the `_loaded_issue_status` whenever the issue status changes. I also fixed some typos in some signal doc comments.	2582feb8cad6ef8f041f64f9e87501c4a25aa144
	Add a signal for when the issue status of a comment gets updated. This change adds a signal for when the issues status of a comment gets updated. This signal is needed for the WIP Bitbucket Server Pull Request integration (and future pull request integrations), which will listen to the signal so that it can update the review request approval status displayed on the pull request whenever an issue is dropped/fixed/re-opened. It's also just generally useful to have. While here, I also fixed up two bugs that I noticed with our comments: We override the save method for the `BaseComment` class, but failed to pass any `**kwargs` to `django.db.models.Model.save()`, so full model updates were happening every time, even if `update_fields` was passed. I added a unit test that had I used to check this. We use an attribute called `_loaded_issue_status` to track the previous issue status on a comment, and compare it to the new issue status on updates. Previously, we only set this attribute on comment initialization. So if one instance of a comment went from open, to dropped, to re-opened, the issue state wouldn't actually update from dropped to re-opened. Our issues status updating logic would compare the re-opened state to the initial open state that was set during initialization, even though the real previous issue state at the time was dropped. This was never an issue in production because when toggling issue statuses from the UI, a new comment object instance would be created upon each toggle. This is now fixed by setting the `_loaded_issue_status` whenever the issue status changes. I also fixed some typos in some signal doc comments.	860435f794a3dcd01437bfb837eb45f48ab42c9b

Branch:

release-7.x

release-7.1.x

Diff:

Revision 4 (+424 -16)

Show changes

	reviewboard/reviews/signals.py
	reviewboard/reviews/models/base_comment.py
	reviewboard/reviews/tests/test_base_comment.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Forgot to ammend these to the last commit:
- Update a test to check for positional args too.
- Changed version strings from 7.0.4 to 7.1

Commits:

	Summary	ID
	Add a signal for when the issue status of a comment gets updated. This change adds a signal for when the issues status of a comment gets updated. This signal is needed for the WIP Bitbucket Server Pull Request integration (and future pull request integrations), which will listen to the signal so that it can update the review request approval status displayed on the pull request whenever an issue is dropped/fixed/re-opened. It's also just generally useful to have. While here, I also fixed up two bugs that I noticed with our comments: We override the save method for the `BaseComment` class, but failed to pass any `**kwargs` to `django.db.models.Model.save()`, so full model updates were happening every time, even if `update_fields` was passed. I added a unit test that had I used to check this. We use an attribute called `_loaded_issue_status` to track the previous issue status on a comment, and compare it to the new issue status on updates. Previously, we only set this attribute on comment initialization. So if one instance of a comment went from open, to dropped, to re-opened, the issue state wouldn't actually update from dropped to re-opened. Our issues status updating logic would compare the re-opened state to the initial open state that was set during initialization, even though the real previous issue state at the time was dropped. This was never an issue in production because when toggling issue statuses from the UI, a new comment object instance would be created upon each toggle. This is now fixed by setting the `_loaded_issue_status` whenever the issue status changes. I also fixed some typos in some signal doc comments.	860435f794a3dcd01437bfb837eb45f48ab42c9b
	Add a signal for when the issue status of a comment gets updated. This change adds a signal for when the issues status of a comment gets updated. This signal is needed for the WIP Bitbucket Server Pull Request integration (and future pull request integrations), which will listen to the signal so that it can update the review request approval status displayed on the pull request whenever an issue is dropped/fixed/re-opened. It's also just generally useful to have. While here, I also fixed up two bugs that I noticed with our comments: We override the save method for the `BaseComment` class, but failed to pass any `**kwargs` to `django.db.models.Model.save()`, so full model updates were happening every time, even if `update_fields` was passed. I added a unit test that had I used to check this. We use an attribute called `_loaded_issue_status` to track the previous issue status on a comment, and compare it to the new issue status on updates. Previously, we only set this attribute on comment initialization. So if one instance of a comment went from open, to dropped, to re-opened, the issue state wouldn't actually update from dropped to re-opened. Our issues status updating logic would compare the re-opened state to the initial open state that was set during initialization, even though the real previous issue state at the time was dropped. This was never an issue in production because when toggling issue statuses from the UI, a new comment object instance would be created upon each toggle. This is now fixed by setting the `_loaded_issue_status` whenever the issue status changes. I also fixed some typos in some signal doc comments.	6e2e697e44a62ef40df075b7a5ff2c41488b035e

Diff:

Revision 5 (+428 -16)

Show changes

	reviewboard/reviews/signals.py
	reviewboard/reviews/models/base_comment.py
	reviewboard/reviews/tests/test_base_comment.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/reviews/signals.py (Diff revision 5)
The issue has been resolved. Show all issues
```
Would it be worth including the old and new values of the status?
```
1. maubin Feb. 8, 2026, 8 p.m.
  Yeah, that would be useful.
reviewboard/reviews/tests/test_base_comment.py (Diff revision 5)
The issue has been resolved. Show all issues
```
Can we add a blank line between these two?
```
reviewboard/reviews/tests/test_base_comment.py (Diff revision 5)
The issue has been resolved. Show all issues
```
Can you add -> None on here?
Same below.
```

reviewboard/reviews/tests/test_base_comment.py (Diff revision 5)

The issue has been resolved. Show all issues

If we have an assertion failure, this will never run, leaving the signal connected.

We could either put everything into a try/finally, or we can use:

self.addCleanup(comment_issue_status_updated.disconnect,
                on_issue_status_updated)

Same below.

Change Summary:


Include the previous and current issue status values in the signal.
Fix some unit test cleanup.

Commits:

	Summary	ID
	Add a signal for when the issue status of a comment gets updated. This change adds a signal for when the issues status of a comment gets updated. This signal is needed for the WIP Bitbucket Server Pull Request integration (and future pull request integrations), which will listen to the signal so that it can update the review request approval status displayed on the pull request whenever an issue is dropped/fixed/re-opened. It's also just generally useful to have. While here, I also fixed up two bugs that I noticed with our comments: We override the save method for the `BaseComment` class, but failed to pass any `**kwargs` to `django.db.models.Model.save()`, so full model updates were happening every time, even if `update_fields` was passed. I added a unit test that had I used to check this. We use an attribute called `_loaded_issue_status` to track the previous issue status on a comment, and compare it to the new issue status on updates. Previously, we only set this attribute on comment initialization. So if one instance of a comment went from open, to dropped, to re-opened, the issue state wouldn't actually update from dropped to re-opened. Our issues status updating logic would compare the re-opened state to the initial open state that was set during initialization, even though the real previous issue state at the time was dropped. This was never an issue in production because when toggling issue statuses from the UI, a new comment object instance would be created upon each toggle. This is now fixed by setting the `_loaded_issue_status` whenever the issue status changes. I also fixed some typos in some signal doc comments.	6e2e697e44a62ef40df075b7a5ff2c41488b035e
	Add a signal for when the issue status of a comment gets updated. This change adds a signal for when the issues status of a comment gets updated. This signal is needed for the WIP Bitbucket Server Pull Request integration (and future pull request integrations), which will listen to the signal so that it can update the review request approval status displayed on the pull request whenever an issue is dropped/fixed/re-opened. It's also just generally useful to have. While here, I also fixed up two bugs that I noticed with our comments: We override the save method for the `BaseComment` class, but failed to pass any `**kwargs` to `django.db.models.Model.save()`, so full model updates were happening every time, even if `update_fields` was passed. I added a unit test that had I used to check this. We use an attribute called `_loaded_issue_status` to track the previous issue status on a comment, and compare it to the new issue status on updates. Previously, we only set this attribute on comment initialization. So if one instance of a comment went from open, to dropped, to re-opened, the issue state wouldn't actually update from dropped to re-opened. Our issues status updating logic would compare the re-opened state to the initial open state that was set during initialization, even though the real previous issue state at the time was dropped. This was never an issue in production because when toggling issue statuses from the UI, a new comment object instance would be created upon each toggle. This is now fixed by setting the `_loaded_issue_status` whenever the issue status changes. I also fixed some typos in some signal doc comments.	162023fd448e5bc49f08f4211a439cf6d02237c4

Diff:

Revision 6 (+474 -16)

Show changes

	reviewboard/reviews/signals.py
	reviewboard/reviews/models/base_comment.py
	reviewboard/reviews/tests/test_base_comment.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-7.1.x (95d57e9)