Review Board

beta

Disallow creating review requests against other users' changesets

Updated 1 year, 7 months ago

David Trowbridge Reviewers
trunk reviewboard
35
None Review Board SVN 
At the moment, it's possible to create a review request against
any changeset (with the perforce backend).  If one person
creates a review against another person's changeset number, that
will then prevent the owning user from creating a request.

Since perforce changelists include usernames, it's not hard to
parse this out before we create anything in the database and
check it against the logged-in user.

Part of this change also fixes a bug in the "New Review Request"
form where the selected repository was not being marked as such
if there was an error.
Tried creating a review request against public.perforce.com with
a random change number and my local user ("david").  Got expected
error message.
Christian Hammond
Posted 1 year, 7 months ago (May 28th, 2007, 4:56 p.m.)
Looks good. A few things, though.
Loading diff fragment... 
We should probably check that there actually is a changeset username set to compare against. Right now we have Perforce support for this, but what about SVN/CVS?

We also have a potential future issue where account usernames won't patch changeset usernames. At that point, we might want to provide some repository auth table with a foreign key to the repository, another to the user, and a username.
  1. David Trowbridge 1 year, 7 months ago (May 28th, 2007, 6:42 p.m.) 
    You mean "match changeset usernames"?  I think for now,
it's safe to assume that someone's perforce username
will match their NIS/LDAP username.  We can cross that
bridge later if anyone comes to it ;)
  2. Christian Hammond 1 year, 7 months ago (May 28th, 2007, 6:52 p.m.) 
    Fair enough. Ship it.
Loading diff fragment... 
"The selected file..." is nicer to read.

Also, this does not end in a period, whereas the other error does.