From 7fa514bf5738a17b4fe0dd4b2a3311c74bfd5dee Mon Sep 17 00:00:00 2001
From: Nuwan Samarasinghe <nuwansamarasinghe100@gmail.com>
Date: Mon, 16 Dec 2024 16:14:53 +0000
Subject: [PATCH] addressing review comment

---
 .idea/runConfigurations/FAB.xml            |   6 +-
 app/blueprints/fund_builder/forms/fund.py  |  13 ++-
 app/blueprints/fund_builder/forms/round.py |  12 ++-
 app/blueprints/fund_builder/routes.py      | 101 +++++++++------------
 app/shared/helpers.py                      |   6 +-
 5 files changed, 70 insertions(+), 68 deletions(-)

diff --git a/.idea/runConfigurations/FAB.xml b/.idea/runConfigurations/FAB.xml
index d9a69d0..35781f8 100644
--- a/.idea/runConfigurations/FAB.xml
+++ b/.idea/runConfigurations/FAB.xml
@@ -5,12 +5,14 @@
     <option name="INTERPRETER_OPTIONS" value="" />
     <option name="PARENT_ENVS" value="true" />
     <envs>
+      <env name="PYTHONUNBUFFERED" value="1" />
+      <env name="AUTHENTICATOR_HOST" value="https://authenticator.levellingup.gov.localhost:4004" />
       <env name="DATABASE_URL" value="postgresql://postgres:password@localhost:5432/fab_store" />
       <env name="FLASK_DEBUG" value="0" />
       <env name="FLASK_ENV" value="development" />
       <env name="FORM_RUNNER_EXTERNAL_HOST" value="http://localhost:3009" />
       <env name="FORM_RUNNER_INTERNAL_HOST" value="http://localhost:3009" />
-      <env name="PYTHONUNBUFFERED" value="1" />
+      <env name="RSA256_PUBLIC_KEY_BASE64" value="&quot;LS0tLS1CRUdJTiBQVUJMSUMgS0VZLS0tLS0KTUlHZU1BMEdDU3FHU0liM0RRRUJBUVVBQTRHTUFEQ0JpQUtCZ0hHYnRGMXlWR1crckNBRk9JZGFrVVZ3Q2Z1dgp4SEUzOGxFL2kwS1dwTXdkU0haRkZMWW5IakJWT09oMTVFaWl6WXphNEZUSlRNdkwyRTRRckxwcVlqNktFNnR2CkhyaHlQL041ZnlwU3p0OHZDajlzcFo4KzBrRnVjVzl6eU1rUHVEaXNZdG1rV0dkeEJta2QzZ3RZcDNtT0k1M1YKVkRnS2J0b0lGVTNzSWs1TkFnTUJBQUU9Ci0tLS0tRU5EIFBVQkxJQyBLRVktLS0tLQ==&quot;" />
       <env name="SECRET_KEY" value="local" />
     </envs>
     <option name="SDK_HOME" value="" />
@@ -21,7 +23,7 @@
     <option name="ADD_SOURCE_ROOTS" value="true" />
     <EXTENSION ID="PythonCoverageRunConfigurationExtension" runner="coverage.py" />
     <option name="SCRIPT_NAME" value="flask" />
-    <option name="PARAMETERS" value="run --port 3020 --cert=$PROJECT_DIR$/../../../../fund_projects/funding-service-design-docker-runner/certs/cert.pem --key=$PROJECT_DIR$/../../../../fund_projects/funding-service-design-docker-runner/certs/key.pem" />
+    <option name="PARAMETERS" value="run --port 3011 --cert=$PROJECT_DIR$/../../certs/cert.pem --key=$PROJECT_DIR$/../../certs/key.pem" />
     <option name="SHOW_COMMAND_LINE" value="false" />
     <option name="EMULATE_TERMINAL" value="false" />
     <option name="MODULE_MODE" value="true" />
diff --git a/app/blueprints/fund_builder/forms/fund.py b/app/blueprints/fund_builder/forms/fund.py
index 8ffc31c..e60988e 100644
--- a/app/blueprints/fund_builder/forms/fund.py
+++ b/app/blueprints/fund_builder/forms/fund.py
@@ -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
@@ -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")
diff --git a/app/blueprints/fund_builder/forms/round.py b/app/blueprints/fund_builder/forms/round.py
index a9d0f1b..f8c1f07 100644
--- a/app/blueprints/fund_builder/forms/round.py
+++ b/app/blueprints/fund_builder/forms/round.py
@@ -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
 
 
@@ -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)
@@ -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")
diff --git a/app/blueprints/fund_builder/routes.py b/app/blueprints/fund_builder/routes.py
index 95e79bc..32b9750 100644
--- a/app/blueprints/fund_builder/routes.py
+++ b/app/blueprints/fund_builder/routes.py
@@ -254,49 +254,41 @@ def fund(fund_id=None):
     else:
         form = FundForm()
 
-    error = {}
     if form.validate_on_submit():
