From 446c2690978d3950d2a7e998cf6263b29c32c0fc Mon Sep 17 00:00:00 2001 From: nsyed22 Date: Wed, 19 Mar 2025 09:48:46 -0400 Subject: [PATCH 1/3] add ownership id to project create --- pybossa/api/project.py | 6 ++++-- pybossa/forms/dynamic_forms.py | 7 ++++++- pybossa/forms/validator.py | 16 +++++++++++++++- pybossa/themes/default | 2 +- pybossa/util.py | 8 ++++---- pybossa/view/projects.py | 2 +- test/test_forms.py | 19 +++++++++++++++++++ test/test_util.py | 10 +++++----- 8 files changed, 55 insertions(+), 15 deletions(-) diff --git a/pybossa/api/project.py b/pybossa/api/project.py index bb61c7d60a..c89c0cef58 100644 --- a/pybossa/api/project.py +++ b/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, valid_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 @@ -149,7 +149,9 @@ 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')) + if not valid_ownership_id(project.info.get('ownership_id')): + ownership_id_title = current_app.config.get('OWNERSHIP_ID_TITLE', 'Ownership ID') + raise ValueError(f"{ownership_id_title} must be numeric and less than 20 characters.") def _log_changes(self, old_project, new_project): auditlogger.add_log_entry(old_project, new_project, current_user) diff --git a/pybossa/forms/dynamic_forms.py b/pybossa/forms/dynamic_forms.py index ea5719e8f8..e731a871e1 100644 --- a/pybossa/forms/dynamic_forms.py +++ b/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) diff --git a/pybossa/forms/validator.py b/pybossa/forms/validator.py index fb2972bb80..fecd3ad05d 100644 --- a/pybossa/forms/validator.py +++ b/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 .") + + +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. Got: {o_id}") diff --git a/pybossa/themes/default b/pybossa/themes/default index 42195081a6..f24287be54 160000 --- a/pybossa/themes/default +++ b/pybossa/themes/default @@ -1 +1 @@ -Subproject commit 42195081a6bb08ce711d5eea2c63ba73cb37d899 +Subproject commit f24287be54fadee345abcafd371836d563ca9b3d diff --git a/pybossa/util.py b/pybossa/util.py index 28df1e40f6..87c00d0a64 100644 --- a/pybossa/util.py +++ b/pybossa/util.py @@ -373,12 +373,12 @@ 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') +def valid_ownership_id(o_id): if o_id == None or len(o_id) == 0: - return + return True 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}") + return False + return True class Pagination(object): diff --git a/pybossa/view/projects.py b/pybossa/view/projects.py index ee996a17cd..7fa1a19522 100644 --- a/pybossa/view/projects.py +++ b/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'] diff --git a/test/test_forms.py b/test/test_forms.py index c87a4c682b..780ec06440 100644 --- a/test/test_forms.py +++ b/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): diff --git a/test/test_util.py b/test/test_util.py index 0054120a08..b9053c2d5e 100644 --- a/test/test_util.py +++ b/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 From 5b1db8057c70b94ff84516f12e63d4e1dab4c195 Mon Sep 17 00:00:00 2001 From: nsyed22 Date: Mon, 31 Mar 2025 10:33:11 -0400 Subject: [PATCH 2/3] cr changes --- pybossa/api/project.py | 5 +---- pybossa/forms/validator.py | 2 +- pybossa/repositories/project_repository.py | 10 +++++++++- pybossa/util.py | 8 ++++---- 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/pybossa/api/project.py b/pybossa/api/project.py index c89c0cef58..7d653f918b 100644 --- a/pybossa/api/project.py +++ b/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, valid_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,9 +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) - if not valid_ownership_id(project.info.get('ownership_id')): - ownership_id_title = current_app.config.get('OWNERSHIP_ID_TITLE', 'Ownership ID') - raise ValueError(f"{ownership_id_title} must be numeric and less than 20 characters.") def _log_changes(self, old_project, new_project): auditlogger.add_log_entry(old_project, new_project, current_user) diff --git a/pybossa/forms/validator.py b/pybossa/forms/validator.py index fecd3ad05d..6f0bd2eade 100644 --- a/pybossa/forms/validator.py +++ b/pybossa/forms/validator.py @@ -267,4 +267,4 @@ def __init__(self, message=None): 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. Got: {o_id}") + raise ValidationError(f"{self.ownership_id_title} must be numeric and less than 20 characters.") diff --git a/pybossa/repositories/project_repository.py b/pybossa/repositories/project_repository.py index 47852c4d8e..a0d66e026e 100644 --- a/pybossa/repositories/project_repository.py +++ b/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', []) diff --git a/pybossa/util.py b/pybossa/util.py index 87c00d0a64..c648fe9d28 100644 --- a/pybossa/util.py +++ b/pybossa/util.py @@ -374,11 +374,11 @@ def datetime_filter(source, fmt): def valid_ownership_id(o_id): - if o_id == None or len(o_id) == 0: + if not isinstance(o_id, str): + o_id = str(o_id) + if not o_id or (isinstance(o_id, str) and o_id.isnumeric() and len(o_id) <= 20): return True - if not (o_id.isnumeric() and len(o_id) <= 20): - return False - return True + return False class Pagination(object): From fea6f2174aeb56835972b5e8bd2193bd395e44fa Mon Sep 17 00:00:00 2001 From: nsyed22 Date: Tue, 1 Apr 2025 11:15:28 -0400 Subject: [PATCH 3/3] update --- pybossa/util.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/pybossa/util.py b/pybossa/util.py index c648fe9d28..943a27af0d 100644 --- a/pybossa/util.py +++ b/pybossa/util.py @@ -374,8 +374,6 @@ def datetime_filter(source, fmt): def valid_ownership_id(o_id): - if not isinstance(o_id, str): - o_id = str(o_id) if not o_id or (isinstance(o_id, str) and o_id.isnumeric() and len(o_id) <= 20): return True return False