Skip to content
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

FS-4895: When creating a fund or a round check, whether given round shortnames are available #149

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions .idea/runConfigurations/FAB.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 11 additions & 2 deletions app/blueprints/fund_builder/forms/fund.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,21 @@
from wtforms import RadioField
from wtforms import StringField
from wtforms import TextAreaField
from wtforms.validators import DataRequired
from wtforms.validators import DataRequired, ValidationError
from wtforms.validators import Length

from app.db.models.fund import FundingType
from app.db.queries.fund import get_fund_by_short_name
from app.shared.helpers import no_spaces_between_letters


def validate_unique_fund_short_name(form, field):
if field.data and form.data:
fund_data = get_fund_by_short_name(field.data)
if fund_data and str(fund_data.fund_id) != form.data.get('fund_id'):
raise ValidationError('Given fund short name already exists.')


class GovUkRadioEnumField(RadioField):
source_enum: Enum
gov_uk_choices: list
Expand All @@ -37,7 +45,8 @@ class FundForm(FlaskForm):
name_cy = StringField("Name (Welsh)", description="Leave blank for English-only funds")
title_en = StringField("Title (English)", validators=[DataRequired()])
title_cy = StringField("Title (Welsh)", description="Leave blank for English-only funds")
short_name = StringField("Short name", validators=[DataRequired(), Length(max=10), no_spaces_between_letters])
short_name = StringField("Short name", validators=[DataRequired(), Length(max=10), no_spaces_between_letters,
validate_unique_fund_short_name])
description_en = TextAreaField("Description", validators=[DataRequired()])
description_cy = TextAreaField("Description (Welsh)", description="Leave blank for English-only funds")
welsh_available = RadioField("Welsh available", choices=[("true", "Yes"), ("false", "No")], default="false")
Expand Down
12 changes: 11 additions & 1 deletion app/blueprints/fund_builder/forms/round.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
from wtforms.validators import Length
from wtforms.validators import ValidationError

from app.db.queries.fund import get_fund_by_id
from app.db.queries.round import get_round_by_short_name_and_fund_id
from app.shared.helpers import no_spaces_between_letters


Expand Down Expand Up @@ -58,6 +60,14 @@ def validate_flexible_url(form, field):
raise ValidationError("Invalid URL. Please enter a valid web address.")


def validate_unique_round_short_name(form, field):
if form.data and field.data and form.data.get('fund_id'):
rond_data = get_round_by_short_name_and_fund_id(form.data.get('fund_id'), field.data)
if rond_data and str(rond_data.round_id) != form.data.get('round_id'):
fund_data = get_fund_by_id(form.data.get('fund_id'))
raise ValidationError(f'Given short name already exists in the fund {fund_data.title_json.get("en")}.')


def get_datetime(form_field):
day = int(form_field.day.data)
month = int(form_field.month.data)
Expand Down Expand Up @@ -128,7 +138,7 @@ class RoundForm(FlaskForm):
short_name = StringField(
"Short name",
description="Choose a unique short name with 6 or fewer characters",
validators=[DataRequired(), Length(max=6), no_spaces_between_letters],
validators=[DataRequired(), Length(max=6), no_spaces_between_letters, validate_unique_round_short_name],
)
opens = FormField(DateInputForm, label="Opens")
deadline = FormField(DateInputForm, label="Deadline")
Expand Down
16 changes: 6 additions & 10 deletions app/blueprints/fund_builder/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@
from app.db.queries.application import move_section_down
from app.db.queries.application import move_section_up
from app.db.queries.application import update_section
from app.db.queries.fund import add_fund
from app.db.queries.fund import add_fund, get_fund_by_short_name
from app.db.queries.fund import get_all_funds
from app.db.queries.fund import get_fund_by_id
from app.db.queries.fund import update_fund
from app.db.queries.round import add_round
from app.db.queries.round import add_round, get_round_by_short_name_and_fund_id
from app.db.queries.round import get_round_by_id
from app.db.queries.round import update_round
from app.export_config.generate_all_questions import print_html
Expand Down Expand Up @@ -288,7 +288,6 @@ def fund(fund_id=None):
add_fund(new_fund)
flash(f"Created fund {form.name_en.data}")
return redirect(url_for(BUILD_FUND_BP_DASHBOARD))

error = error_formatter(form.errors)
return render_template("fund.html", form=form, fund_id=fund_id, error=error)