-        is_short_name_available = False
-        if form.data and form.data['short_name']:
-            fund_data = get_fund_by_short_name(form.data['short_name'])
-            is_short_name_available = fund_data and str(fund_data.fund_id) != fund_id
-        if not is_short_name_available:
-            if fund_id:
-                fund.name_json["en"] = form.name_en.data
-                fund.name_json["cy"] = form.name_cy.data
-                fund.title_json["en"] = form.title_en.data
-                fund.title_json["cy"] = form.title_cy.data
-                fund.description_json["en"] = form.description_en.data
-                fund.description_json["cy"] = form.description_cy.data
-                fund.welsh_available = form.welsh_available.data == "true"
-                fund.short_name = form.short_name.data
-                fund.audit_info = {"user": "dummy_user", "timestamp": datetime.now().isoformat(), "action": "update"}
-                fund.funding_type = form.funding_type.data
-                fund.ggis_scheme_reference_number = (
-                    form.ggis_scheme_reference_number.data if form.ggis_scheme_reference_number.data else ""
-                )
-                update_fund(fund)
-                flash(f"Updated fund {form.title_en.data}")
-                return redirect(url_for("build_fund_bp.view_fund", fund_id=fund.fund_id))
-
-            new_fund = Fund(
-                name_json={"en": form.name_en.data},
-                title_json={"en": form.title_en.data},
-                description_json={"en": form.description_en.data},
-                welsh_available=form.welsh_available.data == "true",
-                short_name=form.short_name.data,
-                audit_info={"user": "dummy_user", "timestamp": datetime.now().isoformat(), "action": "create"},
-                funding_type=FundingType(form.funding_type.data),
-                ggis_scheme_reference_number=(
-                    form.ggis_scheme_reference_number.data if form.ggis_scheme_reference_number.data else ""
-                ),
+        if fund_id:
+            fund.name_json["en"] = form.name_en.data
+            fund.name_json["cy"] = form.name_cy.data
+            fund.title_json["en"] = form.title_en.data
+            fund.title_json["cy"] = form.title_cy.data
+            fund.description_json["en"] = form.description_en.data
+            fund.description_json["cy"] = form.description_cy.data
+            fund.welsh_available = form.welsh_available.data == "true"
+            fund.short_name = form.short_name.data
+            fund.audit_info = {"user": "dummy_user", "timestamp": datetime.now().isoformat(), "action": "update"}
+            fund.funding_type = form.funding_type.data
+            fund.ggis_scheme_reference_number = (
+                form.ggis_scheme_reference_number.data if form.ggis_scheme_reference_number.data else ""
             )
-            add_fund(new_fund)
-            flash(f"Created fund {form.name_en.data}")
-            return redirect(url_for(BUILD_FUND_BP_DASHBOARD))
-        error = {**error,
-                 'short_name': [f'Given fund short name already exists.']}
-    error = error_formatter(form.errors, error)
+            update_fund(fund)
+            flash(f"Updated fund {form.title_en.data}")
+            return redirect(url_for("build_fund_bp.view_fund", fund_id=fund.fund_id))
+
+        new_fund = Fund(
+            name_json={"en": form.name_en.data},
+            title_json={"en": form.title_en.data},
+            description_json={"en": form.description_en.data},
+            welsh_available=form.welsh_available.data == "true",
+            short_name=form.short_name.data,
+            audit_info={"user": "dummy_user", "timestamp": datetime.now().isoformat(), "action": "create"},
+            funding_type=FundingType(form.funding_type.data),
+            ggis_scheme_reference_number=(
+                form.ggis_scheme_reference_number.data if form.ggis_scheme_reference_number.data else ""
+            ),
+        )
+        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)
 
 
@@ -310,30 +302,20 @@ 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)
-    error = {}
     if round_id:
         round = get_round_by_id(round_id)
         form = populate_form_with_round_data(round)
     if form.validate_on_submit():
-        is_short_name_available = False
-        if form.data and form.data['short_name'] and form.data['fund_id']:
-            rond_data = get_round_by_short_name_and_fund_id(form.data['fund_id'],
-                                                            form.data['short_name'])
-            is_short_name_available = rond_data and str(rond_data.round_id) != round_id
-        if not is_short_name_available:
-            if round_id:
-                update_existing_round(round, form)
-                flash(f"Updated round {round.title_json['en']}")
-                return redirect(url_for("build_fund_bp.view_fund", fund_id=round.fund_id))
-            create_new_round(form)
-            flash(f"Created round {form.title_en.data}")
-            return redirect(url_for(BUILD_FUND_BP_DASHBOARD))
-        fund = get_fund_by_id(form.data['fund_id'])
-        error = {**error,
-                 'short_name': [f'Given short name already exists in the fund {fund.title_json.get("en")}.']}
+        if round_id:
+            update_existing_round(round, form)
+            flash(f"Updated round {round.title_json['en']}")
+            return redirect(url_for("build_fund_bp.view_fund", fund_id=round.fund_id))
+        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, error)
+    error = error_formatter(params["form"].errors)
     return render_template("round.html", **params, error=error)
 
 
@@ -358,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,
diff --git a/app/shared/helpers.py b/app/shared/helpers.py
index 886b16e..5994616 100644
--- a/app/shared/helpers.py
+++ b/app/shared/helpers.py
@@ -41,13 +41,11 @@ def get_all_pages_in_parent_form(db, page_id):
 
 
 # This formatter will read all the errors and then convert them to the required format to support error-summary display
-def error_formatter(errors, custom_errors=None):
-    # Merge errors with custom_errors if provided
-    merged_errors = {**errors, **(custom_errors or {})}
+def error_formatter(errors):
     # Generate the error list if there are errors
     error_list = [
         {'text': err, 'href': f'#{field}'}
-        for field, errs in merged_errors.items()
+        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'])