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

Review Request #14297 — Created Jan. 17, 2025 and updated

Information

Review Board
release-7.x

Reviewers

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.

  • Ran unit tests.
  • Tested creating comments with and without issues, then toggling between open/fixed/dropped/re-opened states.
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
Description From Last Updated

'django.contrib.auth.models.User' imported but unused Column: 5 Error code: F401

reviewbotreviewbot
There are no open issues
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

maubin
Review request changed
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.
david
  1. Ship It!
  2. 
      
Loading...