Expand All @@ -303,11 +302,9 @@ def round(round_id=None):
form = RoundForm()
params = {"all_funds": all_funds_as_govuk_select_items(get_all_funds())}
params["selected_fund_id"] = request.form.get("fund_id", None)

if round_id:
round = get_round_by_id(round_id)
form = populate_form_with_round_data(round)

if form.validate_on_submit():
if round_id:
update_existing_round(round, form)
Expand All @@ -316,10 +313,8 @@ def round(round_id=None):
create_new_round(form)
flash(f"Created round {form.title_en.data}")
return redirect(url_for(BUILD_FUND_BP_DASHBOARD))

params["round_id"] = round_id
params["form"] = form

error = error_formatter(params["form"].errors)
return render_template("round.html", **params, error=error)

Expand All @@ -345,6 +340,7 @@ def populate_form_with_round_data(round):
"""
round_data = {
"fund_id": round.fund_id,
"round_id": round.round_id,
"title_en": round.title_json.get("en", ""),
"title_cy": round.title_json.get("cy", ""),
"short_name": round.short_name,
Expand Down Expand Up @@ -399,19 +395,19 @@ def populate_form_with_round_data(round):
"is_feedback_survey_optional": (
"true"
if round.feedback_survey_config
and round.feedback_survey_config.get("is_feedback_survey_optional", "") == "true"
and round.feedback_survey_config.get("is_feedback_survey_optional", "") == "true"
else "false"
),
"is_section_feedback_optional": (
"true"
if round.feedback_survey_config
and round.feedback_survey_config.get("is_section_feedback_optional", "") == "true"
and round.feedback_survey_config.get("is_section_feedback_optional", "") == "true"
else "false"
),
"is_research_survey_optional": (
"true"
if round.feedback_survey_config
and round.feedback_survey_config.get("is_research_survey_optional", "") == "true"
and round.feedback_survey_config.get("is_research_survey_optional", "") == "true"
else "false"
),
"eligibility_config": (
Expand Down
4 changes: 4 additions & 0 deletions app/db/queries/fund.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,7 @@ def get_fund_by_id(id: str) -> Fund:
if not fund:
raise ValueError(f"Fund with id {id} not found")
return fund


def get_fund_by_short_name(short_name: str) -> Fund:
return db.session.query(Fund).filter_by(short_name=short_name).first()
4 changes: 4 additions & 0 deletions app/db/queries/round.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ def get_round_by_id(id: str) -> Round:
return round


def get_round_by_short_name_and_fund_id(fund_id: str, short_name: str) -> Round:
return db.session.query(Round).filter_by(fund_id=fund_id, short_name=short_name).first()


def get_all_rounds() -> list:
stmt = select(Round).order_by(Round.short_name)
return db.session.scalars(stmt).all()
27 changes: 13 additions & 14 deletions app/shared/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,21 +39,20 @@ def get_all_pages_in_parent_form(db, page_id):

return page_ids


# This formatter will read all the errors and then convert them to the required format to support error-summary display
def error_formatter(errors):
error = None
if errors:
errorsList = []
for field, errors in errors.items():
if isinstance(errors, list):
errorsList.extend([{'text': err, 'href': f'#{field}'} for err in errors])
elif isinstance(errors, dict):
# Check if any of the datetime fields have errors
if any(len(errors.get(key, '')) > 0 for key in ['day', 'month', 'years', 'hour', 'minute']):
errorsList.append({'text': "Enter valid datetime", 'href': f'#{field}'})
if errorsList:
error = {'titleText': "There is a problem", 'errorList': errorsList}
return error
# Generate the error list if there are errors
error_list = [
{'text': err, 'href': f'#{field}'}
for field, errs in errors.items()
for err in (errs if isinstance(errs, list) else ['Enter valid datetime']
if isinstance(errs, dict) and any(
len(errs.get(key, '')) > 0 for key in ['day', 'month', 'year', 'hour', 'minute'])
else [])
]
# Return the formatted error structure if there are errors
return {'titleText': "There is a problem", 'errorList': error_list} if error_list else None


# Custom validator to check for spaces between letters
Expand All @@ -62,4 +61,4 @@ def no_spaces_between_letters(form, field):
if not field.data:
return
if re.search(r'\b\w+\s+\w+\b', field.data): # Matches sequences with spaces in between
raise ValidationError("Spaces between letters are not allowed.")
raise ValidationError("Spaces between letters are not allowed.")
92 changes: 92 additions & 0 deletions tests/test_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,38 @@ def test_create_fund(flask_test_client):
assert created_fund.__getattribute__(key) == value


@pytest.mark.usefixtures("set_auth_cookie", "patch_validate_token_rs256_internal_user")
def test_create_fund_with_existing_short_name(flask_test_client):
"""
Tests that a fund can be successfully created using the /fund route
Verifies that the created fund has the correct attributes
"""
create_data = {
"name_en": "New Fund 2",
"title_en": "New Fund Title 2",
"description_en": "New Fund Description 2",
"welsh_available": "false",
"short_name": "SMP1",
"funding_type": FundingType.COMPETITIVE.value,
"ggis_scheme_reference_number": "G1-SCH-0000092415",
}
response = submit_form(flask_test_client, "/fund", create_data)
assert response.status_code == 200
create_data = {
"name_en": "New Fund 3",
"title_en": "New Fund Title 3",
"description_en": "New Fund Description 3",
"welsh_available": "false",
"short_name": "SMP1",
"funding_type": FundingType.COMPETITIVE.value,
"ggis_scheme_reference_number": "G1-SCH-0000092415",
}
response = submit_form(flask_test_client, "/fund", create_data)
assert response.status_code == 200
html = response.data.decode('utf-8')
assert '<a href="#short_name">Given fund short name already exists.</a>' in html, "Not having the fund short name already exists error"


@pytest.mark.usefixtures("set_auth_cookie", "patch_validate_token_rs256_internal_user")
def test_update_fund(flask_test_client, seed_dynamic_data):
"""
Expand Down Expand Up @@ -115,6 +147,66 @@ def test_update_fund(flask_test_client, seed_dynamic_data):
elif key != "submit":
assert updated_fund.__getattribute__(key) == value

