-
-
I think perhaps publish() shouldn't do anything if there's no draft and the request is already public. (e.g., don't send a review mail)
-
The JSON call needs these permission and validity checks too, so perhaps you should move them into review_request.publish.
-
The permissions check you see on delete() is for admin/staff privileges. A user with that privilege can delete any review request permanently, whereas normal users can only "discard" their own requests (which still leaves it in the DB). If you go into a user page in the admin interface, you can see the possible permission assignments, but you don't need any of that for normal users. Any user is allowed to publish, but you do need the is_mutable_by call to make sure that they have control over that particular review request.
Create a JSON API for publishing a review request
Review Request #352 — Created April 6, 2008 and submitted
Here's a third patch incorporating the changes you suggested. Yay! -- Okay, I disappeared for 2.5 months. I'm sorry about that. Here's a patch that addresses your comments. Parts of it are almost certainly still not what you want. In particular, the PermissionError thing is maybe really dumb? Let me know and I promise to be more prompt with my next diff! :) -- Right now post-review just uses the standard web form for publishing its reviews. I would like a JSON API instead so that I can use it from my IDE plugin. Possibly at this point it will be clear that I'm monkeying around with the internals of Review Board without understanding much of anything about Django or the other internals of the project. If this patch is really stupid and it's clear I'm way off track, feel free to tell me to just leave it alone and I'll submit a feature request instead. Oh, at the very least, I assume there should be some permission check before the JSON publish() method. The delete() method, for example, looks for the 'reviews.delete_reviewrequest' permission. I tried to figure out how that permission is defined and I failed. I guess it's some Django functionality? I have no idea.
I published some reviews with post-review, the web UI, and my IDEA plugin.
CU
-
-
-
I don't think we want this code like this anymore. I know we had it before but that's from way back before the current draft model we have today. The right thing to do is probably to raise an exception stating that the review request was not dirty, or maybe return None. We should also add a can_publish() function on the model that checks if there's an associated draft. We would check that function before calling publish() in our two callers.
-
-
Sorry for stalling on this review. It's looking good. A few things left to do and it'll be ready to commit!
-
No leading "/" before "api", and no need for the path=. Also, the parameter for the string should align with the string itself.
-
We can probably simplify this to: return get_object_or_none(self.reviewrequestdraft_set) is not None
-
We should probably do something if the draft doesn't exist. Either raise an exception, or just return silently and pretend it worked.
-
This was fine when this was confined to the view, but now since we actually are operating on a Review Request, I think we should go another route. save_draft should now take an optional review request that it can modify instead of fetching one. That way we don't have to do something like: review_request = review_request.publish() Since we're doing this, we can also get rid of the "return saved"