Allow comments to be deleted from review dialog
Review Request #7744 — Created Oct. 28, 2015 and submitted
The review dialog now displays a delete icon next to all deletable
comments (diff comments and attachment comments, currently). Previously,
the only way to delete a comment was to delete it from the review UI
where it was created. However, with general comments coming soon, there
was no review UI to manage them and delete them. Hence, it makes sense
for all deletable comments to be deletable from within the review
dialog.JS Unit tests have been added for deletion of diff comments, attachment
comments, and screenshot comments.This patch also contains some minor JS cleanups:
-RB.BaseResourceModel.readynow ensures itssuccessanderror
options are functions before bdinging them.
- Replaced a binding ofself = thisand calling a function by binding
said function to the correct context.
- Cleaned up some assignments inRB.BaseResourceModel._finishDestroy
so that it only does oneset()call instead of several.
- Ran JS tests.
- Deleted attachment comments and diff comments from the review dialog.
It worked as expected. - Manually verified that the deletion confirmation works in both cases.
| Description | From | Last Updated |
|---|---|---|
|
The order of things is weird here. Can we put the pencil first? We may also want to make the … |
david
|
|
|
I suspect this should not be within the <label> element. |
david
|
|
|
How about: this._commentViews = _.without(this._commentViews, view); |
david
|
|
|
This variable isn't used anymore. |
david
|
-
Tool: PEP8 Style Checker Ignored Files: reviewboard/static/rb/js/views/reviewDialogView.js reviewboard/static/rb/css/pages/reviews.less reviewboard/static/rb/js/resources/models/baseResourceModel.js Tool: Pyflakes Ignored Files: reviewboard/static/rb/js/views/reviewDialogView.js reviewboard/static/rb/css/pages/reviews.less reviewboard/static/rb/js/resources/models/baseResourceModel.js
Review request changed
- Change Summary:
-
Add unit tests
- Description:
-
The review dialog now displays a delete icon next to all deletable
comments (diff comments and attachment comments, currently). Previously, the only way to delete a comment was to delete it from the review UI where it was created. However, with general comments coming soon, there was no review UI to manage them and delete them. Hence, it makes sense for all deletable comments to be deletable from within the review dialog. + JS Unit tests have been added for deletion of diff comments, attachment
+ comments, and screenshot comments. + This patch also contains some minor JS cleanups:
- RB.BaseResourceModel.readynow ensures itssuccessanderroroptions are functions before bdinging them. - Replaced a binding of self = thisand calling a function by bindingsaid function to the correct context. - Cleaned up some assignments in RB.BaseResourceModel._finishDestroyso that it only does one set()call instead of several. - Testing Done:
-
- Ran JS tests.
- Deleted attachment comments and diff comments from the review dialog.
It worked as expected.
+ - Manually verified that the deletion confirmation works in both cases.
-
Tool: Pyflakes Ignored Files: reviewboard/static/rb/js/views/reviewDialogView.js reviewboard/static/rb/css/pages/reviews.less reviewboard/static/rb/js/views/tests/reviewDialogViewTests.js reviewboard/static/rb/js/resources/models/baseResourceModel.js Tool: PEP8 Style Checker Ignored Files: reviewboard/static/rb/js/views/reviewDialogView.js reviewboard/static/rb/css/pages/reviews.less reviewboard/static/rb/js/views/tests/reviewDialogViewTests.js reviewboard/static/rb/js/resources/models/baseResourceModel.js
-
Tool: Pyflakes Ignored Files: reviewboard/static/rb/js/views/reviewDialogView.js reviewboard/static/rb/css/pages/reviews.less reviewboard/static/rb/js/views/tests/reviewDialogViewTests.js reviewboard/static/rb/js/resources/models/baseResourceModel.js Tool: PEP8 Style Checker Ignored Files: reviewboard/static/rb/js/views/reviewDialogView.js reviewboard/static/rb/css/pages/reviews.less reviewboard/static/rb/js/views/tests/reviewDialogViewTests.js reviewboard/static/rb/js/resources/models/baseResourceModel.js
Tool: PEP8 Style Checker Ignored Files: reviewboard/static/rb/js/views/reviewDialogView.js reviewboard/static/rb/css/pages/reviews.less reviewboard/static/rb/js/resources/models/baseResourceModel.js Tool: Pyflakes Ignored Files: reviewboard/static/rb/js/views/reviewDialogView.js reviewboard/static/rb/css/pages/reviews.less reviewboard/static/rb/js/resources/models/baseResourceModel.js