Fix discarding of review replies, and several issues with drafts.
Review Request #4023 — Created April 5, 2013 and submitted
Fix discarding of review replies, and several issues with drafts. Review replies weren't given the same draft logic that reviews were given, but since they're a variant on a review, it needed the same logic. Furthermore, replies couldn't be discarded. A new mixin was added that contains all the draft logic. It's based on what was in ReviewModel, but modified. Both ReviewModel and ReviewReplyModel use it for all their draft handling now. The logic was changed to handle the case where an object goes from unloaded to loaded and back to unloaded (such as after a destruction). Instead of overriding its url() and ready() functions, it now just includes some state indicating whether to use its own functions or other ones. Since the mixin needs to call parent functions, but may not know what the parent is, a _.super() function was added, which will compute the super prototype for an instance. BaseResource got some fixes to handle parentObject loading properly when using an old-style resource object. It also now resets the object state after a destroy(), so it can be used again. In order to work around some Chrome bugs I encountered, I've changed the draft resources on the server side to return HTTP 302s instead of 301s, as the latter is meant to be more permanent. I've also updated the Last Modified time when updating a reply. I'm not sure this solved all the problems yet (time will tell, as it was intermittent), but it's probably more correct anyway.
Lots of manual testing of discarding, and with an upcoming change to fix the "discard if empty" (which went missing in the rewrite, but also was pretty broken before it anyway). Unit tests all passed (server-side and JavaScript).
- Change Summary:
-
Fix a comment.
- Diff:
-
Revision 2 (+407 -82)
-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: reviewboard/settings.py reviewboard/webapi/resources.py Ignored Files: reviewboard/static/rb/js/views/tests/screenshotThumbnailViewTests.js reviewboard/static/rb/js/models/reviewReplyModel.js reviewboard/static/rb/js/models/baseResourceModel.js reviewboard/static/rb/js/utils/underscoreUtils.js reviewboard/static/rb/js/models/draftResourceModelMixin.js reviewboard/static/rb/js/models/draftReviewModel.js reviewboard/static/rb/js/models/tests/reviewReplyModelTests.js reviewboard/static/rb/js/models/reviewModel.js
- Change Summary:
-
Figured out how to reliably work around the weird Chrome brokenness, and documented it.
- Diff:
-
Revision 3 (+426 -82)
-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: reviewboard/settings.py reviewboard/webapi/resources.py Ignored Files: reviewboard/static/rb/js/views/tests/screenshotThumbnailViewTests.js reviewboard/static/rb/js/models/reviewReplyModel.js reviewboard/static/rb/js/models/baseResourceModel.js reviewboard/static/rb/js/utils/underscoreUtils.js reviewboard/static/rb/js/models/draftResourceModelMixin.js reviewboard/static/rb/js/models/draftReviewModel.js reviewboard/static/rb/js/models/tests/reviewReplyModelTests.js reviewboard/static/rb/js/models/reviewModel.js