-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.reviewboard.org/r/3242/ -----------------------------------------------------------
Review request for Review Board.
Description
-------
This is what I have so far for the multi user collaboration on review requests. So far, what I have done is allow all reviewers (users in the target people and groups) to be allowed to change the review request. This would all the reviewers to all the normal changing review request functionalities.
I'm not sure if we have decided who can be able to modify the review request. For now, I put them as all the reviewers.
Also, I've included the last_updated_by field to the review request model in order to display who has last modified the review request or published new review on the dashboard.
Manual testing. I've ran the unit tests and had a bunch of errors and one failure. The errors are all expected due to the change of the ReviewRequestDraft.create() function and I need to look into that one failure. I think it's expected failure though.
I'll go through them all and make modifications/fixes as necessary.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.reviewboard.org/r/3242/ -----------------------------------------------------------
(Updated Aug. 5, 2012, 6:32 a.m.)
Review request for Review Board.
Changes
-------
Fixed the unit tests errors and failures.
Description
-------
This is what I have so far for the multi user collaboration on review requests. So far, what I have done is allow all reviewers (users in the target people and groups) to be allowed to change the review request. This would all the reviewers to all the normal changing review request functionalities.
I'm not sure if we have decided who can be able to modify the review request. For now, I put them as all the reviewers.
Also, I've included the last_updated_by field to the review request model in order to display who has last modified the review request or published new review on the dashboard.
Manual testing. I've ran the unit tests and had a bunch of errors and one failure. The errors are all expected due to the change of the ReviewRequestDraft.create() function and I need to look into that one failure. I think it's expected failure though.
I'll go through them all and make modifications/fixes as necessary.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.reviewboard.org/r/3242/#review4130 -----------------------------------------------------------
I haven't run this patch yet, but this looks like the right idea to me. You're on the right track. Let me know when you're done iterating, and I'll give it a full review.
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.reviewboard.org/r/3242/ > -----------------------------------------------------------
> (Updated Aug. 5, 2012, 6:32 a.m.)
> Review request for Review Board.
> Description
> -------
> This is what I have so far for the multi user collaboration on review requests. So far, what I have done is allow all reviewers (users in the target people and groups) to be allowed to change the review request. This would all the reviewers to all the normal changing review request functionalities.
> I'm not sure if we have decided who can be able to modify the review request. For now, I put them as all the reviewers.
> Also, I've included the last_updated_by field to the review request model in order to display who has last modified the review request or published new review on the dashboard.
> Manual testing. I've ran the unit tests and had a bunch of errors and one failure. The errors are all expected due to the change of the ReviewRequestDraft.create() function and I need to look into that one failure. I think it's expected failure though.
> I'll go through them all and make modifications/fixes as necessary.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.reviewboard.org/r/3242/ -----------------------------------------------------------
(Updated Aug. 5, 2012, 6:46 p.m.)
Review request for Review Board.
Changes
-------
Updated "Testing Done" details.
Summary (updated)
-----------------
Multi user collaboration on review requests
Description
-------
This is what I have so far for the multi user collaboration on review requests. So far, what I have done is allow all reviewers (users in the target people and groups) to be allowed to change the review request. This would all the reviewers to all the normal changing review request functionalities.
I'm not sure if we have decided who can be able to modify the review request. For now, I put them as all the reviewers.
Also, I've included the last_updated_by field to the review request model in order to display who has last modified the review request or published new review on the dashboard.
Manual testing. At first, I've ran the unit tests and had a bunch of errors and one failure. The errors are all expected due to the change of the ReviewRequestDraft.create() function and I need to look into that one failure. I think it's expected failure though.
As of diff 2, all the test failures/errors have been fixed.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.reviewboard.org/r/3242/#review4190 -----------------------------------------------------------
Would someone be able to kindly give me a brief code review please? :)
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.reviewboard.org/r/3242/ > -----------------------------------------------------------
> (Updated Aug. 5, 2012, 6:46 p.m.)
> Review request for Review Board.
> Description
> -------
> This is what I have so far for the multi user collaboration on review requests. So far, what I have done is allow all reviewers (users in the target people and groups) to be allowed to change the review request. This would all the reviewers to all the normal changing review request functionalities.
> I'm not sure if we have decided who can be able to modify the review request. For now, I put them as all the reviewers.
> Also, I've included the last_updated_by field to the review request model in order to display who has last modified the review request or published new review on the dashboard.
> Manual testing. At first, I've ran the unit tests and had a bunch of errors and one failure. The errors are all expected due to the change of the ReviewRequestDraft.create() function and I need to look into that one failure. I think it's expected failure though.
> As of diff 2, all the test failures/errors have been fixed.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.reviewboard.org/r/3242/#review4197 -----------------------------------------------------------
Hey. This is looking great. As I mention below, we need to figure out what the rules should be for who can modify another user's review requests, because I don't think target reviewers is quite correct. Let's discuss it.
I think we need to figure out what the rules should be for modifying a review request.
I don't think necessarily that every user listed should have the ability (and this query is expensive anyway).
My feeling is that the ability to modify someone else's review request should be pretty limited, and may even be something we want to be opt-in in an organization (or part of an organization).
Perhaps we should have a discussion in IRC about what's most correct here.
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.reviewboard.org/r/3242/ > -----------------------------------------------------------
> (Updated Aug. 5, 2012, 6:46 p.m.)
> Review request for Review Board.
> Description
> -------
> This is what I have so far for the multi user collaboration on review requests. So far, what I have done is allow all reviewers (users in the target people and groups) to be allowed to change the review request. This would all the reviewers to all the normal changing review request functionalities.
> I'm not sure if we have decided who can be able to modify the review request. For now, I put them as all the reviewers.
> Also, I've included the last_updated_by field to the review request model in order to display who has last modified the review request or published new review on the dashboard.
> Manual testing. At first, I've ran the unit tests and had a bunch of errors and one failure. The errors are all expected due to the change of the ReviewRequestDraft.create() function and I need to look into that one failure. I think it's expected failure though.
> As of diff 2, all the test failures/errors have been fixed.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.reviewboard.org/r/3242/ -----------------------------------------------------------
(Updated Sept. 1, 2012, 7:08 p.m.)
Review request for Review Board.
Changes
-------
Modified is_mutable_by method so that it doesn't query for reviewers' groups or people anymore. Basically reverted back to where it was before. When we decide on what type of users can modify the review request, we can modify that method again to accommodate the multi user collaboration.
I still really like the idea of having the idea where the submitter can add a list of other users as owners of the ReviewRequest. I think that is simple and intuitive but may take a bit of work to redo the UI on the ReviewRequest. I love to hear suggestions or feedback! :)
Description
-------
This is what I have so far for the multi user collaboration on review requests. So far, what I have done is allow all reviewers (users in the target people and groups) to be allowed to change the review request. This would all the reviewers to all the normal changing review request functionalities.
I'm not sure if we have decided who can be able to modify the review request. For now, I put them as all the reviewers.
Also, I've included the last_updated_by field to the review request model in order to display who has last modified the review request or published new review on the dashboard.
Manual testing. At first, I've ran the unit tests and had a bunch of errors and one failure. The errors are all expected due to the change of the ReviewRequestDraft.create() function and I need to look into that one failure. I think it's expected failure though.
As of diff 2, all the test failures/errors have been fixed.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.reviewboard.org/r/3242/ -----------------------------------------------------------
(Updated Oct. 7, 2012, 7:28 p.m.)
Review request for Review Board.
Changes
-------
Allowed to change the owner of a ReviewRequest. The field 'Submitter' into the UI is changed to 'Owner'. Refactored some of the JS to use the jQuery autocompleter for the owner field as well. Added the ability of the owner to be able to modify a ReviewRequest.
The submitter is now shown right underneath the summary and beside the created time.
Description
-------
This is what I have so far for the multi user collaboration on review requests. So far, what I have done is allow all reviewers (users in the target people and groups) to be allowed to change the review request. This would all the reviewers to all the normal changing review request functionalities.
I'm not sure if we have decided who can be able to modify the review request. For now, I put them as all the reviewers.
Also, I've included the last_updated_by field to the review request model in order to display who has last modified the review request or published new review on the dashboard.
Manual testing. At first, I've ran the unit tests and had a bunch of errors and one failure. The errors are all expected due to the change of the ReviewRequestDraft.create() function and I need to look into that one failure. I think it's expected failure though.
As of diff 2, all the test failures/errors have been fixed.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.reviewboard.org/r/3242/#review4399 -----------------------------------------------------------
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.reviewboard.org/r/3242/ > -----------------------------------------------------------
> (Updated Oct. 7, 2012, 7:28 p.m.)
> Review request for Review Board.
> Description
> -------
> This is what I have so far for the multi user collaboration on review requests. So far, what I have done is allow all reviewers (users in the target people and groups) to be allowed to change the review request. This would all the reviewers to all the normal changing review request functionalities.
> I'm not sure if we have decided who can be able to modify the review request. For now, I put them as all the reviewers.
> Also, I've included the last_updated_by field to the review request model in order to display who has last modified the review request or published new review on the dashboard.
> Manual testing. At first, I've ran the unit tests and had a bunch of errors and one failure. The errors are all expected due to the change of the ReviewRequestDraft.create() function and I need to look into that one failure. I think it's expected failure though.
> As of diff 2, all the test failures/errors have been fixed.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.reviewboard.org/r/3242/#review4400 -----------------------------------------------------------
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.reviewboard.org/r/3242/ > -----------------------------------------------------------
> (Updated Oct. 7, 2012, 7:28 p.m.)
> Review request for Review Board.
> Description
> -------
> This is what I have so far for the multi user collaboration on review requests. So far, what I have done is allow all reviewers (users in the target people and groups) to be allowed to change the review request. This would all the reviewers to all the normal changing review request functionalities.
> I'm not sure if we have decided who can be able to modify the review request. For now, I put them as all the reviewers.
> Also, I've included the last_updated_by field to the review request model in order to display who has last modified the review request or published new review on the dashboard.
> Manual testing. At first, I've ran the unit tests and had a bunch of errors and one failure. The errors are all expected due to the change of the ReviewRequestDraft.create() function and I need to look into that one failure. I think it's expected failure though.
> As of diff 2, all the test failures/errors have been fixed.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.reviewboard.org/r/3242/#review4444 -----------------------------------------------------------
Spotted a couple things, but it's very close, and I'm hopeful we can get this into 1.7 if you feel good about it.
Remind me, is it just admins that can create drafts, or can users with the can_edit permission also do it?
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.reviewboard.org/r/3242/ > -----------------------------------------------------------
> (Updated Oct. 7, 2012, 7:28 p.m.)
> Review request for Review Board.
> Description
> -------
> This is what I have so far for the multi user collaboration on review requests. So far, what I have done is allow all reviewers (users in the target people and groups) to be allowed to change the review request. This would all the reviewers to all the normal changing review request functionalities.
> I'm not sure if we have decided who can be able to modify the review request. For now, I put them as all the reviewers.
> Also, I've included the last_updated_by field to the review request model in order to display who has last modified the review request or published new review on the dashboard.
> Manual testing. At first, I've ran the unit tests and had a bunch of errors and one failure. The errors are all expected due to the change of the ReviewRequestDraft.create() function and I need to look into that one failure. I think it's expected failure though.
> As of diff 2, all the test failures/errors have been fixed.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.reviewboard.org/r/3242/#review4400 -----------------------------------------------------------
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.reviewboard.org/r/3242/ > -----------------------------------------------------------
> (Updated Oct. 7, 2012, 7:28 p.m.)
> Review request for Review Board.
> Description
> -------
> This is what I have so far for the multi user collaboration on review requests. So far, what I have done is allow all reviewers (users in the target people and groups) to be allowed to change the review request. This would all the reviewers to all the normal changing review request functionalities.
> I'm not sure if we have decided who can be able to modify the review request. For now, I put them as all the reviewers.
> Also, I've included the last_updated_by field to the review request model in order to display who has last modified the review request or published new review on the dashboard.
> Manual testing. At first, I've ran the unit tests and had a bunch of errors and one failure. The errors are all expected due to the change of the ReviewRequestDraft.create() function and I need to look into that one failure. I think it's expected failure though.
> As of diff 2, all the test failures/errors have been fixed.
> On Oct. 26, 2012, 9:44 a.m., Christian Hammond wrote:
> > Spotted a couple things, but it's very close, and I'm hopeful we can get this into 1.7 if you feel good about it.
> > Remind me, is it just admins that can create drafts, or can users with the can_edit permission also do it?
Admins, owners and submitters can edit the review request. It's based on the is_mutable_by function in the ReviewRequest class.
Please let me know if this is not the desired behaviour.
> > Is it worth folding the permission into that variable?
Oops - Apparently perms.reviews.can_edit_reviewrequest is already included in can_edit_review_request. Therefore, I removed "perms.reviews.can_edit_reviewrequest" in if statements that have "can_edit_review_request".
> > This really can just be one evolution. We tend to do one per feature/change.
> > .. And actually, I don't see the other file referenced?
I forgot to add the new file "add_reviewrequest_owner". I merged those two files into one and included it.
- CrystalLokKoo
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.reviewboard.org/r/3242/#review4444 -----------------------------------------------------------
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.reviewboard.org/r/3242/ > -----------------------------------------------------------
> (Updated Oct. 7, 2012, 7:28 p.m.)
> Review request for Review Board.
> Description
> -------
> This is what I have so far for the multi user collaboration on review requests. So far, what I have done is allow all reviewers (users in the target people and groups) to be allowed to change the review request. This would all the reviewers to all the normal changing review request functionalities.
> I'm not sure if we have decided who can be able to modify the review request. For now, I put them as all the reviewers.
> Also, I've included the last_updated_by field to the review request model in order to display who has last modified the review request or published new review on the dashboard.
> Manual testing. At first, I've ran the unit tests and had a bunch of errors and one failure. The errors are all expected due to the change of the ReviewRequestDraft.create() function and I need to look into that one failure. I think it's expected failure though.
> As of diff 2, all the test failures/errors have been fixed.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.reviewboard.org/r/3242/ -----------------------------------------------------------
(Updated Oct. 29, 2012, 12:44 a.m.)
Review request for Review Board.
Changes
-------
Fixed all the minor issues that were bought up during the last code review. Added a missing evolution file. More manual testing done.
Description
-------
This is what I have so far for the multi user collaboration on review requests. So far, what I have done is allow all reviewers (users in the target people and groups) to be allowed to change the review request. This would all the reviewers to all the normal changing review request functionalities.
I'm not sure if we have decided who can be able to modify the review request. For now, I put them as all the reviewers.
Also, I've included the last_updated_by field to the review request model in order to display who has last modified the review request or published new review on the dashboard.
Manual testing. At first, I've ran the unit tests and had a bunch of errors and one failure. The errors are all expected due to the change of the ReviewRequestDraft.create() function and I need to look into that one failure. I think it's expected failure though.
As of diff 2, all the test failures/errors have been fixed.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.reviewboard.org/r/3242/ -----------------------------------------------------------
(Updated Nov. 14, 2012, 10:22 a.m.)
Review request for Review Board.
Changes
-------
Updated testing section.
Description
-------
This is what I have so far for the multi user collaboration on review requests. So far, what I have done is allow all reviewers (users in the target people and groups) to be allowed to change the review request. This would all the reviewers to all the normal changing review request functionalities.
I'm not sure if we have decided who can be able to modify the review request. For now, I put them as all the reviewers.
Also, I've included the last_updated_by field to the review request model in order to display who has last modified the review request or published new review on the dashboard.
Manual testing. At first, I've ran the unit tests and had a bunch of errors and one failure. The errors are all expected due to the change of the ReviewRequestDraft.create() function and I need to look into that one failure. I think it's expected failure though.
As of diff 2, all the test failures/errors have been fixed.
Manual testing include: - Creating new review requests and changing submitter to another account. Permissions to edit the review request behaved correctly for new user
- The original author correctly still has permission.
- Changing the submitter back to the original submitter. Previous submitter has no permission to edit.
- Check Admin. Admin is always able to edit any review request.
- Discarding and undiscarding a review request. - Dealing with previous review request that existed before my change. The submitter field is populated correctly. Edit permissions acting properly before changing submitter. Changed submitter and still acting correctly.
