RDISCROWD-7088 Ownership ID v2 (#980)

* update ownership_id handling

* add test

* update test

* update test

* update themes

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
+from pybossa.util import is_reserved_name, description_from_long_description, validate_ownership_id
 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
@@ -147,6 +147,7 @@ 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/util.py

@@ -368,10 +368,12 @@ def datetime_filter(source, fmt):
     return source.strftime(fmt)
 
 
-def validate_ownership_id(ownership_id):
-    if ownership_id == None or len(ownership_id) == 0:
-        return True
-    return ownership_id.isnumeric() and len(ownership_id) <= 20
+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}")
 
 class Pagination(object):
 

pybossa/view/projects.py

@@ -3513,35 +3513,6 @@ def del_coowner(short_name, user_name=None):
     return abort(404)
 
 
-@blueprint.route('/<short_name>/ownership_id', methods=['GET', 'PUT'])
-@login_required
-def ownership_id(short_name):
-    """Manage project project ownership identifer."""
-    project, owner, ps = project_by_shortname(short_name)
-    response = {'title': current_app.config.get('OWNERSHIP_ID_TITLE', 'Ownership ID')}
-
-    if request.method == 'GET':
-        ensure_authorized_to('read', project)
-        response['ownership_id'] = project.info.get('ownership_id', None)
-
-    if request.method == 'PUT':
-        ensure_authorized_to('update', project)
-        data = request.get_json()
-        new_ownership_id = data.get("ownership_id")
-        if not validate_ownership_id(new_ownership_id):
-            flash(gettext('Ownership ID must be numeric and less than 20 characters!'), 'error')
-            handle_content_type(response)
-        old_ownership_id = project.info.get('ownership_id', None)
-        if new_ownership_id != old_ownership_id:
-            auditlogger.log_event(project, current_user, 'update', 'project.ownership_id',
-                old_ownership_id, new_ownership_id)
-            project.info['ownership_id'] = new_ownership_id
-            project_repo.save(project)
-        response['ownership_id'] = new_ownership_id
-        flash(gettext('Ownership ID updated successfully'), 'success')
-
-    return handle_content_type(response)
-
 @blueprint.route('/<short_name>/projectreport/export', methods=['GET', 'POST'])
 @login_required
 @admin_or_subadmin_required

test/test_util.py

@@ -1898,3 +1898,21 @@ def test_map_locations_none(self):
         mapped_locations = util.map_locations(input_locations)
 
         assert mapped_locations['locations'] == expected_locations
+
+    @with_context
+    def test_validate_ownership_id(self):
+        # valid ownership_id
+        ownership_id = "1111"
+        util.validate_ownership_id(ownership_id)
+
+        # empty ownership_id
+        ownership_id = ""
+        util.validate_ownership_id(ownership_id)
+
+        # ownership_id too long (> 20 chars)
+        ownership_id = "123412341234123412341234"
+        assert_raises(ValueError, util.validate_ownership_id, ownership_id)
+
+        # ownership_id not numeric
+        ownership_id = "1234abcd1234"
+        assert_raises(ValueError, util.validate_ownership_id, ownership_id)