Skip to content

RDISCROWD-7728 ownership id on project create page #1055

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

n00rsy
Copy link

@n00rsy n00rsy commented Mar 20, 2025

*Issue number of the reported bug or feature request: RDISCROWD-7728

Describe your changes

  • add ownership id field to project create field
  • refactor ownership ID validator

image

Related
bloomberg/pybossa-default-theme#488

@coveralls
Copy link

coveralls commented Mar 20, 2025

Pull Request Test Coverage Report for Build 14199405470

Details

  • 26 of 28 (92.86%) changed or added relevant lines in 6 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.005%) to 94.158%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pybossa/repositories/project_repository.py 6 8 75.0%
Totals Coverage Status
Change from base Build 13841964328: -0.005%
Covered Lines: 17632
Relevant Lines: 18726

💛 - Coveralls

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.")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems inconsistent behavior with the error is getting reported via api above and from fronted here as the error messages are different; line above missing Got :<input-number>. It would be nice to have a single place from where a consistent error message is reported back both for api or frontend. This can be achieved via existing similar implementations here

pybossa/util.py Outdated
Comment on lines 376 to 381
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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Is it possible to pass numeric value to this function thereby causing isnumeric call to raise exception
  2. Can this function be refactored so that it returns True only when condition is satisfied and False otherwise
    ex
if not o_id or (isinstance(o_id, str) and o_id.isnumeric() and len(o_id) <= 20):
    return True
return False

@n00rsy n00rsy changed the title RDISCROWD-7728 project RDISCROWD-7728 ownership id on project create page Apr 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants