Changed v.Set validation into our own that actually raises exceptions on missing values.

Added NotReviewedRevisions validator

rhodecode/model/forms.py

@@ -333,8 +333,8 b' def PullRequestForm():'
         org_ref = v.UnicodeString(strip=True, required=True)
         other_repo = v.UnicodeString(strip=True, required=True)
         other_ref = v.UnicodeString(strip=True, required=True)
-        revisions = v.Set(required=True)
-        review_members = v.Set(required=True)
+        revisions = All(v.NotReviewedRevisions()(), v.UniqueList(not_empty=True))
+        review_members = v.UniqueList(not_empty=True)
 
         pullrequest_title = v.UnicodeString(strip=True, required=True, min=3)
         pullrequest_desc = v.UnicodeString(strip=True, required=False)

rhodecode/model/validators.py

@@ -10,17 +10,46 b' from webhelpers.pylonslib.secure_form im'
 
 from formencode.validators import (
     UnicodeString, OneOf, Int, Number, Regex, Email, Bool, StringBoolean, Set,
+    NotEmpty
 )
 from rhodecode.lib.utils import repo_name_slug
-from rhodecode.model.db import RepoGroup, Repository, UsersGroup, User
+from rhodecode.model.db import RepoGroup, Repository, UsersGroup, User,\
+    ChangesetStatus
 from rhodecode.lib.exceptions import LdapImportError
 from rhodecode.config.routing import ADMIN_PREFIX
+
 # silence warnings and pylint
-UnicodeString, OneOf, Int, Number, Regex, Email, Bool, StringBoolean, Set
+UnicodeString, OneOf, Int, Number, Regex, Email, Bool, StringBoolean, Set, \
+    NotEmpty
 
 log = logging.getLogger(__name__)
 
 
+class UniqueList(formencode.FancyValidator):
+    """
+    Unique List !
+    """
+    messages = dict(
+        empty=_('Value cannot be an empty list'),
+        missing_value=_('Value cannot be an empty list'),
+    )
+
+    def _to_python(self, value, state):
+        if isinstance(value, list):
+            return value
+        elif isinstance(value, set):
+            return list(value)
+        elif isinstance(value, tuple):
+            return list(value)
+        elif value is None:
+            return []
+        else:
+            return [value]
+
+    def empty_value(self, value):
+        return []
+
+
 class StateObj(object):
     """
     this is needed to translate the messages using _() in validators
@@ -599,3 +628,33 b' def AttrLoginValidator():'
                 )
 
     return _validator
+
+
+def NotReviewedRevisions():
+    class _validator(formencode.validators.FancyValidator):
+        messages = {
+            'rev_already_reviewed':
+                  _(u'Revisions %(revs)s are already part of pull request '
+                    'or have set status')
+        }
+
+        def validate_python(self, value, state):
+            # check revisions if they are not reviewed, or a part of another
+            # pull request
+            statuses = ChangesetStatus.query()\
+                .filter(ChangesetStatus.revision.in_(value)).all()
+            errors = []
+            for cs in statuses:
+                if cs.pull_request_id:
+                    errors.append(['pull_req', cs.revision[:12]])
+                elif cs.status:
+                    errors.append(['status', cs.revision[:12]])
+
+            if errors:
+                revs = ','.join([x[1] for x in errors])
+                msg = M(self, 'rev_already_reviewed', state, revs=revs)
+                raise formencode.Invalid(msg, value, state,
+                    error_dict=dict(revisions=revs)
+                )
+
+    return _validator