From 888e4451dfcb77703aab2c5d24c7a04a63576e5b Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Apr 07 2017 15:34:45 +0000
Subject: Use a validator to keep the model in consistent with the alembic migration


This way we ensure the token_id cannot be null in the software level
since we cannot do that at the DB level.

We cannot do it at the DB level since for existing entries in the DB we
cannot find back what was the token_id. So there are two ways:
- have the model define nullable=False and the migration nullable=True
- be consistent between the model and the migration and enforce
  nullable=False for new entries in the software.

This commit applies the second option.

---

diff --git a/pagure/lib/model.py b/pagure/lib/model.py
index d258c36..dc8ecbd 100644
--- a/pagure/lib/model.py
+++ b/pagure/lib/model.py
@@ -27,6 +27,7 @@ from sqlalchemy.orm import backref
 from sqlalchemy.orm import sessionmaker
 from sqlalchemy.orm import scoped_session
 from sqlalchemy.orm import relation
+from sqlalchemy.orm import validates
 
 
 CONVENTION = {
@@ -1709,7 +1710,7 @@ class PullRequestFlag(BASE):
         sa.String(64), sa.ForeignKey(
             'tokens.id',
         ),
-        nullable=False)
+        nullable=True)
     user_id = sa.Column(
         sa.Integer,
         sa.ForeignKey(
@@ -1747,6 +1748,11 @@ class PullRequestFlag(BASE):
         foreign_keys=[pull_request_uid],
         remote_side=[PullRequest.uid])
 
+    @validates('token_id')
+    def validate_token_id(self, _, token_id):
+        assert token_id is not None
+        return token_id
+
     def to_json(self, public=False):
         ''' Returns a dictionnary representation of the pull-request.
 
diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py
index 42b0447..e5fae2a 100644
--- a/tests/test_pagure_lib.py
+++ b/tests/test_pagure_lib.py
@@ -2060,6 +2060,35 @@ class PagureLibtests(tests.Modeltests):
         self.assertEqual(len(request.flags), 1)
         self.assertEqual(request.flags[0].token_id, 'aaabbbcccddd')
 
+    @patch('pagure.lib.notify.send_email')
+    def test_add_pull_request_flag_no_token(self, mockemail):
+        """ Test add_pull_request_flag of pagure.lib. """
+        mockemail.return_value = True
+
+        self.test_new_pull_request()
+        tests.create_tokens(self.session)
+
+        request = pagure.lib.search_pull_requests(self.session, requestid=1)
+        self.assertEqual(len(request.flags), 0)
+
+        self.assertRaises(
+            AssertionError,
+            pagure.lib.add_pull_request_flag,
+            session=self.session,
+            request=request,
+            username="jenkins",
+            percent=100,
+            comment="Build passes",
+            url="http://jenkins.cloud.fedoraproject.org",
+            uid="jenkins_build_pagure_34",
+            user='foo',
+            token=None,
+            requestfolder=None,
+        )
+
+        request = pagure.lib.search_pull_requests(self.session, requestid=1)
+        self.assertEqual(len(request.flags), 0)
+
     def test_search_pull_requests(self):
         """ Test search_pull_requests of pagure.lib. """