RDISCROWD-7728 ownership id on project create page

pybossa/api/project.py

@@ -30,7 +30,7 @@
 from .api_base import APIBase
 from pybossa.model.project import Project
 from pybossa.cache.categories import get_all as get_categories
-from pybossa.util import is_reserved_name, description_from_long_description, validate_ownership_id
+from pybossa.util import is_reserved_name, description_from_long_description
 from pybossa.core import auditlog_repo, result_repo, http_signer
 from pybossa.auditlogger import AuditLogger
 from pybossa.data_access import ensure_user_assignment_to_project, set_default_amp_store
@@ -149,7 +149,6 @@ def _validate_instance(self, project):
             msg = "Project short_name is not valid, as it's used by the system."
             raise ValueError(msg)
         ensure_user_assignment_to_project(project)
-        validate_ownership_id(project.info.get('ownership_id'))
 
     def _log_changes(self, old_project, new_project):
         auditlogger.add_log_entry(old_project, new_project, current_user)

pybossa/forms/dynamic_forms.py

@@ -1,8 +1,9 @@
+from flask import current_app
 from flask_babel import lazy_gettext
 from flask_wtf import FlaskForm as Form
 from wtforms import SelectField, validators, TextField, BooleanField
 from pybossa.forms.fields.select_two import Select2Field
-from .validator import AmpPvfValidator
+from .validator import AmpPvfValidator, OwnershipIdValidator
 
 import wtforms
 
@@ -38,6 +39,10 @@ def __init__(self, *args, **kwargs):
             lazy_gettext('Annotation Store PVF'),
             [AmpPvfValidator()])
 
+    ProjectFormExtraInputs.ownership_id = TextField(
+        lazy_gettext(current_app.config.get('OWNERSHIP_ID_TITLE', 'Ownership ID')),
+        [OwnershipIdValidator()])
+
     generate_form = ProjectFormExtraInputs(form_data, obj=obj)
     if data_access_levels and not form_data:
         generate_form.amp_store.data = bool(not obj or obj.amp_store)

pybossa/forms/validator.py

@@ -23,7 +23,7 @@
 import re
 import requests
 
-from pybossa.util import is_reserved_name, check_password_strength
+from pybossa.util import is_reserved_name, check_password_strength, valid_ownership_id
 from pybossa.data_access import valid_user_type_based_data_access
 
 
@@ -254,3 +254,17 @@ def __call__(self, form, field):
         amp_store = form.amp_store.data
         if amp_store and not(amp_pvf and self.pvf_format.match(amp_pvf)):
             raise ValidationError("Invalid PVF format. Must contain <PVF name> <PVF val>.")
+
+
+class OwnershipIdValidator(object):
+    """Ensure Ownership ID is valid."""
+    def __init__(self, message=None):
+        self.ownership_id_title = current_app.config.get('OWNERSHIP_ID_TITLE', 'Ownership ID')
+        if not message:
+            message = lazy_gettext(f"Invalid {self.ownership_id_title}.")
+        self.message = message
+
+    def __call__(self, form, field):
+        o_id = form.ownership_id.data
+        if not valid_ownership_id(o_id):
+            raise ValidationError(f"{self.ownership_id_title} must be numeric and less than 20 characters.")

pybossa/repositories/project_repository.py

@@ -32,7 +32,7 @@
 from flask import current_app
 import re
 from functools import reduce
-
+from pybossa.util import valid_ownership_id
 
 class ProjectRepository(Repository):
 
@@ -72,6 +72,7 @@ def save(self, project):
         self._verify_required_fields(project)
         self._verify_product_subproduct(project)
         self._verify_project_info_fields(project)
+        self._verify_ownership_id(project)
         try:
             self.db.session.add(project)
             self.db.session.commit()
@@ -86,6 +87,7 @@ def update(self, project):
         self._verify_has_password(project)
         self._verify_data_classification(project)
         self._verify_annotation_config(project)
+        self._verify_ownership_id(project)
         try:
             self.db.session.merge(project)
             self.db.session.commit()