@pytest.mark.usefixtures("set_auth_cookie", "patch_validate_token_rs256_internal_user")
def test_create_round_with_existing_short_name(flask_test_client, seed_dynamic_data):
"""
Tests that a round can be successfully created using the /round route
Verifies that the created round has the correct attributes
"""
test_fund = seed_dynamic_data["funds"][0]
new_round_data = {
"fund_id": test_fund.fund_id,
"title_en": "New Round",
"short_name": "NR123",
"opens-day": "01",
"opens-month": "10",
"opens-year": "2024",
"opens-hour": "09",
"opens-minute": "00",
"deadline-day": "01",
"deadline-month": "12",
"deadline-year": "2024",
"deadline-hour": "17",
"deadline-minute": "00",
"assessment_start-day": "02",
"assessment_start-month": "12",
"assessment_start-year": "2024",
"assessment_start-hour": "09",
"assessment_start-minute": "00",
"reminder_date-day": "15",
"reminder_date-month": "11",
"reminder_date-year": "2024",
"reminder_date-hour": "09",
"reminder_date-minute": "00",
"assessment_deadline-day": "15",
"assessment_deadline-month": "12",
"assessment_deadline-year": "2024",
"assessment_deadline-hour": "17",
"assessment_deadline-minute": "00",
"prospectus_link": "http://example.com/prospectus",
"privacy_notice_link": "http://example.com/privacy",
"contact_email": "[email protected]",
"submit": "Submit",
"contact_phone": "1234567890",
"contact_textphone": "0987654321",
"support_times": "9am - 5pm",
"support_days": "Monday to Friday",
"feedback_link": "http://example.com/feedback",
"project_name_field_id": 1,
"guidance_url": "http://example.com/guidance",
}

response = submit_form(flask_test_client, "/round", new_round_data)
assert response.status_code == 200
new_round_data = {**new_round_data, 'short_name': 'NR1234'}
response = submit_form(flask_test_client, "/round", new_round_data)
assert response.status_code == 200
new_round_data = {**new_round_data, 'short_name': 'NR123'}
response = submit_form(flask_test_client, "/round", new_round_data)
assert response.status_code == 200
html = response.data.decode('utf-8')
assert '<a href="#short_name">Given short name already exists in the fund funding to improve testing.</a>' in html, "Not having the fund round short name already exists error"


@pytest.mark.usefixtures("set_auth_cookie", "patch_validate_token_rs256_internal_user")
def test_create_new_round(flask_test_client, seed_dynamic_data):
Expand Down
Loading