@@ -215,6 +217,12 @@ def _verify_data_classification(self, project):
         if data_access:
             project.info['data_access'] = [data_access]
 
+    def _verify_ownership_id(self, project):
+        ownership_id = project.info.get('ownership_id')
+        if not valid_ownership_id(ownership_id):
+            ownership_id_title = current_app.config.get('OWNERSHIP_ID_TITLE', 'Ownership ID')
+            raise BadRequest(f"{ownership_id_title} must be numeric and less than 20 characters.")
+
     def _verify_annotation_config(self, project):
         data_access = project.info.get('data_access', [])[0]
         valid_data_access = current_app.config.get('VALID_ACCESS_LEVELS', [])

pybossa/util.py

@@ -373,12 +373,10 @@ def datetime_filter(source, fmt):
     return source.strftime(fmt)
 
 
-def validate_ownership_id(o_id):
-    ownership_id_title = current_app.config.get('OWNERSHIP_ID_TITLE', 'Ownership ID')
-    if o_id == None or len(o_id) == 0:
-        return
-    if not (o_id.isnumeric() and len(o_id) <= 20):
-        raise ValueError(f"{ownership_id_title} must be numeric and less than 20 characters. Got: {o_id}")
+def valid_ownership_id(o_id):
+    if not o_id or (isinstance(o_id, str) and o_id.isnumeric() and len(o_id) <= 20):
+        return True
+    return False
 
 class Pagination(object):
 

pybossa/view/projects.py

@@ -108,7 +108,7 @@
 from copy import deepcopy
 from pybossa.cache import delete_memoized
 from sqlalchemy.orm.attributes import flag_modified
-from pybossa.util import admin_or_project_owner, validate_ownership_id
+from pybossa.util import admin_or_project_owner, valid_ownership_id
 from pybossa.api.project import ProjectAPI
 
 cors_headers = ['Content-Type', 'Authorization']

test/test_forms.py

@@ -168,6 +168,25 @@ def test_amp_pvf_validator_pass(self):
         u = validator.AmpPvfValidator()
         u.__call__(form, form.amp_pvf)
 
+    @with_context
+    @raises(ValidationError)
+    def test_ownership_id_validator_raises_error(self):
+        """Test OwnershipIdValidator raise exception with invalid Ownership ID value """
+        form_data = dict()
+        form = dynamic_project_form(ProjectForm, form_data, [], {}, {})
+        form.ownership_id.data = 'xxx yyy'
+        u = validator.OwnershipIdValidator()
+        u.__call__(form, form.ownership_id)
+
+    @with_context
+    def test_ownership_id_validator_pass(self):
+        """Test OwnershipIdValidator pass with correct format Ownership ID value """
+        form_data = dict()
+        form = dynamic_project_form(ProjectForm, form_data, [], {}, {})
+        form.ownership_id.data = '123123123'
+        u = validator.OwnershipIdValidator()
+        u.__call__(form, form.ownership_id)
+
 
 class TestRegisterForm(Test):
 

test/test_util.py

@@ -1930,19 +1930,19 @@ def test_map_locations_none(self):
         assert mapped_locations['locations'] == expected_locations, (mapped_locations['locations'], expected_locations)
 
     @with_context
-    def test_validate_ownership_id(self):
+    def test_valid_ownership_id(self):
         # valid ownership_id
         ownership_id = "1111"
-        util.validate_ownership_id(ownership_id)
+        assert util.valid_ownership_id(ownership_id) == True
 
         # empty ownership_id
         ownership_id = ""
-        util.validate_ownership_id(ownership_id)
+        assert util.valid_ownership_id(ownership_id) == True
 
         # ownership_id too long (> 20 chars)
         ownership_id = "123412341234123412341234"
-        assert_raises(ValueError, util.validate_ownership_id, ownership_id)
+        assert util.valid_ownership_id(ownership_id) == False
 
         # ownership_id not numeric
         ownership_id = "1234abcd1234"
-        assert_raises(ValueError, util.validate_ownership_id, ownership_id)
+        assert util.valid_ownership_id(ownership_id) == False